diff --git a/CHANGELOG.md b/CHANGELOG.md index 975bbe398..0bc6d69d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Support for PHP 8.4 (PR #1454) ### 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) - 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) diff --git a/src/CryptTrait.php b/src/CryptTrait.php index 8b1946fc4..ee481b55c 100644 --- a/src/CryptTrait.php +++ b/src/CryptTrait.php @@ -15,8 +15,11 @@ namespace League\OAuth2\Server; use Defuse\Crypto\Crypto; +use Defuse\Crypto\Exception\EnvironmentIsBrokenException; +use Defuse\Crypto\Exception\WrongKeyOrModifiedCiphertextException; use Defuse\Crypto\Key; use Exception; +use InvalidArgumentException; use LogicException; use function is_string; @@ -64,6 +67,17 @@ protected function decrypt(string $encryptedData): string } throw new LogicException('Encryption key not set when attempting to decrypt'); + } catch (WrongKeyOrModifiedCiphertextException $e) { + $exceptionMessage = 'The authcode or decryption key/password used ' + . 'is not correct'; + + throw new InvalidArgumentException($exceptionMessage, 0, $e); + } catch (EnvironmentIsBrokenException $e) { + $exceptionMessage = 'Auth code decryption failed. This is likely ' + . 'due to an environment issue or runtime bug in the ' + . 'decryption library'; + + throw new LogicException($exceptionMessage, 0, $e); } catch (Exception $e) { throw new LogicException($e->getMessage(), 0, $e); } diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 22b00d02a..9fb5271c3 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -15,6 +15,7 @@ use DateInterval; use DateTimeImmutable; use Exception; +use InvalidArgumentException; use League\OAuth2\Server\CodeChallengeVerifiers\CodeChallengeVerifierInterface; use League\OAuth2\Server\CodeChallengeVerifiers\PlainVerifier; use League\OAuth2\Server\CodeChallengeVerifiers\S256Verifier; @@ -123,8 +124,10 @@ public function respondToAccessTokenRequest( $authCodePayload->user_id, $authCodePayload->auth_code_id ); + } catch (InvalidArgumentException $e) { + throw OAuthServerException::invalidGrant('Cannot validate the provided authorization code'); } catch (LogicException $e) { - throw OAuthServerException::invalidRequest('code', 'Cannot decrypt the authorization code', $e); + throw OAuthServerException::invalidRequest('code', 'Issue decrypting the authorization code', $e); } $codeVerifier = $this->getRequestParameter('code_verifier', $request); @@ -206,7 +209,7 @@ private function validateAuthorizationCode( } if (time() > $authCodePayload->expire_time) { - throw OAuthServerException::invalidRequest('code', 'Authorization code has expired'); + throw OAuthServerException::invalidGrant('Authorization code has expired'); } if ($this->authCodeRepository->isAuthCodeRevoked($authCodePayload->auth_code_id) === true) { diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index 646056618..390001721 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -1621,7 +1621,7 @@ public function testRespondToAccessTokenRequestClientMismatch(): void } } - public function testRespondToAccessTokenRequestBadCodeEncryption(): void + public function testRespondToAccessTokenRequestBadCode(): void { $client = new ClientEntity(); @@ -1663,7 +1663,7 @@ public function testRespondToAccessTokenRequestBadCodeEncryption(): void 'grant_type' => 'authorization_code', 'client_id' => 'foo', 'redirect_uri' => self::REDIRECT_URI, - 'code' => 'sdfsfsd', + 'code' => 'badCode', ] ); @@ -1671,7 +1671,63 @@ public function testRespondToAccessTokenRequestBadCodeEncryption(): void /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - self::assertEquals($e->getHint(), 'Cannot decrypt the authorization code'); + self::assertEquals($e->getErrorType(), 'invalid_grant'); + self::assertEquals($e->getHint(), 'Cannot validate the provided authorization code'); + } + } + + public function testRespondToAccessTokenRequestNoEncryptionKey(): void + { + $client = new ClientEntity(); + + $client->setIdentifier('foo'); + $client->setRedirectUri(self::REDIRECT_URI); + $client->setConfidential(); + + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + + $clientRepositoryMock->method('getClientEntity')->willReturn($client); + $clientRepositoryMock->method('validateClient')->willReturn(true); + + $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); + $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); + + $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); + + $grant = new AuthCodeGrant( + $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), + $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(), + new DateInterval('PT10M') + ); + $grant->setClientRepository($clientRepositoryMock); + $grant->setAccessTokenRepository($accessTokenRepositoryMock); + $grant->setRefreshTokenRepository($refreshTokenRepositoryMock); + // We deliberately don't set an encryption key here + + $request = new ServerRequest( + [], + [], + null, + 'POST', + 'php://input', + [], + [], + [], + [ + 'grant_type' => 'authorization_code', + 'client_id' => 'foo', + 'redirect_uri' => self::REDIRECT_URI, + 'code' => 'badCode', + ] + ); + + try { + /* @var StubResponseType $response */ + $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); + } catch (OAuthServerException $e) { + self::assertEquals($e->getErrorType(), 'invalid_request'); + self::assertEquals($e->getHint(), 'Issue decrypting the authorization code'); } }