search

thephpleague / oauth2-server

A spec compliant, secure by default PHP OAuth 2.0 Server
https://oauth2.thephpleague.com
MIT License
6.49k stars 1.12k forks source link

RefreshTokenGrant: add option whether to revoke refreshed access tokens #1377

Open josiasmontag opened 9 months ago

josiasmontag commented 9 months ago

Currently, the RefreshTokenGrant immediately revokes an access token when it gets refreshed.

The RFC Section 6 makes no mention that this should happen.

The current behavior sometimes causes issues: Some clients assume the old access token is still valid because it has not reached its expiration date yet. Also, there are race conditions with simultaneous requests when one client refreshes the token and the other client still uses the old, non-expired token.

This PR adds an option revokeRefreshedAccessTokens to configure revoking old access token after refreshing. It defaults to true, wich is the current behaviour.

Also see #1347

josiasmontag commented 9 months ago

Some additional thoughts: The current behaviour of revoking the access tokens might even be wrong according to the RFC. Section 1.5 says:

Refresh tokens are issued to the client by the authorization server and are used to obtain a new access token when the current access token becomes invalid or expires, or to obtain additional access tokens with identical or narrower scope (access tokens may have a shorter lifetime and fewer permissions than authorized by the resource owner).

The current implementation does not allow to obtain additional access tokens as it revokes all previous ones.

Sephster commented 8 months ago

We already have a function for this called revokeRefreshTokens which can be set to true or false. We used to automatically revoke and appreciate this isn't following spec so added in a method to allow implementers to choose whether this is enforced or not.

The boolean should be set in the AuthServer class. Thank you for your PR though. I hope this functionality helps solve your problem.

josiasmontag commented 8 months ago

@Sephster I am aware of revokeRefreshTokens which is about revoking the old refresh token, it does not control whether the old access token is revoked. This PR is about adding a setting for revoking the previous access token. Please have a look at the relevant change here.

Sephster commented 8 months ago

Apologies I had missed this. Will reopen to review later. Thanks for clarifying.

pat0s commented 4 months ago

Is there any progress on clarifying this PR? @Sephster

Sephster commented 4 months ago

No not yet. All my efforts are on releasing v9 then this will be picked up along with others. Cheers