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

Ability to register IInterceptor without an IDbContextOptionsExtension #21578

Open
mderriey opened this issue Jul 10, 2020 · 9 comments
Open

Comments

@mderriey
Copy link

mderriey commented Jul 10, 2020

I want to use Azure Managed Identities to connect to an Azure SQL instance.

I'm trying to use a DbConnectionInterceptor to handle the ConnectionOpeningAsync "event" to use the Azure.Identity SDK to grab a token and give it to the SqlConnection instance.

I'd like some services to be injected into the interceptor, but unfortunately today the DbContextOptionsBuilder.AddInterceptors method accepts constructed instances only.

I tried registering interceptors in the application service provider, but they're not picked up by EF Core.

While reverse-engineering how the AddInterceptors method works, I found a solution, but it's quite heavy:

  • Need to create a class that implements IDbContextOptionsExtension
  • Need to create a class that derives from DbContextOptionsExtensionInfo, which abstract members are not super obvious in how they need to be impemented
  • ((IDbContextOptionsBuilderInfrastructure)options).AddOrUpdateExtension(new MyExtension()) as options.Options.WithExtension(new MyExtension()) doesn't seem to work

I'd love for that process to be easier, along the lines of:

services.AddDbContext<MyContext>(options =>
{
    options.UseSqlServer("<my-connection-string");
    options.RegisterInterceptor<MyInterceptor>().
});
@ajcvickers
Copy link
Member

ajcvickers commented Jul 10, 2020

@mderriey Agreed that this would be a useful feature. Interceptors can be resolved from the internal service provider, but it's currently not easy to do it from application provider.

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Jul 10, 2020

But this particular scenario would be handled better by #13261

@mderriey
Copy link
Author

mderriey commented Jul 13, 2020

but it's currently not easy to do it from application provider

How would one achieve this? And are there any gotchas in doing so?

I've done some research over the WE, and I realised that many common services are not registered in the internal service provider — in my case, I couldn't inject ILogger<T> in the interceptor because it's not registered, so I had to use the scoped ILoggerFactory.

@ajcvickers
Copy link
Member

ajcvickers commented Jul 13, 2020

@mderriey CoreOptionsExtension contains a reference to the ApplicationServiceProvider. In addition, this PR has some useful information on the internal service provider: dotnet/EntityFramework.Docs#2066

@ajcvickers ajcvickers added this to the Backlog milestone Jul 13, 2020
@mderriey
Copy link
Author

mderriey commented Jul 14, 2020

Thanks for the pointer!

Would you say the following approach is OK to resolve interceptors from the application service provider?

// Startup.cs
public void ConfigureServices(IServiceCollection services)
{
    // Register interceptor as itself so we can resolve it in the extension
    services.AddScoped<AadAuthenticationDbConnectionInterceptor>();

    // Register DbContext and add custom extension
    services.AddDbContext<AppDbContext>(options =>
    {
        options.UseSqlServer(Configuration.GetConnectionString("UserDb"));
        ((IDbContextOptionsBuilderInfrastructure)options).AddOrUpdateExtension(new AppOptionsExtension());
    });
}

// AppOptionsExtension.cs
public class AppOptionsExtension : IDbContextOptionsExtension
{
    private DbContextOptionsExtensionInfo _info;

    public DbContextOptionsExtensionInfo Info => _info ??= new ExtensionInfo(this);

    public void ApplyServices(IServiceCollection services)
    {
        services.AddScoped<IInterceptor>(provider =>
        {
            // Get application service provider from CoreOptionsExtension
            var applicationServiceProvider = provider
                .GetRequiredService<IDbContextOptions>()
                .FindExtension<CoreOptionsExtension>()
                .ApplicationServiceProvider;

            // Resolve interceptor, and register in internal service provider as IInterceptor
            return applicationServiceProvider.GetRequiredService<AadAuthenticationDbConnectionInterceptor>();
        });
    }

    public void Validate(IDbContextOptions options)
    {
    }

    private class ExtensionInfo : DbContextOptionsExtensionInfo
    {
        public ExtensionInfo(IDbContextOptionsExtension extension) : base(extension)
        {
        }

        public override bool IsDatabaseProvider => false;
        public override string LogFragment => null;
        public override long GetServiceProviderHashCode() => 0L;
        public override void PopulateDebugInfo(IDictionary<string, string> debugInfo)
        {
        }
    }
}

