From 781692b1b73b84c655cb2af45406dfcaee18b8dd Mon Sep 17 00:00:00 2001 From: James Read Date: Wed, 6 Apr 2022 20:30:10 +0100 Subject: [PATCH 1/8] Dropping support for older version of firebase/php-jwt --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 490a448..e25f63c 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,7 @@ "require": { "php": "^7.4|^8.0", "psr/log": "^1.0|^2.0|^3.0", - "firebase/php-jwt": "^3.0|^4.0|^5.0", + "firebase/php-jwt": "^6.0", "psr/http-message": "^1.0|^2.0", "tuupola/http-factory": "^1.3", "tuupola/callable-handler": "^1.0", From d07a9cb16dacc2c8aa288cd1c8e4d6334fe9dc45 Mon Sep 17 00:00:00 2001 From: James Read Date: Thu, 7 Apr 2022 00:12:58 +0100 Subject: [PATCH 2/8] Creating key objects to pass to JWT::decode --- src/JwtAuthentication.php | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/JwtAuthentication.php b/src/JwtAuthentication.php index 6a453bd..24e1ab8 100644 --- a/src/JwtAuthentication.php +++ b/src/JwtAuthentication.php @@ -34,13 +34,15 @@ namespace Tuupola\Middleware; +use ArrayAccess; use Closure; use DomainException; -use InvalidArgumentException; use Exception; use Firebase\JWT\JWT; -use Psr\Http\Message\ServerRequestInterface; +use Firebase\JWT\Key; +use InvalidArgumentException; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; @@ -90,7 +92,7 @@ final class JwtAuthentication implements MiddlewareInterface private array $options = [ "secure" => true, "relaxed" => ["localhost", "127.0.0.1"], - "algorithm" => ["HS256", "HS512", "HS384"], + "algorithm" => ["HS256"], "header" => "Authorization", "regexp" => "/Bearer\s+(.*)$/i", "cookie" => "token", @@ -306,11 +308,15 @@ private function fetchToken(ServerRequestInterface $request): string */ private function decodeToken(string $token): array { + $keys = $this->createKeysFromAlgorithms(); + if (count($keys) === 1) { + $keys = current($keys); + } + try { $decoded = JWT::decode( $token, - $this->options["secret"], - (array) $this->options["algorithm"] + $keys ); return (array) $decoded; } catch (Exception $exception) { @@ -504,4 +510,22 @@ private function rules(array $rules): void $this->rules->push($callable); } } + + private function createKeysFromAlgorithms(): array + { + $keyObjects = []; + foreach ($this->options["algorithm"] as $kid => $algorithm) { + $keyId = !is_numeric($kid) ? $kid : $algorithm; + + $secret = $this->options["secret"]; + + if (is_array($this->options["secret"]) || $secret instanceof ArrayAccess) { + $secret = $this->options["secret"][$kid]; + } + + $keyObjects[$keyId] = new Key($secret, $algorithm); + } + + return $keyObjects; + } } From d423545955ff16377c633d02ec0513ee7216934e Mon Sep 17 00:00:00 2001 From: James Read Date: Thu, 7 Apr 2022 00:16:11 +0100 Subject: [PATCH 3/8] Fixing failing unit tests after upgrade of firebase/php-jwt --- tests/JwtAuthenticationTest.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/JwtAuthenticationTest.php b/tests/JwtAuthenticationTest.php index 12c704f..76ab99d 100644 --- a/tests/JwtAuthenticationTest.php +++ b/tests/JwtAuthenticationTest.php @@ -33,6 +33,8 @@ namespace Tuupola\Middleware; use Equip\Dispatch\MiddlewareCollection; +use Firebase\JWT\JWT; +use Firebase\JWT\Key; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; @@ -106,6 +108,7 @@ public function testShouldReturn200WithTokenFromHeader(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", + "algorithm" => ['acme' => 'HS256'], "header" => "X-Token" ]) ]); @@ -131,6 +134,7 @@ public function testShouldReturn200WithTokenFromHeaderWithCustomRegexp(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", + "algorithm" => ['acme' => 'HS256'], "header" => "X-Token", "regexp" => "/(.*)/" ]) @@ -157,6 +161,7 @@ public function testShouldReturn200WithTokenFromCookie(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", + "algorithm" => ['acme' => 'HS256'], "cookie" => "nekot", ]) ]); @@ -182,6 +187,7 @@ public function testShouldReturn200WithTokenFromBearerCookie(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", + "algorithm" => ['acme' => 'HS256'], "cookie" => "nekot", ]) ]); @@ -210,7 +216,8 @@ public function testShouldReturn200WithSecretArray(): void "secret" => [ "acme" =>"supersecretkeyyoushouldnotcommittogithub", "beta" =>"anothersecretkeyfornevertocommittogithub" - ] + ], + "algorithm" => ['acme' => 'HS256', 'beta' => 'HS256'], ]) ]); @@ -263,7 +270,8 @@ public function testShouldReturn200WithSecretArrayAccess(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ - "secret" => $secret + "secret" => $secret, + "algorithm" => ['acme' => 'HS256', 'beta' => 'HS256'], ]) ]); @@ -290,7 +298,8 @@ public function testShouldReturn401WithSecretArrayAccess(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ - "secret" => $secret + "secret" => $secret, + "algorithm" => ['xxxx' => 'HS256', 'yyyy' => 'HS256',], ]) ]); From b3ed45054ee2c028b9d3b94154f6bd511087966f Mon Sep 17 00:00:00 2001 From: James Read Date: Sat, 6 Aug 2022 15:10:03 +0100 Subject: [PATCH 4/8] Adding docblock to createKeysFromAlgorithms for better typehinting --- src/JwtAuthentication.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/JwtAuthentication.php b/src/JwtAuthentication.php index 24e1ab8..a9673ab 100644 --- a/src/JwtAuthentication.php +++ b/src/JwtAuthentication.php @@ -511,6 +511,9 @@ private function rules(array $rules): void } } + /** + * @return array + */ private function createKeysFromAlgorithms(): array { $keyObjects = []; From 0d3615c5b7885e6f95c06255e23570a1b7217299 Mon Sep 17 00:00:00 2001 From: James Read Date: Sat, 6 Aug 2022 15:11:13 +0100 Subject: [PATCH 5/8] Reverting changes to unit tests Removing the key id from when only one secret/algorithm is supplied --- tests/JwtAuthenticationTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/JwtAuthenticationTest.php b/tests/JwtAuthenticationTest.php index 76ab99d..ccc109a 100644 --- a/tests/JwtAuthenticationTest.php +++ b/tests/JwtAuthenticationTest.php @@ -108,7 +108,7 @@ public function testShouldReturn200WithTokenFromHeader(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", - "algorithm" => ['acme' => 'HS256'], + "algorithm" => ['HS256'], "header" => "X-Token" ]) ]); @@ -134,7 +134,7 @@ public function testShouldReturn200WithTokenFromHeaderWithCustomRegexp(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", - "algorithm" => ['acme' => 'HS256'], + "algorithm" => ['HS256'], "header" => "X-Token", "regexp" => "/(.*)/" ]) @@ -161,7 +161,7 @@ public function testShouldReturn200WithTokenFromCookie(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", - "algorithm" => ['acme' => 'HS256'], + "algorithm" => ['HS256'], "cookie" => "nekot", ]) ]); @@ -187,7 +187,7 @@ public function testShouldReturn200WithTokenFromBearerCookie(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", - "algorithm" => ['acme' => 'HS256'], + "algorithm" => ['HS256'], "cookie" => "nekot", ]) ]); From 6346d2f6412f7845bcb52df28efa159126a2452a Mon Sep 17 00:00:00 2001 From: James Read Date: Sat, 6 Aug 2022 16:22:44 +0100 Subject: [PATCH 6/8] Manipulating algorithm in hydrator When only one algorithm is passed into the configuration but multiple secrets are provided the algorithm array then needs to be manipulated into a key value store, using the key from the secrets list and the algorithm being used for the value. for example: ``` [ 'secret' => [ 'foo' => 'keepItSecret', 'bar' => 'tooManySecrets', ], 'algorithm' => [ 'HS256', ], ] ``` will become ``` [ 'secret' => [ 'foo' => 'keepItSecret', 'bar' => 'tooManySecrets', ], 'algorithm' => [ 'foo' => 'HS256', 'bar' => 'HS256', ], ] ``` --- src/JwtAuthentication.php | 15 +++++++++ tests/ArrayAccessImpl.php | 58 --------------------------------- tests/JwtAuthenticationTest.php | 14 ++------ 3 files changed, 18 insertions(+), 69 deletions(-) delete mode 100644 tests/ArrayAccessImpl.php diff --git a/src/JwtAuthentication.php b/src/JwtAuthentication.php index a9673ab..444ddcb 100644 --- a/src/JwtAuthentication.php +++ b/src/JwtAuthentication.php @@ -54,6 +54,11 @@ use Tuupola\Middleware\JwtAuthentication\RequestPathRule; use Tuupola\Middleware\JwtAuthentication\RuleInterface; +use function array_fill_keys; +use function array_keys; +use function count; +use function is_array; + final class JwtAuthentication implements MiddlewareInterface { use DoublePassTrait; @@ -332,6 +337,16 @@ private function decodeToken(string $token): array */ private function hydrate(array $data = []): void { + $data['algorithm'] = $data['algorithm'] ?? $this->options['algorithm']; + if ((is_array($data['secret']) || $data['secret'] instanceof ArrayAccess) + && is_array($data['algorithm']) + && count($data['algorithm']) === 1 + && count($data['secret']) > count($data['algorithm']) + ) { + $secretIndex = array_keys((array) $data['secret']); + $data['algorithm'] = array_fill_keys($secretIndex, $data['algorithm'][0]); + } + foreach ($data as $key => $value) { /* https://github.com/facebook/hhvm/issues/6368 */ $key = str_replace(".", " ", $key); diff --git a/tests/ArrayAccessImpl.php b/tests/ArrayAccessImpl.php deleted file mode 100644 index 9f3fa33..0000000 --- a/tests/ArrayAccessImpl.php +++ /dev/null @@ -1,58 +0,0 @@ -array[$offset]); - } - - public function offsetGet($offset) - { - return $this->array[$offset]; - } - - public function offsetSet($offset, $value) - { - $this->array[$offset] = $value; - } - - public function offsetUnset($offset) - { - unset($this->array[$offset]); - } -} diff --git a/tests/JwtAuthenticationTest.php b/tests/JwtAuthenticationTest.php index ccc109a..3e0e925 100644 --- a/tests/JwtAuthenticationTest.php +++ b/tests/JwtAuthenticationTest.php @@ -32,9 +32,8 @@ namespace Tuupola\Middleware; +use ArrayObject; use Equip\Dispatch\MiddlewareCollection; -use Firebase\JWT\JWT; -use Firebase\JWT\Key; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; @@ -108,7 +107,6 @@ public function testShouldReturn200WithTokenFromHeader(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", - "algorithm" => ['HS256'], "header" => "X-Token" ]) ]); @@ -134,7 +132,6 @@ public function testShouldReturn200WithTokenFromHeaderWithCustomRegexp(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", - "algorithm" => ['HS256'], "header" => "X-Token", "regexp" => "/(.*)/" ]) @@ -161,7 +158,6 @@ public function testShouldReturn200WithTokenFromCookie(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", - "algorithm" => ['HS256'], "cookie" => "nekot", ]) ]); @@ -187,7 +183,6 @@ public function testShouldReturn200WithTokenFromBearerCookie(): void $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", - "algorithm" => ['HS256'], "cookie" => "nekot", ]) ]); @@ -217,7 +212,6 @@ public function testShouldReturn200WithSecretArray(): void "acme" =>"supersecretkeyyoushouldnotcommittogithub", "beta" =>"anothersecretkeyfornevertocommittogithub" ], - "algorithm" => ['acme' => 'HS256', 'beta' => 'HS256'], ]) ]); @@ -264,14 +258,13 @@ public function testShouldReturn200WithSecretArrayAccess(): void return $response; }; - $secret = new ArrayAccessImpl(); + $secret = new ArrayObject(); $secret["acme"] = "supersecretkeyyoushouldnotcommittogithub"; $secret["beta"] ="anothersecretkeyfornevertocommittogithub"; $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => $secret, - "algorithm" => ['acme' => 'HS256', 'beta' => 'HS256'], ]) ]); @@ -292,14 +285,13 @@ public function testShouldReturn401WithSecretArrayAccess(): void return $response; }; - $secret = new ArrayAccessImpl(); + $secret = new ArrayObject(); $secret["xxxx"] = "supersecretkeyyoushouldnotcommittogithub"; $secret["yyyy"] = "anothersecretkeyfornevertocommittogithub"; $collection = new MiddlewareCollection([ new JwtAuthentication([ "secret" => $secret, - "algorithm" => ['xxxx' => 'HS256', 'yyyy' => 'HS256',], ]) ]); From 2c25fc231a6a961d26b565d93bd65b0654146ab7 Mon Sep 17 00:00:00 2001 From: James Read Date: Sun, 17 Dec 2023 17:43:06 +0800 Subject: [PATCH 7/8] Updating docs to explain change in behavour Updatig the readme to explain whe the KID is now needed when passing when decodig the token also why the token need the KID to be set in the header --- README.md | 13 +++++++++++-- src/JwtAuthentication.php | 5 +++-- tests/JwtAuthenticationTest.php | 27 +++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index e4152cb..a12e7a4 100644 --- a/README.md +++ b/README.md @@ -127,17 +127,26 @@ $app->add(new Tuupola\Middleware\JwtAuthentication([ ### Algorithm -You can set supported algorithms via `algorithm` parameter. This can be either string or array of strings. Default value is `["HS256", "HS512", "HS384"]`. Supported algorithms are `HS256`, `HS384`, `HS512` and `RS256`. Note that enabling both `HS256` and `RS256` is a [security risk](https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/). +You can set supported algorithms via `algorithm` parameter. This can be either string or array of strings. Default value is `["HS256"]`. Supported algorithms are `HS256`, `HS384`, `HS512` and `RS256`. Note that enabling both `HS256` and `RS256` is a [security risk](https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/). + +When passing multiple algorithm it be a key value array, with the key being the `KID` of the jwt. ``` php $app = new Slim\App; $app->add(new Tuupola\Middleware\JwtAuthentication([ "secret" => "supersecretkeyyoushouldnotcommittogithub", - "algorithm" => ["HS256", "HS384"] + "algorithm" => [ + "amce" => "HS256", + "beta" => "HS384" + ] ])); ``` +> :warning: **Warning**:
+Because of changes in `firebase/php-jwt` the `kid` is now checked when multiple algorithm are passed, failing to provide a key the algorithm will be used for the kid. +this also means the `kid` will now need to be present in the JWT header as well. + ### Attribute When the token is decoded successfully and authentication succeeds the contents of the decoded token is saved as `token` attribute to the `$request` object. You can change this with. `attribute` parameter. Set to `null` or `false` to disable this behavour diff --git a/src/JwtAuthentication.php b/src/JwtAuthentication.php index 444ddcb..0faab67 100644 --- a/src/JwtAuthentication.php +++ b/src/JwtAuthentication.php @@ -79,7 +79,7 @@ final class JwtAuthentication implements MiddlewareInterface * Stores all the options passed to the middleware. * * @var array{ - * secret?: string|array, + * secret?: string|array|array, * secure: bool, * relaxed: array, * algorithm: array, @@ -321,7 +321,8 @@ private function decodeToken(string $token): array try { $decoded = JWT::decode( $token, - $keys + $keys, + $this->options['algorithm'] ); return (array) $decoded; } catch (Exception $exception) { diff --git a/tests/JwtAuthenticationTest.php b/tests/JwtAuthenticationTest.php index 3e0e925..9f232f6 100644 --- a/tests/JwtAuthenticationTest.php +++ b/tests/JwtAuthenticationTest.php @@ -220,6 +220,33 @@ public function testShouldReturn200WithSecretArray(): void $this->assertEquals("Success", $response->getBody()); } + public function testShouldReturn200WithSecretArrayCheckKid(): void + { + $request = (new ServerRequestFactory) + ->createServerRequest("GET", "https://example.com/api") + ->withHeader("Authorization", "Bearer " . self::$betaToken); + + $default = function (ServerRequestInterface $request) { + $response = (new ResponseFactory)->createResponse(); + $response->getBody()->write("Success"); + return $response; + }; + + $collection = new MiddlewareCollection([ + new JwtAuthentication([ + "algorithm" => ["acme" => "HS256", "beta" => "HS256"], + "secret" => [ + "acme" =>"supersecretkeyyoushouldnotcommittogithub", + "beta" =>"anothersecretkeyfornevertocommittogithub" + ], + ]) + ]); + + $response = $collection->dispatch($request, $default); + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals("Success", $response->getBody()); + } + public function testShouldReturn401WithSecretArray(): void { $request = (new ServerRequestFactory) From 3e9924bf6b9e2496795fa078674650d70cec5656 Mon Sep 17 00:00:00 2001 From: James Read Date: Sun, 17 Dec 2023 18:26:09 +0800 Subject: [PATCH 8/8] Fixing type warnings form phpstan --- src/JwtAuthentication.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/JwtAuthentication.php b/src/JwtAuthentication.php index 0faab67..e0dab4c 100644 --- a/src/JwtAuthentication.php +++ b/src/JwtAuthentication.php @@ -82,7 +82,7 @@ final class JwtAuthentication implements MiddlewareInterface * secret?: string|array|array, * secure: bool, * relaxed: array, - * algorithm: array, + * algorithm: array|array, * header: string, * regexp: string, * cookie: string, @@ -322,7 +322,6 @@ private function decodeToken(string $token): array $decoded = JWT::decode( $token, $keys, - $this->options['algorithm'] ); return (array) $decoded; } catch (Exception $exception) { @@ -342,7 +341,7 @@ private function hydrate(array $data = []): void if ((is_array($data['secret']) || $data['secret'] instanceof ArrayAccess) && is_array($data['algorithm']) && count($data['algorithm']) === 1 - && count($data['secret']) > count($data['algorithm']) + && count((array) $data['secret']) > count($data['algorithm']) ) { $secretIndex = array_keys((array) $data['secret']); $data['algorithm'] = array_fill_keys($secretIndex, $data['algorithm'][0]); @@ -528,17 +527,23 @@ private function rules(array $rules): void } /** - * @return array + * @return array */ private function createKeysFromAlgorithms(): array { + if (!isset($this->options["secret"])) { + throw new InvalidArgumentException( + 'Secret must be either a string or an array of "kid" => "secret" pairs' + ); + } + $keyObjects = []; foreach ($this->options["algorithm"] as $kid => $algorithm) { $keyId = !is_numeric($kid) ? $kid : $algorithm; $secret = $this->options["secret"]; - if (is_array($this->options["secret"]) || $secret instanceof ArrayAccess) { + if (is_array($secret) || $secret instanceof ArrayAccess) { $secret = $this->options["secret"][$kid]; }