Merge pull request #1449 from hafezdivandari/fix-revoke-refresh-token

Fix issue and persist a new refresh token when revoking refresh token is disabled
thephpleague · Oct 14, 2024 · b8e1830 · b8e1830
2 parents 2c698e2 + c25b439
commit b8e1830
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Auto-generated event emitter is now persisted. Previously, a new emitter was generated every time (PR #1428)
 - Fixed bug where you could not omit a redirect uri even if one had not been specified during the auth request (PR #1428)
 - Fixed bug where "state" parameter wasn't present on `invalid_scope` error response and wasn't on fragment part of `access_denied` redirect URI on Implicit grant (PR #1298) 
-
+- Fixed bug where disabling refresh token revocation via `revokeRefreshTokens(false)` unintentionally disables issuing new refresh token (PR #1449)
+- 
 ## [9.0.0] - released 2024-05-13
 ### Added
 - Device Authorization Grant added (PR #1074)

diff --git a/src/Grant/RefreshTokenGrant.php b/src/Grant/RefreshTokenGrant.php
@@ -83,13 +83,11 @@ public function respondToAccessTokenRequest(
         $responseType->setAccessToken($accessToken);
 
         // Issue and persist new refresh token if given
-        if ($this->revokeRefreshTokens) {
-            $refreshToken = $this->issueRefreshToken($accessToken);
+        $refreshToken = $this->issueRefreshToken($accessToken);
 
-            if ($refreshToken !== null) {
-                $this->getEmitter()->emit(new RequestRefreshTokenEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request, $refreshToken));
-                $responseType->setRefreshToken($refreshToken);
-            }
+        if ($refreshToken !== null) {
+            $this->getEmitter()->emit(new RequestRefreshTokenEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request, $refreshToken));
+            $responseType->setRefreshToken($refreshToken);
         }
 
         return $responseType;

diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php
@@ -5,6 +5,7 @@
 namespace LeagueTests\Grant;
 
 use DateInterval;
+use Laminas\Diactoros\Response;
 use Laminas\Diactoros\ServerRequest;
 use League\OAuth2\Server\CryptKey;
 use League\OAuth2\Server\Entities\RefreshTokenEntityInterface;
@@ -14,6 +15,7 @@
 use League\OAuth2\Server\Repositories\ClientRepositoryInterface;
 use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface;
 use League\OAuth2\Server\Repositories\ScopeRepositoryInterface;
+use League\OAuth2\Server\ResponseTypes\BearerTokenResponse;
 use LeagueTests\Stubs\AccessTokenEntity;
 use LeagueTests\Stubs\ClientEntity;
 use LeagueTests\Stubs\CryptTraitStub;
@@ -688,11 +690,15 @@ public function testUnrevokedRefreshToken(): void
         $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity);
         $scopeRepositoryMock->method('finalizeScopes')->willReturn([$scopeEntity]);
 
+        $accessTokenEntity = new AccessTokenEntity();
+        $accessTokenEntity->setClient($client);
+
         $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
-        $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+        $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessTokenEntity);
         $accessTokenRepositoryMock->expects(self::once())->method('persistNewAccessToken')->willReturnSelf();
 
         $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
+        $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity());
         $refreshTokenRepositoryMock->method('isRefreshTokenRevoked')->willReturn(false);
         $refreshTokenRepositoryMock->expects(self::never())->method('revokeRefreshToken');
 
@@ -727,11 +733,22 @@ public function testUnrevokedRefreshToken(): void
         $grant->setScopeRepository($scopeRepositoryMock);
         $grant->setAccessTokenRepository($accessTokenRepositoryMock);
         $grant->setEncryptionKey($this->cryptStub->getKey());
-        $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));
+        $grant->setPrivateKey($privateKey = new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));
         $grant->revokeRefreshTokens(false);
 
-        $grant->respondToAccessTokenRequest($serverRequest, new StubResponseType(), new DateInterval('PT5M'));
+        $responseType = new BearerTokenResponse();
+        $responseType->setPrivateKey($privateKey);
+        $responseType->setEncryptionKey($this->cryptStub->getKey());
+
+        $response = $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M'))
+            ->generateHttpResponse(new Response());
+
+        $json = json_decode((string) $response->getBody());
 
         self::assertFalse($refreshTokenRepositoryMock->isRefreshTokenRevoked($refreshTokenId));
+        self::assertEquals('Bearer', $json->token_type);
+        self::assertObjectHasProperty('expires_in', $json);
+        self::assertObjectHasProperty('access_token', $json);
+        self::assertObjectHasProperty('refresh_token', $json);
     }
 }