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.

Sign up for GitHub

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

Jump to bottom

Optionally validate the payload of a token before verification #972

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Optionally validate the payload of a token before verification #972

wants to merge 2 commits into from

Conversation

georgejmx
Copy link

Description

Describe the purpose of this PR along with any background information and the impacts of the proposed change. For the benefit of the community, please do not assume prior context.

Provide details that support your chosen implementation, including: breaking changes, alternatives considered, changes to the API, etc.

Currently the only way to know that a token returned from jwt.verify() is of the correct format is by verifying the payload returned from the function. This MR makes it so the user can specify a callback payloadCallback that can be provided as an argument to jwt.verify(), which facilitates sanitising the token payload before it is verified

References

Include any links supporting this change such as a:

  • GitHub Issue/PR number addressed or fixed

Closes #955

Testing

Describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.

Please include any manual steps for testing end-to-end or functionality not covered by unit/integration tests.

Also include details of the environment this PR was developed in (language/platform/browser version).

  • This change adds full test coverage for new/changed/fixed functionality
  • This change was made on Ubuntu with Node 20.14

Checklist

  • I have added documentation for new/changed functionality in the README
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not the default branch

georgejmx
georgejmx commented Jun 9, 2024
View reviewed changes
test/payload_callback.test.js

it('should check that the payload satisfies the provided callback', function () {
const token = jwt.sign({ foo: 'bar' }, KEY);
const result = jwt.verify(token, KEY, undefined, undefined, testPayloadCallback);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not keen on the double undefined, however had concerns around making any other changes to the function signature wrt backward compatibility

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a link so I can verify

Copy link
Author

@georgejmx georgejmx Jul 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a repo that demos the feature https://github.com/georgejmx/demo-jsonwebtoken-payload-callback

Any more manual testing you need just let me know @Vero7979

This was referenced Jun 9, 2024
Open
Open
@georgejmx georgejmx changed the title Draft: Optionally verify the payload of a token before verification Optionally verify the payload of a token before verification Jun 9, 2024
@georgejmx georgejmx changed the title Optionally verify the payload of a token before verification Optionally validate the payload of a token before verification Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Vero7979 Vero7979 Vero7979 left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Verify an already decoded token
2 participants
@georgejmx @Vero7979