Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add signing key rotation and custom JWKS URI support #426

Merged
merged 3 commits into from
Feb 10, 2020
Merged

Add signing key rotation and custom JWKS URI support #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f557e67
Add signing key rotation and custom JWKS URI support
joshcanhelp Feb 5, 2020
cb11146
Feedback: clarity
joshcanhelp Feb 7, 2020
6126574
Feedback: better custom JWKS URI handling
joshcanhelp Feb 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/API/Helpers/RequestBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class RequestBuilder
public function __construct(array $config)
{
$this->method = $config['method'];
$this->domain = $config['domain'];
$this->domain = $config['domain'] ?? '';
$this->basePath = $config['basePath'] ?? '';
$this->guzzleOptions = $config['guzzleOptions'] ?? [];
$this->headers = $config['headers'] ?? [];
Expand Down
17 changes: 13 additions & 4 deletions src/Auth0.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,13 @@ class Auth0
*/
protected $idTokenLeeway;

/**
* URI to the JWKS when accepting RS256 ID tokens.
*
* @var string
*/
protected $jwksUri;

/**
* Maximum time allowed between authentication and ID token verification.
*
Expand Down Expand Up @@ -225,9 +232,10 @@ class Auth0
* - max_age (Integer) Optional. Maximum time allowed between authentication and callback
* - id_token_alg (String) Optional. ID token algorithm expected; RS256 (default) or HS256 only
* - id_token_leeway (Integer) Optional. Leeway, in seconds, for ID token validation.
* - jwks_uri (String) Optional. URI to the JWKS when accepting RS256 ID tokens.
* - store (Mixed) Optional. StorageInterface for identity and token persistence;
* leave empty to default to SessionStore
* - transient_store (Mixed) Optional. StorageInterface for transient auth data;
* - transient_store (Mixed) Optional. StorageInterface for transient auth data;
* leave empty to default to CookieStore
* - cache_handler (Mixed) Optional. CacheInterface instance or false for none
* - persist_user (Boolean) Optional. Persist the user info, default true
Expand Down Expand Up @@ -268,6 +276,7 @@ public function __construct(array $config)
$this->skipUserinfo = $config['skip_userinfo'] ?? true;
$this->maxAge = $config['max_age'] ?? null;
$this->idTokenLeeway = $config['id_token_leeway'] ?? null;
$this->jwksUri = $config['jwks_uri'] ?? 'https://'.$this->domain.'/.well-known/jwks.json';

$this->idTokenAlg = $config['id_token_alg'] ?? 'RS256';
if (! in_array( $this->idTokenAlg, ['HS256', 'RS256'] )) {
Expand Down Expand Up @@ -647,9 +656,9 @@ public function decodeIdToken(string $idToken, array $verifierOptions = []) : ar
$idTokenIss = 'https://'.$this->domain.'/';
$sigVerifier = null;
if ('RS256' === $this->idTokenAlg) {
$jwksFetcher = new JWKFetcher($this->cacheHandler, $this->guzzleOptions);
$jwks = $jwksFetcher->getKeys($idTokenIss.'.well-known/jwks.json');
$sigVerifier = new AsymmetricVerifier($jwks);
$jwksHttpOptions = array_merge( $this->guzzleOptions, [ 'base_uri' => $this->jwksUri ] );
$jwksFetcher = new JWKFetcher($this->cacheHandler, $jwksHttpOptions);
$sigVerifier = new AsymmetricVerifier($jwksFetcher);
} else if ('HS256' === $this->idTokenAlg) {
$sigVerifier = new SymmetricVerifier($this->clientSecret);
}
Expand Down
53 changes: 43 additions & 10 deletions src/Helpers/JWKFetcher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@
*/
class JWKFetcher
{
/**
* How long should the cache persist? Set to 10 minutes.
*
* @see https://www.php-fig.org/psr/psr-16/#12-definitions
*/
const CACHE_TTL = 600;

/**
* Cache handler or null for no caching.
Expand All @@ -35,7 +41,7 @@ class JWKFetcher
* JWKFetcher constructor.
*
* @param CacheInterface|null $cache Cache handler or null for no caching.
* @param array $guzzleOptions Options for the Guzzle HTTP client.
* @param array $guzzleOptions Guzzle HTTP options.
*/
public function __construct(CacheInterface $cache = null, array $guzzleOptions = [])
{
Expand All @@ -62,19 +68,44 @@ protected function convertCertToPem(string $cert) : string
return $output;
}

/**
* Get a specific kid from a JWKS.
*
* @param string $kid Key ID to get.
* @param string|null $jwksUri JWKS URI to use, or fallback on class-level one.
*
* @return mixed|null
*/
public function getKey(string $kid, string $jwksUri = null)
{
$keys = $this->getKeys( $jwksUri );

if (! empty( $keys ) && empty( $keys[$kid] )) {
$keys = $this->getKeys( $jwksUri, false );
}

return $keys[$kid] ?? null;
}

/**
* Gets an array of keys from the JWKS as kid => x5c.
*
* @param string $jwks_url Full URL to the JWKS.
* @param string $jwks_url Full URL to the JWKS.
* @param boolean $use_cache Set to false to skip cache check; default true to use caching.
*
* @return array
*/
public function getKeys(string $jwks_url) : array
public function getKeys(string $jwks_url = null, bool $use_cache = true) : array
{
$cache_key = md5($jwks_url);
$keys = $this->cache->get($cache_key);
if (is_array($keys) && ! empty($keys)) {
return $keys;
$jwks_url = $jwks_url ?? $this->guzzleOptions['base_uri'] ?? '';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out there is a base_uri setting in the HTTP library already so I'm leveraging that here. We need to know the final URI we're using here to check the cache (if necessary). I'm sticking with the "return empty array if URL is empty" because that is indicating "we don't have keys for that URL" and will fail with the "no kid found in that JWKS" error mentioned earlier. Same outcome for a malformed/empty JWKS.

if (empty( $jwks_url )) {
return [];
}

$cache_key = md5($jwks_url);
$cached_value = $use_cache ? $this->cache->get($cache_key) : null;
if (! empty($cached_value) && is_array($cached_value)) {
return $cached_value;
}

$jwks = $this->requestJwks($jwks_url);
Expand All @@ -92,7 +123,7 @@ public function getKeys(string $jwks_url) : array
$keys[$key['kid']] = $this->convertCertToPem( $key['x5c'][0] );
}

$this->cache->set($cache_key, $keys);
$this->cache->set($cache_key, $keys, self::CACHE_TTL);
return $keys;
}

Expand All @@ -108,11 +139,13 @@ public function getKeys(string $jwks_url) : array
*/
protected function requestJwks(string $jwks_url) : array
{
$options = array_merge( $this->guzzleOptions, [ 'base_uri' => $jwks_url ] );

$request = new RequestBuilder([
'domain' => $jwks_url,
'method' => 'GET',
'guzzleOptions' => $this->guzzleOptions
'guzzleOptions' => $options,
]);

return $request->call();
}
}
16 changes: 9 additions & 7 deletions src/Helpers/Tokens/AsymmetricVerifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace Auth0\SDK\Helpers\Tokens;

use Auth0\SDK\Exception\InvalidTokenException;
use Auth0\SDK\Helpers\JWKFetcher;
use Lcobucci\JWT\Signer\Key;
use Lcobucci\JWT\Signer\Rsa\Sha256 as RsSigner;
use Lcobucci\JWT\Token;
Expand All @@ -17,18 +18,18 @@ final class AsymmetricVerifier extends SignatureVerifier
{

/**
* JWKS array with kid as keys, PEM cert as values.
* Array of kid => keys or a JWKFetcher instance.
*
* @var array
* @var array|JWKFetcher
*/
private $jwks;

/**
* JwksVerifier constructor.
*
* @param array $jwks JWKS to use.
* @param array|JWKFetcher $jwks Array of kid => keys or a JWKFetcher instance.
*/
public function __construct(array $jwks)
public function __construct($jwks)
{
$this->jwks = $jwks;
parent::__construct('RS256');
Expand All @@ -45,11 +46,12 @@ public function __construct(array $jwks)
*/
protected function checkSignature(Token $token) : bool
{
$tokenKid = $token->getHeader('kid', false);
if (! array_key_exists($tokenKid, $this->jwks)) {
$tokenKid = $token->getHeader('kid', false);
$signingKey = is_array( $this->jwks ) ? ($this->jwks[$tokenKid] ?? null) : $this->jwks->getKey( $tokenKid );
if (! $signingKey) {
throw new InvalidTokenException( 'ID token key ID "'.$tokenKid.'" was not found in the JWKS' );
}

return $token->verify(new RsSigner(), new Key($this->jwks[$tokenKid]));
return $token->verify(new RsSigner(), new Key($signingKey));
}
}
12 changes: 8 additions & 4 deletions tests/Auth0Test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Auth0\SDK\Exception\CoreException;
use Auth0\SDK\Exception\InvalidTokenException;
use Auth0\SDK\Store\SessionStore;
use Auth0\Tests\Helpers\Tokens\AsymmetricVerifierTest;
use Auth0\Tests\Helpers\Tokens\SymmetricVerifierTest;
use Auth0\Tests\Traits\ErrorHelpers;
use Cache\Adapter\PHPArray\ArrayCachePool;
Expand Down Expand Up @@ -657,7 +658,7 @@ public function testThatCacheHandlerCanBeSet()
{
$request_history = [];
$mock = new MockHandler([
new Response( 200, [ 'Content-Type' => 'application/json' ], '{"keys":[{"kid":"abc","x5c":["123"]}]}' ),
new Response( 200, [ 'Content-Type' => 'application/json' ], '{"keys":[{"kid":"__test_kid__","x5c":["123"]}]}' ),
]);
$handler = HandlerStack::create($mock);
$handler->push( Middleware::history($request_history) );
Expand All @@ -668,21 +669,24 @@ public function testThatCacheHandlerCanBeSet()
'client_id' => uniqid(),
'redirect_uri' => uniqid(),
'cache_handler' => $pool,
'transient_store' => new SessionStore(),
'guzzle_options' => [
'handler' => $handler
]
]);
$_SESSION['auth0__nonce'] = '__test_nonce__';

try {
@$auth0->setIdToken(uniqid());
@$auth0->setIdToken(AsymmetricVerifierTest::getToken());
} catch ( \Exception $e ) {
// ...
}

$stored_jwks = $pool->get(md5('https://test.auth0.com/.well-known/jwks.json'));

$this->assertArrayHasKey('abc', $stored_jwks);
$this->assertEquals("-----BEGIN CERTIFICATE-----\n123\n-----END CERTIFICATE-----\n", $stored_jwks['abc']);
$this->assertNotEmpty($stored_jwks);
$this->assertArrayHasKey('__test_kid__', $stored_jwks);
$this->assertEquals("-----BEGIN CERTIFICATE-----\n123\n-----END CERTIFICATE-----\n", $stored_jwks['__test_kid__']);
}

/*
Expand Down
73 changes: 70 additions & 3 deletions tests/Helpers/JWKFetcherTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php
namespace Auth0\Tests\Helpers;

use Auth0\SDK\Helpers\JWKFetcher;
use Cache\Adapter\PHPArray\ArrayCachePool;
use GuzzleHttp\Psr7\Response;
use PHPUnit\Framework\TestCase;
Expand All @@ -13,7 +14,7 @@
class JWKFetcherTest extends TestCase
{

public function testThatGetFormattedReturnsKeys()
public function testThatGetKeysReturnsKeys()
{
$test_jwks = file_get_contents( AUTH0_PHP_TEST_JSON_DIR.'localhost--well-known-jwks-json.json' );
$jwks = new MockJwks( [ new Response( 200, [ 'Content-Type' => 'application/json' ], $test_jwks ) ] );
Expand All @@ -36,7 +37,7 @@ public function testThatGetFormattedReturnsKeys()
$this->assertEquals( '-----END CERTIFICATE-----', $pem_parts_2[2] );
}

public function testThatGetFormattedEmptyJwksReturnsEmptyArray()
public function testThatGetKeysEmptyJwksReturnsEmptyArray()
{
$jwks = new MockJwks( [
new Response( 200, [ 'Content-Type' => 'application/json' ], '{}' ),
Expand All @@ -50,7 +51,7 @@ public function testThatGetFormattedEmptyJwksReturnsEmptyArray()
$this->assertEquals( [], $jwks_formatted );
}

public function testThatGetFormattedUsesCache()
public function testThatGetKeysUsesCache()
{
$jwks_body_1 = '{"keys":[{"kid":"abc","x5c":["123"]}]}';
$jwks_body_2 = '{"keys":[{"kid":"def","x5c":["456"]}]}';
Expand All @@ -75,4 +76,70 @@ public function testThatGetFormattedUsesCache()
$this->assertArrayHasKey( 'def', $jwks_formatted_2 );
$this->assertContains( '456', $jwks_formatted_2['def'] );
}

public function testThatGetKeyBreaksCache()
{
$jwks_body_1 = '{"keys":[{"kid":"__kid_1__","x5c":["__x5c_1__"]}]}';
$jwks_body_2 = '{"keys":[{"kid":"__kid_1__","x5c":["__x5c_1__"]},{"kid":"__kid_2__","x5c":["__x5c_2__"]}]}';
$jwks = new MockJwks(
[
new Response( 200, [ 'Content-Type' => 'application/json' ], $jwks_body_1 ),
new Response( 200, [ 'Content-Type' => 'application/json' ], $jwks_body_2 ),
],
[
'cache' => new ArrayCachePool()
]
);

$jwks_formatted_1 = $jwks->call()->getKeys( '__test_url__' );
$this->assertArrayHasKey( '__kid_1__', $jwks_formatted_1 );
$this->assertArrayNotHasKey( '__kid_2__', $jwks_formatted_1 );

$jwks_formatted_2 = $jwks->call()->getKeys( '__test_url__' );
$this->assertEquals( $jwks_formatted_1, $jwks_formatted_2 );

$jwks_formatted_3 = $jwks->call()->getKeys( '__test_url__', false );
$this->assertArrayHasKey( '__kid_1__', $jwks_formatted_3 );
$this->assertArrayHasKey( '__kid_2__', $jwks_formatted_3 );
}

public function testThatGetKeysUsesOptionsUrl()
{
$jwks_body = '{"keys":[{"kid":"__kid_1__","x5c":["__x5c_1__"]}]}';
$jwks = new MockJwks(
[ new Response( 200, [ 'Content-Type' => 'application/json' ], $jwks_body ) ],
[ 'cache' => new ArrayCachePool() ],
[ 'base_uri' => '__test_jwks_url__' ]
);

$jwks->call()->getKeys();
$this->assertEquals( '__test_jwks_url__', $jwks->getHistoryUrl() );
}

public function testThatGetKeyGetsSpecificKid() {
$cache = new ArrayCachePool();
$jwks = new JWKFetcher( $cache, [ 'base_uri' => '__test_jwks_url__' ] );
$cache->set(md5('__test_jwks_url__'), ['__test_kid_1__' => '__test_x5c_1__']);
$this->assertEquals('__test_x5c_1__', $jwks->getKey('__test_kid_1__'));
}

public function testThatGetKeyBreaksCacheIsKidMissing() {
$cache = new ArrayCachePool();

$jwks_body = '{"keys":[{"kid":"__test_kid_2__","x5c":["__test_x5c_2__"]}]}';
$jwks = new MockJwks(
[ new Response( 200, [ 'Content-Type' => 'application/json' ], $jwks_body ) ],
[ 'cache' => $cache ],
[ 'base_uri' => '__test_jwks_url__' ]
);

$cache->set(md5('__test_jwks_url__'), ['__test_kid_1__' => '__test_x5c_1__']);

$this->assertContains('__test_x5c_2__', $jwks->call()->getKey('__test_kid_2__'));
}

public function testThatEmptyUrlReturnsEmptyKeys() {
$jwks_formatted_1 = (new JWKFetcher())->getKeys();
$this->assertEquals( [], $jwks_formatted_1 );
}
}
2 changes: 1 addition & 1 deletion tests/Helpers/MockJwks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class MockJwks extends MockApi
*/
public function setClient(array $guzzleOptions, array $config = [])
{
$cache = isset( $config['cache'] ) && $config['cache'] instanceof CacheInterface ? $config['cache'] : null;
$cache = isset( $config['cache'] ) && $config['cache'] instanceof CacheInterface ? $config['cache'] : null;
$this->client = new JWKFetcher( $cache, $guzzleOptions );
}
}
3 changes: 2 additions & 1 deletion tests/Helpers/Tokens/AsymmetricVerifierTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ public static function getTokenBuilder() : Builder
return (new Builder())->withClaim('sub', '__test_sub__')->withHeader('kid', '__test_kid__');
}

public static function getToken(string $rsa_private_key, Builder $builder = null) : Token
public static function getToken(string $rsa_private_key = null, Builder $builder = null) : Token
{
$rsa_private_key = $rsa_private_key ?? self::getRsaKeys()['private'];
$builder = $builder ?? self::getTokenBuilder();
return $builder->getToken( new RsSigner(), new Key( $rsa_private_key ));
}
Expand Down
4 changes: 2 additions & 2 deletions tests/MockApi.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ abstract class MockApi
*
* @param array $responses Array of GuzzleHttp\Psr7\Response objects.
* @param array $config Additional optional configuration needed for mocked class.
* @param array $guzzleOptions Additional Guzzle HTTP options.
*/
public function __construct(array $responses = [], array $config = [])
public function __construct(array $responses = [], array $config = [], array $guzzleOptions = [])
{
$guzzleOptions = [];
if (count( $responses )) {
$mock = new MockHandler($responses);
$handler = HandlerStack::create($mock);
Expand Down