Skip to content

fix: correct token TTL from 30 days to documented 24 hours#80

Closed
JanTvrdik wants to merge 1 commit intocontember:mainfrom
JanTvrdik:fix/75-token-ttl
Closed

fix: correct token TTL from 30 days to documented 24 hours#80
JanTvrdik wants to merge 1 commit intocontember:mainfrom
JanTvrdik:fix/75-token-ttl

Conversation

@JanTvrdik
Copy link
Copy Markdown
Contributor

Summary

  • Change TOKEN_TTL_SECS from 30 * 24 * 3600 (30 days) to 24 * 3600 (24 hours)
  • Update TOKEN_REFRESH_AGE_SECS comment for consistency
  • Matches the "24h TTL" documented in client config comments

Closes #75

Test plan

  • cargo test passes
  • Verify tokens expire after 24 hours, not 30 days
  • Verify client token refresh still works within the 24h window

Co-Authored-By: Claude Code

TOKEN_TTL_SECS was set to 30 days (2,592,000s) while documentation and
comments stated 24 hours. Reduce to 24 * 3600 (86,400s) to match the
documented behavior and limit the window of exposure for stolen tokens.

Also update TOKEN_REFRESH_AGE_SECS comment for consistency.

Closes contember#75

Co-Authored-By: Claude Code
Copilot AI review requested due to automatic review settings March 27, 2026 15:45
Copy link
Copy Markdown

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

Aligns remote auth token expiration behavior with the documented “24h TTL” to reduce the window of exposure from long-lived bearer tokens.

Changes:

  • Update server-side TOKEN_TTL_SECS from 30 days to 24 hours.
  • Update client-side TOKEN_REFRESH_AGE_SECS from 14 days to 20 hours (and clarify the comment) so refresh happens before the 24h server TTL.

Reviewed changes

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

File Description
src/remote/auth.rs Shortens server token TTL to 24h to match documentation and intended security posture.
crates/okena-core/src/client/types.rs Adjusts client refresh threshold to occur within the new 24h TTL window.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +88 to +89
/// Token age threshold for refresh (20 hours — must be shorter than the 24h server TTL).
pub const TOKEN_REFRESH_AGE_SECS: i64 = 20 * 3600;
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

TOKEN_REFRESH_AGE_SECS is now hard-coded relative to the server TTL (“must be shorter than the 24h server TTL”), but the client already receives expires_in from /v1/pair and /v1/refresh (currently ignored). Consider basing refresh scheduling on the server-provided expires_in (minus a safety margin) to avoid future drift between client and server TTLs.

Suggested change
/// Token age threshold for refresh (20 hours — must be shorter than the 24h server TTL).
pub const TOKEN_REFRESH_AGE_SECS: i64 = 20 * 3600;
/// Safety margin (in seconds) before token expiry when a refresh should be attempted.
/// Intended to be used in combination with the server-provided `expires_in` value.
pub const TOKEN_REFRESH_AGE_SECS: i64 = 4 * 3600;

Copilot uses AI. Check for mistakes.
Comment on lines +88 to +89
/// Token age threshold for refresh (20 hours — must be shorter than the 24h server TTL).
pub const TOKEN_REFRESH_AGE_SECS: i64 = 20 * 3600;
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

The PR description says this change is only a comment update for TOKEN_REFRESH_AGE_SECS, but the diff also changes the constant value from 14 days to 20 hours (behavioral change that affects refresh frequency). Please update the PR description (and any release notes/changelog if applicable) to reflect the runtime behavior change so reviewers/users don’t miss it.

Copilot uses AI. Check for mistakes.
matej21 added a commit that referenced this pull request Mar 27, 2026
The previous 30-day TTL with 14-day refresh threshold left a narrow
window for refresh and kept stolen tokens valid for too long.
New values: 14-day TTL with 3-day refresh — active clients refresh
early and live indefinitely, inactive clients survive up to 2 weeks.

Supersedes #80 which proposed 24h TTL (too aggressive for paired
desktops that may be unused for a week).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matej21
Copy link
Copy Markdown
Member

matej21 commented Mar 27, 2026

Closing in favor of #89 which uses 14-day TTL + 3-day refresh threshold — better balance between security and the use case of paired desktops that may be unused for a week.

@matej21 matej21 closed this Mar 27, 2026
matej21 added a commit that referenced this pull request Mar 30, 2026
The previous 30-day TTL with 14-day refresh threshold left a narrow
window for refresh and kept stolen tokens valid for too long.
New values: 14-day TTL with 3-day refresh — active clients refresh
early and live indefinitely, inactive clients survive up to 2 weeks.

Supersedes #80 which proposed 24h TTL (too aggressive for paired
desktops that may be unused for a week).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

Token TTL is 30 days but documented as 24 hours

3 participants