diff --git a/CHANGELOG.md b/CHANGELOG.md index 88d8f16f..8ad6e3f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,23 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - The `invalidClient()` function accepts a PSR-7 compliant `$serverRequest` argument to avoid accessing the `$_SERVER` global variable and improve testing (PR #899) - `issueAccessToken()` in the Abstract Grant no longer sets access token client, user ID or scopes. These values should already have been set when calling `getNewToken()` (PR #919) +## [7.2.0] - released 2018-06-23 + +### Changed +- Added new`validateRedirectUri` method AbstractGrant to remove three instances of code duplication (PR #912) +- Allow 640 as a crypt key file permission (PR #917) + +### Added +- Function `hasRedirect()` added to `OAuthServerException` (PR #703) + +### Fixed +- Catch and handle `BadMethodCallException` from the `verify()` method of the JWT token in the `validateAuthorization` method (PR #904) + +## [4.1.7] - released 2018-06-23 + +### Fixed +- Ensure `empty()` function call only contains variable to be compatible with PHP 5.4 (PR #918) + ## [7.1.1] - released 2018-05-21 ### Fixed @@ -86,6 +103,8 @@ 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) @@ -432,8 +451,6 @@ 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/README.md b/README.md index e4d90f46..4d5fd215 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ The following versions of PHP are supported: The `openssl` extension is also required. -All HTTP messages passed to the server should be [PSR-7 compliant](https://www.php-fig.org/psr/psr-7/). This ensures interoperability between other packages and frameworks. +All HTTP messages passed to the server should be [PSR-7 compliant](https://www.php-fig.org/psr/psr-7/). This ensures interoperability with other packages and frameworks. ## Installation diff --git a/src/AuthorizationValidators/BearerTokenValidator.php b/src/AuthorizationValidators/BearerTokenValidator.php index 6f299ce4..2efa3c8e 100644 --- a/src/AuthorizationValidators/BearerTokenValidator.php +++ b/src/AuthorizationValidators/BearerTokenValidator.php @@ -65,8 +65,12 @@ class BearerTokenValidator implements AuthorizationValidatorInterface try { // Attempt to parse and validate the JWT $token = (new Parser())->parse($jwt); - if ($token->verify(new Sha256(), $this->publicKey->getKeyPath()) === false) { - throw OAuthServerException::accessDenied('Access token could not be verified'); + try { + if ($token->verify(new Sha256(), $this->publicKey->getKeyPath()) === false) { + throw OAuthServerException::accessDenied('Access token could not be verified'); + } + } catch (\BadMethodCallException $exception) { + throw OAuthServerException::accessDenied('Access token is not signed'); } // Ensure access token hasn't expired diff --git a/src/CryptKey.php b/src/CryptKey.php index efc5f5e8..98b53222 100644 --- a/src/CryptKey.php +++ b/src/CryptKey.php @@ -48,7 +48,7 @@ class CryptKey if ($keyPermissionsCheck === true) { // Verify the permissions of the key $keyPathPerms = decoct(fileperms($keyPath) & 0777); - if (in_array($keyPathPerms, ['400', '440', '600', '660'], true) === false) { + if (in_array($keyPathPerms, ['400', '440', '600', '640', '660'], true) === false) { trigger_error(sprintf( 'Key file "%s" permissions are not correct, recommend changing to 600 or 660 instead of %s', $keyPath, diff --git a/src/CryptTrait.php b/src/CryptTrait.php index c9a6d7a6..672c7e2e 100644 --- a/src/CryptTrait.php +++ b/src/CryptTrait.php @@ -1,6 +1,6 @@ * @copyright Copyright (c) Alex Bilbie @@ -22,7 +22,7 @@ trait CryptTrait protected $encryptionKey; /** - * Encrypt data with a private key. + * Encrypt data with encryptionKey. * * @param string $unencryptedData * @@ -44,7 +44,7 @@ trait CryptTrait } /** - * Decrypt data with a public key. + * Decrypt data with encryptionKey. * * @param string $encryptedData * diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php index 345b99a0..264f0e87 100644 --- a/src/Exception/OAuthServerException.php +++ b/src/Exception/OAuthServerException.php @@ -323,6 +323,21 @@ class OAuthServerException extends \Exception return $headers; } + /** + * Check if the exception has an associated redirect URI. + * + * Returns whether the exception includes a redirect, since + * getHttpStatusCode() doesn't return a 302 when there's a + * redirect enabled. This helps when you want to override local + * error pages but want to let redirects through. + * + * @return bool + */ + public function hasRedirect() + { + return $this->redirectUri !== null; + } + /** * Returns the HTTP status code to send when the exceptions is output. * diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index ed05dfac..e34436e5 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -196,24 +196,40 @@ abstract class AbstractGrant implements GrantTypeInterface // If a redirect URI is provided ensure it matches what is pre-registered $redirectUri = $this->getRequestParameter('redirect_uri', $request, null); if ($redirectUri !== null) { - if ( - is_string($client->getRedirectUri()) - && (strcmp($client->getRedirectUri(), $redirectUri) !== 0) - ) { - $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - 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($request); - } + $this->validateRedirectUri($redirectUri, $client, $request); } return $client; } + /** + * Validate redirectUri from the request. + * If a redirect URI is provided ensure it matches what is pre-registered + * + * @param string $redirectUri + * @param ClientEntityInterface $client + * @param ServerRequestInterface $request + * + * @throws OAuthServerException + */ + protected function validateRedirectUri( + string $redirectUri, + ClientEntityInterface $client, + ServerRequestInterface $request + ) { + if (is_string($client->getRedirectUri()) + && (strcmp($client->getRedirectUri(), $redirectUri) !== 0) + ) { + $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); + 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($request); + } + } + /** * Validate scopes in the request. * diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 084e21d4..790315af 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -242,19 +242,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant $redirectUri = $this->getQueryStringParameter('redirect_uri', $request); if ($redirectUri !== null) { - if ( - is_string($client->getRedirectUri()) - && (strcmp($client->getRedirectUri(), $redirectUri) !== 0) - ) { - $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - 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($request); - } + $this->validateRedirectUri($redirectUri, $client, $request); } elseif (is_array($client->getRedirectUri()) && count($client->getRedirectUri()) !== 1 || empty($client->getRedirectUri())) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 5d6035e4..7caadbc7 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -118,6 +118,7 @@ class ImplicitGrant extends AbstractAuthorizeGrant $request, $this->getServerParameter('PHP_AUTH_USER', $request) ); + if (is_null($clientId)) { throw OAuthServerException::invalidRequest('client_id'); } @@ -135,20 +136,9 @@ class ImplicitGrant extends AbstractAuthorizeGrant } $redirectUri = $this->getQueryStringParameter('redirect_uri', $request); + if ($redirectUri !== null) { - if ( - is_string($client->getRedirectUri()) - && (strcmp($client->getRedirectUri(), $redirectUri) !== 0) - ) { - $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); - 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($request); - } + $this->validateRedirectUri($redirectUri, $client, $request); } elseif (is_array($client->getRedirectUri()) && count($client->getRedirectUri()) !== 1 || empty($client->getRedirectUri())) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); diff --git a/tests/AuthorizationValidators/BearerTokenValidatorTest.php b/tests/AuthorizationValidators/BearerTokenValidatorTest.php new file mode 100644 index 00000000..801846cb --- /dev/null +++ b/tests/AuthorizationValidators/BearerTokenValidatorTest.php @@ -0,0 +1,40 @@ +getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); + + $bearerTokenValidator = new BearerTokenValidator($accessTokenRepositoryMock); + $bearerTokenValidator->setPublicKey(new CryptKey('file://' . __DIR__ . '/../Stubs/public.key')); + + $unsignedJwt = (new Builder()) + ->setAudience('client-id') + ->setId('token-id', true) + ->setIssuedAt(time()) + ->setNotBefore(time()) + ->setExpiration(time()) + ->setSubject('user-id') + ->set('scopes', 'scope1 scope2 scope3 scope4') + ->getToken(); + + $request = new ServerRequest(); + $request = $request->withHeader('authorization', sprintf('Bearer %s', $unsignedJwt)); + + $bearerTokenValidator->validateAuthorization($request); + } +} diff --git a/tests/Exception/OAuthServerExceptionTest.php b/tests/Exception/OAuthServerExceptionTest.php index bac914e4..cead7d6a 100644 --- a/tests/Exception/OAuthServerExceptionTest.php +++ b/tests/Exception/OAuthServerExceptionTest.php @@ -64,4 +64,18 @@ class OAuthServerExceptionTest extends TestCase $validateClientMethod->invoke($grantMock, $serverRequest); } + + public function testHasRedirect() + { + $exceptionWithRedirect = OAuthServerException::accessDenied('some hint', 'https://example.com/error'); + + $this->assertTrue($exceptionWithRedirect->hasRedirect()); + } + + public function testDoesNotHaveRedirect() + { + $exceptionWithoutRedirect = OAuthServerException::accessDenied('Some hint'); + + $this->assertFalse($exceptionWithoutRedirect->hasRedirect()); + } }