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

[Bug] Protecting WebAPI does not work if one wants to use multiple bearer schemes, e.g. for AAD *and* B2C #468

Closed
astaykov opened this issue Aug 18, 2020 · 6 comments · Fixed by #475
Assignees
Labels
bug Something isn't working duplicate This issue or pull request already exists fixed P2
Milestone

Comments

@astaykov
Copy link

astaykov commented Aug 18, 2020

Using Microsoft Identity Web 0.2.3-preview protecting WebAPI with multiple Bearer schemes for different authorities.

Referring to this sample for multi bearer auth for Web Apis, it is impossible to use Microsoft.Identity.Web package to accomplish the required setup.

Steps to reproduce:

Create the following configuration file (only relevant sections are listed here for clarity):

{
  "AzureAdB2C": {
    "Instance": "https://snetga.b2clogin.com/",
    "ClientId": "3422b1f4-8039-4023-9373-dbe747b40125",
    "TenantId": "126bbbd5-e975-43a7-88ac-babc84279788",
    "Domain": "snetga.onmicrosoft.com",
    "SignUpSignInPolicyId": "B2C_1A_signup_signin_google"
  },
  "AzureAd": {
    "Instance": "https://login.microsoftonline.com/",
    "TenantId": "126bbbd5-e975-43a7-88ac-babc84279788",
    "ClientId": "3422b1f4-8039-4023-9373-dbe747b40125"
  }
}

Then try to register both AzureAd and AzureAdB2C the following way:

            services.AddMicrosoftWebApiAuthentication(Configuration, "AzureAdB2C", "B2C");
            services.AddMicrosoftWebApiAuthentication(Configuration, "AzureAd", "v2.0");

            services.AddAuthorization(options =>
            {
                options.DefaultPolicy = new AuthorizationPolicyBuilder()
                    .RequireClaim("iss")
                    .AddAuthenticationSchemes("B2C", "v2.0")
                    .Build();
            }
            );

If the order is like here displayed - first registering AzureAdB2C and then registering AzureAd, you will get the following error when calling the API with valid token:

System.InvalidOperationException: IDX20803: Unable to obtain configuration from: 'https://login.microsoftonline.com/snetga.onmicrosoft.com/B2C_1A_signup_signin_google/v2.0/.well-known/openid-configuration'.
 ---> System.IO.IOException: IDX20807: Unable to retrieve document from: 'https://login.microsoftonline.com/snetga.onmicrosoft.com/B2C_1A_signup_signin_google/v2.0/.well-known/openid-configuration'. HttpResponseMessage: 'StatusCode: 404, ReasonPhrase: 'Not Found', Version: 1.1, Content: System.Net.Http.HttpConnectionResponseContent, Headers:
{

If changing the order - first register the AzureAd and then AzureAdB2C, e.g.:

            services.AddMicrosoftWebApiAuthentication(Configuration, "AzureAd", "v2.0");
            services.AddMicrosoftWebApiAuthentication(Configuration, "AzureAdB2C", "B2C");
            ...

You will get success (200) calling the WebAPI with Azure AD B2C token, but 401 calling the API using v2.0 Token.

If you only use one of them (either AzureAd only or AzureAdB2C only) - it works.

Also, if you use the Microsoft.Identity.Web extensions to register only one of the providers (AzureAd OR AzureAdB2C), but use the Microsoft.AspNetCore.Authentication.XXX for the other provider, e.g.:

          services.AddAuthentication()
            .AddJwtBearer("v2.0", options =>
            {
                options.Authority = $"{Configuration["AzureAd:Instance"]}/{Configuration["AzureAd:TenantId"]}/v2.0";
                options.TokenValidationParameters = new TokenValidationParameters
                {
                    ValidateIssuer = true,
                    ValidIssuer = $"{Configuration["AzureAd:Instance"]}/{Configuration["AzureAd:TenantId"]}/v2.0",
                    ValidateAudience = true,
                    ValidAudience = Configuration["AzureAd:ClientId"],
                    ValidateLifetime = true
                };
            }); services.AddMicrosoftWebApiAuthentication(Configuration, "AzureAd", "v2.0");
            
            services.AddMicrosoftWebApiAuthentication(Configuration, "AzureAdB2C", "B2C");


            services.AddAuthorization(options =>
            {
                options.DefaultPolicy = new AuthorizationPolicyBuilder()
                    .RequireClaim("iss")
                    .AddAuthenticationSchemes("B2C", "v2.0")
                    .Build();
            }
            );

Then the API accepts both AzureAd (v2.0) AND AzureAdB2C tokens.

After some very short search through the code, it seems this registration in WebApiAuthenticationBuilderExtensions might be causing the issue. Because of builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<IValidateOptions<MicrosoftIdentityOptions>, MicrosoftIdentityOptionsValidation>()), which is trying to add a singleton of MicrosoftIdentityOptions

  • basically the second use of AddMicrosoftWebApiAuthentication will override any configuration settings that are different from the first registration. If I read correctly the documentation about TryAddEnumerable, it explains the behavior I am observing. But this is just a guess.

