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] Metadata refresh not working when metadata is changed, since version 8.0.2 #3008

Open
shaheer-k opened this issue Nov 14, 2024 · 8 comments
Labels
More info needed Customer has been asked for more information

Comments

@shaheer-k
Copy link

shaheer-k commented Nov 14, 2024

Which version of Microsoft.IdentityModel are you using?
Microsoft.IdentityModel.Protocols.OpenIdConnect 8.0.2

Where is the issue?

  • Microsoft.IdentityModel.Protocols.OpenIdConnect 8.0.2

We have upgraded to version 8.0.2 from 8.0.1.
After this upgrade, the metadata refresh not seems to be working as expected.

We have added a test to ensure that the metadata refresh is working when there is change in the metadata.

var token = tokenHandler.ValidateTokneAsync(.....)
// added test code to refresh the metadata, and then call the token validation again as below:
token = tokenHandler.ValidateTokneAsync(.....)
// Here it is failing with invalid token signature

Expected behavior
Token is validated succesfully.

Actual behavior
Token validation fails with SecurityTokenInvalidSignatureException exception.

The above test is working with Microsoft.IdentityModel.Protocols.OpenIdConnect 8.0.1.
But it is failng with version 8.0.2 onwards.

Possible solution
Please check if this issue is related to the change #2780

@pmaytak
Copy link
Contributor

pmaytak commented Nov 14, 2024

Does this repro with the latest 8.2.0?

@pmaytak pmaytak added More info needed Customer has been asked for more information and removed needs attention untriaged labels Nov 14, 2024
@keegan-caruso
Copy link
Contributor

keegan-caruso commented Nov 14, 2024

The metadata refresh was moved to a background thread after the initial request, it should refresh, but in the timespan of a unit test, the task to update the metadata may not have completed.

@shaheer-k
Copy link
Author

shaheer-k commented Nov 15, 2024

@pmaytak yes its happening with version 8.0.2 onwards, also in latest version 8.2.0.

@keegan-caruso

In version 8.0.1, when generating a new bearer token with keys not present in the current in-memory configuration, the metadata was automatically refreshed upon encountering an initial SecurityTokenInvalidSignatureException, allowing the token to successfully validate automatically (due to the retry in JwtSecurityTokenHandler.cs). In this scenario, line 898 was executed, resulting in successful token validation. See line 898 in JwtSecurityTokenHandler.cs.

The unit test pseudo-code sequence is as follows:

Generate metadata
Generate new access token
Validate access token
Generate metadata again
Generate new access token (with new key)
Validate access token

Note: In the unit test, metadata is read from a local JSON file.

However, in version 8.0.2 (and the latest versions), if we generate a bearer token with new keys not found in the current in-memory configuration, token validation fails without attempting a retry with the updated metadata. In this case, line 911 is executed, and the SecurityTokenInvalidSignatureException is returned to the caller.
See line 911 in JwtSecurityTokenHandler.cs.

We have also tried adding a delay in the unit test and retrying the token validation after regenerating the metadata, but all validation attempts continue to fail.

Due to this behavior, we are unable to proceed with the package update to the latest version in Production. Could you please advise if any modifications are required in our token validation process or if adjustments to the unit test code are necessary?

Notes:
The project includes the following package references:

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration.Abstractions" Version="8.0.0" />
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.2" />
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="8.0.2" />
    <PackageReference Include="Microsoft.Extensions.Options" Version="8.0.2" />
    <PackageReference Include="Microsoft.IdentityModel.Abstractions" Version="8.2.0" />
    <PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.2.0" />
    <PackageReference Include="BouncyCastle.Cryptography" Version="2.4.0" />
  </ItemGroup>

@shaheer-k
Copy link
Author

Please let me know if any additional information is required to understand the problem.

@keegan-caruso
Copy link
Contributor

A minimal repro of code would help us a lot. thanks @shaheer-k

@jennyf19
Copy link
Collaborator

@shaheer-k any update on a repro?

@brentschmaltz
Copy link
Member

@shaheer-k I understand your issue in reviewing the code.
We moved GetConfigurationAsync() onto a background task as users were complaining that we were blocking when the interval was small. These users were updating metadata on a 15 minute interval. Every 15 minutes the lock would get in the way.

However, upon reflection, we should think about RequestRefresh() as blocking, as the logic will not work as before.

Note: you should make sure all the version of IdentityModel are the same.

@brentschmaltz
Copy link
Member

@shaheer-k @jennyf19 @keegan-caruso @pmaytak I opened this issue which may provide a solution.
#3040

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
More info needed Customer has been asked for more information
Projects
None yet
Development

No branches or pull requests

5 participants