From 841dac7237a7a6da071181aff1212f59cd20db14 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       | 11 +++++++----
 tests/JWTTest.php | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/src/JWT.php b/src/JWT.php
index 37a9e0e6..51094452 100644
--- a/src/JWT.php
+++ b/src/JWT.php
@@ -460,19 +460,22 @@ 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])) {
             throw new UnexpectedValueException('"kid" invalid, unable to lookup correct key');
         }
diff --git a/tests/JWTTest.php b/tests/JWTTest.php
index d09d43e3..8dcc2e39 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 testInvalidKeyOrKeyArray()
+    {
+        $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');