diff --git a/CHANGELOG.md b/CHANGELOG.md index b3f974a7..20a54fb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Changed +- The `invalidClient()` function accepts a PSR-7 compliant `$serverRequest` argument to avoid accessing the `$_SERVER` global variable and improve testing (PR #899) + ## [7.1.1] - released 2018-05-21 ### Fixed @@ -81,8 +84,6 @@ To address feedback from the security release the following change has been made ## [5.1.4] - 2017-07-01 -- Fixed multiple security vulnerabilities as a result of a security audit paid for by the [Mozilla Secure Open Source Fund](https://wiki.mozilla.org/MOSS/Secure_Open_Source). All users of this library are encouraged to update as soon as possible to this version or version 6.0 or greater. - - It is recommended on each `AuthorizationServer` instance you set the `setEncryptionKey()`. This will result in stronger encryption being used. If this method is not set messages will be sent to the defined error handling routines (using `error_log`). Please see the examples and documentation for examples. - TravisCI now tests PHP 7.1 (Issue #671) - Fix middleware example fatal error (Issue #682) - Fix typo in the first README sentence (Issue #690) @@ -429,6 +430,8 @@ Version 5 is a complete code rewrite. [3.0.0]: https://github.com/thephpleague/oauth2-server/compare/2.1.1...3.0.0 [2.1.1]: https://github.com/thephpleague/oauth2-server/compare/2.1.0...2.1.1 [2.1.0]: https://github.com/thephpleague/oauth2-server/compare/2.0.5...2.1.0 +[2.1.1]: https://github.com/thephpleague/oauth2-server/compare/2.1.0...2.1.1 +[2.1.0]: https://github.com/thephpleague/oauth2-server/compare/2.0.5...2.1.0 [2.0.5]: https://github.com/thephpleague/oauth2-server/compare/2.0.4...2.0.5 [2.0.4]: https://github.com/thephpleague/oauth2-server/compare/2.0.3...2.0.4 [2.0.3]: https://github.com/thephpleague/oauth2-server/compare/2.0.2...2.0.3 diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index a62d961d..345b99a0 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -10,6 +10,7 @@ namespace League\OAuth2\Server\Exception; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; class OAuthServerException extends \Exception { @@ -38,6 +39,11 @@ class OAuthServerException extends \Exception */ private $payload; + /** + * @var ServerRequestInterface + */ + private $serverRequest; + /** * Throw a new exception. * @@ -84,6 +90,16 @@ class OAuthServerException extends \Exception $this->payload = $payload; } + /** + * Set the server request that is responsible for generating the exception + * + * @param ServerRequestInterface $serverRequest + */ + public function setServerRequest($serverRequest) + { + $this->serverRequest = $serverRequest; + } + /** * Unsupported grant type error. * @@ -117,13 +133,17 @@ class OAuthServerException extends \Exception /** * Invalid client error. * + * @param ServerRequestInterface $serverRequest + * * @return static */ - public static function invalidClient() + public static function invalidClient($serverRequest) { - $errorMessage = 'Client authentication failed'; + $exception = new static('Client authentication failed', 4, 'invalid_client', 401); - return new static($errorMessage, 4, 'invalid_client', 401); + $exception->setServerRequest($serverRequest); + + return $exception; } /** @@ -294,8 +314,8 @@ class OAuthServerException extends \Exception // include the "WWW-Authenticate" response header field // matching the authentication scheme used by the client. // @codeCoverageIgnoreStart - if ($this->errorType === 'invalid_client' && array_key_exists('HTTP_AUTHORIZATION', $_SERVER) !== false) { - $authScheme = strpos($_SERVER['HTTP_AUTHORIZATION'], 'Bearer') === 0 ? 'Bearer' : 'Basic'; + if ($this->errorType === 'invalid_client' && $this->serverRequest->hasHeader('Authorization') === true) { + $authScheme = strpos($this->serverRequest->getHeader('Authorization')[0], 'Bearer') === 0 ? 'Bearer' : 'Basic'; $headers['WWW-Authenticate'] = $authScheme . ' realm="OAuth"'; } diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 79a1ac47..05b73faa 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -190,7 +190,7 @@ abstract class AbstractGrant implements GrantTypeInterface if ($client instanceof ClientEntityInterface === false) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient(); + throw OAuthServerException::invalidClient($request); } // If a redirect URI is provided ensure it matches what is pre-registered @@ -201,13 +201,13 @@ abstract class AbstractGrant implements GrantTypeInterface && (strcmp($client->getRedirectUri(), $redirectUri) !== 0) ) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient(); + throw OAuthServerException::invalidClient($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(); + throw OAuthServerException::invalidClient($request); } } diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 80e1cd0f..084e21d4 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -236,7 +236,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant if ($client instanceof ClientEntityInterface === false) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient(); + throw OAuthServerException::invalidClient($request); } $redirectUri = $this->getQueryStringParameter('redirect_uri', $request); @@ -247,18 +247,18 @@ class AuthCodeGrant extends AbstractAuthorizeGrant && (strcmp($client->getRedirectUri(), $redirectUri) !== 0) ) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient(); + throw OAuthServerException::invalidClient($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(); + throw OAuthServerException::invalidClient($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(); + throw OAuthServerException::invalidClient($request); } else { $redirectUri = is_array($client->getRedirectUri()) ? $client->getRedirectUri()[0] diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index b4157883..c740f75c 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -131,7 +131,7 @@ class ImplicitGrant extends AbstractAuthorizeGrant if ($client instanceof ClientEntityInterface === false) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient(); + throw OAuthServerException::invalidClient($request); } $redirectUri = $this->getQueryStringParameter('redirect_uri', $request); @@ -141,18 +141,18 @@ class ImplicitGrant extends AbstractAuthorizeGrant && (strcmp($client->getRedirectUri(), $redirectUri) !== 0) ) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidClient(); + throw OAuthServerException::invalidClient($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(); + throw OAuthServerException::invalidClient($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(); + throw OAuthServerException::invalidClient($request); } else { $redirectUri = is_array($client->getRedirectUri()) ? $client->getRedirectUri()[0] diff --git a/tests/Exception/OAuthServerExceptionTest.php b/tests/Exception/OAuthServerExceptionTest.php new file mode 100644 index 00000000..bac914e4 --- /dev/null +++ b/tests/Exception/OAuthServerExceptionTest.php @@ -0,0 +1,67 @@ +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('getClientEntity')->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); + } +} diff --git a/tests/Grant/AbstractGrantTest.php b/tests/Grant/AbstractGrantTest.php index 6266df0a..5da2776e 100644 --- a/tests/Grant/AbstractGrantTest.php +++ b/tests/Grant/AbstractGrantTest.php @@ -122,7 +122,7 @@ class AbstractGrantTest extends TestCase $validateClientMethod = $abstractGrantReflection->getMethod('validateClient'); $validateClientMethod->setAccessible(true); - $result = $validateClientMethod->invoke($grantMock, $serverRequest, true, true); + $result = $validateClientMethod->invoke($grantMock, $serverRequest); $this->assertEquals($client, $result); }