Updated changelog

Removed chmod from CryptKey and add toggle to disable checking

CHANGELOG.md

@@ -1,5 +1,17 @@
 # Changelog
 
+## 6.0.2 (released 2017-08-03)
+
+* An invalid refresh token that can't be decrypted now returns a HTTP 401 error instead of HTTP 400 (Issue #759)
+* Removed chmod from CryptKey and add toggle to disable checking (Issue #776)
+* Fixes invalid code challenge method payload key name (Issue #777)
+
+## 6.0.1 (released 2017-07-19)
+
+To address feedback from the security release the following change has been made:
+  
+* If an RSA key cannot be chmod'ed to 600 then it will now throw a E_USER_NOTICE instead of an exception.
+
 ## 6.0.0 (released 2017-07-01)
 
 * Breaking change: The `AuthorizationServer` constructor now expects an encryption key string instead of a public key

CONTRIBUTING.md

@@ -1,7 +1,7 @@
 Thanks for contributing to this project.
 
 
-**Please submit your pull request against the `develop` branch only.**
+**Please submit your pull request against the `master` branch only.**
 
 
 Please ensure that you run `phpunit` from the project root after you've made any changes.

README.md

@@ -3,7 +3,7 @@
 ### :warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning:
 ### Security Notice 
 
-### Please upgrade to version `>=5.1.4` (backwards compatible) or `6.x` (one tiny breaking change) to fix some potential security vulnerabilities 
+### Please upgrade to version `>=5.1.4` (backwards compatible) or `6.x` (one tiny breaking change) to fix some potential security vulnerabilities - [visit this page for more information](https://oauth2.thephpleague.com/v5-security-improvements/)
 ### :warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning::warning:
 
 [![Latest Version](http://img.shields.io/packagist/v/league/oauth2-server.svg?style=flat-square)](https://github.com/thephpleague/oauth2-server/releases)

src/CryptKey.php

@@ -29,8 +29,9 @@ class CryptKey
     /**
      * @param string      $keyPath
      * @param null|string $passPhrase
+     * @param bool        $keyPermissionsCheck
      */
-    public function __construct($keyPath, $passPhrase = null)
+    public function __construct($keyPath, $passPhrase = null, $keyPermissionsCheck = true)
     {
         if (preg_match(self::RSA_KEY_PATTERN, $keyPath)) {
             $keyPath = $this->saveKeyToFile($keyPath);
@@ -44,20 +45,15 @@ class CryptKey
             throw new \LogicException(sprintf('Key path "%s" does not exist or is not readable', $keyPath));
         }
 
-        // Verify the permissions of the key
-        $keyPathPerms = decoct(fileperms($keyPath) & 0777);
-        if ($keyPathPerms !== '600') {
-            // Attempt to correct the permissions
-            if (chmod($keyPath, 0600) === false) {
-                // @codeCoverageIgnoreStart
-                throw new \LogicException(
-                    sprintf(
-                        'Key file "%s" permissions are not correct, should be 600 instead of %s, unable to automatically resolve the issue',
-                        $keyPath,
-                        $keyPathPerms
-                    )
-                );
-                // @codeCoverageIgnoreEnd
+        if ($keyPermissionsCheck === true) {
+            // Verify the permissions of the key
+            $keyPathPerms = decoct(fileperms($keyPath) & 0777);
+            if (in_array($keyPathPerms, ['600', '660'], true) === false) {
+                trigger_error(sprintf(
+                    'Key file "%s" permissions are not correct, should be 600 or 660 instead of %s',
+                    $keyPath,
+                    $keyPathPerms
+                ), E_USER_NOTICE);
             }
         }
 
@@ -79,19 +75,19 @@ class CryptKey
 
         if (!file_exists($keyPath) && !touch($keyPath)) {
             // @codeCoverageIgnoreStart
-            throw new \RuntimeException('"%s" key file could not be created', $keyPath);
+            throw new \RuntimeException(sprintf('"%s" key file could not be created', $keyPath));
             // @codeCoverageIgnoreEnd
         }
 
         if (file_put_contents($keyPath, $key) === false) {
             // @codeCoverageIgnoreStart
-            throw new \RuntimeException('Unable to write key file to temporary directory "%s"', $tmpDir);
+            throw new \RuntimeException(sprintf('Unable to write key file to temporary directory "%s"', $tmpDir));
             // @codeCoverageIgnoreEnd
         }
 
         if (chmod($keyPath, 0600) === false) {
             // @codeCoverageIgnoreStart
-            throw new \RuntimeException('The key file "%s" file mode could not be changed with chmod to 600', $keyPath);
+            throw new \RuntimeException(sprintf('The key file "%s" file mode could not be changed with chmod to 600', $keyPath));
             // @codeCoverageIgnoreEnd
         }
 

src/Exception/OAuthServerException.php

@@ -152,7 +152,7 @@ class OAuthServerException extends \Exception
      */
     public static function invalidRefreshToken($hint = null)
     {
-        return new static('The refresh token is invalid.', 8, 'invalid_request', 400, $hint);
+        return new static('The refresh token is invalid.', 8, 'invalid_request', 401, $hint);
     }
 
     /**

src/Grant/AuthCodeGrant.php

@@ -312,14 +312,14 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
             );
 
             $payload = [
-                'client_id'               => $authCode->getClient()->getIdentifier(),
-                'redirect_uri'            => $authCode->getRedirectUri(),
-                'auth_code_id'            => $authCode->getIdentifier(),
-                'scopes'                  => $authCode->getScopes(),
-                'user_id'                 => $authCode->getUserIdentifier(),
-                'expire_time'             => (new \DateTime())->add($this->authCodeTTL)->format('U'),
-                'code_challenge'          => $authorizationRequest->getCodeChallenge(),
-                'code_challenge_method  ' => $authorizationRequest->getCodeChallengeMethod(),
+                'client_id'             => $authCode->getClient()->getIdentifier(),
+                'redirect_uri'          => $authCode->getRedirectUri(),
+                'auth_code_id'          => $authCode->getIdentifier(),
+                'scopes'                => $authCode->getScopes(),
+                'user_id'               => $authCode->getUserIdentifier(),
+                'expire_time'           => (new \DateTime())->add($this->authCodeTTL)->format('U'),
+                'code_challenge'        => $authorizationRequest->getCodeChallenge(),
+                'code_challenge_method' => $authorizationRequest->getCodeChallengeMethod(),
             ];
 
             $response = new RedirectResponse();

src/Repositories/AccessTokenRepositoryInterface.php

@@ -12,6 +12,7 @@ namespace League\OAuth2\Server\Repositories;
 use League\OAuth2\Server\Entities\AccessTokenEntityInterface;
 use League\OAuth2\Server\Entities\ClientEntityInterface;
 use League\OAuth2\Server\Entities\ScopeEntityInterface;
+use League\OAuth2\Server\Exception\UniqueTokenIdentifierConstraintViolationException;
 
 /**
  * Access token interface.
@@ -33,6 +34,8 @@ interface AccessTokenRepositoryInterface extends RepositoryInterface
      * Persists a new access token to permanent storage.
      *
      * @param AccessTokenEntityInterface $accessTokenEntity
+     *
+     * @throws UniqueTokenIdentifierConstraintViolationException
      */
     public function persistNewAccessToken(AccessTokenEntityInterface $accessTokenEntity);
 

src/Repositories/AuthCodeRepositoryInterface.php

@@ -10,6 +10,7 @@
 namespace League\OAuth2\Server\Repositories;
 
 use League\OAuth2\Server\Entities\AuthCodeEntityInterface;
+use League\OAuth2\Server\Exception\UniqueTokenIdentifierConstraintViolationException;
 
 /**
  * Auth code storage interface.
@@ -27,6 +28,8 @@ interface AuthCodeRepositoryInterface extends RepositoryInterface
      * Persists a new auth code to permanent storage.
      *
      * @param AuthCodeEntityInterface $authCodeEntity
+     *
+     * @throws UniqueTokenIdentifierConstraintViolationException
      */
     public function persistNewAuthCode(AuthCodeEntityInterface $authCodeEntity);
 

src/Repositories/RefreshTokenRepositoryInterface.php

@@ -10,6 +10,7 @@
 namespace League\OAuth2\Server\Repositories;
 
 use League\OAuth2\Server\Entities\RefreshTokenEntityInterface;
+use League\OAuth2\Server\Exception\UniqueTokenIdentifierConstraintViolationException;
 
 /**
  * Refresh token interface.
@@ -27,6 +28,8 @@ interface RefreshTokenRepositoryInterface extends RepositoryInterface
      * Create a new refresh token_name.
      *
      * @param RefreshTokenEntityInterface $refreshTokenEntity
+     *
+     * @throws UniqueTokenIdentifierConstraintViolationException
      */
     public function persistNewRefreshToken(RefreshTokenEntityInterface $refreshTokenEntity);
 

tests/AuthorizationServerTest.php

@@ -26,6 +26,13 @@ use Zend\Diactoros\ServerRequestFactory;
 
 class AuthorizationServerTest extends \PHPUnit_Framework_TestCase
 {
+    public function setUp()
+    {
+        // Make sure the keys have the correct permissions.
+        chmod(__DIR__ . '/Stubs/private.key', 0600);
+        chmod(__DIR__ . '/Stubs/public.key', 0600);
+    }
+
     public function testRespondToRequestInvalidGrantType()
     {
         $server = new AuthorizationServer(