Skip to content

fix(auth): revoke web sessions on logout#2351

Open
jeanduplessis wants to merge 4 commits intomainfrom
fix/pentest-m2-session-handling
Open

fix(auth): revoke web sessions on logout#2351
jeanduplessis wants to merge 4 commits intomainfrom
fix/pentest-m2-session-handling

Conversation

@jeanduplessis
Copy link
Copy Markdown
Contributor

@jeanduplessis jeanduplessis commented Apr 13, 2026

Summary

Code-related pentest findings [private access to Kilo team members only] are now addressed without broad product rewrites. Each fix focuses on closing the reported behavior while preserving existing auth, analytics, payments, and device-token flows.

M2: Web session revocation after logout

  • Why needed: NextAuth logout only cleared the client cookie. A copied or stale web session cookie could still authenticate until normal JWT expiry.
  • Solution chosen: Added kilocode_users.web_session_version, stores that version in the NextAuth JWT/session at sign-in, and rejects requests in getUserFromAuth when the session version no longer matches the database row.
  • Logout behavior: Added POST /api/auth/revoke-web-session and call it from normal sign-out, auto sign-out, and dev account nuke flows before client-side sign-out.
  • Concurrent login behavior: This does not enforce a single active browser session. Multiple browsers can stay logged in at the same time because each sign-in receives the current version. Logging out from any browser increments the version and invalidates all existing web sessions for that user.
  • Legacy session behavior: NextAuth JWTs minted before this deploy do not contain webSessionVersion; those are treated as version 0 only while the database row is still 0. After any revocation increments the row, legacy cookies fail like any other stale session.
  • Counter decision: A numeric increment is sufficient because web_session_version is not a secret. It is a revocation counter inside a signed/encrypted NextAuth JWT. Knowing the next number does not help unless an attacker can mint a valid NextAuth token.
  • Trade-off: Did not rotate api_token_pepper on logout. That would have fixed stale web sessions, but it would also revoke CLI/device/API tokens unexpectedly. A separate web-only version keeps logout scoped to browser sessions.

M2: Route-level admin API authorization

  • Why needed: Admin credit category APIs were relying on middleware as the effective auth boundary. Middleware JWT state alone should not be the final authorization check for admin APIs.
  • Solution chosen: Added route-level getUserFromAuth({ adminOnly: true }) checks to the credit category admin API endpoints.
  • Current coverage: Non-credit admin API paths are not known to share the same gap. Most already call direct route-level admin auth. The remaining enrichment upsert route delegates through tRPC, where context calls getUserFromAuth and adminProcedure enforces ctx.user.is_admin.
  • Trade-off: Kept the change targeted to the middleware-only gaps identified in the plan instead of refactoring all admin routing helpers. If we want every /admin/api/** file to visibly call direct admin auth, the tRPC-backed enrichment route can be tightened separately for consistency.

Verification

  • Spec alignment: matched .plans/pentest-findings.md for code-related L1 scope.
  • Code evaluations: security, logic, types, data, resource, and style review agents ran across the implementation. Security review found a GTM dataLayer PII issue, the issue was fixed, and security re-review reported no findings.
  • Browser evaluations: agent-browser verified fake user login, CSP presence with nonce/Stripe sources, /api/user returning 401 after revocation with stale cookie, re-login after revocation, and stale admin route rejection for /admin/api/credit-categories.

Visual Changes

N/A

Reviewer Notes

Code Reviewer Notes
  • Session revocation uses a new kilocode_users.web_session_version column and stores the version in the NextAuth JWT/session at sign-in.
  • Numeric session versions are counters, not secrets. Security relies on NextAuth token integrity; the DB comparison provides revocation.
  • This is logout-everywhere for web sessions. It does not block concurrent browser logins.
  • Existing pre-deploy web sessions are treated as version 0 while web_session_version remains 0, avoiding a blanket logout on rollout.
  • getUserFromAuth rejects stale web sessions when the session version no longer matches the database row.
  • Logout entry points call POST /api/auth/revoke-web-session before client-side signOut; the endpoint is intentionally idempotent and only increments web session state.
  • The API token pepper check was not reused for logout because that would unexpectedly revoke CLI/device/API tokens.
  • Credit category admin APIs now have direct route-level admin auth; other admin APIs are either directly protected or protected through tRPC adminProcedure.
  • Migration was regenerated against current main as 0086_cool_night_thrasher after origin/main added its own 0085 migration.

@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot bot commented Apr 13, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (15 files)
  • apps/web/src/app/(app)/components/SidebarUserFooter.tsx
  • apps/web/src/app/admin/api/credit-categories/[key]/route.ts
  • apps/web/src/app/admin/api/credit-categories/route.ts
  • apps/web/src/app/api/auth/revoke-web-session/route.ts
  • apps/web/src/app/api/cloud-agent/sessions/prepare/route.test.ts
  • apps/web/src/app/auto-signout/page.tsx
  • apps/web/src/components/dev/actions.ts
  • apps/web/src/lib/token.test.ts
  • apps/web/src/lib/user.server.ts
  • apps/web/src/lib/web-session-revocation.test.ts
  • apps/web/src/lib/web-session-revocation.ts
  • apps/web/src/tests/helpers/user.helper.ts
  • apps/web/src/types/next-auth.d.ts
  • packages/db/src/migrations/0086_cool_night_thrasher.sql
  • packages/db/src/schema.ts

Reviewed by gpt-5.4-20260305 · 368,075 tokens

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.

1 participant