OpenIdConnectHandler - Design for override

[slaneyrw]

Is your feature request related to a problem? Please describe.

I'm trying to augment the standard OpenId/Connect handler to configure additional information in the OIDC challenge and expose additional events for specific Auth failure reasons.

I want to subclass the existing OpenIdConnect handler but I'm have issues with getting access to some private variables, getting hooks for configuring the OpenIdConnect protocol message prior to raising events, and the hardcoding of the Options type into the base RemoteAuthenticationHandler class.

Describe the solution you'd like

Create an open generic OpenIdConnectHandler that has an OpenIdConnectOptions type constraint. Refactor the current OpenIdConnectHandler to close the new generic over the existing OpenIdConnectOptions.

Expose the OpenIdConnectConfiguration private variable to protected scope

Add virtual methods to allow configuring/creation of the OpenIdConnect protocol messages BEFORE invoking the RedirectToIdentityProvider event

Allow setting of the JSON parsing settings ( JObject or JsonDocument ) instead of calling the overload of Parse without settings

Allow a way of Removing default claim actions added in the OpenIdConnectOptions constructor, we have to clear and duplicate the actions just because I want some of the OpenId claims ( specifically AMR ) to be available to the OpenId client's system.

Describe alternatives you've considered

I created a new set of classes inheriting from RemoteAuthenticationHandler, but creating an internal OpenIdConnect handler and routing the HandleChallengeAsync, HandleRemoteAuthenticateAsync, and SignOutAsync from the outer handler to the inner OpenIdConnect handler. I'm getting significant friction with the way the Events property is created (InitializeEventsAsync) to coordinate the Events instance between the 2 handlers

Additional context

Add any other context or screenshots about the feature request here.

[brockallen]

specifically AMR

I have a feeling amr will be automatically available in a upcoming release, and other claims easier to make available.

[blowdart]

As this would be a rather large, and potentially break change, it could only take place ion a major release, so moving to the backlog until we plan the next major release

[slaneyrw]

Just an additional note that to get my Options subclass into the Events context I need to inherit from each context type, override the Options property and EACH Handle method. A lot of work when doing from the outside, and trying to navigate the mire of private and protected scopes.

[slaneyrw]

specifically AMR

I have a feeling amr will be automatically available in a upcoming release, and other claims easier to make available.

Not really my sticking point... getting my Options subclass into the entire Handler/Events object structure means I have to wrap most of them and forward method calls.

[DumboJet]

I have the same requirement, because our authentication provider (Signicat) seems to be trying to make the token endpoint to require the client ID and secret passed as a basic auth header. They have only enabled this behavior in a test environment of theirs and created a mess for us. From what I have seen, this is now recommended practice:

https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1

Including the client credentials in the request-body using the two parameters is NOT RECOMMENDED and SHOULD be limited to clients unable to directly utilize the HTTP Basic authentication scheme (or other password-based HTTP authentication schemes). The parameters can only be transmitted in the request-body and MUST NOT be included in the request URI.

The only way to support this, seems to be overriding the OpenIdConnectHandler (the RedeemAuthorizationCodeAsync method here) but, unlike @slaneyrw, I am still struggling on how to do this... I mostly have a problem making the framework use my handler, not overriding its methods (although, getting the TokenEndpoint URL in there might be a challenge too...), so if someone can help on this it would be nice.

I am trying things like:

            services.AddSingleton<AuthenticationHandler<OpenIdConnectOptions>, CustomOpenIdConnectHandler>();

..but they don't work.

---

UPDATE: OK, I think this is how you replace the handler:

services.Replace(ServiceDescriptor.Transient<OpenIdConnectHandler, CustomOpenIdConnectHandler>());

But turns out there is an easier way to do what I want: https://github.com/signicat/Quick-start-guide-Authentication-.NET-Core

 options.Events.OnAuthorizationCodeReceived = RedeemAuthorizationCodeAsync;

....
protected virtual async Task RedeemAuthorizationCodeAsync(AuthorizationCodeReceivedContext context)
{
    var configuration = await context.Options.ConfigurationManager.GetConfigurationAsync(CancellationToken.None);
    var requestMessage = new HttpRequestMessage(HttpMethod.Post, configuration.TokenEndpoint);
    string authInfo = context.TokenEndpointRequest.ClientId + ":" + context.TokenEndpointRequest.ClientSecret;
    authInfo = Convert.ToBase64String(Encoding.Default.GetBytes(authInfo));
    requestMessage.Headers.Authorization = new AuthenticationHeaderValue("Basic", authInfo);
    var msg = context.TokenEndpointRequest.Clone();
    msg.ClientSecret = null;
    requestMessage.Content = new FormUrlEncodedContent(msg.Parameters);

    var responseMessage = await context.Backchannel.SendAsync(requestMessage);
    if (!responseMessage.IsSuccessStatusCode)
    {
        Console.WriteLine(await responseMessage.Content.ReadAsStringAsync());
        return;
    }

    try
    {
        var responseContent = await responseMessage.Content.ReadAsStringAsync();
        var message = new OpenIdConnectMessage(responseContent);
        context.HandleCodeRedemption(message);
    }
    catch (Exception)
    {
    }
}

[slaneyrw]

@DumboJet Handling an existing Event callback is how it was designed to work. But I wanted to add new events and add additional properties to the options. That is where my "fun" started.