diff --git a/src/Controller/AuthorizationController.php b/src/Controller/AuthorizationController.php index 7b5215e4..7045b9eb 100644 --- a/src/Controller/AuthorizationController.php +++ b/src/Controller/AuthorizationController.php @@ -111,6 +111,42 @@ public function indexAction(Request $request): Response $response = $this->server->completeAuthorizationRequest($authRequest, $serverResponse); } catch (OAuthServerException $e) { + // https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1 + // If the request fails due to a missing, invalid, or mismatching + // redirection URI, or if the client identifier is missing or invalid, + // the authorization server SHOULD inform the resource owner of the + // error and MUST NOT automatically redirect the user-agent to the + // invalid redirection URI. + // + // If the resource owner denies the access request or if the request + // fails for reasons other than a missing or invalid redirection URI, + // the authorization server informs the client by adding the following + // parameters to the query component of the redirection URI using the + // "application/x-www-form-urlencoded" format + // + // so if redirectUri is not already set, we try to set request redirect_uri params, fallback to first redirectUri of client + /** @psalm-suppress RiskyTruthyFalsyComparison !empty($e->getHint()),empty($e->getRedirectUri()) we really want to check null and empty */ + if (!empty($client) + && ('invalid_client' === $e->getErrorType() + || ('invalid_request' === $e->getErrorType() && !empty($e->getHint()) + && !\in_array(sscanf($e->getHint() ?? '', 'Check the `%s` parameter')[0] ?? null, ['client_id', 'client_secret', 'redirect_uri']))) + && empty($e->getRedirectUri())) { + /** @var \League\Bundle\OAuth2ServerBundle\Model\ClientInterface $client */ + $redirectUri = $request->query->get('redirect_uri', // query string has priority + (string)$request->request->get('redirect_uri', // then we check body to support POST request + $client->getRedirectUris()[0]?->__toString() ?? '')); // then first client redirect uri + if (!empty($redirectUri)) { + $e = new OAuthServerException( + $e->getMessage(), + $e->getCode(), + $e->getErrorType(), + $e->getHttpStatusCode(), + $e->getHint(), + $redirectUri, + $e->getPrevious(), + ); + } + } $response = $e->generateHttpResponse($serverResponse); } diff --git a/tests/Acceptance/AuthorizationEndpointTest.php b/tests/Acceptance/AuthorizationEndpointTest.php index b28bea98..b91592f7 100644 --- a/tests/Acceptance/AuthorizationEndpointTest.php +++ b/tests/Acceptance/AuthorizationEndpointTest.php @@ -206,15 +206,15 @@ public function testAuthCodeRequestWithClientWhoIsNotAllowedToMakeARequestWithPl $response = $this->client->getResponse(); - $this->assertSame(400, $response->getStatusCode()); - - $this->assertSame('application/json', $response->headers->get('Content-Type')); - - $jsonResponse = json_decode($response->getContent(), true); + $this->assertSame(302, $response->getStatusCode()); + $redirectUri = $response->headers->get('Location'); - $this->assertSame('invalid_request', $jsonResponse['error']); - $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $jsonResponse['error_description']); - $this->assertSame('Plain code challenge method is not allowed for this client', $jsonResponse['hint']); + $this->assertStringStartsWith(FixtureFactory::FIXTURE_CLIENT_FIRST_REDIRECT_URI, $redirectUri); + $query = []; + parse_str(parse_url($redirectUri, \PHP_URL_QUERY), $query); + $this->assertSame('invalid_request', $query['error']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $query['error_description']); + $this->assertSame('Plain code challenge method is not allowed for this client', $query['hint']); } public function testAuthCodeRequestWithClientWhoIsAllowedToMakeARequestWithPlainCodeChallengeMethod(): void