Make _configuration protected for OpenIdConnectHandler

[hvanbakel]

When deriving from the handler it's useful for an implementer to have access to the retrieved configuration for the openid connect provider (for example to get the token endpoint).

[Tratcher]

We've long written off deriving from OpenIdConnectHandler (and others) as a useful extensibility model. The methods are not nearly granular enough. We provided events instead.

What are you doing in your derived handler?

[hvanbakel]

I use the derived handler so I can inject a type which I can then use to generate a private_key_jwt as described in https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

I guess my biggest problem is the lack of injection in the events.

[Tratcher]

@blowdart and @HaoK are writing an example for event injection as we speak.

[hvanbakel]

In the current setup? Or is that with changes to the API?

[blowdart]

It should be current. If I can figure it out.

[hvanbakel]

    internal sealed class CustomOpenIdConnectHandler : OpenIdConnectHandler
    {
        private readonly IClientAssertionCertificateLoader certificateLoader;
        private readonly Task<OpenIdConnectConfiguration> getConfigurationTask;

        public CustomOpenIdConnectHandler(
            IClientAssertionCertificateLoader certificateLoader,
            IOptionsMonitor<Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions> internalOptions, 
            ILoggerFactory logger, 
            HtmlEncoder htmlEncoder, 
            UrlEncoder encoder, 
            ISystemClock clock) : base(internalOptions, logger, htmlEncoder, encoder, clock)
        {
            this.certificateLoader = certificateLoader;
            this.getConfigurationTask = this.Options.ConfigurationManager.GetConfigurationAsync(System.Threading.CancellationToken.None);
        }

this is my current workaround, I then just await that one task whenever I need a bit of configuration

[muratg]

We're not planning to do this.

[hvanbakel]

@blowdart I'm assuming you're still writing something up how to do DI here, right?

[blowdart]

Oh lord apologies, I forgot to update you on that. So, we had a bunch of discussions about it and the devs slapped me with a clue stick. The context passed through to every delegate has the ability to access the DI container, so rather than mess around with having derived classes which creates problems if you have multiple instances of an authentication type (you'd need separate derived classes per instance), the expected pattern is something like

     OnValidateCredentials = context =>
      {
        var validationService =
          context.HttpContext.RequestServices.GetService<IUserValidationService>();
        if (validationService.AreCredentialsValid(context.Username, context.Password))
        {
           /// Do your thing
        }

        return Task.CompletedTask;
      }

[hvanbakel]

Agreed that this works, this and DI however are 2 separate things, agreed?

This logic all happens in AddServices, which is problematic in a net core world. I want to be able to inject things like options/different services all with different dependencies that are valid at runtime.

The current setup requires me to resolve things every single time when a method executes which I feel is very contrary to the intent that net core was (rightfully) pushing.