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

Bugfix/unable to extract jwk without kid #1517

Merged

Conversation

erichjsonfosse
Copy link
Contributor

@erichjsonfosse erichjsonfosse commented Aug 25, 2022

Fixes: #1339
Tests passing?: Affirmative

  • Added JwkExtractor, a service that conforms with RFC7517 when it comes to the kty, kid and use claims.
  • Added JwkWindowCryptoService, a "sibling" to JwtWindowCryptoService
  • Added an extra test case for TokenValidationService.validateSignatureIdToken to test for a successful result

Description:
The TokenValidationService now tries to extract keys based on kty and use, and kid if it exists. If no matches are found, it tries to extract keys based on kty, and kid if it exists (kty is required by the spec). If no matches are found, a sensible error is thrown. If matches are found, the first one is chosen, regardless of the number of matches.

Tested sample-code-flow-refresh-tokens successfully with https://offeringsolutions-sts.azurewebsites.net when running the project locally.

Let me know if anything is missing or not up to expectations, and I'll update the PR asap 👍

@andreaslarssen
Copy link
Contributor

@damienbod Could you find the time to check out this PR? It's fixing what stops us from upgrading from Angular 13

@@ -44,9 +44,9 @@ describe('JwkWindowCryptoService', () => {
});
});

beforeEach(() => {
beforeEach(async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use waitforasync

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FabianGosebrink Thanks for the feedback! How about now? 😄

@damienbod
Copy link
Owner

damienbod commented Aug 26, 2022

Thanks @erichjsonfosse PR LGTM I do some manual testing and release, all goes well, this weekend

One test is failing

@erichjsonfosse
Copy link
Contributor Author

Thanks @erichjsonfosse PR LGTM I do some manual testing and release, all goes well, this weekend

One test is failing

Thanks for the feedback @damienbod ! Managed to reproduce locally now. Working on it.

@erichjsonfosse
Copy link
Contributor Author

@damienbod Both the test-lib and test-lib-ci runs successfully locally after the latest update. Awaiting feedback :)

@damienbod damienbod merged commit f052525 into damienbod:main Aug 26, 2022
@erichjsonfosse erichjsonfosse deleted the bugfix/unable-to-extract-jwk-without-kid branch August 26, 2022 16:58
@FabianGosebrink
Copy link
Collaborator

Thanks so much @erichjsonfosse !!!!! ❣️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants