From 3b983ad0b4786206e103d9337c3bf3b186d2e81c Mon Sep 17 00:00:00 2001 From: Marc Ypes Date: Mon, 12 Nov 2018 13:47:17 +0100 Subject: [PATCH 1/5] Include previous exception in catch and throw --- .../BearerTokenValidator.php | 6 ++-- src/CryptTrait.php | 4 +-- src/Exception/OAuthServerException.php | 29 ++++++++++++------- src/Grant/AbstractGrant.php | 6 ++-- src/Grant/AuthCodeGrant.php | 2 +- src/Grant/RefreshTokenGrant.php | 2 +- 6 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/AuthorizationValidators/BearerTokenValidator.php b/src/AuthorizationValidators/BearerTokenValidator.php index 2efa3c8e..80e69fdc 100644 --- a/src/AuthorizationValidators/BearerTokenValidator.php +++ b/src/AuthorizationValidators/BearerTokenValidator.php @@ -70,7 +70,7 @@ class BearerTokenValidator implements AuthorizationValidatorInterface throw OAuthServerException::accessDenied('Access token could not be verified'); } } catch (\BadMethodCallException $exception) { - throw OAuthServerException::accessDenied('Access token is not signed'); + throw OAuthServerException::accessDenied('Access token is not signed', null, $exception); } // Ensure access token hasn't expired @@ -94,10 +94,10 @@ class BearerTokenValidator implements AuthorizationValidatorInterface ->withAttribute('oauth_scopes', $token->getClaim('scopes')); } catch (\InvalidArgumentException $exception) { // JWT couldn't be parsed so return the request as is - throw OAuthServerException::accessDenied($exception->getMessage()); + throw OAuthServerException::accessDenied($exception->getMessage(), null, $exception); } catch (\RuntimeException $exception) { //JWR couldn't be parsed so return the request as is - throw OAuthServerException::accessDenied('Error while decoding to JSON'); + throw OAuthServerException::accessDenied('Error while decoding to JSON', null, $exception); } } } diff --git a/src/CryptTrait.php b/src/CryptTrait.php index 672c7e2e..db6c8d01 100644 --- a/src/CryptTrait.php +++ b/src/CryptTrait.php @@ -39,7 +39,7 @@ trait CryptTrait return Crypto::encryptWithPassword($unencryptedData, $this->encryptionKey); } catch (\Exception $e) { - throw new \LogicException($e->getMessage()); + throw new \LogicException($e->getMessage(), null, $e); } } @@ -61,7 +61,7 @@ trait CryptTrait return Crypto::decryptWithPassword($encryptedData, $this->encryptionKey); } catch (\Exception $e) { - throw new \LogicException($e->getMessage()); + throw new \LogicException($e->getMessage(), null, $e); } } diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 2c7bc28b..6bcfa3be 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -47,10 +47,11 @@ class OAuthServerException extends \Exception * @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) + public function __construct($message, $code, $errorType, $httpStatusCode = 400, $hint = null, $redirectUri = null, \Throwable $previous = null) { - parent::__construct($message, $code); + parent::__construct($message, $code, $previous); $this->httpStatusCode = $httpStatusCode; $this->errorType = $errorType; $this->hint = $hint; @@ -102,16 +103,17 @@ class OAuthServerException extends \Exception * * @param string $parameter The invalid parameter * @param null|string $hint + * @param \Throwable $previous Previous exception * * @return static */ - public static function invalidRequest($parameter, $hint = null) + public static function invalidRequest($parameter, $hint = null, \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); + return new static($errorMessage, 3, 'invalid_request', 400, $hint, null, $previous); } /** @@ -163,20 +165,22 @@ class OAuthServerException extends \Exception /** * Server error. * - * @param string $hint + * @param string $hint + * @param \Throwable $previous * * @return static * * @codeCoverageIgnore */ - public static function serverError($hint) + public static function serverError($hint, \Throwable $previous = null) { return new static( 'The authorization server encountered an unexpected condition which prevented it from fulfilling' . ' the request: ' . $hint, 7, 'server_error', - 500 + 500, + $previous ); } @@ -184,12 +188,13 @@ class OAuthServerException extends \Exception * Invalid refresh token. * * @param null|string $hint + * @param \Throwable $previous * * @return static */ - public static function invalidRefreshToken($hint = null) + public static function invalidRefreshToken($hint = null, \Throwable $previous = null) { - return new static('The refresh token is invalid.', 8, 'invalid_request', 401, $hint); + return new static('The refresh token is invalid.', 8, 'invalid_request', 401, $hint, null, $previous); } /** @@ -197,10 +202,11 @@ class OAuthServerException extends \Exception * * @param null|string $hint * @param null|string $redirectUri + * @param \Throwable $previous * * @return static */ - public static function accessDenied($hint = null, $redirectUri = null) + public static function accessDenied($hint = null, $redirectUri = null, \Throwable $previous = null) { return new static( 'The resource owner or authorization server denied the request.', @@ -208,7 +214,8 @@ class OAuthServerException extends \Exception 'access_denied', 401, $hint, - $redirectUri + $redirectUri, + $previous ); } diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 9b5486c3..76c2f8e2 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -505,12 +505,12 @@ abstract class AbstractGrant implements GrantTypeInterface return bin2hex(random_bytes($length)); // @codeCoverageIgnoreStart } catch (\TypeError $e) { - throw OAuthServerException::serverError('An unexpected error has occurred'); + throw OAuthServerException::serverError('An unexpected error has occurred', $e); } catch (\Error $e) { - throw OAuthServerException::serverError('An unexpected error has occurred'); + throw OAuthServerException::serverError('An unexpected error has occurred', $e); } catch (\Exception $e) { // If you get this message, the CSPRNG failed hard. - throw OAuthServerException::serverError('Could not generate a random string'); + throw OAuthServerException::serverError('Could not generate a random string', $e); } // @codeCoverageIgnoreEnd } diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index db2dee47..3d963d02 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -91,7 +91,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant $authCodePayload->user_id ); } catch (\LogicException $e) { - throw OAuthServerException::invalidRequest('code', 'Cannot decrypt the authorization code'); + throw OAuthServerException::invalidRequest('code', 'Cannot decrypt the authorization code', $e); } // Validate code challenge diff --git a/src/Grant/RefreshTokenGrant.php b/src/Grant/RefreshTokenGrant.php index 519954be..d48a9058 100644 --- a/src/Grant/RefreshTokenGrant.php +++ b/src/Grant/RefreshTokenGrant.php @@ -95,7 +95,7 @@ class RefreshTokenGrant extends AbstractGrant try { $refreshToken = $this->decrypt($encryptedRefreshToken); } catch (\Exception $e) { - throw OAuthServerException::invalidRefreshToken('Cannot decrypt the refresh token'); + throw OAuthServerException::invalidRefreshToken('Cannot decrypt the refresh token', $e); } $refreshTokenData = json_decode($refreshToken, true); From 9542af627eed680df5bf1857758d898ee45ff7ee Mon Sep 17 00:00:00 2001 From: sephster Date: Mon, 12 Nov 2018 19:57:35 +0000 Subject: [PATCH 2/5] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91ab6b30..7362fc74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - Added a ScopeTrait to provide an implementation for jsonSerialize (PR #952) +- Ability to nest exceptions (PR #965) ### Fixed - Fix issue where AuthorizationServer is not stateless as ResponseType could store state of a previous request (PR #960) From 58689969617ce65d537c855fda2a9dde038656ce Mon Sep 17 00:00:00 2001 From: Marc Ypes Date: Tue, 13 Nov 2018 00:25:22 +0100 Subject: [PATCH 3/5] Add test for previous exceptions --- tests/Exception/OAuthServerExceptionTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/Exception/OAuthServerExceptionTest.php b/tests/Exception/OAuthServerExceptionTest.php index 976362e3..201fb5dd 100644 --- a/tests/Exception/OAuthServerExceptionTest.php +++ b/tests/Exception/OAuthServerExceptionTest.php @@ -2,6 +2,7 @@ namespace LeagueTests\Exception; +use Exception; use League\OAuth2\Server\Exception\OAuthServerException; use PHPUnit\Framework\TestCase; @@ -20,4 +21,19 @@ class OAuthServerExceptionTest extends TestCase $this->assertFalse($exceptionWithoutRedirect->hasRedirect()); } + + public function testHasPrevious() + { + $previous = new Exception('This is the previous'); + $exceptionWithPrevious = OAuthServerException::accessDenied(null, null, $previous); + + $this->assertSame('This is the previous', $exceptionWithPrevious->getPrevious()->getMessage()); + } + + public function testDoesNotHavePrevious() + { + $exceptionWithoutPrevious = OAuthServerException::accessDenied(); + + $this->assertNull($exceptionWithoutPrevious->getPrevious()); + } } From f6c1070ccca1d6b0f31dc43b078bb120d775809b Mon Sep 17 00:00:00 2001 From: sephster Date: Tue, 13 Nov 2018 12:32:52 +0000 Subject: [PATCH 4/5] Add use Throwable --- src/Exception/OAuthServerException.php | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index b4dc8f14..67de1293 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -11,6 +11,7 @@ namespace League\OAuth2\Server\Exception; use Exception; use Psr\Http\Message\ResponseInterface; +use Throwable; class OAuthServerException extends Exception { @@ -48,9 +49,9 @@ class OAuthServerException extends Exception * @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 + * @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, $hint = null, $redirectUri = null, Throwable $previous = null) { parent::__construct($message, $code, $previous); $this->httpStatusCode = $httpStatusCode; @@ -104,11 +105,11 @@ class OAuthServerException extends Exception * * @param string $parameter The invalid parameter * @param null|string $hint - * @param \Throwable $previous Previous exception + * @param Throwable $previous Previous exception * * @return static */ - public static function invalidRequest($parameter, $hint = null, \Throwable $previous = null) + public static function invalidRequest($parameter, $hint = null, 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.'; @@ -166,14 +167,14 @@ class OAuthServerException extends Exception /** * Server error. * - * @param string $hint - * @param \Throwable $previous + * @param string $hint + * @param Throwable $previous * * @return static * * @codeCoverageIgnore */ - public static function serverError($hint, \Throwable $previous = null) + public static function serverError($hint, Throwable $previous = null) { return new static( 'The authorization server encountered an unexpected condition which prevented it from fulfilling' @@ -189,11 +190,11 @@ class OAuthServerException extends Exception * Invalid refresh token. * * @param null|string $hint - * @param \Throwable $previous + * @param Throwable $previous * * @return static */ - public static function invalidRefreshToken($hint = null, \Throwable $previous = null) + public static function invalidRefreshToken($hint = null, Throwable $previous = null) { return new static('The refresh token is invalid.', 8, 'invalid_request', 401, $hint, null, $previous); } @@ -203,11 +204,11 @@ class OAuthServerException extends Exception * * @param null|string $hint * @param null|string $redirectUri - * @param \Throwable $previous + * @param Throwable $previous * * @return static */ - public static function accessDenied($hint = null, $redirectUri = null, \Throwable $previous = null) + public static function accessDenied($hint = null, $redirectUri = null, Throwable $previous = null) { return new static( 'The resource owner or authorization server denied the request.', From 7982275757956f508ac2b21dbab5c9bc4afec9f9 Mon Sep 17 00:00:00 2001 From: sephster Date: Tue, 13 Nov 2018 12:34:16 +0000 Subject: [PATCH 5/5] Fix docblock alignment --- 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 67de1293..ed5f9c78 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -105,7 +105,7 @@ class OAuthServerException extends Exception * * @param string $parameter The invalid parameter * @param null|string $hint - * @param Throwable $previous Previous exception + * @param Throwable $previous Previous exception * * @return static */ @@ -204,7 +204,7 @@ class OAuthServerException extends Exception * * @param null|string $hint * @param null|string $redirectUri - * @param Throwable $previous + * @param Throwable $previous * * @return static */