Conversation
google-cla
Bot
added
the
cla: yes
| if (isset($v['alg'])) { | ||
| $keys[$kid] = new Key($key, $v['alg']); | ||
| } else { | ||
| // The "alg" parameter is optional in a KTY, but is required | ||
| // for parsing in this library. Add it manually to your JWK | ||
| // array if it doesn't already exist. | ||
| // @see https://datatracker.ietf.org/doc/html/rfc7517#section-4.4 | ||
| throw new InvalidArgumentException('JWK key is missing "alg"'); | ||
| } |
There was a problem hiding this comment.
Hi @bshaffer
This is a breaking change that is currently not obvious from the 6.0 release notes.
As a notable example, Microsoft do not output JWK with the alg key populated:
https://login.microsoftonline.com/common/discovery/keys
I think the release notes should encourage developers to inspect JWK::parseKeySet beyond just its return type.
Thanks!
There was a problem hiding this comment.
I just updated the release notes, @Nextra, thanks for the feedback.
I think it would be a better user-experience to have a second optional argument $algorithm to parseKeySet, to help with this edgecase.
$algorithm = 'RS256'; // default algorithm used when "alg" isn't set
$jwks = JWK::parseKeySet($jwks, $algorithm);I didn't initially add it because I was hoping in practice that most JWKS would be populating the alg parameter. What do you think?
4 participants
TODO
algparameter inJWT::encoderequired (chore: make alg required for JWT::sign and JWT::encode #377)