-
-
Notifications
You must be signed in to change notification settings - Fork 694
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
Support JWK without alg. #624
Conversation
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.
jwi.io lists the following possibilities, only some of which are supported in this pull request
- HS256
- HS384
- HS512
- PS256
- PS384
- PS512
- RS256
- RS384
- RS512
- ES256
- ES256K
- ES384
- ES512
- EdDSA
TBH I'm not sure where to get sample tokens for all of the algorithms
(jwt.io has sample tokens for most, but not all)
jwt.io tokens are here: https://github.com/jsonwebtoken/jsonwebtoken.github.io/blob/bf9a99c8f37aacbcac0b58b9877179073aea32a1/src/editor/default-tokens.js |
@dimaqq Thanks for your comments and information. At the first place, keys in JWKs are independent of hash algorithms, at least in "RSA"(RS*, PS*) and "oct"(HS*). Therefore, the hash algorithm should not be specified when the JWK is loaded (especially if there is no However, in the current implementation of pyjwt, the hash algorithm must be specified during the JWK decoding phase due to pyjwt class structure, and this PR is a workaround to implicitly set the default value of In case of kty=RSA, I adopted RS256 as a default alg value as it is "Recommended" in RFC7518. Again, because it is not possible to determine whether it is RS256, RS384, RS512 from the information contained in JWK (kty=RSA). Similarly, In case of kty=oct, I adopted HS256 as a default alg value. as it is a unique "Required" algorithm in RFC7518. On the other hand, in case of kty=EC,
At least, we need another PR to support it (RFC8812). |
After #623 merged, I'll add support for ES256K, add tests and remove WIP. |
For anyone else (@MarcinBMD) facing this, here's my workaround that you can apply to a 2.0.1 (or before) version of the library that fixes the bug. It just takes monkey-patches the
|
I don't think it is the best way but for now it is a reasonable solution for supporting JWKs which don't have "alg" (e.g., Azure AD public JWKs).
Considering backward compatibility, I made kty validation (and alg guessing) activated only when a JWK does not have "alg".Since all of existing tests passed, I made
kty
parameter mandatory compliant with RFC7517.In addition, test coverage is not perfect because Ed25519Algorithm does not have from_jwk() function.if you merge PR #623, I can send another PR for that.
I'm very happy if you check and merge it.
Close #603
Close #616