Repository: thephpleague/oauth2-server (php)

Issue:
Add an option to always expose the `interval` param for the device code grant
The title. I want to always expose the `interval` param. Clients aren't always responsible for reading the standard to know that the poll interval defaults to 5 seconds. Microsoft and Google always return this parameter.

My suggestion is simply to remove the next condition:
https://github.com/thephpleague/oauth2-server/blob/7b9ec37d5077c0382aa3748c6e125c5408d5adb9/src/ResponseTypes/DeviceCodeResponse.php#L48-L50

Pull request:
Fix including interval in response of device authorization request
This PR does:

* ~Fix a typo on `DeviceCodeRepositoryInterface::getDeviceCodeEntityByDeviceCode()` argument, `string $deviceCodeEntity` -> `string $deviceCode`.~ TBD later.
* Fix a bug where enabling the interval visibility is ignored:
   *  [RFC8628 Section 3.2](https://datatracker.ietf.org/doc/html/rfc8628#section-3.2:~:text=interval%0A%20%20%20%20%20%20OPTIONAL.%20%20The%20minimum%20amount%20of%20time%20in%20seconds%20that%20the%20client%0A%20%20%20%20%20%20SHOULD%20wait%20between%20polling%20requests%20to%20the%20token%20endpoint.%20%20If%20no%0A%20%20%20%20%20%20value%20is%20provided%2C%20clients%20MUST%20use%205%20as%20the%20default.)
   * Fixes #1464 
   * Even by explicitly calling `$grant->setIntervalVisibility(true)` the response still doesn't include `interval`. On the response you are excluding `interval` when it's set to default 5. Although the server may use the default interval value, we still want to include the `internal` on the response.
* Fix a bug where the new poll date is not persisted when `slow_down` error happens, because the exception is thrown before calling `persistDeviceCode`.
* ~Increase interval by 5 when `slow_down` error happens according to [section 3.5 of RFC8628](https://datatracker.ietf.org/doc/html/rfc8628#section-3.5:~:text=but%20the%20interval%20MUST%0A%20%20%20%20%20%20be%20increased%20by%205%20seconds%20for%20this%20and%20all%20subsequent%20requests.).~
* Fix a bug where `slow_down` error response may have been returned even after the user has completed the auth flow (already approved / denied the request).
* ~Include the `interval` value on `authorization_pending` and `slow_down` error responses.~

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0bc6d69d..6101234b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,8 +10,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Fixed
 - In the Auth Code grant, when requesting an access token with an invalid auth code, we now respond with an invalid_grant error instead of invalid_request (PR #1433)
-- Fixed spec compliance issue  where device access token request was mistakenly expecting to receive scopes in the request (PR #1412)
+- Fixed spec compliance issue where device access token request was mistakenly expecting to receive scopes in the request (PR #1412)
 - Refresh tokens pre version 9 might have had user IDs set as ints which meant they were incorrectly rejected. We now cast these values to strings to allow old refresh tokens (PR #1436)
+- Fixed bug on setting interval visibility of device authorization grant (PR #1410)
+- Fix a bug where the new poll date were not persisted when `slow_down` error happens, because the exception is thrown before calling `persistDeviceCode`. (PR #1410)
+- Fix a bug where `slow_down` error response may have been returned even after the user has completed the auth flow (already approved / denied the request). (PR #1410)
 
 ## [9.0.1] - released 2024-10-14
 ### Fixed
diff --git a/src/Grant/DeviceCodeGrant.php b/src/Grant/DeviceCodeGrant.php
index 9c913340..aa8cba0f 100644
--- a/src/Grant/DeviceCodeGrant.php
+++ b/src/Grant/DeviceCodeGrant.php
@@ -51,7 +51,7 @@ class DeviceCodeGrant extends AbstractGrant
         RefreshTokenRepositoryInterface $refreshTokenRepository,
         private DateInterval $deviceCodeTTL,
         string $verificationUri,
-        private int $retryInterval = 5
+        private readonly int $retryInterval = 5
     ) {
         $this->setDeviceCodeRepository($deviceCodeRepository);
         $this->setRefreshTokenRepository($refreshTokenRepository);
@@ -101,6 +101,10 @@ class DeviceCodeGrant extends AbstractGrant
             $response->includeVerificationUriComplete();
         }
 
+        if ($this->intervalVisibility === true) {
+            $response->includeInterval();
+        }
+
         $response->setDeviceCodeEntity($deviceCodeEntity);
 
         return $response;
@@ -139,11 +143,17 @@ class DeviceCodeGrant extends AbstractGrant
         $client = $this->validateClient($request);
         $deviceCodeEntity = $this->validateDeviceCode($request, $client);
 
-        $deviceCodeEntity->setLastPolledAt(new DateTimeImmutable());
-        $this->deviceCodeRepository->persistDeviceCode($deviceCodeEntity);
-
-        // If device code has no user associated, respond with pending
+        // If device code has no user associated, respond with pending or slow down
         if (is_null($deviceCodeEntity->getUserIdentifier())) {
+            $shouldSlowDown = $this->deviceCodePolledTooSoon($deviceCodeEntity->getLastPolledAt());
+
+            $deviceCodeEntity->setLastPolledAt(new DateTimeImmutable());
+            $this->deviceCodeRepository->persistDeviceCode($deviceCodeEntity);
+
+            if ($shouldSlowDown) {
+                throw OAuthServerException::slowDown();
+            }
+
             throw OAuthServerException::authorizationPending();
         }
 
@@ -205,10 +215,6 @@ class DeviceCodeGrant extends AbstractGrant
             throw OAuthServerException::invalidRequest('device_code', 'Device code was not issued to this client');
         }
 
-        if ($this->deviceCodePolledTooSoon($deviceCodeEntity->getLastPolledAt()) === true) {
-            throw OAuthServerException::slowDown();
-        }
-
         return $deviceCodeEntity;
     }
 
@@ -258,10 +264,7 @@ class DeviceCodeGrant extends AbstractGrant
         $deviceCode->setExpiryDateTime((new DateTimeImmutable())->add($deviceCodeTTL));
         $deviceCode->setClient($client);
         $deviceCode->setVerificationUri($verificationUri);
-
-        if ($this->getIntervalVisibility() === true) {
-            $deviceCode->setInterval($this->retryInterval);
-        }
+        $deviceCode->setInterval($this->retryInterval);
 
         foreach ($scopes as $scope) {
             $deviceCode->addScope($scope);
diff --git a/src/ResponseTypes/DeviceCodeResponse.php b/src/ResponseTypes/DeviceCodeResponse.php
index 91a8df69..0a988db1 100644
--- a/src/ResponseTypes/DeviceCodeResponse.php
+++ b/src/ResponseTypes/DeviceCodeResponse.php
@@ -25,7 +25,7 @@ class DeviceCodeResponse extends AbstractResponseType
 {
     protected DeviceCodeEntityInterface $deviceCodeEntity;
     private bool $includeVerificationUriComplete = false;
-    private const DEFAULT_INTERVAL = 5;
+    private bool $includeInterval = false;
 
     /**
      * {@inheritdoc}
@@ -45,7 +45,7 @@ class DeviceCodeResponse extends AbstractResponseType
             $responseParams['verification_uri_complete'] = $this->deviceCodeEntity->getVerificationUriComplete();
         }
 
-        if ($this->deviceCodeEntity->getInterval() !== self::DEFAULT_INTERVAL) {
+        if ($this->includeInterval === true) {
             $responseParams['interval'] = $this->deviceCodeEntity->getInterval();
         }
 
@@ -79,6 +79,11 @@ class DeviceCodeResponse extends AbstractResponseType
         $this->includeVerificationUriComplete = true;
     }
 
+    public function includeInterval(): void
+    {
+        $this->includeInterval = true;
+    }
+
     /**
      * Add custom fields to your Bearer Token response here, then override
      * AuthorizationServer::getResponseType() to pull in your version of


Test patch:
diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php
index 4a9cebbd..42157a49 100644
--- a/tests/Grant/DeviceCodeGrantTest.php
+++ b/tests/Grant/DeviceCodeGrantTest.php
@@ -656,6 +656,51 @@ class DeviceCodeGrantTest extends TestCase
         $this::assertEquals(self::INTERVAL_RATE, $deviceCode->interval);
     }
 
+    public function testSettingInternalVisibility(): void
+    {
+        $client = new ClientEntity();
+        $client->setIdentifier('foo');
+
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+
+        $deviceCode = new DeviceCodeEntity();
+
+        $deviceCodeRepository = $this->getMockBuilder(DeviceCodeRepositoryInterface::class)->getMock();
+        $deviceCodeRepository->method('getNewDeviceCode')->willReturn($deviceCode);
+
+        $scope = new ScopeEntity();
+        $scope->setIdentifier('basic');
+        $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
+        $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope);
+
+        $grant = new DeviceCodeGrant(
+            $deviceCodeRepository,
+            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
+            new DateInterval('PT10M'),
+            'http://foo/bar',
+        );
+
+        $grant->setClientRepository($clientRepositoryMock);
+        $grant->setDefaultScope(self::DEFAULT_SCOPE);
+        $grant->setScopeRepository($scopeRepositoryMock);
+        $grant->setIntervalVisibility(true);
+
+        $request = (new ServerRequest())->withParsedBody([
+            'client_id' => 'foo',
+            'scope' => 'basic',
+        ]);
+
+        $deviceCodeResponse = $grant
+            ->respondToDeviceAuthorizationRequest($request)
+            ->generateHttpResponse(new Response());
+
+        $deviceCode = json_decode((string) $deviceCodeResponse->getBody());
+
+        $this::assertObjectHasProperty('interval', $deviceCode);
+        $this::assertEquals(5, $deviceCode->interval);
+    }
+
     public function testIssueAccessDeniedError(): void
     {
         $client = new ClientEntity();