Merge master into version 8 branch

CHANGELOG.md

@@ -8,7 +8,24 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Changed
 - Replace `convertToJWT()` interface with a more generic `__toString()` to improve extensibility (PR #874)
-- The `invalidClient()` function accepts a PSR-7 compliant `$serverRequest` argument to avoid accessing the `$_SERVER` global variable and improve testing (PR #899) 
+- The `invalidClient()` function accepts a PSR-7 compliant `$serverRequest` argument to avoid accessing the `$_SERVER` global variable and improve testing (PR #899)
+
+## [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
 
@@ -85,6 +102,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)
@@ -431,8 +450,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

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
 

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) {
+            try {
-                throw OAuthServerException::accessDenied('Access token could not be verified');
+                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

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,

src/CryptTrait.php

@@ -1,6 +1,6 @@
 <?php
 /**
- * Public/private key encryption.
+ * Encrypt/decrypt with encryptionKey.
  *
  * @author      Alex Bilbie <hello@alexbilbie.com>
  * @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
      *

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.
      *

src/Grant/AbstractGrant.php

@@ -190,25 +190,13 @@ abstract class AbstractGrant implements GrantTypeInterface
 
         if ($client instanceof ClientEntityInterface === false) {
             $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
-            throw OAuthServerException::invalidClient($request);
+            throw OAuthServerException::invalidClient();
         }
 
         // If a redirect URI is provided ensure it matches what is pre-registered
         $redirectUri = $this->getRequestParameter('redirect_uri', $request, null);
         if ($redirectUri !== null) {
-            if (
+            $this->validateRedirectUri($redirectUri, $client, $request);
-                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);
-            }
         }
 
         return $client;

src/Grant/AuthCodeGrant.php

@@ -236,29 +236,17 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
 
         if ($client instanceof ClientEntityInterface === false) {
             $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
-            throw OAuthServerException::invalidClient($request);
+            throw OAuthServerException::invalidClient();
         }
 
         $redirectUri = $this->getQueryStringParameter('redirect_uri', $request);
 
         if ($redirectUri !== null) {
-            if (
+            $this->validateRedirectUri($redirectUri, $client, $request);
-                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);
-            }
         } 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();
         } else {
             $redirectUri = is_array($client->getRedirectUri())
                 ? $client->getRedirectUri()[0]

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 (
+            $this->validateRedirectUri($redirectUri, $client, $request);
-                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);
-            }
         } elseif (is_array($client->getRedirectUri()) && count($client->getRedirectUri()) !== 1
             || empty($client->getRedirectUri())) {
             $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));

tests/AuthorizationValidators/BearerTokenValidatorTest.php

@@ -0,0 +1,40 @@
+<?php
+
+namespace LeagueTests\AuthorizationValidators;
+
+use Lcobucci\JWT\Builder;
+use League\OAuth2\Server\AuthorizationValidators\BearerTokenValidator;
+use League\OAuth2\Server\CryptKey;
+use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface;
+use PHPUnit\Framework\TestCase;
+use Zend\Diactoros\ServerRequest;
+
+class BearerTokenValidatorTest extends TestCase
+{
+    /**
+     * @expectedException League\OAuth2\Server\Exception\OAuthServerException
+     * @expectedExceptionCode 9
+     */
+    public function testThrowExceptionWhenAccessTokenIsNotSigned()
+    {
+        $accessTokenRepositoryMock = $this->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);
+    }
+}

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());
+    }
 }