From 993a11c9ed99b4c2a2b45e87a96839a0703f2317 Mon Sep 17 00:00:00 2001
From: Ryan Neufeld <ryan@neufeldmail.com>
Date: Wed, 23 Oct 2024 10:58:03 -0700
Subject: [PATCH] Allow JWT::decode to accept an empty string as a valid kid

There are instances when using CachedKeySet where a key is returned with an empty string as the kid.
This is a valid use case and should be allowed.

For example Teleport Proxy uses this pattern to allow for a default key.

The getKey method can be simplified, as well as refactored to follow the same pattern as
the CachedKeySet class which casts null kids to an empty string.

This change also adds a test to ensure that an empty string kid is a valid kid.
---
 src/JWT.php       | 12 +++++-------
 tests/JWTTest.php | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/src/JWT.php b/src/JWT.php
index 9100bf0f..7d6c3713 100644
--- a/src/JWT.php
+++ b/src/JWT.php
@@ -465,17 +465,15 @@ private static function getKey(
         $keyOrKeyArray,
         ?string $kid
     ): Key {
+
+        $kid = (string) $kid;
+
         if ($keyOrKeyArray instanceof Key) {
             return $keyOrKeyArray;
         }
 
-        if (empty($kid) && $kid !== '0') {
-            throw new UnexpectedValueException('"kid" empty, unable to lookup correct key');
-        }
-
-        if ($keyOrKeyArray instanceof CachedKeySet) {
-            // Skip "isset" check, as this will automatically refresh if not set
-            return $keyOrKeyArray[$kid];
+        if (!is_array($keyOrKeyArray) && !$keyOrKeyArray instanceof ArrayAccess) {
+            throw new UnexpectedValueException('Expecting a Key or an associative array of keys');
         }
 
         if (!isset($keyOrKeyArray[$kid])) {
diff --git a/tests/JWTTest.php b/tests/JWTTest.php
index d09d43e3..f42e6775 100644
--- a/tests/JWTTest.php
+++ b/tests/JWTTest.php
@@ -327,6 +327,19 @@ public function testKIDChooser()
         $this->assertEquals($decoded, $expected);
     }
 
+    public function testArrayAccessKIDChooserWhenJWTHasNoKey()
+    {
+        $key = new Key('my_key0', 'HS256');
+        $keys = new ArrayObject([
+            '' => $key,
+        ]);
+        $msg = JWT::encode(['message' => 'abc'], $key->getKeyMaterial(), 'HS256');
+        $decoded = JWT::decode($msg, $keys);
+        $expected = new stdClass();
+        $expected->message = 'abc';
+        $this->assertEquals($decoded, $expected);
+    }
+
     public function testArrayAccessKIDChooser()
     {
         $keys = new ArrayObject([
@@ -383,6 +396,26 @@ public function testInvalidSignatureEncoding()
         JWT::decode($msg, new Key('secret', 'HS256'));
     }
 
+    public function testInvalideKeyOrKeyArray()
+    {
+        $key = 'yma6Hq4XQegCVND8ef23OYgxSrC3IKqk';
+        $payload = ['foo' => [1, 2, 3]];
+        $jwt = JWT::encode($payload, $key, 'HS256');
+        $this->expectException(UnexpectedValueException::class);
+        $this->expectExceptionMessage('Expecting a Key or an associative array of keys');
+        JWT::decode($jwt, 'SomeKeyNotAnArray');
+    }
+
+    public function testKeyNotInKeyOrKeyArray()
+    {
+        $key = 'yma6Hq4XQegCVND8ef23OYgxSrC3IKqk';
+        $payload = ['foo' => [1, 2, 3]];
+        $jwt = JWT::encode($payload, $key, 'HS256');
+        $this->expectException(UnexpectedValueException::class);
+        $this->expectExceptionMessage('"kid" invalid, unable to lookup correct key');
+        JWT::decode($jwt, ['notrealkey' => 'SomeKeyNotAnArray']);
+    }
+
     public function testHSEncodeDecode()
     {
         $msg = JWT::encode(['message' => 'abc'], 'my_key', 'HS256');