azure-activedirectory-identitymodel-extensions-for-dotnet icon indicating copy to clipboard operation
azure-activedirectory-identitymodel-extensions-for-dotnet copied to clipboard

[Bug] Well-known openId configuration parsing is causing IDX1050 errors

Open AlexandreBossard opened this issue 1 year ago • 14 comments
trafficstars

Which version of Microsoft.IdentityModel are you using? Identity.Models 7.4

Where is the issue?

  • [x] 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
  • [x] M.IM.Tokens
  • [ ] M.IM.Tokens.Saml
  • [ ] M.IM.Validators
  • [ ] M.IM.Xml
  • [x] S.IM.Tokens.Jwt
  • Other (please describe)

After updating to the latest 7.4 release, all my app tests failed due to invalid jwt token authentication. reverting to 7.3.1 works as expected. No code change between the package upgrade has been made.

We use asymmetric jwt token signature with a custom mock identity server that provide the /.well-known and jwks_uri endpoints. We notice that, even so the well-known endpoint is called when validating a token, the jwks_uri one is not.

Repro Here is the exact endpoint code that worked previously:

app.MapGet("/.well-known/openid-configuration", () =>
            new
            {
                Issuer = issuerUrl,
                jwks_uri = new Uri(new(issuerUrl), ".well-known/jwks.json").ToString(),
            });

which produce the following

$ http --format json.sort_keys:false http://localhost:5001/.well-known/openid-configuration
HTTP/1.1 200 OK
Content-Type: application/json; charset=utf-8
Date: Wed, 13 Mar 2024 12:10:06 GMT
Server: Kestrel
Transfer-Encoding: chunked

{
    "issuer": "http://localhost:5001",
    "jwks_uri": "http://localhost:5001/.well-known/jwks.json"
}

All seems good for all I know.

Now watch this! If I swap the 2 lines above like this:

app.MapGet("/.well-known/openid-configuration", () =>
            new
            {
                jwks_uri = new Uri(new(issuerUrl), ".well-known/jwks.json").ToString(),
                Issuer = issuerUrl,
            });

I get

$ http --format json.sort_keys:false http://localhost:5001/.well-known/openid-configuration
HTTP/1.1 200 OK
Content-Type: application/json; charset=utf-8
Date: Wed, 13 Mar 2024 11:54:19 GMT
Server: Kestrel
Transfer-Encoding: chunked

{
    "jwks_uri": "http://localhost:5001/.well-known/jwks.json",
    "issuer": "http://localhost:5001"
}

And everything works ! 2 things I get from that:

  • System.Text.Json Serialization keeps the arbitrary specified order
  • The last patch broke the .well-known document parsing

Expected behavior Asymmetric Token validation to work regardless of the json key order.

Actual behavior It fails to parse the json document and do not fetch keys.

Possible solution I've pinned the dependency to the last known working version the 7.3.1

Additional context / logs / screenshots / links to code

dotnet is great, it allows us to specify the key order of the serialized json. But that's not the case of Python for exemple. Also, Json object property serialization order is, sadly, unspecified. You must not rely on a specific order for parsing / validation.

I see a lot of code change around json parsing in the last release (e.g. https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/commit/051d164e3c025a0d7276f1d6acf38c902a4893fc ). I suspect a regression slipped in there.

AlexandreBossard avatar Mar 13 '24 12:03 AlexandreBossard

Check if you have consistent versions of referenced Microsoft.IdentityModel.* packages. I think my issue was the same - every second property in configuration json was ignored. In my case the problem was mixed versions of Microsoft.IdentityModel.Tokens/7.4.0 vs Microsoft.IdentityModel.Protocols.OpenIdConnect/7.1.2.

dolly22 avatar Mar 13 '24 14:03 dolly22

Awesome! It has fixed it !

dotnet add package Microsoft.IdentityModel.Protocols.OpenIdConnect --version 7.4.0

It is now a direct dependency instead of a transitive one. I have no way to infer what pulls it, due to the lack of tooling.

Now, if this is a problem that cause regression, a packaging fix still needs to be done, somewhere.

AlexandreBossard avatar Mar 13 '24 15:03 AlexandreBossard

