Skip to content

fixed: refresh-token - #123

Open
Adeyemi-cmd wants to merge 1 commit into
StepFi-app:mainfrom
Adeyemi-cmd:refresh_token
Open

fixed: refresh-token#123
Adeyemi-cmd wants to merge 1 commit into
StepFi-app:mainfrom
Adeyemi-cmd:refresh_token

Conversation

@Adeyemi-cmd

Copy link
Copy Markdown
Contributor

Close #119

PR: Session families, refresh-token replay detection, blocked-user enforcement & session cleanup

Summary

This PR hardens the refresh-token rotation flow and closes three security/hygiene gaps in the auth module:

  1. No reuse detection — replaying an already-rotated refresh token previously returned a plain 401. Whoever presented the (still-valid) stolen token first won; the victim was silently logged out and no alarm was raised. Now, replay is treated as theft evidence: the entire session family is revoked, a auth.refresh_token_reuse audit event is written, and re-authentication is forced.
  2. Blocked users retained access — user status was only checked when minting tokens. A blocked account kept full API access until its access token naturally expired (up to 15 minutes). Now every authenticated request consults a short-TTL cached status check.
  3. Session rows accumulated forever — no GC existed for expired sessions. A new hourly cron job removes them.

Response shapes are backward-compatible: success responses are unchanged, and legacy refresh tokens without a family claim keep returning the original AUTH_SESSION_NOT_FOUND error.


Changes

1. Session families + refresh-token replay detection

Files: src/modules/auth/auth.service.ts, supabase/migrations/20260824000001_add_session_family_id.sql, src/modules/admin/audit.service.ts (consumed), src/modules/auth/auth.module.ts

  • Migration adds sessions.family_id UUID NOT NULL DEFAULT gen_random_uuid() with index idx_sessions_family_id.
  • All tokens minted from one login — or any of its subsequent refreshes — share one family_id. The refresh JWT now carries a fam claim ({ wallet, type: 'refresh', fam }).
  • On refresh:
    • Valid, live session → old row deleted, new pair minted into the same family (same behavior as before, plus family continuity).
    • Expired session → AUTH_SESSION_EXPIRED (unchanged).
    • Session row not found + valid signature + fam claim present → replay detected:
      • Every session in that family is deleted (theft containment).
      • An audit log entry is written via AuditService: action auth.refresh_token_reuse, resource session, metadata { family_id }, after-state { revoked_sessions: N }.
      • ERROR-level structured log lines emitted (replay detected + revocation count) so the event is observable in logs/Sentry.
      • Client receives 401 AUTH_REFRESH_TOKEN_REUSED and must sign in again.
    • Row not found but no fam claim (pre-migration token) → falls back to the original AUTH_SESSION_NOT_FOUND response; nothing to revoke.
  • Audit-log write failures never mask the revocation outcome — they are caught and logged.
  • AuthModule imports AdminModule (which exports AuditService); no circular dependency introduced.

2. Blocked-user enforcement on every request

Files: src/modules/auth/user-status.service.ts (new), src/modules/auth/jwt.strategy.ts

  • New UserStatusService: per-wallet status lookup with an in-memory TTL cache.
  • JwtStrategy.validate() is now async and calls ensureNotBlocked(wallet) after signature verification. Blocked wallets get 401 AUTH_USER_BLOCKED on every request instead of retaining access until token expiry.
  • Documented staleness bound: 30 seconds (USER_STATUS_CACHE_TTL_MS = 30_000). Blocking a wallet takes effect within ~30s on each instance, independent of the remaining access-token lifetime.
  • Design notes:
    • Local Map cache rather than Redis: the check runs on every request; a Redis round trip would double auth latency, and a 30s bound does not justify shared state. Multi-instance deployments each hold their own cache with the same bound.
    • Fails open: if the status query errors, the request proceeds as active and the failure is logged — a DB blip must not lock out every authenticated user. Negative results (blocked) are also cached for the TTL, so repeated denied requests do not hammer the DB.
    • invalidate(wallet) helper forces a fresh DB check (admin/test escape hatch).

3. Session cleanup cron job

