diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index df8b58a3d..c5abd4594 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -346,6 +346,16 @@ public function getRedirectUri(): ?string return $this->redirectUri; } + /** + * Set the Redirect URI used for redirecting. + */ + public function withRedirectUri(string $redirectUri): static + { + $this->redirectUri = $redirectUri; + + return $this; + } + /** * Returns the HTTP status code to send when the exceptions is output. */ diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 9075f1144..d4577cbb9 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -280,7 +280,7 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A $scopes = $this->validateScopes( $this->getQueryStringParameter('scope', $request, $this->defaultScope), - $this->makeRedirectUri( + $finalRedirectUri = $this->makeRedirectUri( $redirectUri ?? $this->getClientRedirectUri($client), $stateParameter !== null ? ['state' => $stateParameter] : [] ) @@ -306,7 +306,7 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A throw OAuthServerException::invalidRequest( 'code_challenge_method', 'Code challenge method must be provided when `code_challenge` is set.' - ); + )->withRedirectUri($finalRedirectUri); } if (array_key_exists($codeChallengeMethod, $this->codeChallengeVerifiers) === false) { @@ -318,7 +318,7 @@ function ($method) { }, array_keys($this->codeChallengeVerifiers) )) - ); + )->withRedirectUri($finalRedirectUri); } // Validate code_challenge according to RFC-7636 @@ -327,13 +327,16 @@ function ($method) { throw OAuthServerException::invalidRequest( 'code_challenge', 'Code challenge must follow the specifications of RFC-7636.' - ); + )->withRedirectUri($finalRedirectUri); } $authorizationRequest->setCodeChallenge($codeChallenge); $authorizationRequest->setCodeChallengeMethod($codeChallengeMethod); } elseif ($this->requireCodeChallengeForPublicClients && !$client->isConfidential()) { - throw OAuthServerException::invalidRequest('code_challenge', 'Code challenge must be provided for public clients'); + throw OAuthServerException::invalidRequest( + 'code_challenge', + 'Code challenge must be provided for public clients' + )->withRedirectUri($finalRedirectUri); } return $authorizationRequest; diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index 1c19b4061..2d6904d75 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -251,13 +251,61 @@ public function testValidateAuthorizationRequestCodeChallenge(): void self::assertInstanceOf(AuthorizationRequest::class, $grant->validateAuthorizationRequest($request)); } + public function testValidateAuthorizationRequestCodeChallengeRequired(): void + { + $client = new ClientEntity(); + $client->setRedirectUri(self::REDIRECT_URI); + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $scope = new ScopeEntity(); + $scope->setIdentifier(self::DEFAULT_SCOPE); + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); + + $grant = new AuthCodeGrant( + $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), + $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(), + new DateInterval('PT10M') + ); + + $grant->setDefaultScope(self::DEFAULT_SCOPE); + $grant->setClientRepository($clientRepositoryMock); + $grant->setScopeRepository($scopeRepositoryMock); + + $request = (new ServerRequest())->withQueryParams([ + 'response_type' => 'code', + 'client_id' => 'foo', + 'redirect_uri' => self::REDIRECT_URI, + 'code_challenge' => null, + 'state' => 'foo', + ]); + + try { + $grant->validateAuthorizationRequest($request); + } catch (OAuthServerException $e) { + self::assertSame(3, $e->getCode()); + self::assertSame('Code challenge must be provided for public clients', $e->getHint()); + self::assertSame('invalid_request', $e->getErrorType()); + self::assertSame('https://foo/bar?state=foo', $e->getRedirectUri()); + + return; + } + + self::fail('The expected exception was not thrown'); + } + public function testValidateAuthorizationRequestCodeChallengeInvalidLengthTooShort(): void { $client = new ClientEntity(); $client->setRedirectUri(self::REDIRECT_URI); $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $scope = new ScopeEntity(); + $scope->setIdentifier(self::DEFAULT_SCOPE); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); $grant = new AuthCodeGrant( $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), @@ -274,11 +322,21 @@ public function testValidateAuthorizationRequestCodeChallengeInvalidLengthTooSho 'client_id' => 'foo', 'redirect_uri' => self::REDIRECT_URI, 'code_challenge' => str_repeat('A', 42), + 'state' => 'foo', ]); - $this->expectException(OAuthServerException::class); + try { + $grant->validateAuthorizationRequest($request); + } catch (OAuthServerException $e) { + self::assertSame(3, $e->getCode()); + self::assertSame('Code challenge must follow the specifications of RFC-7636.', $e->getHint()); + self::assertSame('invalid_request', $e->getErrorType()); + self::assertSame('https://foo/bar?state=foo', $e->getRedirectUri()); - $grant->validateAuthorizationRequest($request); + return; + } + + self::fail('The expected exception was not thrown'); } public function testValidateAuthorizationRequestCodeChallengeInvalidLengthTooLong(): void @@ -287,7 +345,11 @@ public function testValidateAuthorizationRequestCodeChallengeInvalidLengthTooLon $client->setRedirectUri(self::REDIRECT_URI); $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $scope = new ScopeEntity(); + $scope->setIdentifier(self::DEFAULT_SCOPE); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); $grant = new AuthCodeGrant( $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), @@ -304,11 +366,21 @@ public function testValidateAuthorizationRequestCodeChallengeInvalidLengthTooLon 'client_id' => 'foo', 'redirect_uri' => self::REDIRECT_URI, 'code_challenge' => str_repeat('A', 129), + 'state' => 'foo', ]); - $this->expectException(OAuthServerException::class); + try { + $grant->validateAuthorizationRequest($request); + } catch (OAuthServerException $e) { + self::assertSame(3, $e->getCode()); + self::assertSame('Code challenge must follow the specifications of RFC-7636.', $e->getHint()); + self::assertSame('invalid_request', $e->getErrorType()); + self::assertSame('https://foo/bar?state=foo', $e->getRedirectUri()); - $grant->validateAuthorizationRequest($request); + return; + } + + self::fail('The expected exception was not thrown'); } public function testValidateAuthorizationRequestCodeChallengeInvalidCharacters(): void @@ -317,7 +389,11 @@ public function testValidateAuthorizationRequestCodeChallengeInvalidCharacters() $client->setRedirectUri(self::REDIRECT_URI); $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $scope = new ScopeEntity(); + $scope->setIdentifier(self::DEFAULT_SCOPE); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope); $grant = new AuthCodeGrant( $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), @@ -334,11 +410,21 @@ public function testValidateAuthorizationRequestCodeChallengeInvalidCharacters() 'client_id' => 'foo', 'redirect_uri' => self::REDIRECT_URI, 'code_challenge' => str_repeat('A', 42) . '!', + 'state' => 'foo', ]); - $this->expectException(OAuthServerException::class); + try { + $grant->validateAuthorizationRequest($request); + } catch (OAuthServerException $e) { + self::assertSame(3, $e->getCode()); + self::assertSame('Code challenge must follow the specifications of RFC-7636.', $e->getHint()); + self::assertSame('invalid_request', $e->getErrorType()); + self::assertSame('https://foo/bar?state=foo', $e->getRedirectUri()); - $grant->validateAuthorizationRequest($request); + return; + } + + self::fail('The expected exception was not thrown'); } public function testValidateAuthorizationRequestMissingClientId(): void @@ -464,12 +550,21 @@ public function testValidateAuthorizationRequestInvalidCodeChallengeMethod(): vo 'redirect_uri' => self::REDIRECT_URI, 'code_challenge' => 'foobar', 'code_challenge_method' => 'foo', + 'state' => 'bar', ]); - $this->expectException(OAuthServerException::class); - $this->expectExceptionCode(3); + try { + $grant->validateAuthorizationRequest($request); + } catch (OAuthServerException $e) { + self::assertSame(3, $e->getCode()); + self::assertSame('Code challenge method must be one of `S256`, `plain`', $e->getHint()); + self::assertSame('invalid_request', $e->getErrorType()); + self::assertSame('https://foo/bar?state=bar', $e->getRedirectUri()); - $grant->validateAuthorizationRequest($request); + return; + } + + self::fail('The expected exception was not thrown'); } public function testValidateAuthorizationRequestInvalidScopes(): void