Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support AccessTokenCallback (SqlClient 5.2) #147

Closed
rayao opened this issue May 22, 2024 · 20 comments
Closed

Support AccessTokenCallback (SqlClient 5.2) #147

rayao opened this issue May 22, 2024 · 20 comments

Comments

@rayao
Copy link

rayao commented May 22, 2024

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
Copy link
Owner

ErikEJ commented May 22, 2024

Are the original SqlProviderServices inheritable?

@ErikEJ
Copy link
Owner

ErikEJ commented May 22, 2024

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

@rayao
Copy link
Author

rayao commented May 22, 2024

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
Copy link
Owner

ErikEJ commented May 22, 2024

@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
Copy link
Author

rayao commented May 22, 2024

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

  • Create a DbContext with an open SqlConnection, the SqlConnection has AccessTokenCallback set.
  • Create a DbMigrator (EF6 built-in class) with the DbContext.
  • Call DbMigrator.Update
    • Internally it'll clone the SqlConnection within DbContext, and use the new connection to run migration SQL statements.
    • But since the cloned SqlConnection doesn't have AccessTokenCallback copied (maybe bug in Microsoft.Data.SqlClient, or maybe by design), so the connection can't successfully open, it throws with authentication failure.

@rayao
Copy link
Author

rayao commented May 22, 2024

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
Copy link
Owner

ErikEJ commented May 22, 2024

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

@rayao
Copy link
Author

rayao commented May 22, 2024

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
Copy link
Owner

ErikEJ commented May 22, 2024

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
Copy link
Author

rayao commented May 22, 2024

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
Copy link
Owner

ErikEJ commented May 22, 2024

Got it, now that you are telling me! 😄

Did this work with System.Data.SqlClient?

@rayao
Copy link
Author

rayao commented May 22, 2024

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

@ErikEJ
Copy link
Owner

ErikEJ commented May 22, 2024

As far as I can tell, an overload was added to DbMigrator to avoid cloning: dotnet#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
Copy link
Author

rayao commented May 22, 2024

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
Copy link
Owner

ErikEJ commented May 22, 2024

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
Copy link
Author

rayao commented May 22, 2024

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
Copy link
Owner

ErikEJ commented May 22, 2024

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

@rayao
Copy link
Author

rayao commented May 22, 2024

Anyway, thanks Erik. 😊

@ErikEJ
Copy link
Owner

ErikEJ commented May 22, 2024

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

@rayao
Copy link
Author

rayao commented May 22, 2024

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

@ErikEJ ErikEJ closed this as completed May 22, 2024
@ErikEJ ErikEJ changed the title MicrosoftSqlProviderServices is sealed Support AccessTokenCallback May 23, 2024
@ErikEJ ErikEJ changed the title Support AccessTokenCallback Support AccessTokenCallback (SqlClien 5.2) May 23, 2024
@ErikEJ ErikEJ changed the title Support AccessTokenCallback (SqlClien 5.2) Support AccessTokenCallback (SqlClient 5.2) May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants