From 0d30e5d6effdb36eb85d309a2c0ded927ff25de1 Mon Sep 17 00:00:00 2001 From: Steven Jones Date: Tue, 20 Nov 2018 06:58:35 +0000 Subject: [PATCH 1/2] Add some bits for PKCE. --- src/OAuth2/Controller/AuthorizeController.php | 1 + .../OpenID/Controller/AuthorizeController.php | 32 +++++++++++++++++++ .../OpenID/ResponseType/AuthorizationCode.php | 8 ++--- .../Storage/AuthorizationCodeInterface.php | 2 +- src/OAuth2/Server.php | 3 +- 5 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/OAuth2/Controller/AuthorizeController.php b/src/OAuth2/Controller/AuthorizeController.php index 4bafb1d24..b2e12bb5d 100644 --- a/src/OAuth2/Controller/AuthorizeController.php +++ b/src/OAuth2/Controller/AuthorizeController.php @@ -87,6 +87,7 @@ public function __construct(ClientInterface $clientStorage, array $responseTypes 'enforce_state' => true, 'require_exact_redirect_uri' => true, 'redirect_status_code' => 302, + 'enforce_pkce' => false, ), $config); if (is_null($scopeUtil)) { diff --git a/src/OAuth2/OpenID/Controller/AuthorizeController.php b/src/OAuth2/OpenID/Controller/AuthorizeController.php index 54c5f9a63..db6302e7f 100644 --- a/src/OAuth2/OpenID/Controller/AuthorizeController.php +++ b/src/OAuth2/OpenID/Controller/AuthorizeController.php @@ -16,6 +16,16 @@ class AuthorizeController extends BaseAuthorizeController implements AuthorizeCo */ private $nonce; + /** + * @var mixed + */ + protected $code_challenge; + + /** + * @var mixed + */ + protected $code_challenge_method; + /** * Set not authorized response * @@ -65,6 +75,10 @@ protected function buildAuthorizeParameters($request, $response, $user_id) // add the nonce to return with the redirect URI $params['nonce'] = $this->nonce; + // Add PKCE code challenge. + $params['code_challenge'] = $this->code_challenge; + $params['code_challenge_method'] = $this->code_challenge_method; + return $params; } @@ -90,6 +104,24 @@ public function validateAuthorizeRequest(RequestInterface $request, ResponseInte $this->nonce = $nonce; + $code_challenge = $request->query('code_challenge'); + $code_challenge_method = $request->query('code_challenge_method'); + + if (!$code_challenge && $this->config['enforce_pkce']) { + $response->setError(400, 'missing_code_challenge', 'This application requires you specify provide a PKCE code challenge'); + + return false; + } + + if (!in_array($code_challenge_method, array('plain', 'S256'), TRUE) && $this->config['enforce_pkce']) { + $response->setError(400, 'missing_code_challenge_method', 'This application requires you specify provide a PKCE code challenge method'); + + return false; + } + + $this->code_challenge = $code_challenge; + $this->code_challenge_method = $code_challenge_method; + return true; } diff --git a/src/OAuth2/OpenID/ResponseType/AuthorizationCode.php b/src/OAuth2/OpenID/ResponseType/AuthorizationCode.php index b8ad41ffb..19e04104d 100644 --- a/src/OAuth2/OpenID/ResponseType/AuthorizationCode.php +++ b/src/OAuth2/OpenID/ResponseType/AuthorizationCode.php @@ -31,9 +31,9 @@ public function getAuthorizeResponse($params, $user_id = null) // build the URL to redirect to $result = array('query' => array()); - $params += array('scope' => null, 'state' => null, 'id_token' => null); + $params += array('scope' => null, 'state' => null, 'id_token' => null, 'code_challenge' => null, 'code_challenge_method' => null); - $result['query']['code'] = $this->createAuthorizationCode($params['client_id'], $user_id, $params['redirect_uri'], $params['scope'], $params['id_token']); + $result['query']['code'] = $this->createAuthorizationCode($params['client_id'], $user_id, $params['redirect_uri'], $params['scope'], $params['id_token'], $params['code_challenge'], $params['code_challenge_method']); if (isset($params['state'])) { $result['query']['state'] = $params['state']; @@ -56,10 +56,10 @@ public function getAuthorizeResponse($params, $user_id = null) * @see http://tools.ietf.org/html/rfc6749#section-4 * @ingroup oauth2_section_4 */ - public function createAuthorizationCode($client_id, $user_id, $redirect_uri, $scope = null, $id_token = null) + public function createAuthorizationCode($client_id, $user_id, $redirect_uri, $scope = null, $id_token = null, $code_challenge = null, $code_challenge_method = null) { $code = $this->generateAuthorizationCode(); - $this->storage->setAuthorizationCode($code, $client_id, $user_id, $redirect_uri, time() + $this->config['auth_code_lifetime'], $scope, $id_token); + $this->storage->setAuthorizationCode($code, $client_id, $user_id, $redirect_uri, time() + $this->config['auth_code_lifetime'], $scope, $id_token, $code_challenge, $code_challenge_method); return $code; } diff --git a/src/OAuth2/OpenID/Storage/AuthorizationCodeInterface.php b/src/OAuth2/OpenID/Storage/AuthorizationCodeInterface.php index 446cec928..8e0988ff4 100644 --- a/src/OAuth2/OpenID/Storage/AuthorizationCodeInterface.php +++ b/src/OAuth2/OpenID/Storage/AuthorizationCodeInterface.php @@ -33,5 +33,5 @@ interface AuthorizationCodeInterface extends BaseAuthorizationCodeInterface * * @ingroup oauth2_section_4 */ - public function setAuthorizationCode($code, $client_id, $user_id, $redirect_uri, $expires, $scope = null, $id_token = null); + public function setAuthorizationCode($code, $client_id, $user_id, $redirect_uri, $expires, $scope = null, $id_token = null, $code_challenge = null, $code_challenge_method = null); } diff --git a/src/OAuth2/Server.php b/src/OAuth2/Server.php index cf040c2bc..1fbc6666d 100644 --- a/src/OAuth2/Server.php +++ b/src/OAuth2/Server.php @@ -172,6 +172,7 @@ public function __construct($storage = array(), array $config = array(), array $ 'enforce_state' => true, 'require_exact_redirect_uri' => true, 'allow_implicit' => false, + 'enforce_pkce' => false, 'allow_credentials_in_request_body' => true, 'allow_public_clients' => true, 'always_issue_new_refresh_token' => false, @@ -577,7 +578,7 @@ protected function createDefaultAuthorizeController() } } - $config = array_intersect_key($this->config, array_flip(explode(' ', 'allow_implicit enforce_state require_exact_redirect_uri'))); + $config = array_intersect_key($this->config, array_flip(explode(' ', 'allow_implicit enforce_state require_exact_redirect_uri enforce_pkce'))); if ($this->config['use_openid_connect']) { return new OpenIDAuthorizeController($this->storages['client'], $this->responseTypes, $config, $this->getScopeUtil()); From ca5bfc65c8dcad73a7eb25a1eb266963c1311f22 Mon Sep 17 00:00:00 2001 From: Steven Jones Date: Tue, 20 Nov 2018 13:27:10 +0000 Subject: [PATCH 2/2] Verify PCKE challenge on the AuthorizationCode grant. --- src/OAuth2/GrantType/AuthorizationCode.php | 39 +++++++++++++++++++ .../OpenID/Controller/AuthorizeController.php | 20 +++++++--- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/OAuth2/GrantType/AuthorizationCode.php b/src/OAuth2/GrantType/AuthorizationCode.php index 784f6b3a3..059cdf78f 100644 --- a/src/OAuth2/GrantType/AuthorizationCode.php +++ b/src/OAuth2/GrantType/AuthorizationCode.php @@ -84,6 +84,45 @@ public function validateRequest(RequestInterface $request, ResponseInterface $re return false; } + // @TODO: Should we enforce presence of a non-falsy code challenge? + if (isset($authCode['code_challenge']) && $authCode['code_challenge']) { + if (!($code_verifier = $request->request('code_verifier'))) { + $response->setError(400, 'code_verifier_missing', "The PKCE code verifier parameter is required."); + + return false; + } + // Validate code_verifier according to RFC-7636 + // @see: https://tools.ietf.org/html/rfc7636#section-4.1 + elseif (preg_match('/^[A-Za-z0-9-._~]{43,128}$/', $code_verifier) !== 1) { + $response->setError(400, 'code_verifier_invalid', "The PKCE code verifier parameter is invalid."); + + return false; + } + else { + $code_verifier = $request->request('code_verifier'); + switch ($authCode['code_challenge_method']) { + case 'S256': + $code_verifier_hashed = strtr(rtrim(base64_encode(hash('sha256', $code_verifier, true)), '='), '+/', '-_'); + break; + + case 'plain': + $code_verifier_hashed = $code_verifier; + break; + + default: + $response->setError(400, 'code_challenge_method_invalid', "Unknown PKCE code challenge method."); + + return FALSE; + } + // @TODO: use hash_equals in recent versions of PHP. + if ($code_verifier_hashed !== $authCode['code_challenge']) { + $response->setError(400, 'code_verifier_mismatch', "The PKCE code verifier parameter does not match the code challenge."); + + return FALSE; + } + } + } + if (!isset($authCode['code'])) { $authCode['code'] = $code; // used to expire the code after the access token is granted } diff --git a/src/OAuth2/OpenID/Controller/AuthorizeController.php b/src/OAuth2/OpenID/Controller/AuthorizeController.php index db6302e7f..52e183bb3 100644 --- a/src/OAuth2/OpenID/Controller/AuthorizeController.php +++ b/src/OAuth2/OpenID/Controller/AuthorizeController.php @@ -107,16 +107,24 @@ public function validateAuthorizeRequest(RequestInterface $request, ResponseInte $code_challenge = $request->query('code_challenge'); $code_challenge_method = $request->query('code_challenge_method'); - if (!$code_challenge && $this->config['enforce_pkce']) { - $response->setError(400, 'missing_code_challenge', 'This application requires you specify provide a PKCE code challenge'); + if ($this->config['enforce_pkce']) { + if (!$code_challenge) { + $response->setError(400, 'missing_code_challenge', 'This application requires you provide a PKCE code challenge'); - return false; - } + return false; + } - if (!in_array($code_challenge_method, array('plain', 'S256'), TRUE) && $this->config['enforce_pkce']) { - $response->setError(400, 'missing_code_challenge_method', 'This application requires you specify provide a PKCE code challenge method'); + if (preg_match('/^[A-Za-z0-9-._~]{43,128}$/', $code_challenge) !== 1) { + $response->setError(400, 'invalid_code_challenge', 'The PKCE code challenge supplied is invalid'); return false; + } + + if (!in_array($code_challenge_method, array('plain', 'S256'), true)) { + $response->setError(400, 'missing_code_challenge_method', 'This application requires you specify a PKCE code challenge method'); + + return false; + } } $this->code_challenge = $code_challenge;