getClaim convenience methods

Issue #55 resolved
Former user created an issue

It would be nice if the JWT Claims Set classes had convenient methods for getClaim() that would return data types like Strings and Integers.

Perhaps something like getClaimAsString(...) that returns a String instead of Object.

Comments (11)

  1. Vladimir Dzhuvinov

    Thanks for suggesting this.

    What method behaviour do you suggest if the claim is not of the expected type? Return null or throw an exception?

  2. William Kim

    Thank you responding.

    In my very humble opinion as a beginning developer, I would suggest that these methods throw an exception if the claim value was not meant to be used as the expected type.

    This would allow a distinction between the claim value not being specified or found (returning null) versus the claim value being the wrong data type (throwing exception).

  3. Justin Richer

    I think that this makes sense, and throwing an exception if the conversion can't be made. I'd use all java object types though (non-primitives) to preserve the possibility of having a null value. An off-the-cuff list of potentially useful types:

    • String
    • Integer
    • Boolean
    • Long
    • Date
    • Double

    Any others?

  4. Vladimir Dzhuvinov

    Ok, those four as proper objects.

    About the exception, checked or unchecked?

    In my opinion this should be a checked exception, since we're processing external data.

  5. Justin Richer

    We've got two precedents for how this kind of stuff is handled in the "known" claims already:

    • JSONObjectUtils throws a ParseException from methods like getLong, which is a checked method. This makes sense though because parsing happens in a single step, so it's a single call to wrap from the outside and check for exceptions.
    • JWTClaimSet.setClaim throws an IllegalArgumentException if the incoming value isn't what's expected. This makes sense also as it's ultimately doing type-matching on the way in and this will be called in multiple places throughout a piece of code, making it harder to wrap.

    I think it should be an unchecked (runtime) exception that's documented since it's over "live" data. Also if it's a checked exception, it's going to make a mess of things with try/catch statements that wouldn't otherwise have them and in the normal case can expect well-formed data.

    This does beg the question of whether we need a setClaim(String), setClaim(Integer), setClaim(Date), etc. I don't think we do but I want to make sure we actually think about that.

  6. Vladimir Dzhuvinov

    Thank you for studying the precedents. That was a good place to start. I also had a look at the precedents and to the ones you mentioned I would add JWT.getClaimsSet() which throws ParseException.

    I remember my original intent to implement it like that:

    You receive a JWT over the network, validate its signature, then proceed with extracting the claims with getClaimsSet(). At this point all reserved claims are parsed into their appropriate types, and if a conversion fails the checked ParseException is thrown. The recipient has no control over the JWT content, it may well contain garbled or even hostile data, so I thought it would be wise to ensure that all reserved claims have the proper types when the JWTClaimsSet object is parsed (as opposed to throwing exceptions / returning nulls later when you do getSubject(), etc). If we had an unchecked exception and it's not observed, that could become a potential attack vector to disrupt a server that is processing JWTs.

    From that perspective sticking with the checked ParseException makes sense. Even the null appears to be safer that the unchecked exception, although it may cause problems with developers who decide to assign the values to primitives after autoboxing is applied and the potential null exception is not observed.

    In terms of the simplifying the try/catch, this could well be combined with the one required for the JWT.getClaimsSet() by extending it to the custom claims retrieved with get{Type}Claim.

        try {
             JWTClaimsSet cs = jwt.getClaimsSet();
             Subject sub = cs.getSubject();
             String someCustomClaim = cs.getStringClaim("custom-claim");
        } catch (ParseException e) {
             // handle bad token
        }
    
  7. Vladimir Dzhuvinov

    Hi again guys,

    I decided to take a few days off and head to the beaches, and probably will not be able to work on this issue until later next week. If you in the meantime manage to submit a pull request I will be forever grateful :)

  8. Vladimir Dzhuvinov

    If you have other suggestions how to improve the library API, just let us know.

    I just released the update as 2.17.1 which should reach Maven central later today.

  9. Log in to comment