From 20252ac5251c199e860f15f78730393a00dcc579 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 8 Sep 2022 22:17:15 +0000 Subject: [PATCH 01/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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/23] 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 981c4da5d3bcfa305680b5b98a10083b6f9ace4c Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Tue, 1 Oct 2024 12:41:28 +0330 Subject: [PATCH 08/23] issue and persist a new refresh token --- src/Grant/RefreshTokenGrant.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Grant/RefreshTokenGrant.php b/src/Grant/RefreshTokenGrant.php index a632990c7..ad7c32326 100644 --- a/src/Grant/RefreshTokenGrant.php +++ b/src/Grant/RefreshTokenGrant.php @@ -83,13 +83,11 @@ public function respondToAccessTokenRequest( $responseType->setAccessToken($accessToken); // Issue and persist new refresh token if given - if ($this->revokeRefreshTokens) { - $refreshToken = $this->issueRefreshToken($accessToken); + $refreshToken = $this->issueRefreshToken($accessToken); - if ($refreshToken !== null) { - $this->getEmitter()->emit(new RequestRefreshTokenEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request, $refreshToken)); - $responseType->setRefreshToken($refreshToken); - } + if ($refreshToken !== null) { + $this->getEmitter()->emit(new RequestRefreshTokenEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request, $refreshToken)); + $responseType->setRefreshToken($refreshToken); } return $responseType; From 127f9207b92a293dadbc25a565863415b5e3e05c Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 3 Oct 2024 15:15:17 +0330 Subject: [PATCH 09/23] 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 10/23] 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 11/23] 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 12/23] 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 13/23] 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 14/23] 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] - ); - } } From f090882655c11255316c8e6be2e3c83fb8df2b03 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Fri, 11 Oct 2024 03:14:18 +0330 Subject: [PATCH 15/23] add tests --- tests/Grant/AuthCodeGrantTest.php | 57 ++++++++++++++++++++++++++- tests/Grant/ImplicitGrantTest.php | 64 +++++++++++++++++++++++++++---- 2 files changed, 111 insertions(+), 10 deletions(-) diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index fc6ac07c3..ffff73895 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -469,6 +469,50 @@ public function testValidateAuthorizationRequestInvalidCodeChallengeMethod(): vo $grant->validateAuthorizationRequest($request); } + public function testValidateAuthorizationRequestInvalidScopes(): void + { + $client = new ClientEntity(); + $client->setRedirectUri(self::REDIRECT_URI); + $client->setConfidential(); + + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn(null); + + $grant = new AuthCodeGrant( + $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), + $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(), + new DateInterval('PT10M') + ); + + $grant->setClientRepository($clientRepositoryMock); + $grant->setScopeRepository($scopeRepositoryMock); + $grant->setDefaultScope(self::DEFAULT_SCOPE); + + $request = (new ServerRequest())->withQueryParams([ + 'response_type' => 'code', + 'client_id' => 'foo', + 'redirect_uri' => self::REDIRECT_URI, + 'scope' => 'foo', + 'state' => 'foo', + ]); + + try { + $grant->validateAuthorizationRequest($request); + } catch (OAuthServerException $e) { + $this->assertSame(5, $e->getCode()); + $this->assertSame('invalid_scope', $e->getErrorType()); + $this->assertSame('https://foo/bar?state=foo', $e->getRedirectUri()); + + return; + } + + $this->expectException(OAuthServerException::class); + $this->expectExceptionCode(5); + } + public function testCompleteAuthorizationRequest(): void { $client = new ClientEntity(); @@ -529,6 +573,7 @@ public function testCompleteAuthorizationRequestDenied(): void $authRequest->setClient($client); $authRequest->setGrantTypeId('authorization_code'); $authRequest->setUser(new UserEntity()); + $authRequest->setState('foo'); $authCodeRepository = $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(); $authCodeRepository->method('getNewAuthCode')->willReturn(new AuthCodeEntity()); @@ -540,10 +585,18 @@ public function testCompleteAuthorizationRequestDenied(): void ); $grant->setEncryptionKey($this->cryptStub->getKey()); + try { + $grant->completeAuthorizationRequest($authRequest); + } catch (OAuthServerException $e) { + $this->assertSame(9, $e->getCode()); + $this->assertSame('access_denied', $e->getErrorType()); + $this->assertSame('http://foo/bar?state=foo', $e->getRedirectUri()); + + return; + } + $this->expectException(OAuthServerException::class); $this->expectExceptionCode(9); - - $grant->completeAuthorizationRequest($authRequest); } public function testRespondToAccessTokenRequest(): void diff --git a/tests/Grant/ImplicitGrantTest.php b/tests/Grant/ImplicitGrantTest.php index c2b943197..2218911d0 100644 --- a/tests/Grant/ImplicitGrantTest.php +++ b/tests/Grant/ImplicitGrantTest.php @@ -95,7 +95,7 @@ public function testValidateAuthorizationRequest(): void $grant->setDefaultScope(self::DEFAULT_SCOPE); $request = (new ServerRequest())->withQueryParams([ - 'response_type' => 'code', + 'response_type' => 'token', 'client_id' => 'foo', 'redirect_uri' => self::REDIRECT_URI, ]); @@ -120,7 +120,7 @@ public function testValidateAuthorizationRequestRedirectUriArray(): void $grant->setDefaultScope(self::DEFAULT_SCOPE); $request = (new ServerRequest())->withQueryParams([ - 'response_type' => 'code', + 'response_type' => 'token', 'client_id' => 'foo', 'redirect_uri' => self::REDIRECT_URI, ]); @@ -135,7 +135,7 @@ public function testValidateAuthorizationRequestMissingClientId(): void $grant = new ImplicitGrant(new DateInterval('PT10M')); $grant->setClientRepository($clientRepositoryMock); - $request = (new ServerRequest())->withQueryParams(['response_type' => 'code']); + $request = (new ServerRequest())->withQueryParams(['response_type' => 'token']); $this->expectException(OAuthServerException::class); $this->expectExceptionCode(3); @@ -152,7 +152,7 @@ public function testValidateAuthorizationRequestInvalidClientId(): void $grant->setClientRepository($clientRepositoryMock); $request = (new ServerRequest())->withQueryParams([ - 'response_type' => 'code', + 'response_type' => 'token', 'client_id' => 'foo', ]); @@ -173,7 +173,7 @@ public function testValidateAuthorizationRequestBadRedirectUriString(): void $grant->setClientRepository($clientRepositoryMock); $request = (new ServerRequest())->withQueryParams([ - 'response_type' => 'code', + 'response_type' => 'token', 'client_id' => 'foo', 'redirect_uri' => 'http://bar', ]); @@ -195,7 +195,7 @@ public function testValidateAuthorizationRequestBadRedirectUriArray(): void $grant->setClientRepository($clientRepositoryMock); $request = (new ServerRequest())->withQueryParams([ - 'response_type' => 'code', + 'response_type' => 'token', 'client_id' => 'foo', 'redirect_uri' => 'http://bar', ]); @@ -206,6 +206,45 @@ public function testValidateAuthorizationRequestBadRedirectUriArray(): void $grant->validateAuthorizationRequest($request); } + public function testValidateAuthorizationRequestInvalidScopes(): void + { + $client = new ClientEntity(); + $client->setRedirectUri(self::REDIRECT_URI); + + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn(null); + + $grant = new ImplicitGrant(new DateInterval('PT10M')); + + $grant->setClientRepository($clientRepositoryMock); + $grant->setScopeRepository($scopeRepositoryMock); + $grant->setDefaultScope(self::DEFAULT_SCOPE); + + $request = (new ServerRequest())->withQueryParams([ + 'response_type' => 'token', + 'client_id' => 'foo', + 'redirect_uri' => self::REDIRECT_URI, + 'scope' => 'foo', + 'state' => 'foo', + ]); + + try { + $grant->validateAuthorizationRequest($request); + } catch (OAuthServerException $e) { + $this->assertSame(5, $e->getCode()); + $this->assertSame('invalid_scope', $e->getErrorType()); + $this->assertSame('https://foo/bar#state=foo', $e->getRedirectUri()); + + return; + } + + $this->expectException(OAuthServerException::class); + $this->expectExceptionCode(5); + } + public function testCompleteAuthorizationRequest(): void { $client = new ClientEntity(); @@ -248,6 +287,7 @@ public function testCompleteAuthorizationRequestDenied(): void $authRequest->setClient($client); $authRequest->setGrantTypeId('authorization_code'); $authRequest->setUser(new UserEntity()); + $authRequest->setState('foo'); $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); @@ -261,10 +301,18 @@ public function testCompleteAuthorizationRequestDenied(): void $grant->setAccessTokenRepository($accessTokenRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); + try { + $grant->completeAuthorizationRequest($authRequest); + } catch (OAuthServerException $e) { + $this->assertSame(9, $e->getCode()); + $this->assertSame('access_denied', $e->getErrorType()); + $this->assertSame('https://foo/bar#state=foo', $e->getRedirectUri()); + + return; + } + $this->expectException(OAuthServerException::class); $this->expectExceptionCode(9); - - $grant->completeAuthorizationRequest($authRequest); } public function testAccessTokenRepositoryUniqueConstraintCheck(): void From 0d98af89800fa7f68188b1d46bef6cc8af65b488 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Fri, 11 Oct 2024 03:21:16 +0330 Subject: [PATCH 16/23] fix static analyse --- tests/Grant/AuthCodeGrantTest.php | 12 ++++++------ tests/Grant/ImplicitGrantTest.php | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index ffff73895..b6c827316 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -502,9 +502,9 @@ public function testValidateAuthorizationRequestInvalidScopes(): void try { $grant->validateAuthorizationRequest($request); } catch (OAuthServerException $e) { - $this->assertSame(5, $e->getCode()); - $this->assertSame('invalid_scope', $e->getErrorType()); - $this->assertSame('https://foo/bar?state=foo', $e->getRedirectUri()); + self::assertSame(5, $e->getCode()); + self::assertSame('invalid_scope', $e->getErrorType()); + self::assertSame('https://foo/bar?state=foo', $e->getRedirectUri()); return; } @@ -588,9 +588,9 @@ public function testCompleteAuthorizationRequestDenied(): void try { $grant->completeAuthorizationRequest($authRequest); } catch (OAuthServerException $e) { - $this->assertSame(9, $e->getCode()); - $this->assertSame('access_denied', $e->getErrorType()); - $this->assertSame('http://foo/bar?state=foo', $e->getRedirectUri()); + self::assertSame(9, $e->getCode()); + self::assertSame('access_denied', $e->getErrorType()); + self::assertSame('http://foo/bar?state=foo', $e->getRedirectUri()); return; } diff --git a/tests/Grant/ImplicitGrantTest.php b/tests/Grant/ImplicitGrantTest.php index 2218911d0..30ad4cb6c 100644 --- a/tests/Grant/ImplicitGrantTest.php +++ b/tests/Grant/ImplicitGrantTest.php @@ -234,9 +234,9 @@ public function testValidateAuthorizationRequestInvalidScopes(): void try { $grant->validateAuthorizationRequest($request); } catch (OAuthServerException $e) { - $this->assertSame(5, $e->getCode()); - $this->assertSame('invalid_scope', $e->getErrorType()); - $this->assertSame('https://foo/bar#state=foo', $e->getRedirectUri()); + self::assertSame(5, $e->getCode()); + self::assertSame('invalid_scope', $e->getErrorType()); + self::assertSame('https://foo/bar#state=foo', $e->getRedirectUri()); return; } @@ -304,9 +304,9 @@ public function testCompleteAuthorizationRequestDenied(): void try { $grant->completeAuthorizationRequest($authRequest); } catch (OAuthServerException $e) { - $this->assertSame(9, $e->getCode()); - $this->assertSame('access_denied', $e->getErrorType()); - $this->assertSame('https://foo/bar#state=foo', $e->getRedirectUri()); + self::assertSame(9, $e->getCode()); + self::assertSame('access_denied', $e->getErrorType()); + self::assertSame('https://foo/bar#state=foo', $e->getRedirectUri()); return; } From fbecb61bd8f7f66cd20dfcaab1ea72a39ebb5f72 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Fri, 11 Oct 2024 03:21:37 +0330 Subject: [PATCH 17/23] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85c99ac6c..8c565614b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Auto-generated event emitter is now persisted. Previously, a new emitter was generated every time (PR #1428) - Fixed bug where you could not omit a redirect uri even if one had not been specified during the auth request (PR #1428) +- Fixed bug where "state" parameter wasn't present on `invalid_scope` error response and wasn't on fragment part of `access_denied` redirect URI on Implicit grant (PR #1298) ## [9.0.0] - released 2024-05-13 ### Added From 921713744e2f84a424d7b151155d37cc9216898c Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Fri, 11 Oct 2024 04:09:49 +0330 Subject: [PATCH 18/23] add tests --- tests/Grant/RefreshTokenGrantTest.php | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php index b37001a80..fbebcac6d 100644 --- a/tests/Grant/RefreshTokenGrantTest.php +++ b/tests/Grant/RefreshTokenGrantTest.php @@ -5,6 +5,7 @@ namespace LeagueTests\Grant; use DateInterval; +use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequest; use League\OAuth2\Server\CryptKey; use League\OAuth2\Server\Entities\RefreshTokenEntityInterface; @@ -14,6 +15,7 @@ use League\OAuth2\Server\Repositories\ClientRepositoryInterface; use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; +use League\OAuth2\Server\ResponseTypes\BearerTokenResponse; use LeagueTests\Stubs\AccessTokenEntity; use LeagueTests\Stubs\ClientEntity; use LeagueTests\Stubs\CryptTraitStub; @@ -688,11 +690,15 @@ public function testUnrevokedRefreshToken(): void $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity); $scopeRepositoryMock->method('finalizeScopes')->willReturn([$scopeEntity]); + $accessTokenEntity = new AccessTokenEntity(); + $accessTokenEntity->setClient($client); + $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); - $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); + $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessTokenEntity); $accessTokenRepositoryMock->expects(self::once())->method('persistNewAccessToken')->willReturnSelf(); $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity()); $refreshTokenRepositoryMock->method('isRefreshTokenRevoked')->willReturn(false); $refreshTokenRepositoryMock->expects(self::never())->method('revokeRefreshToken'); @@ -727,11 +733,22 @@ public function testUnrevokedRefreshToken(): void $grant->setScopeRepository($scopeRepositoryMock); $grant->setAccessTokenRepository($accessTokenRepositoryMock); $grant->setEncryptionKey($this->cryptStub->getKey()); - $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); + $grant->setPrivateKey($privateKey = new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->revokeRefreshTokens(false); - $grant->respondToAccessTokenRequest($serverRequest, new StubResponseType(), new DateInterval('PT5M')); + $responseType = new BearerTokenResponse(); + $responseType->setPrivateKey($privateKey); + $responseType->setEncryptionKey($this->cryptStub->getKey()); + + $response = $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M')) + ->generateHttpResponse(new Response()); + + $json = json_decode((string) $response->getBody()); self::assertFalse($refreshTokenRepositoryMock->isRefreshTokenRevoked($refreshTokenId)); + self::assertEquals('Bearer', $json->token_type); + self::assertObjectHasProperty('expires_in', $json); + self::assertObjectHasProperty('access_token', $json); + self::assertObjectHasProperty('refresh_token', $json); } } From 844d86dc9b5fb8e2929d63c6c99861aa29806b6b Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Fri, 11 Oct 2024 15:47:34 +0330 Subject: [PATCH 19/23] add changelog entry --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85c99ac6c..6aa241cbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Auto-generated event emitter is now persisted. Previously, a new emitter was generated every time (PR #1428) - Fixed bug where you could not omit a redirect uri even if one had not been specified during the auth request (PR #1428) +- Fixed bug where disabling refresh token revocation via `revokeRefreshTokens(false)` unintentionally disables issuing new refresh token (PR #1449) ## [9.0.0] - released 2024-05-13 ### Added - Device Authorization Grant added (PR #1074) -- GrantTypeInterface has a new function, `revokeRefreshTokens()` for enabling or disabling refresh tokens after use (PR #1375) +- GrantTypeInterface has a new function, `revokeRefreshTokens()` for enabling or disabling refresh token revocation (PR #1375) - A CryptKeyInterface to allow developers to change the CryptKey implementation with greater ease (PR #1044) - The authorization server can now finalize scopes when a client uses a refresh token (PR #1094) - An AuthorizationRequestInterface to make it easier to extend the AuthorizationRequest (PR #1110) From e88f89d20c6b3833fd4859cfea007b5421b7c40e Mon Sep 17 00:00:00 2001 From: Andrew Millington Date: Fri, 11 Oct 2024 23:48:20 +0100 Subject: [PATCH 20/23] Update tests to fail if exception not hit --- tests/Grant/AuthCodeGrantTest.php | 6 ++---- tests/Grant/ImplicitGrantTest.php | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index b6c827316..4e0586109 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -509,8 +509,7 @@ public function testValidateAuthorizationRequestInvalidScopes(): void return; } - $this->expectException(OAuthServerException::class); - $this->expectExceptionCode(5); + $this->fail('The expected exception was not thrown'); } public function testCompleteAuthorizationRequest(): void @@ -595,8 +594,7 @@ public function testCompleteAuthorizationRequestDenied(): void return; } - $this->expectException(OAuthServerException::class); - $this->expectExceptionCode(9); + $this->fail('The expected exception was not thrown'); } public function testRespondToAccessTokenRequest(): void diff --git a/tests/Grant/ImplicitGrantTest.php b/tests/Grant/ImplicitGrantTest.php index 30ad4cb6c..c38f59629 100644 --- a/tests/Grant/ImplicitGrantTest.php +++ b/tests/Grant/ImplicitGrantTest.php @@ -241,8 +241,7 @@ public function testValidateAuthorizationRequestInvalidScopes(): void return; } - $this->expectException(OAuthServerException::class); - $this->expectExceptionCode(5); + $this->fail('Did not throw expected exception'); } public function testCompleteAuthorizationRequest(): void @@ -311,8 +310,7 @@ public function testCompleteAuthorizationRequestDenied(): void return; } - $this->expectException(OAuthServerException::class); - $this->expectExceptionCode(9); + $this->fail('Did not throw expected exception'); } public function testAccessTokenRepositoryUniqueConstraintCheck(): void From 2c698e21a3e232958997040bca3fef92e2fc2b56 Mon Sep 17 00:00:00 2001 From: Andrew Millington Date: Fri, 11 Oct 2024 23:55:37 +0100 Subject: [PATCH 21/23] Fix phpstan errors --- tests/Grant/AuthCodeGrantTest.php | 4 ++-- tests/Grant/ImplicitGrantTest.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index 4e0586109..646056618 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -509,7 +509,7 @@ public function testValidateAuthorizationRequestInvalidScopes(): void return; } - $this->fail('The expected exception was not thrown'); + self::fail('The expected exception was not thrown'); } public function testCompleteAuthorizationRequest(): void @@ -594,7 +594,7 @@ public function testCompleteAuthorizationRequestDenied(): void return; } - $this->fail('The expected exception was not thrown'); + self::fail('The expected exception was not thrown'); } public function testRespondToAccessTokenRequest(): void diff --git a/tests/Grant/ImplicitGrantTest.php b/tests/Grant/ImplicitGrantTest.php index c38f59629..617aaa842 100644 --- a/tests/Grant/ImplicitGrantTest.php +++ b/tests/Grant/ImplicitGrantTest.php @@ -241,7 +241,7 @@ public function testValidateAuthorizationRequestInvalidScopes(): void return; } - $this->fail('Did not throw expected exception'); + self::fail('Did not throw expected exception'); } public function testCompleteAuthorizationRequest(): void @@ -310,7 +310,7 @@ public function testCompleteAuthorizationRequestDenied(): void return; } - $this->fail('Did not throw expected exception'); + self::fail('Did not throw expected exception'); } public function testAccessTokenRepositoryUniqueConstraintCheck(): void From 234aafec5b93d4c5261f63f59c0f297a14c385e8 Mon Sep 17 00:00:00 2001 From: Andrew Millington Date: Mon, 14 Oct 2024 23:12:25 +0100 Subject: [PATCH 22/23] Ensure refresh token returned is new after use --- CHANGELOG.md | 2 +- tests/Grant/RefreshTokenGrantTest.php | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc6045ff6..7f205c86d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed bug where you could not omit a redirect uri even if one had not been specified during the auth request (PR #1428) - Fixed bug where "state" parameter wasn't present on `invalid_scope` error response and wasn't on fragment part of `access_denied` redirect URI on Implicit grant (PR #1298) - Fixed bug where disabling refresh token revocation via `revokeRefreshTokens(false)` unintentionally disables issuing new refresh token (PR #1449) -- + ## [9.0.0] - released 2024-05-13 ### Added - Device Authorization Grant added (PR #1074) diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php index fbebcac6d..027731527 100644 --- a/tests/Grant/RefreshTokenGrantTest.php +++ b/tests/Grant/RefreshTokenGrantTest.php @@ -594,10 +594,10 @@ public function testRespondToRequestFinalizeScopes(): void ); $serverRequest = (new ServerRequest())->withParsedBody([ - 'client_id' => 'foo', - 'client_secret' => 'bar', - 'refresh_token' => $encryptedOldRefreshToken, - 'scope' => 'foo bar', + 'client_id' => 'foo', + 'client_secret' => 'bar', + 'refresh_token' => $encryptedOldRefreshToken, + 'scope' => 'foo bar', ]); $responseType = new StubResponseType(); @@ -630,7 +630,7 @@ public function testRevokedRefreshToken(): void $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); $refreshTokenRepositoryMock->method('isRefreshTokenRevoked') - ->will(self::onConsecutiveCalls(false, true)); + ->will(self::onConsecutiveCalls(false, true)); $refreshTokenRepositoryMock->expects(self::once())->method('revokeRefreshToken')->with(self::equalTo($refreshTokenId)); $oldRefreshToken = json_encode( @@ -728,12 +728,14 @@ public function testUnrevokedRefreshToken(): void 'scope' => 'foo', ]); + $privateKey = new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'); + $grant = new RefreshTokenGrant($refreshTokenRepositoryMock); $grant->setClientRepository($clientRepositoryMock); $grant->setScopeRepository($scopeRepositoryMock); $grant->setAccessTokenRepository($accessTokenRepositoryMock); $grant->setEncryptionKey($this->cryptStub->getKey()); - $grant->setPrivateKey($privateKey = new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); + $grant->setPrivateKey($privateKey); $grant->revokeRefreshTokens(false); $responseType = new BearerTokenResponse(); @@ -750,5 +752,6 @@ public function testUnrevokedRefreshToken(): void self::assertObjectHasProperty('expires_in', $json); self::assertObjectHasProperty('access_token', $json); self::assertObjectHasProperty('refresh_token', $json); + self::assertNotSame($json->refresh_token, $encryptedOldRefreshToken); } } From 1dc1ee861e8490a9e73d668b1ceccfa27436d861 Mon Sep 17 00:00:00 2001 From: Andrew Millington Date: Mon, 14 Oct 2024 23:17:04 +0100 Subject: [PATCH 23/23] Update changelog for version 9.0.1 --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f205c86d..342285f28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] + +## [9.0.1] - released 2024-10-14 ### Fixed - Auto-generated event emitter is now persisted. Previously, a new emitter was generated every time (PR #1428) - Fixed bug where you could not omit a redirect uri even if one had not been specified during the auth request (PR #1428) @@ -653,7 +655,8 @@ Version 5 is a complete code rewrite. - First major release -[Unreleased]: https://github.com/thephpleague/oauth2-server/compare/9.0.0...HEAD +[Unreleased]: https://github.com/thephpleague/oauth2-server/compare/9.0.1...HEAD +[9.0.1]: https://github.com/thephpleague/oauth2-server/compare/9.0.0...9.0.1 [9.0.0]: https://github.com/thephpleague/oauth2-server/compare/9.0.0-RC1...9.0.0 [9.0.0-RC1]: https://github.com/thephpleague/oauth2-server/compare/8.5.4...9.0.0-RC1 [8.5.4]: https://github.com/thephpleague/oauth2-server/compare/8.5.3...8.5.4