Check if you have consistent versions of referenced Microsoft.IdentityModel.* packages. I think my issue was the same - every second property in configuration json was ignored. In my case the problem was mixed versions of Microsoft.IdentityModel.Tokens/7.4.0 vs Microsoft.IdentityModel.Protocols.OpenIdConnect/7.1.2.

Thank you for this.

I was using packages:

    <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" Version="7.3.1" />
    <PackageReference Include="System.IdentityModel.Tokens.Jwt" Version="7.3.1" />

    <PackageReference Include="Duende.AccessTokenManagement" Version="2.1.0" />
    <PackageReference Include="Duende.AccessTokenManagement.OpenIdConnect" Version="2.1.0" />

And previously didn't have Microsoft.IdentityModel.Tokens or Microsoft.IdentityModel.Protocols.OpenIdConnect installed. When I upgraded my Microsoft.IdentityModel.JsonWebTokens and System.IdentityModel.Tokens.Jwt packages today to 7.4.0, I encountered runtime errors.

Installing the latest versions of Microsoft.IdentityModel.Tokens and Microsoft.IdentityModel.Protocols.OpenIdConnect resolved erverything.

EDIT: Like @AlexandreBossard, only Microsoft.IdentityModel.Protocols.OpenIdConnect was required.

johnjardinemd avatar Mar 13 '24 15:03 johnjardinemd

@AlexandreBossard looks like this issue is resolved?

jennyf19 avatar Mar 13 '24 20:03 jennyf19

Yes but no. There is packaging issue somewhere. If the Microsoft.IdentityModel.JsonWebTokens broke ABI somewhere, it should be reflected in the packaging dependencies (I doubt nuget is capable of doing that), or at least in the release note. Automatic updating tool will keep triggering that bug and create time sink for unaware developers.

AlexandreBossard avatar Mar 14 '24 09:03 AlexandreBossard

So, after looking a bit more in to this, Microsoft.AspNetCore.Authentication.JwtBearer 8.0.2 pulls Microsoft.IdentityModel.Protocols.OpenIdConnect 7.1.2, and everything else as indirect dependency. My issue was having a Microsoft.IdentityModel.JsonWebTokens direct dependency, that in this instance seems to have silently breaks runtime ABI.

If that's the case 7.4.0 probably should have been a new major release.

Also, AspNetCore 8 did not update their dependency on Microsoft.IdentityModel.* since 7.1.2, so if I want the last and best version, I only need a direct dependency on Microsoft.IdentityModel.Protocols.OpenIdConnect.

This is convoluted and not documented.

AlexandreBossard avatar Mar 14 '24 09:03 AlexandreBossard

I just came across this error with a client and rolling back to Microsoft.IdentityModel.JsonWebTokens @ 7.1.3 fixed it for me.

KieranFoot avatar Mar 15 '24 16:03 KieranFoot

So, if I understand it correctly, Microsoft.IdentityModel.Protocols.OpenIdConnect 7.1.2 has version constraint on System.IdentityModel.Tokens.Jwt (>= 7.1.2) but it should also have constraint (< 7.4)

olegd-superoffice avatar Mar 18 '24 10:03 olegd-superoffice

Yes, [email protected] does not work with [email protected]. But, IIUC, you would have to release an [email protected] to fix the dependency constraints.

AlexandreBossard avatar Mar 18 '24 10:03 AlexandreBossard

We had a similar issue, adding a direct reference to the *.Protocols.OpenIdConnect package solved the issue. The breaking change was here I think https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/2508/files probably just needs a note in the release notes about the breaking change and what to do

A-Stapleton avatar Apr 03 '24 16:04 A-Stapleton

@martinb69 Thanks for this. But it looks like the changes from this PR are only included in [email protected] while the current latest version of [email protected] is still referencing [email protected] so [email protected] still needs to be referenced directly to make it work?

olegd-superoffice avatar Apr 08 '24 10:04 olegd-superoffice

This is still an issue, that broke our dependency update routine today (we keep creating project missing that non obvious dependency). Any news ?

AlexandreBossard avatar Aug 30 '24 15:08 AlexandreBossard

The solution is being worked on in #2513. Expected to go into the next release.

pmaytak avatar Sep 05 '24 17:09 pmaytak