Add an AddOpenIdConnect extension that accepts generic TOptions : OpenIdConnectOptions

[osagga]

The current list of AddOpenIdConnect AuthenticationBuilder extensions only support taking in actions to configure the OpenIdConnectOptions type

aspnetcore/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectExtensions.cs

Line 70 in 8b741bd

public static AuthenticationBuilder AddOpenIdConnect(this AuthenticationBuilder builder, string authenticationScheme, string displayName, Action<OpenIdConnectOptions> configureOptions)

aspnetcore/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectExtensions.cs

Line 55 in 8b741bd

public static AuthenticationBuilder AddOpenIdConnect(this AuthenticationBuilder builder, string authenticationScheme, Action<OpenIdConnectOptions> configureOptions)

aspnetcore/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectExtensions.cs

Line 41 in 8b741bd

public static AuthenticationBuilder AddOpenIdConnect(this AuthenticationBuilder builder, Action<OpenIdConnectOptions> configureOptions)

This pattern is very limiting, since it makes it messy to support multiple OIDC providers, each with a different options configuration (that's configured individually using the ConfigureOptions pattern to take advantage of Dependency Injection when setting the client id/secret). This causes complications like the ones mentioned in #11972.

One possible solution that would fix this issue and improve the extensions is to add an extension that takes in a generic TOptions type that's derived from OpenIdConnectOptions, that way the user can do something like

services
    .AddAuthentication()
        .AddOpenIdConnect<GoogleOpenIdConnectOptions>("Google")
        .AddOpenIdConnect<MicrosoftOpenIdConnectOptions>("Microsoft")

And setup the options configure for each option separately, without worrying about setting up a single options configure class to handle both of them.

This pattern is similar to how the OAuth extensions are already setup here:

aspnetcore/src/Security/Authentication/OAuth/src/OAuthExtensions.cs

Lines 59 to 61 in 8b741bd

	public static AuthenticationBuilder AddOAuth<TOptions, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]THandler>(this AuthenticationBuilder builder, string authenticationScheme, string displayName, Action<TOptions> configureOptions)
	where TOptions : OAuthOptions, new()
	where THandler : OAuthHandler<TOptions>

aspnetcore/src/Security/Authentication/OAuth/src/OAuthExtensions.cs

Lines 46 to 48 in 8b741bd

	public static AuthenticationBuilder AddOAuth<TOptions, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]THandler>(this AuthenticationBuilder builder, string authenticationScheme, Action<TOptions> configureOptions)
	where TOptions : OAuthOptions, new()
	where THandler : OAuthHandler<TOptions>

Its also what the OAuth-based Google, MicrosoftAccount packages take advantage of here:

aspnetcore/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountExtensions.cs

Lines 21 to 22 in 697b397

	public static AuthenticationBuilder AddMicrosoftAccount(this AuthenticationBuilder builder, string authenticationScheme, string displayName, Action<MicrosoftAccountOptions> configureOptions)
	=> builder.AddOAuth<MicrosoftAccountOptions, MicrosoftAccountHandler>(authenticationScheme, displayName, configureOptions);

aspnetcore/src/Security/Authentication/Google/src/GoogleExtensions.cs

Lines 21 to 22 in 697b397

	public static AuthenticationBuilder AddGoogle(this AuthenticationBuilder builder, string authenticationScheme, string displayName, Action<GoogleOptions> configureOptions)
	=> builder.AddOAuth<GoogleOptions, GoogleHandler>(authenticationScheme, displayName, configureOptions);

I can work on a PR that adds the generic extension to the list of extensions here, but let me know if I'm missing something, or if there's a reason why this wasn't added already.

[Tratcher]

So .AddOpenIdConnect<GoogleOpenIdConnectOptions> would still still want an Action<GoogleOpenIdConnectOptions> paramter to configure the options like OAuth does, right?

