Wrong type of exception raised if required claim is missing in jwt.decode

[akikoskinen]

The documentation of jwt.decode says:

Raises:
JWTError: If the signature is invalid in any way.
ExpiredSignatureError: If the signature has expired.
JWTClaimsError: If any claim is invalid in any way.

When an explicitly required claim (one where options["require_XXX"] is set to True) is missing, a JWTError is raised. I think the implementation and the documentation contradict. I like what the documentation says, and wish that the implementation would deliver. So in this case I hope it would raise a JWTClaimsError.

[blag]

You raise an interesting question: if a required claim is not provided, should that claim be considered "invalid"?

I can definitely see your point that yes, it should be considered "invalid", but after reviewing the code I'm not so sure that I agree with you:

def _validate_claims(claims, audience=None, issuer=None, subject=None,
                     algorithm=None, access_token=None, options=None):
    ...

    # Check that all required claims are provided
    for require_claim in [
        e[len("require_"):] for e in options.keys() if e.startswith("require_") and options[e]
    ]:
        if require_claim not in claims:
            raise JWTError('missing required key "%s" among claims' % require_claim)
        else:
            options['verify_' + require_claim] = True  # override verify when required

    ...

    # Each of these blocks validates a claim value if the claim should be verified
    # The `_validate_<claim>` functions all raise JWTClaimsError
    if options.get('verify_iat'):
        _validate_iat(claims)

    if options.get('verify_nbf'):
        _validate_nbf(claims, leeway=leeway)

    ...

I think the original intent in the code was to classify errors for misconfiguration, decoding, or missing (required) claims as JWT errors, and to classify errors about the values of claims themselves as invalid value errors.

To wit, the slightly more generic JWTError is raised in the following cases:

• the JWT signature is not verified
• the JWT payload is not valid JSON
• the protected claims structure cannot be decoded to a JSON object/Python dictionary
• the JWT header cannot be decoded
• the unprotected claims cannot be decoded
• the unprotected claims are not valid JSON
• the unprotected claims structure cannot be decoded to a valid JSON object/Python dictionary
• a required claim is missing
• the possible audience is not a string (or None)

In comparison, the JWTClaimsError is raised in the following cases:

• the "issued at" claim is not an integer
• the "not before" claim is not an integer
• the JWT is not valid according to the "not before" claim
• the "expires" claim is not an integer
• the "audience" claim is not a JSON array/Python list
• at least one "audience" claim is not a string type
• the "audience" claim is not in the allowed audiences
• the "issuer" claim is not an allowed issuer
• the "subject" claim is not a string
• the "subject" claim does not match the required subject
• the "jti" (JWT ID) claim is not a string type
• an access token claim was passed but an access claim is not allowed
• could not calculate a hash to compare against the "at_hash" claim (note: this check does not fit the pattern)
• the "at_hash" claim does not match the calculated hash

So the code differentiates between these two JWT claims (assume that the "issuer" claim is required):

{
  "aud": "..."
  // required issuer claim is just completely missing from the claims
}  // raises a JWTError

versus:

{
  "aud": "..."
  "iss": null  // required issuer claim is provided, but decodes to None, which isn't a valid value
}  // raises a JWTClaimsError

Circling back around to the question you raised, I think that a completely missing claim should be handled differently than a claim value that is not valid.

Now, that's not to say that your question is invalid to begin with. At the very least, the documentation and docstrings should make this difference very clear:

    Raises:
        JWTError: If the signature is invalid in any way, including
                  decoding issues, missing claims, a non-JSON
                  header or claims section, or invalid signatures.
        ...
        JWTClaimsError: If any provided claim value is invalid in any way.

Would that rewording make the difference more clear?

An additional idea would be to create a JWTMissingClaimError subclass of JWTError:

class JWTMissingClaimError(JWTError):
    "Raised when a required JWT claim is not provided."
    pass

and raise that in _validate_claims if a claim is missing. That will be reverse compatible with code that should already be checking for JWTError exceptions for missing claims. I haven't thought through the potential security implications of giving possible attackers more information though, so that's a change that should be well thought through and communicated to users (eg: major version bump, which will probably happen anyway when I drop Python 2.7 support after this next release that I'm trying to work on, so that would be a good time to make this change).

I would be open to pull requests implementing either or both of those if you have the time. And I'll keep this issue open until it's fixed.

[akikoskinen]

Whoa, that's something I call a thorough answer 🙇

My original problem was that I couldn't tell apart "invalid signature" and "missing required claim" cases (just using the exception type). That would be solved by the proposed JWTMissingClaimError type.

Another thing that I've now noticed is that we might interpret the term signature differently. To me signature means only the cryptographic signature, the one that jws.verify checks. But the proposed documentation:

If the signature is invalid in any way, including decoding issues, missing claims, a non-JSON header or claims section, or invalid signatures.

makes me think that here the term signature (its first occurrence) includes more, perhaps the JWT as a whole. If this is the case, perhaps I interpreted the original documentation ("JWTError: If the signature is invalid in any way") also differently than how it was meant.

So, in order to avoid confusion, we should either agree on what signature means, or acknowledge, that we have differing interpretations. Off course this isn't just you or me making interpretations of the term, since this is public documentation. If there's a chance of misinterpretation, then the documentation could off course be improved.