Skip to content
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.

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

Edge case with unsigned JWTs. #131

Closed
torgeirsh opened this issue Feb 19, 2025 · 3 comments · Fixed by #132
Closed

Edge case with unsigned JWTs. #131

torgeirsh opened this issue Feb 19, 2025 · 3 comments · Fixed by #132

Comments

@torgeirsh
Copy link

I expected this code to work:

verifyJWT settingsWithNoneAlgorithmAdded (JWKSet []) tokenWithoutSignature

But it gives me a NoUsableKeys error. I imagine that the code assumes that all algorithms require a key, even if that's not the case. Including a very dummy key makes it work:

verifyJWT settingsWithNoneAlgorithmAdded (JWKSet [undefined]) tokenWithoutSignature

This is perhaps more of an inconvenience than a real bug, but it would nevertheless be useful for tests, even if it can be crudely worked around with undefined.

frasertweedale added a commit that referenced this issue Feb 19, 2025
You don't need a key to validate a "none" signature, but attempting
to do so fails with `NoUsableKeys` error.  Add a special case to the
JWS verification to handle this situation.

Fixes: #131
@frasertweedale
Copy link
Owner

@torgeirsh please take a look at #132 and let me know if it fixes the issue for you.

frasertweedale added a commit that referenced this issue Feb 19, 2025
You don't need a key to validate a "none" signature, but attempting
to do so fails with `NoUsableKeys` error.  Add a special case to the
JWS verification to handle this situation.

Fixes: #131
@frasertweedale
Copy link
Owner

@torgeirsh I'll cut a new release within the next few weeks. I'm not going to mention it in the release notes, because I don't want to advertise using the "none" algorithm 😅. But, thank you for the report, it was an interesting case and thankfully an easy fix.

@torgeirsh
Copy link
Author

That makes sense. Thanks again for the swift fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants