Update to jws@^3.0.0 #78
Conversation
As `jws@3.0.0` changed the verify method signature to be `jws.verify(signature, algorithm, secretOrKey)`, the token header must be decoded first in order to make sure that the `alg` field matches one of the allowed `options.algorithms`. After that, the now validated `header.alg` is passed to `jws.verify`
As the order of steps has changed, the error that was thrown when the JWT was invalid is no longer the `jws` one:
{ [Error: Invalid token: no header in signature 'a.b.c'] code: 'MISSING_HEADER', signature: 'a.b.c' }
That old error (removed from jws) has been replaced by a `JsonWebTokenError` with message `invalid token`. That's why this change will bump be a major.
| return done(new JsonWebTokenError('invalid token')); | ||
| } | ||
|
|
||
| var header = jws.decode(jwtString).header; |
There was a problem hiding this comment.
Does it make sense to check validity and decode? What if we decode and handle the failure?
There was a problem hiding this comment.
Yes, you are right. If I do jws.decode and check for null is the same.
|
@pose couple of questions:
|
We have a couple of tests of invalid tokens: https://github.com/auth0/node-jsonwebtoken/blob/update-to-jws-3/test/jwt.rs.tests.js#L239-L278 Do you have any additional test in mind?
No that I'm aware of. I've taken a look at the PRs and issues. |
| var header = decodedToken.header; | ||
|
|
||
| if (!~options.algorithms.indexOf(header.alg)) { | ||
| return done(new JsonWebTokenError('invalid signature')); |
There was a problem hiding this comment.
@jfromaniello @pose should this be 'invalid header' or 'invalid "alg" claim'?
…dding more mismatch tests.
As
jws@3.0.0changed the verify method signature to bejws.verify(signature, algorithm, secretOrKey), the token header must be decoded first in order to make sure that thealgfield matches one of the allowedoptions.algorithms. After that, the now validatedheader.algis passed tojws.verifyAs the order of steps has changed, the error that was thrown when the JWT was invalid is no longer the
jwsone:That old error (removed from jws) has been replaced by a
JsonWebTokenErrorwith messageinvalid token. That's why this change will bump be a major.