From 20252ac5251c199e860f15f78730393a00dcc579 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 8 Sep 2022 22:17:15 +0000 Subject: [PATCH 01/13] Use fragment for error response on implicit grant --- src/Grant/ImplicitGrant.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 0bd91d5ac..7b3de8a3e 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -225,7 +225,8 @@ public function completeAuthorizationRequest(AuthorizationRequest $authorization $finalRedirectUri, [ 'state' => $authorizationRequest->getState(), - ] + ], + $this->queryDelimiter ) ); } From 2f6f002c6a441be67eeaf67c6e7f00c56c5e6063 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sat, 10 Sep 2022 16:06:23 +0430 Subject: [PATCH 02/13] Add query delimiter to OAuthServerException --- src/Exception/OAuthServerException.php | 31 ++++++++++++------- src/Grant/AbstractGrant.php | 5 +-- src/Grant/ImplicitGrant.php | 7 +++-- .../AuthorizationServerMiddlewareTest.php | 12 +++++++ 4 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index a0be0a5dd..cf737795e 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -36,6 +36,11 @@ class OAuthServerException extends Exception */ private $redirectUri; + /** + * @var string + */ + private $queryDelimiter; + /** * @var array */ @@ -56,14 +61,16 @@ class OAuthServerException extends Exception * @param null|string $hint A helper hint * @param null|string $redirectUri A HTTP URI to redirect the user back to * @param Throwable $previous Previous exception + * @param string $queryDelimiter Query delimiter of the redirect URI */ - public function __construct($message, $code, $errorType, $httpStatusCode = 400, $hint = null, $redirectUri = null, Throwable $previous = null) + public function __construct($message, $code, $errorType, $httpStatusCode = 400, $hint = null, $redirectUri = null, Throwable $previous = null, $queryDelimiter = '?') { parent::__construct($message, $code, $previous); $this->httpStatusCode = $httpStatusCode; $this->errorType = $errorType; $this->hint = $hint; $this->redirectUri = $redirectUri; + $this->queryDelimiter = $queryDelimiter; $this->payload = [ 'error' => $errorType, 'error_description' => $message, @@ -161,12 +168,13 @@ public static function invalidClient(ServerRequestInterface $serverRequest) /** * Invalid scope error. * - * @param string $scope The bad scope - * @param null|string $redirectUri A HTTP URI to redirect the user back to + * @param string $scope The bad scope + * @param null|string $redirectUri A HTTP URI to redirect the user back to + * @param string $queryDelimiter Query delimiter of the redirect URI * * @return static */ - public static function invalidScope($scope, $redirectUri = null) + public static function invalidScope($scope, $redirectUri = null, $queryDelimiter = '?') { $errorMessage = 'The requested scope is invalid, unknown, or malformed'; @@ -179,7 +187,7 @@ public static function invalidScope($scope, $redirectUri = null) ); } - return new static($errorMessage, 5, 'invalid_scope', 400, $hint, $redirectUri); + return new static($errorMessage, 5, 'invalid_scope', 400, $hint, $redirectUri, null, $queryDelimiter); } /** @@ -235,10 +243,11 @@ public static function invalidRefreshToken($hint = null, Throwable $previous = n * @param null|string $hint * @param null|string $redirectUri * @param Throwable $previous + * @param string $queryDelimiter * * @return static */ - public static function accessDenied($hint = null, $redirectUri = null, Throwable $previous = null) + public static function accessDenied($hint = null, $redirectUri = null, Throwable $previous = null, $queryDelimiter = '?') { return new static( 'The resource owner or authorization server denied the request.', @@ -247,7 +256,8 @@ public static function accessDenied($hint = null, $redirectUri = null, Throwable 401, $hint, $redirectUri, - $previous + $previous, + $queryDelimiter ); } @@ -295,11 +305,8 @@ public function generateHttpResponse(ResponseInterface $response, $useFragment = $payload = $this->getPayload(); if ($this->redirectUri !== null) { - if ($useFragment === true) { - $this->redirectUri .= (\strstr($this->redirectUri, '#') === false) ? '#' : '&'; - } else { - $this->redirectUri .= (\strstr($this->redirectUri, '?') === false) ? '?' : '&'; - } + $queryDelimiter = $useFragment === true ? '#' : $this->queryDelimiter; + $this->redirectUri .= (\strstr($this->redirectUri, $queryDelimiter) === false) ? $queryDelimiter : '&'; return $response->withStatus(302)->withHeader('Location', $this->redirectUri . \http_build_query($payload)); } diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 0a5b514fc..1b47dbfc6 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -296,12 +296,13 @@ protected function validateRedirectUri( * * @param string|array $scopes * @param string $redirectUri + * @param string $queryDelimiter * * @throws OAuthServerException * * @return ScopeEntityInterface[] */ - public function validateScopes($scopes, $redirectUri = null) + public function validateScopes($scopes, $redirectUri = null, $queryDelimiter = '?') { if ($scopes === null) { $scopes = []; @@ -319,7 +320,7 @@ public function validateScopes($scopes, $redirectUri = null) $scope = $this->scopeRepository->getScopeEntityByIdentifier($scopeItem); if ($scope instanceof ScopeEntityInterface === false) { - throw OAuthServerException::invalidScope($scopeItem, $redirectUri); + throw OAuthServerException::invalidScope($scopeItem, $redirectUri, $queryDelimiter); } $validScopes[] = $scope; diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 7b3de8a3e..3190c6b6d 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -146,7 +146,8 @@ public function validateAuthorizationRequest(ServerRequestInterface $request) $scopes = $this->validateScopes( $this->getQueryStringParameter('scope', $request, $this->defaultScope), - $redirectUri + $redirectUri, + $this->queryDelimiter ); $stateParameter = $this->getQueryStringParameter('state', $request); @@ -227,7 +228,9 @@ public function completeAuthorizationRequest(AuthorizationRequest $authorization 'state' => $authorizationRequest->getState(), ], $this->queryDelimiter - ) + ), + null, + $this->queryDelimiter ); } } diff --git a/tests/Middleware/AuthorizationServerMiddlewareTest.php b/tests/Middleware/AuthorizationServerMiddlewareTest.php index 69c8d3791..3e7e18da1 100644 --- a/tests/Middleware/AuthorizationServerMiddlewareTest.php +++ b/tests/Middleware/AuthorizationServerMiddlewareTest.php @@ -126,4 +126,16 @@ public function testOAuthErrorResponseRedirectUriFragment() $response->getHeader('location')[0] ); } + + public function testOAuthErrorResponseRedirectUriWithDelimiter() + { + $exception = OAuthServerException::invalidScope('test', 'http://foo/bar', '#'); + $response = $exception->generateHttpResponse(new Response()); + + $this->assertEquals(302, $response->getStatusCode()); + $this->assertEquals( + 'http://foo/bar#error=invalid_scope&error_description=The+requested+scope+is+invalid%2C+unknown%2C+or+malformed&hint=Check+the+%60test%60+scope&message=The+requested+scope+is+invalid%2C+unknown%2C+or+malformed', + $response->getHeader('location')[0] + ); + } } From ea7d7a616e1e465c6d96a9372f9c15c126e25e09 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Fri, 29 Mar 2024 20:53:13 +0330 Subject: [PATCH 03/13] formatting --- src/Exception/OAuthServerException.php | 10 +++------- src/Grant/AbstractGrant.php | 7 ++----- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index a5cc8d6d0..29088b094 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -35,7 +35,7 @@ class OAuthServerException extends Exception /** * Throw a new exception. */ - final public function __construct(string $message, int $code, private string $errorType, private int $httpStatusCode = 400, private ?string $hint = null, private ?string $redirectUri = null, Throwable $previous = null, private ?string $queryDelimiter) + final public function __construct(string $message, int $code, private string $errorType, private int $httpStatusCode = 400, private ?string $hint = null, private ?string $redirectUri = null, Throwable $previous = null, private ?string $queryDelimiter = '?') { parent::__construct($message, $code, $previous); $this->payload = [ @@ -168,12 +168,8 @@ public static function invalidRefreshToken(?string $hint = null, Throwable $prev /** * Access denied. */ - public static function accessDenied( - ?string $hint = null, - ?string $redirectUri = null, - Throwable $previous = null, - string $queryDelimiter = '?' - ): static { + public static function accessDenied(?string $hint = null, ?string $redirectUri = null, Throwable $previous = null, string $queryDelimiter = '?'): static + { return new static( 'The resource owner or authorization server denied the request.', 9, diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index b0adc70d8..f7dc8a8f9 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -249,11 +249,8 @@ protected function validateRedirectUri( * * @return ScopeEntityInterface[] */ - public function validateScopes( - string|array|null $scopes, - string $redirectUri = null, - string $queryDelimiter = '?' - ): array { + public function validateScopes(string|array|null $scopes, string $redirectUri = null, string $queryDelimiter = '?'): array + { if ($scopes === null) { $scopes = []; } elseif (is_string($scopes)) { From 62a46b39cb44821200ed3e30c1d6bdb012a5e2b9 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 29 Aug 2024 19:47:17 +0330 Subject: [PATCH 04/13] fix tests --- tests/Middleware/AuthorizationServerMiddlewareTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Middleware/AuthorizationServerMiddlewareTest.php b/tests/Middleware/AuthorizationServerMiddlewareTest.php index ea02a3732..2e09cb266 100644 --- a/tests/Middleware/AuthorizationServerMiddlewareTest.php +++ b/tests/Middleware/AuthorizationServerMiddlewareTest.php @@ -141,7 +141,7 @@ public function testOAuthErrorResponseRedirectUriWithDelimiter() $this->assertEquals(302, $response->getStatusCode()); $this->assertEquals( - 'http://foo/bar#error=invalid_scope&error_description=The+requested+scope+is+invalid%2C+unknown%2C+or+malformed&hint=Check+the+%60test%60+scope&message=The+requested+scope+is+invalid%2C+unknown%2C+or+malformed', + 'http://foo/bar#error=invalid_scope&error_description=The+requested+scope+is+invalid%2C+unknown%2C+or+malformed&hint=Check+the+%60test%60+scope', $response->getHeader('location')[0] ); } From 20bd570f2b4733be7650048a085a7b690efb2f9f Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 29 Aug 2024 19:52:15 +0330 Subject: [PATCH 05/13] formatting --- tests/Middleware/AuthorizationServerMiddlewareTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Middleware/AuthorizationServerMiddlewareTest.php b/tests/Middleware/AuthorizationServerMiddlewareTest.php index 2e09cb266..63b347f95 100644 --- a/tests/Middleware/AuthorizationServerMiddlewareTest.php +++ b/tests/Middleware/AuthorizationServerMiddlewareTest.php @@ -134,7 +134,7 @@ public function testOAuthErrorResponseRedirectUriFragment(): void ); } - public function testOAuthErrorResponseRedirectUriWithDelimiter() + public function testOAuthErrorResponseRedirectUriWithDelimiter(): void { $exception = OAuthServerException::invalidScope('test', 'http://foo/bar', '#'); $response = $exception->generateHttpResponse(new Response()); From 8d970b3726c7964280e67ac9d7f2b5221f357c71 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 29 Aug 2024 20:13:08 +0330 Subject: [PATCH 06/13] formatting --- src/Exception/OAuthServerException.php | 2 +- tests/Middleware/AuthorizationServerMiddlewareTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 48c1164ce..5b7d98097 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -267,7 +267,7 @@ public function generateHttpResponse(ResponseInterface $response, bool $useFragm $payload = $this->getPayload(); if ($this->redirectUri !== null) { - $queryDelimiter = $useFragment === true ? '#' : $this->queryDelimiter; + $queryDelimiter = $useFragment === true ? '#' : ($this->queryDelimiter ?? '?'); $this->redirectUri .= (str_contains($this->redirectUri, $queryDelimiter) === false) ? $queryDelimiter : '&'; return $response->withStatus(302)->withHeader('Location', $this->redirectUri . http_build_query($payload)); diff --git a/tests/Middleware/AuthorizationServerMiddlewareTest.php b/tests/Middleware/AuthorizationServerMiddlewareTest.php index 63b347f95..f39b41ee6 100644 --- a/tests/Middleware/AuthorizationServerMiddlewareTest.php +++ b/tests/Middleware/AuthorizationServerMiddlewareTest.php @@ -139,8 +139,8 @@ public function testOAuthErrorResponseRedirectUriWithDelimiter(): void $exception = OAuthServerException::invalidScope('test', 'http://foo/bar', '#'); $response = $exception->generateHttpResponse(new Response()); - $this->assertEquals(302, $response->getStatusCode()); - $this->assertEquals( + self::assertEquals(302, $response->getStatusCode()); + self::assertEquals( 'http://foo/bar#error=invalid_scope&error_description=The+requested+scope+is+invalid%2C+unknown%2C+or+malformed&hint=Check+the+%60test%60+scope', $response->getHeader('location')[0] ); From e78ff0571fc851f7a79c42e6941115266aec29fe Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 29 Aug 2024 20:15:58 +0330 Subject: [PATCH 07/13] formatting --- src/Exception/OAuthServerException.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 5b7d98097..8e6c025af 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -33,7 +33,7 @@ class OAuthServerException extends Exception /** * Throw a new exception. */ - final public function __construct(string $message, int $code, private string $errorType, private int $httpStatusCode = 400, private ?string $hint = null, private ?string $redirectUri = null, Throwable $previous = null, private ?string $queryDelimiter = '?') + final public function __construct(string $message, int $code, private string $errorType, private int $httpStatusCode = 400, private ?string $hint = null, private ?string $redirectUri = null, Throwable $previous = null, private string $queryDelimiter = '?') { parent::__construct($message, $code, $previous); $this->payload = [ @@ -267,7 +267,7 @@ public function generateHttpResponse(ResponseInterface $response, bool $useFragm $payload = $this->getPayload(); if ($this->redirectUri !== null) { - $queryDelimiter = $useFragment === true ? '#' : ($this->queryDelimiter ?? '?'); + $queryDelimiter = $useFragment === true ? '#' : $this->queryDelimiter; $this->redirectUri .= (str_contains($this->redirectUri, $queryDelimiter) === false) ? $queryDelimiter : '&'; return $response->withStatus(302)->withHeader('Location', $this->redirectUri . http_build_query($payload)); From 127f9207b92a293dadbc25a565863415b5e3e05c Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 3 Oct 2024 15:15:17 +0330 Subject: [PATCH 08/13] wip --- src/Exception/OAuthServerException.php | 20 ++++++++++---------- src/Grant/AbstractGrant.php | 4 ++-- src/Grant/AuthCodeGrant.php | 15 +++++++++------ src/Grant/ImplicitGrant.php | 18 +++++++++--------- 4 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 8e6c025af..07efe400d 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -33,7 +33,7 @@ class OAuthServerException extends Exception /** * Throw a new exception. */ - final public function __construct(string $message, int $code, private string $errorType, private int $httpStatusCode = 400, private ?string $hint = null, private ?string $redirectUri = null, Throwable $previous = null, private string $queryDelimiter = '?') + final public function __construct(string $message, int $code, private string $errorType, private int $httpStatusCode = 400, private ?string $hint = null, private ?string $redirectUri = null, Throwable $previous = null) { parent::__construct($message, $code, $previous); $this->payload = [ @@ -112,7 +112,7 @@ public static function invalidClient(ServerRequestInterface $serverRequest): sta /** * Invalid scope error */ - public static function invalidScope(string $scope, string|null $redirectUri = null, string $queryDelimiter = '?'): static + public static function invalidScope(string $scope, string|null $redirectUri = null): static { $errorMessage = 'The requested scope is invalid, unknown, or malformed'; @@ -125,7 +125,7 @@ public static function invalidScope(string $scope, string|null $redirectUri = nu ); } - return new static($errorMessage, 5, 'invalid_scope', 400, $hint, $redirectUri, null, $queryDelimiter); + return new static($errorMessage, 5, 'invalid_scope', 400, $hint, $redirectUri); } /** @@ -166,7 +166,7 @@ public static function invalidRefreshToken(?string $hint = null, Throwable $prev /** * Access denied. */ - public static function accessDenied(?string $hint = null, ?string $redirectUri = null, Throwable $previous = null, string $queryDelimiter = '?'): static + public static function accessDenied(?string $hint = null, ?string $redirectUri = null, Throwable $previous = null): static { return new static( 'The resource owner or authorization server denied the request.', @@ -175,8 +175,7 @@ public static function accessDenied(?string $hint = null, ?string $redirectUri = 401, $hint, $redirectUri, - $previous, - $queryDelimiter + $previous ); } @@ -267,10 +266,11 @@ public function generateHttpResponse(ResponseInterface $response, bool $useFragm $payload = $this->getPayload(); if ($this->redirectUri !== null) { - $queryDelimiter = $useFragment === true ? '#' : $this->queryDelimiter; - $this->redirectUri .= (str_contains($this->redirectUri, $queryDelimiter) === false) ? $queryDelimiter : '&'; - - return $response->withStatus(302)->withHeader('Location', $this->redirectUri . http_build_query($payload)); + if ($useFragment === true) { + $this->redirectUri .= (str_contains($this->redirectUri, '#') === false) ? '#' : '&'; + } else { + $this->redirectUri .= (str_contains($this->redirectUri, '?') === false) ? '?' : '&'; + } } foreach ($headers as $header => $content) { diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 70f286015..ea0064c3b 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -243,7 +243,7 @@ protected function validateRedirectUri( * * @return ScopeEntityInterface[] */ - public function validateScopes(string|array|null $scopes, string $redirectUri = null, string $queryDelimiter = '?'): array + public function validateScopes(string|array|null $scopes, string $redirectUri = null): array { if ($scopes === null) { $scopes = []; @@ -257,7 +257,7 @@ public function validateScopes(string|array|null $scopes, string $redirectUri = $scope = $this->scopeRepository->getScopeEntityByIdentifier($scopeItem); if ($scope instanceof ScopeEntityInterface === false) { - throw OAuthServerException::invalidScope($scopeItem, $redirectUri, $queryDelimiter); + throw OAuthServerException::invalidScope($scopeItem, $redirectUri); } $validScopes[] = $scope; diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 3b4467a5f..e5bde175e 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -278,19 +278,22 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); throw OAuthServerException::invalidClient($request); + } else { + $redirectUri = is_array($client->getRedirectUri()) + ? $client->getRedirectUri()[0] + : $client->getRedirectUri(); } - $defaultClientRedirectUri = is_array($client->getRedirectUri()) - ? $client->getRedirectUri()[0] - : $client->getRedirectUri(); + $stateParameter = $this->getQueryStringParameter('state', $request); $scopes = $this->validateScopes( $this->getQueryStringParameter('scope', $request, $this->defaultScope), - $redirectUri ?? $defaultClientRedirectUri + $this->makeRedirectUri( + $redirectUri, + $stateParameter !== null ? ['state' => $stateParameter] : [] + ) ); - $stateParameter = $this->getQueryStringParameter('state', $request); - $authorizationRequest = $this->createAuthorizationRequest(); $authorizationRequest->setGrantTypeId($this->getIdentifier()); $authorizationRequest->setClient($client); diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index d3a9c617a..0ca9f1c81 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -111,8 +111,8 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A if ($redirectUri !== null) { $this->validateRedirectUri($redirectUri, $client, $request); } elseif ( - is_array($client->getRedirectUri()) && count($client->getRedirectUri()) !== 1 - || $client->getRedirectUri() === '' + $client->getRedirectUri() === '' || + (is_array($client->getRedirectUri()) && count($client->getRedirectUri()) !== 1) ) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); throw OAuthServerException::invalidClient($request); @@ -122,14 +122,16 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A : $client->getRedirectUri(); } + $stateParameter = $this->getQueryStringParameter('state', $request); + $scopes = $this->validateScopes( $this->getQueryStringParameter('scope', $request, $this->defaultScope), - $redirectUri, - $this->queryDelimiter + $this->makeRedirectUri( + $redirectUri, + $stateParameter !== null ? ['state' => $stateParameter] : [] + ) ); - $stateParameter = $this->getQueryStringParameter('state', $request); - $authorizationRequest = $this->createAuthorizationRequest(); $authorizationRequest->setGrantTypeId($this->getIdentifier()); $authorizationRequest->setClient($client); @@ -202,9 +204,7 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth 'state' => $authorizationRequest->getState(), ], $this->queryDelimiter - ), - null, - $this->queryDelimiter + ) ); } } From 2fb2d110d57f2248c4f3a63c67790b273a450f68 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 3 Oct 2024 15:15:58 +0330 Subject: [PATCH 09/13] wip --- src/Exception/OAuthServerException.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 07efe400d..9eff92456 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -271,6 +271,8 @@ public function generateHttpResponse(ResponseInterface $response, bool $useFragm } else { $this->redirectUri .= (str_contains($this->redirectUri, '?') === false) ? '?' : '&'; } + + return $response->withStatus(302)->withHeader('Location', $this->redirectUri . http_build_query($payload)); } foreach ($headers as $header => $content) { From b592267f7be96ffd0c053c3c95ed432f4eef2193 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 3 Oct 2024 15:17:02 +0330 Subject: [PATCH 10/13] fix tests --- tests/Middleware/AuthorizationServerMiddlewareTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Middleware/AuthorizationServerMiddlewareTest.php b/tests/Middleware/AuthorizationServerMiddlewareTest.php index f39b41ee6..89fbc3f6c 100644 --- a/tests/Middleware/AuthorizationServerMiddlewareTest.php +++ b/tests/Middleware/AuthorizationServerMiddlewareTest.php @@ -136,8 +136,8 @@ public function testOAuthErrorResponseRedirectUriFragment(): void public function testOAuthErrorResponseRedirectUriWithDelimiter(): void { - $exception = OAuthServerException::invalidScope('test', 'http://foo/bar', '#'); - $response = $exception->generateHttpResponse(new Response()); + $exception = OAuthServerException::invalidScope('test', 'http://foo/bar'); + $response = $exception->generateHttpResponse(new Response(), true); self::assertEquals(302, $response->getStatusCode()); self::assertEquals( From a4090fe2e89cd9c33c2a9e2ed1ffa7918894d6c8 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 3 Oct 2024 15:33:00 +0330 Subject: [PATCH 11/13] wip --- src/CryptKey.php | 2 +- src/Grant/AbstractAuthorizeGrant.php | 11 +++++++++++ src/Grant/AuthCodeGrant.php | 18 ++---------------- src/Grant/ImplicitGrant.php | 16 +++++----------- 4 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/CryptKey.php b/src/CryptKey.php index 3a3bdd9d4..135a2f7b9 100644 --- a/src/CryptKey.php +++ b/src/CryptKey.php @@ -72,7 +72,7 @@ public function __construct(string $keyPath, protected ?string $passPhrase = nul throw new LogicException('Invalid key supplied'); } - if ($keyPermissionsCheck === true) { + if ($keyPermissionsCheck === true && PHP_OS_FAMILY !== 'Windows') { // Verify the permissions of the key $keyPathPerms = decoct(fileperms($this->keyPath) & 0777); if (in_array($keyPathPerms, ['400', '440', '600', '640', '660'], true) === false) { diff --git a/src/Grant/AbstractAuthorizeGrant.php b/src/Grant/AbstractAuthorizeGrant.php index 22b58c4e2..7b44016e4 100644 --- a/src/Grant/AbstractAuthorizeGrant.php +++ b/src/Grant/AbstractAuthorizeGrant.php @@ -14,6 +14,7 @@ namespace League\OAuth2\Server\Grant; +use League\OAuth2\Server\Entities\ClientEntityInterface; use League\OAuth2\Server\RequestTypes\AuthorizationRequest; use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface; @@ -35,4 +36,14 @@ protected function createAuthorizationRequest(): AuthorizationRequestInterface { return new AuthorizationRequest(); } + + /** + * Get the client redirect URI. + */ + protected function getClientRedirectUri(ClientEntityInterface $client): string + { + return is_array($client->getRedirectUri()) + ? $client->getRedirectUri()[0] + : $client->getRedirectUri(); + } } diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index e5bde175e..22b00d02a 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -278,10 +278,6 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); throw OAuthServerException::invalidClient($request); - } else { - $redirectUri = is_array($client->getRedirectUri()) - ? $client->getRedirectUri()[0] - : $client->getRedirectUri(); } $stateParameter = $this->getQueryStringParameter('state', $request); @@ -289,7 +285,7 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A $scopes = $this->validateScopes( $this->getQueryStringParameter('scope', $request, $this->defaultScope), $this->makeRedirectUri( - $redirectUri, + $redirectUri ?? $this->getClientRedirectUri($client), $stateParameter !== null ? ['state' => $stateParameter] : [] ) ); @@ -357,7 +353,7 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth } $finalRedirectUri = $authorizationRequest->getRedirectUri() - ?? $this->getClientRedirectUri($authorizationRequest); + ?? $this->getClientRedirectUri($authorizationRequest->getClient()); // The user approved the client, redirect them back with an auth code if ($authorizationRequest->isAuthorizationApproved() === true) { @@ -411,14 +407,4 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth ) ); } - - /** - * Get the client redirect URI if not set in the request. - */ - private function getClientRedirectUri(AuthorizationRequestInterface $authorizationRequest): string - { - return is_array($authorizationRequest->getClient()->getRedirectUri()) - ? $authorizationRequest->getClient()->getRedirectUri()[0] - : $authorizationRequest->getClient()->getRedirectUri(); - } } diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 0ca9f1c81..81252dea1 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -116,10 +116,6 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A ) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); throw OAuthServerException::invalidClient($request); - } else { - $redirectUri = is_array($client->getRedirectUri()) - ? $client->getRedirectUri()[0] - : $client->getRedirectUri(); } $stateParameter = $this->getQueryStringParameter('state', $request); @@ -127,8 +123,9 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A $scopes = $this->validateScopes( $this->getQueryStringParameter('scope', $request, $this->defaultScope), $this->makeRedirectUri( - $redirectUri, - $stateParameter !== null ? ['state' => $stateParameter] : [] + $redirectUri ?? $this->getClientRedirectUri($client), + $stateParameter !== null ? ['state' => $stateParameter] : [], + $this->queryDelimiter ) ); @@ -155,11 +152,8 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth throw new LogicException('An instance of UserEntityInterface should be set on the AuthorizationRequest'); } - $clientRegisteredRedirectUri = is_array($authorizationRequest->getClient()->getRedirectUri()) - ? $authorizationRequest->getClient()->getRedirectUri()[0] - : $authorizationRequest->getClient()->getRedirectUri(); - - $finalRedirectUri = $authorizationRequest->getRedirectUri() ?? $clientRegisteredRedirectUri; + $finalRedirectUri = $authorizationRequest->getRedirectUri() + ?? $this->getClientRedirectUri($authorizationRequest->getClient()); // The user approved the client, redirect them back with an access token if ($authorizationRequest->isAuthorizationApproved() === true) { From df7d3deb809c578da35fc602ba66c7dccdf1b2d1 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 3 Oct 2024 15:33:42 +0330 Subject: [PATCH 12/13] revert --- src/CryptKey.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CryptKey.php b/src/CryptKey.php index 135a2f7b9..3a3bdd9d4 100644 --- a/src/CryptKey.php +++ b/src/CryptKey.php @@ -72,7 +72,7 @@ public function __construct(string $keyPath, protected ?string $passPhrase = nul throw new LogicException('Invalid key supplied'); } - if ($keyPermissionsCheck === true && PHP_OS_FAMILY !== 'Windows') { + if ($keyPermissionsCheck === true) { // Verify the permissions of the key $keyPathPerms = decoct(fileperms($this->keyPath) & 0777); if (in_array($keyPathPerms, ['400', '440', '600', '640', '660'], true) === false) { From 02691dc2f5350305b18f24031bf1a714194135e9 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 3 Oct 2024 15:49:22 +0330 Subject: [PATCH 13/13] wip --- .../Middleware/AuthorizationServerMiddlewareTest.php | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/Middleware/AuthorizationServerMiddlewareTest.php b/tests/Middleware/AuthorizationServerMiddlewareTest.php index 89fbc3f6c..814e96a6c 100644 --- a/tests/Middleware/AuthorizationServerMiddlewareTest.php +++ b/tests/Middleware/AuthorizationServerMiddlewareTest.php @@ -133,16 +133,4 @@ public function testOAuthErrorResponseRedirectUriFragment(): void $response->getHeader('location')[0] ); } - - public function testOAuthErrorResponseRedirectUriWithDelimiter(): void - { - $exception = OAuthServerException::invalidScope('test', 'http://foo/bar'); - $response = $exception->generateHttpResponse(new Response(), true); - - self::assertEquals(302, $response->getStatusCode()); - self::assertEquals( - 'http://foo/bar#error=invalid_scope&error_description=The+requested+scope+is+invalid%2C+unknown%2C+or+malformed&hint=Check+the+%60test%60+scope', - $response->getHeader('location')[0] - ); - } }