Skip to content

Conversation

@agubler
Copy link
Contributor

@agubler agubler commented Mar 31, 2025

Supports negative expiresIn option when signing tokens

Resolves #555

@agubler agubler requested a review from simoneb March 31, 2025 12:42
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. enhancement New feature or request labels Mar 31, 2025
@simoneb simoneb requested a review from Copilot March 31, 2025 13:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the token signing logic to support negative expiresIn values, which allows generating tokens with an already passed expiration time.

  • Added tests to verify behavior with negative expiresIn values in both the token verification and signing flows
  • Modified validation logic in the signer to allow negative expiresIn values

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
test/verifier.spec.js Added a test case to verify rejection of tokens with negative exp
test/signer.spec.js Added a test for signing tokens with negative expiresIn and removed validation tests that expected negative values to throw errors
src/signer.js Updated the validation logic to permit negative numbers for expiresIn
Comments suppressed due to low confidence (1)

src/signer.js:237

  • Now that negative expiresIn values are supported, consider updating the error message to remove the 'positive' requirement to prevent potential confusion.
if (typeof expiresIn !== 'number' || expiresIn < 0) {

Copy link
Member

@simoneb simoneb left a comment

Choose a reason for hiding this comment

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

LGTM, can you do a quick check if this is explicitly supported in the

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 31, 2025
@agubler
Copy link
Contributor Author

agubler commented Mar 31, 2025

@simoneb From a quick check there is not anything in the spec to say that negative or exp in the past are not allowed

@simoneb
Copy link
Member

simoneb commented Mar 31, 2025

@agubler I didn't actually want to post that comment, after verifying that we're good with negative exps :)

@agubler agubler merged commit 17fd233 into master Mar 31, 2025
6 checks passed
@agubler agubler deleted the feature/support-negative-expiry branch March 31, 2025 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Please review the "positive number" requirement for the "expiresIn" in the signer

2 participants