Proposal: Handle validation, defaults, & refreshes in individual Claim classes

[tdhsmith]

I've been thinking it might be cleaner and more modular to hold all of the validation methods and defaults inside each of the Claim classes. This would make claim initialization and validation a lot more configurable, and checks would be more internalized so the user doesn't have to manually examine the token payload.

Before I go too far, I want to highlight a couple use cases:

• I'd like to have iss verify that it matches the current domain as a sanity check. (Really this shouldn't come up in practice because different issuers would normally have different keys, but it's possible some people want multiple token issuing services all running on the same Laravel instance.)
• I'd like to implement an csrf claim to help mitigate request forgery when storing JWTs in cookies. A couple of existing services already use a claim for this (Stormpath has an xsrfToken claim and OpenID Connect has a nonce claim). I could certainly do this already, but it would be nice if I didn't have to expose the JWT internals to do it.
• People have previously requested different token refresh flows, like being able to chain refreshes infinitely but still having an expired-but-refreshable period (not currently possible since the period is always determined from the original iat) or having a finite number of refreshes (we could have a refresh_count claim that tracked this). As it stands these would require a lot of conditional code inside the manager/validator/factory and extra config values, but with claim handling it would just take a new claim class.

Here's an approximate structure as an example:

abstract class Claim
{
    protected $value;
    public $name; // tells the manager which claim keys to map to this class

    // Faked "enumerated" value for action in case there end up being others
    const CHECK = 0;
    const REFRESH = 1;

    public function __construct($value = null) {
        if (is_null($value)) {
            // if we have no args in our constructor, use the default
            $this->reset();
        } else {
            $this->set($value);
        }
    } 

    public function validate($action = self::CHECK) {
        switch ($action) {
            case self::CHECK:
                return $this->validateCheck();
            case self::REFRESH:
                return $this->validateRefresh();
        }
        throw new \Exception("Invalid action flag.");
    }

    abstract function validateCheck();
    abstract function validateRefresh();

    final public function getInternal() {
        return $this->value;
    }
    final public function setInternal($newValue) {
        $this->value = $newValue;
    }

    // Unlike the above, these may be overrided to transform the values (see NBF)
    public function get() {
        return $this->value;
    }
    public function set($newValue) {
        $this->value = $newValue;
    }

    // reset sets the value to its default
    abstract function reset();
    // modify the value as necessary for refreshing
    abstract function refresh();
}

Example existing claim:

class NotBefore implements Claim
{
    // Note that the value will be a Carbon instance internally
    protected $value;
    public $name = 'nbf';

    public function validateCheck() {
        if ($value->isFuture()) {
            throw new TokenInvalidException('Not Before (nbf) timestamp cannot be in the future');
        }
        return null;
    }
    public function validateRefresh() {
        return $this->validateCheck();
    }

    public function get() {
        return $this->value->timestamp;
    }
    public function set($newValue) {
        $this->value = Utils::timestamp($newValue);
    }

    public function reset() {
        $this->value = Utils::now();
    }
    public function refresh() {
        $this->reset();
    }
}

Example new claim:

class RefreshCount implements Claim
{
    protected $value;
    public $name = 'refresh_count';

    public $limit;
    const DEFAULT_LIMIT = 4;

    public __construct($value = null) {
        parent::__construct($value);
        $this->limit = config('jwt.refresh_chain_limit', self::DEFAULT_LIMIT);
    }

    public function validateCheck() {
        return null;
    }
    public function validateRefresh() {
        if ($this->value >= $this->limit) {
            throw new TokenInvalidException('Token has already been refreshed the maximum number of times');
        }
        return null;
    }

    public function reset() {
        $this->value = 0;
    }
    public function refresh() {
        $this->value += 1;
    }
}

Not shown: the major changes to manager/payload/factory/validator, but hopefully you can see some faint sketches of how it would work based on the interface above. Might need a ClaimManager go-between so the settings aren't duplicated everywhere, but I'm not sure if that's necessary.

There are some challenges I'm already apprehending:

• Not all defaults are easily separable. For example, jti relies on the rest of the claims, how do we ensure it is created last without hard-coding it?
• Return value on validateRefresh requires confusing ternary (?) semantics. Some claims don't care (e.g. sub), some have positive "OR" cases (e.g. exp OR iat + refresh_ttl is in the future), and some absolute rejections (e.g. nbf is in the future). How do we handle those cases? In the above I'm using null for don't care, true for accept, and throwing and exception for reject, but that's a really weird triad. Also do we extend this behavior to validateCheck?
• Not sure yet how to modularize the necessary behavior for blacklisting. Maybe there's a TimestampClaim subclass that requires a blacklistUntil function returning a timestamp of the last point a particular claim could trigger validity either for refresh or check. Then blacklist just takes the max of all these values and uses that for the cache timeout.
• The general difficulty of a larger codebase change

Anyway I just wanted to put these thoughts out there while I had them. It might be a while til I can put together a working demo, and it might end up being too much complexity for the benefit. But I do think it simplifies some aspects and makes them a little more natural.

[tymondesigns]

@tdhsmith Thanks for pulling this together. It's been on my mind for a while too.. it always felt wrong generating the claim values in the Factory class, and I've tried a few things myself, but hadn't come up with anything workable.

You have definitely peaked my interest on the matter again, so I will look at trying some stuff out soon!

As you say though, there are some things that will need consideration, and we'll need to be careful not to introduce too much complexity. Nonetheless, there should be some good wins to be made with this

Cheers!

[tymondesigns]

Just to let you know, I'm working on refactoring the whole validation system. It's coming along nicely!

Can't wait to share what I have 😄