Skip to content

bugfix: 'kid' not in given key list #129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 21, 2017
Merged

bugfix: 'kid' not in given key list #129

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
491ab3a
bugfix: 'kid' not in given key list
stampycode Dec 12, 2016
64d7eb0
cosmetic updates
stampycode Jan 24, 2017
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/JWT.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ public static function decode($jwt, $key, $allowed_algs = array())
}
if (is_array($key) || $key instanceof \ArrayAccess) {
if (isset($header->kid)) {
if(!isset($key[$header->kid])) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a space after if
throw new UnexpectedValueException('"kid" not found in key map, unable to lookup correct key');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kid not found in the map indicates an invalid kid, so a better error message would be simply Invalid "kid". Another option would be to set the value of $key to null here, so the OpenSSL unable to verify data error is thrown. I prefer the first option, however.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to have a distinct exception here, as the kid not being present in the list can then be used to trigger the key list being re-downloaded from the source, and updating the locally cached key list. I have a suspicion the key-list download endpoint is deliberately slow to encourage developers the cache the list and update periodically...
}
$key = $key[$header->kid];
} else {
throw new UnexpectedValueException('"kid" empty, unable to lookup correct key');
Expand Down