Microsoft-identity-web: [Bug] Custom JwtBearerOptions not applied

[CalamityLorenzo]

Which Version of Microsoft Identity Web are you using ?
Note that to get help, you need to run the latest version.

0.14-preview and current souce
Where is the issue?

• Web App
  
  
  
  • [ ] Sign-in users
  
  
  • [ ] Sign-in users and call web APIs
  
  
• Web API
  
  
  
  • [ x] Protected web APIs (Validating tokens)
  
  
  • [ ] Protected web APIs (Validating scopes)
  
  
  • [ ] Protected web APIs call downstream web APIs
  
  
• Token cache serialization
  
  
  
  • [ ] In Memory caches
  
  
  • [ ] Session caches
  
  
  • [ ] Distributed caches
  
  

Other? - please describe;

Is this a new or existing app?
New App
Repro

            services.AddProtectedWebApi(Configuration, subscribeToJwtBearerMiddlewareDiagnosticsEvents: true);
            services.Configure<JwtBearerOptions>(AzureADDefaults.JwtBearerAuthenticationScheme, options =>
                    {
                        // This is an Microsoft identity platform Web API
                        options.Authority += "/v2.0";

                        // The valid audiences are both the Client ID (options.Audience) and api://{ClientID}
                        options.TokenValidationParameters.ValidAudiences = new string[]
                        {
                        options.Audience, $"api://{options.Audience}", $"https://{options.Audience}"
                        };
                        // D-d-d-delegate
                        options.TokenValidationParameters.IssuerValidator = Microsoft.IdentityModel.Tokens.Validators.ValidateIssuer;
                    });