(that's configured individually using the ConfigureOptions pattern to take advantage of Dependency Injection when setting the client id/secret)

Can you expand on this and include examples? You need to be careful with the various ConfigureOptions patterns, named options, etc.. They also have implications for service lifetimes.

[osagga]

So .AddOpenIdConnect would still still want an Action paramter to configure the options like OAuth does, right?

Yeah sorry I didn't implicitly write the full signature, but it would be something like this:

public static AuthenticationBuilder AddOpenIdConnect<TOptions>(this AuthenticationBuilder builder, string authenticationScheme, string displayName, Action<TOptions> configureOptions)
            where TOptions : OpenIdConnectOptions, new()
        {
            ...
        }

Can you expand on this and include examples? You need to be careful with the various ConfigureOptions patterns, named options, etc.. They also have implications for service lifetimes.

I'll explain how my code setup was, and how I was affected by this when migrating from OAuth to OpenIdConnect.

I'm working on an application that needs to support multiple SSO providers (Google and Microsoft for now, could be more later), I started by using the Microsoft.AspNetCore.Authentication.MicrosoftAccount and Microsoft.AspNetCore.Authentication.Google packages (as recommend in the docs here).

To setup the options for both of those providers, I couldn't use the configuration provider IConfiguration since I need to load up the client id/secret from AWS's secrets manager, (AWS doesn't have an official Configuration provider built to load up secrets through the configuration interface).

So I register the providers like this

services
                .AddAuthentication()
                    .AddGoogle()
                    .AddMicrosoftAccount();

And I created the following classes to setup both the GoogleOptions and MicrosoftAccountOptions

ConfigureGoogleAuthenticationOptions.cs

public class ConfigureGoogleAuthenticationOptions : IConfigureNamedOptions<GoogleOptions>

ConfigureMicrosoftAccountAuthenticationOptions.cs

public class ConfigureMicrosoftAccountAuthenticationOptions: IConfigureNamedOptions<MicrosoftAccountOptions>

And I registered both classes as follows in the Startup.cs

services.ConfigureOptions<Configuration.ConfigureGoogleAuthenticationOptions>();
services.ConfigureOptions<Configuration.ConfigureMicrosoftAccountAuthenticationOptions>();

Now I'm able to setup the options in whichever way I want (using dependency injection to get the values from AWS secrets manager) and life was good.

But then I started to require more scopes and also saw #10037 (comment) and #6486, and concluded that I should probably migrate to OIDC based authentication, instead of OAuth. I tried to see if Microsoft.AspNetCore.Authentication.MicrosoftAccount or Microsoft.AspNetCore.Authentication.Google upgraded to be OIDC based, but they were still using OAuth, so I decided to just setup the OIDC handlers myself, but the big issue I noticed is that the OIDC extensions don't support using custom options type (derived from OpenIdConnectOptions), so my setup would not work as it's right now. So after looking more into it, I just went with something like this

services
                .AddAuthentication()
                    .AddOpenIdConnect("Google", o =>
                        {
                            ServiceProvider serviceProvider = services.BuildServiceProvider();
                            var _secretProviderService = serviceProvider.GetService<ISecretProviderService>();
                           
                            // ... code omitted for simplicity ...

                            // get the secret from the secret provider
                            var googleOAuthSecret = _secretProviderService.GetSecretJSONAsync<GoogleOAuthSecret>(googleOAuthSecretName).Result;

                            o.ClientId = googleOAuthSecret.ClientId;
                            o.ClientSecret = googleOAuthSecret.ClientSecret;

                            // ... code omitted for simplicity ...

                            serviceProvider.Dispose();
                        }
                    ).AddOpenIdConnect("Microsoft", o =>
                        {
                            ServiceProvider serviceProvider = services.BuildServiceProvider();
                            var _secretProviderService = serviceProvider.GetService<ISecretProviderService>();

                            // ... code omitted for simplicity ...

                            // get the secret from the secret provider
                            var microsoftOAuthSecret = _secretProviderService.GetSecretJSONAsync<GoogleOAuthSecret>(microsoftOAuthSecretName).Result;

                            o.ClientId = microsoftOAuthSecret.ClientId;
                            o.ClientSecret = microsoftOAuthSecret.ClientSecret;

                           // ... code omitted for simplicity ...

                            serviceProvider.Dispose();
                        }
                    );

This setup works, but kinda annoying that I had to build the service provider myself, it also generates a warning (since I'm creating multiple instances of a singleton service, but I'm disposing it at the end, so hopefully its fine). And that's why I created this issue, to see if its possible that I add that generic extension, which should fix my setup and allows me to go back to the way it was before.

Let me know if you have any questions, or if there's an easier way that I can do this. Thanks!

[Tratcher]

but kinda annoying that I had to build the service provider myself,

In fact you should never be calling BuildServiceProvider as it messes up the service lifetimes.

So what this new generic options extension unblocks is this?

public class ConfigureMicrosoftOpenIdConnectOptions: IConfigureNamedOptions<MicrosoftOpenIdConnectOptions>
{
    public ConfigureMicrosoftOpenIdConnectOptions(ISecretProviderService dependency) { }

Which you could mostly do today, but you'd need to check the options names first?

public class ConfigureOpenIdConnectOptions: IConfigureNamedOptions<OpenIdConnectOptions>
{
    public ConfigureOpenIdConnectOptions(ISecretProviderService dependency) { }

[osagga]

In fact you should never be calling BuildServiceProvider as it messes up the service lifetimes.

Oh no, I guess I should address this soon, thanks for the heads up, any more documentation about what exactly is wrong with calling BuildServiceProvider?

So what this new generic options extension unblocks is this?

Yes

Which you could mostly do today, but you'd need to check the options names first?

I'm honestly not too familar with named options, I know they exist, but I kinda tried to avoid using them, its also not clear what name will be passed to the options when configured, is it authenticationScheme, displayName from this extension?

aspnetcore/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectExtensions.cs

Line 70 in 8b741bd

public static AuthenticationBuilder AddOpenIdConnect(this AuthenticationBuilder builder, string authenticationScheme, string displayName, Action<OpenIdConnectOptions> configureOptions)

I honestly still prefer if I can pass completely different option types (for each provider, like I had it before), and split the configuration based on that, since it gives me better class management, and separation of provider specific options setup. It would also allow me to create extensions like the .AddGoogle() and .AddMicrosoftAccount() that I can build (or update in this repo) to be OIDC based. What do you think?

[Tratcher]

The name passed in would be the authenticationScheme.

[Tratcher]

An new extension like this wouldn't be added until at least 6.0, a year from now, so let's see what can be done with named options first.

[blowdart]

@Tratcher While you're looking at named options, I'm putting this into discussions. If it can't be done that way, then it can go into next sprint planning for 6.

[ghost]

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!