DefaultJwtTokenParser#parseClaims Exception Handling

[SoniaZaldana]

Hi all,

This issue relates to #372. We just upgraded WildFly to version 3.0.0 for smallrye-jwt, so I am finally able to test WFLY-13164 with the fix I submitted. However, I realized we might have missed a small case in the exception handling for DefaultJwtTokenParser#parseClaims.

When a invalid public key has been configured in the mp.jwt.verify.publickey.location, an UnresolvableKeyException should be reported. However, note that if this exception arises inside the call to the JOSE method JWTConsumer#process, then the UnresolvableKeyException gets wrapped in a InvalidJwtException by the JOSE method.

To see how the UnresolvableKeyException gets wrapped, I am leaving some links to the Jose code base:
[0] DefaultJwtTokenParser#parseClaims calls jwtConsumer#process
[1] JwtConsumer#process gets called
[2] JwtConsumer#processContext gets called
[3] JwtConsumer#processContext throws UnresolvableKeyException in verificationKeyResolver#resolveKey
[4] Jose wraps the exception in InvalidJwtException

Therefore, once we reach the exception handling in DefaultJwtTokenParser#parseClaims, it logs that the token is invalid (whereas it should log that the public key is unresolvable) and then that JOSE exception gets passed wrapped as a ParseException to JWTHTTPAuthenticationMechanism#validateRequest where the fix I added won't report an error 500 as e.getCause() is an InvalidJwtException whereas e.getCause().getCause() would be an UnresolvableKeyException, which is what we are looking for.

I was wondering whether it would be appropriate to send an UnresolvableKeyException in the DefaultJWTTokenParser instead in this specific case?

Thanks so much for your help!

[sberyozkin]

@SoniaZaldana Hi, yes, IMHO it makes sense, please open your next PR :-)

FYI, since the location can point to a JWK set it may not be possible to report the exception early since with more than one key we don't know which one will be matched, but I suppose for non-JWK sets or sets with a single key only we can indeed revisit the idea of reporting the invalid key exception early. Something to do in the 3.x line going forward.

Thanks

[sberyozkin]

@SoniaZaldana @darranl I've updated the milestone to 3.1.0 as this fix, even though it is totally correct IMHO, requires a minor version update - I'll send an email later on to the forum, cheers