From 8503e58a255da272cee00821605ba9ef85e87e72 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 23 May 2024 14:56:20 +0330 Subject: [PATCH 01/10] fix scope on device code grant --- examples/src/Repositories/DeviceCodeRepository.php | 9 +++++++++ src/Grant/DeviceCodeGrant.php | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/examples/src/Repositories/DeviceCodeRepository.php b/examples/src/Repositories/DeviceCodeRepository.php index 9495c9a15..ccfb84351 100644 --- a/examples/src/Repositories/DeviceCodeRepository.php +++ b/examples/src/Repositories/DeviceCodeRepository.php @@ -17,6 +17,7 @@ use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface; use OAuth2ServerExamples\Entities\ClientEntity; use OAuth2ServerExamples\Entities\DeviceCodeEntity; +use OAuth2ServerExamples\Entities\ScopeEntity; class DeviceCodeRepository implements DeviceCodeRepositoryInterface { @@ -49,6 +50,14 @@ public function getDeviceCodeEntityByDeviceCode($deviceCode): ?DeviceCodeEntityI $deviceCodeEntity->setIdentifier($deviceCode); $deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('now +1 hour')); $deviceCodeEntity->setClient($clientEntity); + $deviceCodeEntity->setLastPolledAt(new DateTimeImmutable()); + + $scopes = []; + foreach ($scopes as $scope) { + $scopeEntity = new ScopeEntity(); + $scopeEntity->setIdentifier($scope); + $deviceCodeEntity->addScope($scopeEntity); + } // The user identifier should be set when the user authenticates on the // OAuth server, along with whether they approved the request diff --git a/src/Grant/DeviceCodeGrant.php b/src/Grant/DeviceCodeGrant.php index 3804e2027..dea6f03cf 100644 --- a/src/Grant/DeviceCodeGrant.php +++ b/src/Grant/DeviceCodeGrant.php @@ -137,8 +137,8 @@ public function respondToAccessTokenRequest( ): ResponseTypeInterface { // Validate request $client = $this->validateClient($request); - $scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope)); $deviceCodeEntity = $this->validateDeviceCode($request, $client); + $scopes = $deviceCodeEntity->getScopes(); $deviceCodeEntity->setLastPolledAt(new DateTimeImmutable()); $this->deviceCodeRepository->persistDeviceCode($deviceCodeEntity); From b3d24d0a0012f3651ccb8f2dbdc80a848404c3bf Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sat, 24 Aug 2024 19:12:24 +0330 Subject: [PATCH 02/10] formatting --- src/Grant/DeviceCodeGrant.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Grant/DeviceCodeGrant.php b/src/Grant/DeviceCodeGrant.php index dea6f03cf..9c9133408 100644 --- a/src/Grant/DeviceCodeGrant.php +++ b/src/Grant/DeviceCodeGrant.php @@ -138,7 +138,6 @@ public function respondToAccessTokenRequest( // Validate request $client = $this->validateClient($request); $deviceCodeEntity = $this->validateDeviceCode($request, $client); - $scopes = $deviceCodeEntity->getScopes(); $deviceCodeEntity->setLastPolledAt(new DateTimeImmutable()); $this->deviceCodeRepository->persistDeviceCode($deviceCodeEntity); @@ -153,7 +152,7 @@ public function respondToAccessTokenRequest( } // Finalize the requested scopes - $finalizedScopes = $this->scopeRepository->finalizeScopes($scopes, $this->getIdentifier(), $client, $deviceCodeEntity->getUserIdentifier()); + $finalizedScopes = $this->scopeRepository->finalizeScopes($deviceCodeEntity->getScopes(), $this->getIdentifier(), $client, $deviceCodeEntity->getUserIdentifier()); // Issue and persist new access token $accessToken = $this->issueAccessToken($accessTokenTTL, $client, $deviceCodeEntity->getUserIdentifier(), $finalizedScopes); From a94e67c155e70aa8c67c1341d4fca78f4aa9fcf8 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Fri, 11 Oct 2024 18:12:22 +0330 Subject: [PATCH 03/10] add tests and changelog entry --- CHANGELOG.md | 1 + tests/Grant/DeviceCodeGrantTest.php | 140 ++++++++-------------------- 2 files changed, 38 insertions(+), 103 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85c99ac6c..a3fe5f30d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - 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 scopes where not set on access token when using device authorization grant (PR #1412) ## [9.0.0] - released 2024-05-13 ### Added diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php index 396ea760f..fb6fba59e 100644 --- a/tests/Grant/DeviceCodeGrantTest.php +++ b/tests/Grant/DeviceCodeGrantTest.php @@ -28,9 +28,7 @@ use PHPUnit\Framework\TestCase; use function base64_encode; -use function json_encode; use function random_bytes; -use function time; use function uniqid; class DeviceCodeGrantTest extends TestCase @@ -261,6 +259,7 @@ public function testValidateDeviceAuthorizationRequestClientMismatch(): void public function testCompleteDeviceAuthorizationRequest(): void { $deviceCode = new DeviceCodeEntity(); + $deviceCode->setIdentifier('deviceCodeEntityIdentifier'); $deviceCode->setUserCode('foo'); $deviceCodeRepository = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock(); @@ -275,7 +274,7 @@ public function testCompleteDeviceAuthorizationRequest(): void $grant->setEncryptionKey($this->cryptStub->getKey()); - $grant->completeDeviceAuthorizationRequest($deviceCode->getUserCode(), 'userId', true); + $grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), 'userId', true); $this::assertEquals('userId', $deviceCode->getUserIdentifier()); } @@ -346,13 +345,8 @@ public function testRespondToAccessTokenRequest(): void $clientRepositoryMock->method('getClientEntity')->willReturn($client); $clientRepositoryMock->method('validateClient')->willReturn(true); - $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); - $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); - $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); - - $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); - $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); - $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity()); + $scope = new ScopeEntity(); + $scope->setIdentifier('foo'); $deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock(); $deviceCodeEntity = new DeviceCodeEntity(); @@ -362,12 +356,27 @@ public function testRespondToAccessTokenRequest(): void $deviceCodeEntity->setUserCode('123456'); $deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('+1 hour')); $deviceCodeEntity->setClient($client); + $deviceCodeEntity->addScope($scope); - $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); + $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode') + ->with($deviceCodeEntity->getIdentifier()) + ->willReturn($deviceCodeEntity); + + $accessTokenEntity = new AccessTokenEntity(); + $accessTokenEntity->addScope($scope); + + $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); + $accessTokenRepositoryMock->method('getNewToken') + ->with($client, $deviceCodeEntity->getScopes(), $deviceCodeEntity->getUserIdentifier()) + ->willReturn($accessTokenEntity); + $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); + + $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); + $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity()); - $scope = new ScopeEntity(); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -384,23 +393,11 @@ public function testRespondToAccessTokenRequest(): void $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); - $grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getUserCode(), '1', true); + $grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getIdentifier(), $deviceCodeEntity->getUserIdentifier(), true); $serverRequest = (new ServerRequest())->withParsedBody([ 'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code', - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() + 3600, - 'client_id' => 'foo', - 'user_code' => '12345678', - 'scopes' => ['foo'], - 'verification_uri' => 'http://foo/bar', - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => $deviceCodeEntity->getIdentifier(), 'client_id' => 'foo', ]); @@ -408,6 +405,7 @@ public function testRespondToAccessTokenRequest(): void $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M')); $this::assertInstanceOf(RefreshTokenEntityInterface::class, $responseType->getRefreshToken()); + $this::assertSame([$scope], $responseType->getAccessToken()->getScopes()); } public function testRespondToRequestMissingClient(): void @@ -430,19 +428,7 @@ public function testRespondToRequestMissingClient(): void $grant->setAccessTokenRepository($accessTokenRepositoryMock); $serverRequest = (new ServerRequest())->withQueryParams([ - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() + 3600, - 'client_id' => 'foo', - 'user_code' => '12345678', - 'scopes' => ['foo'], - 'verification_uri' => 'http://foo/bar', - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -469,9 +455,8 @@ public function testRespondToRequestMissingDeviceCode(): void $deviceCodeEntity->setUserIdentifier('baz'); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); - $scope = new ScopeEntity(); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -518,9 +503,8 @@ public function testIssueSlowDownError(): void $deviceCodeEntity->setClient($client); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); - $scope = new ScopeEntity(); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -538,19 +522,7 @@ public function testIssueSlowDownError(): void $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() + 3600, - 'client_id' => 'foo', - 'user_code' => '12345678', - 'scopes' => ['foo'], - 'verification_uri' => 'http://foo/bar', - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -579,9 +551,8 @@ public function testIssueAuthorizationPendingError(): void $deviceCodeEntity->setClient($client); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); - $scope = new ScopeEntity(); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -599,19 +570,7 @@ public function testIssueAuthorizationPendingError(): void $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() + 3600, - 'client_id' => 'foo', - 'user_code' => '12345678', - 'scopes' => ['foo'], - 'verification_uri' => 'http://foo/bar', - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -640,9 +599,8 @@ public function testIssueExpiredTokenError(): void $deviceCodeEntity->setClient($client); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); - $scope = new ScopeEntity(); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -660,19 +618,7 @@ public function testIssueExpiredTokenError(): void $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() - 3600, - 'client_id' => 'foo', - 'user_code' => '12345678', - 'scopes' => ['foo'], - 'verification_uri' => 'http://foo/bar', - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -746,14 +692,14 @@ public function testIssueAccessDeniedError(): void $deviceCode = new DeviceCodeEntity(); + $deviceCode->setIdentifier('deviceCodeEntityIdentifier'); $deviceCode->setExpiryDateTime(new DateTimeImmutable('+1 hour')); $deviceCode->setClient($client); $deviceCode->setUserCode('12345678'); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCode); - $scope = new ScopeEntity(); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -769,23 +715,11 @@ public function testIssueAccessDeniedError(): void $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); - $grant->completeDeviceAuthorizationRequest($deviceCode->getUserCode(), '1', false); + $grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), '1', false); $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() + 3600, - 'client_id' => 'foo', - 'user_code' => '12345678', - 'scopes' => ['foo'], - 'verification_uri' => 'http://foo/bar', - ], - JSON_THROW_ON_ERROR - ), - ), + 'device_code' => $deviceCode->getIdentifier(), ]); $responseType = new StubResponseType(); From 37f8537e3127fd9d991f5a8669f46a9e2096607b Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Fri, 11 Oct 2024 18:58:25 +0330 Subject: [PATCH 04/10] fix static analyse --- tests/Grant/DeviceCodeGrantTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php index fb6fba59e..b66d54c78 100644 --- a/tests/Grant/DeviceCodeGrantTest.php +++ b/tests/Grant/DeviceCodeGrantTest.php @@ -393,7 +393,7 @@ public function testRespondToAccessTokenRequest(): void $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); - $grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getIdentifier(), $deviceCodeEntity->getUserIdentifier(), true); + $grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getIdentifier(), 'baz', true); $serverRequest = (new ServerRequest())->withParsedBody([ 'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code', From 7570854ff9b31f1d7da77c0611324b76c714fb8c Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 15 Oct 2024 01:55:43 +0330 Subject: [PATCH 05/10] formatting --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 333020958..52cf4e850 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -656,7 +656,8 @@ Version 5 is a complete code rewrite. - First major release -[Unreleased]: https://github.com/thephpleague/oauth2-server/compare/9.0.0...HEAD +[Unreleased]: https://github.com/thephpleague/oauth2-server/compare/9.0.1...HEAD +[9.0.1]: https://github.com/thephpleague/oauth2-server/compare/9.0.0...9.0.1 [9.0.0]: https://github.com/thephpleague/oauth2-server/compare/9.0.0-RC1...9.0.0 [9.0.0-RC1]: https://github.com/thephpleague/oauth2-server/compare/8.5.4...9.0.0-RC1 [8.5.4]: https://github.com/thephpleague/oauth2-server/compare/8.5.3...8.5.4 From 55dd7c789f5d04f13a08b727625cb99d1f475dbb Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Mon, 28 Oct 2024 03:17:53 +0330 Subject: [PATCH 06/10] fix a typo Co-authored-by: Andrew Millington --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52cf4e850..7e77c0220 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] -- Fixed bug where scopes where not set on access token when using device authorization grant (PR #1412) +- Fixed bug where scopes were not set on access token when using device authorization grant (PR #1412) ## [9.0.1] - released 2024-10-14 ### Fixed From 7522a4603a22d50d5ac0a10eba30e6a7a446d876 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Mon, 28 Oct 2024 05:12:04 +0330 Subject: [PATCH 07/10] no encryption on this grant --- tests/Grant/DeviceCodeGrantTest.php | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php index b66d54c78..4a9cebbd1 100644 --- a/tests/Grant/DeviceCodeGrantTest.php +++ b/tests/Grant/DeviceCodeGrantTest.php @@ -20,7 +20,6 @@ use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; use LeagueTests\Stubs\AccessTokenEntity; use LeagueTests\Stubs\ClientEntity; -use LeagueTests\Stubs\CryptTraitStub; use LeagueTests\Stubs\DeviceCodeEntity; use LeagueTests\Stubs\RefreshTokenEntity; use LeagueTests\Stubs\ScopeEntity; @@ -36,13 +35,6 @@ class DeviceCodeGrantTest extends TestCase private const DEFAULT_SCOPE = 'basic'; private const INTERVAL_RATE = 10; - protected CryptTraitStub $cryptStub; - - public function setUp(): void - { - $this->cryptStub = new CryptTraitStub(); - } - public function testGetIdentifier(): void { $deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock(); @@ -100,7 +92,6 @@ public function testRespondToDeviceAuthorizationRequest(): void $grant->setClientRepository($clientRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setScopeRepository($scopeRepositoryMock); $request = (new ServerRequest())->withParsedBody([ @@ -145,7 +136,6 @@ public function testRespondToDeviceAuthorizationRequestWithVerificationUriComple $grant->setClientRepository($clientRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setScopeRepository($scopeRepositoryMock); $request = (new ServerRequest())->withParsedBody([ @@ -272,8 +262,6 @@ public function testCompleteDeviceAuthorizationRequest(): void 'http://foo/bar', ); - $grant->setEncryptionKey($this->cryptStub->getKey()); - $grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), 'userId', true); $this::assertEquals('userId', $deviceCode->getUserIdentifier()); @@ -323,8 +311,6 @@ public function testDeviceAuthorizationResponse(): void 'http://foo/bar' ); - $deviceCodeGrant->setEncryptionKey($this->cryptStub->getKey()); - $server->enableGrantType($deviceCodeGrant); $response = $server->respondToDeviceAuthorizationRequest($serverRequest, new Response()); @@ -390,7 +376,6 @@ public function testRespondToAccessTokenRequest(): void $grant->setAccessTokenRepository($accessTokenRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getIdentifier(), 'baz', true); @@ -469,7 +454,6 @@ public function testRespondToRequestMissingDeviceCode(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ @@ -517,7 +501,6 @@ public function testIssueSlowDownError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ @@ -565,7 +548,6 @@ public function testIssueAuthorizationPendingError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ @@ -613,7 +595,6 @@ public function testIssueExpiredTokenError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ @@ -657,7 +638,6 @@ public function testSettingDeviceCodeIntervalRate(): void $grant->setClientRepository($clientRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setScopeRepository($scopeRepositoryMock); $grant->setIntervalVisibility(true); @@ -712,7 +692,6 @@ public function testIssueAccessDeniedError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), '1', false); From c9639be6f7415e933614e85f3bce9e115b07863e Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Mon, 28 Oct 2024 23:50:44 +0330 Subject: [PATCH 08/10] encrypt / decrypt the device code formatting formatting simplify tests revert example changes formatting fix style fix sa encrypt device code Revert "no encryption on this grant" This reverts commit 7522a4603a22d50d5ac0a10eba30e6a7a446d876. --- CHANGELOG.md | 2 +- .../src/Repositories/DeviceCodeRepository.php | 9 -- src/Grant/DeviceCodeGrant.php | 46 ++++-- src/ResponseTypes/DeviceCodeResponse.php | 15 +- tests/Grant/DeviceCodeGrantTest.php | 144 ++++++++++++++---- .../DeviceCodeResponseTypeTest.php | 20 ++- 6 files changed, 179 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83bb155e6..485a8ec64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Support for PHP 8.4 (PR #1454) ### Fixed -- Fixed bug where scopes were not set on access token when using device authorization grant (PR #1412) +- Fixed device code encryption / decryption and bug where scopes were not set on access token when using device authorization grant (PR #1412) ## [9.0.1] - released 2024-10-14 ### Fixed diff --git a/examples/src/Repositories/DeviceCodeRepository.php b/examples/src/Repositories/DeviceCodeRepository.php index ccfb84351..9495c9a15 100644 --- a/examples/src/Repositories/DeviceCodeRepository.php +++ b/examples/src/Repositories/DeviceCodeRepository.php @@ -17,7 +17,6 @@ use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface; use OAuth2ServerExamples\Entities\ClientEntity; use OAuth2ServerExamples\Entities\DeviceCodeEntity; -use OAuth2ServerExamples\Entities\ScopeEntity; class DeviceCodeRepository implements DeviceCodeRepositoryInterface { @@ -50,14 +49,6 @@ public function getDeviceCodeEntityByDeviceCode($deviceCode): ?DeviceCodeEntityI $deviceCodeEntity->setIdentifier($deviceCode); $deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('now +1 hour')); $deviceCodeEntity->setClient($clientEntity); - $deviceCodeEntity->setLastPolledAt(new DateTimeImmutable()); - - $scopes = []; - foreach ($scopes as $scope) { - $scopeEntity = new ScopeEntity(); - $scopeEntity->setIdentifier($scope); - $deviceCodeEntity->addScope($scopeEntity); - } // The user identifier should be set when the user authenticates on the // OAuth server, along with whether they approved the request diff --git a/src/Grant/DeviceCodeGrant.php b/src/Grant/DeviceCodeGrant.php index 9c9133408..374db16bb 100644 --- a/src/Grant/DeviceCodeGrant.php +++ b/src/Grant/DeviceCodeGrant.php @@ -28,6 +28,7 @@ use League\OAuth2\Server\RequestEvent; use League\OAuth2\Server\ResponseTypes\DeviceCodeResponse; use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; +use LogicException; use Psr\Http\Message\ServerRequestInterface; use TypeError; @@ -96,6 +97,7 @@ public function respondToDeviceAuthorizationRequest(ServerRequestInterface $requ ); $response = new DeviceCodeResponse(); + $response->setEncryptionKey($this->encryptionKey); if ($this->includeVerificationUriComplete === true) { $response->includeVerificationUriComplete(); @@ -177,38 +179,58 @@ public function respondToAccessTokenRequest( */ protected function validateDeviceCode(ServerRequestInterface $request, ClientEntityInterface $client): DeviceCodeEntityInterface { - $deviceCode = $this->getRequestParameter('device_code', $request); + $encryptedDeviceCode = $this->getRequestParameter('device_code', $request); - if (is_null($deviceCode)) { + if (is_null($encryptedDeviceCode)) { throw OAuthServerException::invalidRequest('device_code'); } - $deviceCodeEntity = $this->deviceCodeRepository->getDeviceCodeEntityByDeviceCode( - $deviceCode - ); - - if ($deviceCodeEntity instanceof DeviceCodeEntityInterface === false) { - $this->getEmitter()->emit(new RequestEvent(RequestEvent::USER_AUTHENTICATION_FAILED, $request)); + try { + $deviceCodePayload = json_decode($this->decrypt($encryptedDeviceCode)); + } catch (LogicException $e) { + throw OAuthServerException::invalidRequest('code', 'Cannot decrypt the device code', $e); + } - throw OAuthServerException::invalidGrant(); + if (!property_exists($deviceCodePayload, 'device_code_id')) { + throw OAuthServerException::invalidRequest('device_code', 'Device code malformed'); } - if (time() > $deviceCodeEntity->getExpiryDateTime()->getTimestamp()) { + if (time() > $deviceCodePayload->expire_time) { throw OAuthServerException::expiredToken('device_code'); } - if ($this->deviceCodeRepository->isDeviceCodeRevoked($deviceCode) === true) { + if ($this->deviceCodeRepository->isDeviceCodeRevoked($deviceCodePayload->device_code_id) === true) { throw OAuthServerException::invalidRequest('device_code', 'Device code has been revoked'); } - if ($deviceCodeEntity->getClient()->getIdentifier() !== $client->getIdentifier()) { + if ($deviceCodePayload->client_id !== $client->getIdentifier()) { throw OAuthServerException::invalidRequest('device_code', 'Device code was not issued to this client'); } + $deviceCodeEntity = $this->deviceCodeRepository->getDeviceCodeEntityByDeviceCode( + $deviceCodePayload->device_code_id + ); + + if ($deviceCodeEntity instanceof DeviceCodeEntityInterface === false) { + $this->getEmitter()->emit(new RequestEvent(RequestEvent::USER_AUTHENTICATION_FAILED, $request)); + + throw OAuthServerException::invalidGrant(); + } + if ($this->deviceCodePolledTooSoon($deviceCodeEntity->getLastPolledAt()) === true) { throw OAuthServerException::slowDown(); } + $deviceCodeEntity->setIdentifier($deviceCodePayload->device_code_id); + $deviceCodeEntity->setClient($client); + $deviceCodeEntity->setExpiryDateTime((new DateTimeImmutable())->setTimestamp($deviceCodePayload->expire_time)); + + $scopes = $this->validateScopes($deviceCodePayload->scopes); + + foreach ($scopes as $scope) { + $deviceCodeEntity->addScope($scope); + } + return $deviceCodeEntity; } diff --git a/src/ResponseTypes/DeviceCodeResponse.php b/src/ResponseTypes/DeviceCodeResponse.php index 91a8df69a..e55c0ea41 100644 --- a/src/ResponseTypes/DeviceCodeResponse.php +++ b/src/ResponseTypes/DeviceCodeResponse.php @@ -34,8 +34,21 @@ public function generateHttpResponse(ResponseInterface $response): ResponseInter { $expireDateTime = $this->deviceCodeEntity->getExpiryDateTime()->getTimestamp(); + $payload = [ + 'client_id' => $this->deviceCodeEntity->getClient()->getIdentifier(), + 'device_code_id' => $this->deviceCodeEntity->getIdentifier(), + 'scopes' => $this->deviceCodeEntity->getScopes(), + 'expire_time' => $expireDateTime, + ]; + + $jsonPayload = json_encode($payload); + + if ($jsonPayload === false) { + throw new LogicException('An error was encountered when JSON encoding the device code request response'); + } + $responseParams = [ - 'device_code' => $this->deviceCodeEntity->getIdentifier(), + 'device_code' => $this->encrypt($jsonPayload), 'user_code' => $this->deviceCodeEntity->getUserCode(), 'verification_uri' => $this->deviceCodeEntity->getVerificationUri(), 'expires_in' => $expireDateTime - time(), diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php index 4a9cebbd1..7b4e09f1a 100644 --- a/tests/Grant/DeviceCodeGrantTest.php +++ b/tests/Grant/DeviceCodeGrantTest.php @@ -20,6 +20,7 @@ use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; use LeagueTests\Stubs\AccessTokenEntity; use LeagueTests\Stubs\ClientEntity; +use LeagueTests\Stubs\CryptTraitStub; use LeagueTests\Stubs\DeviceCodeEntity; use LeagueTests\Stubs\RefreshTokenEntity; use LeagueTests\Stubs\ScopeEntity; @@ -27,7 +28,9 @@ use PHPUnit\Framework\TestCase; use function base64_encode; +use function json_encode; use function random_bytes; +use function time; use function uniqid; class DeviceCodeGrantTest extends TestCase @@ -35,6 +38,13 @@ class DeviceCodeGrantTest extends TestCase private const DEFAULT_SCOPE = 'basic'; private const INTERVAL_RATE = 10; + protected CryptTraitStub $cryptStub; + + public function setUp(): void + { + $this->cryptStub = new CryptTraitStub(); + } + public function testGetIdentifier(): void { $deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock(); @@ -92,6 +102,7 @@ public function testRespondToDeviceAuthorizationRequest(): void $grant->setClientRepository($clientRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); + $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setScopeRepository($scopeRepositoryMock); $request = (new ServerRequest())->withParsedBody([ @@ -136,6 +147,7 @@ public function testRespondToDeviceAuthorizationRequestWithVerificationUriComple $grant->setClientRepository($clientRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); + $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setScopeRepository($scopeRepositoryMock); $request = (new ServerRequest())->withParsedBody([ @@ -262,6 +274,8 @@ public function testCompleteDeviceAuthorizationRequest(): void 'http://foo/bar', ); + $grant->setEncryptionKey($this->cryptStub->getKey()); + $grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), 'userId', true); $this::assertEquals('userId', $deviceCode->getUserIdentifier()); @@ -311,6 +325,8 @@ public function testDeviceAuthorizationResponse(): void 'http://foo/bar' ); + $deviceCodeGrant->setEncryptionKey($this->cryptStub->getKey()); + $server->enableGrantType($deviceCodeGrant); $response = $server->respondToDeviceAuthorizationRequest($serverRequest, new Response()); @@ -334,35 +350,28 @@ public function testRespondToAccessTokenRequest(): void $scope = new ScopeEntity(); $scope->setIdentifier('foo'); - $deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock(); - $deviceCodeEntity = new DeviceCodeEntity(); - - $deviceCodeEntity->setUserIdentifier('baz'); - $deviceCodeEntity->setIdentifier('deviceCodeEntityIdentifier'); - $deviceCodeEntity->setUserCode('123456'); - $deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('+1 hour')); - $deviceCodeEntity->setClient($client); - $deviceCodeEntity->addScope($scope); - - $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode') - ->with($deviceCodeEntity->getIdentifier()) - ->willReturn($deviceCodeEntity); - $accessTokenEntity = new AccessTokenEntity(); $accessTokenEntity->addScope($scope); - $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); - $accessTokenRepositoryMock->method('getNewToken') - ->with($client, $deviceCodeEntity->getScopes(), $deviceCodeEntity->getUserIdentifier()) - ->willReturn($accessTokenEntity); + $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessTokenEntity); $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity()); + $deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock(); + $deviceCodeEntity = new DeviceCodeEntity(); + $deviceCodeEntity->setUserIdentifier('baz'); + $deviceCodeEntity->setIdentifier('deviceCodeEntityIdentifier'); + $deviceCodeEntity->setUserCode('123456'); + $deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('+1 hour')); + $deviceCodeEntity->setClient($client); + + $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -376,13 +385,24 @@ public function testRespondToAccessTokenRequest(): void $grant->setAccessTokenRepository($accessTokenRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); + $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getIdentifier(), 'baz', true); $serverRequest = (new ServerRequest())->withParsedBody([ 'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code', - 'device_code' => $deviceCodeEntity->getIdentifier(), + 'device_code' => $this->cryptStub->doEncrypt( + json_encode( + [ + 'device_code_id' => 'deviceCodeEntityIdentifier', + 'expire_time' => time() + 3600, + 'client_id' => 'foo', + 'scopes' => ['foo'], + ], + JSON_THROW_ON_ERROR + ) + ), 'client_id' => 'foo', ]); @@ -413,7 +433,17 @@ public function testRespondToRequestMissingClient(): void $grant->setAccessTokenRepository($accessTokenRepositoryMock); $serverRequest = (new ServerRequest())->withQueryParams([ - 'device_code' => uniqid(), + 'device_code' => $this->cryptStub->doEncrypt( + json_encode( + [ + 'device_code_id' => uniqid(), + 'expire_time' => time() + 3600, + 'client_id' => 'foo', + 'scopes' => ['foo'], + ], + JSON_THROW_ON_ERROR + ) + ), ]); $responseType = new StubResponseType(); @@ -440,8 +470,9 @@ public function testRespondToRequestMissingDeviceCode(): void $deviceCodeEntity->setUserIdentifier('baz'); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); + $scope = new ScopeEntity(); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -454,6 +485,7 @@ public function testRespondToRequestMissingDeviceCode(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); + $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ @@ -487,8 +519,10 @@ public function testIssueSlowDownError(): void $deviceCodeEntity->setClient($client); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); + $scope = new ScopeEntity(); + $scope->setIdentifier('foo'); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -501,11 +535,22 @@ public function testIssueSlowDownError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); + $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => uniqid(), + 'device_code' => $this->cryptStub->doEncrypt( + json_encode( + [ + 'device_code_id' => uniqid(), + 'expire_time' => time() + 3600, + 'client_id' => 'foo', + 'scopes' => ['foo'], + ], + JSON_THROW_ON_ERROR + ) + ), ]); $responseType = new StubResponseType(); @@ -534,8 +579,10 @@ public function testIssueAuthorizationPendingError(): void $deviceCodeEntity->setClient($client); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); + $scope = new ScopeEntity(); + $scope->setIdentifier('foo'); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -548,11 +595,22 @@ public function testIssueAuthorizationPendingError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); + $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => uniqid(), + 'device_code' => $this->cryptStub->doEncrypt( + json_encode( + [ + 'device_code_id' => uniqid(), + 'expire_time' => time() + 3600, + 'client_id' => 'foo', + 'scopes' => ['foo'], + ], + JSON_THROW_ON_ERROR + ) + ), ]); $responseType = new StubResponseType(); @@ -581,8 +639,9 @@ public function testIssueExpiredTokenError(): void $deviceCodeEntity->setClient($client); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); + $scope = new ScopeEntity(); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -595,11 +654,22 @@ public function testIssueExpiredTokenError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); + $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => uniqid(), + 'device_code' => $this->cryptStub->doEncrypt( + json_encode( + [ + 'device_code_id' => uniqid(), + 'expire_time' => time() - 3600, + 'client_id' => 'foo', + 'scopes' => ['foo'], + ], + JSON_THROW_ON_ERROR + ) + ), ]); $responseType = new StubResponseType(); @@ -638,6 +708,7 @@ public function testSettingDeviceCodeIntervalRate(): void $grant->setClientRepository($clientRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); + $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setScopeRepository($scopeRepositoryMock); $grant->setIntervalVisibility(true); @@ -678,8 +749,10 @@ public function testIssueAccessDeniedError(): void $deviceCode->setUserCode('12345678'); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCode); + $scope = new ScopeEntity(); + $scope->setIdentifier('foo'); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -692,13 +765,24 @@ public function testIssueAccessDeniedError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); + $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), '1', false); $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => $deviceCode->getIdentifier(), + 'device_code' => $this->cryptStub->doEncrypt( + json_encode( + [ + 'device_code_id' => uniqid(), + 'expire_time' => time() + 3600, + 'client_id' => 'foo', + 'scopes' => ['foo'], + ], + JSON_THROW_ON_ERROR + ), + ), ]); $responseType = new StubResponseType(); diff --git a/tests/ResponseTypes/DeviceCodeResponseTypeTest.php b/tests/ResponseTypes/DeviceCodeResponseTypeTest.php index 93bd9d6b3..f8dc265ff 100644 --- a/tests/ResponseTypes/DeviceCodeResponseTypeTest.php +++ b/tests/ResponseTypes/DeviceCodeResponseTypeTest.php @@ -10,21 +10,28 @@ use League\OAuth2\Server\CryptKey; use League\OAuth2\Server\ResponseTypes\DeviceCodeResponse; use LeagueTests\Stubs\ClientEntity; +use LeagueTests\Stubs\CryptTraitStub; use LeagueTests\Stubs\DeviceCodeEntity; use LeagueTests\Stubs\ScopeEntity; use PHPUnit\Framework\TestCase; -use function base64_encode; use function json_decode; -use function random_bytes; +use function json_encode; class DeviceCodeResponseTypeTest extends TestCase { + protected CryptTraitStub $cryptStub; + + public function setUp(): void + { + $this->cryptStub = new CryptTraitStub(); + } + public function testGenerateHttpResponse(): void { $responseType = new DeviceCodeResponse(); $responseType->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); - $responseType->setEncryptionKey(base64_encode(random_bytes(36))); + $responseType->setEncryptionKey($this->cryptStub->getKey()); $client = new ClientEntity(); $client->setIdentifier('clientName'); @@ -53,7 +60,12 @@ public function testGenerateHttpResponse(): void $json = json_decode($response->getBody()->getContents()); $this::assertObjectHasProperty('expires_in', $json); $this::assertObjectHasProperty('device_code', $json); - $this::assertEquals('abcdef', $json->device_code); + $this::assertSame(json_encode([ + 'client_id' => 'clientName', + 'device_code_id' => 'abcdef', + 'scopes' => ['basic'], + 'expire_time' => $deviceCode->getExpiryDateTime()->getTimestamp(), + ]), $this->cryptStub->doDecrypt($json->device_code)); $this::assertObjectHasProperty('verification_uri', $json); $this::assertObjectHasProperty('user_code', $json); } From 3ff445bf3df8cc1980932a974af5d899450b0e1c Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Wed, 13 Nov 2024 01:44:46 +0330 Subject: [PATCH 09/10] Revert "encrypt / decrypt the device code" This reverts commit c9639be6f7415e933614e85f3bce9e115b07863e. --- CHANGELOG.md | 2 +- .../src/Repositories/DeviceCodeRepository.php | 9 ++ src/Grant/DeviceCodeGrant.php | 46 ++---- src/ResponseTypes/DeviceCodeResponse.php | 15 +- tests/Grant/DeviceCodeGrantTest.php | 144 ++++-------------- .../DeviceCodeResponseTypeTest.php | 20 +-- 6 files changed, 57 insertions(+), 179 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 485a8ec64..83bb155e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Support for PHP 8.4 (PR #1454) ### Fixed -- Fixed device code encryption / decryption and bug where scopes were not set on access token when using device authorization grant (PR #1412) +- Fixed bug where scopes were not set on access token when using device authorization grant (PR #1412) ## [9.0.1] - released 2024-10-14 ### Fixed diff --git a/examples/src/Repositories/DeviceCodeRepository.php b/examples/src/Repositories/DeviceCodeRepository.php index 9495c9a15..ccfb84351 100644 --- a/examples/src/Repositories/DeviceCodeRepository.php +++ b/examples/src/Repositories/DeviceCodeRepository.php @@ -17,6 +17,7 @@ use League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface; use OAuth2ServerExamples\Entities\ClientEntity; use OAuth2ServerExamples\Entities\DeviceCodeEntity; +use OAuth2ServerExamples\Entities\ScopeEntity; class DeviceCodeRepository implements DeviceCodeRepositoryInterface { @@ -49,6 +50,14 @@ public function getDeviceCodeEntityByDeviceCode($deviceCode): ?DeviceCodeEntityI $deviceCodeEntity->setIdentifier($deviceCode); $deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('now +1 hour')); $deviceCodeEntity->setClient($clientEntity); + $deviceCodeEntity->setLastPolledAt(new DateTimeImmutable()); + + $scopes = []; + foreach ($scopes as $scope) { + $scopeEntity = new ScopeEntity(); + $scopeEntity->setIdentifier($scope); + $deviceCodeEntity->addScope($scopeEntity); + } // The user identifier should be set when the user authenticates on the // OAuth server, along with whether they approved the request diff --git a/src/Grant/DeviceCodeGrant.php b/src/Grant/DeviceCodeGrant.php index 374db16bb..9c9133408 100644 --- a/src/Grant/DeviceCodeGrant.php +++ b/src/Grant/DeviceCodeGrant.php @@ -28,7 +28,6 @@ use League\OAuth2\Server\RequestEvent; use League\OAuth2\Server\ResponseTypes\DeviceCodeResponse; use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; -use LogicException; use Psr\Http\Message\ServerRequestInterface; use TypeError; @@ -97,7 +96,6 @@ public function respondToDeviceAuthorizationRequest(ServerRequestInterface $requ ); $response = new DeviceCodeResponse(); - $response->setEncryptionKey($this->encryptionKey); if ($this->includeVerificationUriComplete === true) { $response->includeVerificationUriComplete(); @@ -179,58 +177,38 @@ public function respondToAccessTokenRequest( */ protected function validateDeviceCode(ServerRequestInterface $request, ClientEntityInterface $client): DeviceCodeEntityInterface { - $encryptedDeviceCode = $this->getRequestParameter('device_code', $request); + $deviceCode = $this->getRequestParameter('device_code', $request); - if (is_null($encryptedDeviceCode)) { + if (is_null($deviceCode)) { throw OAuthServerException::invalidRequest('device_code'); } - try { - $deviceCodePayload = json_decode($this->decrypt($encryptedDeviceCode)); - } catch (LogicException $e) { - throw OAuthServerException::invalidRequest('code', 'Cannot decrypt the device code', $e); - } + $deviceCodeEntity = $this->deviceCodeRepository->getDeviceCodeEntityByDeviceCode( + $deviceCode + ); - if (!property_exists($deviceCodePayload, 'device_code_id')) { - throw OAuthServerException::invalidRequest('device_code', 'Device code malformed'); + if ($deviceCodeEntity instanceof DeviceCodeEntityInterface === false) { + $this->getEmitter()->emit(new RequestEvent(RequestEvent::USER_AUTHENTICATION_FAILED, $request)); + + throw OAuthServerException::invalidGrant(); } - if (time() > $deviceCodePayload->expire_time) { + if (time() > $deviceCodeEntity->getExpiryDateTime()->getTimestamp()) { throw OAuthServerException::expiredToken('device_code'); } - if ($this->deviceCodeRepository->isDeviceCodeRevoked($deviceCodePayload->device_code_id) === true) { + if ($this->deviceCodeRepository->isDeviceCodeRevoked($deviceCode) === true) { throw OAuthServerException::invalidRequest('device_code', 'Device code has been revoked'); } - if ($deviceCodePayload->client_id !== $client->getIdentifier()) { + if ($deviceCodeEntity->getClient()->getIdentifier() !== $client->getIdentifier()) { throw OAuthServerException::invalidRequest('device_code', 'Device code was not issued to this client'); } - $deviceCodeEntity = $this->deviceCodeRepository->getDeviceCodeEntityByDeviceCode( - $deviceCodePayload->device_code_id - ); - - if ($deviceCodeEntity instanceof DeviceCodeEntityInterface === false) { - $this->getEmitter()->emit(new RequestEvent(RequestEvent::USER_AUTHENTICATION_FAILED, $request)); - - throw OAuthServerException::invalidGrant(); - } - if ($this->deviceCodePolledTooSoon($deviceCodeEntity->getLastPolledAt()) === true) { throw OAuthServerException::slowDown(); } - $deviceCodeEntity->setIdentifier($deviceCodePayload->device_code_id); - $deviceCodeEntity->setClient($client); - $deviceCodeEntity->setExpiryDateTime((new DateTimeImmutable())->setTimestamp($deviceCodePayload->expire_time)); - - $scopes = $this->validateScopes($deviceCodePayload->scopes); - - foreach ($scopes as $scope) { - $deviceCodeEntity->addScope($scope); - } - return $deviceCodeEntity; } diff --git a/src/ResponseTypes/DeviceCodeResponse.php b/src/ResponseTypes/DeviceCodeResponse.php index e55c0ea41..91a8df69a 100644 --- a/src/ResponseTypes/DeviceCodeResponse.php +++ b/src/ResponseTypes/DeviceCodeResponse.php @@ -34,21 +34,8 @@ public function generateHttpResponse(ResponseInterface $response): ResponseInter { $expireDateTime = $this->deviceCodeEntity->getExpiryDateTime()->getTimestamp(); - $payload = [ - 'client_id' => $this->deviceCodeEntity->getClient()->getIdentifier(), - 'device_code_id' => $this->deviceCodeEntity->getIdentifier(), - 'scopes' => $this->deviceCodeEntity->getScopes(), - 'expire_time' => $expireDateTime, - ]; - - $jsonPayload = json_encode($payload); - - if ($jsonPayload === false) { - throw new LogicException('An error was encountered when JSON encoding the device code request response'); - } - $responseParams = [ - 'device_code' => $this->encrypt($jsonPayload), + 'device_code' => $this->deviceCodeEntity->getIdentifier(), 'user_code' => $this->deviceCodeEntity->getUserCode(), 'verification_uri' => $this->deviceCodeEntity->getVerificationUri(), 'expires_in' => $expireDateTime - time(), diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php index 7b4e09f1a..4a9cebbd1 100644 --- a/tests/Grant/DeviceCodeGrantTest.php +++ b/tests/Grant/DeviceCodeGrantTest.php @@ -20,7 +20,6 @@ use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; use LeagueTests\Stubs\AccessTokenEntity; use LeagueTests\Stubs\ClientEntity; -use LeagueTests\Stubs\CryptTraitStub; use LeagueTests\Stubs\DeviceCodeEntity; use LeagueTests\Stubs\RefreshTokenEntity; use LeagueTests\Stubs\ScopeEntity; @@ -28,9 +27,7 @@ use PHPUnit\Framework\TestCase; use function base64_encode; -use function json_encode; use function random_bytes; -use function time; use function uniqid; class DeviceCodeGrantTest extends TestCase @@ -38,13 +35,6 @@ class DeviceCodeGrantTest extends TestCase private const DEFAULT_SCOPE = 'basic'; private const INTERVAL_RATE = 10; - protected CryptTraitStub $cryptStub; - - public function setUp(): void - { - $this->cryptStub = new CryptTraitStub(); - } - public function testGetIdentifier(): void { $deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock(); @@ -102,7 +92,6 @@ public function testRespondToDeviceAuthorizationRequest(): void $grant->setClientRepository($clientRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setScopeRepository($scopeRepositoryMock); $request = (new ServerRequest())->withParsedBody([ @@ -147,7 +136,6 @@ public function testRespondToDeviceAuthorizationRequestWithVerificationUriComple $grant->setClientRepository($clientRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setScopeRepository($scopeRepositoryMock); $request = (new ServerRequest())->withParsedBody([ @@ -274,8 +262,6 @@ public function testCompleteDeviceAuthorizationRequest(): void 'http://foo/bar', ); - $grant->setEncryptionKey($this->cryptStub->getKey()); - $grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), 'userId', true); $this::assertEquals('userId', $deviceCode->getUserIdentifier()); @@ -325,8 +311,6 @@ public function testDeviceAuthorizationResponse(): void 'http://foo/bar' ); - $deviceCodeGrant->setEncryptionKey($this->cryptStub->getKey()); - $server->enableGrantType($deviceCodeGrant); $response = $server->respondToDeviceAuthorizationRequest($serverRequest, new Response()); @@ -350,28 +334,35 @@ public function testRespondToAccessTokenRequest(): void $scope = new ScopeEntity(); $scope->setIdentifier('foo'); - $accessTokenEntity = new AccessTokenEntity(); - $accessTokenEntity->addScope($scope); - $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); - $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessTokenEntity); - $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); - - $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); - $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); - $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity()); - $deviceCodeRepositoryMock = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock(); $deviceCodeEntity = new DeviceCodeEntity(); + $deviceCodeEntity->setUserIdentifier('baz'); $deviceCodeEntity->setIdentifier('deviceCodeEntityIdentifier'); $deviceCodeEntity->setUserCode('123456'); $deviceCodeEntity->setExpiryDateTime(new DateTimeImmutable('+1 hour')); $deviceCodeEntity->setClient($client); + $deviceCodeEntity->addScope($scope); - $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); + $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode') + ->with($deviceCodeEntity->getIdentifier()) + ->willReturn($deviceCodeEntity); + + $accessTokenEntity = new AccessTokenEntity(); + $accessTokenEntity->addScope($scope); + + $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); + $accessTokenRepositoryMock->method('getNewToken') + ->with($client, $deviceCodeEntity->getScopes(), $deviceCodeEntity->getUserIdentifier()) + ->willReturn($accessTokenEntity); + $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); + + $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); + $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity()); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -385,24 +376,13 @@ public function testRespondToAccessTokenRequest(): void $grant->setAccessTokenRepository($accessTokenRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->completeDeviceAuthorizationRequest($deviceCodeEntity->getIdentifier(), 'baz', true); $serverRequest = (new ServerRequest())->withParsedBody([ 'grant_type' => 'urn:ietf:params:oauth:grant-type:device_code', - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => 'deviceCodeEntityIdentifier', - 'expire_time' => time() + 3600, - 'client_id' => 'foo', - 'scopes' => ['foo'], - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => $deviceCodeEntity->getIdentifier(), 'client_id' => 'foo', ]); @@ -433,17 +413,7 @@ public function testRespondToRequestMissingClient(): void $grant->setAccessTokenRepository($accessTokenRepositoryMock); $serverRequest = (new ServerRequest())->withQueryParams([ - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() + 3600, - 'client_id' => 'foo', - 'scopes' => ['foo'], - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -470,9 +440,8 @@ public function testRespondToRequestMissingDeviceCode(): void $deviceCodeEntity->setUserIdentifier('baz'); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); - $scope = new ScopeEntity(); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -485,7 +454,6 @@ public function testRespondToRequestMissingDeviceCode(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ @@ -519,10 +487,8 @@ public function testIssueSlowDownError(): void $deviceCodeEntity->setClient($client); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); - $scope = new ScopeEntity(); - $scope->setIdentifier('foo'); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -535,22 +501,11 @@ public function testIssueSlowDownError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() + 3600, - 'client_id' => 'foo', - 'scopes' => ['foo'], - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -579,10 +534,8 @@ public function testIssueAuthorizationPendingError(): void $deviceCodeEntity->setClient($client); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); - $scope = new ScopeEntity(); - $scope->setIdentifier('foo'); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -595,22 +548,11 @@ public function testIssueAuthorizationPendingError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() + 3600, - 'client_id' => 'foo', - 'scopes' => ['foo'], - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -639,9 +581,8 @@ public function testIssueExpiredTokenError(): void $deviceCodeEntity->setClient($client); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCodeEntity); - $scope = new ScopeEntity(); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -654,22 +595,11 @@ public function testIssueExpiredTokenError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() - 3600, - 'client_id' => 'foo', - 'scopes' => ['foo'], - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -708,7 +638,6 @@ public function testSettingDeviceCodeIntervalRate(): void $grant->setClientRepository($clientRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setScopeRepository($scopeRepositoryMock); $grant->setIntervalVisibility(true); @@ -749,10 +678,8 @@ public function testIssueAccessDeniedError(): void $deviceCode->setUserCode('12345678'); $deviceCodeRepositoryMock->method('getDeviceCodeEntityByDeviceCode')->willReturn($deviceCode); - $scope = new ScopeEntity(); - $scope->setIdentifier('foo'); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); - $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + $scopeRepositoryMock->expects(self::never())->method('getScopeEntityByIdentifier'); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new DeviceCodeGrant( @@ -765,24 +692,13 @@ public function testIssueAccessDeniedError(): void $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setDefaultScope(self::DEFAULT_SCOPE); - $grant->setEncryptionKey($this->cryptStub->getKey()); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), '1', false); $serverRequest = (new ServerRequest())->withParsedBody([ 'client_id' => 'foo', - 'device_code' => $this->cryptStub->doEncrypt( - json_encode( - [ - 'device_code_id' => uniqid(), - 'expire_time' => time() + 3600, - 'client_id' => 'foo', - 'scopes' => ['foo'], - ], - JSON_THROW_ON_ERROR - ), - ), + 'device_code' => $deviceCode->getIdentifier(), ]); $responseType = new StubResponseType(); diff --git a/tests/ResponseTypes/DeviceCodeResponseTypeTest.php b/tests/ResponseTypes/DeviceCodeResponseTypeTest.php index f8dc265ff..93bd9d6b3 100644 --- a/tests/ResponseTypes/DeviceCodeResponseTypeTest.php +++ b/tests/ResponseTypes/DeviceCodeResponseTypeTest.php @@ -10,28 +10,21 @@ use League\OAuth2\Server\CryptKey; use League\OAuth2\Server\ResponseTypes\DeviceCodeResponse; use LeagueTests\Stubs\ClientEntity; -use LeagueTests\Stubs\CryptTraitStub; use LeagueTests\Stubs\DeviceCodeEntity; use LeagueTests\Stubs\ScopeEntity; use PHPUnit\Framework\TestCase; +use function base64_encode; use function json_decode; -use function json_encode; +use function random_bytes; class DeviceCodeResponseTypeTest extends TestCase { - protected CryptTraitStub $cryptStub; - - public function setUp(): void - { - $this->cryptStub = new CryptTraitStub(); - } - public function testGenerateHttpResponse(): void { $responseType = new DeviceCodeResponse(); $responseType->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); - $responseType->setEncryptionKey($this->cryptStub->getKey()); + $responseType->setEncryptionKey(base64_encode(random_bytes(36))); $client = new ClientEntity(); $client->setIdentifier('clientName'); @@ -60,12 +53,7 @@ public function testGenerateHttpResponse(): void $json = json_decode($response->getBody()->getContents()); $this::assertObjectHasProperty('expires_in', $json); $this::assertObjectHasProperty('device_code', $json); - $this::assertSame(json_encode([ - 'client_id' => 'clientName', - 'device_code_id' => 'abcdef', - 'scopes' => ['basic'], - 'expire_time' => $deviceCode->getExpiryDateTime()->getTimestamp(), - ]), $this->cryptStub->doDecrypt($json->device_code)); + $this::assertEquals('abcdef', $json->device_code); $this::assertObjectHasProperty('verification_uri', $json); $this::assertObjectHasProperty('user_code', $json); } From ee3d0b6589c4578c294ffcff209dbfe461c00cb0 Mon Sep 17 00:00:00 2001 From: Andrew Millington Date: Thu, 14 Nov 2024 22:43:49 +0000 Subject: [PATCH 10/10] Update chagelog message --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83bb155e6..bb6ca90ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Support for PHP 8.4 (PR #1454) ### Fixed -- Fixed bug where scopes were not set on access token when using device authorization grant (PR #1412) +- Fixed spec compliance issue where device access token request was mistakenly expecting to receive scopes in the request (PR #1412) ## [9.0.1] - released 2024-10-14 ### Fixed