diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index e1e09df0..c3824bdf 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -95,6 +95,7 @@ + diff --git a/src/Helpers/JWKFetcher.php b/src/Helpers/JWKFetcher.php index 6a263d8c..37c4f7e0 100644 --- a/src/Helpers/JWKFetcher.php +++ b/src/Helpers/JWKFetcher.php @@ -6,6 +6,9 @@ use Auth0\SDK\Helpers\Cache\CacheHandler; use Auth0\SDK\Helpers\Cache\NoCacheHandler; +use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Exception\ClientException; + /** * Class JWKFetcher. * @@ -17,7 +20,7 @@ class JWKFetcher /** * Cache handler or null for no caching. * - * @var CacheHandler|NoCacheHandler|null + * @var CacheHandler|null */ private $cache = null; @@ -59,101 +62,137 @@ protected function convertCertToPem($cert) return $output; } + // phpcs:disable /** - * Fetch x509 cert for RS256 token decoding. + * Appends the default JWKS path to a token issuer to return all keys from a JWKS. + * TODO: Deprecate, use $this->getJwksX5c() instead and explain why/how. + * + * @param string $iss * - * @param string $domain Base domain for the JWKS, including scheme. - * @param string|null $kid Kid to use. - * @param string $path Path to the JWKS from the $domain. + * @return array|mixed|null * - * @return mixed + * @throws \Exception * - * @throws \Exception If the Guzzle HTTP client cannot complete the request. + * @codeCoverageIgnore */ - public function fetchKeys($domain, $kid = null, $path = '.well-known/jwks.json') + public function fetchKeys($iss) { - $jwks_url = $domain.$path; - - // Check for a cached JWKS value. - $secret = $this->cache->get($jwks_url); - if (! is_null($secret)) { - return $secret; - } - - $secret = []; + $url = "{$iss}.well-known/jwks.json"; - $jwks = $this->getJwks( $domain, $path ); + if (($secret = $this->cache->get($url)) === null) { + $secret = []; - if (! is_array( $jwks['keys'] ) || empty( $jwks['keys'] )) { - return $secret; - } - - // No kid passed so get the kid of the first JWK. - if (is_null($kid)) { - $kid = $this->getProp( $jwks, 'kid' ); - } + $request = new RequestBuilder([ + 'domain' => $iss, + 'basePath' => '.well-known/jwks.json', + 'method' => 'GET', + 'guzzleOptions' => $this->guzzleOptions + ]); + $jwks = $request->call(); - $x5c = $this->getProp( $jwks, 'x5c', $kid ); + foreach ($jwks['keys'] as $key) { + $secret[$key['kid']] = $this->convertCertToPem($key['x5c'][0]); + } - // Need the kid and x5c for a well-formed return value. - if (! is_null($kid) && ! is_null($x5c)) { - $secret[$kid] = $this->convertCertToPem($x5c); - $this->cache->set($jwks_url, $secret); + $this->cache->set($url, $secret); } return $secret; } + // phpcs:enable /** - * Get a specific property from a JWKS using a key, if provided. + * Fetch x509 cert for RS256 token decoding. * - * @param array $jwks JWKS to parse. - * @param string $prop Property to retrieve. - * @param null|string $kid Kid to check. + * @param string $jwks_url URL to the JWKS. + * @param string|null $kid Key ID to use; returns first JWK if $kid is null or empty. * - * @return null|string + * @return string|null - Null if an x5c key could not be found for a key ID or if the JWKS is empty/invalid. */ - public function getProp(array $jwks, $prop, $kid = null) + public function requestJwkX5c($jwks_url, $kid = null) { - $r_key = null; - if (! $kid) { - // No kid indicated, get the first entry. - $r_key = $jwks['keys'][0]; - } else { - // Iterate through the JWKS for the correct kid. - foreach ($jwks['keys'] as $key) { - if (isset($key['kid']) && $key['kid'] === $kid) { - $r_key = $key; - break; - } - } + $cache_key = $jwks_url.'|'.(string) $kid; + + $x5c = $this->cache->get($cache_key); + if (! is_null($x5c)) { + return $x5c; } - // If a key was not found or the property does not exist, return. - if (is_null($r_key) || ! isset($r_key[$prop])) { + $jwks = $this->requestJwks($jwks_url); + $jwk = $this->findJwk($jwks, $kid); + + if ($this->subArrayHasEmptyFirstItem($jwk, 'x5c')) { return null; } - // If the value is an array, get the first element. - return is_array( $r_key[$prop] ) ? $r_key[$prop][0] : $r_key[$prop]; + $x5c = $this->convertCertToPem($jwk['x5c'][0]); + $this->cache->set($cache_key, $x5c); + return $x5c; } /** - * Get a JWKS given a domain and path to call. + * Get a JWKS from a specific URL. * - * @param string $domain Base domain for the JWKS, including scheme. - * @param string $path Path to the JWKS from the $domain. + * @param string $jwks_url URL to the JWKS. * * @return mixed|string + * + * @throws RequestException If $jwks_url is empty or malformed. + * @throws ClientException If the JWKS cannot be retrieved. + * + * @codeCoverageIgnore */ - protected function getJwks($domain, $path) + protected function requestJwks($jwks_url) { $request = new RequestBuilder([ - 'domain' => $domain, - 'basePath' => $path, + 'domain' => $jwks_url, 'method' => 'GET', 'guzzleOptions' => $this->guzzleOptions ]); return $request->call(); } + + /** + * Get a JWK from a JWKS using a key ID, if provided. + * + * @param array $jwks JWKS to parse. + * @param null|string $kid Key ID to return; returns first JWK if $kid is null or empty. + * + * @return array|null Null if the keys array is empty or if the key ID is not found. + * + * @codeCoverageIgnore + */ + private function findJwk(array $jwks, $kid = null) + { + if ($this->subArrayHasEmptyFirstItem($jwks, 'keys')) { + return null; + } + + if (! $kid) { + return $jwks['keys'][0]; + } + + foreach ($jwks['keys'] as $key) { + if (isset($key['kid']) && $key['kid'] === $kid) { + return $key; + } + } + + return null; + } + + /** + * Check if an array within an array has a non-empty first item. + * + * @param array|null $array Main array to check. + * @param string $key Key pointing to a sub-array. + * + * @return boolean + * + * @codeCoverageIgnore + */ + private function subArrayHasEmptyFirstItem($array, $key) + { + return empty($array) || ! is_array($array[$key]) || empty($array[$key][0]); + } } diff --git a/src/JWTVerifier.php b/src/JWTVerifier.php index 1aa90d84..73218435 100644 --- a/src/JWTVerifier.php +++ b/src/JWTVerifier.php @@ -23,6 +23,8 @@ class JWTVerifier protected $secret_base64_encoded = null; + protected $jwks_path = '.well-known/jwks.json'; + /** * JWTVerifier Constructor. * @@ -87,6 +89,10 @@ public function __construct($config) $this->client_secret = $config['client_secret']; } + if (isset($config['jwks_path'])) { + $this->jwks_path = (string) $config['jwks_path']; + } + $this->JWKFetcher = new JWKFetcher($cache, $guzzleOptions); } @@ -125,7 +131,8 @@ public function verifyAndDecode($jwt) throw new CoreException("We can't trust on a token issued by: `{$body->iss}`."); } - $secret = $this->JWKFetcher->fetchKeys($body->iss); + $jwks_url = $body->iss.$this->jwks_path; + $secret = $this->JWKFetcher->fetchKeys($jwks_url); } else if ($head->alg === 'HS256') { if ($this->secret_base64_encoded) { $secret = JWT::urlsafeB64Decode($this->client_secret); diff --git a/src/Store/SessionStore.php b/src/Store/SessionStore.php index 2707cfc1..caad5fba 100644 --- a/src/Store/SessionStore.php +++ b/src/Store/SessionStore.php @@ -122,9 +122,10 @@ public function delete($key) public function getSessionKeyName($key) { $key_name = $key; - if ( ! empty( $this->session_base_name ) ) { + if (! empty( $this->session_base_name )) { $key_name = $this->session_base_name.'_'.$key_name; } + return $key_name; } } diff --git a/tests/Helpers/JWKFetcherTest.php b/tests/Helpers/JWKFetcherTest.php index 35857eeb..9cc019e3 100644 --- a/tests/Helpers/JWKFetcherTest.php +++ b/tests/Helpers/JWKFetcherTest.php @@ -1,7 +1,8 @@ getStub(); - $keys = $jwksFetcher->fetchKeys( 'https://localhost/' ); + $pem = $jwksFetcher->requestJwkX5c( 'https://localhost/.well-known/jwks.json' ); - $this->assertCount(1, $keys); + $this->assertNotEmpty($pem); - $pem_parts = $this->getPemParts( $keys ); + $pem_parts = explode( PHP_EOL, $pem ); $this->assertEquals( 4, count($pem_parts) ); $this->assertEquals( '-----BEGIN CERTIFICATE-----', $pem_parts[0] ); @@ -36,15 +37,15 @@ public function testFetchKeysWithoutKid() * * @return void */ - public function testFetchKeysWithKid() + public function testGetJwksX5cWithKid() { $jwksFetcher = $this->getStub(); - $keys = $jwksFetcher->fetchKeys( 'https://localhost/', '__test_kid_2__' ); + $pem = $jwksFetcher->requestJwkX5c( 'https://localhost/.well-known/jwks.json', '__test_kid_2__' ); - $this->assertCount(1, $keys); + $this->assertNotEmpty($pem); - $pem_parts = $this->getPemParts( $keys ); + $pem_parts = explode( PHP_EOL, $pem ); $this->assertEquals( 4, count($pem_parts) ); $this->assertEquals( '-----BEGIN CERTIFICATE-----', $pem_parts[0] ); @@ -57,15 +58,15 @@ public function testFetchKeysWithKid() * * @return void */ - public function testFetchKeysWithPath() + public function testGetJwksX5cWithPath() { $jwksFetcher = $this->getStub(); - $keys = $jwksFetcher->fetchKeys( 'https://localhost/', '__test_custom_kid__', '.custom/jwks.json' ); + $pem = $jwksFetcher->requestJwkX5c( 'https://localhost/.custom/jwks.json', '__test_custom_kid__' ); - $this->assertCount(1, $keys); + $this->assertNotEmpty($pem); - $pem_parts = $this->getPemParts( $keys ); + $pem_parts = explode( PHP_EOL, $pem ); $this->assertEquals( 4, count( $pem_parts ) ); $this->assertEquals( '-----BEGIN CERTIFICATE-----', $pem_parts[0] ); @@ -78,13 +79,13 @@ public function testFetchKeysWithPath() * * @return void */ - public function testFetchKeysNoX5c() + public function testGetJwksX5cNoX5c() { $jwksFetcher = $this->getStub(); - $keys = $jwksFetcher->fetchKeys( 'https://localhost/', '__no_x5c_test_kid_2__' ); + $pem = $jwksFetcher->requestJwkX5c( 'https://localhost/.custom/jwks.json', '__no_x5c_test_kid_2__' ); - $this->assertEmpty($keys); + $this->assertEmpty($pem); } /** @@ -118,27 +119,55 @@ public function testConvertCertToPem() } /** - * Test that the protected getProp method returns correctly. + * Test that the requestJwkX5c method returns a cached value, if set. * * @return void */ - public function testGetProp() + public function testCacheReturn() { - $jwks = $this->getLocalJwks( 'test', '-jwks' ); + $jwks_url = 'https://localhost/.well-known/jwks.json'; + $kid = '__test_kid_2__'; + $cache_value = '__cached_value__'; + $set_spy = $this->once(); + $get_spy = $this->any(); + + // Mock the CacheHandler interface. + $cache_handler = $this->getMockBuilder(CacheHandler::class) + ->getMock(); + + // The set method should only be called once. + $cache_handler->expects($set_spy) + ->method('set') + ->willReturn( null ); + + // The get method should be called once and return no cache first, then a cache value after. + $cache_handler->expects($get_spy) + ->method('get') + ->will( $this->onConsecutiveCalls( null, $cache_value ) ); + + $jwksFetcher = $this->getStub($cache_handler); - $jwksFetcher = new JWKFetcher(); + $pem_not_cached = $jwksFetcher->requestJwkX5c( $jwks_url, $kid ); + $this->assertNotEmpty( $pem_not_cached ); - $this->assertEquals( '__string_value_1__', $jwksFetcher->getProp( $jwks, 'string' ) ); - $this->assertEquals( '__array_value_1__', $jwksFetcher->getProp( $jwks, 'array' ) ); - $this->assertNull( $jwksFetcher->getProp( $jwks, 'invalid' ) ); + $pem_cached = $jwksFetcher->requestJwkX5c( $jwks_url, $kid ); + $this->assertEquals( $cache_value, $pem_cached ); - $test_kid = '__kid_value__'; + // Test that the set method was called with the correct parameters. + $set_invocations = $set_spy->getInvocations(); + $this->assertEquals( $jwks_url.'|'.$kid, $set_invocations[0]->parameters[0] ); + $this->assertEquals( $pem_not_cached, $set_invocations[0]->parameters[1] ); - $this->assertEquals( '__string_value_2__', $jwksFetcher->getProp( $jwks, 'string', $test_kid ) ); - $this->assertEquals( '__array_value_3__', $jwksFetcher->getProp( $jwks, 'array', $test_kid ) ); - $this->assertNull( $jwksFetcher->getProp( $jwks, 'invalid', $test_kid ) ); + // Test that the get method was only called twice. + $this->assertEquals( 2, $get_spy->getInvocationCount() ); } + /* + * + * Test helper functions. + * + */ + /** * Get a test JSON fixture instead of a remote one. * @@ -165,32 +194,20 @@ public function getLocalJwks($domain = '', $path = '') /** * Stub the JWKFetcher class. * + * @param CacheHandler|null $cache_handler Cache handler to use or null if no cache. + * * @return \PHPUnit_Framework_MockObject_MockObject */ - private function getStub() + private function getStub($cache_handler = null) { $stub = $this->getMockBuilder(JWKFetcher::class) - ->setMethods(['getJwks']) + ->setConstructorArgs([$cache_handler]) + ->setMethods(['requestJwks']) ->getMock(); - $stub->method('getJwks') + $stub->method('requestJwks') ->will($this->returnCallback([$this, 'getLocalJwks'])); return $stub; } - - /** - * Get array of PEM parts. - * - * @param array $keys JWKS keys. - * - * @return array - */ - private function getPemParts(array $keys) - { - $keys_keys = array_keys($keys); - $kid = $keys_keys[0]; - $pem = $keys[$kid]; - return explode( PHP_EOL, $pem ); - } } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 89115849..cf58548f 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -1,8 +1,8 @@