More granular control over token validation

[doekenorg]

Reason for this feature

Currently Sanctum has a bit of a shotgun approach to validation. You either specify:

1. how long all tokens are valid
2. they are all valid forever

There is no easy user control for overwriting this behavior.

By implementing this feature developers are able to make:

• Single use tokens
• Limited time tokens & unlimited time tokens at the same time
• Use any other business logic that prevents a specific user from being able to use that token

Usage

The Guard now checks for a validation callback on the Sanctum class. This callback receives:

• The used access token
• The current validation state (bool) according to the Guard

To make a single use token you can to this for instance:

// AppServiceProvider.php
use Laravel\Sanctum\PersonalAccessToken
use Laravel\Sanctum\Sanctum;

public function boot()
{
    // ...
    
    Sanctum::$validateCallback = static function ($accessToken, bool $is_valid) {
        if (!$is_valid || !$accessToken instanceof PersonalAccessToken) {
            return $is_valid; // keep regular behavior
        }

        return $accessToken->last_used_at === null;
    };
}

Since Sanctum::$validateCallback is expected to be a callable, you can replace it with a service from the container if you need to, or make an __invoke()-able class, or a combination of the two.

And because the Guard checks if the callable is set, this does not cause any breaking changes.

Alternative

I first thought about introducing 2 events that could aid in these situations. One to overwrite the validation much like this callable is doing. And one after the Guard validates the token, so some action can be taken to invalidate them; when the business logic is something funky (probably in combination with a custom Sanctum::$personalAccessTokenModel setting).

But I changed my mind because the feature this PR adds is more final as it where. Using events opens the door to other packages that start adding validation logic on top of one another, and I feel like this is something the developer needs to control. Not some behind the scenes magic.

It also made more sense to me in the current style of the package. Much like overwriting the PersonalAccessToken model on the Sanctum class.

[taylorotwell]

This check doesn't read the same to me. The access token must be not null and then you specify it MUST also have an explicit expiration date. But, I don't think we required tokens to have an expiration date before this.

[doekenorg]

You're right. I tried to inverse the statement to prevent a double negation back to valid. But in the process I did end up changing the behavior. Was more intent on keeping the tests working. It's fixed now. Hope it makes more sense this way.

Also added an early return on a missing $accessToken. Because without it we can't deduce a User so it makes sense to short-circuit there.

[taylorotwell]

I guess I'm confused - why would you not just keep this entire block the exact same - just move it down into your method? Trying to invert the second part of it is honestly confusing me and opening us up to small bugs 😅

[doekenorg]

@taylorotwell I get that; but it seemed weird to me to create an isInvalidAccesToken method, and use that as logic to return;. But If you think thats a better approach than making it isValidAccessToken I can change it back. I'm more accustomed "positive" named method than the inverse.

[taylorotwell]

Well even if it was a isValidAccessToken method, couldn't this entire check be moved into that method and return false if the code passes this check?

[doekenorg]

It could be negated and assigned to the $is_valid variable. Something like this:

protected function isValidAccessToken($accessToken): bool
    {
        if (! $accessToken) {
            return false;
        }

        $is_valid = ! (
            ($this->expiration && $accessToken->created_at->lte(now()->subMinutes($this->expiration)))
            || !$this->hasValidProvider($accessToken->tokenable)
        );

        if (is_callable(Sanctum::$validateCallback)) {
            $is_valid = (bool) (Sanctum::$validateCallback)($accessToken, $is_valid);
        }

        return $is_valid;
    }

The early false return in case of no accessToken feels normal; we need an access token. But if we have one, and the Guard deemed it invalid is that the end of it? Because in that case we can indeed keep the original logic and just return false. I would argue against this, because it should be possible to overwrite it to valid, based on some logic. Or we need to extract another method like isValidGuardAccessToken and use that result in isValidAccessToken. But that seems a bit weird.

In this situation we can:

1. Set the default expiration to unlimited (null) and create a callback that makes the token expire by some logic.
2. Set the default expiration to limited, and create a callback that makes it unlimited (or one time, whatever).

Otherwise only option 1 is possible. Hope that makes sense.

How do you feed about this solution?

[doekenorg]

@taylorotwell Sorry for bugging you again on this; but I notices you removed the $isValid parameter from the callback in your reformatting. Any particular reason you did this? I would like to ask to revert that because now I cannot actually add to the validation, but only completely replace it. If I don't know if the Guard deemed it valid I need to check those validations again myself inside the callback; which is a bit of a pain.

Say I want to use the normal behavior, but only use the token one time, I now have to re-add the checks to see if the token hasn't expired already. This seems a bit counter productive to me. It can even make the validation less secure, because If iI only implemented a check to see if the token hasn't been used before, I can potentially use a token that has expired long ago.

[driesvints]

@doekenorg best that you send a follow up pr as its unlikely that Taylor will see your reply