Expected behavior
A clear and concise description of what you expected to happen (or code).
The JwtBearerOptions are never applied. So my settings audience is <>, but I expect the valid audenices to be [<>, api://<>,https://<>]
Actual behavior
Instead the only valid audiences is only <>

Additional context/ Logs / Screenshots
The audience is an easy example. The problem looks like the JwtBearerOptions are not being applied by Identity.Web

[pmaytak]

Hello.

• What is the value of AzureADDefaults.JwtBearerAuthenticationScheme?
• Please instead try
  services.Configure<JwtBearerOptions>(JwtBearerDefaults.AuthenticationScheme, options =>.

In AddProtectedWebApi we specify JwtBearerDefaults.AuthenticationScheme (which is "Bearer") as the name of the JwtBearerOptions config instance. So when you want to add some custom logic, it has to specify the same configuration instance name.

[CalamityLorenzo]

That works great, my breadkpoint was hit.
I got the example from the docs here :
https://github.com/AzureAD/microsoft-identity-web/wiki/customization
You can see the use of AzureADDefaults.JwtBearerAuthenticationScheme.

[CalamityLorenzo]

I spoke too soon.
The audience validation is still ignoring the ones created in the JwtBearerOptions delegate.
The RegisterValidAudience.ValidateAudience delegate takes in a TokenValidationParameters
This contains ValidAudience and ValidAudiences

The conditional check at the end of the method is (in my case) incorrect.

If I provide a validationParameters.ValidAudience in the settings.json the condition is met, and the first part is executed. However, to fill the validationParameters.ValidAudiences array, it uses the same setting [Audience] from settings.json, and thus ValidAudience is also filled.
// The valid audiences are both the Client ID (options.Audience) and api://{ClientID} options.TokenValidationParameters.ValidAudiences = new string[] { options.Audience, $"api://{options.Audience}", $"https://{options.Audience}" };
If I don't have a ValidAudience it checks the interesect, against a faulty collection of audiences.

So my question, is that a bug or am I applying my audiences incorrectly?

[jennyf19]

@CalamityLorenzo Do you mind trying this branch to see if the issue is resolved? thx.

[CalamityLorenzo]

@jennyf19 No, the if statement in RegisterValidAudience that causes the problem is exactly the same.

 // Cases where developers explicitly provided the valid audiences
            else if (!string.IsNullOrEmpty(validationParameters.ValidAudience))
            {
                return audiences.Contains(validationParameters.ValidAudience);
            }
            else
            {
                return audiences.Intersect(validationParameters.ValidAudiences).Any();
            }

ValidAudience will ALWAYS have a value if read from the settings.json
Should I be blanking the ValidAudience parameter in when I configure the JwtBearerOptions in app Startup?

            services.Configure<JwtBearerOptions>(JwtBearerDefaults.AuthenticationScheme, options =>
                    {
                        // This is an Microsoft identity platform Web API
                        //options.Authority += "/v2.0";

                        // The valid audiences are both the Client ID (options.Audience) and api://{ClientID}
                        options.TokenValidationParameters.ValidAudiences = new string[]
                        {
                        options.Audience, $"api://{options.Audience}", $"https://{options.Audience}"
                        };
                        // D-d-d-delegate
                        options.TokenValidationParameters.IssuerValidator = Microsoft.IdentityModel.Tokens.Validators.ValidateIssuer;
                    });

[jmprieur]

@CalamityLorenzo : thank you.
I see this is because ASP.NET Core will give it a default value. we'll check this.

What behavior would you expect? take valid audiences first? and then valid audience if valid audiences is empty? Or the union of both?
@brentschmaltz. What would you recommend?

[CalamityLorenzo]

@jmprieur In this instance I am providing one Audience, which I am expanding out to 3 to capture the possibilities, so the union would make sense.
If instead I provided a list of audiences then, a union with the valid audience parameter if present (discard if not). [So imagine a settings file with Aud1:a1, Aud2:a2 &etc. I have not completed the explicit Audience parameter]
In both cases I am explictly adding Audiences so I would expect them to be tested somewhere.

I hope that seems clear.

[jmprieur]

It is clear @CalamityLorenzo. thanks!

[brentschmaltz]

@jmprieur @CalamityLorenzo if you set TokenValidationParameters.AudienceValidator then it will ONLY be called.
see: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/63f25857adfaba66327681cb47c4b5589d06c887/src/Microsoft.IdentityModel.Tokens/Validators.cs#L87

to understand how the audience is validated.

[jennyf19]

Thanks for the explanation @CalamityLorenzo

You actually need to do this:

services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
            .AddMicrosoftWebApi(jwtBearerOptions =>
      {
            Configuration.Bind("AzureAD", jwtBearerOptions);
            jwtBearerOptions.TokenValidationParameters.ValidAudiences = new string[]
            {
                   jwtBearerOptions.Audience, $"api://{jwtBearerOptions.Audience}", $"https://{jwtBearerOptions.Audience}"
             };
       },
            microsoftIdentityOptions => { Configuration.Bind("AzureAD", microsoftIdentityOptions); }
);

I tested it and this works without any internal code changes on our end. Can you try it out?

[CalamityLorenzo]

@jennyf19 Nope that has the exact same problem.
ValidAudience && ValidAudiences are both filled, the audience check still fails at the exact same place.
I am unable check a tokens audience, against the multiple of audiences I have deemed valid.

[jmprieur]

@CalamityLorenzo : can you please share (send us?) your appsettings.json? (removing the secrets, clientsID)
I'm confused

[pmaytak]

If I understand correctly, the issue is that if both TokenValidationParameters.ValidAudience and TokenValidationParameters.ValidAudiences are specified, and audiences does not contain ValidAudience, we return false. But we also need to check other audiences in ValidAudiences.

https://github.com/AzureAD/microsoft-identity-web/blob/09bb43e5b44073a1013a2b69cc85cf3514a1a928/src/Microsoft.Identity.Web/Resource/RegisterValidAudience.cs#L88-L96

So we either need to add ValidAudience to ValidAudiences and then just check audiences against ValidAudiences. Or on line 91, return only if it's true. If it's false, we also need to check against ValidAudiences.

[brentschmaltz]

@pmaytak is this code validating the audience? Could you call Microsoft.IdentityModel.Tokens.Validators,ValidateAudience(...)?

https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/63f25857adfaba66327681cb47c4b5589d06c887/src/Microsoft.IdentityModel.Tokens/Validators.cs#L87

[CalamityLorenzo]

  "AzureAd": {
    "Instance": "https://login.microsoftonline.com/",
    "ClientId": "<<ClientIdGuid>>", 
    "Audience": "<<AudienceGuid>>", 
    "TenantId": "Organizations/TenanId" 
  }

as @pmaytak has deduced a solution, I'll restate the problem.
My AzureAd.Audience is added to the jwtToken.Audience as part of the Configuration. This is a good thing.
However, depending on environment/policies comtemplated for use with this application I cannot guarantee this being the correct audience being recieved.
So I now add variations of the Audience into ValidAudiences. [These cover naked client id(original), ResourceUrl (api://) and app hostname (https)]. I may at a later date wish to add https://services.flow.com, etc.

services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
            .AddMicrosoftWebApi(jwtBearerOptions =>
      {
            Configuration.Bind("AzureAD", jwtBearerOptions);
            jwtBearerOptions.TokenValidationParameters.ValidAudiences = new string[]
            {
                   jwtBearerOptions.Audience, $"api://{jwtBearerOptions.Audience}", $"https://{jwtBearerOptions.Audience}"
             };
       },
            microsoftIdentityOptions => { Configuration.Bind("AzureAD", microsoftIdentityOptions); }
);

At this point both ValidAudience && ValidAudiences are populated,
https://github.com/AzureAD/microsoft-identity-web/blob/09bb43e5b44073a1013a2b69cc85cf3514a1a928/src/Microsoft.Identity.Web/Resource/RegisterValidAudience.cs#L88-L96
The if statement only checks the contents of ValidAudience, and ValidAudiences is ignored. If my Audience is api://<<AudienceGuid>> it fails.
The only way to do make populate the ValidAudiences is to not use the ValidAudience setting in the first instance. I asked previously if that was a valid approach.

Should I be blanking the ValidAudience parameter in when I configure the JwtBearerOptions in app Startup?

[brentschmaltz]

@CalamityLorenzo yes i agree with this specific issue.
What I am attempting to describe if Validators.ValidateAudience was called instead of these if statements there would be no issue.
Also, a different exception would be thrown.

[jmprieur]

@CalamityLorenzo : would you want to try out this branch ?
https://github.com/AzureAD/microsoft-identity-web/tree/jmprieur/fix211
note that you'll need to install .NET 5.0, unless you remove the net50 target from the TargetFramework in the project file

[CalamityLorenzo]

I still get exactly the same problem in exactly the same if statement.
ValidAudience and ValidAudiences are both populated and so ValidAudiences are never tested.

[jmprieur]

I don't understand, @CalamityLorenzo
the if statement is not longer there: See https://github.com/AzureAD/microsoft-identity-web/blob/01002a2e852db1bb78957cb76417bb8b1dbc2a3c/src/Microsoft.Identity.Web/Resource/RegisterValidAudience.cs#L90

did you really checkout the https://github.com/AzureAD/microsoft-identity-web/tree/jmprieur/fix211 branch? (not master)

[CalamityLorenzo]

I'll confirm and get back to you. Agreed i probably snagged the wrong branch

[CalamityLorenzo]

@jmprieur Yep, wrong branch. and the fix works .
The Audiences are validated as expected

[jmprieur]

Thanks for confirming @CalamityLorenzo
sorry for the delay!

[CalamityLorenzo]

No apologies needed. If anything it's me ought to be thanking you guys.
This particular project has been absolutely criticial in helping me understanding OAUTH flows/tickets/audiences and how all that translates to code.
Thanks!

[jennyf19]

Included in 0.2.0-preview release
cc: @CalamityLorenzo