From 6ae9397631003da8d4df1576367fa1e026dd27ab Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Fri, 18 Feb 2022 07:42:58 -0600 Subject: [PATCH 01/16] WIP --- composer.json | 2 +- src/OAuth2.php | 62 +++++++++++++++++-- .../ServiceAccountCredentialsTest.php | 10 ++- tests/OAuth2Test.php | 15 +++-- 4 files changed, 75 insertions(+), 14 deletions(-) diff --git a/composer.json b/composer.json index 226017be69..0c6d50995a 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,7 @@ }, "require": { "php": ">=5.4", - "firebase/php-jwt": "~2.0|~3.0|~4.0|~5.0", + "firebase/php-jwt": "~2.0||~3.0||~4.0||~5.0||^6.0", "guzzlehttp/guzzle": "^5.3.1|^6.2.1|^7.0", "guzzlehttp/psr7": "^1.7|^2.0", "psr/http-message": "^1.0", diff --git a/src/OAuth2.php b/src/OAuth2.php index 9c7c659eda..a040bac265 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -26,6 +26,8 @@ use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\UriInterface; +use Firebase\JWT\JWT; +use Firebase\JWT\Key; /** * OAuth2 supports authentication by OAuth2 2-legged flows. @@ -1382,16 +1384,68 @@ private function coerceUri($uri) private function jwtDecode($idToken, $publicKey, $allowedAlgs) { if (class_exists('Firebase\JWT\JWT')) { - return \Firebase\JWT\JWT::decode($idToken, $publicKey, $allowedAlgs); + // Preserve Backwards Compatibility with firebase/php-jwt:6.0 by + // creating a Key object for every public key / alg combination. + if (class_exists('Firebase\JWT\Key') // True when using php-jwt v5.5 and v6.0 + && !defined('Firebase\JWT\JWT::ASN1_INTEGER') // False when using php-jwt v5.5 + && !$publicKey instanceof Key // True with previous (deprecated) usage + ) { + $keys = $this->getFirebaseJwtKeys($publicKey, $allowedAlgs); + + // Default exception if none are caught + $e = new \InvalidArgumentException('Key may not be empty'); + foreach ($keys as $key) { + try { + return JWT::decode($idToken, $key); + } catch (\Exception $e) { + // try next alg + } + } + throw $e; + } + return JWT::decode($idToken, $publicKey, $allowedAlgs); + } + + return JWT::decode($idToken, $publicKey, $allowedAlgs); + } + + private function getFirebaseJwtKeys($publicKey, $allowedAlgs) + { + if (count($allowedAlgs) > 1) { + throw new \InvalidArgumentException( + 'To have multiple allowed algorithms, You must provide an' + . ' array of Firebase\JWT\Key objects.' + . ' See https://github.com/firebase/php-jwt for more information.'); + } + + // If $allowedAlgs is empty, assume $publicKey is Key or Key[]. + if (count($allowedAlgs) == 0) { + return $publicKey; + } + $allowedAlg = array_pop($allowedAlgs); + if (is_array($publicKey)) { + // When publicKey is greater than 1, create keys with the single alg. + if (count($publicKey) > 1) { + $keys = array(); + foreach ($publicKey as $kid => $pubkey) { + $keys[$kid] = new Key($pubkey, $allowedAlg); + } + return $keys; + } + + // We have one key and one alg, create a key with them. + $key = new Key(array_pop($publicKey), $allowedAlgs); + } else { + $key = new Key($publicKey, $allowedAlgs); } - return \JWT::decode($idToken, $publicKey, $allowedAlgs); + return array($key); } private function jwtEncode($assertion, $signingKey, $signingAlgorithm, $signingKeyId = null) { if (class_exists('Firebase\JWT\JWT')) { - return \Firebase\JWT\JWT::encode( + return JWT::encode( $assertion, $signingKey, $signingAlgorithm, @@ -1399,7 +1453,7 @@ private function jwtEncode($assertion, $signingKey, $signingAlgorithm, $signingK ); } - return \JWT::encode($assertion, $signingKey, $signingAlgorithm, $signingKeyId); + return JWT::encode($assertion, $signingKey, $signingAlgorithm, $signingKeyId); } /** diff --git a/tests/Credentials/ServiceAccountCredentialsTest.php b/tests/Credentials/ServiceAccountCredentialsTest.php index 41eb65811f..9e93f03708 100644 --- a/tests/Credentials/ServiceAccountCredentialsTest.php +++ b/tests/Credentials/ServiceAccountCredentialsTest.php @@ -815,8 +815,16 @@ public function testJwtAccessFromApplicationDefault() if (class_exists('Firebase\JWT\JWT')) { $class = 'Firebase\JWT\JWT'; } + if (class_exists('Firebase\JWT\JWT')) { + $class = 'Firebase\JWT\JWT'; + } $jwt = new $class(); - $result = $jwt::decode($token, $key, ['RS256']); + if (class_exists('Firebase\JWT\Key')) { + $key = new \Firebase\JWT\Key($key, 'RS256'); + $result = $jwt::decode($token, $key); + } else { + $result = $jwt::decode($token, $key, ['RS256']); + } $this->assertEquals($authUri, $result->aud); } diff --git a/tests/OAuth2Test.php b/tests/OAuth2Test.php index 30fafcd582..16b2a11a9f 100644 --- a/tests/OAuth2Test.php +++ b/tests/OAuth2Test.php @@ -535,15 +535,14 @@ public function testCanHaveAdditionalClaims() $this->assertEquals($roundTrip->target_audience, $targetAud); } - private function jwtDecode() + private function jwtDecode($payload, $publicKey, $algs) { - $args = func_get_args(); - $class = 'JWT'; - if (class_exists('Firebase\JWT\JWT')) { - $class = 'Firebase\JWT\JWT'; - } + // use private jwtDecode method since the logic exists there already + $o = new OAuth2([]); + $method = new \ReflectionMethod($o, 'jwtDecode'); + $method->setAccessible(true); - return call_user_func_array("$class::decode", $args); + return $method->invoke($o, $payload, $publicKey, $algs); } } @@ -940,7 +939,7 @@ public function testFailsIfIdTokenIsInvalid() $not_a_jwt = 'not a jot'; $o = new OAuth2($testConfig); $o->setIdToken($not_a_jwt); - $o->verifyIdToken($this->publicKey); + $o->verifyIdToken($this->publicKey, ['RS256']); } /** From 9d1dc65316667b73920a0629f12d4e62f36fc387 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 4 Apr 2022 12:01:45 -0600 Subject: [PATCH 02/16] fix bad merge --- composer.json | 2 +- src/OAuth2.php | 36 ++++++++++++++++-------------------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/composer.json b/composer.json index 541c6716f4..6de5e209c8 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,7 @@ }, "require": { "php": ">=5.6", - "firebase/php-jwt": "~5.0", + "firebase/php-jwt": "^5.0||^6.0", "guzzlehttp/guzzle": "^6.2.1|^7.0", "guzzlehttp/psr7": "^1.7|^2.0", "psr/http-message": "^1.0", diff --git a/src/OAuth2.php b/src/OAuth2.php index 08fedaafca..02e3bb7228 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -17,7 +17,6 @@ namespace Google\Auth; -use Firebase\JWT\JWT; use Google\Auth\HttpHandler\HttpClientCache; use Google\Auth\HttpHandler\HttpHandlerFactory; use GuzzleHttp\Psr7\Query; @@ -1384,27 +1383,24 @@ private function coerceUri($uri) */ private function jwtDecode($idToken, $publicKey, $allowedAlgs) { - if (class_exists('Firebase\JWT\JWT')) { - // Preserve Backwards Compatibility with firebase/php-jwt:6.0 by - // creating a Key object for every public key / alg combination. - if (class_exists('Firebase\JWT\Key') // True when using php-jwt v5.5 and v6.0 - && !defined('Firebase\JWT\JWT::ASN1_INTEGER') // False when using php-jwt v5.5 - && !$publicKey instanceof Key // True with previous (deprecated) usage - ) { - $keys = $this->getFirebaseJwtKeys($publicKey, $allowedAlgs); - - // Default exception if none are caught - $e = new \InvalidArgumentException('Key may not be empty'); - foreach ($keys as $key) { - try { - return JWT::decode($idToken, $key); - } catch (\Exception $e) { - // try next alg - } + // Preserve Backwards Compatibility with firebase/php-jwt:6.0 by + // creating a Key object for every public key / alg combination. + if (class_exists('Firebase\JWT\Key') // True when using php-jwt v5.5 and v6.0 + && !defined('Firebase\JWT\JWT::ASN1_INTEGER') // False when using php-jwt v5.5 + && !$publicKey instanceof Key // True with previous (deprecated) usage + ) { + $keys = $this->getFirebaseJwtKeys($publicKey, $allowedAlgs); + + // Default exception if none are caught + $e = new \InvalidArgumentException('Key may not be empty'); + foreach ($keys as $key) { + try { + return JWT::decode($idToken, $key); + } catch (\Exception $e) { + // try next alg } - throw $e; } - return JWT::decode($idToken, $publicKey, $allowedAlgs); + throw $e; } return JWT::decode($idToken, $publicKey, $allowedAlgs); From 6bef9ae090989a9f5e8718c867a6f4e1d678d912 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 4 Apr 2022 12:12:49 -0600 Subject: [PATCH 03/16] better validation --- src/OAuth2.php | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index 02e3bb7228..7e5832b68e 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -1377,15 +1377,15 @@ private function coerceUri($uri) /** * @param string $idToken - * @param string|array|null $publicKey - * @param array $allowedAlgs + * @param Key|Key[]|string|array|null $publicKey + * @param string|array $allowedAlg * @return object */ private function jwtDecode($idToken, $publicKey, $allowedAlgs) { // Preserve Backwards Compatibility with firebase/php-jwt:6.0 by // creating a Key object for every public key / alg combination. - if (class_exists('Firebase\JWT\Key') // True when using php-jwt v5.5 and v6.0 + if (class_exists(Key::class) // True when using php-jwt v5.5 and v6.0 && !defined('Firebase\JWT\JWT::ASN1_INTEGER') // False when using php-jwt v5.5 && !$publicKey instanceof Key // True with previous (deprecated) usage ) { @@ -1406,20 +1406,30 @@ private function jwtDecode($idToken, $publicKey, $allowedAlgs) return JWT::decode($idToken, $publicKey, $allowedAlgs); } + /** + * @return Key|Keys[] + */ private function getFirebaseJwtKeys($publicKey, $allowedAlgs) { - if (count($allowedAlgs) > 1) { - throw new \InvalidArgumentException( - 'To have multiple allowed algorithms, You must provide an' - . ' array of Firebase\JWT\Key objects.' - . ' See https://github.com/firebase/php-jwt for more information.'); - } - // If $allowedAlgs is empty, assume $publicKey is Key or Key[]. - if (count($allowedAlgs) == 0) { + if (empty($allowedAlgs)) { return $publicKey; } - $allowedAlg = array_pop($allowedAlgs); + + if (is_string($allowedAlgs)) { + $allowedAlg = $allowedAlg; + } elseif (is_array($allowedAlgs)) { + if (count($allowedAlgs) > 1) { + throw new \InvalidArgumentException( + 'To have multiple allowed algorithms, You must provide an' + . ' array of Firebase\JWT\Key objects.' + . ' See https://github.com/firebase/php-jwt for more information.'); + } + $allowedAlg = array_pop($allowedAlgs); + } else { + throw new \InvalidArgumentException('$allowedAlgs must be a string or array.'); + } + if (is_array($publicKey)) { // When publicKey is greater than 1, create keys with the single alg. if (count($publicKey) > 1) { From 10dbff725869ecaca5a38d005ce1042f908f1395 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 4 Apr 2022 12:25:46 -0600 Subject: [PATCH 04/16] fix tests --- src/OAuth2.php | 11 +++++++---- tests/OAuth2Test.php | 31 ++++++++++++++++--------------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index 7e5832b68e..071c9ae51c 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -380,12 +380,15 @@ public function __construct(array $config) * newer versions, if a public key is not given, this method will throw an * `\InvalidArgumentException`. * - * @param string $publicKey The public key to use to authenticate the token - * @param array $allowed_algs List of supported verification algorithms + * @param string|Key|Key[] $publicKey The public key to use to authenticate the token + * @param string|array $allowed_algs algorithm or array of supported verification algorithms. + * IMPORTANT: providing more than one algorithm will throw an exception, use the Key + * object instead. * @throws \DomainException if the token is missing an audience. * @throws \DomainException if the audience does not match the one set in * the OAuth2 class instance. * @throws \UnexpectedValueException If the token is invalid + * @throws \InvalidArgumentException If more than one value for allowed_algs is supplied * @throws SignatureInvalidException If the signature is invalid. * @throws BeforeValidException If the token is not yet valid. * @throws ExpiredException If the token has expired. @@ -1441,9 +1444,9 @@ private function getFirebaseJwtKeys($publicKey, $allowedAlgs) } // We have one key and one alg, create a key with them. - $key = new Key(array_pop($publicKey), $allowedAlgs); + $key = new Key(array_pop($publicKey), $allowedAlg); } else { - $key = new Key($publicKey, $allowedAlgs); + $key = new Key($publicKey, $allowedAlg); } return array($key); diff --git a/tests/OAuth2Test.php b/tests/OAuth2Test.php index ab9375784a..c94911f0be 100644 --- a/tests/OAuth2Test.php +++ b/tests/OAuth2Test.php @@ -19,6 +19,7 @@ use DomainException; use Firebase\JWT\JWT; +use Firebase\JWT\Key; use Google\Auth\OAuth2; use GuzzleHttp\Psr7\Query; use GuzzleHttp\Psr7\Utils; @@ -442,15 +443,15 @@ public function testFailsWithMissingSigningAlgorithm() public function testCanHS256EncodeAValidPayloadWithSigningKeyId() { $testConfig = $this->signingMinimal; - $keys = array( - 'example_key_id1' => 'example_key1', - 'example_key_id2' => 'example_key2' - ); - $testConfig['signingKey'] = $keys['example_key_id2']; + $keys = [ + 'example_key_id1' => new Key('example_key1', 'HS256'), + 'example_key_id2' => new Key('example_key2', 'HS256'), + ]; + $testConfig['signingKey'] = $keys['example_key_id2']->getKeyMaterial(); $testConfig['signingKeyId'] = 'example_key_id2'; $o = new OAuth2($testConfig); $payload = $o->toJwt(); - $roundTrip = JWT::decode($payload, $keys, array('HS256')); + $roundTrip = JWT::decode($payload, $keys); $this->assertEquals($roundTrip->iss, $testConfig['issuer']); $this->assertEquals($roundTrip->aud, $testConfig['audience']); $this->assertEquals($roundTrip->scope, $testConfig['scope']); @@ -459,16 +460,16 @@ public function testCanHS256EncodeAValidPayloadWithSigningKeyId() public function testFailDecodeWithoutSigningKeyId() { $testConfig = $this->signingMinimal; - $keys = array( - 'example_key_id1' => 'example_key1', - 'example_key_id2' => 'example_key2' - ); - $testConfig['signingKey'] = $keys['example_key_id2']; + $keys = [ + 'example_key_id1' => new Key('example_key1', 'HS256'), + 'example_key_id2' => new Key('example_key2', 'HS256'), + ]; + $testConfig['signingKey'] = $keys['example_key_id2']->getKeyMaterial(); $o = new OAuth2($testConfig); $payload = $o->toJwt(); try { - JWT::decode($payload, $keys, array('HS256')); + JWT::decode($payload, $keys); } catch (\Exception $e) { // Workaround: In old JWT versions throws DomainException $this->assertTrue( @@ -485,7 +486,7 @@ public function testCanHS256EncodeAValidPayload() $testConfig = $this->signingMinimal; $o = new OAuth2($testConfig); $payload = $o->toJwt(); - $roundTrip = JWT::decode($payload, $testConfig['signingKey'], array('HS256')); + $roundTrip = JWT::decode($payload, new Key($testConfig['signingKey'], 'HS256')); $this->assertEquals($roundTrip->iss, $testConfig['issuer']); $this->assertEquals($roundTrip->aud, $testConfig['audience']); $this->assertEquals($roundTrip->scope, $testConfig['scope']); @@ -500,7 +501,7 @@ public function testCanRS256EncodeAValidPayload() $o->setSigningAlgorithm('RS256'); $o->setSigningKey($privateKey); $payload = $o->toJwt(); - $roundTrip = JWT::decode($payload, $publicKey, array('RS256')); + $roundTrip = JWT::decode($payload, new Key($publicKey, 'RS256')); $this->assertEquals($roundTrip->iss, $testConfig['issuer']); $this->assertEquals($roundTrip->aud, $testConfig['audience']); $this->assertEquals($roundTrip->scope, $testConfig['scope']); @@ -517,7 +518,7 @@ public function testCanHaveAdditionalClaims() $o->setSigningAlgorithm('RS256'); $o->setSigningKey($privateKey); $payload = $o->toJwt(); - $roundTrip = JWT::decode($payload, $publicKey, array('RS256')); + $roundTrip = JWT::decode($payload, new Key($publicKey, 'RS256')); $this->assertEquals($roundTrip->target_audience, $targetAud); } } From 504d4695313404e61db518de79d6e4306dad6b74 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 4 Apr 2022 12:29:17 -0600 Subject: [PATCH 05/16] test, style, and docs fix --- src/OAuth2.php | 14 ++++++-------- .../Credentials/ServiceAccountCredentialsTest.php | 3 ++- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index 071c9ae51c..468e639d59 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -17,6 +17,8 @@ namespace Google\Auth; +use Firebase\JWT\JWT; +use Firebase\JWT\Key; use Google\Auth\HttpHandler\HttpClientCache; use Google\Auth\HttpHandler\HttpHandlerFactory; use GuzzleHttp\Psr7\Query; @@ -26,8 +28,6 @@ use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\UriInterface; -use Firebase\JWT\JWT; -use Firebase\JWT\Key; /** * OAuth2 supports authentication by OAuth2 2-legged flows. @@ -375,15 +375,13 @@ public function __construct(array $config) * - otherwise returns the payload in the idtoken as a PHP object. * * The behavior of this method varies depending on the version of - * `firebase/php-jwt` you are using. In versions lower than 3.0.0, if - * `$publicKey` is null, the key is decoded without being verified. In - * newer versions, if a public key is not given, this method will throw an - * `\InvalidArgumentException`. + * `firebase/php-jwt` you are using. In versions 6.0 and above, you cannot + * provide multiple $allowed_algs, and instead must provide an array of Key + * objects as the $publicKey. * * @param string|Key|Key[] $publicKey The public key to use to authenticate the token * @param string|array $allowed_algs algorithm or array of supported verification algorithms. - * IMPORTANT: providing more than one algorithm will throw an exception, use the Key - * object instead. + * Providing more than one algorithm will throw an exception. * @throws \DomainException if the token is missing an audience. * @throws \DomainException if the audience does not match the one set in * the OAuth2 class instance. diff --git a/tests/Credentials/ServiceAccountCredentialsTest.php b/tests/Credentials/ServiceAccountCredentialsTest.php index d30bf40da5..e4dbb0fb9a 100644 --- a/tests/Credentials/ServiceAccountCredentialsTest.php +++ b/tests/Credentials/ServiceAccountCredentialsTest.php @@ -19,6 +19,7 @@ use DomainException; use Firebase\JWT\JWT; +use Firebase\JWT\Key; use Google\Auth\ApplicationDefaultCredentials; use Google\Auth\Credentials\ServiceAccountCredentials; use Google\Auth\Credentials\ServiceAccountJwtAccessCredentials; @@ -799,7 +800,7 @@ public function testJwtAccessFromApplicationDefault() $this->assertArrayHasKey('authorization', $metadata); $token = str_replace('Bearer ', '', $metadata['authorization'][0]); $key = file_get_contents(__DIR__ . '/../fixtures3/key.pub'); - $result = JWT::decode($token, $key, ['RS256']); + $result = JWT::decode($token, new Key($key, 'RS256')); $this->assertEquals($authUri, $result->aud); } From f172065027b658691aa87cebd64b6fe31d922e0c Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 4 Apr 2022 12:30:17 -0600 Subject: [PATCH 06/16] require at least php-jwt 5.5 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 6de5e209c8..487d1759d8 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,7 @@ }, "require": { "php": ">=5.6", - "firebase/php-jwt": "^5.0||^6.0", + "firebase/php-jwt": "^5.5||^6.0", "guzzlehttp/guzzle": "^6.2.1|^7.0", "guzzlehttp/psr7": "^1.7|^2.0", "psr/http-message": "^1.0", From d5fd7f39682df6d80675f9264aec28a1db0b8cc6 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 4 Apr 2022 12:50:45 -0600 Subject: [PATCH 07/16] remove attempt to use 5.5 in a compatible way --- src/OAuth2.php | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index 468e639d59..aa6adc5f5b 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -1384,27 +1384,18 @@ private function coerceUri($uri) */ private function jwtDecode($idToken, $publicKey, $allowedAlgs) { - // Preserve Backwards Compatibility with firebase/php-jwt:6.0 by - // creating a Key object for every public key / alg combination. - if (class_exists(Key::class) // True when using php-jwt v5.5 and v6.0 - && !defined('Firebase\JWT\JWT::ASN1_INTEGER') // False when using php-jwt v5.5 - && !$publicKey instanceof Key // True with previous (deprecated) usage - ) { - $keys = $this->getFirebaseJwtKeys($publicKey, $allowedAlgs); - - // Default exception if none are caught - $e = new \InvalidArgumentException('Key may not be empty'); - foreach ($keys as $key) { - try { - return JWT::decode($idToken, $key); - } catch (\Exception $e) { - // try next alg - } + $keys = $this->getFirebaseJwtKeys($publicKey, $allowedAlgs); + + // Default exception if none are caught + $e = new \InvalidArgumentException('Key may not be empty'); + foreach ($keys as $key) { + try { + return JWT::decode($idToken, $key); + } catch (\Exception $e) { + // try next alg } - throw $e; } - - return JWT::decode($idToken, $publicKey, $allowedAlgs); + throw $e; } /** From 38e6325f59bfa3b06ec103d913aeac5fbdfaf919 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 4 Apr 2022 13:02:29 -0600 Subject: [PATCH 08/16] casts as array --- src/OAuth2.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index aa6adc5f5b..28f05d24c2 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -1399,13 +1399,13 @@ private function jwtDecode($idToken, $publicKey, $allowedAlgs) } /** - * @return Key|Keys[] + * @return Keys[] */ private function getFirebaseJwtKeys($publicKey, $allowedAlgs) { // If $allowedAlgs is empty, assume $publicKey is Key or Key[]. if (empty($allowedAlgs)) { - return $publicKey; + return (array) $publicKey; } if (is_string($allowedAlgs)) { From 5453ddbe2c0613f19f2c7f1150866644d8e774a2 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Wed, 6 Apr 2022 09:00:15 -0600 Subject: [PATCH 09/16] simplify JWT logic --- src/OAuth2.php | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index 28f05d24c2..02a74107a8 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -457,7 +457,7 @@ public function toJwt(array $config = []) } $assertion += $this->getAdditionalClaims(); - return $this->jwtEncode( + return JWT::encode( $assertion, $this->getSigningKey(), $this->getSigningAlgorithm(), @@ -1424,31 +1424,14 @@ private function getFirebaseJwtKeys($publicKey, $allowedAlgs) if (is_array($publicKey)) { // When publicKey is greater than 1, create keys with the single alg. - if (count($publicKey) > 1) { - $keys = array(); - foreach ($publicKey as $kid => $pubkey) { - $keys[$kid] = new Key($pubkey, $allowedAlg); - } - return $keys; + $keys = []; + foreach ($publicKey as $kid => $pubkey) { + $keys[$kid] = new Key($pubkey, $allowedAlg); } - - // We have one key and one alg, create a key with them. - $key = new Key(array_pop($publicKey), $allowedAlg); - } else { - $key = new Key($publicKey, $allowedAlg); + return $keys; } - return array($key); - } - - private function jwtEncode($assertion, $signingKey, $signingAlgorithm, $signingKeyId = null) - { - return JWT::encode( - $assertion, - $signingKey, - $signingAlgorithm, - $signingKeyId - ); + return [new Key($publicKey, $allowedAlg)]; } /** From 316acd33b8a42af7f33e68c749e3a27cbeea9643 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 12 Apr 2022 09:32:08 -0500 Subject: [PATCH 10/16] Update src/OAuth2.php Co-authored-by: David Supplee --- src/OAuth2.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index 02a74107a8..a91f78e560 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -1378,7 +1378,7 @@ private function coerceUri($uri) /** * @param string $idToken - * @param Key|Key[]|string|array|null $publicKey + * @param Key|Key[]|string|array $publicKey * @param string|array $allowedAlg * @return object */ From a6bc81c0a90ac6217479015a65f648c2d1907c35 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 12 Apr 2022 09:32:15 -0500 Subject: [PATCH 11/16] Update src/OAuth2.php Co-authored-by: David Supplee --- src/OAuth2.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index a91f78e560..a01f706555 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -1379,7 +1379,7 @@ private function coerceUri($uri) /** * @param string $idToken * @param Key|Key[]|string|array $publicKey - * @param string|array $allowedAlg + * @param string|array $allowedAlgs * @return object */ private function jwtDecode($idToken, $publicKey, $allowedAlgs) From 328730f064a2a00d338f9c0debcc2320ea78de53 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 12 Apr 2022 08:35:16 -0700 Subject: [PATCH 12/16] Update OAuth2.php --- src/OAuth2.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index 61aeff70a3..cd99393583 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -1443,8 +1443,8 @@ private function coerceUri($uri) /** * @param string $idToken - * @param Key|Key[]|string|array $publicKey - * @param string|array $allowedAlgs + * @param Key|Key[]|string|string[] $publicKey + * @param string|string[] $allowedAlgs * @return object */ private function jwtDecode($idToken, $publicKey, $allowedAlgs) @@ -1464,7 +1464,9 @@ private function jwtDecode($idToken, $publicKey, $allowedAlgs) } /** - * @return Keys[] + * @param string|string[] $publicKey + * @param string|string[] allowedAlgs + * @return \Firebase\JWT\Keys[] */ private function getFirebaseJwtKeys($publicKey, $allowedAlgs) { From ddfd838239450308051630b121b62063aa0a01fd Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 12 Apr 2022 11:13:04 -0500 Subject: [PATCH 13/16] fix phpstan, add tests --- src/OAuth2.php | 36 +++++++++++++++++++++++++------ tests/OAuth2Test.php | 51 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index cd99393583..09a7296755 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -1464,17 +1464,35 @@ private function jwtDecode($idToken, $publicKey, $allowedAlgs) } /** - * @param string|string[] $publicKey - * @param string|string[] allowedAlgs - * @return \Firebase\JWT\Keys[] + * @param Key|Key[]|string|string[] $publicKey + * @param string|string[] $allowedAlgs + * @return Key[] */ private function getFirebaseJwtKeys($publicKey, $allowedAlgs) { - // If $allowedAlgs is empty, assume $publicKey is Key or Key[]. + // If $publicKey is instance of Key, return it + if ($publicKey instanceof Key) { + return [$publicKey]; + } + + // If $allowedAlgs is empty, $publicKey must be Key or Key[]. if (empty($allowedAlgs)) { - return (array) $publicKey; + $pubKeys = []; + foreach ((array) $publicKey as $kid => $pubKey) { + if (!$pubKey instanceof Key) { + throw new \InvalidArgumentException(sprintf( + 'When allowed algorithms is empty, the public key must' + . 'be an instance of %s or an array of %s objects', + Key::class, + Key::class + )); + } + $pubKeys[$kid] = $pubKey; + } + return $pubKeys; } + $allowedAlg = null; if (is_string($allowedAlgs)) { $allowedAlg = $allowedAlg; } elseif (is_array($allowedAlgs)) { @@ -1486,14 +1504,18 @@ private function getFirebaseJwtKeys($publicKey, $allowedAlgs) } $allowedAlg = array_pop($allowedAlgs); } else { - throw new \InvalidArgumentException('$allowedAlgs must be a string or array.'); + throw new \InvalidArgumentException('allowed algorithms must be a string or array.'); } if (is_array($publicKey)) { // When publicKey is greater than 1, create keys with the single alg. $keys = []; foreach ($publicKey as $kid => $pubkey) { - $keys[$kid] = new Key($pubkey, $allowedAlg); + if ($pubkey instanceof Key) { + $keys[$kid] = $pubkey; + } else { + $keys[$kid] = new Key($pubkey, $allowedAlg); + } } return $keys; } diff --git a/tests/OAuth2Test.php b/tests/OAuth2Test.php index c94911f0be..5429616634 100644 --- a/tests/OAuth2Test.php +++ b/tests/OAuth2Test.php @@ -944,6 +944,57 @@ public function testFailsIfAudienceIsWrong() $o->verifyIdToken($this->publicKey, ['RS256']); } + public function testFailsWithStringPublicKeyAndAllowedAlgsGreaterThanOne() + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('To have multiple allowed algorithms'); + + $testConfig = $this->verifyIdTokenMinimal; + $not_a_jwt = 'not a jot'; + $o = new OAuth2($testConfig); + $o->setIdToken($not_a_jwt); + $o->verifyIdToken($this->publicKey, ['RS256', 'ES256']); + } + + public function testFailsWithStringPublicKeyAndNoAllowedAlgs() + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('When allowed algorithms is empty'); + + $testConfig = $this->verifyIdTokenMinimal; + $not_a_jwt = 'not a jot'; + $o = new OAuth2($testConfig); + $o->setIdToken($not_a_jwt); + $o->verifyIdToken($this->publicKey, []); + } + + public function testFailsWithStringInPublicKeyArrayAndNoAllowedAlgs() + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('When allowed algorithms is empty'); + + $testConfig = $this->verifyIdTokenMinimal; + $not_a_jwt = 'not a jot'; + $o = new OAuth2($testConfig); + $o->setIdToken($not_a_jwt); + $o->verifyIdToken([ + new Key($this->publicKey, 'RS256'), + $this->publicKey, + ], []); + } + + public function testFailsWithInalidAllowedAlgs() + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('allowed algorithms must be a string or array'); + + $testConfig = $this->verifyIdTokenMinimal; + $not_a_jwt = 'not a jot'; + $o = new OAuth2($testConfig); + $o->setIdToken($not_a_jwt); + $o->verifyIdToken($this->publicKey, 123); + } + public function testShouldReturnAValidIdToken() { $testConfig = $this->verifyIdTokenMinimal; From ebd8e6bfa05391b93d3af971b38d6180a1b6bc60 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 12 Apr 2022 11:15:39 -0500 Subject: [PATCH 14/16] add helpful comment for default exception --- src/OAuth2.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index 09a7296755..bc4ae19d58 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -1451,7 +1451,9 @@ private function jwtDecode($idToken, $publicKey, $allowedAlgs) { $keys = $this->getFirebaseJwtKeys($publicKey, $allowedAlgs); - // Default exception if none are caught + // Default exception if none are caught. We are using the same exception + // class and message from firebase/php-jwt to preserve backwards + // compatibility. $e = new \InvalidArgumentException('Key may not be empty'); foreach ($keys as $key) { try { From a331a88bf3217ad623ef3bdb01ba34a0e2a9504b Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Wed, 13 Apr 2022 13:04:46 -0700 Subject: [PATCH 15/16] nit - variable name consistency --- src/OAuth2.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/OAuth2.php b/src/OAuth2.php index bc4ae19d58..76075bdef7 100644 --- a/src/OAuth2.php +++ b/src/OAuth2.php @@ -1479,7 +1479,7 @@ private function getFirebaseJwtKeys($publicKey, $allowedAlgs) // If $allowedAlgs is empty, $publicKey must be Key or Key[]. if (empty($allowedAlgs)) { - $pubKeys = []; + $keys = []; foreach ((array) $publicKey as $kid => $pubKey) { if (!$pubKey instanceof Key) { throw new \InvalidArgumentException(sprintf( @@ -1489,9 +1489,9 @@ private function getFirebaseJwtKeys($publicKey, $allowedAlgs) Key::class )); } - $pubKeys[$kid] = $pubKey; + $keys[$kid] = $pubKey; } - return $pubKeys; + return $keys; } $allowedAlg = null; @@ -1512,11 +1512,11 @@ private function getFirebaseJwtKeys($publicKey, $allowedAlgs) if (is_array($publicKey)) { // When publicKey is greater than 1, create keys with the single alg. $keys = []; - foreach ($publicKey as $kid => $pubkey) { - if ($pubkey instanceof Key) { - $keys[$kid] = $pubkey; + foreach ($publicKey as $kid => $pubKey) { + if ($pubKey instanceof Key) { + $keys[$kid] = $pubKey; } else { - $keys[$kid] = new Key($pubkey, $allowedAlg); + $keys[$kid] = new Key($pubKey, $allowedAlg); } } return $keys; From a4264dba0a7e80786d784a858ee9be0cff84f7a0 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Wed, 13 Apr 2022 13:08:07 -0700 Subject: [PATCH 16/16] typo fix --- tests/OAuth2Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/OAuth2Test.php b/tests/OAuth2Test.php index 5429616634..6a08471fb3 100644 --- a/tests/OAuth2Test.php +++ b/tests/OAuth2Test.php @@ -983,7 +983,7 @@ public function testFailsWithStringInPublicKeyArrayAndNoAllowedAlgs() ], []); } - public function testFailsWithInalidAllowedAlgs() + public function testFailsWithInvalidTypeForAllowedAlgs() { $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('allowed algorithms must be a string or array');