Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: authorization code error should be redirect #193

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: authorization code error should be redirect #193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b637211
fix: authorization code error should be redirect
TLG-Gildas Jul 24, 2024
6322066
Merge branch 'master' into fix/authorization-code-error-redirect
TLG-Gildas Sep 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/Controller/AuthorizationController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
16 changes: 8 additions & 8 deletions tests/Acceptance/AuthorizationEndpointTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down