[12.x] Add access token revoked event

[axlon]

This PR adds an event when Passport revokes an access token. This is implemented in the same way as the access token creation event.

[hafezdivandari]

I was thinking about revocation on Passport the other day, and it seems totally unnecessary. I mean every revoke can be a simple delete as we never un-revoke an entity, We just purge revoked entities manually using passport:purge. We may fix this on 13.x? So having an event for this may be unnecessary, what you think?

[axlon]

@hafezdivandari I think that keeping revoked tokens is probably useful in some workflows because it allows you to view revoked tokens in the database (I don't do this myself), this is primarily so because there are no events/callbacks to hook into.

If Passport were to start deleting revoked tokens immediately, I'd say the argument for an event becomes even stronger because after revocation is done there is no way to check if tokens were revoked.

[hafezdivandari]

The problem is that we revoke tokens in some other places without calling AccessTokenRepository::revokeAccessToken(), like:

passport/src/Http/Controllers/AuthorizedAccessTokenController.php

Line 71 in ead5067

$token->revoke();

If we fix this and remove the revoked entity, you can easily listen for Eloquent model's deleted event instead.

Another problem is that revocation is currently inconsistent, for example we never revoke a client, we always delete it:

passport/src/Http/Controllers/ClientController.php

Line 138 in ead5067

$this->clients->delete($client);

I'm going to fix these on 13.x, but this PR makes it just more complex?

[axlon]

Tokens being revoked from outside the repository is indeed a problem for this PR, but its not that big of an issue to fix. The controller in question could be adjusted or the revoke could simply also fire the event, either would work.

Whether an event should be added at all is debatable, Passport is missing a lot of events (I wanted to add a similar event for refresh tokens but immediately ran into inconsistencies there as well). On the other hand there are creation events for both access tokens and refresh tokens.

As for your point about deleting revoked records, personally I think it'd be a bad idea to have Passport delete tokens when they are revoked (as default behavior), because it will be a problem for anyone relying on that data to create insights, for example. Instead it would make more sense to create a revoke action and interface that Passport will call, allowing developers to easily extend/change the default behavior if they want to.

[hafezdivandari]

Whether an event should be added at all is debatable, Passport is missing a lot of events.

It may be much better / safer to define Passport's events as a wrapper for eloquent model's events instead.

As for your point about deleting revoked records, personally I think it'd be a bad idea to have Passport delete tokens when they are revoked (as default behavior), because it will be a problem for anyone relying on that data to create insights, for example. Instead it would make more sense to create a revoke action and interface that Passport will call, allowing developers to easily extend/change the default behavior if they want to.

Revocation suppose to work like soft deletion, but currently it doesn't. Seems easy to fix, I'll try to prepare a PR for this.