[Bug] Microsoft.Identity.Web should not call BuildServiceProvider

[jmprieur]

Which Version of Microsoft Identity Web are you using ?
Microsoft Identity Web 0.1.5-preview

Where is the issue?

• Web App
  • [x ] Sign-in users
• Web API
  • [x ] Protected web APIs (Validating tokens)

Repro
See:

microsoft-identity-web/src/Microsoft.Identity.Web/WebApiAuthenticationBuilderExtensions.cs

Line 145 in 5bc881b

var diags = builder.Services.BuildServiceProvider().GetRequiredService<IJwtBearerMiddlewareDiagnostics>();

microsoft-identity-web/src/Microsoft.Identity.Web/WebAppAuthenticationBuilderExtensions.cs

Line 202 in 5bc881b

var diags = builder.Services.BuildServiceProvider().GetRequiredService<IOpenIdConnectMiddlewareDiagnostics>();

Expected behavior
NEVER call BuildServiceProvider.

Actual behavior
Microsoft.Identity.Web calls BuildServiceProvider

Discussion

• It's a MUST FIX

• In NET 5.0, there is an overload of AddJwtBearer / AddOpenIdConnect with a service that you want to inject

• We need to come-up with alternatives for 3.0. @Tratcher will help

• in addition raise an issue with ASP.NET Core to have these diagnostics available by default without wrapping the events.

Possible design
(but feel free to do differently)

1. Add a constant in the Microsoft.Identity.Web.csproj file depending on the TargetFramework (maybe something like:

     <PropertyGroup Condition="'$(TargetFramework)' == 'netcoreapp3.1'">
       <DefineConstants>$(DefineConstants);DOTNET_CORE_31</DefineConstants>
     </PropertyGroup>

2. Based on the constant use one form of the other of AddOpenIdConnect and AddJwtBearer. For instance for AddOpenIdConnect we could have something like the following:

   #if DOTNET_CORE_31
            builder.AddOpenIdConnect(openIdConnectScheme, options =>
            {
             // Todo: replace by the work around that @Tratcher will provider
             IServiceProvider serviceProvider = builder.Services.BuildServiceProvider();
   #else
            builder.AddOpenIdConnect<IServiceProvider>(openIdConnectScheme, (options, serviceProvider) =>
            {
   #endif

Note that the aspnetcoreapp3.1 case would still use the BuildServiceProvider until Chris provides a workaround

3. In the section garded by the subscribeToXXXMiddlewareDiagnosticsEvents boolean, just use the serviceProvider to call GetRequiredService<>
   For instance for the OIDC case, something like:

    var diags = serviceProvider.GetRequiredService<IOpenIdConnectMiddlewareDiagnostics>();

4. Consider doing [Bug] Remove the weird pattern for newing up redundant copies of MicrosoftIdentityOptions #239 soon after this one, as it leverages similar mechanisms

5. Follow-up with @Tratcher for the NET 3.1 work around to populate the service provider in the case of netcore3.1

[Tratcher]

dotnet/aspnetcore#18772 (comment)
This should work in prior versions:

services.AddOptions<OpenIdConnectOptions>(authSchemeName)
  .Configure<IOpenIdConnectMiddlewareDiagnostics>((options, diagnostics) =>
  {
  });

In fact, you could do it in both versions and avoid the #ifs.

[jmprieur]

thanks @Tratcher

[jmprieur]

So I understand that the section in WebApiAuthenticationBuilderExtensions.cs could be:

#if DOTNET_CORE_31
            // Change the authentication configuration to accommodate the Microsoft identity platform endpoint (v2.0).
            builder.AddJwtBearer(jwtBearerScheme, options => { });
            builder.Services.AddOptions<JwtBearerOptions>(jwtBearerScheme)
                .Configure<IServiceProvider>((options, serviceProvider) =>
            {
#else
            builder.AddJwtBearer<IServiceProvider>(jwtBearerScheme, (options, serviceProvider) =>
            {
#endif

And in WebAppAuthenticationBuilderExtensions.cs, it could be:

#if DOTNET_CORE_31
            builder.AddOpenIdConnect(openIdConnectScheme, options => { });
            builder.Services.AddOptions<OpenIdConnectOptions>(openIdConnectScheme)
                .Configure<IServiceProvider>((options, serviceProvider) =>
                {
#else
            builder.AddOpenIdConnect<IServiceProvider>(openIdConnectScheme, (options, serviceProvider) =>
            {
#endif

@jennyf19, moving back to "in progress" now that Chris has shared the work around for .NET Core 3.1

[jmprieur]

Finalized (even better), part of #277

[jennyf19]

Included in 0.2.0-preview release