Skip to content

Require exp when verifying access tokens - #97

Open
SashaMIT wants to merge 2 commits into
livekit:mainfrom
SashaMIT:fix/require-exp-on-verify
Open

Require exp when verifying access tokens#97
SashaMIT wants to merge 2 commits into
livekit:mainfrom
SashaMIT:fix/require-exp-on-verify

Conversation

@SashaMIT

Copy link
Copy Markdown

Summary

TokenVerifier#verify called JWT.decode without required_claims. ruby-jwt only checks exp when the claim is present, so a valid HS256 token with no exp never expired.

This sets required_claims: ["exp"]. Language-split of livekit/protocol#1706, livekit/python-sdks#779, and livekit/node-sdks#710. Distinct from #88 (jwt 3.x bump).

Test plan

  • bundle exec rspec 23 examples, 0 failures (2 pending mock-server)
  • New case: signed token with no exp raises JWT::MissingRequiredClaim
  • Revert-test: drop required_claims and that case fails (token is accepted)

Made with Cursor

ruby-jwt only checks expiry when the claim is present. A signed token with no exp therefore never expired. Match protocol#1706, python-sdks#779, and node-sdks#710.

@frostbyte73 frostbyte73 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See below

true,
{
algorithm: AccessToken::SIGNING_ALGORITHM,
required_claims: ["exp"],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

required_claims landed in ruby-jwt v2.3.0 - Gemfile.lock needs to bump the minimum from v2.2.3

required_claims landed in ruby-jwt 2.3.0. Keep the resolved 2.10.3, raise the floor so verify cannot install a gem that ignores the option.
@SashaMIT

Copy link
Copy Markdown
Author

Thanks. Floor is now jwt >= 2.3.0 in the gemspec and Gemfile.lock. Resolved jwt stays 2.10.3. required_claims is a no-op below 2.3.0, so that floor is the real fix.

bundle exec rspec still 23 examples, 0 failures (2 pending mock-server).

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 this pull request may close these issues.

2 participants