Skip to content

Commit

Permalink
Allow JWT::decode to accept an empty string as a valid kid
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rneufeldcisco committed Oct 23, 2024
1 parent 76808fa commit a09f76f
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 7 deletions.
12 changes: 5 additions & 7 deletions src/JWT.php
Original file line number Diff line number Diff line change
Expand Up @@ -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])) {
Expand Down
33 changes: 33 additions & 0 deletions tests/JWTTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down Expand Up @@ -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');
Expand Down

0 comments on commit a09f76f

Please sign in to comment.