diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bc6d69d1..6101234b4 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 9c9133408..12f56b161 100644 --- a/src/Grant/DeviceCodeGrant.php +++ b/src/Grant/DeviceCodeGrant.php @@ -51,7 +51,7 @@ public function __construct( RefreshTokenRepositoryInterface $refreshTokenRepository, private DateInterval $deviceCodeTTL, string $verificationUri, - private int $retryInterval = 5 + private readonly int $defaultInterval = 5 ) { $this->setDeviceCodeRepository($deviceCodeRepository); $this->setRefreshTokenRepository($refreshTokenRepository); @@ -101,6 +101,10 @@ public function respondToDeviceAuthorizationRequest(ServerRequestInterface $requ $response->includeVerificationUriComplete(); } + if ($this->intervalVisibility === true) { + $response->includeInterval(); + } + $response->setDeviceCodeEntity($deviceCodeEntity); return $response; @@ -139,11 +143,17 @@ public function respondToAccessTokenRequest( $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,16 +215,12 @@ protected function validateDeviceCode(ServerRequestInterface $request, ClientEnt throw OAuthServerException::invalidRequest('device_code', 'Device code was not issued to this client'); } - if ($this->deviceCodePolledTooSoon($deviceCodeEntity->getLastPolledAt()) === true) { - throw OAuthServerException::slowDown(); - } - return $deviceCodeEntity; } private function deviceCodePolledTooSoon(?DateTimeImmutable $lastPoll): bool { - return $lastPoll !== null && $lastPoll->getTimestamp() + $this->retryInterval > time(); + return $lastPoll !== null && $lastPoll->getTimestamp() + $this->defaultInterval > time(); } /** @@ -258,10 +264,7 @@ protected function issueDeviceCode( $deviceCode->setExpiryDateTime((new DateTimeImmutable())->add($deviceCodeTTL)); $deviceCode->setClient($client); $deviceCode->setVerificationUri($verificationUri); - - if ($this->getIntervalVisibility() === true) { - $deviceCode->setInterval($this->retryInterval); - } + $deviceCode->setInterval($this->defaultInterval); foreach ($scopes as $scope) { $deviceCode->addScope($scope); diff --git a/src/Repositories/DeviceCodeRepositoryInterface.php b/src/Repositories/DeviceCodeRepositoryInterface.php index 09575ab18..453ba3bb7 100644 --- a/src/Repositories/DeviceCodeRepositoryInterface.php +++ b/src/Repositories/DeviceCodeRepositoryInterface.php @@ -33,7 +33,7 @@ public function persistDeviceCode(DeviceCodeEntityInterface $deviceCodeEntity): * Get a device code entity. */ public function getDeviceCodeEntityByDeviceCode( - string $deviceCodeEntity + string $deviceCode ): ?DeviceCodeEntityInterface; /** diff --git a/src/ResponseTypes/DeviceCodeResponse.php b/src/ResponseTypes/DeviceCodeResponse.php index 91a8df69a..0a988db10 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 @@ public function generateHttpResponse(ResponseInterface $response): ResponseInter $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 @@ public function includeVerificationUriComplete(): void $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 diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php index 4a9cebbd1..42157a494 100644 --- a/tests/Grant/DeviceCodeGrantTest.php +++ b/tests/Grant/DeviceCodeGrantTest.php @@ -656,6 +656,51 @@ public function testSettingDeviceCodeIntervalRate(): void $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();