Token Revocation question

[chervand]

Are you going to include something RFC7009 compliant to the lib that could be used in a straightforward way like $server->respondToRevokeTokenRequest($request, $response);?

[Sephster]

I think it would be good to work towards this so would hope for it to be included in the future. Thanks for the suggestion

[jacobweber]

I might try working on this for my own purposes. I can try to submit a PR if you want.

I could add a respondToRevokeTokenRequest method to AuthorizationServer. It would need to duplicate some of the code in AbstractGrant->validateClient, since that's not available to AuthorizationServer.

Then, depending on the token_type_hint, it would call either accessTokenRepository->revokeAccessToken or refreshTokenRepository->revokeRefreshToken. You'd probably need to pass the refresh token repository into the method, since it's not otherwise available to AuthorizationServer.

I think we'd also want to pass in a flag for whether to revoke the access token, when token_type_hint=refresh_token, since this is optional.

[lordrhodos]

I could add a respondToRevokeTokenRequest method to AuthorizationServer. It would need to duplicate some of the code in AbstractGrant->validateClient, since that's not available to AuthorizationServer.

@jacobweber if you are going to submit a PR try to avoid code duplication and try to decouple the validatClient feature if that is being duplicated. Maybe even submit a differnt PR if neccessary to get this right.

just my 0.002 cents ;-)

[jacobweber]

Sure, if you don’t mind me moving that out of the Grant classes, and maybe into its own class, I can take that approach.

[jacobweber]

Before I go ahead with writing tests and submitting a pull request, could I ask if this is something you're interested in at all, and if my approach in this commit is reasonable? It's based on master.

Basically I did this:

• Copied the client validation code from AbstractGrant into a ClientValidator class (later I can separate this into its own PR, and change the Grants to use this same class).
• Added a RevokeTokenHandler class to handle revoke requests. It checks to see if the token is an access or refresh token, and revokes it if possible. If $canRevokeAccessTokens is true, it will allow you to revoke access tokens, and it will revoke the associated access token when you revoke a refresh token.
• Added a enableRevokeTokenHandler method to AuthorizationServer, which can be used like this:

$refreshTokenRepository = [...];
$authServer = new \League\OAuth2\Server\AuthorizationServer([...]);
$handler = new \League\OAuth2\Server\RevokeTokenHandler($refreshTokenRepository);
$authServer->enableRevokeTokenHandler($handler);

• Added a respondToRevokeTokenRequest method to AuthorizationServer, which can be used in the same way as respondToAccessTokenRequest.

[Sephster]

Hi @jacobweber - this is definitely something I would like to add to the server so please feel free to submit a PR. Thanks!

[chervand]

Hi @Sephster @jacobweber. I've already done it for the Yii2 framework integration library. In case you might want it here, I'll submit a PR.

[jacobweber]

@Sephster Do you have a preferred approach? I won’t be offended if it’s not mine 😊. @chervand is using a new Grant, and I’m using a non-Grant class.

[Sephster]

Thanks both for your offers of help with this. I think that Jacob's proposal would be more likely to be accepted. If my understanding of the RFC is correct, this change is adding an endpoint rather than a grant. Thanks both again for your offers of help here.

[jacobweber]

No problem. I will try to work it up into a PR. I may steal some of @chervand's more mature code if he doesn't mind.

[chervand]

@jacobweber I don't mind :)

[filecage]

It's been a while, still I think that revoking the tokens should be part of this library (because it's pure pain to handle it outside of the library).

The big question is: can we resurrect #995 or do we need an entirely new implementation?

[Sephster]

Probably can rectify. V9 is priority just now though. Then probably custom claims.