jwt-cli
jwt-cli copied to clipboard
mike-engel
feat: Improved error messages/warnings
New: If no secret/publicKey is provided by the user, no signature check is performed. Rather then silently skipping the signature check, this commit introduces a warning in this case.
New: If the exp claim is not set and the token validation check is
skipped, a warning is shown.
New: After a successful signature validation a success message is shown.
Changed: The InvalidAlgorithm error message now shows the selected
algorithm and the algorithm specified in the JWT header.
New: If no signature validation is performed, the return code is now
2.
All tests have been updated accordingly.
Summary
Preflight checklist
- [x] Code formatted with rustfmt
- [x] Relevant tests added
- [x] Any new documentation added (I don't think this requires additional documentation).
Thanks for making this @codedust! I like all of the changes other than the new exit status. I have a feeling this may break more workflows than it seems, and I wonder if a new --validate flag should be added, which would then return an exit status of 2 like you have. What do you think?
This is intended behavior. To prevent users from accidentally skipping signature validation, signature validation should be the default . An exit status of 0 indicates that everything is fine. If token validation fails, the exit status is set to 1.
What changes with the merge request is, that if no secret is given via the --secret flag (and signature validation cannot be performed), the exit status is set to 2. If workflows break because the jwt signature cannot be verified, it's not a bug, it's a feature.
I would strongly argue against an exit status of 0 when no signature verification has been performed (since this suggests "everything is fine").
From my point of view, the following two options would also be fine:
- a) introduce a new
--skip-signature-validationflag that explicitly sets the exit status to0even if the signature could not be verified - b) return an exit status of
1if the signature could not be verified because of a missing secret key (instead of2)
@codedust the original intent of this tool wasn't only for verification—it was originally just a way to view the contents of a JWT. Verification was secondary. I'm unsure how many people use it just to view the contents of a token, and how many use that in an automated fashion, where a non-zero exit code would break it. I'll need to think about it some more, but I'm still not convinced the default should be changed. I'd rather add a verify sub command
I think, adding a verify command would be a good solution. This way, decode could still be used to simply show the contents of a jwt and verify could be used to enforce signature verification. I'll give it a try.
I've now added the verify subcommand and rebased to main.
Now, the decode subcommand does not verify any signatures any more and does not expect the --secret, ignore_expand--alg` parameters. If this breaking change is acceptable, we're fine.
If backwards compatibility is really important, we could also add the parameters to the decode subcommand again but ignore them in the implementation. However, that's not really great cli design.
An alternative would be to add signature verification to the decode subcommand again if the --secret parameter is given. We should think about when to return an exit code of 1 in this case, though.
What do you think?
An alternative would be to add signature verification to the decode subcommand again if the --secret parameter is given. We should think about when to return an exit code of 1 in this case, though.
I kind of like this approach, since it shortcuts the occasional need to do both verification and decoding. It will probably also be useful if this tool ever handles encrypted tokens.
It seems like the exit code should always be 0 unless there was an error decoding, though