-
Notifications
You must be signed in to change notification settings - Fork 434
Trigger metadata refresh for token decryption errors #3149
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
Conversation
SummarySummary
CoverageMicrosoft.IdentityModel.JsonWebTokens - 80.3%
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-setting my review, I need a bit more time to look at this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pmaytak
You changed the public API, but didn't update the PublicApi.Unshipped.txt files (you didn't use the fixer in the IDE)
- IDX10603 is not part of the declared API in LogMessages.cs.
- IDX10907 is not part of the declared API in LogMessages.cs.
- IsRecoverableException is not part of the declared API in TokenUtilities.cs.
- IsRecoverableExceptionType is not part of the declared API in TokenUtilities.cs.
SummarySummary
CoverageMicrosoft.IdentityModel.JsonWebTokens - 80.3%
|
src/Microsoft.IdentityModel.Tokens/Validation/Results/Details/ValidationError.cs
Outdated
Show resolved
Hide resolved
SummarySummary
CoverageMicrosoft.IdentityModel.JsonWebTokens - 80.3%
|
SummarySummary
CoverageMicrosoft.IdentityModel.JsonWebTokens - 80.3%
|
SummarySummary
CoverageMicrosoft.IdentityModel.JsonWebTokens - 80.3%
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @pmaytak. Great work!
Fixes #3148
Summary
aims to address decryption issues by triggering a metadata refresh when token decryption fails due to the token's Key ID (Kid) not matching any of the decryption keys' IDs. It introduces changes to handle SecurityTokenEncryptionKeyNotFoundException as a recoverable exception when the current configuration contains decryption keys.
Key changes include:
Details:
SecurityTokenEncryptionKeyNotFoundException
when decryption fails and token's Kid doesn't match the decryption keys' IDs.SecurityTokenEncryptionKeyNotFoundException
is considered a recoverable exception when the current configuration contains decryption keys and in that case a metadata refresh will be triggered.SecurityTokenEncryptionKeyNotFoundException
already existed but it seems like it has never been used since it was created years ago and seems like the purpose was for this exact scenario.