parseSignedClaims(JWE) calls key locator
laurids opened this issue · 2 comments
Is your feature request related to a problem? Please describe.
The JavaDoc for parseSignedClaims
says that it throws an exception if the input string is not actually a JWS: https://github.com/jwtk/jjwt/blob/0.12.3/api/src/main/java/io/jsonwebtoken/JwtParser.java#L324
But if the input string is a JWE, the execution ends up calling the key locator instead of throwing an exception:
https://github.com/jwtk/jjwt/blob/0.12.3/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java#L540
In our key locator, to lookup the key, we are using the key ID and certificate thumbprint from the JwsHeader. However, now we cannot just cast the Header
to JwsHeader
, if we want to avoid running into a generic ClassCastException
.
Describe the solution you'd like
That jjwt throws exception before calling locate
on the key locator when you use parseSignedClaims
on a JWE.
Describe alternatives you've considered
To avoid a generic ClassCastException
, we have put a type check in our key locator:
if (!(header instanceof final JwsHeader jwsHeader)) {
// throwing custom exception...
}
final var keyId = jwsHeader.getKeyId();
final var certificateThumbprint = jwsHeader.getX509Sha256Thumbprint();
Additional context
N/A
This is expected behavior.
.parseSignedClaims
will still throw an exception before returning, it's just that your Locator
is being called before it has the ability to throw the exception.
Per the parseSignedClaims
JavaDoc:
This is a convenience method logically equivalent to the following:
parse(jws).accept(Jws.CLAIMS);
So it is fully parsed/verified first (via the .parse(jws)
call) and then asserted that is the expected type (via the .accept(Jws.CLAIMS)
call. Verification (during .parse
) requires calling the key Locator
if available, so that's why it is invoked before the type assertion occurs.
The JwtParserBuilder
's .keyLocator
method JavaDoc shows in the code example that the base .locate
method implementation doesn't know what type of Header
instance will be passed, and uses instanceof
to determine the type:
jjwt/api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java
Lines 460 to 471 in d4a0827
Even so, if you only ever need to support JWSs, you can simply subclass the LocatorAdapter
class as showin in the https://github.com/jwtk/jjwt#key-locator documentation:
public class MyKeyLocator extends LocatorAdapter<Key> {
@Override
public Key locate(ProtectedHeader<?> header) { // a JwsHeader or JweHeader
// implement me
}
}
But instead of overriding locate(ProtectedHeader)
, override locate(JwsHeader)
instead:
public class MyKeyLocator extends LocatorAdapter<Key> {
@Override
public Key locate(JwsHeader header) { // only ever called for JWSs
// implement me
}
}
The locator will still be called when parsing a JWE, but it will always return null
to indicate no key is found for such a header instance and will not be decrypted (i.e. it's a 'no op'), and an exception thrown.
Based on this, there doesn't appear to be a need to change JJWT's code, so I'm closing the issue, but feel free to continue to discuss if you need any clarification.
Ok, thanks for the hint. It is a little more suitable to extend from LocatorAdapter
since we only need to support JWS :)