trackslash
TRACK-65 P3

Session sweep revokes by age, so expired sessions linger for three years

0
All issues

Description

Problem

Sessions are issued with an expiry (TRACK_SLASH_SESSION_TTL, default 7 days), but nothing revokes them when that expiry passes. The sweep in 0040_session_token_expiry.sql selects on created_at < now() - INTERVAL '3 years' and ignores expires_at entirely.

A session is therefore dead for roughly three years before its row is marked revoked. revoked_at IS NULL stops meaning "usable" about a week after sign-in.

This is not an authentication hole: AuthenticateToken checks expires_at > now(), so an expired session cannot sign anyone in. It is a data-hygiene problem, and it has already produced one user-visible bug — the Tokens page counted those rows as active web sessions (TRACK-44, fixed in #147 by filtering in the UI rather than at the source).

Proposed direction

Sweep on the expiry the token actually carries. Revoking expires_at < now() alongside the existing age rule keeps revoked_at IS NULL aligned with what the token can do, and lets callers reason about liveness from revoked_at again.

Worth deciding as part of this: whether the three-year age rule stays as a backstop for sessions with a NULL expires_at, and whether swept rows should eventually be deleted rather than left revoked forever.

Acceptance criteria

  • The sweep revokes sessions whose expires_at has passed, not only those older than three years.
  • The trigger keeps sparing the token being refreshed, and keeps its LIMIT so a single request cannot be made to wait on an unbounded update.
  • API tokens are still never touched by the sweep.
  • uiPartitionAuthTokens is revisited: with the source corrected, its expiry filter may become redundant, but it should stay unless the DB guarantees the invariant.
  • Migration test covers an expired-but-young session being swept and an unexpired session surviving.

Sub-issues

0

Linked issues

0

GitHub

0

No branches or pull requests linked.

Comments

1
Bradley

Fixed in #149, merged to main as 55f79b5. CI green on main.

Change: 0041_sweep_expired_sessions.sql replaces the sweep function so a session goes when its own expiry has passed:

AND (expires_at < now() OR created_at < now() - INTERVAL '3 years')

Decisions on the open questions in this ticket:

  • The three-year age rule stays as the backstop for sessions with no expires_at — expires_at < now() is NULL for those rows, so they fall through to it, and nothing else would ever clear them.
  • Swept rows stay revoked rather than being deleted. That is a retention decision, not a correctness one, and nothing currently reads revoked rows in a way that suffers. Worth a separate ticket if the table growth ever matters.
  • uiPartitionAuthTokens keeps its expiry filter. The sweep is lazy — at most hourly, and only on the back of a token refresh — so the database does not guarantee the invariant at render time. Its comment claimed sessions "are only swept long after" expiry, which this change makes false, so it now describes the laziness instead.

Unchanged: API tokens are never touched, the refreshing token is spared, the LIMIT 1000 bounds one refresh's work, and the hourly claim still rate-limits the sweep. A partial index on (kind, expires_at) WHERE revoked_at IS NULL keeps the new predicate off a full scan. The Down migration restores the age-only body and is exercised by the existing goose.DownTo(..., 28) test.

Coverage: TestSessionSweepRevokesExpiredSessions covers expired session swept, unexpired survives, expired API token survives, young no-expiry session survives on the age rule, refreshing token spared. Verified non-vacuous — with 0041 removed it fails on "an expired session survived the sweep".