Merge pull request #938 from Sephster/force-pkce-for-public-clients

Force PKCE for public clients

CHANGELOG.md

@@ -6,10 +6,21 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+### Added
+- Flag, `requireCodeChallengeForPublicClients`, used to reject public clients that do not provide a code challenge for the Auth Code Grant (PR #938)
+- Public clients can now use the Auth Code Grant (PR #938)
+- `isConfidential` property added to `ClientEntity` to identify type of client (PR #938)
+- Function `validateClient()` added to validate clients which was previously performed by the `getClientEntity()` function (PR #938)
+
 ### 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)
 - `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)
+- No longer need to enable PKCE with `enableCodeExchangeProof` flag. Any client sending a code challenge will initiate PKCE checks. (PR #938)
+- Function `getClientEntity()` no longer performs client validation (PR #938)
+
+### Removed
+- `enableCodeExchangeProof` flag (PR #938)
 
 ## [7.2.0] - released 2018-06-23
 

examples/composer.lock

@@ -1,10 +1,10 @@
 {
     "_readme": [
         "This file locks the dependencies of your project to a known state",
-        "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file",
+        "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
         "This file is @generated automatically"
     ],
-    "content-hash": "9813ed7c3b6dcf107f44df9392935b8f",
+    "content-hash": "6701e0eaa09f74e1ebb19c3d61f39068",
     "packages": [
         {
             "name": "container-interop/container-interop",
@@ -82,25 +82,29 @@
         },
         {
             "name": "pimple/pimple",
-            "version": "v3.0.2",
+            "version": "v3.2.3",
             "source": {
                 "type": "git",
                 "url": "https://github.com/silexphp/Pimple.git",
-                "reference": "a30f7d6e57565a2e1a316e1baf2a483f788b258a"
+                "reference": "9e403941ef9d65d20cba7d54e29fe906db42cf32"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/silexphp/Pimple/zipball/a30f7d6e57565a2e1a316e1baf2a483f788b258a",
+                "url": "https://api.github.com/repos/silexphp/Pimple/zipball/9e403941ef9d65d20cba7d54e29fe906db42cf32",
-                "reference": "a30f7d6e57565a2e1a316e1baf2a483f788b258a",
+                "reference": "9e403941ef9d65d20cba7d54e29fe906db42cf32",
                 "shasum": ""
             },
             "require": {
-                "php": ">=5.3.0"
+                "php": ">=5.3.0",
+                "psr/container": "^1.0"
+            },
+            "require-dev": {
+                "symfony/phpunit-bridge": "^3.2"
             },
             "type": "library",
             "extra": {
                 "branch-alias": {
-                    "dev-master": "3.0.x-dev"
+                    "dev-master": "3.2.x-dev"
                 }
             },
             "autoload": {
@@ -124,7 +128,7 @@
                 "container",
                 "dependency injection"
             ],
-            "time": "2015-09-11T15:10:35+00:00"
+            "time": "2018-01-21T07:42:36+00:00"
         },
         {
             "name": "psr/container",
@@ -295,25 +299,25 @@
     "packages-dev": [
         {
             "name": "defuse/php-encryption",
-            "version": "v2.1.0",
+            "version": "v2.2.1",
             "source": {
                 "type": "git",
                 "url": "https://github.com/defuse/php-encryption.git",
-                "reference": "5176f5abb38d3ea8a6e3ac6cd3bbb54d8185a689"
+                "reference": "0f407c43b953d571421e0020ba92082ed5fb7620"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/defuse/php-encryption/zipball/5176f5abb38d3ea8a6e3ac6cd3bbb54d8185a689",
+                "url": "https://api.github.com/repos/defuse/php-encryption/zipball/0f407c43b953d571421e0020ba92082ed5fb7620",
-                "reference": "5176f5abb38d3ea8a6e3ac6cd3bbb54d8185a689",
+                "reference": "0f407c43b953d571421e0020ba92082ed5fb7620",
                 "shasum": ""
             },
             "require": {
                 "ext-openssl": "*",
-                "paragonie/random_compat": "~2.0",
+                "paragonie/random_compat": ">= 2",
                 "php": ">=5.4.0"
             },
             "require-dev": {
-                "nikic/php-parser": "^2.0|^3.0",
+                "nikic/php-parser": "^2.0|^3.0|^4.0",
                 "phpunit/phpunit": "^4|^5"
             },
             "bin": [
@@ -354,20 +358,20 @@
                 "security",
                 "symmetric key cryptography"
             ],
-            "time": "2017-05-18T21:28:48+00:00"
+            "time": "2018-07-24T23:27:56+00:00"
         },
         {
             "name": "lcobucci/jwt",
-            "version": "3.2.1",
+            "version": "3.2.4",
             "source": {
                 "type": "git",
                 "url": "https://github.com/lcobucci/jwt.git",
-                "reference": "ddce703826f9c5229781933b1a39069e38e6a0f3"
+                "reference": "c9704b751315d21735dc98d78d4f37bd73596da7"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/lcobucci/jwt/zipball/ddce703826f9c5229781933b1a39069e38e6a0f3",
+                "url": "https://api.github.com/repos/lcobucci/jwt/zipball/c9704b751315d21735dc98d78d4f37bd73596da7",
-                "reference": "ddce703826f9c5229781933b1a39069e38e6a0f3",
+                "reference": "c9704b751315d21735dc98d78d4f37bd73596da7",
                 "shasum": ""
             },
             "require": {
@@ -412,7 +416,7 @@
                 "JWS",
                 "jwt"
             ],
-            "time": "2016-10-31T20:09:32+00:00"
+            "time": "2018-08-03T11:23:50+00:00"
         },
         {
             "name": "league/event",
@@ -466,16 +470,16 @@
         },
         {
             "name": "paragonie/random_compat",
-            "version": "v2.0.10",
+            "version": "v2.0.17",
             "source": {
                 "type": "git",
                 "url": "https://github.com/paragonie/random_compat.git",
-                "reference": "634bae8e911eefa89c1abfbf1b66da679ac8f54d"
+                "reference": "29af24f25bab834fcbb38ad2a69fa93b867e070d"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/paragonie/random_compat/zipball/634bae8e911eefa89c1abfbf1b66da679ac8f54d",
+                "url": "https://api.github.com/repos/paragonie/random_compat/zipball/29af24f25bab834fcbb38ad2a69fa93b867e070d",
-                "reference": "634bae8e911eefa89c1abfbf1b66da679ac8f54d",
+                "reference": "29af24f25bab834fcbb38ad2a69fa93b867e070d",
                 "shasum": ""
             },
             "require": {
@@ -507,10 +511,74 @@
             "description": "PHP 5.x polyfill for random_bytes() and random_int() from PHP 7",
             "keywords": [
                 "csprng",
+                "polyfill",
                 "pseudorandom",
                 "random"
             ],
-            "time": "2017-03-13T16:27:32+00:00"
+            "time": "2018-07-04T16:31:37+00:00"
+        },
+        {
+            "name": "zendframework/zend-diactoros",
+            "version": "1.8.4",
+            "source": {
+                "type": "git",
+                "url": "https://github.com/zendframework/zend-diactoros.git",
+                "reference": "736ffa7c2bfa4a60e8a10acb316fa2ac456c5fba"
+            },
+            "dist": {
+                "type": "zip",
+                "url": "https://api.github.com/repos/zendframework/zend-diactoros/zipball/736ffa7c2bfa4a60e8a10acb316fa2ac456c5fba",
+                "reference": "736ffa7c2bfa4a60e8a10acb316fa2ac456c5fba",
+                "shasum": ""
+            },
+            "require": {
+                "php": "^5.6 || ^7.0",
+                "psr/http-message": "^1.0"
+            },
+            "provide": {
+                "psr/http-message-implementation": "1.0"
+            },
+            "require-dev": {
+                "ext-dom": "*",
+                "ext-libxml": "*",
+                "phpunit/phpunit": "^5.7.16 || ^6.0.8 || ^7.2.7",
+                "zendframework/zend-coding-standard": "~1.0"
+            },
+            "type": "library",
+            "extra": {
+                "branch-alias": {
+                    "dev-master": "1.8.x-dev",
+                    "dev-develop": "1.9.x-dev",
+                    "dev-release-2.0": "2.0.x-dev"
+                }
+            },
+            "autoload": {
+                "files": [
+                    "src/functions/create_uploaded_file.php",
+                    "src/functions/marshal_headers_from_sapi.php",
+                    "src/functions/marshal_method_from_sapi.php",
+                    "src/functions/marshal_protocol_version_from_sapi.php",
+                    "src/functions/marshal_uri_from_sapi.php",
+                    "src/functions/normalize_server.php",
+                    "src/functions/normalize_uploaded_files.php",
+                    "src/functions/parse_cookie_header.php"
+                ],
+                "psr-4": {
+                    "Zend\\Diactoros\\": "src/"
+                }
+            },
+            "notification-url": "https://packagist.org/downloads/",
+            "license": [
+                "BSD-2-Clause"
+            ],
+            "description": "PSR HTTP Message implementations",
+            "homepage": "https://github.com/zendframework/zend-diactoros",
+            "keywords": [
+                "http",
+                "psr",
+                "psr-7"
+            ],
+            "time": "2018-08-01T13:47:49+00:00"
         }
     ],
     "aliases": [],

src/CodeChallengeVerifiers/CodeChallengeVerifierInterface.php

@@ -0,0 +1,30 @@
+<?php
+/**
+ * @author      Lukáš Unger <lookymsc@gmail.com>
+ * @copyright   Copyright (c) Lukáš Unger
+ * @license     http://mit-license.org/
+ *
+ * @link        https://github.com/thephpleague/oauth2-server
+ */
+
+namespace League\OAuth2\Server\CodeChallengeVerifiers;
+
+interface CodeChallengeVerifierInterface
+{
+    /**
+     * Return code challenge method.
+     *
+     * @return string
+     */
+    public function getMethod();
+
+    /**
+     * Verify the code challenge.
+     *
+     * @param string $codeVerifier
+     * @param string $codeChallenge
+     *
+     * @return bool
+     */
+    public function verifyCodeChallenge($codeVerifier, $codeChallenge);
+}

src/CodeChallengeVerifiers/PlainVerifier.php

@@ -0,0 +1,36 @@
+<?php
+/**
+ * @author      Lukáš Unger <lookymsc@gmail.com>
+ * @copyright   Copyright (c) Lukáš Unger
+ * @license     http://mit-license.org/
+ *
+ * @link        https://github.com/thephpleague/oauth2-server
+ */
+
+namespace League\OAuth2\Server\CodeChallengeVerifiers;
+
+class PlainVerifier implements CodeChallengeVerifierInterface
+{
+    /**
+     * Return code challenge method.
+     *
+     * @return string
+     */
+    public function getMethod()
+    {
+        return 'plain';
+    }
+
+    /**
+     * Verify the code challenge.
+     *
+     * @param string $codeVerifier
+     * @param string $codeChallenge
+     *
+     * @return bool
+     */
+    public function verifyCodeChallenge($codeVerifier, $codeChallenge)
+    {
+        return hash_equals($codeVerifier, $codeChallenge);
+    }
+}

src/CodeChallengeVerifiers/S256Verifier.php

@@ -0,0 +1,39 @@
+<?php
+/**
+ * @author      Lukáš Unger <lookymsc@gmail.com>
+ * @copyright   Copyright (c) Lukáš Unger
+ * @license     http://mit-license.org/
+ *
+ * @link        https://github.com/thephpleague/oauth2-server
+ */
+
+namespace League\OAuth2\Server\CodeChallengeVerifiers;
+
+class S256Verifier implements CodeChallengeVerifierInterface
+{
+    /**
+     * Return code challenge method.
+     *
+     * @return string
+     */
+    public function getMethod()
+    {
+        return 'S256';
+    }
+
+    /**
+     * Verify the code challenge.
+     *
+     * @param string $codeVerifier
+     * @param string $codeChallenge
+     *
+     * @return bool
+     */
+    public function verifyCodeChallenge($codeVerifier, $codeChallenge)
+    {
+        return hash_equals(
+            strtr(rtrim(base64_encode(hash('sha256', $codeVerifier, true)), '='), '+/', '-_'),
+            $codeChallenge
+        );
+    }
+}

src/Entities/ClientEntityInterface.php

@@ -33,4 +33,11 @@ interface ClientEntityInterface
      * @return string|string[]
      */
     public function getRedirectUri();
+
+    /**
+     * Returns true if the client is confidential.
+     *
+     * @return bool
+     */
+    public function isConfidential();
 }

src/Entities/Traits/ClientTrait.php

@@ -21,6 +21,11 @@ trait ClientTrait
      */
     protected $redirectUri;
 
+    /**
+     * @var bool
+     */
+    protected $isConfidential = false;
+
     /**
      * Get the client's name.
      *
@@ -43,4 +48,14 @@ trait ClientTrait
     {
         return $this->redirectUri;
     }
+
+    /**
+     * Returns true if the client is confidential.
+     *
+     * @return bool
+     */
+    public function isConfidential()
+    {
+        return $this->isConfidential;
+    }
 }

src/Grant/AbstractGrant.php

@@ -174,27 +174,24 @@ abstract class AbstractGrant implements GrantTypeInterface
         list($basicAuthUser, $basicAuthPassword) = $this->getBasicAuthCredentials($request);
 
         $clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser);
+
         if (is_null($clientId)) {
             throw OAuthServerException::invalidRequest('client_id');
         }
 
-        // If the client is confidential require the client secret
         $clientSecret = $this->getRequestParameter('client_secret', $request, $basicAuthPassword);
 
-        $client = $this->clientRepository->getClientEntity(
+        if ($this->clientRepository->validateClient($clientId, $clientSecret, $this->getIdentifier()) === false) {
-            $clientId,
-            $this->getIdentifier(),
-            $clientSecret,
-            true
-        );
-
-        if ($client instanceof ClientEntityInterface === false) {
             $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
+
             throw OAuthServerException::invalidClient($request);
         }
 
+        $client = $this->clientRepository->getClientEntity($clientId);
+
         // If a redirect URI is provided ensure it matches what is pre-registered
         $redirectUri = $this->getRequestParameter('redirect_uri', $request, null);
+
         if ($redirectUri !== null) {
             $this->validateRedirectUri($redirectUri, $client, $request);
         }

src/Grant/AuthCodeGrant.php

@@ -9,6 +9,9 @@
 
 namespace League\OAuth2\Server\Grant;
 
+use League\OAuth2\Server\CodeChallengeVerifiers\CodeChallengeVerifierInterface;
+use League\OAuth2\Server\CodeChallengeVerifiers\PlainVerifier;
+use League\OAuth2\Server\CodeChallengeVerifiers\S256Verifier;
 use League\OAuth2\Server\Entities\ClientEntityInterface;
 use League\OAuth2\Server\Entities\ScopeEntityInterface;
 use League\OAuth2\Server\Entities\UserEntityInterface;
@@ -31,7 +34,12 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
     /**
      * @var bool
      */
-    private $enableCodeExchangeProof = false;
+    private $requireCodeChallengeForPublicClients = true;
+
+    /**
+     * @var CodeChallengeVerifierInterface[]
+     */
+    private $codeChallengeVerifiers = [];
 
     /**
      * @param AuthCodeRepositoryInterface     $authCodeRepository
@@ -47,11 +55,23 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
         $this->setRefreshTokenRepository($refreshTokenRepository);
         $this->authCodeTTL = $authCodeTTL;
         $this->refreshTokenTTL = new \DateInterval('P1M');
+
+        // SHOULD ONLY DO THIS IS SHA256 is supported
+        $s256Verifier = new S256Verifier();
+        $plainVerifier = new PlainVerifier();
+
+        $this->codeChallengeVerifiers = [
+            $s256Verifier->getMethod() => $s256Verifier,
+            $plainVerifier->getMethod() => $plainVerifier,
+        ];
     }
 
-    public function enableCodeExchangeProof()
+    /**
+     * Disable the requirement for a code challenge for public clients.
+     */
+    public function disableRequireCodeChallengeForPublicClients()
     {
-        $this->enableCodeExchangeProof = true;
+        $this->requireCodeChallengeForPublicClients = false;
     }
 
     /**
@@ -70,8 +90,19 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
         ResponseTypeInterface $responseType,
         \DateInterval $accessTokenTTL
     ) {
-        // Validate request
+        $clientId = $this->getRequestParameter('client_id', $request, null);
-        $client = $this->validateClient($request);
+
+        if ($clientId === null) {
+            throw OAuthServerException::invalidRequest('client_id');
+        }
+
+        $client = $this->clientRepository->getClientEntity($clientId);
+
+        // Only validate the client if it is confidential
+        if ($client->isConfidential()) {
+            $this->validateClient($request);
+        }
+
         $encryptedAuthCode = $this->getRequestParameter('code', $request, null);
 
         if ($encryptedAuthCode === null) {
@@ -81,6 +112,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
         // Validate the authorization code
         try {
             $authCodePayload = json_decode($this->decrypt($encryptedAuthCode));
+
             if (time() > $authCodePayload->expire_time) {
                 throw OAuthServerException::invalidRequest('code', 'Authorization code has expired');
             }
@@ -95,6 +127,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
 
             // The redirect URI is required in this request
             $redirectUri = $this->getRequestParameter('redirect_uri', $request, null);
+
             if (empty($authCodePayload->redirect_uri) === false && $redirectUri === null) {
                 throw OAuthServerException::invalidRequest('redirect_uri');
             }
@@ -104,6 +137,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
             }
 
             $scopes = [];
+
             foreach ($authCodePayload->scopes as $scopeId) {
                 $scope = $this->scopeRepository->getScopeEntityByIdentifier($scopeId);
 
@@ -128,8 +162,9 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
         }
 
         // Validate code challenge
-        if ($this->enableCodeExchangeProof === true) {
+        if (!empty($authCodePayload->code_challenge)) {
             $codeVerifier = $this->getRequestParameter('code_verifier', $request, null);
+
             if ($codeVerifier === null) {
                 throw OAuthServerException::invalidRequest('code_verifier');
             }
@@ -143,32 +178,19 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
                 );
             }
 
-            switch ($authCodePayload->code_challenge_method) {
+            if (isset($this->codeChallengeVerifiers[$authCodePayload->code_challenge_method])) {
-                case 'plain':
+                $codeChallengeVerifier = $this->codeChallengeVerifiers[$authCodePayload->code_challenge_method];
-                    if (hash_equals($codeVerifier, $authCodePayload->code_challenge) === false) {
-                        throw OAuthServerException::invalidGrant('Failed to verify `code_verifier`.');
-                    }
 
-                    break;
+                if ($codeChallengeVerifier->verifyCodeChallenge($codeVerifier, $authCodePayload->code_challenge) === false) {
-                case 'S256':
-                    if (
-                        hash_equals(
-                            strtr(rtrim(base64_encode(hash('sha256', $codeVerifier, true)), '='), '+/', '-_'),
-                            $authCodePayload->code_challenge
-                        ) === false
-                    ) {
                     throw OAuthServerException::invalidGrant('Failed to verify `code_verifier`.');
                 }
-                    // @codeCoverageIgnoreStart
+            } else {
-                    break;
-                default:
                 throw OAuthServerException::serverError(
                     sprintf(
                         'Unsupported code challenge method `%s`',
                         $authCodePayload->code_challenge_method
                     )
                 );
-                // @codeCoverageIgnoreEnd
             }
         }
 
@@ -227,12 +249,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
             throw OAuthServerException::invalidRequest('client_id');
         }
 
-        $client = $this->clientRepository->getClientEntity(
+        $client = $this->clientRepository->getClientEntity($clientId);
-            $clientId,
-            $this->getIdentifier(),
-            null,
-            false
-        );
 
         if ($client instanceof ClientEntityInterface === false) {
             $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
@@ -271,18 +288,20 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
 
         $authorizationRequest->setScopes($scopes);
 
-        if ($this->enableCodeExchangeProof === true) {
         $codeChallenge = $this->getQueryStringParameter('code_challenge', $request);
-            if ($codeChallenge === null) {
-                throw OAuthServerException::invalidRequest('code_challenge');
-            }
 
+        if ($codeChallenge !== null) {
             $codeChallengeMethod = $this->getQueryStringParameter('code_challenge_method', $request, 'plain');
 
-            if (in_array($codeChallengeMethod, ['plain', 'S256'], true) === false) {
+            if (array_key_exists($codeChallengeMethod, $this->codeChallengeVerifiers) === false) {
                 throw OAuthServerException::invalidRequest(
                     'code_challenge_method',
-                    'Code challenge method must be `plain` or `S256`'
+                    'Code challenge method must be one of ' . implode(', ', array_map(
+                        function ($method) {
+                            return '`' . $method . '`';
+                        },
+                        array_keys($this->codeChallengeVerifiers)
+                    ))
                 );
             }
 
@@ -297,6 +316,8 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
 
             $authorizationRequest->setCodeChallenge($codeChallenge);
             $authorizationRequest->setCodeChallengeMethod($codeChallengeMethod);
+        } elseif ($this->requireCodeChallengeForPublicClients && !$client->isConfidential()) {
+            throw OAuthServerException::invalidRequest('code_challenge', 'Code challenge must be provided for public clients');
         }
 
         return $authorizationRequest;

src/Grant/ImplicitGrant.php

@@ -123,12 +123,7 @@ class ImplicitGrant extends AbstractAuthorizeGrant
             throw OAuthServerException::invalidRequest('client_id');
         }
 
-        $client = $this->clientRepository->getClientEntity(
+        $client = $this->clientRepository->getClientEntity($clientId);
-            $clientId,
-            $this->getIdentifier(),
-            null,
-            false
-        );
 
         if ($client instanceof ClientEntityInterface === false) {
             $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));

src/Repositories/ClientRepositoryInterface.php

@@ -20,12 +20,19 @@ interface ClientRepositoryInterface extends RepositoryInterface
      * Get a client.
      *
      * @param string $clientIdentifier The client's identifier
-     * @param null|string $grantType          The grant type used (if sent)
-     * @param null|string $clientSecret       The client's secret (if sent)
-     * @param bool        $mustValidateSecret If true the client must attempt to validate the secret if the client
-     *                                        is confidential
      *
      * @return ClientEntityInterface
      */
-    public function getClientEntity($clientIdentifier, $grantType = null, $clientSecret = null, $mustValidateSecret = true);
+    public function getClientEntity($clientIdentifier);
+
+    /**
+     * Validate a client's secret.
+     *
+     * @param string      $clientIdentifier The client's identifier
+     * @param null|string $clientSecret     The client's secret (if sent)
+     * @param null|string $grantType        The type of grant the client is using (if sent)
+     *
+     * @return bool
+     */
+    public function validateClient($clientIdentifier, $clientSecret, $grantType);
 }

tests/AuthorizationServerTest.php

@@ -148,6 +148,7 @@ class AuthorizationServerTest extends TestCase
     {
         $client = new ClientEntity();
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 

tests/CodeChallengeVerifiers/PlainVerifierTest.php

@@ -0,0 +1,24 @@
+<?php
+
+namespace LeagueTests\CodeChallengeVerifiers;
+
+use League\OAuth2\Server\CodeChallengeVerifiers\PlainVerifier;
+use PHPUnit\Framework\TestCase;
+
+class PlainVerifierTest extends TestCase
+{
+    public function testGetMethod()
+    {
+        $verifier = new PlainVerifier();
+
+        $this->assertEquals('plain', $verifier->getMethod());
+    }
+
+    public function testVerifyCodeChallenge()
+    {
+        $verifier = new PlainVerifier();
+
+        $this->assertTrue($verifier->verifyCodeChallenge('foo', 'foo'));
+        $this->assertFalse($verifier->verifyCodeChallenge('foo', 'bar'));
+    }
+}

tests/CodeChallengeVerifiers/S256VerifierTest.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace LeagueTests\CodeChallengeVerifiers;
+
+use League\OAuth2\Server\CodeChallengeVerifiers\S256Verifier;
+use PHPUnit\Framework\TestCase;
+
+class S256VerifierTest extends TestCase
+{
+    public function testGetMethod()
+    {
+        $verifier = new S256Verifier();
+
+        $this->assertEquals('S256', $verifier->getMethod());
+    }
+
+    public function testVerifyCodeChallengeSucceeds()
+    {
+        $codeChallenge = $this->createCodeChallenge('foo');
+        $verifier = new S256Verifier();
+
+        $this->assertTrue($verifier->verifyCodeChallenge('foo', $codeChallenge));
+    }
+
+    public function testVerifyCodeChallengeFails()
+    {
+        $codeChallenge = $this->createCodeChallenge('bar');
+        $verifier = new S256Verifier();
+
+        $this->assertFalse($verifier->verifyCodeChallenge('foo', $codeChallenge));
+    }
+
+    private function createCodeChallenge($codeVerifier)
+    {
+        return strtr(rtrim(base64_encode(hash('sha256', $codeVerifier, true)), '='), '+/', '-_');
+    }
+}

tests/Exception/OAuthServerExceptionTest.php

@@ -52,7 +52,7 @@ class OAuthServerExceptionTest extends TestCase
     private function issueInvalidClientException($serverRequest)
     {
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
-        $clientRepositoryMock->method('getClientEntity')->willReturn(false);
+        $clientRepositoryMock->method('validateClient')->willReturn(false);
 
         $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
         $grantMock->setClientRepository($clientRepositoryMock);

tests/Grant/AbstractGrantTest.php

@@ -175,7 +175,7 @@ class AbstractGrantTest extends TestCase
     public function testValidateClientMissingClientSecret()
     {
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
-        $clientRepositoryMock->method('getClientEntity')->willReturn(null);
+        $clientRepositoryMock->method('validateClient')->willReturn(false);
 
         /** @var AbstractGrant $grantMock */
         $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
@@ -200,7 +200,7 @@ class AbstractGrantTest extends TestCase
     public function testValidateClientInvalidClientSecret()
     {
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
-        $clientRepositoryMock->method('getClientEntity')->willReturn(null);
+        $clientRepositoryMock->method('validateClient')->willReturn(false);
 
         /** @var AbstractGrant $grantMock */
         $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
@@ -282,7 +282,7 @@ class AbstractGrantTest extends TestCase
     public function testValidateClientBadClient()
     {
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
-        $clientRepositoryMock->method('getClientEntity')->willReturn(null);
+        $clientRepositoryMock->method('validateClient')->willReturn(false);
 
         /** @var AbstractGrant $grantMock */
         $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);

tests/Grant/AuthCodeGrantTest.php

@@ -84,6 +84,7 @@ class AuthCodeGrantTest extends TestCase
     {
         $client = new ClientEntity();
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
 
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
@@ -123,6 +124,7 @@ class AuthCodeGrantTest extends TestCase
     {
         $client = new ClientEntity();
         $client->setRedirectUri(['http://foo/bar']);
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -173,7 +175,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setScopeRepository($scopeRepositoryMock);
         $grant->setDefaultScope(self::DEFAULT_SCOPE);
@@ -212,7 +214,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
 
         $request = new ServerRequest(
@@ -249,7 +251,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
 
         $request = new ServerRequest(
@@ -286,7 +288,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
 
         $request = new ServerRequest(
@@ -444,49 +446,6 @@ class AuthCodeGrantTest extends TestCase
         $grant->validateAuthorizationRequest($request);
     }
 
-    /**
-     * @expectedException \League\OAuth2\Server\Exception\OAuthServerException
-     * @expectedExceptionCode 3
-     */
-    public function testValidateAuthorizationRequestMissingCodeChallenge()
-    {
-        $client = new ClientEntity();
-        $client->setRedirectUri('http://foo/bar');
-        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
-        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
-
-        $scope = new ScopeEntity();
-        $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
-        $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope);
-
-        $grant = new AuthCodeGrant(
-            $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
-            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
-            new \DateInterval('PT10M')
-        );
-        $grant->enableCodeExchangeProof();
-        $grant->setClientRepository($clientRepositoryMock);
-        $grant->setScopeRepository($scopeRepositoryMock);
-        $grant->setDefaultScope(self::DEFAULT_SCOPE);
-
-        $request = new ServerRequest(
-            [],
-            [],
-            null,
-            null,
-            'php://input',
-            [],
-            [],
-            [
-                'response_type' => 'code',
-                'client_id'     => 'foo',
-                'redirect_uri'  => 'http://foo/bar',
-            ]
-        );
-
-        $grant->validateAuthorizationRequest($request);
-    }
-
     /**
      * @expectedException \League\OAuth2\Server\Exception\OAuthServerException
      * @expectedExceptionCode 3
@@ -507,7 +466,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setScopeRepository($scopeRepositoryMock);
         $grant->setDefaultScope(self::DEFAULT_SCOPE);
@@ -579,6 +538,75 @@ class AuthCodeGrantTest extends TestCase
     }
 
     public function testRespondToAccessTokenRequest()
+    {
+        $client = new ClientEntity();
+        $client->setIdentifier('foo');
+        $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+
+        $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
+        $scopeEntity = new ScopeEntity();
+        $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity);
+        $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
+
+        $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
+        $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+        $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
+
+        $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
+        $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf();
+        $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity());
+
+        $grant = new AuthCodeGrant(
+            $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
+            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
+            new \DateInterval('PT10M')
+        );
+        $grant->setClientRepository($clientRepositoryMock);
+        $grant->setScopeRepository($scopeRepositoryMock);
+        $grant->setAccessTokenRepository($accessTokenRepositoryMock);
+        $grant->setRefreshTokenRepository($refreshTokenRepositoryMock);
+        $grant->setEncryptionKey($this->cryptStub->getKey());
+        $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key'));
+
+        $request = new ServerRequest(
+            [],
+            [],
+            null,
+            'POST',
+            'php://input',
+            [],
+            [],
+            [],
+            [
+                'grant_type'   => 'authorization_code',
+                'client_id'    => 'foo',
+                'redirect_uri' => 'http://foo/bar',
+                'code'         => $this->cryptStub->doEncrypt(
+                    json_encode(
+                        [
+                            'auth_code_id' => uniqid(),
+                            'expire_time'  => time() + 3600,
+                            'client_id'    => 'foo',
+                            'user_id'      => 123,
+                            'scopes'       => ['foo'],
+                            'redirect_uri' => 'http://foo/bar',
+                        ]
+                    )
+                ),
+            ]
+        );
+
+        /** @var StubResponseType $response */
+        $response = $grant->respondToAccessTokenRequest($request, new StubResponseType(), new \DateInterval('PT10M'));
+
+        $this->assertInstanceOf(AccessTokenEntityInterface::class, $response->getAccessToken());
+        $this->assertInstanceOf(RefreshTokenEntityInterface::class, $response->getRefreshToken());
+    }
+
+    public function testRespondToAccessTokenRequestForPublicClient()
     {
         $client = new ClientEntity();
         $client->setIdentifier('foo');
@@ -651,6 +679,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -672,7 +701,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setScopeRepository($scopeRepositoryMock);
         $grant->setAccessTokenRepository($accessTokenRepositoryMock);
@@ -723,6 +752,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -744,7 +774,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setScopeRepository($scopeRepositoryMock);
         $grant->setAccessTokenRepository($accessTokenRepositoryMock);
@@ -798,6 +828,7 @@ class AuthCodeGrantTest extends TestCase
     {
         $client = new ClientEntity();
         $client->setIdentifier('foo');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -845,6 +876,7 @@ class AuthCodeGrantTest extends TestCase
     {
         $client = new ClientEntity();
         $client->setIdentifier('foo');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -893,6 +925,7 @@ class AuthCodeGrantTest extends TestCase
     {
         $client = new ClientEntity();
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -935,6 +968,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -995,6 +1029,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -1058,6 +1093,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -1118,6 +1154,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -1167,6 +1204,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -1188,7 +1226,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setAccessTokenRepository($accessTokenRepositoryMock);
         $grant->setRefreshTokenRepository($refreshTokenRepositoryMock);
@@ -1239,6 +1277,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -1260,7 +1299,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setAccessTokenRepository($accessTokenRepositoryMock);
         $grant->setRefreshTokenRepository($refreshTokenRepositoryMock);
@@ -1311,6 +1350,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -1332,7 +1372,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setAccessTokenRepository($accessTokenRepositoryMock);
         $grant->setRefreshTokenRepository($refreshTokenRepositoryMock);
@@ -1383,6 +1423,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -1404,7 +1445,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setAccessTokenRepository($accessTokenRepositoryMock);
         $grant->setRefreshTokenRepository($refreshTokenRepositoryMock);
@@ -1455,6 +1496,7 @@ class AuthCodeGrantTest extends TestCase
         $client = new ClientEntity();
         $client->setIdentifier('foo');
         $client->setRedirectUri('http://foo/bar');
+        $client->setConfidential();
         $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
         $clientRepositoryMock->method('getClientEntity')->willReturn($client);
 
@@ -1476,7 +1518,7 @@ class AuthCodeGrantTest extends TestCase
             $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
             new \DateInterval('PT10M')
         );
-        $grant->enableCodeExchangeProof();
+
         $grant->setClientRepository($clientRepositoryMock);
         $grant->setAccessTokenRepository($accessTokenRepositoryMock);
         $grant->setRefreshTokenRepository($refreshTokenRepositoryMock);
@@ -1823,4 +1865,47 @@ class AuthCodeGrantTest extends TestCase
 
         $grant->completeAuthorizationRequest(new AuthorizationRequest());
     }
+
+    public function testPublicClientAuthCodeRequestRejectedWhenCodeChallengeRequiredButNotGiven()
+    {
+        $client = new ClientEntity();
+        $client->setRedirectUri('http://foo/bar');
+
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+
+        $scope = new ScopeEntity();
+        $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
+        $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope);
+
+        $grant = new AuthCodeGrant(
+            $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
+            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
+            new \DateInterval('PT10M')
+        );
+
+        $grant->setClientRepository($clientRepositoryMock);
+        $grant->setScopeRepository($scopeRepositoryMock);
+        $grant->setDefaultScope(self::DEFAULT_SCOPE);
+
+        $request = new ServerRequest(
+            [],
+            [],
+            null,
+            null,
+            'php://input',
+            [],
+            [],
+            [
+                'response_type' => 'code',
+                'client_id'     => 'foo',
+                'redirect_uri'  => 'http://foo/bar',
+            ]
+        );
+
+        $this->expectException(OAuthServerException::class);
+        $this->expectExceptionCode(3);
+
+        $grant->validateAuthorizationRequest($request);
+    }
 }

tests/Middleware/AuthorizationServerMiddlewareTest.php

@@ -66,7 +66,7 @@ class AuthorizationServerMiddlewareTest extends TestCase
     public function testOAuthErrorResponse()
     {
         $clientRepository = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
-        $clientRepository->method('getClientEntity')->willReturn(null);
+        $clientRepository->method('validateClient')->willReturn(false);
 
         $server = new AuthorizationServer(
             $clientRepository,

tests/Stubs/ClientEntity.php

@@ -14,4 +14,9 @@ class ClientEntity implements ClientEntityInterface
     {
         $this->redirectUri = $uri;
     }
+
+    public function setConfidential()
+    {
+        $this->isConfidential = true;
+    }
 }