Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@joshcanhelp
joshcanhelp committed Sep 20, 2018
1 parent 4e12f64 commit a339af8
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 96 deletions.
1 change: 1 addition & 0 deletions .phpcs.xml.dist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
<rule ref="Squiz.PHP">
<exclude name="Squiz.PHP.DisallowComparisonAssignment.AssignedComparison"/>
<exclude name="Squiz.PHP.DisallowInlineIf.Found"/>
<exclude name="Squiz.PHP.DisallowBooleanStatement.Found"/>
</rule>
<rule ref="Squiz.Scope"/>
<rule ref="Squiz.Strings"/>
Expand Down
156 changes: 96 additions & 60 deletions src/Helpers/JWKFetcher.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -59,101 +62,134 @@ protected function convertCertToPem($cert)
return $output;
}

// phpcs:disable
/**
* Fetch x509 cert for RS256 token decoding.
* TODO: Deprecate, use $this->getJwksX5c() instead.
*
* @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.
* @param string $iss
*
* @return mixed
* @return array|mixed|null
*
* @throws \Exception If the Guzzle HTTP client cannot complete the request.
* @throws \Exception
*
* @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 = [];

$jwks = $this->getJwks( $domain, $path );

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' );
}

$x5c = $this->getProp( $jwks, 'x5c', $kid );
$url = "{$iss}.well-known/jwks.json";
if (($secret = $this->cache->get($url)) === null) {
$secret = [];
$request = new RequestBuilder([
'domain' => $iss,
'basePath' => '.well-known/jwks.json',
'method' => 'GET',
'guzzleOptions' => $this->guzzleOptions
]);
$jwks = $request->call();
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.
*
* @return null|string
* @return string|null
*/
public function getProp(array $jwks, $prop, $kid = null)
public function getJwksX5c($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->getJwks($jwks_url);
$jwk = $this->getJwk($jwks, $kid);

if (! $this->subArrayHasNonEmptyFirstItem($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.
*
* @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 getJwks($jwks_url)
{
$request = new RequestBuilder([
'domain' => $domain,
'basePath' => $path,
'domain' => $jwks_url,
'basePath' => '',
'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 getJwk(array $jwks, $kid = null)
{
if (! $this->subArrayHasNonEmptyFirstItem($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 subArrayHasNonEmptyFirstItem($array, $key)
{
return ! empty($array) && is_array($array[$key]) && ! empty($array[$key][0]);
}
}
9 changes: 8 additions & 1 deletion src/JWTVerifier.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ class JWTVerifier

protected $secret_base64_encoded = null;

protected $jwks_path = '.well-known/jwks.json';

/**
* JWTVerifier Constructor.
*
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down
63 changes: 31 additions & 32 deletions tests/Helpers/JWKFetcherTest.php
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
<?php
namespace Auth0\Tests\Helpers\Cache;
namespace Auth0\Tests\Helpers;

use \Auth0\SDK\Helpers\JWKFetcher;
use \Auth0\Tests\TestCacheHandler;

/**
* Class JWKFetcherTest.
Expand All @@ -15,15 +16,15 @@ class JWKFetcherTest extends \PHPUnit_Framework_TestCase
*
* @return void
*/
public function testFetchKeysWithoutKid()
public function testGetJwksX5cWithoutKid()
{
$jwksFetcher = $this->getStub();

$keys = $jwksFetcher->fetchKeys( 'https://localhost/' );
$pem = $jwksFetcher->getJwksX5c( '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] );
Expand All @@ -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->getJwksX5c( '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] );
Expand All @@ -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->getJwksX5c( '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] );
Expand All @@ -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->getJwksX5c( 'https://localhost/.custom/jwks.json', '__no_x5c_test_kid_2__' );

$this->assertEmpty($keys);
$this->assertEmpty($pem);
}

/**
Expand Down Expand Up @@ -117,26 +118,21 @@ public function testConvertCertToPem()
$this->assertEquals( '-----END CERTIFICATE-----', $pem_parts[3] );
}

/**
* Test that the protected getProp method returns correctly.
*
* @return void
*/
public function testGetProp()
public function testCacheReturn()
{
$jwks = $this->getLocalJwks( 'test', '-jwks' );
$cache_handler = new TestCacheHandler();
$jwksFetcher = $this->getStub($cache_handler);

$jwksFetcher = new JWKFetcher();
$jwks_url = 'https://localhost/.well-known/jwks.json';
$kid = '__test_kid_2__';

$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 = $jwksFetcher->getJwksX5c( $jwks_url, $kid );
$this->assertNotEmpty($pem);

$test_kid = '__kid_value__';
$cache_handler->set( $jwks_url.'|'.$kid, '__new_value__' );
$pem = $jwksFetcher->getJwksX5c( $jwks_url, $kid );

$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 ) );
$this->assertEquals( '__new_value__', $pem );
}

/**
Expand Down Expand Up @@ -165,11 +161,14 @@ public function getLocalJwks($domain = '', $path = '')
/**
* Stub the JWKFetcher class.
*
* @param 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)
->setConstructorArgs([$cache_handler])
->setMethods(['getJwks'])
->getMock();

Expand Down
Loading

0 comments on commit a339af8

Please sign in to comment.