Files: src/jobs/session-cleanup/session-cleanup.module.ts, src/jobs/session-cleanup/session-cleanup.service.ts (new), src/app.module.ts

  • Hourly @Cron(CronExpression.EVERY_HOUR) mirroring the established nonce-cleanup pattern (per architecture rules: @nestjs/schedule, no BullMQ).
  • Deletes only rows with expires_at older than 1 hour (grace window mirrors nonce cleanup and keeps borderline "expired" responses accurate).
  • Catches its own errors, logs them, never throws unhandled; logs the deleted-row count on success.
  • Registered in app.module.ts alongside the other job modules.

Files changed

File Change
supabase/migrations/20260824000001_add_session_family_id.sql New migration: family_id column + index
src/modules/auth/auth.service.ts Family-aware minting, replay detection + family revocation + audit event
src/modules/auth/auth.module.ts Import AdminModule, provide/export UserStatusService
src/modules/auth/jwt.strategy.ts Async validate(), blocked-user check
src/modules/auth/user-status.service.ts New: TTL-cached user status service
src/jobs/session-cleanup/* New: expired-session cleanup job
src/app.module.ts Register SessionCleanupModule
test/unit/modules/auth/auth.service.spec.ts Refresh/rotation/replay tests, updated assertions
test/unit/modules/auth/user-status.service.spec.ts New suite
test/unit/jobs/session-cleanup/session-cleanup.service.spec.ts New suite
context/progress-tracker.md Entry under 2026-08-24

Tests

New coverage (13 tests):

refreshTokens (auth.service.spec.ts):

  • rotates into the same session family (fam claim preserved across rotation)
  • deletes the presented session row on successful rotation
  • replayed (already-rotated) token → entire family revoked + auth.refresh_token_reuse audit event written + AUTH_REFRESH_TOKEN_REUSED
  • unknown legacy token without fam claim → original AUTH_SESSION_NOT_FOUND, no audit event
  • expired-but-present session → AUTH_SESSION_EXPIRED
  • refresh for blocked user → AUTH_USER_BLOCKED
  • invalid/expired refresh JWT → AUTH_REFRESH_TOKEN_INVALID

user-status.service.spec.ts:

  • DB lookup on first call; served from cache within TTL (single query for multiple requests)
  • re-queries once the cache entry expires
  • throws AUTH_USER_BLOCKED when status is blocked
  • invalidate() forces fresh check
  • fails open on DB error

session-cleanup.service.spec.ts:

  • deletes only rows past expires_at cutoff (cutoff asserted ≈ now − 1h)
  • logs deleted count
  • swallows delete errors and unexpected exceptions without throwing

Also updated existing generateTokens tests for the fam claim and verified the session insert carries the same family_id.

Verification

npm run build   ✅ passes (zero TS errors)
npm run lint    ✅ clean on all touched files
                (7 pre-existing @typescript-eslint/no-explicit-any errors exist on
                 the base branch in src/common/interceptors/audit.interceptor.ts and
                 src/jobs/transaction-status-checker/*.processor.ts — untouched here)
npm test        ✅ 364 passed / 364 total, 31 suites (was 351 tests)

Acceptance criteria

  • Refresh-token replay revokes the entire session family and is observable in logs/events (ERROR logs + audit_logs entry)
  • Blocked wallets lose API access within a bounded, documented delay (30 seconds, documented in code and progress tracker)
  • Expired sessions garbage-collected (hourly cron, deletes only expired rows)
  • Suite green; test count increased (351 → 364)
  • Response shapes backward-compatible (success payloads unchanged; legacy tokens retain original error)

Deployment notes

  • Apply the migration before/with deploy: existing session rows get a generated family_id; existing refresh tokens lack the fam claim and degrade gracefully to the old behavior until their next login.
  • No environment variables added.
  • Rollback is safe: reverting the code leaves an unused nullable-by-default column; old clients are unaffected at every step.

Risks / trade-offs

  • False-positive revocation on client retry: a legitimate client that retries a refresh with an already-consumed token will trigger family revocation by design (this is standard rotation semantics — the client simply signs in again).
  • Multi-instance staleness: each instance's status cache expires independently, but the worst-case staleness remains ≤ 30s everywhere.
  • Fail-open on status check: availability is prioritized over instant blocking during DB outages; the 30s cache plus fail-open behavior is documented.

@Adeyemi-cmd
Adeyemi-cmd requested a review from EmeditWeb as a code owner August 24, 2026 19:34
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.

hard: refresh-token rotation lacks reuse detection and blocked users keep live sessions

1 participant