diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e8f4dab9..bb6ca90ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - Support for PHP 8.4 (PR #1454) +### Fixed +- 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 - Auto-generated event emitter is now persisted. Previously, a new emitter was generated every time (PR #1428) 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..9c9133408 100644 --- a/src/Grant/DeviceCodeGrant.php +++ b/src/Grant/DeviceCodeGrant.php @@ -137,7 +137,6 @@ public function respondToAccessTokenRequest( ): ResponseTypeInterface { // Validate request $client = $this->validateClient($request); - $scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope)); $deviceCodeEntity = $this->validateDeviceCode($request, $client); $deviceCodeEntity->setLastPolledAt(new DateTimeImmutable()); @@ -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); diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php index 396ea760f..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([ @@ -261,6 +249,7 @@ public function testValidateDeviceAuthorizationRequestClientMismatch(): void public function testCompleteDeviceAuthorizationRequest(): void { $deviceCode = new DeviceCodeEntity(); + $deviceCode->setIdentifier('deviceCodeEntityIdentifier'); $deviceCode->setUserCode('foo'); $deviceCodeRepository = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock(); @@ -273,9 +262,7 @@ public function testCompleteDeviceAuthorizationRequest(): void 'http://foo/bar', ); - $grant->setEncryptionKey($this->cryptStub->getKey()); - - $grant->completeDeviceAuthorizationRequest($deviceCode->getUserCode(), 'userId', true); + $grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), 'userId', true); $this::assertEquals('userId', $deviceCode->getUserIdentifier()); } @@ -324,8 +311,6 @@ public function testDeviceAuthorizationResponse(): void 'http://foo/bar' ); - $deviceCodeGrant->setEncryptionKey($this->cryptStub->getKey()); - $server->enableGrantType($deviceCodeGrant); $response = $server->respondToDeviceAuthorizationRequest($serverRequest, new Response()); @@ -346,13 +331,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 +342,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( @@ -381,26 +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->getUserCode(), '1', true); + $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' => 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 +390,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 +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', - 'user_code' => '12345678', - 'scopes' => ['foo'], - 'verification_uri' => 'http://foo/bar', - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -469,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( @@ -484,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([ @@ -518,9 +487,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( @@ -533,24 +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', - 'user_code' => '12345678', - 'scopes' => ['foo'], - 'verification_uri' => 'http://foo/bar', - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -579,9 +534,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( @@ -594,24 +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', - 'user_code' => '12345678', - 'scopes' => ['foo'], - 'verification_uri' => 'http://foo/bar', - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -640,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( @@ -655,24 +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', - 'user_code' => '12345678', - 'scopes' => ['foo'], - 'verification_uri' => 'http://foo/bar', - ], - JSON_THROW_ON_ERROR - ) - ), + 'device_code' => uniqid(), ]); $responseType = new StubResponseType(); @@ -711,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); @@ -746,14 +672,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( @@ -766,26 +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->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();