Fix/jwt verify decode jose#2648
Open
alleria173 wants to merge 5 commits into
Open
Conversation
…nse' label for cases where reviewer asks a question
Temporary: needed so pull_request_review/pull_request_target/issue_comment triggers are recognised for the throwaway test PR. Revert this merge commit once testing is done to keep main aligned with upstream/master.
jsonwebtoken is unmaintained and does not work correctly in the browser at 9.x. Following the JWT Sign migration (gchq#2473), switch the remaining two JWT operations to jose and remove the jsonwebtoken dependency. Unsigned (alg:none) tokens are now rejected by Verify, and PKCS#1 RSA public keys are converted to SPKI for WebCrypto.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replaces the remaining uses of
jsonwebtoken(JWT Verify and JWT Decode) with thejoselibrary, and removes thejsonwebtokendependency, completing the migration started by #2473 for JWT Sign.decodeJwt. Malformed tokens produce a clear error instead of an unhandledTypeError.jwtVerify. The key is detected by its PEM header: SPKI public keys, PKCS#1 RSA public keys (converted to SPKI via a newpkcs1ToSpkihelper inlib/RSA.mjs, mirroring Fix jwt sign #2473'spkcs1ToPkcs8), X.509 certificates, or an HMAC secret. The algorithm is taken from the token header and checked against the supported list.JWT_ALGORITHMSarray exported fromlib/JWT.mjs, replacing"None"with"none"in JWT Sign's dropdown list.Behaviour changes to be aware of, both inherited from jose's stricter security posture:
"alg": "none") are no longer accepted by JWT Verify. They now produce an error directing the user to JWT Decode. The old behaviour (any unsigned token "verifies" against any key) is a well-known JWT attack vector.expandnbfclaims are now validated during verification, with dedicated error messages.This PR is blocked by #2473: it removes the
jsonwebtokendependency, which JWT Sign still imports until #2473 merges. CI will fail on the JWT Sign tests until then; once #2473 is merged I will rebase (the only overlaps are additive: both PRs append a helper tolib/RSA.mjs, enable a JWT line intests/browser/02_ops.js, and add"jose": "^6.2.3"to package.json).Existing Issue
Fixes #2472
Screenshots
N/A — no visual changes. The JWT Verify operation description text was updated to mention certificate support, claim validation, and the unsigned-token rejection.
AI disclosure
Used Claude Code (Anthropic) to help write this migration and the tests. I have reviewed the code myself and understand how it works.
Test Coverage
tests/operations/tests/JWTVerify.mjs: the existing HS256/RS256/ES256 verification tests are retained (RS256 regenerated with a 2048-bit key). New tests cover: PKCS#1 RSA public key input, sub-2048-bit RSA key rejection, wrong-key signature failure, expired token, and unsigned (alg: none) token rejection.tests/operations/tests/JWTDecode.mjs: existing tests unchanged; added a malformed-token error test.tests/browser/02_ops.js: enabled the previously commented-out sanity tests for JWT Decode and JWT Verify.npm test) and lint run locally on an integration branch with Fix jwt sign #2473 merged in, since this branch alone cannot pass while JWT Sign still needs jsonwebtoken.