From 972808561d85e1f3e0a7778238d589221ea75323 Mon Sep 17 00:00:00 2001 From: Andrew Millington Date: Sun, 12 Aug 2018 20:06:34 +0100 Subject: [PATCH] Add optional code challenge check for public clients --- examples/composer.lock | 120 ++++++++++++++---- src/Entities/ClientEntityInterface.php | 7 + src/Grant/AuthCodeGrant.php | 36 +++++- .../ClientRepositoryInterface.php | 9 ++ 4 files changed, 142 insertions(+), 30 deletions(-) diff --git a/examples/composer.lock b/examples/composer.lock index 7210f31e..8603438a 100644 --- a/examples/composer.lock +++ b/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", - "reference": "a30f7d6e57565a2e1a316e1baf2a483f788b258a", + "url": "https://api.github.com/repos/silexphp/Pimple/zipball/9e403941ef9d65d20cba7d54e29fe906db42cf32", + "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", - "reference": "5176f5abb38d3ea8a6e3ac6cd3bbb54d8185a689", + "url": "https://api.github.com/repos/defuse/php-encryption/zipball/0f407c43b953d571421e0020ba92082ed5fb7620", + "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", - "reference": "ddce703826f9c5229781933b1a39069e38e6a0f3", + "url": "https://api.github.com/repos/lcobucci/jwt/zipball/c9704b751315d21735dc98d78d4f37bd73596da7", + "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", - "reference": "634bae8e911eefa89c1abfbf1b66da679ac8f54d", + "url": "https://api.github.com/repos/paragonie/random_compat/zipball/29af24f25bab834fcbb38ad2a69fa93b867e070d", + "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": [], diff --git a/src/Entities/ClientEntityInterface.php b/src/Entities/ClientEntityInterface.php index 80cc70c8..971c6124 100644 --- a/src/Entities/ClientEntityInterface.php +++ b/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(); } diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 6a2ee795..406f6ab8 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -28,6 +28,11 @@ class AuthCodeGrant extends AbstractAuthorizeGrant */ private $authCodeTTL; + /** + * @var bool + */ + private $requireCodeChallengeForPublicClients = true; + /** * @param AuthCodeRepositoryInterface $authCodeRepository * @param RefreshTokenRepositoryInterface $refreshTokenRepository @@ -60,8 +65,29 @@ class AuthCodeGrant extends AbstractAuthorizeGrant ResponseTypeInterface $responseType, \DateInterval $accessTokenTTL ) { + $clientId = $this->getRequestParameter('client_id', $request, null); + + if ($clientId === null) { + throw OAuthServerException::invalidRequest('client_id'); + } + + if ($this->clientRepository->isClientConfidential($clientId)) { + $client = $this->validateClient($request); + } else { + $client = $this->clientRepository->getClientEntity( + $clientId, + $this->getIdentifier(), + null, + false + ); + } + // Validate request - $client = $this->validateClient($request); + + // HERE I ONLY WANT TO VALIDATE IF THE CLIENT IS CONFIDENTIAL! + + + $encryptedAuthCode = $this->getRequestParameter('code', $request, null); if ($encryptedAuthCode === null) { @@ -71,7 +97,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'); } @@ -86,6 +112,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'); } @@ -119,10 +146,9 @@ class AuthCodeGrant extends AbstractAuthorizeGrant throw OAuthServerException::invalidRequest('code', 'Cannot decrypt the authorization code'); } - // Validate code challenge if (!empty($authCodePayload->code_challenge)) { - $codeVerifier = $this->getRequestParameter('code_verifier', $request, null); + $codeVerifier = $this->getRequestParameter('code_verifier', $request, null); if ($codeVerifier === null) { throw OAuthServerException::invalidRequest('code_verifier'); @@ -164,6 +190,8 @@ class AuthCodeGrant extends AbstractAuthorizeGrant ); // @codeCoverageIgnoreEnd } + } else if ($this->$requireCodeChallengeForPublicClients && !$client->isConfidential()) { + throw OAuthServerException::invalidRequest('code_challenge', 'Code challenge must be provided for public clients'); } // Issue and persist access + refresh tokens diff --git a/src/Repositories/ClientRepositoryInterface.php b/src/Repositories/ClientRepositoryInterface.php index ba0610d5..9e863af8 100644 --- a/src/Repositories/ClientRepositoryInterface.php +++ b/src/Repositories/ClientRepositoryInterface.php @@ -28,4 +28,13 @@ interface ClientRepositoryInterface extends RepositoryInterface * @return ClientEntityInterface */ public function getClientEntity($clientIdentifier, $grantType = null, $clientSecret = null, $mustValidateSecret = true); + + /** + * Check if a client is confidential. + * + * @param string $clientIdentifier The client's identifier + * + * @return bool + */ + public function isClientConfidential($clientIdentifier); }