From bd1973ad37876fb6b1295f127a73744534cc629a Mon Sep 17 00:00:00 2001 From: sephster Date: Fri, 11 Dec 2020 22:31:13 +0000 Subject: [PATCH 01/10] Remove hint parameter from OAuthServerException --- src/Exception/OAuthServerException.php | 72 ++++++-------------------- 1 file changed, 15 insertions(+), 57 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index bb6af44de..480158466 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -26,11 +26,6 @@ class OAuthServerException extends Exception */ private $errorType; - /** - * @var null|string - */ - private $hint; - /** * @var null|string */ @@ -53,24 +48,19 @@ class OAuthServerException extends Exception * @param int $code Error code * @param string $errorType Error type * @param int $httpStatusCode HTTP status code to send (default = 400) - * @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 */ - public function __construct($message, $code, $errorType, $httpStatusCode = 400, $hint = null, $redirectUri = null, Throwable $previous = null) + public function __construct($message, $code, $errorType, $httpStatusCode = 400, $redirectUri = null, Throwable $previous = null) { parent::__construct($message, $code, $previous); $this->httpStatusCode = $httpStatusCode; $this->errorType = $errorType; - $this->hint = $hint; $this->redirectUri = $redirectUri; $this->payload = [ 'error' => $errorType, 'error_description' => $message, ]; - if ($hint !== null) { - $this->payload['hint'] = $hint; - } } /** @@ -118,28 +108,22 @@ public function setServerRequest(ServerRequestInterface $serverRequest) */ public static function unsupportedGrantType() { - $errorMessage = 'The authorization grant type is not supported by the authorization server.'; - $hint = 'Check that all required parameters have been provided'; + $errorMessage = 'The grant type is not supported by the authorization server.'; - return new static($errorMessage, 2, 'unsupported_grant_type', 400, $hint); + return new static($errorMessage, 2, 'unsupported_grant_type', 400); } /** * Invalid request error. * - * @param string $parameter The invalid parameter - * @param null|string $hint - * @param Throwable $previous Previous exception + * @param string $errorMessage The error message + * @param Throwable $previous Previous exception * * @return static */ - public static function invalidRequest($parameter, $hint = null, Throwable $previous = null) + public static function invalidRequest($errorMessage, Throwable $previous = null) { - $errorMessage = 'The request is missing a required parameter, includes an invalid parameter value, ' . - 'includes a parameter more than once, or is otherwise malformed.'; - $hint = ($hint === null) ? \sprintf('Check the `%s` parameter', $parameter) : $hint; - - return new static($errorMessage, 3, 'invalid_request', 400, $hint, null, $previous); + return new static($errorMessage, 3, 'invalid_request', 400, null, $previous); } /** @@ -170,16 +154,7 @@ public static function invalidScope($scope, $redirectUri = null) { $errorMessage = 'The requested scope is invalid, unknown, or malformed'; - if (empty($scope)) { - $hint = 'Specify a scope in the request or set a default scope'; - } else { - $hint = \sprintf( - 'Check the `%s` scope', - \htmlspecialchars($scope, ENT_QUOTES, 'UTF-8', false) - ); - } - - return new static($errorMessage, 5, 'invalid_scope', 400, $hint, $redirectUri); + return new static($errorMessage, 5, 'invalid_scope', 400, $redirectUri); } /** @@ -195,18 +170,17 @@ public static function invalidCredentials() /** * Server error. * - * @param string $hint * @param Throwable $previous * * @return static * * @codeCoverageIgnore */ - public static function serverError($hint, Throwable $previous = null) + public static function serverError(Throwable $previous = null) { return new static( 'The authorization server encountered an unexpected condition which prevented it from fulfilling' - . ' the request: ' . $hint, + . ' the request', 7, 'server_error', 500, @@ -219,33 +193,30 @@ public static function serverError($hint, Throwable $previous = null) /** * Invalid refresh token. * - * @param null|string $hint * @param Throwable $previous * * @return static */ - public static function invalidRefreshToken($hint = null, Throwable $previous = null) + public static function invalidRefreshToken(Throwable $previous = null) { - return new static('The refresh token is invalid.', 8, 'invalid_grant', 400, $hint, null, $previous); + return new static('The refresh token is invalid.', 8, 'invalid_grant', 400, null, $previous); } /** * Access denied. * - * @param null|string $hint * @param null|string $redirectUri * @param Throwable $previous * * @return static */ - public static function accessDenied($hint = null, $redirectUri = null, Throwable $previous = null) + public static function accessDenied($redirectUri = null, Throwable $previous = null) { return new static( 'The resource owner or authorization server denied the request.', 9, 'access_denied', 401, - $hint, $redirectUri, $previous ); @@ -254,20 +225,15 @@ public static function accessDenied($hint = null, $redirectUri = null, Throwable /** * Invalid grant. * - * @param string $hint - * * @return static */ - public static function invalidGrant($hint = '') + public static function invalidGrant($message) { return new static( - 'The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token ' - . 'is invalid, expired, revoked, does not match the redirection URI used in the authorization request, ' - . 'or was issued to another client.', + $message, 10, 'invalid_grant', 400, - $hint ); } @@ -378,12 +344,4 @@ public function getHttpStatusCode() { return $this->httpStatusCode; } - - /** - * @return null|string - */ - public function getHint() - { - return $this->hint; - } } From 89b35bccb5caf0dbedc42ef6a21acf1a7bf3552d Mon Sep 17 00:00:00 2001 From: sephster Date: Fri, 11 Dec 2020 22:31:44 +0000 Subject: [PATCH 02/10] Change invalidRequest message --- src/Grant/AuthCodeGrant.php | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 71289d288..0f9f29d26 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -107,7 +107,7 @@ public function respondToAccessTokenRequest( $encryptedAuthCode = $this->getRequestParameter('code', $request, null); if ($encryptedAuthCode === null) { - throw OAuthServerException::invalidRequest('code'); + throw OAuthServerException::invalidRequest('The auth code is missing'); } try { @@ -123,7 +123,7 @@ public function respondToAccessTokenRequest( $authCodePayload->auth_code_id ); } catch (LogicException $e) { - throw OAuthServerException::invalidRequest('code', 'Cannot decrypt the authorization code', $e); + throw OAuthServerException::invalidRequest('Cannot decrypt the authorization code', $e); } // Validate code challenge @@ -131,16 +131,13 @@ public function respondToAccessTokenRequest( $codeVerifier = $this->getRequestParameter('code_verifier', $request, null); if ($codeVerifier === null) { - throw OAuthServerException::invalidRequest('code_verifier'); + throw OAuthServerException::invalidRequest('The code_verifier is missing'); } // Validate code_verifier according to RFC-7636 // @see: https://tools.ietf.org/html/rfc7636#section-4.1 if (\preg_match('/^[A-Za-z0-9-._~]{43,128}$/', $codeVerifier) !== 1) { - throw OAuthServerException::invalidRequest( - 'code_verifier', - 'Code Verifier must follow the specifications of RFC-7636.' - ); + throw OAuthServerException::invalidRequest('The code_verifier contains illegal characters'); } if (\property_exists($authCodePayload, 'code_challenge_method')) { @@ -193,11 +190,11 @@ private function validateAuthorizationCode( ServerRequestInterface $request ) { if (!\property_exists($authCodePayload, 'auth_code_id')) { - throw OAuthServerException::invalidRequest('code', 'Authorization code malformed'); + throw OAuthServerException::invalidRequest('The auth_code_id property is missing from the auth code'); } if (\time() > $authCodePayload->expire_time) { - throw OAuthServerException::invalidRequest('code', 'Authorization code has expired'); + throw OAuthServerException::invalidRequest('Authorization code has expired'); } if ($this->authCodeRepository->isAuthCodeRevoked($authCodePayload->auth_code_id) === true) { @@ -205,17 +202,17 @@ private function validateAuthorizationCode( } if ($authCodePayload->client_id !== $client->getIdentifier()) { - throw OAuthServerException::invalidRequest('code', 'Authorization code was not issued to this client'); + throw OAuthServerException::invalidRequest('Authorization code was not issued to this client'); } // The redirect URI is required in this request $redirectUri = $this->getRequestParameter('redirect_uri', $request, null); if (empty($authCodePayload->redirect_uri) === false && $redirectUri === null) { - throw OAuthServerException::invalidRequest('redirect_uri'); + throw OAuthServerException::invalidRequest('Missing redirect_uri'); } if ($authCodePayload->redirect_uri !== $redirectUri) { - throw OAuthServerException::invalidRequest('redirect_uri', 'Invalid redirect URI'); + throw OAuthServerException::invalidRequest('Invalid redirect URI'); } } @@ -253,7 +250,7 @@ public function validateAuthorizationRequest(ServerRequestInterface $request) ); if ($clientId === null) { - throw OAuthServerException::invalidRequest('client_id'); + throw OAuthServerException::invalidRequest('client_id is not set'); } $client = $this->getClientEntityOrFail($clientId, $request); @@ -297,7 +294,6 @@ public function validateAuthorizationRequest(ServerRequestInterface $request) if (\array_key_exists($codeChallengeMethod, $this->codeChallengeVerifiers) === false) { throw OAuthServerException::invalidRequest( - 'code_challenge_method', 'Code challenge method must be one of ' . \implode(', ', \array_map( function ($method) { return '`' . $method . '`'; @@ -311,15 +307,14 @@ function ($method) { // @see: https://tools.ietf.org/html/rfc7636#section-4.2 if (\preg_match('/^[A-Za-z0-9-._~]{43,128}$/', $codeChallenge) !== 1) { throw OAuthServerException::invalidRequest( - 'code_challenge', - 'Code challenge must follow the specifications of RFC-7636.' + 'Code challenge contains illegal characters' ); } $authorizationRequest->setCodeChallenge($codeChallenge); $authorizationRequest->setCodeChallengeMethod($codeChallengeMethod); } elseif ($this->requireCodeChallengeForPublicClients && !$client->isConfidential()) { - throw OAuthServerException::invalidRequest('code_challenge', 'Code challenge must be provided for public clients'); + throw OAuthServerException::invalidRequest('Code challenge must be provided for public clients'); } return $authorizationRequest; From c4607caee8bc343f60c9834322d508e92719d4b4 Mon Sep 17 00:00:00 2001 From: sephster Date: Fri, 11 Dec 2020 22:36:19 +0000 Subject: [PATCH 03/10] Update invalidRequest calls to use new function signature --- src/Grant/AbstractGrant.php | 2 +- src/Grant/ImplicitGrant.php | 2 +- src/Grant/PasswordGrant.php | 4 ++-- src/Grant/RefreshTokenGrant.php | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 49c3a1038..def0b6d3d 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -239,7 +239,7 @@ protected function getClientCredentials(ServerRequestInterface $request) $clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser); if (\is_null($clientId)) { - throw OAuthServerException::invalidRequest('client_id'); + throw OAuthServerException::invalidRequest('client_id parameter is missing from the request'); } $clientSecret = $this->getRequestParameter('client_secret', $request, $basicAuthPassword); diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index f4b15f22f..fa9f9eb20 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -121,7 +121,7 @@ public function validateAuthorizationRequest(ServerRequestInterface $request) ); if (\is_null($clientId)) { - throw OAuthServerException::invalidRequest('client_id'); + throw OAuthServerException::invalidRequest('client_id parameter is missing from the request'); } $client = $this->getClientEntityOrFail($clientId, $request); diff --git a/src/Grant/PasswordGrant.php b/src/Grant/PasswordGrant.php index cff5d40c3..4388feb11 100644 --- a/src/Grant/PasswordGrant.php +++ b/src/Grant/PasswordGrant.php @@ -88,13 +88,13 @@ protected function validateUser(ServerRequestInterface $request, ClientEntityInt $username = $this->getRequestParameter('username', $request); if (\is_null($username)) { - throw OAuthServerException::invalidRequest('username'); + throw OAuthServerException::invalidRequest('username parameter is missing from the request'); } $password = $this->getRequestParameter('password', $request); if (\is_null($password)) { - throw OAuthServerException::invalidRequest('password'); + throw OAuthServerException::invalidRequest('password parameter is missing from the request'); } $user = $this->userRepository->getUserEntityByUserCredentials( diff --git a/src/Grant/RefreshTokenGrant.php b/src/Grant/RefreshTokenGrant.php index b6c47f7a9..af4ec003a 100644 --- a/src/Grant/RefreshTokenGrant.php +++ b/src/Grant/RefreshTokenGrant.php @@ -95,7 +95,7 @@ protected function validateOldRefreshToken(ServerRequestInterface $request, $cli { $encryptedRefreshToken = $this->getRequestParameter('refresh_token', $request); if (\is_null($encryptedRefreshToken)) { - throw OAuthServerException::invalidRequest('refresh_token'); + throw OAuthServerException::invalidRequest('refresh_token parameter is missing from the request'); } // Validate refresh token From 84c13f473005fcbd8b393245de985fa540095906 Mon Sep 17 00:00:00 2001 From: sephster Date: Fri, 11 Dec 2020 22:48:33 +0000 Subject: [PATCH 04/10] Update invalidClient calls --- src/Exception/OAuthServerException.php | 5 +++-- src/Grant/AbstractGrant.php | 8 ++++---- src/Grant/AuthCodeGrant.php | 2 +- src/Grant/ClientCredentialsGrant.php | 2 +- src/Grant/ImplicitGrant.php | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 480158466..6e1011d99 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -129,13 +129,14 @@ public static function invalidRequest($errorMessage, Throwable $previous = null) /** * Invalid client error. * + * @param string $errorMessage * @param ServerRequestInterface $serverRequest * * @return static */ - public static function invalidClient(ServerRequestInterface $serverRequest) + public static function invalidClient($errorMessage, ServerRequestInterface $serverRequest) { - $exception = new static('Client authentication failed', 4, 'invalid_client', 401); + $exception = new static($errorMessage, 4, 'invalid_client', 401); $exception->setServerRequest($serverRequest); diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index def0b6d3d..2cca23b86 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -182,7 +182,7 @@ protected function validateClient(ServerRequestInterface $request) if ($this->clientRepository->validateClient($clientId, $clientSecret, $this->getIdentifier()) === false) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient($request); + throw OAuthServerException::invalidClient('Client authentication failure', $request); } $client = $this->getClientEntityOrFail($clientId, $request); @@ -218,7 +218,7 @@ protected function getClientEntityOrFail($clientId, ServerRequestInterface $requ if ($client instanceof ClientEntityInterface === false || empty($client->getRedirectUri())) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient($request); + throw OAuthServerException::invalidClient('Client instance could not be retrieved or client redirect_uri missing', $request); } return $client; @@ -266,12 +266,12 @@ protected function validateRedirectUri( && (\strcmp($client->getRedirectUri(), $redirectUri) !== 0) ) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient($request); + throw OAuthServerException::invalidClient('Invalid redirect_uri provided', $request); } elseif (\is_array($client->getRedirectUri()) && \in_array($redirectUri, $client->getRedirectUri(), true) === false ) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient($request); + throw OAuthServerException::invalidClient('Invalid redirect_uri provided', $request); } } diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 0f9f29d26..101cfd0b2 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -262,7 +262,7 @@ public function validateAuthorizationRequest(ServerRequestInterface $request) } elseif (\is_array($client->getRedirectUri()) && \count($client->getRedirectUri()) !== 1) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient($request); + throw OAuthServerException::invalidClient('No redirect_uri to use with the request', $request); } $defaultClientRedirectUri = \is_array($client->getRedirectUri()) diff --git a/src/Grant/ClientCredentialsGrant.php b/src/Grant/ClientCredentialsGrant.php index 691f421bc..22e72f4df 100644 --- a/src/Grant/ClientCredentialsGrant.php +++ b/src/Grant/ClientCredentialsGrant.php @@ -37,7 +37,7 @@ public function respondToAccessTokenRequest( if (!$client->isConfidential()) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient($request); + throw OAuthServerException::invalidClient('The client is not confidential', $request); } // Validate request diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index fa9f9eb20..1a28e8090 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -133,7 +133,7 @@ public function validateAuthorizationRequest(ServerRequestInterface $request) } elseif (\is_array($client->getRedirectUri()) && \count($client->getRedirectUri()) !== 1 || empty($client->getRedirectUri())) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient($request); + throw OAuthServerException::invalidClient('Could not find a redirect_uri to use with the request', $request); } else { $redirectUri = \is_array($client->getRedirectUri()) ? $client->getRedirectUri()[0] From 2197f604659ea4668be40059de0002c32a49a870 Mon Sep 17 00:00:00 2001 From: sephster Date: Fri, 11 Dec 2020 22:53:34 +0000 Subject: [PATCH 05/10] Pass error message to invalidScope --- src/Exception/OAuthServerException.php | 8 +++----- src/Grant/AbstractGrant.php | 5 ++++- src/Grant/RefreshTokenGrant.php | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 6e1011d99..40dc855a5 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -146,15 +146,13 @@ public static function invalidClient($errorMessage, ServerRequestInterface $serv /** * Invalid scope error. * - * @param string $scope The bad scope - * @param null|string $redirectUri A HTTP URI to redirect the user back to + * @param string $errorMessage The error message + * @param null|string $redirectUri A HTTP URI to redirect the user back to * * @return static */ - public static function invalidScope($scope, $redirectUri = null) + public static function invalidScope($errorMessage, $redirectUri = null) { - $errorMessage = 'The requested scope is invalid, unknown, or malformed'; - return new static($errorMessage, 5, 'invalid_scope', 400, $redirectUri); } diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 2cca23b86..4a3e71084 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -297,7 +297,10 @@ 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 . ' is not a recognised scope identifier', + $redirectUri + ); } $validScopes[] = $scope; diff --git a/src/Grant/RefreshTokenGrant.php b/src/Grant/RefreshTokenGrant.php index af4ec003a..29195715d 100644 --- a/src/Grant/RefreshTokenGrant.php +++ b/src/Grant/RefreshTokenGrant.php @@ -57,7 +57,7 @@ public function respondToAccessTokenRequest( // the request doesn't include any new scopes foreach ($scopes as $scope) { if (\in_array($scope->getIdentifier(), $oldRefreshToken['scopes'], true) === false) { - throw OAuthServerException::invalidScope($scope->getIdentifier()); + throw OAuthServerException::invalidScope('Scope ' . $scope->getIdentifier() . ' cannot be granted'); } } From 359546465e1e1aed1aaac5e5242bd8a23ac0f246 Mon Sep 17 00:00:00 2001 From: sephster Date: Fri, 11 Dec 2020 22:54:01 +0000 Subject: [PATCH 06/10] Remove invalid_credentials function --- src/Exception/OAuthServerException.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 40dc855a5..ee7859ae3 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -156,16 +156,6 @@ public static function invalidScope($errorMessage, $redirectUri = null) return new static($errorMessage, 5, 'invalid_scope', 400, $redirectUri); } - /** - * Invalid credentials error. - * - * @return static - */ - public static function invalidCredentials() - { - return new static('The user credentials were incorrect.', 6, 'invalid_credentials', 401); - } - /** * Server error. * From e4e36a49391175a68aa9a938dbadb8257e61dc3c Mon Sep 17 00:00:00 2001 From: sephster Date: Fri, 11 Dec 2020 23:08:17 +0000 Subject: [PATCH 07/10] Fix accessDenied function call --- src/Exception/OAuthServerException.php | 25 +++++++++---------------- src/Grant/RefreshTokenGrant.php | 8 ++++---- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index ee7859ae3..46db6e716 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -179,18 +179,6 @@ public static function serverError(Throwable $previous = null) ); } - /** - * Invalid refresh token. - * - * @param Throwable $previous - * - * @return static - */ - public static function invalidRefreshToken(Throwable $previous = null) - { - return new static('The refresh token is invalid.', 8, 'invalid_grant', 400, null, $previous); - } - /** * Access denied. * @@ -199,10 +187,10 @@ public static function invalidRefreshToken(Throwable $previous = null) * * @return static */ - public static function accessDenied($redirectUri = null, Throwable $previous = null) + public static function accessDenied($errorMessage, $redirectUri = null, Throwable $previous = null) { return new static( - 'The resource owner or authorization server denied the request.', + $errorMessage, 9, 'access_denied', 401, @@ -214,15 +202,20 @@ public static function accessDenied($redirectUri = null, Throwable $previous = n /** * Invalid grant. * + * @param string $errorMessage + * @param Throwable $previous + * * @return static */ - public static function invalidGrant($message) + public static function invalidGrant($errorMessage, Throwable $previous) { return new static( - $message, + $errorMessage, 10, 'invalid_grant', 400, + null, + $previous ); } diff --git a/src/Grant/RefreshTokenGrant.php b/src/Grant/RefreshTokenGrant.php index 29195715d..91e234e23 100644 --- a/src/Grant/RefreshTokenGrant.php +++ b/src/Grant/RefreshTokenGrant.php @@ -102,21 +102,21 @@ protected function validateOldRefreshToken(ServerRequestInterface $request, $cli try { $refreshToken = $this->decrypt($encryptedRefreshToken); } catch (Exception $e) { - throw OAuthServerException::invalidRefreshToken('Cannot decrypt the refresh token', $e); + throw OAuthServerException::invalidGrant('Cannot decrypt the refresh token', $e); } $refreshTokenData = \json_decode($refreshToken, true); if ($refreshTokenData['client_id'] !== $clientId) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::REFRESH_TOKEN_CLIENT_FAILED, $request)); - throw OAuthServerException::invalidRefreshToken('Token is not linked to client'); + throw OAuthServerException::invalidGrant('Refresh token is not linked to given client'); } if ($refreshTokenData['expire_time'] < \time()) { - throw OAuthServerException::invalidRefreshToken('Token has expired'); + throw OAuthServerException::invalidGrant('Refresh token has expired'); } if ($this->refreshTokenRepository->isRefreshTokenRevoked($refreshTokenData['refresh_token_id']) === true) { - throw OAuthServerException::invalidRefreshToken('Token has been revoked'); + throw OAuthServerException::invalidGrant('Refresh token has been revoked'); } return $refreshTokenData; From 7199019de757cd34da2822883ffc8fc648a32dfc Mon Sep 17 00:00:00 2001 From: sephster Date: Sat, 12 Dec 2020 00:02:30 +0000 Subject: [PATCH 08/10] Fix tests --- src/Exception/OAuthServerException.php | 2 +- src/Grant/PasswordGrant.php | 2 +- tests/Exception/OAuthServerExceptionTest.php | 2 +- tests/Grant/AuthCodeGrantTest.php | 26 +++++++++---------- tests/Grant/ImplicitGrantTest.php | 2 +- tests/Grant/RefreshTokenGrantTest.php | 8 +++--- .../AuthorizationServerMiddlewareTest.php | 4 +-- tests/ResourceServerTest.php | 2 +- .../ResponseTypes/BearerResponseTypeTest.php | 8 +++--- 9 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 46db6e716..65f9a3b85 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -207,7 +207,7 @@ public static function accessDenied($errorMessage, $redirectUri = null, Throwabl * * @return static */ - public static function invalidGrant($errorMessage, Throwable $previous) + public static function invalidGrant($errorMessage, Throwable $previous = null) { return new static( $errorMessage, diff --git a/src/Grant/PasswordGrant.php b/src/Grant/PasswordGrant.php index 4388feb11..9af18c7e1 100644 --- a/src/Grant/PasswordGrant.php +++ b/src/Grant/PasswordGrant.php @@ -107,7 +107,7 @@ protected function validateUser(ServerRequestInterface $request, ClientEntityInt if ($user instanceof UserEntityInterface === false) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::USER_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidGrant(); + throw OAuthServerException::invalidGrant('Could not retrieve the user instance'); } return $user; diff --git a/tests/Exception/OAuthServerExceptionTest.php b/tests/Exception/OAuthServerExceptionTest.php index 024911884..3ddbbcfe0 100644 --- a/tests/Exception/OAuthServerExceptionTest.php +++ b/tests/Exception/OAuthServerExceptionTest.php @@ -92,7 +92,7 @@ public function testHasPrevious() public function testDoesNotHavePrevious() { - $exceptionWithoutPrevious = OAuthServerException::accessDenied(); + $exceptionWithoutPrevious = OAuthServerException::accessDenied('Generic error message'); $this->assertNull($exceptionWithoutPrevious->getPrevious()); } diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index 63706270e..b24645de4 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -1131,7 +1131,7 @@ public function testRespondToAccessTokenRequestWithRefreshTokenInsteadOfAuthCode /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals('Authorization code malformed', $e->getHint()); + $this->assertEquals('The auth_code_id property is missing from the auth code', $e->getMessage()); } } @@ -1184,7 +1184,7 @@ public function testRespondToAccessTokenRequestExpiredCode() /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals($e->getHint(), 'Authorization code has expired'); + $this->assertEquals('Authorization code has expired', $e->getMessage()); } } @@ -1248,8 +1248,8 @@ public function testRespondToAccessTokenRequestRevokedCode() /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals($e->getHint(), 'Authorization code has been revoked'); - $this->assertEquals($e->getErrorType(), 'invalid_grant'); + $this->assertEquals('Authorization code has been revoked', $e->getMessage()); + $this->assertEquals('invalid_grant', $e->getErrorType()); } } @@ -1310,7 +1310,7 @@ public function testRespondToAccessTokenRequestClientMismatch() /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals($e->getHint(), 'Authorization code was not issued to this client'); + $this->assertEquals('Authorization code was not issued to this client', $e->getMessage()); } } @@ -1360,7 +1360,7 @@ public function testRespondToAccessTokenRequestBadCodeEncryption() /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals($e->getHint(), 'Cannot decrypt the authorization code'); + $this->assertEquals('Cannot decrypt the authorization code', $e->getMessage()); } } @@ -1433,7 +1433,7 @@ public function testRespondToAccessTokenRequestBadCodeVerifierPlain() /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals($e->getHint(), 'Failed to verify `code_verifier`.'); + $this->assertEquals('Failed to verify `code_verifier`.', $e->getMessage()); } } @@ -1506,7 +1506,7 @@ public function testRespondToAccessTokenRequestBadCodeVerifierS256() /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals($e->getHint(), 'Code Verifier must follow the specifications of RFC-7636.'); + $this->assertEquals('The code_verifier contains illegal characters', $e->getMessage()); } } @@ -1579,7 +1579,7 @@ public function testRespondToAccessTokenRequestMalformedCodeVerifierS256WithInva /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals($e->getHint(), 'Code Verifier must follow the specifications of RFC-7636.'); + $this->assertEquals('The code_verifier contains illegal characters', $e->getMessage()); } } @@ -1652,7 +1652,7 @@ public function testRespondToAccessTokenRequestMalformedCodeVerifierS256WithInva /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals($e->getHint(), 'Code Verifier must follow the specifications of RFC-7636.'); + $this->assertEquals('The code_verifier contains illegal characters', $e->getMessage()); } } @@ -1724,7 +1724,7 @@ public function testRespondToAccessTokenRequestMissingCodeVerifier() /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals($e->getHint(), 'Check the `code_verifier` parameter'); + $this->assertEquals('The code_verifier is missing', $e->getMessage()); } } @@ -1771,7 +1771,7 @@ public function testAuthCodeRepositoryFailToPersist() $authCodeRepository = $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(); $authCodeRepository->method('getNewAuthCode')->willReturn(new AuthCodeEntity()); - $authCodeRepository->method('persistNewAuthCode')->willThrowException(OAuthServerException::serverError('something bad happened')); + $authCodeRepository->method('persistNewAuthCode')->willThrowException(OAuthServerException::serverError()); $grant = new AuthCodeGrant( $authCodeRepository, @@ -1907,7 +1907,7 @@ public function testRefreshTokenRepositoryFailToPersist() $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity()); - $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willThrowException(OAuthServerException::serverError('something bad happened')); + $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willThrowException(OAuthServerException::serverError()); $grant = new AuthCodeGrant( $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), diff --git a/tests/Grant/ImplicitGrantTest.php b/tests/Grant/ImplicitGrantTest.php index 546450384..0c61dc6a1 100644 --- a/tests/Grant/ImplicitGrantTest.php +++ b/tests/Grant/ImplicitGrantTest.php @@ -310,7 +310,7 @@ public function testAccessTokenRepositoryFailToPersist() /** @var AccessTokenRepositoryInterface|\PHPUnit\Framework\MockObject\MockObject $accessTokenRepositoryMock */ $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); - $accessTokenRepositoryMock->method('persistNewAccessToken')->willThrowException(OAuthServerException::serverError('something bad happened')); + $accessTokenRepositoryMock->method('persistNewAccessToken')->willThrowException(OAuthServerException::serverError()); $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php index d6a0d42e2..fd9322d42 100644 --- a/tests/Grant/RefreshTokenGrantTest.php +++ b/tests/Grant/RefreshTokenGrantTest.php @@ -327,7 +327,7 @@ public function testRespondToRequestInvalidOldToken() $responseType = new StubResponseType(); $this->expectException(\League\OAuth2\Server\Exception\OAuthServerException::class); - $this->expectExceptionCode(8); + $this->expectExceptionCode(10); $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M')); } @@ -375,7 +375,7 @@ public function testRespondToRequestClientMismatch() $responseType = new StubResponseType(); $this->expectException(\League\OAuth2\Server\Exception\OAuthServerException::class); - $this->expectExceptionCode(8); + $this->expectExceptionCode(10); $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M')); } @@ -420,7 +420,7 @@ public function testRespondToRequestExpiredToken() $responseType = new StubResponseType(); $this->expectException(\League\OAuth2\Server\Exception\OAuthServerException::class); - $this->expectExceptionCode(8); + $this->expectExceptionCode(10); $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M')); } @@ -466,7 +466,7 @@ public function testRespondToRequestRevokedToken() $responseType = new StubResponseType(); $this->expectException(\League\OAuth2\Server\Exception\OAuthServerException::class); - $this->expectExceptionCode(8); + $this->expectExceptionCode(10); $grant->respondToAccessTokenRequest($serverRequest, $responseType, new DateInterval('PT5M')); } diff --git a/tests/Middleware/AuthorizationServerMiddlewareTest.php b/tests/Middleware/AuthorizationServerMiddlewareTest.php index 69c8d3791..d59618321 100644 --- a/tests/Middleware/AuthorizationServerMiddlewareTest.php +++ b/tests/Middleware/AuthorizationServerMiddlewareTest.php @@ -110,7 +110,7 @@ public function testOAuthErrorResponseRedirectUri() $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=test&message=test', $response->getHeader('location')[0] ); } @@ -122,7 +122,7 @@ public function testOAuthErrorResponseRedirectUriFragment() $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=test&message=test', $response->getHeader('location')[0] ); } diff --git a/tests/ResourceServerTest.php b/tests/ResourceServerTest.php index 7281070e2..b9ee1f527 100644 --- a/tests/ResourceServerTest.php +++ b/tests/ResourceServerTest.php @@ -21,7 +21,7 @@ public function testValidateAuthenticatedRequest() try { $server->validateAuthenticatedRequest(ServerRequestFactory::fromGlobals()); } catch (OAuthServerException $e) { - $this->assertEquals('Missing "Authorization" header', $e->getHint()); + $this->assertEquals('Missing "Authorization" header', $e->getMessage()); } } } diff --git a/tests/ResponseTypes/BearerResponseTypeTest.php b/tests/ResponseTypes/BearerResponseTypeTest.php index a57820d00..b687bc0c7 100644 --- a/tests/ResponseTypes/BearerResponseTypeTest.php +++ b/tests/ResponseTypes/BearerResponseTypeTest.php @@ -191,7 +191,7 @@ public function testDetermineAccessTokenInHeaderInvalidJWT() } catch (OAuthServerException $e) { $this->assertEquals( 'Access token could not be verified', - $e->getHint() + $e->getMessage() ); } } @@ -236,7 +236,7 @@ public function testDetermineAccessTokenInHeaderRevokedToken() } catch (OAuthServerException $e) { $this->assertEquals( 'Access token has been revoked', - $e->getHint() + $e->getMessage() ); } } @@ -259,7 +259,7 @@ public function testDetermineAccessTokenInHeaderInvalidToken() } catch (OAuthServerException $e) { $this->assertEquals( 'The JWT string must have two dots', - $e->getHint() + $e->getMessage() ); } } @@ -282,7 +282,7 @@ public function testDetermineMissingBearerInHeader() } catch (OAuthServerException $e) { $this->assertEquals( 'Error while decoding from JSON', - $e->getHint() + $e->getMessage() ); } } From b81297dd56df61040ab113bffb41a6ff32cc6b97 Mon Sep 17 00:00:00 2001 From: sephster Date: Sat, 12 Dec 2020 00:12:04 +0000 Subject: [PATCH 09/10] StyleCI fixes --- src/Exception/OAuthServerException.php | 4 ++-- src/Grant/PasswordGrant.php | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 65f9a3b85..2d1efc6bc 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -116,8 +116,8 @@ public static function unsupportedGrantType() /** * Invalid request error. * - * @param string $errorMessage The error message - * @param Throwable $previous Previous exception + * @param string $errorMessage The error message + * @param Throwable $previous Previous exception * * @return static */ diff --git a/src/Grant/PasswordGrant.php b/src/Grant/PasswordGrant.php index 9af18c7e1..6cefab563 100644 --- a/src/Grant/PasswordGrant.php +++ b/src/Grant/PasswordGrant.php @@ -57,7 +57,8 @@ public function respondToAccessTokenRequest( $scopes, $this->getIdentifier(), $client, - $user->getIdentifier()); + $user->getIdentifier() + ); // Issue and persist new access token $accessToken = $this->issueAccessToken($accessTokenTTL, $client, $user->getIdentifier(), $finalizedScopes); From 7536240843dbde02ef645fa290ab163766ba38ec Mon Sep 17 00:00:00 2001 From: sephster Date: Wed, 27 Jan 2021 11:06:58 +0000 Subject: [PATCH 10/10] Begin changing exceptions --- src/Exception/InvalidRequestException.php | 95 +++++++++++++++++++++++ src/Grant/AuthCodeGrant.php | 34 ++++---- 2 files changed, 108 insertions(+), 21 deletions(-) create mode 100644 src/Exception/InvalidRequestException.php diff --git a/src/Exception/InvalidRequestException.php b/src/Exception/InvalidRequestException.php new file mode 100644 index 000000000..d411d9add --- /dev/null +++ b/src/Exception/InvalidRequestException.php @@ -0,0 +1,95 @@ +getRequestParameter('code', $request, null); if ($encryptedAuthCode === null) { - throw OAuthServerException::invalidRequest('The auth code is missing'); + throw InvalidRequestException::missingAuthCode(); } try { @@ -123,7 +124,7 @@ public function respondToAccessTokenRequest( $authCodePayload->auth_code_id ); } catch (LogicException $e) { - throw OAuthServerException::invalidRequest('Cannot decrypt the authorization code', $e); + throw InvalidRequestException::authCodeDecryptError($e); } // Validate code challenge @@ -131,13 +132,13 @@ public function respondToAccessTokenRequest( $codeVerifier = $this->getRequestParameter('code_verifier', $request, null); if ($codeVerifier === null) { - throw OAuthServerException::invalidRequest('The code_verifier is missing'); + throw InvalidRequestException::missingCodeVerifier(); } // Validate code_verifier according to RFC-7636 // @see: https://tools.ietf.org/html/rfc7636#section-4.1 if (\preg_match('/^[A-Za-z0-9-._~]{43,128}$/', $codeVerifier) !== 1) { - throw OAuthServerException::invalidRequest('The code_verifier contains illegal characters'); + throw InvalidRequestException::illegalCharacters('code_verifier'); } if (\property_exists($authCodePayload, 'code_challenge_method')) { @@ -190,11 +191,11 @@ private function validateAuthorizationCode( ServerRequestInterface $request ) { if (!\property_exists($authCodePayload, 'auth_code_id')) { - throw OAuthServerException::invalidRequest('The auth_code_id property is missing from the auth code'); + throw InvalidRequestException::missingAuthCodeId(); } if (\time() > $authCodePayload->expire_time) { - throw OAuthServerException::invalidRequest('Authorization code has expired'); + throw InvalidRequestException::authCodeExpired(); } if ($this->authCodeRepository->isAuthCodeRevoked($authCodePayload->auth_code_id) === true) { @@ -202,17 +203,17 @@ private function validateAuthorizationCode( } if ($authCodePayload->client_id !== $client->getIdentifier()) { - throw OAuthServerException::invalidRequest('Authorization code was not issued to this client'); + throw InvalidRequestException::authCodeNotClients(); } // The redirect URI is required in this request $redirectUri = $this->getRequestParameter('redirect_uri', $request, null); if (empty($authCodePayload->redirect_uri) === false && $redirectUri === null) { - throw OAuthServerException::invalidRequest('Missing redirect_uri'); + throw InvalidRequestException::missingParameter('redirect_uri'); } if ($authCodePayload->redirect_uri !== $redirectUri) { - throw OAuthServerException::invalidRequest('Invalid redirect URI'); + throw InvalidRequestException::invalidRedirectUri(); } } @@ -250,7 +251,7 @@ public function validateAuthorizationRequest(ServerRequestInterface $request) ); if ($clientId === null) { - throw OAuthServerException::invalidRequest('client_id is not set'); + throw InvalidRequestException::missingParameter('client_id'); } $client = $this->getClientEntityOrFail($clientId, $request); @@ -293,22 +294,13 @@ public function validateAuthorizationRequest(ServerRequestInterface $request) $codeChallengeMethod = $this->getQueryStringParameter('code_challenge_method', $request, 'plain'); if (\array_key_exists($codeChallengeMethod, $this->codeChallengeVerifiers) === false) { - throw OAuthServerException::invalidRequest( - 'Code challenge method must be one of ' . \implode(', ', \array_map( - function ($method) { - return '`' . $method . '`'; - }, - \array_keys($this->codeChallengeVerifiers) - )) - ); + throw InvalidRequestException::invalidCodeChallengeMethod(\array_keys($this->codeChallengeVerifiers)); } // Validate code_challenge according to RFC-7636 // @see: https://tools.ietf.org/html/rfc7636#section-4.2 if (\preg_match('/^[A-Za-z0-9-._~]{43,128}$/', $codeChallenge) !== 1) { - throw OAuthServerException::invalidRequest( - 'Code challenge contains illegal characters' - ); + throw InvalidRequestException::illegalCharacters('code_challenge'); } $authorizationRequest->setCodeChallenge($codeChallenge);