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

feat: Feature parity for a_decode_token and decode_token #616

Merged

Conversation

Krismix1
Copy link
Contributor

@Krismix1 Krismix1 commented Nov 13, 2024

I saw that the decode_token allows configuring the leeway (introduced as part of #568), but the same feature is unavailable in a_decode_token.
In an attempt to bridge the gap between, I refactored out the logic to validate the token, leaving the functions only responsible for fetching the public key.
Also fix the case with using validate=False + a key resulting in the validation still being executed.

There is a slight difference in the behaviour though, with regards to how key=None is handled:
Before:
A call like decode_token(token, validate=True, key=None) would be considered "okay", but fail with jwcrypto.jws.InvalidJWSSignature: Verification failed for all signatures["Failed: [ValueError('Unrecognized key type')]"] because the key was None.
With the new change:
The same call will fetch the public key and use it to validate the token.

I think it's an acceptable change in behaviour/API, as I doubt an exception was what consumers would want.

Copy link
Collaborator

@ryshoooo ryshoooo left a comment

Choose a reason for hiding this comment

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

Thanks a lot @Krismix1 LGTM

@ryshoooo ryshoooo merged commit ac07820 into marcospereirampj:master Nov 17, 2024
4 of 5 checks passed
@Krismix1 Krismix1 deleted the feature/a_decode_token-feature-parity branch November 17, 2024 11:10
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.

2 participants