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

How to handle Microsoft.Data.SqlClient new AccessTokenCallback #33205

Open
kevinharing opened this issue Feb 29, 2024 · 3 comments
Open

How to handle Microsoft.Data.SqlClient new AccessTokenCallback #33205

kevinharing opened this issue Feb 29, 2024 · 3 comments

Comments

@kevinharing
Copy link

kevinharing commented Feb 29, 2024

In version 5.2.0 of Microsoft.Data.SqlClient a new callback was added to the SqlConnection class in order to retrieve an access token. What is the best place to set this property. I don’t think it’s necessary to set it on each execution of the IDbConnection interceptor like I did before with the AccessToken property, right?

More info on the new functionality here.

Previous:

public override async ValueTask<InterceptionResult> ConnectionOpeningAsync(DbConnection connection, ConnectionEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default)
{
    var sqlConnection = (SqlConnection)connection;
    var accessToken = await _credential.GetTokenAsync(AzureSqlScope, cancellationToken: cancellationToken);

    sqlConnection.AccessToken = accessToken.Token;

    return ValueTask.FromResult(result);
}

New:

private readonly Func<SqlAuthenticationParameters, CancellationToken, Task<SqlAuthenticationToken>> _accessTokenCallback;

public AzureAdAuthenticationDbConnectionInterceptor()
{
    _accessTokenCallback = AccessTokenCallback;
}

public override ValueTask<InterceptionResult> ConnectionOpeningAsync(DbConnection connection, ConnectionEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default)
{
    if (connection is SqlConnection sqlConnection && sqlConnection.AccessTokenCallback is null)
    {
        sqlConnection.AccessTokenCallback = _accessTokenCallback;
    }

    return ValueTask.FromResult(result);
}

private async Task<SqlAuthenticationToken> AccessTokenCallback(SqlAuthenticationParameters sqlAuthenticationParameters, CancellationToken cancellationToken)
{
    var accessToken = await _credential.GetTokenAsync(AzureSqlScope, cancellationToken: cancellationToken);

    return new SqlAuthenticationToken(accessToken.Token, accessToken.ExpiresOn);
}
@ajcvickers
Copy link
Member

Note for team triage: looks like we may want to add some sugar here, especially for cases where the EF is creating the SqlConnection object.

@ajcvickers
Copy link
Member

@kevinharing What you have looks fine, but given that setting the callback doesn't need to be done asynchronously, and if it only needs to be done when EF creates the SqlConnection (i.e. you never create one and pass it to EF, or, if you do, it already has the callback set), then using ConnectionCreated might be better. For example:

public class Interceptor(
    Func<SqlAuthenticationParameters, CancellationToken, Task<SqlAuthenticationToken>> accessTokenCallback)
    : DbConnectionInterceptor
{
    public override DbConnection ConnectionCreated(ConnectionCreatedEventData eventData, DbConnection result)
    {
        ((SqlConnection)result).AccessTokenCallback = accessTokenCallback;
        
        return result;
    }
}

Can you let me know what you end up with and how it is working out for you to help drive design for the sugar?

@kevinharing
Copy link
Author

@kevinharing What you have looks fine, but given that setting the callback doesn't need to be done asynchronously, and if it only needs to be done when EF creates the SqlConnection (i.e. you never create one and pass it to EF, or, if you do, it already has the callback set), then using ConnectionCreated might be better. For example:

public class Interceptor(
    Func<SqlAuthenticationParameters, CancellationToken, Task<SqlAuthenticationToken>> accessTokenCallback)
    : DbConnectionInterceptor
{
    public override DbConnection ConnectionCreated(ConnectionCreatedEventData eventData, DbConnection result)
    {
        ((SqlConnection)result).AccessTokenCallback = accessTokenCallback;
        
        return result;
    }
}

Can you let me know what you end up with and how it is working out for you to help drive design for the sugar?

Thank you for your reply. That sounds indeed as a better option. Will test this out soon.

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