-
Notifications
You must be signed in to change notification settings - Fork 252
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
JWK alg
types are missing and deserialization from spec-compliant JWKS will fail
#252
Comments
alg
types are wrong and deserialization from spec-compliant JWKS will failalg
types are missing and deserialization from spec-compliant JWKS will fail
This is kind of expected, the Algorithm enum only includes things that this library can handle. This library also intentionally does not support JWE to not get too crazy in terms of scope. I think this is a wontfix |
That's kind of a bummer; this means I can't use the library to directly deserialize JWKS from my IdP. I don't expect the library to support JWE, but I wonder if you'll consider a POC that at least includes the enum variants? |
No, because then it looks like the library is supporting them |
I added new variant Anyway, I will respect your decision. For myself or anyone else who lands here in the future (hello from 2022) via Google, what would you recommend since we can not safely depend on being able to deserialize an IdP's JWKS response directly into |
@jblachly I landed here but unfortunately after I fixed our case in other way. We have introduced the other variant which can do the catch all and skip it altogether from the deserialization. |
Since this issue remains open (Thank you @Keats for leaving it open for discoverability), I would add that my fork (https://github.com/jblachly/jsonwebtoken/commits/jwe_alg) which explicitly handles JWE alg types, or @nick9822 fork, is necessary to support KeyCloak as an IdP/JWT source |
Can we have another enum containing jblachly@7977492#diff-9f82b0117510d782b99dcd289413911ac2a437265e598c9562877ad9613736d8R50-R101 and have 2 types for JWK: one supported and one unsupported? So at least the deserialization works but you don't get the impression that those are supported for encoding/decoding. |
What is benefit of those keys in JWK Set when we don't have encoding/decoding capabilities yet? If we still want it in Jwk Set, I think we can cover |
Just so it doesn't error out like when deserializing like in your issue #285 |
To be frank, I am not sure how many such algorithms are out there, hence I followed the |
Me neither. If we use a different enum than |
What issues you foresee with Sign and Verify erroring out with something |
It's just not self-documenting. The |
Yes, I think this will work like replacing |
Precisely! I still return Kind regards |
Anyone interested in writing the PR? |
Sure. I will do it. |
|
I'll let you handle |
Hey! Any progress? |
Having troubles with Keycloak too because of this issue. Is there any news? Upd: for anyone interested, temporal workaround – disable |
Sorry was caught up with other things. Will raise the PR this week. |
Is there any progress on this? |
That should be fixed |
Hi,
First, thanks for this library which has been really helpful and is built in much the same way I would have built my own JWT/JWK library.
Background
When building an API that works with Keycloak and reads its JWKS (JSON web key set), I found that I could not properly deserialize the JWKS and my program panicked on startup. Ultimately, this is because one of the JWK has an
alg
field that is not a member of theAlgorithm
enum[1]. However, this JWK is allowed by the specs (details below) and thus we cannot deserialize a spec-compliant JWK.Detailed problem
JWK specifications [2] allow an
alg
field to take the same values as allowed in JWS (JSON web signature; these values match theAlgorithm
enum variants and include things likeHS256
,RS256
,ES256
, etc.; see ref [3]) , but also to take JWE (JSON web encryption) alg types defined in [4]. These include principallyRSA-OAEP
andECDH-ES
. Note that these values would ALSO be valid values for thealg
parameter of a JWT if the JWT were encrypted (and not just signed).I can write a patch and PR for this, but I wanted to get an ACK or NACK on the overall idea, and also on specific fix that would be backward-compatible.
Proposal for backward-compatible fix
To minimize complexity, admix the
alg
types from JWS [3] and JWE [4] as variants of theAlgorithm
enum [1]. This would be an easy fix.A more complex fix might involve separating the algorithms into signing algorithms and encryption algorithms, but I don't see a ton of value in this unless additional API work is done to the crate to introspect whether a key was simply signed, or whether was encrypted. This would likely be backward-incompatible/API breaking.
@Keats let me know what you think and I'll put it together ASAP
Example JWKS with two JWK keys:
References
Algorithm
enumjsonwebtoken/src/algorithms.rs
Line 16 in e869bff
The text was updated successfully, but these errors were encountered: