Tidy up tests and add error handling

examples/public/device_code.php

@@ -51,7 +51,7 @@
                 $deviceCodeRepository,
                 $refreshTokenRepository,
                 new \DateInterval('PT10M'),
-                5
+                'http://foo/bar'
             ),
             new \DateInterval('PT1H')
         );
@@ -65,17 +65,17 @@
     $server = $app->getContainer()->get(AuthorizationServer::class);
 
     try {
-        $deviceAuthRequest = $server->validateDeviceAuthorizationRequest($request);
+        $deviceCodeResponse = $server->respondToDeviceAuthorizationRequest($request, $response);
 
-        // TODO: I don't think this is right as the user can't approve the request via the same client...
-        // Once the user has logged in, set the user on the authorization request
-        //$deviceAuthRequest->setUserIdentifier();
+        return $deviceCodeResponse;
 
-        // Once the user has approved or denied the client, update the status
-        //$deviceAuthRequest->setAuthorizationApproved(true);
+        // Extract the device code. Usually we would then assign the user ID to
+        // the device code but for the purposes of this example, we've hard 
+        // coded it in the response above.
+        // $deviceCode = json_decode((string) $deviceCodeResponse->getBody());
 
-        // Return the HTTP redirect response
-        return $server->completeDeviceAuthorizationRequest($deviceAuthRequest, $response);
+        // Once the user has logged in and approved the request, set the user on the device code
+        // $server->completeDeviceAuthorizationRequest($deviceCode->user_code, 1);
     } catch (OAuthServerException $exception) {
         return $exception->generateHttpResponse($response);
     } catch (\Exception $exception) {

examples/src/Repositories/DeviceCodeRepository.php

@@ -35,10 +35,12 @@ public function persistDeviceCode(DeviceCodeEntityInterface $deviceCodeEntity)
     /**
      * {@inheritdoc}
      */
-    public function getDeviceCodeEntityByDeviceCode($deviceCode, $grantType, ClientEntityInterface $clientEntity)
+    public function getDeviceCodeEntityByDeviceCode($deviceCode)
     {
         $deviceCode = new DeviceCodeEntity();
 
+        $deviceCode->setIdentifier('device_code_1234');
+
         // The user identifier should be set when the user authenticates on the OAuth server
         $deviceCode->setUserIdentifier(1);
 

src/AuthorizationServer.php

@@ -207,17 +207,11 @@ public function respondToDeviceAuthorizationRequest(ServerRequestInterface $requ
 
     /**
      * Complete a device authorization request
-     *
-     * @param DeviceAuthorizationRequest $deviceRequest
-     * @param ResponseInterface          $response
-     *
-     * @return ResponseInterface
      */
-    public function completeDeviceAuthorizationRequest(DeviceAuthorizationRequest $deviceRequest, ResponseInterface $response)
+    public function completeDeviceAuthorizationRequest(string $deviceCode, string|int $userId): ResponseInterface
     {
-        // TODO: Check why we aren't just using completeAuthorizationRequest
-        return $this->enabledGrantTypes[$deviceRequest->getGrantTypeId()]
-            ->completeDeviceAuthorizationRequest($deviceRequest)
+        return $this->enabledGrantTypes['urn:ietf:params:oauth:grant-type:device_code']
+            ->completeDeviceAuthorizationRequest($deviceCode, $userId)
             ->generateHttpResponse($response);
     }
 

src/Entities/DeviceCodeEntityInterface.php

@@ -24,4 +24,16 @@ public function setVerificationUri(string $verificationUri);
     public function getLastPolledAt(): ?DateTimeImmutable;
 
     public function setLastPolledAt(DateTimeImmutable $lastPolledAt): void;
+
+    public function getInterval(): int;
+
+    public function setInterval(int $interval): void;
+
+    public function getIntervalInAuthResponse(): bool;
+
+    public function setIntervalInAuthResponse(bool $intervalInAuthResponse): bool;
+
+    public function getUserApproved(): bool;
+
+    public function setUserApproved(bool $userApproved): void;
 }

src/Entities/Traits/DeviceCodeTrait.php

@@ -15,6 +15,9 @@
 
 trait DeviceCodeTrait
 {
+    private bool $userApproved = false;
+    private bool $intervalInAuthResponse = false;
+    private int $interval = 5;
     private string $userCode;
     private string $verificationUri;
     private ?DateTimeImmutable $lastPolledAt = null;
@@ -59,4 +62,34 @@ public function setLastPolledAt(DateTimeImmutable $lastPolledAt): void
     {
         $this->lastPolledAt = $lastPolledAt;
     }
+
+    public function getInterval(): int
+    {
+        return $this->interval;
+    }
+
+    public function setInterval(int $interval): void
+    {
+        $this->interval = $interval;
+    }
+
+    public function getIntervalInAuthResponse(): bool
+    {
+        return $this->intervalInAuthResponse;
+    }
+
+    public function setIntervalInAuthResponse(bool $intervalInAuthResponse): bool
+    {
+        return $this->intervalInAuthResponse = $intervalInAuthResponse;
+    }
+
+    public function getUserApproved(): bool
+    {
+        return $this->userApproved;
+    }
+
+    public function setUserApproved(bool $userApproved): void
+    {
+        $this->userApproved = $userApproved;
+    }
 }

src/Grant/AbstractGrant.php

@@ -641,7 +641,7 @@ public function respondToDeviceAuthorizationRequest(ServerRequestInterface $requ
     /**
      * {@inheritdoc}
      */
-    public function completeDeviceAuthorizationRequest(string $deviceCode, string|int $userId)
+    public function completeDeviceAuthorizationRequest(string $deviceCode, string|int $userId, bool $userApproved)
     {
         throw new LogicException('This grant cannot complete a device authorization request');
     }

src/Grant/DeviceCodeGrant.php

@@ -41,44 +41,26 @@
  */
 class DeviceCodeGrant extends AbstractGrant
 {
-    /**
-     * @var DeviceCodeRepositoryInterface
-     */
-    protected $deviceCodeRepository;
-
-    /**
-     * @var DateInterval
-     */
-    private $deviceCodeTTL;
+    protected DeviceCodeRepositoryInterface $deviceCodeRepository;
+    private DateInterval $deviceCodeTTL;
+    private bool $intervalVisibility = false;
+    private int $retryInterval;
+    private string $verificationUri;
 
-    /**
-     * @var int
-     */
-    private $retryInterval;
-
-    /**
-     * @var string
-     */
-    private $verificationUri;
-
-    /**
-     * @param DeviceCodeRepositoryInterface        $deviceCodeRepository
-     * @param RefreshTokenRepositoryInterface      $refreshTokenRepository
-     * @param DateInterval                         $deviceCodeTTL
-     * @param int                                  $retryInterval
-     */
     public function __construct(
         DeviceCodeRepositoryInterface $deviceCodeRepository,
         RefreshTokenRepositoryInterface $refreshTokenRepository,
         DateInterval $deviceCodeTTL,
-        $retryInterval = 5
+        string $verificationUri,
+        int $retryInterval = 5
     ) {
         $this->setDeviceCodeRepository($deviceCodeRepository);
         $this->setRefreshTokenRepository($refreshTokenRepository);
 
         $this->refreshTokenTTL = new DateInterval('P1M');
 
         $this->deviceCodeTTL = $deviceCodeTTL;
+        $this->setVerificationUri($verificationUri);
         $this->retryInterval = $retryInterval;
     }
 
@@ -107,21 +89,17 @@ public function respondToDeviceAuthorizationRequest(ServerRequestInterface $requ
 
         $client = $this->getClientEntityOrFail($clientId, $request);
 
-        // TODO: Make sure the grant type is set...
-
         $scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope));
 
-        // TODO: Don't think I need the deviceauthorizationrequest any more. Might repurpose it...
-        // $deviceAuthorizationRequest = new DeviceAuthorizationRequest();
-        // $deviceAuthorizationRequest->setGrantTypeId($this->getIdentifier());
-
         $deviceCode = $this->issueDeviceCode(
             $this->deviceCodeTTL,
             $client,
             $this->verificationUri,
             $scopes
         );
 
+        // TODO: Why do we need this? Why not just generate a random number? Is it a security concern?
+        // TODO: Do I need to set the interval in this?
         $payload = [
             'device_code_id' => $deviceCode->getIdentifier(),
             'user_code' => $deviceCode->getUserCode(),
@@ -143,11 +121,12 @@ public function respondToDeviceAuthorizationRequest(ServerRequestInterface $requ
     /**
      * {@inheritdoc}
      */
-    public function completeDeviceAuthorizationRequest(string $deviceCode, string|int $userId)
+    public function completeDeviceAuthorizationRequest(string $deviceCode, string|int $userId, bool $approved)
     {
         $deviceCode = $this->deviceCodeRepository->getDeviceCodeEntityByDeviceCode($deviceCode);
 
         $deviceCode->setUserIdentifier($userId);
+        $deviceCode->setUserApproved($approved);
 
         $this->deviceCodeRepository->persistDeviceCode($deviceCode);
     }
@@ -161,27 +140,22 @@ public function respondToAccessTokenRequest(
         DateInterval $accessTokenTTL
     ) {
         // Validate request
-        // TODO: Check that the correct grant type has been sent
         $client = $this->validateClient($request);
         $scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope));
         $deviceCode = $this->validateDeviceCode($request, $client);
 
-        // TODO: Should the last poll and user check be done in the validateDeviceCode method?
-        $lastPoll = $deviceCode->getLastPolledAt();
-
-        if ($lastPoll !== null && $lastPoll->getTimestamp() + $this->retryInterval > time()) {
-            throw OAuthServerException::slowDown();
-        }
-
         $deviceCode->setLastPolledAt(new DateTimeImmutable());
-
         $this->deviceCodeRepository->persistDeviceCode($deviceCode);
 
-        // if device code has no user associated, respond with pending
+        // If device code has no user associated, respond with pending
         if (is_null($deviceCode->getUserIdentifier())) {
             throw OAuthServerException::authorizationPending();
         }
 
+        if ($deviceCode->getUserApproved() === false) {
+            throw OAuthServerException::accessDenied();
+        }
+
         // Finalize the requested scopes
         $finalizedScopes = $this->scopeRepository->finalizeScopes($scopes, $this->getIdentifier(), $client, (string) $deviceCode->getUserIdentifier());
 
@@ -238,10 +212,14 @@ protected function validateDeviceCode(ServerRequestInterface $request, ClientEnt
         }
 
         $deviceCode = $this->deviceCodeRepository->getDeviceCodeEntityByDeviceCode(
-            $deviceCodePayload->device_code_id,
-            $this->getIdentifier(),
-            $client
-        );
+                $deviceCodePayload->device_code_id,
+                $this->getIdentifier(),
+                $client
+                );
+
+        if ($this->deviceCodePolledTooSoon($deviceCode->getLastPolledAt()) === true) {
+            throw OAuthServerException::slowDown();
+        }
 
         if ($deviceCode instanceof DeviceCodeEntityInterface === false) {
             $this->getEmitter()->emit(new RequestEvent(RequestEvent::USER_AUTHENTICATION_FAILED, $request));
@@ -252,6 +230,11 @@ protected function validateDeviceCode(ServerRequestInterface $request, ClientEnt
         return $deviceCode;
     }
 
+    private function deviceCodePolledTooSoon(?DateTimeImmutable $lastPoll): bool
+    {
+        return $lastPoll !== null && $lastPoll->getTimestamp() + $this->retryInterval > time();
+    }
+
     /**
      * @param string $encryptedDeviceCode
      *
@@ -320,6 +303,10 @@ protected function issueDeviceCode(
         $deviceCode->setClient($client);
         $deviceCode->setVerificationUri($verificationUri);
 
+        if ($this->getIntervalVisibility() === true) {
+            $deviceCode->setInterval($this->retryInterval);
+        }
+
         foreach ($scopes as $scope) {
             $deviceCode->addScope($scope);
         }
@@ -370,4 +357,14 @@ protected function generateUniqueUserCode($length = 8)
         }
         // @codeCoverageIgnoreEnd
     }
+
+    public function setIntervalVisibility(bool $intervalVisibility): void
+    {
+        $this->intervalVisibility = $intervalVisibility;
+    }
+
+    public function getIntervalVisibility(): bool
+    {
+        return $this->intervalVisibility;
+    }
 }

src/Grant/GrantTypeInterface.php

@@ -133,7 +133,7 @@ public function respondToDeviceAuthorizationRequest(ServerRequestInterface $requ
      *
      * @return ResponseTypeInterface
      */
-    public function completeDeviceAuthorizationRequest(string $deviceCode, string|int $userId);
+    public function completeDeviceAuthorizationRequest(string $deviceCode, string|int $userId, bool $userApproved);
 
     /**
      * Set the client repository.

src/ResponseTypes/DeviceCodeResponse.php

@@ -15,17 +15,12 @@
 use LogicException;
 use Psr\Http\Message\ResponseInterface;
 
+use function time;
+
 class DeviceCodeResponse extends AbstractResponseType
 {
-    /**
-     * @var DeviceCodeEntityInterface
-     */
-    protected $deviceCode;
-
-    /**
-     * @var string
-     */
-    protected $payload;
+    protected DeviceCodeEntityInterface $deviceCode;
+    protected string $payload;
 
     /**
      * {@inheritdoc}
@@ -38,11 +33,14 @@ public function generateHttpResponse(ResponseInterface $response)
             'device_code' => $this->payload,
             'user_code' => $this->deviceCode->getUserCode(),
             'verification_uri' => $this->deviceCode->getVerificationUri(),
-            'expires_in'   => $expireDateTime - \time(),
-            // TODO: Add interval in here
+            'expires_in'   => $expireDateTime - time(),
             // TODO: Potentially add in verification_uri_complete - it is optional
         ];
 
+        if ($this->deviceCode->getIntervalInAuthResponse() === true) {
+            $responseParams['interval'] = $this->deviceCode->getInterval();
+        }
+
         $responseParams = \json_encode($responseParams);
 
         if ($responseParams === false) {