Repository: thephpleague/oauth2-server (php)

Issue:
The `scope` parameter has been mistakenly required on device access token request
On device authorization flow, according to [RFC8628](https://datatracker.ietf.org/doc/html/rfc8628#section-3.4), the "Device Access Token Request" should send these parameters:

* `grant_type`
* `device_code`
* `client_id`

As you can see there is no `scope` parameter, because the scope parameter had been sent on the first step "Device Authorization Request":

* `client_id`
* `scope`

The scopes are requested by the client on the first request and **should be persisted on the DB**. When user enters the `user code` we display the client info and list of scopes to be approved by user. So the client shouldn't specify scopes on the last request, but the current implementation requires `scopes` on `DeviceCodeGrant::respondToAccessTokenRequest()` mistakenly:

https://github.com/thephpleague/oauth2-server/blob/2ed9e5f65045bebf9e99c33ef1558dcd6d0206b7/src/Grant/DeviceCodeGrant.php#L140-L141

I think you should get the scopes from `$deviceCodeEntity` instead, which was persisted on the DB.

```diff
-$scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope));
$deviceCodeEntity = $this->validateDeviceCode($request, $client);
+$scopes = $deviceCodeEntity->getScopes(); // no need to call `validateScope` because already done on `respondToDeviceAuthorizationRequest` before persisting to DB
```

Am I missing something?


Pull request:
Fix scope on device code grant
* Fixes #1411
* [RFC8628 section 3.1](https://datatracker.ietf.org/doc/html/rfc8628#section-3.1:~:text=scope%0A%20%20%20%20%20%20OPTIONAL.%20%20The%20scope%20of%20the%20access%20request%20as%20defined%20by%0A%20%20%20%20%20%20Section%C2%A03.3%20of%20%5BRFC6749%5D.) accepts `scope` parameter and [section 3.4](https://datatracker.ietf.org/doc/html/rfc8628#section-3.4) doesn't expect `scope` parameter.
* ~Encrypt / decrypt the device code.~
* Improves tests.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8e8f4dab..bb6ca90e 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 9495c9a1..ccfb8435 100644
--- a/examples/src/Repositories/DeviceCodeRepository.php
+++ b/examples/src/Repositories/DeviceCodeRepository.php
@@ -17,6 +17,7 @@ use League\OAuth2\Server\Entities\DeviceCodeEntityInterface;
 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 @@ class DeviceCodeRepository implements DeviceCodeRepositoryInterface
         $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 3804e202..9c913340 100644
--- a/src/Grant/DeviceCodeGrant.php
+++ b/src/Grant/DeviceCodeGrant.php
@@ -137,7 +137,6 @@ class DeviceCodeGrant extends AbstractGrant
     ): 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 @@ class DeviceCodeGrant extends AbstractGrant
         }
 
         // 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);


Test patch:
diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php
index 396ea760..4a9cebbd 100644
--- a/tests/Grant/DeviceCodeGrantTest.php
+++ b/tests/Grant/DeviceCodeGrantTest.php
@@ -20,7 +20,6 @@ use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface;
 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 LeagueTests\Stubs\StubResponseType;
 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 @@ class DeviceCodeGrantTest extends TestCase
 
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setDefaultScope(self::DEFAULT_SCOPE);
-        $grant->setEncryptionKey($this->cryptStub->getKey());
         $grant->setScopeRepository($scopeRepositoryMock);
 
         $request = (new ServerRequest())->withParsedBody([
@@ -147,7 +136,6 @@ class DeviceCodeGrantTest extends TestCase
 
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setDefaultScope(self::DEFAULT_SCOPE);
-        $grant->setEncryptionKey($this->cryptStub->getKey());
         $grant->setScopeRepository($scopeRepositoryMock);
 
         $request = (new ServerRequest())->withParsedBody([
@@ -261,6 +249,7 @@ class DeviceCodeGrantTest extends TestCase
     public function testCompleteDeviceAuthorizationRequest(): void
     {
         $deviceCode = new DeviceCodeEntity();
+        $deviceCode->setIdentifier('deviceCodeEntityIdentifier');
         $deviceCode->setUserCode('foo');
 
         $deviceCodeRepository = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock();
@@ -273,9 +262,7 @@ class DeviceCodeGrantTest extends TestCase
             '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 @@ class DeviceCodeGrantTest extends TestCase
             'http://foo/bar'
         );
 
-        $deviceCodeGrant->setEncryptionKey($this->cryptStub->getKey());
-
         $server->enableGrantType($deviceCodeGrant);
 
         $response = $server->respondToDeviceAuthorizationRequest($serverRequest, new Response());
@@ -346,13 +331,8 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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 @@ class DeviceCodeGrantTest extends TestCase
 
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setDefaultScope(self::DEFAULT_SCOPE);
-        $grant->setEncryptionKey($this->cryptStub->getKey());
         $grant->setScopeRepository($scopeRepositoryMock);
         $grant->setIntervalVisibility(true);
 
@@ -746,14 +672,14 @@ class DeviceCodeGrantTest extends TestCase
 
         $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 @@ class DeviceCodeGrantTest extends TestCase
         $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();