// AadAuthenticationDbConnectionInterceptor.cs
public class AadAuthenticationDbConnectionInterceptor : DbConnectionInterceptor
{
    private readonly ILogger _logger;

    // Could also inject whatever other service here as they'd be resolved from the application service provider
    public AadAuthenticationDbConnectionInterceptor(ILogger<AadAuthenticationDbConnectionInterceptor> logger)
    {
        _logger = logger;
    }

    public override async Task<InterceptionResult> ConnectionOpeningAsync(
        DbConnection connection,
        ConnectionEventData eventData,
        InterceptionResult result,
        CancellationToken cancellationToken)
    {
        var sqlConnection = (SqlConnection)connection;

        //
        // Only try to get a token from AAD if
        //  - We connect to an Azure SQL instance; and
        //  - The connection doesn't specify a username.
        //
        var connectionStringBuilder = new SqlConnectionStringBuilder(sqlConnection.ConnectionString);
        if (connectionStringBuilder.DataSource.Contains("database.windows.net", StringComparison.OrdinalIgnoreCase) && string.IsNullOrEmpty(connectionStringBuilder.UserID))
        {
            try
            {
                sqlConnection.AccessToken = await GetAzureSqlAccessToken(cancellationToken);
                _logger.LogInformation("Successfully acquired a token to connect to Azure SQL");
            }
            catch (Exception e)
            {
                _logger.LogError(e, "Unable to acquire a token to connect to Azure SQL");
            }
        }
        else
        {
            _logger.LogInformation("No need to get a token");
        }

        return await base.ConnectionOpeningAsync(connection, eventData, result, cancellationToken);
    }

    private static async Task<string> GetAzureSqlAccessToken(CancellationToken cancellationToken)
    {
        if (RandomNumberGenerator.GetInt32(10) >= 5)
        {
            throw new Exception("Faking an exception while tying to get a token to make sure errors are logged");
        }

        // See https://docs.microsoft.com/en-us/azure/active-directory/managed-identities-azure-resources/services-support-managed-identities#azure-sql
        var tokenRequestContext = new TokenRequestContext(new[] { "https://database.windows.net//.default" });
        var tokenRequestResult = await new DefaultAzureCredential().GetTokenAsync(tokenRequestContext, cancellationToken);

        return tokenRequestResult.Token;
    }
}

@ajcvickers
Copy link
Member

ajcvickers commented Jul 16, 2020

@mderriey Looks reasonable.

@mderriey
Copy link
Author

mderriey commented Jul 16, 2020

Thanks

@mderriey
Copy link
Author

mderriey commented Sep 16, 2020

Hey folks 👋

Quick update as I found a much easier solution. It's embarrassing I didn't find that before.
Using the AddDbContext overloads that take an Action<IServiceProvider, DbContextOptionsBuilder> makes resolving the interceptors from the application service provider super easy, without having to register an extension with all the plumbing code that goes with it.

Here's a quick sample:

public class Startup
{
    public Startup(IConfiguration configuration)
    {
        Configuration = configuration;
    }

    public IConfiguration Configuration { get; }

    public void ConfigureServices(IServiceCollection services)
    {
        // 1. Register the interceptor in the dependency injection container
        services.AddSingleton<AadAuthenticationDbConnectionInterceptor>();

        // 2. Use one of the overload of AddDbContext that takes a parameter of type Action<IServiceProvider, DbContextOptionsBuilder>
        services.AddDbContext<AppDbContext>((provider, options) =>
        {
            options.UseSqlServer(Configuration.GetConnectionString("<connection-string-name>"));

            // 3. Resolve the interceptor from the service provider
            options.AddInterceptors(provider.GetRequiredService<AadAuthenticationDbConnectionInterceptor>());
        });
    }
}

public class AadAuthenticationDbConnectionInterceptor : DbConnectionInterceptor
{
    // Implementation ommitted for brevity
}

I've blogged about that and a couple of other things over at https://purple.telstra.com/blog/a-better-way-of-resolving-ef-core-interceptors-with-dependency-injection.

@ajcvickers ajcvickers reopened this Sep 16, 2020
@ajcvickers
Copy link
Member

ajcvickers commented Sep 16, 2020

@mderriey The main point of this issue is allowing registration of an interceptor when you don't control the AddDbContext or OnConfigurng code. Sorry we didn't point out earlier that this can be done in AddDbContext.

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

3 participants