RefreshTokenGrant should not revoke the access token

[Starfox64]

In the current implementation of the refresh_token grant, the access token associated with the old refresh token is revoked

oauth2-server/src/Grant/RefreshTokenGrant.php

Line 67 in 591a631

$this->accessTokenRepository->revokeAccessToken($oldRefreshToken['access_token_id']);

The RFC however makes no mention of this being a requirement, only that the refresh token MAY be revoked which is something that is now configurable.

[Sephster]

Thanks for raising this. We already have an issue for this in #946 so closing accordingly to avoid duplicates

[Starfox64]

Unless I'm missing something, that issue doesn't seem related to this.
I'm not talking about having no expiry on the access token just that they should not be revoked early when using the refresh token grant and should instead be allowed to expire naturally.

[Starfox64]

@Sephster Sorry for the ping, I don't know if you saw my last message but I think what you mentioned is a different issue.

[Sephster]

Sorry @Starfox64 - the reason I've linked them is that currently, if your access token is revoked, you can't use your refresh token as that has also been revoked or expired. The issue I linked to would resolve this by having indefinite refresh tokens so you aren't stuck in limbo.

I get your point though. It won't be directly solving what you've raised here. What would you like to happen in the library? It seems safer to just revoke the access token to me from a security point of view.

[Starfox64]

In my case I would still very much like to keep a TTL on the refresh token.

I'm asking this mostly for spec compliance since some clients will expect to be able to reuse access tokens until they expire but it's true that this isn't a problem for most people.

Problems arise however in distributed systems where multiple requests can be in flight which will cause 401 errors when a refresh happens.

In the interest of providing added security on top of the spec we could leave this as is but add a shouldRevokeAccessToken attribute to the refresh grant to leave the door open for those that need to strictly follow the spec.