Merge pull request #250 from CodeForPhilly/fix/sticky-sessions

fix(auth): keep members signed in for months

Author: themightychris

apps/api/src/auth/cookies.ts

@@ -6,14 +6,15 @@
  * per specs/behaviors/authorization.md.
  */
 import type { FastifyReply } from 'fastify';
+import { REFRESH_TTL_SECONDS } from './jwt.js';
 
 const COOKIE_OPTS_BASE = {
   httpOnly: true,
   sameSite: 'lax' as const,
 };
 
 const ACCESS_TTL_MS = 15 * 60 * 1000;
-const REFRESH_TTL_MS = 30 * 24 * 60 * 60 * 1000;
+const REFRESH_TTL_MS = REFRESH_TTL_SECONDS * 1000;
 const CLAIM_TTL_MS = 5 * 60 * 1000;
 const OAUTH_TTL_MS = 10 * 60 * 1000;
 
@@ -41,6 +42,15 @@ export function setSessionCookies(
     path: '/api/auth/refresh',
     maxAge: REFRESH_TTL_MS / 1000,
   });
+
+  // Not HttpOnly and not a secret: tells the SPA a refresh is worth trying,
+  // so anonymous visitors never call /api/auth/refresh.
+  reply.setCookie('cfp_has_session', '1', {
+    sameSite: 'lax',
+    secure,
+    path: '/',
+    maxAge: REFRESH_TTL_MS / 1000,
+  });
 }
 
 export function setClaimCookie(reply: FastifyReply, token: string, nodeEnv: string): void {
@@ -55,6 +65,7 @@ export function setClaimCookie(reply: FastifyReply, token: string, nodeEnv: stri
 export function clearSessionCookies(reply: FastifyReply): void {
   reply.clearCookie('cfp_session', { path: '/' });
   reply.clearCookie('cfp_refresh', { path: '/api/auth/refresh' });
+  reply.clearCookie('cfp_has_session', { path: '/' });
 }
 
 export function clearClaimCookie(reply: FastifyReply): void {

apps/api/src/auth/jwt.ts

@@ -65,7 +65,8 @@ export interface ClaimPendingClaims {
 
 const CLOCK_SKEW_SECONDS = 60;
 const ACCESS_TTL_SECONDS = 15 * 60;
-const REFRESH_TTL_SECONDS = 30 * 24 * 60 * 60;
+/** Refresh-token lifetime: 90 days from the last refresh (specs/behaviors/authorization.md). */
+export const REFRESH_TTL_SECONDS = 90 * 24 * 60 * 60;
 const CLAIM_TTL_SECONDS = 5 * 60;
 
 function keyBytes(signingKey: string): Uint8Array {

apps/api/src/routes/account-claim.ts

@@ -28,7 +28,7 @@ import {
   UnauthenticatedError,
 } from '../lib/errors.js';
 import { requireAuth } from '../auth/guards.js';
-import { verifyClaimPending } from '../auth/jwt.js';
+import { REFRESH_TTL_SECONDS, verifyClaimPending } from '../auth/jwt.js';
 import {
   clearClaimCookie,
   setSessionCookies,
@@ -60,7 +60,7 @@ async function persistSession(
     userAgent: String(request.headers['user-agent'] ?? ''),
     ipAddress: clientIp(request),
     issuedAt: new Date(now).toISOString(),
-    expiresAt: new Date(now + 30 * 24 * 60 * 60 * 1000).toISOString(),
+    expiresAt: new Date(now + REFRESH_TTL_SECONDS * 1000).toISOString(),
   };
   await fastify.sessionMetadata.add(meta, fastify.store.private);
 }

apps/api/src/routes/auth.ts

@@ -15,7 +15,7 @@ import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify';
 import { errors as JoseErrors } from 'jose';
 import { ok } from '../lib/response.js';
 import { UnauthenticatedError, ConflictError, ApiNotFoundError } from '../lib/errors.js';
-import { verifyRefresh, issueSession } from '../auth/jwt.js';
+import { REFRESH_TTL_SECONDS, verifyRefresh, issueSession } from '../auth/jwt.js';
 import {
   setSessionCookies,
   clearSessionCookies,
@@ -95,7 +95,7 @@ async function persistSessionMetadata(
     userAgent: String(request.headers['user-agent'] ?? ''),
     ipAddress: clientIp(request),
     issuedAt: new Date(now).toISOString(),
-    expiresAt: new Date(now + 30 * 24 * 60 * 60 * 1000).toISOString(),
+    expiresAt: new Date(now + REFRESH_TTL_SECONDS * 1000).toISOString(),
   };
   await fastify.sessionMetadata.add(meta, fastify.store.private);
 }
@@ -691,14 +691,12 @@ export async function authRoutes(fastify: FastifyInstance): Promise<void> {
         claims.loginMethod ? { loginMethod: claims.loginMethod } : undefined,
       );
 
-      const oldExpiresAt = new Date(claims.exp * 1000).toISOString();
-      await fastify.revocations.revoke(
-        { jti: claims.jti, personId: claims.sub, expiresAt: oldExpiresAt },
-        fastify.store.public,
-      );
+      // No revocation of the old refresh token: refresh writes nothing to the
+      // public store (specs/behaviors/authorization.md → "Refresh writes
+      // nothing to the public store"). It stays valid until it expires.
       await fastify.sessionMetadata.remove(claims.jti, fastify.store.private);
 
-      const newExpiresAt = new Date(Date.now() + 30 * 24 * 60 * 60 * 1000).toISOString();
+      const newExpiresAt = new Date(Date.now() + REFRESH_TTL_SECONDS * 1000).toISOString();
       const newMeta: SessionMeta = {
         refreshJti: newTokens.refreshJti,
         personId: claims.sub,

apps/api/tests/auth.test.ts

@@ -293,8 +293,16 @@ describe('POST /api/auth/refresh', () => {
     expect(hasSession).toBe(true);
     expect(hasRefresh).toBe(true);
 
-    // Old refresh jti should now be revoked
-    expect(app.revocations.isRevoked(refreshJti)).toBe(true);
+    // 90 days from now, and the hint cookie that tells the SPA to refresh.
+    const refreshCookie = cookiesArr.find((c) => c.startsWith('cfp_refresh='))!;
+    expect(refreshCookie).toContain(`Max-Age=${90 * 24 * 60 * 60}`);
+    const hint = cookiesArr.find((c) => c.startsWith('cfp_has_session=1'));
+    expect(hint).toBeDefined();
+    expect(hint).not.toMatch(/HttpOnly/i);
+
+    // Refresh writes nothing to the public store: the old jti is not revoked
+    // (specs/behaviors/authorization.md).
+    expect(app.revocations.isRevoked(refreshJti)).toBe(false);
   });
 });
 
@@ -338,6 +346,7 @@ describe('POST /api/auth/logout', () => {
       : String(setCookieHeaders ?? '');
     expect(cookieStr).toContain('cfp_session=;');
     expect(cookieStr).toContain('cfp_refresh=;');
+    expect(cookieStr).toContain('cfp_has_session=;');
   });
 
   it('subsequent /api/auth/me after logout returns anonymous', async () => {

apps/web/src/hooks/useAuth.tsx

@@ -7,6 +7,7 @@ import {
   useState,
   type ReactNode,
 } from 'react';
+import { hasSessionHint, refreshSession } from '@/lib/api';
 
 export type AccountLevel = 'anonymous' | 'user' | 'staff' | 'administrator';
 
@@ -60,7 +61,7 @@ const ANON_SNAPSHOT: MeSnapshot = {
   lastLoginMethod: null,
 };
 
-async function fetchMe(): Promise<MeSnapshot> {
+async function fetchMeOnce(): Promise<MeSnapshot> {
   try {
     const res = await fetch('/api/auth/me', { credentials: 'include' });
     if (!res.ok) {
@@ -79,6 +80,17 @@ async function fetchMe(): Promise<MeSnapshot> {
   }
 }
 
+/**
+ * The signed-in state, renewing an expired access cookie first when the
+ * session hint says there is a refresh cookie to renew it from
+ * (specs/behaviors/authorization.md → "The SPA keeps the session alive").
+ */
+async function fetchMe(): Promise<MeSnapshot> {
+  const snap = await fetchMeOnce();
+  if (snap.person || !hasSessionHint()) return snap;
+  return (await refreshSession()) ? fetchMeOnce() : snap;
+}
+
 export function AuthProvider({ children }: { children: ReactNode }) {
   const [snapshot, setSnapshot] = useState<MeSnapshot>(ANON_SNAPSHOT);
   const [loading, setLoading] = useState(true);

apps/web/src/lib/api.ts

@@ -502,6 +502,30 @@ function buildQuery(params: object): string {
   return s ? `?${s}` : '';
 }
 
+// ---------------------------------------------------------------------------
+// Session refresh — specs/behaviors/authorization.md → "The SPA keeps the
+// session alive". The access cookie lives 15 minutes; the refresh cookie
+// 90 days. Concurrent callers share one in-flight refresh.
+// ---------------------------------------------------------------------------
+
+let refreshInFlight: Promise<boolean> | null = null;
+
+/** Whether the server left the session hint cookie (set with the refresh cookie). */
+export function hasSessionHint(): boolean {
+  return typeof document !== 'undefined' && /(?:^|;\s*)cfp_has_session=1(?:;|$)/.test(document.cookie);
+}
+
+/** Mint a fresh session from the refresh cookie. Resolves true on success. */
+export function refreshSession(): Promise<boolean> {
+  refreshInFlight ??= fetch('/api/auth/refresh', { method: 'POST', credentials: 'include' })
+    .then((res) => res.ok)
+    .catch(() => false)
+    .finally(() => {
+      refreshInFlight = null;
+    });
+  return refreshInFlight;
+}
+
 async function request<T>(path: string, init?: RequestInit): Promise<T> {
   const headers: Record<string, string> = {
     Accept: 'application/json',
@@ -517,11 +541,13 @@ async function request<T>(path: string, init?: RequestInit): Promise<T> {
   ) {
     headers['Content-Type'] = 'application/json';
   }
-  const res = await fetch(path, {
-    credentials: 'include',
-    ...init,
-    headers,
-  });
+  const send = () => fetch(path, { credentials: 'include', ...init, headers });
+  let res = await send();
+  // An expired access cookie: refresh once and retry. Auth endpoints answer
+  // 401 for their own reasons and are never retried.
+  if (res.status === 401 && !path.startsWith('/api/auth/') && (await refreshSession())) {
+    res = await send();
+  }
 
   // 204 No Content
   if (res.status === 204) {

apps/web/tests/useAuth.test.tsx

@@ -100,3 +100,71 @@ describe('useAuth', () => {
     });
   });
 });
+
+describe('session refresh (specs/behaviors/authorization.md → "The SPA keeps the session alive")', () => {
+  const me = (person: unknown) =>
+    new Response(JSON.stringify({ success: true, data: { person, accountLevel: person ? 'user' : 'anonymous' } }), {
+      status: 200,
+      headers: { 'content-type': 'application/json' },
+    });
+  const PERSON = { id: 'p1', slug: 'jane', fullName: 'Jane Doe', avatarUrl: null, accountLevel: 'user' };
+
+  afterEach(() => {
+    document.cookie = 'cfp_has_session=; max-age=0; path=/';
+    vi.restoreAllMocks();
+  });
+
+  it('refreshes on load when the hint cookie says a session exists', async () => {
+    document.cookie = 'cfp_has_session=1; path=/';
+    let signedIn = false;
+    const fetchMock = vi.spyOn(globalThis, 'fetch').mockImplementation(async (input) => {
+      const url = String(input);
+      if (url === '/api/auth/refresh') {
+        signedIn = true;
+        return new Response(JSON.stringify({ success: true, data: null }), { status: 200 });
+      }
+      return me(signedIn ? PERSON : null);
+    });
+
+    renderWithRouter(<Wrapped />);
+    expect(await screen.findByTestId('user')).toHaveTextContent('Jane Doe');
+    expect(fetchMock.mock.calls.filter(([u]) => String(u) === '/api/auth/refresh')).toHaveLength(1);
+  });
+
+  it('never refreshes for an anonymous visitor without the hint', async () => {
+    const fetchMock = vi.spyOn(globalThis, 'fetch').mockImplementation(async () => me(null));
+
+    renderWithRouter(<Wrapped />);
+    expect(await screen.findByTestId('anon')).toBeInTheDocument();
+    expect(fetchMock.mock.calls.some(([u]) => String(u) === '/api/auth/refresh')).toBe(false);
+  });
+
+  it('retries an API call once after refreshing on a 401', async () => {
+    const { api } = await import('../src/lib/api.js');
+    let refreshed = false;
+    const fetchMock = vi.spyOn(globalThis, 'fetch').mockImplementation(async (input) => {
+      const url = String(input);
+      if (url === '/api/auth/refresh') {
+        refreshed = true;
+        return new Response(JSON.stringify({ success: true, data: null }), { status: 200 });
+      }
+      if (!refreshed) {
+        return new Response(JSON.stringify({ success: false, error: { code: 'unauthenticated', message: 'no' } }), {
+          status: 401,
+          headers: { 'content-type': 'application/json' },
+        });
+      }
+      return new Response(JSON.stringify({ success: true, data: [], metadata: { totalItems: 0 } }), {
+        status: 200,
+        headers: { 'content-type': 'application/json' },
+      });
+    });
+
+    await api.projects.list({ perPage: 1 });
+    expect(fetchMock.mock.calls.map(([u]) => String(u))).toEqual([
+      '/api/projects?perPage=1',
+      '/api/auth/refresh',
+      '/api/projects?perPage=1',
+    ]);
+  });
+});

plans/sticky-sessions.md

@@ -0,0 +1,73 @@
+---
+status: done
+depends: []
+specs:
+  - specs/behaviors/authorization.md
+  - specs/api/conventions.md
+issues: []
+pr: 250
+---
+
+# Plan: Sessions that stick for months
+
+## Scope
+
+Chris (2026-10-15): "I get logged out soooo quick. I want the login to be
+super sticky like weeks/months." The spec already promised a 30-day
+sliding session, but the SPA never called `POST /api/auth/refresh`, so
+every session ended when the 15-minute access cookie expired. And the
+refresh endpoint revoked the old refresh token with a public commit,
+which the spec rules out ("no per-request writes anywhere").
+
+**In:** refresh TTL 90 days, sliding; a non-secret `cfp_has_session` hint
+cookie set and cleared with the refresh cookie; the SPA refreshes on load
+when the hint is present and `/me` says anonymous, and on any 401 (once,
+single-flight, then retries); refresh no longer writes a revocation.
+
+**Out:** refresh-token reuse detection (still deferred per the spec);
+longer access tokens.
+
+## Implements
+
+- [behaviors/authorization.md](../specs/behaviors/authorization.md#session-model--stateless-jwts)
+- [api/conventions.md](../specs/api/conventions.md) — cookie list.
+
+## Approach
+
+1. `REFRESH_TTL_SECONDS` (90 days) exported from `auth/jwt.ts`, used by the
+   JWT, the cookie and the session metadata.
+2. `setSessionCookies` / `clearSessionCookies` handle `cfp_has_session`.
+3. `refreshSession()` + `hasSessionHint()` in `lib/api.ts`; `request()`
+   retries after a refresh on 401 (not for `/api/auth/*`); `useAuth`
+   refreshes on load when hinted.
+
+## Validation
+
+- [x] `npm run type-check && npm run lint && npm test` clean.
+- [x] API test: refresh sets a 90-day refresh cookie and the hint, writes no
+  revocation; logout clears the hint.
+- [x] Web tests: load-time refresh only with the hint; a 401 refreshes once
+  and retries.
+- [x] Devbox: with the access cookie gone and the refresh cookie kept, a
+  reload comes back signed in.
+
+## Risks / unknowns
+
+- Members signed in before this ships have no hint cookie: their next
+  signed-in action still recovers through the 401 path, and the first
+  successful refresh sets the hint.
+
+## Notes
+
+- Devbox check (curl, the SPA's sequence): password sign-in set
+  `cfp_session`, `cfp_refresh` (90 days) and `cfp_has_session`; with the
+  access cookie dropped `/me` was anonymous, `POST /api/auth/refresh`
+  returned 200, and `/me` was signed in again. No data-repo commit came
+  from the refresh. The SPA's side is covered by the web tests.
+- Before this, the refresh endpoint revoked the old token with a public
+  commit on every refresh; it never fired in practice because nothing
+  called it.
+
+## Follow-ups
+
+- **None.**

specs/api/conventions.md

@@ -71,7 +71,7 @@ For paginated lists, the `data` is an array and `metadata` includes pagination:
 
 ## Authentication
 
-- Two cookies carry **stateless JWTs**: `cfp_session` (15-minute access JWT) and `cfp_refresh` (30-day refresh JWT, path-scoped to `/api/auth/refresh`). Cleared by `POST /api/auth/logout`. See [behaviors/authorization.md](../behaviors/authorization.md) for the full token model and [api/auth.md](auth.md) for the surviving session endpoints.
+- Two cookies carry **stateless JWTs**: `cfp_session` (15-minute access JWT) and `cfp_refresh` (90-day refresh JWT, path-scoped to `/api/auth/refresh`), plus the non-secret `cfp_has_session` hint. Cleared by `POST /api/auth/logout`. See [behaviors/authorization.md](../behaviors/authorization.md) for the full token model and [api/auth.md](auth.md) for the surviving session endpoints.
 - Both cookies: `HttpOnly`, `Secure`, `SameSite=Lax`. In development, `Secure` is dropped when the host is `localhost`.
 - There is no `sessions` table or sheet. Revocation is tracked in a small `revocations` sheet plus an in-memory `Set` of revoked `jti`s.
 - The endpoints that *issue* JWTs (GitHub OAuth callback, account-claim flow) are not yet specified. The endpoints that *manage* JWTs once issued are in [api/auth.md](auth.md).