ErikEJ / EntityFramework6PowerTools

This is the codebase for Entity Framework 6 Power Tools Community Edition, and modern EF 6 providers for SQL Server and SQL Server Compact
Other
183 stars 27 forks source link

Support AccessTokenCallback (SqlClient 5.2) #147

Closed rayao closed 1 month ago

rayao commented 1 month ago

Is it possible to make MicrosoftSqlProviderServices inheritable? I only want to override a single method CloneDbConnection, now SqlConnection.Clone won't copy AccessTokenCallback to new connection, this blocks our "disable local auth" movement.

ErikEJ commented 1 month ago

Are the original SqlProviderServices inheritable?

ErikEJ commented 1 month ago

@rayao Also, please share some code sample of what you are trying to do?

rayao commented 1 month ago

Thanks Erik, I know the original SqlProviderServices is also sealed, but do we need to strictly follow that?

The code is in EF6 itself. When I call DbMigrator.Update, it internally clones a DbConnection and uses that connection to execute SQL statements.

    private DbConnection CreateConnection()
    {
        DbConnection dbConnection = ((_connection == null) ? _providerFactory.CreateConnection() : 
            DbProviderServices.GetProviderServices(_connection).CloneDbConnection(_connection, _providerFactory));
        DbConnectionPropertyInterceptionContext<string> dbConnectionPropertyInterceptionContext = new DbConnectionPropertyInterceptionContext<string>().WithValue(_usersContextInfo.ConnectionString);
        if (_usersContext != null)
        {
            dbConnectionPropertyInterceptionContext = dbConnectionPropertyInterceptionContext.WithDbContext(_usersContext);
        }

        DbInterception.Dispatch.Connection.SetConnectionString(dbConnection, dbConnectionPropertyInterceptionContext);
        return dbConnection;
    }

BTW, inherit DbMigrator is a dead end too, all important virtual methods are internal.

ErikEJ commented 1 month ago

@rayao I am stiil not sure what you are trying to achieve, could you please create a small runnable console app that demonstrates it?

rayao commented 1 month ago

It's a little bit complicated because it only repro when applying DbModel schema migration. Basically,

rayao commented 1 month ago

This apparently is not a bug in ErikEJ.EF6 package, might be a bug in Microsoft.Data.SqlClient, I just want to override MicrosoftSqlProviderServices .CloneDbConnection to easily work around it.

ErikEJ commented 1 month ago

Can you not just use the built in connection string options for Entra auth? (and use another connection string for local scenarios) ?

rayao commented 1 month ago

I think I don't fully understand your question, our ultimate goal is to disable local auth (with User & Password) in Azure SQL, so I must find a way to let DbMigrator to use a SqlConnection with AccessTokenCallback properly set.

ErikEJ commented 1 month ago

I am referring to this, which removes the need for AccessTokenCallback - but I might be misunderstanding.

https://learn.microsoft.com/en-us/sql/connect/ado-net/sql/azure-active-directory-authentication?view=sql-server-ver16

rayao commented 1 month ago

What we use is a CertificateAssertionCredential, I don't find a way to set it in connection string directly, so eventually I use AccessTokenCallback. If you have any suggestion, please enlighten me.

ErikEJ commented 1 month ago

Got it, now that you are telling me! 😄

Did this work with System.Data.SqlClient?

rayao commented 1 month ago

No, System.Data.SqlClient doesn't even have AccessTokenCallback, many thanks to this great package from you. 😊

ErikEJ commented 1 month ago

As far as I can tell, an overload was added to DbMigrator to avoid cloning: https://github.com/dotnet/ef6/issues/522

/// <summary>
/// Initializes a new instance of the DbMigrator class using the supplied context.
/// Use this constructor when applying migrations from code to avoid having migrations attempt
/// to create a context/connection for you.
/// </summary>
/// <param name="configuration"> Configuration to be used for the migration process. </param>
/// <param name="context"> The <see cref="DbContext"/> to use. </param>

[SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling")]
[SuppressMessage("Microsoft.Usage", "CA2214:DoNotCallOverridableMethodsInConstructors")]
public DbMigrator(DbMigrationsConfiguration configuration, DbContext context)
    : this(configuration, context, DatabaseExistenceState.Unknown, calledByCreateDatabase: false)
{
}
rayao commented 1 month ago

Yes I give it a DbContext, but that only avoids master connection creation, when applying migration, a connection will still be created anyway. You can see internal void ExecuteStatements(IEnumerable<MigrationStatement> migrationStatements, DbTransaction existingTransaction) when existingTransaction is null. And there's no way to give it a non-null transaction, all related virtual methods are internal.

ErikEJ commented 1 month ago

OK, it looks like the fix was not complete, so you are facing a EF6 bug - and it will never be fixed 😢

Probably your best option is to use migration scripts instead.

rayao commented 1 month ago

How to generate a migration script? 1 script per migration? We have many DBs (1 per user), every has different migration history, and every has its own schema name, is there a way to dynamically change schema name in those scripts?

ErikEJ commented 1 month ago

No idea, I do not use EF6 any more 😆 - and I do not use migrations....

rayao commented 1 month ago

Anyway, thanks Erik. 😊

ErikEJ commented 1 month ago

You could try to create a PR with the minimal changes required for your use case, and I will have a look.

rayao commented 1 month ago

@ErikEJ I created 3 PRs, to me maybe the "unseal" one has least impact, please leave your comments.

148 #149 #150