From 6f2af0d64c5ed7e94b5e396e5776a75b5c8f4808 Mon Sep 17 00:00:00 2001 From: Patrick Rodacker Date: Mon, 20 Apr 2020 21:27:01 +0200 Subject: [PATCH 1/9] adds a public getter for the redirectUri property --- src/Exception/OAuthServerException.php | 8 ++++++++ tests/Exception/OAuthServerExceptionTest.php | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 6c04936bd..e1bd382d1 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -376,4 +376,12 @@ public function getHint() { return $this->hint; } + + /** + * @return null|string + */ + public function getRedirectUri() + { + return $this->redirectUri; + } } diff --git a/tests/Exception/OAuthServerExceptionTest.php b/tests/Exception/OAuthServerExceptionTest.php index 0d8c50118..88a88fb5b 100644 --- a/tests/Exception/OAuthServerExceptionTest.php +++ b/tests/Exception/OAuthServerExceptionTest.php @@ -96,4 +96,12 @@ public function testDoesNotHavePrevious() $this->assertNull($exceptionWithoutPrevious->getPrevious()); } + + public function testGetRedirectUri(): void + { + $redirectUri = 'https://bar.test'; + $exception = OAuthServerException::invalidScope('foo', $redirectUri); + + $this->assertSame($redirectUri, $exception->getRedirectUri()); + } } From 9f6f27bd88f83430a033bbf48096414c837b3235 Mon Sep 17 00:00:00 2001 From: Patrick Rodacker Date: Mon, 20 Apr 2020 21:27:10 +0200 Subject: [PATCH 2/9] adds an ExceptionResponseHandler --- src/Exception/ExceptionResponseHandler.php | 52 ++++++++ .../ExceptionResponseHandlerInterface.php | 25 ++++ .../ExceptionResponseHandlerTest.php | 117 ++++++++++++++++++ 3 files changed, 194 insertions(+) create mode 100644 src/Exception/ExceptionResponseHandler.php create mode 100644 src/Exception/ExceptionResponseHandlerInterface.php create mode 100644 tests/Exception/ExceptionResponseHandlerTest.php diff --git a/src/Exception/ExceptionResponseHandler.php b/src/Exception/ExceptionResponseHandler.php new file mode 100644 index 000000000..a2dfe54e3 --- /dev/null +++ b/src/Exception/ExceptionResponseHandler.php @@ -0,0 +1,52 @@ +getHttpHeaders(); + + $payload = $exception->getPayload(); + + $redirectUri = $exception->getRedirectUri(); + if ($redirectUri !== null) { + if ($useFragment === true) { + $redirectUri .= (\strstr($redirectUri, '#') === false) ? '#' : '&'; + } else { + $redirectUri .= (\strstr($redirectUri, '?') === false) ? '?' : '&'; + } + + return $response->withStatus(302)->withHeader('Location', $redirectUri . \http_build_query($payload)); + } + + foreach ($headers as $header => $content) { + $response = $response->withHeader($header, $content); + } + + $responseBody = \json_encode($payload, $jsonOptions) ?: 'JSON encoding of payload failed'; + + $response->getBody()->write($responseBody); + + return $response->withStatus($exception->getHttpStatusCode()); + } +} diff --git a/src/Exception/ExceptionResponseHandlerInterface.php b/src/Exception/ExceptionResponseHandlerInterface.php new file mode 100644 index 000000000..160a837d1 --- /dev/null +++ b/src/Exception/ExceptionResponseHandlerInterface.php @@ -0,0 +1,25 @@ +handler = new ExceptionResponseHandler(); + } + + public function testHandlerImplementsContract(): void + { + $this->assertInstanceOf(ExceptionResponseHandlerInterface::class, $this->handler); + } + + public function testGenerateRedirectResponseStatusCodeForInvalidScopeExceptionWithRedirectUri() + { + $exception = OAuthServerException::invalidScope('foo', 'https://bar.test'); + $response = $this->handler->generateHttpResponse($exception, new Response()); + $this->assertSame(302, $response->getStatusCode()); + } + + public function testGenerateRedirectResponseLocationHeaderForInvalidScopeExceptionWithRedirectUri() + { + $redirectUri = 'https://bar.test'; + $exception = OAuthServerException::invalidScope('foo', $redirectUri); + $response = $this->handler->generateHttpResponse($exception, new Response()); + + $payload = $exception->getPayload(); + $query = \http_build_query($payload); + $expectedRedirectUri = $redirectUri . '?' . $query; + $header = $response->getHeader('Location')[0]; + $this->assertSame($expectedRedirectUri, $header); + } + + public function testGenerateRedirectResponseLocationHeaderForInvalidScopeExceptionWithRedirectUriUsingFragment() + { + $redirectUri = 'https://bar.test'; + $exception = OAuthServerException::invalidScope('foo', $redirectUri); + $response = $this->handler->generateHttpResponse($exception, new Response(), true); + + $payload = $exception->getPayload(); + $query = \http_build_query($payload); + $expectedRedirectUri = $redirectUri . '#' . $query; + $header = $response->getHeader('Location')[0]; + $this->assertSame($expectedRedirectUri, $header); + } + + public function testInvalidClientExceptionSetsAuthenticateHeader() + { + $serverRequest = (new ServerRequest()) + ->withParsedBody([ + 'client_id' => 'foo', + ]) + ->withAddedHeader('Authorization', 'Basic fakeauthdetails'); + + try { + $this->issueInvalidClientException($serverRequest); + } catch (OAuthServerException $e) { + $response = $this->handler->generateHttpResponse($e, new Response()); + + $this->assertTrue($response->hasHeader('WWW-Authenticate')); + } + } + + public function testInvalidClientExceptionOmitsAuthenticateHeader() + { + $serverRequest = (new ServerRequest()) + ->withParsedBody([ + 'client_id' => 'foo', + ]); + + try { + $this->issueInvalidClientException($serverRequest); + } catch (OAuthServerException $e) { + $response = $this->handler->generateHttpResponse($e, new Response()); + + $this->assertFalse($response->hasHeader('WWW-Authenticate')); + } + } + + /** + * Issue an invalid client exception + * + * @throws OAuthServerException + */ + private function issueInvalidClientException($serverRequest) + { + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('validateClient')->willReturn(false); + + $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); + $grantMock->setClientRepository($clientRepositoryMock); + + $abstractGrantReflection = new \ReflectionClass($grantMock); + + $validateClientMethod = $abstractGrantReflection->getMethod('validateClient'); + $validateClientMethod->setAccessible(true); + + $validateClientMethod->invoke($grantMock, $serverRequest); + } +} From 06fd9f0114b8dd5628765dd8d8a864865e37f1af Mon Sep 17 00:00:00 2001 From: Patrick Rodacker Date: Mon, 20 Apr 2020 21:46:06 +0200 Subject: [PATCH 3/9] refactor by extracting generation of redirect response to method --- src/Exception/ExceptionResponseHandler.php | 36 ++++++++++++++----- .../ExceptionResponseHandlerInterface.php | 2 +- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Exception/ExceptionResponseHandler.php b/src/Exception/ExceptionResponseHandler.php index a2dfe54e3..5fd699e9d 100644 --- a/src/Exception/ExceptionResponseHandler.php +++ b/src/Exception/ExceptionResponseHandler.php @@ -9,7 +9,7 @@ class ExceptionResponseHandler implements ExceptionResponseHandlerInterface { /** - * Generate a HTTP response. + * Generate a HTTP response from am OAuthServerException * * @param OAuthServerException $exception * @param ResponseInterface $response @@ -30,13 +30,7 @@ public function generateHttpResponse( $redirectUri = $exception->getRedirectUri(); if ($redirectUri !== null) { - if ($useFragment === true) { - $redirectUri .= (\strstr($redirectUri, '#') === false) ? '#' : '&'; - } else { - $redirectUri .= (\strstr($redirectUri, '?') === false) ? '?' : '&'; - } - - return $response->withStatus(302)->withHeader('Location', $redirectUri . \http_build_query($payload)); + return $this->generateRedirectResponse($redirectUri, $response, $payload, $useFragment); } foreach ($headers as $header => $content) { @@ -49,4 +43,30 @@ public function generateHttpResponse( return $response->withStatus($exception->getHttpStatusCode()); } + + /** + * Generate a HTTP response from am OAuthServerException + * + * @param string $redirectUri + * @param ResponseInterface $response + * @param string[] $payload + * @param bool $useFragment + * + * @return ResponseInterface + */ + protected function generateRedirectResponse( + string $redirectUri, + ResponseInterface $response, + $payload, + $useFragment + ): ResponseInterface { + if ($useFragment === true) { + $querySeparator = '#'; + } else { + $querySeparator = '?'; + } + $redirectUri .= (\strstr($redirectUri, '?') === false) ? $querySeparator : '&'; + + return $response->withStatus(302)->withHeader('Location', $redirectUri . \http_build_query($payload)); + } } diff --git a/src/Exception/ExceptionResponseHandlerInterface.php b/src/Exception/ExceptionResponseHandlerInterface.php index 160a837d1..0b0ccb594 100644 --- a/src/Exception/ExceptionResponseHandlerInterface.php +++ b/src/Exception/ExceptionResponseHandlerInterface.php @@ -7,7 +7,7 @@ interface ExceptionResponseHandlerInterface { /** - * Generate a HTTP response from a OAuthServerException + * Generate a HTTP response from am OAuthServerException * * @param OAuthServerException $exception * @param ResponseInterface $response From 34df0db7085cb949bf64c458a65f41f3955074ee Mon Sep 17 00:00:00 2001 From: Patrick Rodacker Date: Mon, 20 Apr 2020 22:03:20 +0200 Subject: [PATCH 4/9] adds proxy method for exception response handler to generate http response from an OAuthServerException --- src/AuthorizationServer.php | 37 +++++++++++++++++++++++++------ tests/AuthorizationServerTest.php | 32 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/AuthorizationServer.php b/src/AuthorizationServer.php index ac3e4afd0..3121db1bb 100644 --- a/src/AuthorizationServer.php +++ b/src/AuthorizationServer.php @@ -13,6 +13,8 @@ use Defuse\Crypto\Key; use League\Event\EmitterAwareInterface; use League\Event\EmitterAwareTrait; +use League\OAuth2\Server\Exception\ExceptionResponseHandler; +use League\OAuth2\Server\Exception\ExceptionResponseHandlerInterface; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Grant\GrantTypeInterface; use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; @@ -79,15 +81,21 @@ class AuthorizationServer implements EmitterAwareInterface */ private $defaultScope = ''; + /** + * @var ExceptionResponseHandlerInterface + */ + private $exceptionResponseHandler; + /** * New server instance. * - * @param ClientRepositoryInterface $clientRepository - * @param AccessTokenRepositoryInterface $accessTokenRepository - * @param ScopeRepositoryInterface $scopeRepository - * @param CryptKeyInterface|string $privateKey - * @param string|Key $encryptionKey - * @param null|ResponseTypeInterface $responseType + * @param ClientRepositoryInterface $clientRepository + * @param AccessTokenRepositoryInterface $accessTokenRepository + * @param ScopeRepositoryInterface $scopeRepository + * @param CryptKeyInterface|string $privateKey + * @param string|Key $encryptionKey + * @param null|ResponseTypeInterface $responseType + * @param null|ExceptionResponseHandlerInterface $exceptionResponseHandler */ public function __construct( ClientRepositoryInterface $clientRepository, @@ -95,7 +103,8 @@ public function __construct( ScopeRepositoryInterface $scopeRepository, $privateKey, $encryptionKey, - ResponseTypeInterface $responseType = null + ResponseTypeInterface $responseType = null, + ExceptionResponseHandlerInterface $exceptionResponseHandler = null ) { $this->clientRepository = $clientRepository; $this->accessTokenRepository = $accessTokenRepository; @@ -115,6 +124,11 @@ public function __construct( } $this->responseType = $responseType; + + if ($exceptionResponseHandler === null) { + $exceptionResponseHandler = new ExceptionResponseHandler(); + } + $this->exceptionResponseHandler = $exceptionResponseHandler; } /** @@ -235,4 +249,13 @@ public function setDefaultScope($defaultScope) { $this->defaultScope = $defaultScope; } + + public function generateHttpResponse( + OAuthServerException $exception, + ResponseInterface $response, + $useFragment = false, + $jsonOptions = 0 + ) { + return $this->exceptionResponseHandler->generateHttpResponse($exception, $response, $useFragment, $jsonOptions); + } } diff --git a/tests/AuthorizationServerTest.php b/tests/AuthorizationServerTest.php index 25725aca6..e812cf82f 100644 --- a/tests/AuthorizationServerTest.php +++ b/tests/AuthorizationServerTest.php @@ -8,6 +8,8 @@ use Laminas\Diactoros\ServerRequestFactory; use League\OAuth2\Server\AuthorizationServer; use League\OAuth2\Server\CryptKeyInterface; +use League\OAuth2\Server\Exception\ExceptionResponseHandler; +use League\OAuth2\Server\Exception\ExceptionResponseHandlerInterface; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Grant\AuthCodeGrant; use League\OAuth2\Server\Grant\ClientCredentialsGrant; @@ -345,4 +347,34 @@ public function testValidateAuthorizationRequestUnregistered() $server->validateAuthorizationRequest($request); } + + public function testGenerateHttpResponseFromExceptionIsProxiesExceptionResponseHandlerMethod() + { + $exception = OAuthServerException::invalidScope('foo', 'https://bar.test'); + $existingResponse = new Response(); + $useFragment = true; + $jsonOptions = 0; + + $responseMock = $this->createMock(ResponseInterface::class); + $exceptionResponseHandlerMock = $this->createMock(ExceptionResponseHandlerInterface::class); + $exceptionResponseHandlerMock + ->expects($this->once()) + ->method('generateHttpResponse') + ->with($exception, $existingResponse, $useFragment, $jsonOptions) + ->willReturn($responseMock); + + $server = new AuthorizationServer( + $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(), + $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(), + $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(), + 'file://' . __DIR__ . '/Stubs/private.key', + \base64_encode(\random_bytes(36)), + new StubResponseType(), + $exceptionResponseHandlerMock + ); + + $response = $server->generateHttpResponse($exception, $existingResponse, $useFragment, $jsonOptions); + + $this->assertSame($responseMock, $response); + } } From 899e7ad023e5926610365e3062221d02d8ccee77 Mon Sep 17 00:00:00 2001 From: Patrick Rodacker Date: Mon, 20 Apr 2020 22:23:46 +0200 Subject: [PATCH 5/9] extract the generateHttpResponse to a trait and use it in the ResourceServer as well --- src/AuthorizationServer.php | 13 ++------ .../ExceptionResponseHandlerTrait.php | 27 ++++++++++++++++ src/ResourceServer.php | 25 ++++++++++++--- tests/ResourceServerTest.php | 31 +++++++++++++++++++ 4 files changed, 82 insertions(+), 14 deletions(-) create mode 100644 src/Exception/ExceptionResponseHandlerTrait.php diff --git a/src/AuthorizationServer.php b/src/AuthorizationServer.php index 3121db1bb..129745244 100644 --- a/src/AuthorizationServer.php +++ b/src/AuthorizationServer.php @@ -15,6 +15,7 @@ use League\Event\EmitterAwareTrait; use League\OAuth2\Server\Exception\ExceptionResponseHandler; use League\OAuth2\Server\Exception\ExceptionResponseHandlerInterface; +use League\OAuth2\Server\Exception\ExceptionResponseHandlerTrait; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Grant\GrantTypeInterface; use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; @@ -29,7 +30,8 @@ class AuthorizationServer implements EmitterAwareInterface { - use EmitterAwareTrait; + use EmitterAwareTrait, + ExceptionResponseHandlerTrait; /** * @var GrantTypeInterface[] @@ -249,13 +251,4 @@ public function setDefaultScope($defaultScope) { $this->defaultScope = $defaultScope; } - - public function generateHttpResponse( - OAuthServerException $exception, - ResponseInterface $response, - $useFragment = false, - $jsonOptions = 0 - ) { - return $this->exceptionResponseHandler->generateHttpResponse($exception, $response, $useFragment, $jsonOptions); - } } diff --git a/src/Exception/ExceptionResponseHandlerTrait.php b/src/Exception/ExceptionResponseHandlerTrait.php new file mode 100644 index 000000000..ac3a28442 --- /dev/null +++ b/src/Exception/ExceptionResponseHandlerTrait.php @@ -0,0 +1,27 @@ +exceptionResponseHandler->generateHttpResponse($exception, $response, $useFragment, $jsonOptions); + } +} diff --git a/src/ResourceServer.php b/src/ResourceServer.php index 92a72763e..407c712d8 100644 --- a/src/ResourceServer.php +++ b/src/ResourceServer.php @@ -11,12 +11,17 @@ use League\OAuth2\Server\AuthorizationValidators\AuthorizationValidatorInterface; use League\OAuth2\Server\AuthorizationValidators\BearerTokenValidator; +use League\OAuth2\Server\Exception\ExceptionResponseHandler; +use League\OAuth2\Server\Exception\ExceptionResponseHandlerInterface; +use League\OAuth2\Server\Exception\ExceptionResponseHandlerTrait; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; use Psr\Http\Message\ServerRequestInterface; class ResourceServer { + use ExceptionResponseHandlerTrait; + /** * @var AccessTokenRepositoryInterface */ @@ -32,17 +37,24 @@ class ResourceServer */ private $authorizationValidator; + /** + * @var ExceptionResponseHandlerInterface + */ + private $exceptionResponseHandler; + /** * New server instance. * - * @param AccessTokenRepositoryInterface $accessTokenRepository - * @param CryptKeyInterface|string $publicKey - * @param null|AuthorizationValidatorInterface $authorizationValidator + * @param AccessTokenRepositoryInterface $accessTokenRepository + * @param CryptKeyInterface|string $publicKey + * @param null|AuthorizationValidatorInterface $authorizationValidator + * @param null|ExceptionResponseHandlerInterface $exceptionResponseHandler */ public function __construct( AccessTokenRepositoryInterface $accessTokenRepository, $publicKey, - AuthorizationValidatorInterface $authorizationValidator = null + AuthorizationValidatorInterface $authorizationValidator = null, + ExceptionResponseHandlerInterface $exceptionResponseHandler = null ) { $this->accessTokenRepository = $accessTokenRepository; @@ -52,6 +64,11 @@ public function __construct( $this->publicKey = $publicKey; $this->authorizationValidator = $authorizationValidator; + + if ($exceptionResponseHandler === null) { + $exceptionResponseHandler = new ExceptionResponseHandler(); + } + $this->exceptionResponseHandler = $exceptionResponseHandler; } /** diff --git a/tests/ResourceServerTest.php b/tests/ResourceServerTest.php index 7281070e2..34ae43cd3 100644 --- a/tests/ResourceServerTest.php +++ b/tests/ResourceServerTest.php @@ -3,11 +3,15 @@ namespace LeagueTests; +use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequestFactory; +use League\OAuth2\Server\AuthorizationValidators\AuthorizationValidatorInterface; +use League\OAuth2\Server\Exception\ExceptionResponseHandlerInterface; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; use League\OAuth2\Server\ResourceServer; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ResponseInterface; class ResourceServerTest extends TestCase { @@ -24,4 +28,31 @@ public function testValidateAuthenticatedRequest() $this->assertEquals('Missing "Authorization" header', $e->getHint()); } } + + public function testGenerateHttpResponseFromExceptionIsProxiesExceptionResponseHandlerMethod() + { + $exception = OAuthServerException::invalidScope('foo', 'https://bar.test'); + $existingResponse = new Response(); + $useFragment = true; + $jsonOptions = 0; + + $responseMock = $this->createMock(ResponseInterface::class); + $exceptionResponseHandlerMock = $this->createMock(ExceptionResponseHandlerInterface::class); + $exceptionResponseHandlerMock + ->expects($this->once()) + ->method('generateHttpResponse') + ->with($exception, $existingResponse, $useFragment, $jsonOptions) + ->willReturn($responseMock); + + $server = new ResourceServer( + $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(), + 'file://' . __DIR__ . '/Stubs/public.key', + $this->getMockBuilder(AuthorizationValidatorInterface::class)->getMock(), + $exceptionResponseHandlerMock + ); + + $response = $server->generateHttpResponse($exception, $existingResponse, $useFragment, $jsonOptions); + + $this->assertSame($responseMock, $response); + } } From d9bc7d2162285478e2c08ac40431be22288a0a24 Mon Sep 17 00:00:00 2001 From: Patrick Rodacker Date: Mon, 20 Apr 2020 23:00:01 +0200 Subject: [PATCH 6/9] removes generateHttpResponse method from OAuthServerException and refactors lib accordingly --- examples/public/auth_code.php | 5 +- examples/public/client_credentials.php | 2 +- examples/public/implicit.php | 2 +- examples/public/password.php | 2 +- examples/public/refresh_token.php | 2 +- src/Exception/OAuthServerException.php | 36 ------------- .../AuthorizationServerMiddleware.php | 8 ++- src/Middleware/ResourceServerMiddleware.php | 8 ++- tests/Exception/OAuthServerExceptionTest.php | 54 ------------------- .../AuthorizationServerMiddlewareTest.php | 35 +++++++++++- .../ResourceServerMiddlewareTest.php | 29 ++++++++++ 11 files changed, 74 insertions(+), 109 deletions(-) diff --git a/examples/public/auth_code.php b/examples/public/auth_code.php index c082e3b3f..b52838bbe 100644 --- a/examples/public/auth_code.php +++ b/examples/public/auth_code.php @@ -59,7 +59,6 @@ return $server; }, ]); - $app->get('/authorize', function (ServerRequestInterface $request, ResponseInterface $response) use ($app) { /* @var \League\OAuth2\Server\AuthorizationServer $server */ $server = $app->getContainer()->get(AuthorizationServer::class); @@ -79,7 +78,7 @@ // Return the HTTP redirect response return $server->completeAuthorizationRequest($authRequest, $response); } catch (OAuthServerException $exception) { - return $exception->generateHttpResponse($response); + return $server->generateHttpResponse($exception, $response); } catch (\Exception $exception) { $body = new Stream('php://temp', 'r+'); $body->write($exception->getMessage()); @@ -95,7 +94,7 @@ try { return $server->respondToAccessTokenRequest($request, $response); } catch (OAuthServerException $exception) { - return $exception->generateHttpResponse($response); + return $server->generateHttpResponse($exception, $response); } catch (\Exception $exception) { $body = new Stream('php://temp', 'r+'); $body->write($exception->getMessage()); diff --git a/examples/public/client_credentials.php b/examples/public/client_credentials.php index 51a1ca0b7..94c0919b2 100644 --- a/examples/public/client_credentials.php +++ b/examples/public/client_credentials.php @@ -64,7 +64,7 @@ } catch (OAuthServerException $exception) { // All instances of OAuthServerException can be formatted into a HTTP response - return $exception->generateHttpResponse($response); + return $server->generateHttpResponse($exception, $response); } catch (\Exception $exception) { // Unknown exception diff --git a/examples/public/implicit.php b/examples/public/implicit.php index ac43f5dd1..b2d34e6a9 100644 --- a/examples/public/implicit.php +++ b/examples/public/implicit.php @@ -68,7 +68,7 @@ // Return the HTTP redirect response return $server->completeAuthorizationRequest($authRequest, $response); } catch (OAuthServerException $exception) { - return $exception->generateHttpResponse($response); + return $server->generateHttpResponse($exception, $response); } catch (\Exception $exception) { $body = new Stream('php://temp', 'r+'); $body->write($exception->getMessage()); diff --git a/examples/public/password.php b/examples/public/password.php index 6857e988a..a99c936bf 100644 --- a/examples/public/password.php +++ b/examples/public/password.php @@ -57,7 +57,7 @@ function (ServerRequestInterface $request, ResponseInterface $response) use ($ap } catch (OAuthServerException $exception) { // All instances of OAuthServerException can be converted to a PSR-7 response - return $exception->generateHttpResponse($response); + return $server->generateHttpResponse($exception, $response); } catch (\Exception $exception) { // Catch unexpected exceptions diff --git a/examples/public/refresh_token.php b/examples/public/refresh_token.php index 39be08262..5aa6b247c 100644 --- a/examples/public/refresh_token.php +++ b/examples/public/refresh_token.php @@ -62,7 +62,7 @@ try { return $server->respondToAccessTokenRequest($request, $response); } catch (OAuthServerException $exception) { - return $exception->generateHttpResponse($response); + return $server->generateHttpResponse($exception, $response); } catch (\Exception $exception) { $response->getBody()->write($exception->getMessage()); diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index e1bd382d1..c9199a511 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -279,42 +279,6 @@ public function getErrorType() return $this->errorType; } - /** - * Generate a HTTP response. - * - * @param ResponseInterface $response - * @param bool $useFragment True if errors should be in the URI fragment instead of query string - * @param int $jsonOptions options passed to json_encode - * - * @return ResponseInterface - */ - public function generateHttpResponse(ResponseInterface $response, $useFragment = false, $jsonOptions = 0) - { - $headers = $this->getHttpHeaders(); - - $payload = $this->getPayload(); - - if ($this->redirectUri !== null) { - if ($useFragment === true) { - $this->redirectUri .= (\strstr($this->redirectUri, '#') === false) ? '#' : '&'; - } else { - $this->redirectUri .= (\strstr($this->redirectUri, '?') === false) ? '?' : '&'; - } - - return $response->withStatus(302)->withHeader('Location', $this->redirectUri . \http_build_query($payload)); - } - - foreach ($headers as $header => $content) { - $response = $response->withHeader($header, $content); - } - - $responseBody = \json_encode($payload, $jsonOptions) ?: 'JSON encoding of payload failed'; - - $response->getBody()->write($responseBody); - - return $response->withStatus($this->getHttpStatusCode()); - } - /** * Get all headers that have to be send with the error response. * diff --git a/src/Middleware/AuthorizationServerMiddleware.php b/src/Middleware/AuthorizationServerMiddleware.php index 9b78b4585..8ed380234 100644 --- a/src/Middleware/AuthorizationServerMiddleware.php +++ b/src/Middleware/AuthorizationServerMiddleware.php @@ -42,12 +42,10 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res try { $response = $this->server->respondToAccessTokenRequest($request, $response); } catch (OAuthServerException $exception) { - return $exception->generateHttpResponse($response); - // @codeCoverageIgnoreStart + return $this->server->generateHttpResponse($exception, $response); } catch (Exception $exception) { - return (new OAuthServerException($exception->getMessage(), 0, 'unknown_error', 500)) - ->generateHttpResponse($response); - // @codeCoverageIgnoreEnd + $serverException = new OAuthServerException($exception->getMessage(), 0, 'unknown_error', 500); + return $this->server->generateHttpResponse($serverException, $response); } // Pass the request and response on to the next responder in the chain diff --git a/src/Middleware/ResourceServerMiddleware.php b/src/Middleware/ResourceServerMiddleware.php index e152a9999..b217aa9f5 100644 --- a/src/Middleware/ResourceServerMiddleware.php +++ b/src/Middleware/ResourceServerMiddleware.php @@ -42,12 +42,10 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res try { $request = $this->server->validateAuthenticatedRequest($request); } catch (OAuthServerException $exception) { - return $exception->generateHttpResponse($response); - // @codeCoverageIgnoreStart + return $this->server->generateHttpResponse($exception, $response); } catch (Exception $exception) { - return (new OAuthServerException($exception->getMessage(), 0, 'unknown_error', 500)) - ->generateHttpResponse($response); - // @codeCoverageIgnoreEnd + $serverException = new OAuthServerException($exception->getMessage(), 0, 'unknown_error', 500); + return $this->server->generateHttpResponse($serverException, $response); } // Pass the request and response on to the next responder in the chain diff --git a/tests/Exception/OAuthServerExceptionTest.php b/tests/Exception/OAuthServerExceptionTest.php index 88a88fb5b..be77b0663 100644 --- a/tests/Exception/OAuthServerExceptionTest.php +++ b/tests/Exception/OAuthServerExceptionTest.php @@ -12,60 +12,6 @@ class OAuthServerExceptionTest extends TestCase { - public function testInvalidClientExceptionSetsAuthenticateHeader() - { - $serverRequest = (new ServerRequest()) - ->withParsedBody([ - 'client_id' => 'foo', - ]) - ->withAddedHeader('Authorization', 'Basic fakeauthdetails'); - - try { - $this->issueInvalidClientException($serverRequest); - } catch (OAuthServerException $e) { - $response = $e->generateHttpResponse(new Response()); - - $this->assertTrue($response->hasHeader('WWW-Authenticate')); - } - } - - public function testInvalidClientExceptionOmitsAuthenticateHeader() - { - $serverRequest = (new ServerRequest()) - ->withParsedBody([ - 'client_id' => 'foo', - ]); - - try { - $this->issueInvalidClientException($serverRequest); - } catch (OAuthServerException $e) { - $response = $e->generateHttpResponse(new Response()); - - $this->assertFalse($response->hasHeader('WWW-Authenticate')); - } - } - - /** - * Issue an invalid client exception - * - * @throws OAuthServerException - */ - private function issueInvalidClientException($serverRequest) - { - $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); - $clientRepositoryMock->method('validateClient')->willReturn(false); - - $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); - $grantMock->setClientRepository($clientRepositoryMock); - - $abstractGrantReflection = new \ReflectionClass($grantMock); - - $validateClientMethod = $abstractGrantReflection->getMethod('validateClient'); - $validateClientMethod->setAccessible(true); - - $validateClientMethod->invoke($grantMock, $serverRequest); - } - public function testHasRedirect() { $exceptionWithRedirect = OAuthServerException::accessDenied('some hint', 'https://example.com/error'); diff --git a/tests/Middleware/AuthorizationServerMiddlewareTest.php b/tests/Middleware/AuthorizationServerMiddlewareTest.php index 54fb193d6..b65540fb9 100644 --- a/tests/Middleware/AuthorizationServerMiddlewareTest.php +++ b/tests/Middleware/AuthorizationServerMiddlewareTest.php @@ -3,9 +3,11 @@ namespace LeagueTests\Middleware; use DateInterval; +use Laminas\Diactoros\Request; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequestFactory; use League\OAuth2\Server\AuthorizationServer; +use League\OAuth2\Server\Exception\ExceptionResponseHandler; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Grant\ClientCredentialsGrant; use League\OAuth2\Server\Middleware\AuthorizationServerMiddleware; @@ -17,6 +19,8 @@ use LeagueTests\Stubs\ScopeEntity; use LeagueTests\Stubs\StubResponseType; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; class AuthorizationServerMiddlewareTest extends TestCase { @@ -105,7 +109,8 @@ function () { public function testOAuthErrorResponseRedirectUri() { $exception = OAuthServerException::invalidScope('test', 'http://foo/bar'); - $response = $exception->generateHttpResponse(new Response()); + $exceptionResponseHandler = new ExceptionResponseHandler(); + $response = $exceptionResponseHandler->generateHttpResponse($exception, 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', @@ -115,10 +120,36 @@ public function testOAuthErrorResponseRedirectUri() public function testOAuthErrorResponseRedirectUriFragment() { $exception = OAuthServerException::invalidScope('test', 'http://foo/bar'); - $response = $exception->generateHttpResponse(new Response(), true); + $exceptionResponseHandler = new ExceptionResponseHandler(); + $response = $exceptionResponseHandler->generateHttpResponse($exception, new Response(), true); $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]); } + + public function testExceptionIsTurnedIntoOAuthServerException(): void + { + $responseMock = $this->createMock(ResponseInterface::class); + $serverMock = $this->createMock(AuthorizationServer::class); + $serverMock->expects($this->once()) + ->method('respondToAccessTokenRequest') + ->willThrowException(new \Exception('foo')); + + $serverMock->expects($this->once()) + ->method('generateHttpResponse') + ->willReturn($responseMock); + + $middleware = new AuthorizationServerMiddleware($serverMock); + + $response = $middleware->__invoke( + $this->createMock(ServerRequestInterface::class), + new Response(), + function () { + return \func_get_args()[1]; + } + ); + + $this->assertSame($responseMock, $response); + } } diff --git a/tests/Middleware/ResourceServerMiddlewareTest.php b/tests/Middleware/ResourceServerMiddlewareTest.php index 106111885..4dadf1b54 100644 --- a/tests/Middleware/ResourceServerMiddlewareTest.php +++ b/tests/Middleware/ResourceServerMiddlewareTest.php @@ -6,13 +6,17 @@ use DateTimeImmutable; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequest; +use League\OAuth2\Server\AuthorizationServer; use League\OAuth2\Server\CryptKey; +use League\OAuth2\Server\Middleware\AuthorizationServerMiddleware; use League\OAuth2\Server\Middleware\ResourceServerMiddleware; use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; use League\OAuth2\Server\ResourceServer; use LeagueTests\Stubs\AccessTokenEntity; use LeagueTests\Stubs\ClientEntity; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; class ResourceServerMiddlewareTest extends TestCase { @@ -106,4 +110,29 @@ function () { $this->assertEquals(401, $response->getStatusCode()); } + + public function testExceptionIsTurnedIntoOAuthServerException(): void + { + $responseMock = $this->createMock(ResponseInterface::class); + $serverMock = $this->createMock(ResourceServer::class); + $serverMock->expects($this->once()) + ->method('validateAuthenticatedRequest') + ->willThrowException(new \Exception('foo')); + + $serverMock->expects($this->once()) + ->method('generateHttpResponse') + ->willReturn($responseMock); + + $middleware = new ResourceServerMiddleware($serverMock); + + $response = $middleware->__invoke( + $this->createMock(ServerRequestInterface::class), + new Response(), + function () { + return \func_get_args()[1]; + } + ); + + $this->assertSame($responseMock, $response); + } } From a8b7b2e22dd92263b576186f22856be09107a415 Mon Sep 17 00:00:00 2001 From: Patrick Rodacker Date: Mon, 20 Apr 2020 23:15:13 +0200 Subject: [PATCH 7/9] fixes phpdoc formatting --- src/AuthorizationServer.php | 14 +++++++------- src/Exception/ExceptionResponseHandler.php | 8 ++++---- .../ExceptionResponseHandlerInterface.php | 8 ++++---- src/Exception/ExceptionResponseHandlerTrait.php | 8 ++++---- src/ResourceServer.php | 8 ++++---- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/AuthorizationServer.php b/src/AuthorizationServer.php index 129745244..7f6d50b79 100644 --- a/src/AuthorizationServer.php +++ b/src/AuthorizationServer.php @@ -91,13 +91,13 @@ class AuthorizationServer implements EmitterAwareInterface /** * New server instance. * - * @param ClientRepositoryInterface $clientRepository - * @param AccessTokenRepositoryInterface $accessTokenRepository - * @param ScopeRepositoryInterface $scopeRepository - * @param CryptKeyInterface|string $privateKey - * @param string|Key $encryptionKey - * @param null|ResponseTypeInterface $responseType - * @param null|ExceptionResponseHandlerInterface $exceptionResponseHandler + * @param ClientRepositoryInterface $clientRepository + * @param AccessTokenRepositoryInterface $accessTokenRepository + * @param ScopeRepositoryInterface $scopeRepository + * @param CryptKeyInterface|string $privateKey + * @param string|Key $encryptionKey + * @param null|ResponseTypeInterface $responseType + * @param null|ExceptionResponseHandlerInterface $exceptionResponseHandler */ public function __construct( ClientRepositoryInterface $clientRepository, diff --git a/src/Exception/ExceptionResponseHandler.php b/src/Exception/ExceptionResponseHandler.php index 5fd699e9d..c8dcb0111 100644 --- a/src/Exception/ExceptionResponseHandler.php +++ b/src/Exception/ExceptionResponseHandler.php @@ -11,10 +11,10 @@ class ExceptionResponseHandler implements ExceptionResponseHandlerInterface /** * Generate a HTTP response from am OAuthServerException * - * @param OAuthServerException $exception - * @param ResponseInterface $response - * @param bool $useFragment True if errors should be in the URI fragment instead of query string - * @param int $jsonOptions options passed to json_encode + * @param OAuthServerException $exception + * @param ResponseInterface $response + * @param bool $useFragment True if errors should be in the URI fragment instead of query string + * @param int $jsonOptions options passed to json_encode * * @return ResponseInterface */ diff --git a/src/Exception/ExceptionResponseHandlerInterface.php b/src/Exception/ExceptionResponseHandlerInterface.php index 0b0ccb594..3d9e3b1a9 100644 --- a/src/Exception/ExceptionResponseHandlerInterface.php +++ b/src/Exception/ExceptionResponseHandlerInterface.php @@ -9,10 +9,10 @@ interface ExceptionResponseHandlerInterface /** * Generate a HTTP response from am OAuthServerException * - * @param OAuthServerException $exception - * @param ResponseInterface $response - * @param bool $useFragment True if errors should be in the URI fragment instead of query string - * @param int $jsonOptions options passed to json_encode + * @param OAuthServerException $exception + * @param ResponseInterface $response + * @param bool $useFragment True if errors should be in the URI fragment instead of query string + * @param int $jsonOptions options passed to json_encode * * @return ResponseInterface */ diff --git a/src/Exception/ExceptionResponseHandlerTrait.php b/src/Exception/ExceptionResponseHandlerTrait.php index ac3a28442..04e5a3d8b 100644 --- a/src/Exception/ExceptionResponseHandlerTrait.php +++ b/src/Exception/ExceptionResponseHandlerTrait.php @@ -9,10 +9,10 @@ trait ExceptionResponseHandlerTrait /** * Generate a HTTP response from am OAuthServerException * - * @param OAuthServerException $exception - * @param ResponseInterface $response - * @param bool $useFragment True if errors should be in the URI fragment instead of query string - * @param int $jsonOptions options passed to json_encode + * @param OAuthServerException $exception + * @param ResponseInterface $response + * @param bool $useFragment True if errors should be in the URI fragment instead of query string + * @param int $jsonOptions options passed to json_encode * * @return ResponseInterface */ diff --git a/src/ResourceServer.php b/src/ResourceServer.php index 407c712d8..4c511bb38 100644 --- a/src/ResourceServer.php +++ b/src/ResourceServer.php @@ -45,10 +45,10 @@ class ResourceServer /** * New server instance. * - * @param AccessTokenRepositoryInterface $accessTokenRepository - * @param CryptKeyInterface|string $publicKey - * @param null|AuthorizationValidatorInterface $authorizationValidator - * @param null|ExceptionResponseHandlerInterface $exceptionResponseHandler + * @param AccessTokenRepositoryInterface $accessTokenRepository + * @param CryptKeyInterface|string $publicKey + * @param null|AuthorizationValidatorInterface $authorizationValidator + * @param null|ExceptionResponseHandlerInterface $exceptionResponseHandler */ public function __construct( AccessTokenRepositoryInterface $accessTokenRepository, From b60de466346af2bd2cba73d18c5880cc1edf7e36 Mon Sep 17 00:00:00 2001 From: Patrick Rodacker Date: Mon, 20 Apr 2020 23:16:29 +0200 Subject: [PATCH 8/9] removes unused imports --- src/Exception/OAuthServerException.php | 1 - tests/AuthorizationServerTest.php | 1 - tests/Exception/OAuthServerExceptionTest.php | 4 ---- tests/Middleware/AuthorizationServerMiddlewareTest.php | 1 - tests/Middleware/ResourceServerMiddlewareTest.php | 2 -- 5 files changed, 9 deletions(-) diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index c9199a511..9dc91352d 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -10,7 +10,6 @@ namespace League\OAuth2\Server\Exception; use Exception; -use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Throwable; diff --git a/tests/AuthorizationServerTest.php b/tests/AuthorizationServerTest.php index e812cf82f..6048cba24 100644 --- a/tests/AuthorizationServerTest.php +++ b/tests/AuthorizationServerTest.php @@ -8,7 +8,6 @@ use Laminas\Diactoros\ServerRequestFactory; use League\OAuth2\Server\AuthorizationServer; use League\OAuth2\Server\CryptKeyInterface; -use League\OAuth2\Server\Exception\ExceptionResponseHandler; use League\OAuth2\Server\Exception\ExceptionResponseHandlerInterface; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Grant\AuthCodeGrant; diff --git a/tests/Exception/OAuthServerExceptionTest.php b/tests/Exception/OAuthServerExceptionTest.php index be77b0663..6a9dc81d1 100644 --- a/tests/Exception/OAuthServerExceptionTest.php +++ b/tests/Exception/OAuthServerExceptionTest.php @@ -3,11 +3,7 @@ namespace LeagueTests\Exception; use Exception; -use Laminas\Diactoros\Response; -use Laminas\Diactoros\ServerRequest; use League\OAuth2\Server\Exception\OAuthServerException; -use League\OAuth2\Server\Grant\AbstractGrant; -use League\OAuth2\Server\Repositories\ClientRepositoryInterface; use PHPUnit\Framework\TestCase; class OAuthServerExceptionTest extends TestCase diff --git a/tests/Middleware/AuthorizationServerMiddlewareTest.php b/tests/Middleware/AuthorizationServerMiddlewareTest.php index b65540fb9..daa250307 100644 --- a/tests/Middleware/AuthorizationServerMiddlewareTest.php +++ b/tests/Middleware/AuthorizationServerMiddlewareTest.php @@ -3,7 +3,6 @@ namespace LeagueTests\Middleware; use DateInterval; -use Laminas\Diactoros\Request; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequestFactory; use League\OAuth2\Server\AuthorizationServer; diff --git a/tests/Middleware/ResourceServerMiddlewareTest.php b/tests/Middleware/ResourceServerMiddlewareTest.php index 4dadf1b54..e16b9b751 100644 --- a/tests/Middleware/ResourceServerMiddlewareTest.php +++ b/tests/Middleware/ResourceServerMiddlewareTest.php @@ -6,9 +6,7 @@ use DateTimeImmutable; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequest; -use League\OAuth2\Server\AuthorizationServer; use League\OAuth2\Server\CryptKey; -use League\OAuth2\Server\Middleware\AuthorizationServerMiddleware; use League\OAuth2\Server\Middleware\ResourceServerMiddleware; use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; use League\OAuth2\Server\ResourceServer; From 94114baf832289d706cacb4690d6e8095f24d639 Mon Sep 17 00:00:00 2001 From: Patrick Rodacker Date: Mon, 20 Apr 2020 23:17:16 +0200 Subject: [PATCH 9/9] adds blank line before return statement --- src/Middleware/AuthorizationServerMiddleware.php | 1 + src/Middleware/ResourceServerMiddleware.php | 1 + 2 files changed, 2 insertions(+) diff --git a/src/Middleware/AuthorizationServerMiddleware.php b/src/Middleware/AuthorizationServerMiddleware.php index 8ed380234..a7c2b58ef 100644 --- a/src/Middleware/AuthorizationServerMiddleware.php +++ b/src/Middleware/AuthorizationServerMiddleware.php @@ -45,6 +45,7 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res return $this->server->generateHttpResponse($exception, $response); } catch (Exception $exception) { $serverException = new OAuthServerException($exception->getMessage(), 0, 'unknown_error', 500); + return $this->server->generateHttpResponse($serverException, $response); } diff --git a/src/Middleware/ResourceServerMiddleware.php b/src/Middleware/ResourceServerMiddleware.php index b217aa9f5..1a630fd29 100644 --- a/src/Middleware/ResourceServerMiddleware.php +++ b/src/Middleware/ResourceServerMiddleware.php @@ -45,6 +45,7 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res return $this->server->generateHttpResponse($exception, $response); } catch (Exception $exception) { $serverException = new OAuthServerException($exception->getMessage(), 0, 'unknown_error', 500); + return $this->server->generateHttpResponse($serverException, $response); }