From 744e7caf9281090674440bfe69543bd82ac48871 Mon Sep 17 00:00:00 2001 From: Maarten de Boer Date: Fri, 6 Oct 2017 08:05:27 +0200 Subject: [PATCH] Add interval argument for rotatable key set (#292) * Fix RotatableJWKSet to rotate keys at interval * Minor fixes and cleanup * Change BaseJWKSet from trait to abstract class and minor fixes. * Fix style issues and leftover debug code --- .gitignore | 4 + examples/NestedTokens.php | 1 - src/Factory/JWKFactory.php | 4 +- src/Factory/JWKFactoryInterface.php | 9 +- src/Object/BaseJWKSet.php | 5 +- src/Object/DownloadedJWKSet.php | 11 +- src/Object/JWKSet.php | 13 +- src/Object/JWKSetInterface.php | 17 ++- src/Object/JWKSets.php | 11 +- src/Object/PublicJWKSet.php | 11 +- src/Object/RotatableJWKSet.php | 71 +++++++++-- src/Object/Storable.php | 4 + src/Object/StorableJWK.php | 2 +- src/Object/StorableJWKSet.php | 8 ++ tests/Unit/Objects/RotatableJWKSetTest.php | 132 +++++++++++++++++++++ tests/Unit/Objects/StorableJWKSetTest.php | 17 ++- 16 files changed, 281 insertions(+), 39 deletions(-) create mode 100644 .gitignore create mode 100644 tests/Unit/Objects/RotatableJWKSetTest.php diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..7f3e3714 --- /dev/null +++ b/.gitignore @@ -0,0 +1,4 @@ +/vendor + +phpunit.xml +composer.lock diff --git a/examples/NestedTokens.php b/examples/NestedTokens.php index 8b121c92..20e4b975 100644 --- a/examples/NestedTokens.php +++ b/examples/NestedTokens.php @@ -72,4 +72,3 @@ 'zip' => 'DEF', ] ); -var_dump($jwe); diff --git a/src/Factory/JWKFactory.php b/src/Factory/JWKFactory.php index b5f93aad..7238c9b6 100644 --- a/src/Factory/JWKFactory.php +++ b/src/Factory/JWKFactory.php @@ -60,9 +60,9 @@ public static function createStorableKey($filename, array $parameters) /** * {@inheritdoc} */ - public static function createRotatableKeySet($filename, array $parameters, $nb_keys) + public static function createRotatableKeySet($filename, array $parameters, $nb_keys, $interval = null) { - return new RotatableJWKSet($filename, $parameters, $nb_keys); + return new RotatableJWKSet($filename, $parameters, $nb_keys, $interval); } /** diff --git a/src/Factory/JWKFactoryInterface.php b/src/Factory/JWKFactoryInterface.php index d140223a..739e2951 100644 --- a/src/Factory/JWKFactoryInterface.php +++ b/src/Factory/JWKFactoryInterface.php @@ -40,13 +40,14 @@ public static function createKeySets(array $jwksets = []); public static function createStorableKeySet($filename, array $parameters, $nb_keys); /** - * @param string $filename - * @param array $parameters - * @param int $nb_keys + * @param string $filename + * @param array $parameters + * @param int $nb_keys + * @param int|null $interval * * @return \Jose\Object\JWKSetInterface */ - public static function createRotatableKeySet($filename, array $parameters, $nb_keys); + public static function createRotatableKeySet($filename, array $parameters, $nb_keys, $interval = null); /** * @param string $filename diff --git a/src/Object/BaseJWKSet.php b/src/Object/BaseJWKSet.php index 40d9f28b..acfd1114 100644 --- a/src/Object/BaseJWKSet.php +++ b/src/Object/BaseJWKSet.php @@ -16,7 +16,7 @@ /** * Class BaseJWKSet. */ -trait BaseJWKSet +abstract class BaseJWKSet { /** * @var int @@ -172,7 +172,8 @@ public function selectKey($type, $algorithm = null, array $restrictions = []) Assertion::nullOrString($algorithm); $result = []; - foreach ($this->getKeys() as $key) { + $keys = $this->getKeys(); + foreach ($keys as $key) { $ind = 0; // Check usage diff --git a/src/Object/DownloadedJWKSet.php b/src/Object/DownloadedJWKSet.php index 39242b67..938df77c 100644 --- a/src/Object/DownloadedJWKSet.php +++ b/src/Object/DownloadedJWKSet.php @@ -17,9 +17,8 @@ /** * Class DownloadedJWKSet. */ -abstract class DownloadedJWKSet implements JWKSetInterface +abstract class DownloadedJWKSet extends BaseJWKSet implements JWKSetInterface { - use BaseJWKSet; use JWKSetPEM; /** @@ -78,6 +77,14 @@ public function addKey(JWKInterface $key) //Not available } + /** + * {@inheritdoc} + */ + public function prependKey(JWKInterface $key) + { + //Not available + } + /** * {@inheritdoc} */ diff --git a/src/Object/JWKSet.php b/src/Object/JWKSet.php index f63ccb0c..c90ac543 100644 --- a/src/Object/JWKSet.php +++ b/src/Object/JWKSet.php @@ -14,9 +14,8 @@ /** * Class JWKSet. */ -final class JWKSet implements JWKSetInterface +final class JWKSet extends BaseJWKSet implements JWKSetInterface { - use BaseJWKSet; use JWKSetPEM; /** @@ -49,12 +48,20 @@ public function addKey(JWKInterface $key) $this->keys[] = $key; } + /** + * {@inheritdoc} + */ + public function prependKey(JWKInterface $key) + { + array_unshift($this->keys, $key); + } + /** * {@inheritdoc} */ public function removeKey($key) { - if (isset($this->keys[$key])) { + if (array_key_exists($key, $this->keys)) { unset($this->keys[$key]); } } diff --git a/src/Object/JWKSetInterface.php b/src/Object/JWKSetInterface.php index 29119dd7..97dc455c 100644 --- a/src/Object/JWKSetInterface.php +++ b/src/Object/JWKSetInterface.php @@ -14,14 +14,18 @@ interface JWKSetInterface extends \Countable, \Iterator, \JsonSerializable, \ArrayAccess { /** - * @param $index + * Get key from set at index. + * + * @param int $index * * @return \Jose\Object\JWKInterface */ public function getKey($index); /** - * @param $index + * Check if set has key at index. + * + * @param int $index * * @return bool */ @@ -37,10 +41,17 @@ public function getKeys(); /** * Add key in the key set. * - * @param \Jose\Object\JWKInterface A key to store in the key set + * @param \Jose\Object\JWKInterface $key A key to store in the key set */ public function addKey(JWKInterface $key); + /** + * Prepend key to the set. + * + * @param \Jose\Object\JWKInterface $key A key to store in the key set + */ + public function prependKey(JWKInterface $key); + /** * Remove key from the key set. * diff --git a/src/Object/JWKSets.php b/src/Object/JWKSets.php index 0d5a6992..22fc9631 100644 --- a/src/Object/JWKSets.php +++ b/src/Object/JWKSets.php @@ -16,9 +16,8 @@ /** * Class JWKSets. */ -final class JWKSets implements JWKSetsInterface +final class JWKSets extends BaseJWKSet implements JWKSetsInterface { - use BaseJWKSet; use JWKSetPEM; /** @@ -71,6 +70,14 @@ public function addKey(JWKInterface $key) //Not available } + /** + * {@inheritdoc} + */ + public function prependKey(JWKInterface $key) + { + //Not available + } + /** * {@inheritdoc} */ diff --git a/src/Object/PublicJWKSet.php b/src/Object/PublicJWKSet.php index d77809b3..bf95a392 100644 --- a/src/Object/PublicJWKSet.php +++ b/src/Object/PublicJWKSet.php @@ -14,9 +14,8 @@ /** * Class PublicJWKSet. */ -final class PublicJWKSet implements JWKSetInterface +final class PublicJWKSet extends BaseJWKSet implements JWKSetInterface { - use BaseJWKSet; use JWKSetPEM; /** @@ -59,6 +58,14 @@ public function addKey(JWKInterface $key) $this->jwkset->addKey($key); } + /** + * {@inheritdoc} + */ + public function prependKey(JWKInterface $key) + { + $this->jwkset->prependKey($key); + } + /** * {@inheritdoc} */ diff --git a/src/Object/RotatableJWKSet.php b/src/Object/RotatableJWKSet.php index 855865ae..818d5c14 100644 --- a/src/Object/RotatableJWKSet.php +++ b/src/Object/RotatableJWKSet.php @@ -14,24 +14,71 @@ /** * Class RotatableJWKSet. */ -final class RotatableJWKSet extends StorableJWKSet implements RotatableInterface, JWKSetInterface +final class RotatableJWKSet extends StorableJWKSet implements RotatableInterface { + /** + * Interval at which keys should be rotated. + * + * @var int|null + */ + private $interval; + + /** + * RotatableJWKSet constructor. + * + * @param string $filename + * @param array $parameters + * @param int $nb_keys + * @param int|null $interval + */ + public function __construct($filename, array $parameters, $nb_keys, $interval = null) + { + parent::__construct($filename, $parameters, $nb_keys); + + $this->interval = $interval; + } + + /** + * {@inheritdoc} + */ + public function getJWKSet() + { + // Check if we need to rotate keys upon every interaction with the underlying JWK set + $this->rotateIfNeeded(); + + return parent::getJWKSet(); + } + /** * {@inheritdoc} */ public function rotate() { - $this->loadObjectIfNeeded(); - $jwkset = $this->getObject(); - - $keys = $jwkset->getKeys(); - unset($keys[count($keys) - 1]); - $jwkset = new JWKSet(); - $jwkset->addKey($this->createJWK()); - foreach ($keys as $key) { - $jwkset->addKey($key); - } - $this->setObject($jwkset); + $jwkset = parent::getJWKSet(); + + // Remove last key in set + $jwkset->removeKey($jwkset->countKeys() - 1); + + // Prepend new key to set + $jwkset->prependKey($this->createJWK()); + + // Save new key set $this->saveObject($jwkset); } + + /** + * Rotate key set if last modification time is due. + */ + private function rotateIfNeeded() + { + if (isset($this->interval) && $this->interval >= 0) { + $modificationTime = $this->getLastModificationTime(); + + if (null === $modificationTime) { + $this->regen(); + } elseif (($modificationTime + $this->interval) < time()) { + $this->rotate(); + } + } + } } diff --git a/src/Object/Storable.php b/src/Object/Storable.php index 0fcbb2c0..6b9cdb52 100644 --- a/src/Object/Storable.php +++ b/src/Object/Storable.php @@ -137,9 +137,13 @@ abstract protected function createObjectFromFileContent(array $file_content); */ public function getLastModificationTime() { + clearstatcache(null, $this->getFilename()); + if (file_exists($this->getFilename())) { return filemtime($this->getFilename()); } + + return null; } /** diff --git a/src/Object/StorableJWK.php b/src/Object/StorableJWK.php index fbe4e2bd..078c5768 100644 --- a/src/Object/StorableJWK.php +++ b/src/Object/StorableJWK.php @@ -27,7 +27,7 @@ class StorableJWK implements StorableInterface, JWKInterface protected $parameters; /** - * RotatableJWK constructor. + * StorableJWK constructor. * * @param string $filename * @param array $parameters diff --git a/src/Object/StorableJWKSet.php b/src/Object/StorableJWKSet.php index 861f6e89..a4769d5e 100644 --- a/src/Object/StorableJWKSet.php +++ b/src/Object/StorableJWKSet.php @@ -153,6 +153,14 @@ public function addKey(JWKInterface $key) // Not available } + /** + * {@inheritdoc} + */ + public function prependKey(JWKInterface $key) + { + //Not available + } + /** * {@inheritdoc} */ diff --git a/tests/Unit/Objects/RotatableJWKSetTest.php b/tests/Unit/Objects/RotatableJWKSetTest.php new file mode 100644 index 00000000..bbe4b0d9 --- /dev/null +++ b/tests/Unit/Objects/RotatableJWKSetTest.php @@ -0,0 +1,132 @@ + 'EC', + 'crv' => 'P-256', + ], + 3 + ); + + $this->assertEquals(3, $jwkset->count()); + $this->assertEquals(3, $jwkset->countKeys()); + + $this->assertInstanceOf(JWKInterface::class, $jwkset[0]); + $this->assertInstanceOf(JWKInterface::class, $jwkset[1]); + $this->assertInstanceOf(JWKInterface::class, $jwkset[2]); + $this->assertFalse(isset($jwkset[3])); + $this->assertTrue($jwkset->hasKey(0)); + $this->assertEquals($jwkset->getKey(0), $jwkset[0]); + foreach ($jwkset->getKeys() as $key) { + $this->assertInstanceOf(JWKInterface::class, $key); + } + foreach ($jwkset as $key) { + $this->assertInstanceOf(JWKInterface::class, $key); + } + + $actual_content = json_encode($jwkset); + + $this->assertEquals($actual_content, json_encode($jwkset)); + + $jwkset->rotate(); + + $this->assertNotEquals($actual_content, json_encode($jwkset)); + + $actual_content = json_encode($jwkset); + + $jwkset[] = JWKFactory::createKey(['kty' => 'EC', 'crv' => 'P-521']); + $this->assertEquals(3, $jwkset->count()); + $this->assertEquals(3, $jwkset->countKeys()); + $this->assertEquals($actual_content, json_encode($jwkset)); + + unset($jwkset[count($jwkset) - 1]); + $this->assertEquals(3, $jwkset->count()); + $this->assertEquals(3, $jwkset->countKeys()); + $this->assertEquals($actual_content, json_encode($jwkset)); + + $jwkset->addKey(JWKFactory::createKey(['kty' => 'EC', 'crv' => 'P-521'])); + $this->assertEquals(3, $jwkset->count()); + $this->assertEquals(3, $jwkset->countKeys()); + $this->assertEquals($actual_content, json_encode($jwkset)); + + $jwkset->prependKey(JWKFactory::createKey(['kty' => 'EC', 'crv' => 'P-521'])); + $this->assertEquals(3, $jwkset->count()); + $this->assertEquals(3, $jwkset->countKeys()); + $this->assertEquals($actual_content, json_encode($jwkset)); + + $jwkset->removeKey(count($jwkset) - 1); + $this->assertEquals(3, $jwkset->count()); + $this->assertEquals(3, $jwkset->countKeys()); + $this->assertEquals($actual_content, json_encode($jwkset)); + + $jwkset->delete(); + } + + public function testKeyInterval() + { + @unlink(sys_get_temp_dir().'/JWKSet.key'); + $jwkset = JWKFactory::createRotatableKeySet( + sys_get_temp_dir().'/JWKSet.key', + [ + 'kty' => 'EC', + 'crv' => 'P-256', + ], + 3, + 1 // 1 second rotation interval + ); + + $this->assertEquals(3, $jwkset->count()); + $this->assertEquals(3, $jwkset->countKeys()); + + $before_rotate = json_decode(json_encode($jwkset))->keys; + + // Sleep to ensure next calls trigger rotation + sleep(2); + + // Make sure that a manual call to getKeys triggered rotation + $after_rotate = json_decode(json_encode($jwkset->getKeys())); + + $this->assertCount(3, $after_rotate); + $this->assertEquals($before_rotate[0], $after_rotate[1]); + $this->assertEquals($before_rotate[1], $after_rotate[2]); + + // Sleep to ensure next calls trigger rotation + sleep(2); + + // Make sure that json serialization also triggered rotation + $after_second_rotate = json_decode(json_encode($jwkset))->keys; + + $this->assertCount(3, $after_second_rotate); + $this->assertEquals($after_rotate[0], $after_second_rotate[1]); + $this->assertEquals($after_rotate[1], $after_second_rotate[2]); + + // Make sure that subsequent calls to get keys within the interval period do not trigger rotation + $this->assertEquals($after_second_rotate, json_decode(json_encode($jwkset->getKeys()))); + $this->assertEquals($after_second_rotate, json_decode(json_encode($jwkset))->keys); + } +} diff --git a/tests/Unit/Objects/StorableJWKSetTest.php b/tests/Unit/Objects/StorableJWKSetTest.php index 9ed8d54a..ddf26c2b 100644 --- a/tests/Unit/Objects/StorableJWKSetTest.php +++ b/tests/Unit/Objects/StorableJWKSetTest.php @@ -22,7 +22,9 @@ class StorableJWKSetTest extends \PHPUnit_Framework_TestCase { public function testKey() { - $jwkset = JWKFactory::createRotatableKeySet( + @unlink(sys_get_temp_dir().'/JWKSet.key'); + + $jwkset = JWKFactory::createStorableKeySet( sys_get_temp_dir().'/JWKSet.key', [ 'kty' => 'EC', @@ -51,25 +53,30 @@ public function testKey() $this->assertEquals($actual_content, json_encode($jwkset)); - $jwkset->rotate(); - - $this->assertNotEquals($actual_content, json_encode($jwkset)); - $jwkset[] = JWKFactory::createKey(['kty' => 'EC', 'crv' => 'P-521']); $this->assertEquals(3, $jwkset->count()); $this->assertEquals(3, $jwkset->countKeys()); + $this->assertEquals($actual_content, json_encode($jwkset)); unset($jwkset[count($jwkset) - 1]); $this->assertEquals(3, $jwkset->count()); $this->assertEquals(3, $jwkset->countKeys()); + $this->assertEquals($actual_content, json_encode($jwkset)); $jwkset->addKey(JWKFactory::createKey(['kty' => 'EC', 'crv' => 'P-521'])); $this->assertEquals(3, $jwkset->count()); $this->assertEquals(3, $jwkset->countKeys()); + $this->assertEquals($actual_content, json_encode($jwkset)); + + $jwkset->prependKey(JWKFactory::createKey(['kty' => 'EC', 'crv' => 'P-521'])); + $this->assertEquals(3, $jwkset->count()); + $this->assertEquals(3, $jwkset->countKeys()); + $this->assertEquals($actual_content, json_encode($jwkset)); $jwkset->removeKey(count($jwkset) - 1); $this->assertEquals(3, $jwkset->count()); $this->assertEquals(3, $jwkset->countKeys()); + $this->assertEquals($actual_content, json_encode($jwkset)); $jwkset->delete(); }