From 20aa7ab92043757c0e24da8347df5e8fa5567399 Mon Sep 17 00:00:00 2001 From: Tom Riglar Date: Thu, 9 Jul 2026 10:48:08 +0100 Subject: [PATCH] fix: recover cleanly when the stored session is dead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Companion to devicecloud-dev/dcd#1084 (which fixes the root cause of "Invalid Refresh Token: Already Used" by minting the CLI its own session at login). This fixes the recovery path for sessions that are already broken: - CliAuthGateway.refresh now throws a typed SessionRefreshError whose `definitive` flag distinguishes a GoTrue 4xx token rejection from transient failures (network, 5xx). - On a definitive rejection, resolveAuth drops the dead session from the config (keeping env/api_url/org), so subsequent commands report a clean "Not authenticated" instead of failing the same refresh every time. - `dcd login` no longer dead-ends users whose session has expired: the "Already logged in… Sign out and log in again?" confirm is skipped for expired sessions (it proceeds straight to sign-in), and defaults to Yes for live ones — running `dcd login` deliberately almost always means "log me in". Also updates the login-flow comments and CLAUDE.md for the new handoff contract (access token as identity proof; api mints a dedicated session). Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 2 +- src/commands/login.ts | 43 ++++++++++++++++++++---------- src/gateways/cli-auth-gateway.ts | 30 ++++++++++++++++++--- src/utils/auth.ts | 37 +++++++++++++++++++++----- test/unit/auth.test.ts | 45 +++++++++++++++++++++++++++++++- 5 files changed, 132 insertions(+), 25 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 985e4a3..9b32b1f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,7 +39,7 @@ Top-level `defineCommand` in `src/index.ts` wires eleven subcommands (`cloud`, ` **Auth.** Every command calls `resolveAuth({ apiKeyFlag })` (`src/utils/auth.ts`) once and threads the returned `AuthContext` into gateways/services. `ApiGateway` and `fetchCompatibilityData` spread `auth.headers` into fetch headers — they no longer accept a raw api key. Precedence: `--api-key` flag > `DEVICE_CLOUD_API_KEY` env > stored session from `dcd login`. `resolveAuth` refreshes expiring Supabase sessions via `CliAuthGateway.refresh` and rewrites the config atomically. -**Config store.** `dcd login` writes `$XDG_CONFIG_HOME/dcd/config.json` (fallback `~/.dcd/config.json`, 0600). Shape: `{ version, env, api_url, supabase_url, session: { access_token, refresh_token, expires_at, user_email, user_id }, current_org_id, current_org_name }`. `DCD_CONFIG_DIR` overrides the directory (used by tests). The login command itself (`src/commands/login.ts`) uses PKCE (S256) with a server rendezvous — no loopback server: it mints `state`, `code_verifier`, and `code_challenge`, opens `/cli-login?state=...&code_challenge=...`, then polls the dcd API's `POST /cli-login/claim` with `{state, code_verifier}` while the frontend POSTs the session to `POST /cli-login/handoff`; the API verifies `sha256(verifier) === challenge` and returns the session. After claiming, the CLI fetches `/me/orgs` and prompts for an org (the same picker `dcd switch-org` uses). The frontend lives in `../dcd/frontend/app/features/cli-login/CliLoginScreen.tsx`. +**Config store.** `dcd login` writes `$XDG_CONFIG_HOME/dcd/config.json` (fallback `~/.dcd/config.json`, 0600). Shape: `{ version, env, api_url, supabase_url, session: { access_token, refresh_token, expires_at, user_email, user_id }, current_org_id, current_org_name }`. `DCD_CONFIG_DIR` overrides the directory (used by tests). The login command itself (`src/commands/login.ts`) uses PKCE (S256) with a server rendezvous — no loopback server: it mints `state`, `code_verifier`, and `code_challenge`, opens `/cli-login?state=...&code_challenge=...`, then polls the dcd API's `POST /cli-login/claim` with `{state, code_verifier}` while the frontend POSTs proof of identity (the browser's access token) to `POST /cli-login/handoff`; the API verifies the token, **mints a dedicated Supabase session for the CLI** (its own refresh-token family — sharing the browser's tokens caused "Invalid Refresh Token: Already Used" whenever either client rotated them), stores it keyed by state, then on claim verifies `sha256(verifier) === challenge` and returns it. After claiming, the CLI fetches `/me/orgs` and prompts for an org (the same picker `dcd switch-org` uses). The frontend lives in `../dcd/frontend/app/features/cli-login/CliLoginScreen.tsx`. **Cross-repo auth surface.** The dcd API's `ApiKeyGuard` accepts either `x-app-api-key` (existing) or `Authorization: Bearer ` + `x-dcd-org: `. For Bearer it verifies the JWT, checks `user_org_profile` membership, and injects the org's api_key back into the request headers so existing `@Headers(APP_API_KEY_HEADER)` controller code keeps working unchanged. `dcd switch-org` calls `GET /me/orgs`, a JWT-only endpoint at `../dcd/api/src/apps/me/me.controller.ts`. diff --git a/src/commands/login.ts b/src/commands/login.ts index b451813..ba6e9dc 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -10,8 +10,11 @@ * 2. CLI opens /cli-login?state=...&code_challenge=... in the browser. * 3. User signs in (OTP or SSO) and explicitly authorizes the handoff on * the frontend. - * 4. Frontend POSTs {state, code_challenge, session...} to the dcd api at - * POST /cli-login/handoff. The api stores a short-TTL row keyed by state. + * 4. Frontend POSTs {state, code_challenge, access_token} to the dcd api at + * POST /cli-login/handoff — the access token is proof of identity only. + * The api verifies it, mints a *dedicated* Supabase session for the CLI + * (its own refresh-token family, so browser token rotation can't + * invalidate it), and stores it in a short-TTL row keyed by state. * 5. Meanwhile, the CLI polls POST /cli-login/claim with {state, code_verifier}. * Once the api has the row, it verifies sha256(verifier) === challenge, * deletes the row, and returns the session. @@ -81,20 +84,32 @@ export const loginCommand = defineCommand({ // If there's an existing stored session, make the user confirm before we // overwrite it. Silent clobber is fine for power users but surprising if - // someone runs `dcd login` by mistake while already authenticated. + // someone runs `dcd login` by mistake while already authenticated. An + // already-expired session gets no confirm: the user is here because a + // command told them to re-login, and "Already logged in… keep session?" + // would dead-end them on a session that no longer works. const existing = readConfig(); if (existing?.session) { - const currentOrg = existing.current_org_name ?? existing.current_org_id; - const ok = await p.confirm({ - message: - `Already logged in as ${existing.session.user_email}` + - (currentOrg ? ` (org ${currentOrg})` : '') + - `. Sign out and log in again?`, - initialValue: false, - }); - if (p.isCancel(ok) || !ok) { - logger.log(ui.info('Keeping existing session.')); - return; + const now = Math.floor(Date.now() / 1000); + if (existing.session.expires_at <= now) { + logger.log( + ui.info( + `Your session for ${existing.session.user_email} has expired — signing in again.`, + ), + ); + } else { + const currentOrg = existing.current_org_name ?? existing.current_org_id; + const ok = await p.confirm({ + message: + `Already logged in as ${existing.session.user_email}` + + (currentOrg ? ` (org ${currentOrg})` : '') + + `. Sign out and log in again?`, + initialValue: true, + }); + if (p.isCancel(ok) || !ok) { + logger.log(ui.info('Keeping existing session.')); + return; + } } } diff --git a/src/gateways/cli-auth-gateway.ts b/src/gateways/cli-auth-gateway.ts index 247609a..35b10cf 100644 --- a/src/gateways/cli-auth-gateway.ts +++ b/src/gateways/cli-auth-gateway.ts @@ -3,12 +3,28 @@ * and sign-out (best-effort revocation on the Supabase side). * * Does not talk to the dcd API — the dcd-side session exchange lives in the - * login command's loopback flow, where the frontend POSTs ciphertext back. + * login command's PKCE rendezvous flow (see src/commands/login.ts). */ -import { createClient } from '@supabase/supabase-js'; +import { createClient, isAuthApiError } from '@supabase/supabase-js'; import type { StoredSession } from '../utils/config-store.js'; +/** + * Thrown when a session refresh fails. `definitive` distinguishes "Supabase + * rejected this refresh token" (revoked, already used, malformed — re-login + * is the only fix) from transient failures (network, GoTrue 5xx) where the + * stored session may still be good on the next attempt. + */ +export class SessionRefreshError extends Error { + constructor( + message: string, + readonly definitive: boolean, + ) { + super(message); + this.name = 'SessionRefreshError'; + } +} + export interface RefreshedSession { access_token: string; refresh_token: string; @@ -35,9 +51,17 @@ export const CliAuthGateway = { refresh_token: session.refresh_token, }); if (error || !data.session || !data.user) { - throw new Error( + // 4xx from GoTrue means the token itself was rejected; anything else + // (fetch failure, 5xx) could succeed on retry with the same token. + const definitive = + error != null && + isAuthApiError(error) && + error.status >= 400 && + error.status < 500; + throw new SessionRefreshError( `Failed to refresh session: ${error?.message ?? 'no session returned'}. ` + `Run \`dcd login\` again.`, + definitive, ); } const s = data.session; diff --git a/src/utils/auth.ts b/src/utils/auth.ts index 62a376f..21aecac 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -10,7 +10,10 @@ import { closeSync, openSync, rmSync, statSync } from 'node:fs'; import { ENVIRONMENTS } from '../config/environments.js'; -import { CliAuthGateway } from '../gateways/cli-auth-gateway.js'; +import { + CliAuthGateway, + SessionRefreshError, +} from '../gateways/cli-auth-gateway.js'; import { telemetry } from '../services/telemetry.service.js'; import type { AuthContext } from '../types/domain/auth.types.js'; @@ -124,11 +127,19 @@ async function refreshSessionWithLock( } const { anonKey } = ENVIRONMENTS[current.env].supabase; - const refreshed = await CliAuthGateway.refresh( - current.supabase_url, - anonKey, - session, - ); + let refreshed; + try { + refreshed = await CliAuthGateway.refresh( + current.supabase_url, + anonKey, + session, + ); + } catch (error) { + if (error instanceof SessionRefreshError && error.definitive) { + dropStoredSession(); + } + throw error; + } // Re-read again and merge only `session` so a concurrent `switch-org` // write (org fields) isn't reverted by our pre-refresh snapshot. const merged: StoredConfig = { ...(readConfig() ?? current), session: refreshed }; @@ -139,6 +150,20 @@ async function refreshSessionWithLock( } } +/** + * Remove only the (definitively dead) session from the stored config, keeping + * env/api_url/org so a re-login lands back in the same environment. With the + * dead session gone, subsequent commands report "Not authenticated" instead + * of failing the same refresh, and `dcd login` skips its "already logged in" + * confirm. Exported for tests. + */ +export function dropStoredSession(): void { + const config = readConfig(); + if (!config?.session) return; + delete config.session; + writeConfig(config); +} + async function acquireRefreshLock(lockPath: string): Promise { const deadline = Date.now() + LOCK_WAIT_MS; for (;;) { diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index 6140715..89ed4dd 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -3,7 +3,8 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { resolveAuth } from '../../src/utils/auth.js'; +import { SessionRefreshError } from '../../src/gateways/cli-auth-gateway.js'; +import { dropStoredSession, resolveAuth } from '../../src/utils/auth.js'; import { clearConfig, configFileMode, @@ -181,6 +182,48 @@ describe('resolveAuth precedence', () => { }); }); + it('surfaces definitive vs transient refresh failures via SessionRefreshError', () => { + const dead = new SessionRefreshError('Invalid Refresh Token: Already Used', true); + const blip = new SessionRefreshError('fetch failed', false); + expect(dead.definitive).to.equal(true); + expect(blip.definitive).to.equal(false); + expect(dead).to.be.instanceOf(Error); + }); + + it('dropStoredSession removes only the session, keeping env/org fields', async () => { + await withTempConfigDir(() => { + writeConfig({ + version: 1, + env: 'dev', + api_url: 'https://api.dev.devicecloud.dev', + supabase_url: 'https://lbmsowehtjwnqlurpemb.supabase.co', + session: { + access_token: 'a', + refresh_token: 'consumed-by-another-client', + expires_at: Math.floor(Date.now() / 1000) - 60, + user_email: 'u@example.com', + user_id: 'u1', + }, + current_org_id: '42', + current_org_name: 'Acme', + }); + + dropStoredSession(); + + const after = readConfig(); + expect(after).to.not.equal(null); + expect(after!.session).to.equal(undefined); + expect(after!.env).to.equal('dev'); + expect(after!.api_url).to.equal('https://api.dev.devicecloud.dev'); + expect(after!.current_org_id).to.equal('42'); + expect(after!.current_org_name).to.equal('Acme'); + + // Idempotent when there's no session to drop. + dropStoredSession(); + expect(readConfig()!.session).to.equal(undefined); + }); + }); + it('uses a non-expired stored session as a last resort', async () => { await withTempConfigDir(async () => { writeConfig({