Expected behavior is that I am able to use AddMicrosoftWebApiAuthentication multiple times with different configuration settings. There are a lot of valid scenarios where one would like to have this:

  • Using Azure AD B2C for end-users and client credentials flow (which is exactly what this sample is about)
  • Having an application that authenticates users from different authorities - e.g. Commercial and China/GCC etc.
@astaykov astaykov changed the title This does not work if one wants to use multiple bearer schemes, e.g. for AAD *and* B2C Protecting WebAPI does not work if one wants to use multiple bearer schemes, e.g. for AAD *and* B2C Aug 18, 2020
@astaykov astaykov changed the title Protecting WebAPI does not work if one wants to use multiple bearer schemes, e.g. for AAD *and* B2C [Bug] Protecting WebAPI does not work if one wants to use multiple bearer schemes, e.g. for AAD *and* B2C Aug 18, 2020
@jmprieur
Copy link
Collaborator

jmprieur commented Aug 18, 2020

@astaykov
By using AddMicrosoftWebApiAuthentication, this also sets the default scheme.

Do you try?

services.AddAuthentication()
 .AddMicrosoftWebApi(Configuration, "AzureAdB2C", "B2C")
 .AddMicrosoftWebApi(Configuration, "AzureAd", "v2.0");

You make a good point about the options. We might need to try to use named options (with an IOptionsSnapshot) for MicrosoftIdentityOptions to support this scenario @jennyf19 and @pmaytak

@astaykov; Does your work around fully work for you? is it ok to prioritize this fix after our 0.3.0-preview?

@jmprieur jmprieur added the bug Something isn't working label Aug 18, 2020
@jmprieur jmprieur added this to the [6] Support new scenarios milestone Aug 18, 2020
@pmaytak
Copy link
Contributor

pmaytak commented Aug 18, 2020

Thanks @astaykov for investigation and a write-up.

This is also a duplicate of #429. I wrote a reply there.

The issue is with calling builder.Services.Configure(configureMicrosoftIdentityOptions); twice - with second time overwriting the first options.

builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<IValidateOptions<MicrosoftIdentityOptions>, MicrosoftIdentityOptionsValidation>()); I think is fine because it adds a singleton of a validator class, not options class.

@astaykov
Copy link
Author

@astaykov
By using AddMicrosoftWebApiAuthentication, this also sets the default scheme.

Do you try?

services.AddAuthentication()
 .AddMicrosoftWebApi(Configuration, "AzureAdB2C", "B2C")
 .AddMicrosoftWebApi(Configuration, "AzureAd", "v2.0");

You make a good point about the options. We might need to try to use named options (with an IOptionsSnapshot) for MicrosoftIdentityOptions to support this scenario @jennyf19 and @pmaytak

@astaykov; Does your work around fully work for you? is it ok to prioritize this fix after our 0.3.0-preview?

Using the suggested approach has absolutely same (erronous) results like in my original approach.
My workaround works fully, and it is so far fine to work this way given the preview status of the package.

@pmaytak pmaytak self-assigned this Aug 27, 2020
@pmaytak pmaytak linked a pull request Aug 28, 2020 that will close this issue
@pmaytak
Copy link
Contributor

pmaytak commented Aug 28, 2020

Thanks @astaykov for all the feedback. A fix is in PR #475 (which will be included in the next release). Also added a small section to the wiki related to this. I'll close this issue as duplicate of #429.

@pmaytak pmaytak closed this as completed Aug 28, 2020
@pmaytak pmaytak removed this from the [6] Support new scenarios milestone Aug 28, 2020
@jmprieur jmprieur added the fixed label Aug 28, 2020
@jmprieur jmprieur added this to the 0.3.2-preview milestone Aug 28, 2020
@jmprieur jmprieur added the P2 label Aug 28, 2020
@pmaytak pmaytak added the duplicate This issue or pull request already exists label Aug 28, 2020
@jennyf19
Copy link
Collaborator

Included in 0.4.0-preview release

1 similar comment
@jennyf19
Copy link
Collaborator

Included in 0.4.0-preview release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists fixed P2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants