Skip to content

Commit b637211

Browse files
committed
fix: authorization code error should be redirect
1 parent 58d4b11 commit b637211

File tree

2 files changed

+44
-8
lines changed

2 files changed

+44
-8
lines changed

src/Controller/AuthorizationController.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,42 @@ public function indexAction(Request $request): Response
111111

112112
$response = $this->server->completeAuthorizationRequest($authRequest, $serverResponse);
113113
} catch (OAuthServerException $e) {
114+
// https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1
115+
// If the request fails due to a missing, invalid, or mismatching
116+
// redirection URI, or if the client identifier is missing or invalid,
117+
// the authorization server SHOULD inform the resource owner of the
118+
// error and MUST NOT automatically redirect the user-agent to the
119+
// invalid redirection URI.
120+
//
121+
// If the resource owner denies the access request or if the request
122+
// fails for reasons other than a missing or invalid redirection URI,
123+
// the authorization server informs the client by adding the following
124+
// parameters to the query component of the redirection URI using the
125+
// "application/x-www-form-urlencoded" format
126+
//
127+
// so if redirectUri is not already set, we try to set request redirect_uri params, fallback to first redirectUri of client
128+
/** @psalm-suppress RiskyTruthyFalsyComparison !empty($e->getHint()),empty($e->getRedirectUri()) we really want to check null and empty */
129+
if (!empty($client)
130+
&& ('invalid_client' === $e->getErrorType()
131+
|| ('invalid_request' === $e->getErrorType() && !empty($e->getHint())
132+
&& !\in_array(sscanf($e->getHint() ?? '', 'Check the `%s` parameter')[0] ?? null, ['client_id', 'client_secret', 'redirect_uri'])))
133+
&& empty($e->getRedirectUri())) {
134+
/** @var \League\Bundle\OAuth2ServerBundle\Model\ClientInterface $client */
135+
$redirectUri = $request->query->get('redirect_uri', // query string has priority
136+
(string)$request->request->get('redirect_uri', // then we check body to support POST request
137+
$client->getRedirectUris()[0]?->__toString() ?? '')); // then first client redirect uri
138+
if (!empty($redirectUri)) {
139+
$e = new OAuthServerException(
140+
$e->getMessage(),
141+
$e->getCode(),
142+
$e->getErrorType(),
143+
$e->getHttpStatusCode(),
144+
$e->getHint(),
145+
$redirectUri,
146+
$e->getPrevious(),
147+
);
148+
}
149+
}
114150
$response = $e->generateHttpResponse($serverResponse);
115151
}
116152

tests/Acceptance/AuthorizationEndpointTest.php

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,15 +191,15 @@ public function testAuthCodeRequestWithClientWhoIsNotAllowedToMakeARequestWithPl
191191

192192
$response = $this->client->getResponse();
193193

194-
$this->assertSame(400, $response->getStatusCode());
195-
196-
$this->assertSame('application/json', $response->headers->get('Content-Type'));
197-
198-
$jsonResponse = json_decode($response->getContent(), true);
194+
$this->assertSame(302, $response->getStatusCode());
195+
$redirectUri = $response->headers->get('Location');
199196

200-
$this->assertSame('invalid_request', $jsonResponse['error']);
201-
$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['message']);
202-
$this->assertSame('Plain code challenge method is not allowed for this client', $jsonResponse['hint']);
197+
$this->assertStringStartsWith(FixtureFactory::FIXTURE_CLIENT_FIRST_REDIRECT_URI, $redirectUri);
198+
$query = [];
199+
parse_str(parse_url($redirectUri, \PHP_URL_QUERY), $query);
200+
$this->assertEquals('invalid_request', $query['error']);
201+
$this->assertEquals('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $query['message']);
202+
$this->assertEquals('Plain code challenge method is not allowed for this client', $query['hint']);
203203
}
204204

205205
public function testAuthCodeRequestWithClientWhoIsAllowedToMakeARequestWithPlainCodeChallengeMethod(): void

0 commit comments

Comments
 (0)