From 0d5b2ebb8b62caa8b88df3ee2dd1c61f4b90715b Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 10 Dec 2024 14:59:06 +0330 Subject: [PATCH 1/6] fix events --- src/Grant/DeviceCodeGrant.php | 6 ++++-- src/Grant/ImplicitGrant.php | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Grant/DeviceCodeGrant.php b/src/Grant/DeviceCodeGrant.php index 9c9133408..b3b69f30c 100644 --- a/src/Grant/DeviceCodeGrant.php +++ b/src/Grant/DeviceCodeGrant.php @@ -25,7 +25,9 @@ use League\OAuth2\Server\Exception\UniqueTokenIdentifierConstraintViolationException; use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface; use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; +use League\OAuth2\Server\RequestAccessTokenEvent; use League\OAuth2\Server\RequestEvent; +use League\OAuth2\Server\RequestRefreshTokenEvent; use League\OAuth2\Server\ResponseTypes\DeviceCodeResponse; use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; use Psr\Http\Message\ServerRequestInterface; @@ -156,14 +158,14 @@ public function respondToAccessTokenRequest( // Issue and persist new access token $accessToken = $this->issueAccessToken($accessTokenTTL, $client, $deviceCodeEntity->getUserIdentifier(), $finalizedScopes); - $this->getEmitter()->emit(new RequestEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request)); + $this->getEmitter()->emit(new RequestAccessTokenEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request, $accessToken)); $responseType->setAccessToken($accessToken); // Issue and persist new refresh token if given $refreshToken = $this->issueRefreshToken($accessToken); if ($refreshToken !== null) { - $this->getEmitter()->emit(new RequestEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request)); + $this->getEmitter()->emit(new RequestRefreshTokenEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request, $refreshToken)); $responseType->setRefreshToken($refreshToken); } diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 81252dea1..30f4211dc 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -172,6 +172,9 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth $finalizedScopes ); + // TODO: next major release: this method needs `ServerRequestInterface` as an argument + // $this->getEmitter()->emit(new RequestAccessTokenEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request, $accessToken)); + $response = new RedirectResponse(); $response->setRedirectUri( $this->makeRedirectUri( From befbb6975cd466ab970ca0e21204679f709d981b Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 10 Dec 2024 15:01:25 +0330 Subject: [PATCH 2/6] add one more TODO comment --- src/Repositories/DeviceCodeRepositoryInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Repositories/DeviceCodeRepositoryInterface.php b/src/Repositories/DeviceCodeRepositoryInterface.php index 09575ab18..b5df5ab73 100644 --- a/src/Repositories/DeviceCodeRepositoryInterface.php +++ b/src/Repositories/DeviceCodeRepositoryInterface.php @@ -33,7 +33,7 @@ public function persistDeviceCode(DeviceCodeEntityInterface $deviceCodeEntity): * Get a device code entity. */ public function getDeviceCodeEntityByDeviceCode( - string $deviceCodeEntity + string $deviceCodeEntity // TODO: next major release: rename to `$deviceCode` ): ?DeviceCodeEntityInterface; /** From ec2969cc4f1bfd1636732db31137dafdf351003a Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sun, 29 Dec 2024 19:26:35 +0330 Subject: [PATCH 3/6] fix changelog --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4899ef506..fd9c5aa09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] ### Changed - Key permission checks ignored on Windows regardless of userland choice as cannot be run successfully on this OS (PR #1447) +- Fixed bug on setting interval visibility of device authorization grant (PR #1410) +- Fix a bug where the new poll date were not persisted when `slow_down` error happens, because the exception is thrown before calling `persistDeviceCode`. (PR #1410) +- Fix a bug where `slow_down` error response may have been returned even after the user has completed the auth flow (already approved / denied the request). (PR #1410) ## [9.1.0] - released 2024-11-21 ### Added @@ -16,9 +19,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - In the Auth Code grant, when requesting an access token with an invalid auth code, we now respond with an invalid_grant error instead of invalid_request (PR #1433) - Fixed spec compliance issue where device access token request was mistakenly expecting to receive scopes in the request (PR #1412) - Refresh tokens pre version 9 might have had user IDs set as ints which meant they were incorrectly rejected. We now cast these values to strings to allow old refresh tokens (PR #1436) -- Fixed bug on setting interval visibility of device authorization grant (PR #1410) -- Fix a bug where the new poll date were not persisted when `slow_down` error happens, because the exception is thrown before calling `persistDeviceCode`. (PR #1410) -- Fix a bug where `slow_down` error response may have been returned even after the user has completed the auth flow (already approved / denied the request). (PR #1410) ## [9.0.1] - released 2024-10-14 ### Fixed From cfd7713cc48f2bf83357850087b7c1169949af89 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sun, 29 Dec 2024 19:26:46 +0330 Subject: [PATCH 4/6] fix style --- tests/AuthorizationServerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/AuthorizationServerTest.php b/tests/AuthorizationServerTest.php index 72add207b..6e41a17f3 100644 --- a/tests/AuthorizationServerTest.php +++ b/tests/AuthorizationServerTest.php @@ -183,7 +183,7 @@ public function testMultipleRequestsGetDifferentResponseTypeInstances(): void $privateKey = 'file://' . __DIR__ . '/Stubs/private.key'; $encryptionKey = 'file://' . __DIR__ . '/Stubs/public.key'; - $responseTypePrototype = new class() extends BearerTokenResponse { + $responseTypePrototype = new class () extends BearerTokenResponse { protected CryptKeyInterface $privateKey; protected Key|string|null $encryptionKey = null; From 8c7aec5c5ab5b693471b88f74a2832e3d9a8bf45 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Mon, 30 Dec 2024 00:19:31 +0330 Subject: [PATCH 5/6] add tests --- tests/Grant/AuthCodeGrantTest.php | 32 ++++++++++++++++++++++ tests/Grant/ClientCredentialsGrantTest.php | 17 ++++++++++++ tests/Grant/DeviceCodeGrantTest.php | 32 ++++++++++++++++++++++ tests/Grant/ImplicitGrantTest.php | 17 ++++++++++++ tests/Grant/PasswordGrantTest.php | 32 ++++++++++++++++++++++ tests/Grant/RefreshTokenGrantTest.php | 32 ++++++++++++++++++++++ 6 files changed, 162 insertions(+) diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index 390001721..76750f1e9 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -17,6 +17,9 @@ use League\OAuth2\Server\Repositories\ClientRepositoryInterface; use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; +use League\OAuth2\Server\RequestAccessTokenEvent; +use League\OAuth2\Server\RequestEvent; +use League\OAuth2\Server\RequestRefreshTokenEvent; use League\OAuth2\Server\RequestTypes\AuthorizationRequest; use League\OAuth2\Server\ResponseTypes\RedirectResponse; use LeagueTests\Stubs\AccessTokenEntity; @@ -635,6 +638,27 @@ public function testRespondToAccessTokenRequest(): void $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); + $accessTokenEventEmitted = false; + $refreshTokenEventEmitted = false; + + $grant->getListenerRegistry()->subscribeTo( + RequestEvent::ACCESS_TOKEN_ISSUED, + function ($event) use (&$accessTokenEventEmitted): void { + self::assertInstanceOf(RequestAccessTokenEvent::class, $event); + + $accessTokenEventEmitted = true; + } + ); + + $grant->getListenerRegistry()->subscribeTo( + RequestEvent::REFRESH_TOKEN_ISSUED, + function ($event) use (&$refreshTokenEventEmitted): void { + self::assertInstanceOf(RequestRefreshTokenEvent::class, $event); + + $refreshTokenEventEmitted = true; + } + ); + $request = new ServerRequest( [], [], @@ -665,6 +689,14 @@ public function testRespondToAccessTokenRequest(): void $response = $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); self::assertInstanceOf(RefreshTokenEntityInterface::class, $response->getRefreshToken()); + + if (!$accessTokenEventEmitted) { + self::fail('Access token issued event is not emitted.'); + } + + if (!$refreshTokenEventEmitted) { + self::fail('Refresh token issued event is not emitted.'); + } } public function testRespondToAccessTokenRequestWithDefaultRedirectUri(): void diff --git a/tests/Grant/ClientCredentialsGrantTest.php b/tests/Grant/ClientCredentialsGrantTest.php index 69f756c37..788749ebb 100644 --- a/tests/Grant/ClientCredentialsGrantTest.php +++ b/tests/Grant/ClientCredentialsGrantTest.php @@ -11,6 +11,8 @@ use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; +use League\OAuth2\Server\RequestAccessTokenEvent; +use League\OAuth2\Server\RequestEvent; use LeagueTests\Stubs\AccessTokenEntity; use LeagueTests\Stubs\ClientEntity; use LeagueTests\Stubs\ScopeEntity; @@ -53,6 +55,17 @@ public function testRespondToRequest(): void $grant->setDefaultScope(self::DEFAULT_SCOPE); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); + $accessTokenEventEmitted = false; + + $grant->getListenerRegistry()->subscribeTo( + RequestEvent::ACCESS_TOKEN_ISSUED, + function ($event) use (&$accessTokenEventEmitted): void { + self::assertInstanceOf(RequestAccessTokenEvent::class, $event); + + $accessTokenEventEmitted = true; + } + ); + $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', 'client_secret' => 'bar', @@ -64,5 +77,9 @@ public function testRespondToRequest(): void $response = $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M')); self::assertNotEmpty($response->getAccessToken()->getIdentifier()); + + if (!$accessTokenEventEmitted) { + self::fail('Access token issued event is not emitted.'); + } } } diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php index 42157a494..a2dcb5534 100644 --- a/tests/Grant/DeviceCodeGrantTest.php +++ b/tests/Grant/DeviceCodeGrantTest.php @@ -18,6 +18,9 @@ use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface; use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; +use League\OAuth2\Server\RequestAccessTokenEvent; +use League\OAuth2\Server\RequestEvent; +use League\OAuth2\Server\RequestRefreshTokenEvent; use LeagueTests\Stubs\AccessTokenEntity; use LeagueTests\Stubs\ClientEntity; use LeagueTests\Stubs\DeviceCodeEntity; @@ -380,6 +383,27 @@ public function testRespondToAccessTokenRequest(): void $grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getIdentifier(), 'baz', true); + $accessTokenEventEmitted = false; + $refreshTokenEventEmitted = false; + + $grant->getListenerRegistry()->subscribeTo( + RequestEvent::ACCESS_TOKEN_ISSUED, + function ($event) use (&$accessTokenEventEmitted): void { + self::assertInstanceOf(RequestAccessTokenEvent::class, $event); + + $accessTokenEventEmitted = true; + } + ); + + $grant->getListenerRegistry()->subscribeTo( + RequestEvent::REFRESH_TOKEN_ISSUED, + function ($event) use (&$refreshTokenEventEmitted): void { + self::assertInstanceOf(RequestRefreshTokenEvent::class, $event); + + $refreshTokenEventEmitted = true; + } + ); + $serverRequest = (new ServerRequest())->withParsedBody([ 'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code', 'device_code' => $deviceCodeEntity->getIdentifier(), @@ -391,6 +415,14 @@ public function testRespondToAccessTokenRequest(): void $this::assertInstanceOf(RefreshTokenEntityInterface::class, $responseType->getRefreshToken()); $this::assertSame([$scope], $responseType->getAccessToken()->getScopes()); + + if (!$accessTokenEventEmitted) { + self::fail('Access token issued event is not emitted.'); + } + + if (!$refreshTokenEventEmitted) { + self::fail('Refresh token issued event is not emitted.'); + } } public function testRespondToRequestMissingClient(): void diff --git a/tests/Grant/ImplicitGrantTest.php b/tests/Grant/ImplicitGrantTest.php index 617aaa842..60d0d26d7 100644 --- a/tests/Grant/ImplicitGrantTest.php +++ b/tests/Grant/ImplicitGrantTest.php @@ -14,6 +14,8 @@ use League\OAuth2\Server\Repositories\ClientRepositoryInterface; use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; +use League\OAuth2\Server\RequestAccessTokenEvent; +use League\OAuth2\Server\RequestEvent; use League\OAuth2\Server\RequestTypes\AuthorizationRequest; use League\OAuth2\Server\ResponseTypes\RedirectResponse; use LeagueTests\Stubs\AccessTokenEntity; @@ -272,7 +274,22 @@ public function testCompleteAuthorizationRequest(): void $grant->setAccessTokenRepository($accessTokenRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); + $accessTokenEventEmitted = false; + + $grant->getListenerRegistry()->subscribeTo( + RequestEvent::ACCESS_TOKEN_ISSUED, + function ($event) use (&$accessTokenEventEmitted): void { + self::assertInstanceOf(RequestAccessTokenEvent::class, $event); + + $accessTokenEventEmitted = true; + } + ); + self::assertInstanceOf(RedirectResponse::class, $grant->completeAuthorizationRequest($authRequest)); + + if (!$accessTokenEventEmitted) { + // self::fail('Access token issued event is not emitted.'); // TODO: next major release + } } public function testCompleteAuthorizationRequestDenied(): void diff --git a/tests/Grant/PasswordGrantTest.php b/tests/Grant/PasswordGrantTest.php index 8c60a8c78..231d1505f 100644 --- a/tests/Grant/PasswordGrantTest.php +++ b/tests/Grant/PasswordGrantTest.php @@ -15,6 +15,9 @@ use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; use League\OAuth2\Server\Repositories\UserRepositoryInterface; +use League\OAuth2\Server\RequestAccessTokenEvent; +use League\OAuth2\Server\RequestEvent; +use League\OAuth2\Server\RequestRefreshTokenEvent; use LeagueTests\Stubs\AccessTokenEntity; use LeagueTests\Stubs\ClientEntity; use LeagueTests\Stubs\RefreshTokenEntity; @@ -69,6 +72,27 @@ public function testRespondToRequest(): void $grant->setDefaultScope(self::DEFAULT_SCOPE); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); + $accessTokenEventEmitted = false; + $refreshTokenEventEmitted = false; + + $grant->getListenerRegistry()->subscribeTo( + RequestEvent::ACCESS_TOKEN_ISSUED, + function ($event) use (&$accessTokenEventEmitted): void { + self::assertInstanceOf(RequestAccessTokenEvent::class, $event); + + $accessTokenEventEmitted = true; + } + ); + + $grant->getListenerRegistry()->subscribeTo( + RequestEvent::REFRESH_TOKEN_ISSUED, + function ($event) use (&$refreshTokenEventEmitted): void { + self::assertInstanceOf(RequestRefreshTokenEvent::class, $event); + + $refreshTokenEventEmitted = true; + } + ); + $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', 'client_secret' => 'bar', @@ -80,6 +104,14 @@ public function testRespondToRequest(): void $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M')); self::assertInstanceOf(RefreshTokenEntityInterface::class, $responseType->getRefreshToken()); + + if (!$accessTokenEventEmitted) { + self::fail('Access token issued event is not emitted.'); + } + + if (!$refreshTokenEventEmitted) { + self::fail('Refresh token issued event is not emitted.'); + } } public function testRespondToRequestNullRefreshToken(): void diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php index b2dbbadd2..34c8444c0 100644 --- a/tests/Grant/RefreshTokenGrantTest.php +++ b/tests/Grant/RefreshTokenGrantTest.php @@ -15,6 +15,9 @@ use League\OAuth2\Server\Repositories\ClientRepositoryInterface; use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; +use League\OAuth2\Server\RequestAccessTokenEvent; +use League\OAuth2\Server\RequestEvent; +use League\OAuth2\Server\RequestRefreshTokenEvent; use League\OAuth2\Server\ResponseTypes\BearerTokenResponse; use LeagueTests\Stubs\AccessTokenEntity; use LeagueTests\Stubs\ClientEntity; @@ -76,6 +79,27 @@ public function testRespondToRequest(): void $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->revokeRefreshTokens(true); + $accessTokenEventEmitted = false; + $refreshTokenEventEmitted = false; + + $grant->getListenerRegistry()->subscribeTo( + RequestEvent::ACCESS_TOKEN_ISSUED, + function ($event) use (&$accessTokenEventEmitted): void { + self::assertInstanceOf(RequestAccessTokenEvent::class, $event); + + $accessTokenEventEmitted = true; + } + ); + + $grant->getListenerRegistry()->subscribeTo( + RequestEvent::REFRESH_TOKEN_ISSUED, + function ($event) use (&$refreshTokenEventEmitted): void { + self::assertInstanceOf(RequestRefreshTokenEvent::class, $event); + + $refreshTokenEventEmitted = true; + } + ); + $oldRefreshToken = json_encode( [ 'client_id' => 'foo', @@ -106,6 +130,14 @@ public function testRespondToRequest(): void $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M')); self::assertInstanceOf(RefreshTokenEntityInterface::class, $responseType->getRefreshToken()); + + if (!$accessTokenEventEmitted) { + self::fail('Access token issued event is not emitted.'); + } + + if (!$refreshTokenEventEmitted) { + self::fail('Refresh token issued event is not emitted.'); + } } public function testRespondToRequestNullRefreshToken(): void From 53fbe69f811116daf7ba8ee018a4cb85515cad19 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Mon, 30 Dec 2024 00:24:50 +0330 Subject: [PATCH 6/6] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd9c5aa09..238833dfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed bug on setting interval visibility of device authorization grant (PR #1410) - Fix a bug where the new poll date were not persisted when `slow_down` error happens, because the exception is thrown before calling `persistDeviceCode`. (PR #1410) - Fix a bug where `slow_down` error response may have been returned even after the user has completed the auth flow (already approved / denied the request). (PR #1410) +- Emit `RequestAccessTokenEvent` and `RequestRefreshTokenEvent` events instead of the general `RequestEvent` event when an access / refresh token is issued using device authorization grant. (PR #1467) ## [9.1.0] - released 2024-11-21 ### Added