The failure tests 'should throw error when signature is wrong' test case was incorrectly building a test input in two ways:
The payload content was encoded as a base64 string, which in this case added special padding chars ('='). These chars do not comply with the JWT specification, and are expected to not be present by downstreams that perform the token validation and verification. This change fixes the test token to remove the padding chars, the same way this is done in the downstream (code pointer).
Also, the payload content was being built from a string literal, meant to represent a stringified POJO (aka a JSON string), which was ill-formed (missing quotes). This change fixes the payload to be well-formed.
Additionally, the assertion of the error message was incorrect, this change fixes it to assert the message as "invalid signature".
The reason why 1. was not caught is that the test token technically still failed validation, but at the earlier stage of checking format validity, and was caught as the same error (with a different message). The reason why 2. was not caught was that the token payload was never parsed, as it failed at an earlier step due to 1.
Reviewers should test that this test case actually exercises the token signature verification, by ensuring that the isValidJWS() function from the node-jws module dependency (code pointer) returns true. Namely, the regex test against var JWS_REGEX = /^[a-zA-Z0-9\-_]+?\.[a-zA-Z0-9\-_]+?\.([a-zA-Z0-9\-_]+)?$/; should pass.
[X] This change adds test coverage for new/changed/fixed functionality
Checklist
[ ] I have added documentation for new/changed functionality in this PR or in auth0.com/docs
[X] All active GitHub checks for tests, formatting, and security are passing
[X] The correct base branch is being used, if not the default branch
jfromaniello
commented
1 year ago
Description
The failure tests 'should throw error when signature is wrong' test case was incorrectly building a test input in two ways:
Additionally, the assertion of the error message was incorrect, this change fixes it to assert the message as "invalid signature".
The reason why 1. was not caught is that the test token technically still failed validation, but at the earlier stage of checking format validity, and was caught as the same error (with a different message). The reason why 2. was not caught was that the token payload was never parsed, as it failed at an earlier step due to 1.
References
See also https://github.com/auth0/node-jws/issues/49 for details about forbidding '=' base64 padding characters in complying with specification
Testing
Reviewers should test that this test case actually exercises the token signature verification, by ensuring that the
isValidJWS()function from thenode-jwsmodule dependency (code pointer) returns true. Namely, the regex test againstvar JWS_REGEX = /^[a-zA-Z0-9\-_]+?\.[a-zA-Z0-9\-_]+?\.([a-zA-Z0-9\-_]+)?$/;should pass.Checklist