[Bug] When using AddJwtBearer it can not read .well-known/openid-configuration if it contains an object

[tehho]

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

.net 8.0

Where is the issue?

• [ ] M.IM.JsonWebTokens
• [ ] M.IM.KeyVaultExtensions
• [ ] M.IM.Logging
• [ ] M.IM.ManagedKeyVaultSecurityKey
• [ ] M.IM.Protocols
• [X] M.IM.Protocols.OpenIdConnect
• [ ] M.IM.Protocols.SignedHttpRequest
• [ ] M.IM.Protocols.WsFederation
• [ ] M.IM.TestExtensions
• [ ] M.IM.Tokens
• [ ] M.IM.Tokens.Saml
• [ ] M.IM.Validators
• [ ] M.IM.Xml
• [ ] S.IM.Tokens.Jwt
• Other (please describe)

Is this a new or an existing app? New for .net 8.0

Repro

// Minimal api with below config

builder.Services
    .AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
    .AddJwtBearer(JwtBearerDefaults.AuthenticationScheme, options =>
    {
        options.Authority = "your-idp-here";
        options.TokenValidationParameters.ValidateAudience = false;

        options.Events = new JwtBearerEvents
        {
            OnAuthenticationFailed = context =>
            {
                return Task.CompletedTask;
            },
            OnTokenValidated = context =>
            {
                return Task.CompletedTask;
            }
        };
    });

Expected behavior Be able to read .well-known/openid-configuration with object that does not affect OIDC config

Actual behavior Ends reading config on the first }. This stops the read of the rest of the config and in our case ignored the jwks endpoint setting.

Possible solution Ignore unknown objects with a while read loop

Additional context / logs / screenshots / links to code

[keegan-caruso]

@tehho which OIDC provider are you using?

[tehho]

@keegan-caruso Curity. We have got it down to the mtls_endpoint_aliases post in the config causing the issue. https://curity.io/resources/learn/oidc-spring-boot-mtls-auth/ https://datatracker.ietf.org/doc/html/rfc8705#name-metadata-for-mutual-tls-end

[brentschmaltz]

@tehho can you provide a sample of the json that is causing you an issue?

[brockallen]

@tehho can you provide a sample of the json that is causing you an issue?

https://www.rfc-editor.org/rfc/rfc8705.html#section-5

[brentschmaltz]

@brockallen @tehho the reason i asked is because i debugged using that code and didn't see the issue. Did i simplify the metadata too much?

https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/3d994fb87319e12a79631a85c960fafe2fd55867/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/OpenIdConnectConfigurationTests.cs#L22

[tehho]

@brentschmaltz possibly. The issue we've seen is when jwks_uri is below the mtls_endpoint_aliases object. But any object before the jwks_uri should trigger the problem.

A zipped solution with an example. jwks-8.0.zip

[brentschmaltz]

@tehho thanks.

[brentschmaltz]

@tehho Yep, you called it. We get away with arrays as the exist condition is JsonTokenType.EndObject, JsonTokenType.EndArray is just gobbled up.

With Objects, we bail early.

Nice Demo code :-)

Will get this fixed.

[unger]

Maybe I am missing something but wouldn't is work just to remove the if statement at the bottom checking for JsonTokenType.EndObject, to me it seems that this is a somewhat unnecessary optimization as the file should be done reading the next iteration anyway? This will avoid the complexity of skipping/handling objects, also unknown property-names are already stored in the AdditionalData property even if it is an object.

[bgirts]

This is such a painful bug. I am seeing people doing crazy workarounds, like specifying all the values of configuration explicitly in the configuration.

I believe the correct solution is to move it as the first thing after the read - when object is read, the reader is positioned at the end of that object. So after the read in the while cycle it will move to the next element. Besides, if it is the end of the configuration, there is no reason to perform some analysis of that element before stopping the serialization - so doing that at the end is counterintuitive anyways.

            while (reader.Read())
            {
                if (IfJsonSerializerPrimitives.IsReaderAtTokenType(ref reader, JsonTokenType.EndObject, false))
                {
                    break;
                }

Probably the hardest part would be to straighten the tests. As I tried to do it, I bumped against RoundTripFromJson tests, where AdditionalData objects undergo a transformation (loosing the indentations used in test data) - comparer tests differences by comparing strings, so it will fail even if a single whitespace is different.

[bgirts]

Looks that there is a bug in jsonelement comparer, where indents (whitespaces) between elements has impact on the comparison result. E.g. {{ "p": "v"}} is considered to be different from {{"p":"v"}}

[ClassyCircuit]

When is this going to be fixed? This bug is causing serious issues with authentication.

[bgirts]

If it helps in any way solving the bug, I've conducted some experiments that might be helpful. These include a scenario where a certain setup leads to only part of the configuration being read.

https://github.com/bgirts/azure-activedirectory-identitymodel-extensions-for-dotnet/commit/a03b0f9a31d631548a594d85fbc2d92e2f3b8a7c#diff-5dc43db9caf591914cb06c1d6568ea34a22ee5b6f66fd7bdc5fb95cfaff70c5d

[brentschmaltz]

@bgirts @ClassyCircuit @tehho @unger @brockallen this has been fixed in this PR. Hope to have a release out soon.

[AThomsen]

@brentschmaltz I still have trouble with 8.0.3 - is it supposed to be fixed in that version?

[n-goran]

@AThomsen installing explicitly Microsoft.AspNetCore.Authentication.OpenIdConnect nuget package 8.0.3 solved issue on my side

[AThomsen]

Note to others coming here: It's only necessary to include Microsoft.IdentityModel.Protocols.OpenIdConnect (7.5.2 or later), not Microsoft.AspNetCore.Authentication.OpenIdConnect.

It would be great if MS would update Microsoft.AspNetCore.Authentication.JwtBearer to depend on the updated version!