diff --git a/CHANGELOG.md b/CHANGELOG.md index 43f9c9a81..88df41c89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,10 @@ file is the human-readable summary of what changed and why. from the sidebar, which no longer shows a Help row — and a new **Log out**. Logging out mirrors `codevhub logout`: it ends the CoDev SSO session (and revokes its tokens) but keeps the gateway API key, so chat keeps working; - the next sign-in asks for credentials again. + the next sign-in asks for credentials again. A session whose sign-in has + expired beyond recovery — the SSO wrapper rejects its refresh token — also + shows as logged out (checked with a short, bounded probe; an unreachable + server never logs you out visually). ### Sessions diff --git a/packages/codev-gateway/src/index.ts b/packages/codev-gateway/src/index.ts index 8d9464638..c8320c0f7 100644 --- a/packages/codev-gateway/src/index.ts +++ b/packages/codev-gateway/src/index.ts @@ -33,4 +33,13 @@ export { slugifyProviderName, type ProviderIdentity, } from "./provider" -export { login, logout, parseCallbackParams, silentSso, type LoginOptions, type OnLoginReady } from "./sso" +export { + login, + logout, + parseCallbackParams, + probeSession, + silentSso, + type LoginOptions, + type OnLoginReady, + type SessionProbe, +} from "./sso" diff --git a/packages/codev-gateway/src/sso.ts b/packages/codev-gateway/src/sso.ts index a7ff6e0b1..89aebed61 100644 --- a/packages/codev-gateway/src/sso.ts +++ b/packages/codev-gateway/src/sso.ts @@ -151,6 +151,54 @@ export async function silentSso(): Promise { } } +export type SessionProbe = "live" | "dead" | "unknown" + +// Whether the SSO session on disk is actually usable, distinguishing a dead +// session from an unreachable IdP: only the wrapper definitively rejecting the +// refresh token reads as "dead" (mirrors ensureKey's rule that a network +// failure must never cost someone their session — callers treat "unknown" as +// optimistically live). A successful probe persists the refreshed tokens, so +// the next check short-circuits on the unexpired access token. +export async function probeSession(opts?: { fetchImpl?: typeof fetch }): Promise { + if (loadAuth()) return "live" + const stale = readAuthFile() + if (!stale?.refresh_token) return "dead" + const fetchImpl = opts?.fetchImpl ?? fetch + const res = await fetchImpl(`${SSO_URL}/token`, { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body: new URLSearchParams({ + grant_type: "refresh_token", + refresh_token: stale.refresh_token, + client_id: CLIENT_ID, + }), + signal: AbortSignal.timeout(SSO_FETCH_TIMEOUT_MS), + }).catch(() => null) + if (!res) return "unknown" + if (!res.ok) { + // 400 is what OAuth's invalid_grant rides on; 401/403 are wrapper + // rejections. Anything else (5xx, gateway errors) proves nothing. + return res.status === 400 || res.status === 401 || res.status === 403 ? "dead" : "unknown" + } + const tokens = (await res.json().catch(() => null)) as TokenResponse | null + if (!tokens) return "unknown" + // Persist before returning: a successful refresh may rotate the + // refresh_token, and dropping the rotation would strand the very session + // the probe just proved live. The stale user block stands in when the + // userinfo round-trip fails — the identity does not change on a refresh. + const user = await fetchUserInfo(tokens.access_token, fetchImpl).catch(() => null) + saveAuth({ + access_token: tokens.access_token, + id_token: tokens.id_token, + refresh_token: tokens.refresh_token || stale.refresh_token, + expires_at: Date.now() + tokens.expires_in * 1000, + user: user + ? { sub: user.sub, email: user.email, displayName: user.displayName || user.name || user.sub } + : (stale.user ?? { sub: "", email: "", displayName: "" }), + }) + return "live" +} + // Port of hub logout (src/lib/auth.ts): drop the SSO block from the auth file // (the api-key/config blocks and hub cookie survive — the app keeps running on // the gateway key), leave the force-login sentinel so the next login re-prompts @@ -448,8 +496,8 @@ async function refreshTokens(refreshToken: string): Promise { return (await res.json()) as TokenResponse } -async function fetchUserInfo(accessToken: string) { - const res = await fetch(`${SSO_URL}/userinfo`, { +async function fetchUserInfo(accessToken: string, fetchImpl: typeof fetch = fetch) { + const res = await fetchImpl(`${SSO_URL}/userinfo`, { headers: { Authorization: `Bearer ${accessToken}` }, signal: AbortSignal.timeout(SSO_FETCH_TIMEOUT_MS), }) diff --git a/packages/codev-gateway/test/sso.test.ts b/packages/codev-gateway/test/sso.test.ts index 9dbb351ed..ed7fe063c 100644 --- a/packages/codev-gateway/test/sso.test.ts +++ b/packages/codev-gateway/test/sso.test.ts @@ -1,9 +1,9 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test" -import { existsSync, mkdtempSync, rmSync } from "node:fs" +import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" -import { logout, parseCallbackParams } from "../src/sso" -import { forceLoginPath, loadApiKey, saveApiKey, saveAuth } from "../src/auth-file" +import { logout, parseCallbackParams, probeSession } from "../src/sso" +import { authFilePath, forceLoginPath, loadApiKey, saveApiKey, saveAuth } from "../src/auth-file" describe("parseCallbackParams", () => { test("parses a full callback URL", () => { @@ -93,3 +93,74 @@ describe("logout", () => { expect(existsSync(forceLoginPath())).toBe(false) }) }) + +describe("probeSession", () => { + let tempDir: string + let originalTestHome: string | undefined + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "codev-gateway-probe-")) + originalTestHome = process.env.CODEV_TEST_HOME + process.env.CODEV_TEST_HOME = tempDir + }) + + afterEach(() => { + if (originalTestHome === undefined) delete process.env.CODEV_TEST_HOME + if (originalTestHome !== undefined) process.env.CODEV_TEST_HOME = originalTestHome + rmSync(tempDir, { recursive: true, force: true }) + }) + + const user = { sub: "s", email: "e@x", displayName: "E" } + const respond = (fn: () => Promise): typeof fetch => + (async (_input: string | URL | Request) => fn()) as typeof fetch + const noFetch: typeof fetch = (async (_input: string | URL | Request): Promise => { + throw new Error("must not be called") + }) as typeof fetch + + test("an unexpired access token is live without a network call", async () => { + saveAuth({ access_token: "at", id_token: "it", expires_at: Date.now() + 60_000, user }) + expect(await probeSession({ fetchImpl: noFetch })).toBe("live") + }) + + test("nothing to refresh with is dead without a network call", async () => { + expect(await probeSession({ fetchImpl: noFetch })).toBe("dead") + saveAuth({ access_token: "at", id_token: "it", expires_at: Date.now() - 1000, user }) + expect(await probeSession({ fetchImpl: noFetch })).toBe("dead") + }) + + test("a definite rejection of the refresh token is dead", async () => { + saveAuth({ access_token: "at", id_token: "it", refresh_token: "rt", expires_at: Date.now() - 1000, user }) + const fetchImpl = respond(async () => new Response("invalid_grant", { status: 400 })) + expect(await probeSession({ fetchImpl })).toBe("dead") + }) + + test("network failures and server errors prove nothing", async () => { + saveAuth({ access_token: "at", id_token: "it", refresh_token: "rt", expires_at: Date.now() - 1000, user }) + const down = respond(async () => { + throw new Error("network down") + }) + expect(await probeSession({ fetchImpl: down })).toBe("unknown") + const flaky = respond(async () => new Response("oops", { status: 502 })) + expect(await probeSession({ fetchImpl: flaky })).toBe("unknown") + }) + + test("a successful refresh is live and persists the rotated tokens", async () => { + saveAuth({ access_token: "old-at", id_token: "it", refresh_token: "old-rt", expires_at: Date.now() - 1000, user }) + // First call is the token refresh; the userinfo round-trip fails, so the + // stale user block must survive the rewrite. + let calls = 0 + const fetchImpl = respond(async () => { + calls++ + if (calls === 1) + return Response.json({ access_token: "new-at", id_token: "new-it", refresh_token: "new-rt", expires_in: 3600 }) + throw new Error("userinfo down") + }) + expect(await probeSession({ fetchImpl })).toBe("live") + const raw = JSON.parse(readFileSync(authFilePath(), "utf-8")) + expect(raw.access_token).toBe("new-at") + expect(raw.refresh_token).toBe("new-rt") + expect(raw.user).toEqual(user) + // The persisted session now short-circuits: no further network calls. + expect(await probeSession({ fetchImpl: noFetch })).toBe("live") + }) +}) diff --git a/packages/desktop/src/main/gateway-sso/controller.test.ts b/packages/desktop/src/main/gateway-sso/controller.test.ts index 45b05b7a7..b4b0dc947 100644 --- a/packages/desktop/src/main/gateway-sso/controller.test.ts +++ b/packages/desktop/src/main/gateway-sso/controller.test.ts @@ -203,10 +203,62 @@ describe("gateway sso controller", () => { expect(savedKeys[0]?.baseUrl).toBe("https://gw.example.test") }) - test("getAccount reads the injected loader", () => { + // The avatar's data source: disk identity hardened by the session probe. + // Only a definite "dead" hides it — an unreachable IdP proves nothing. + describe("getAccount", () => { const account = { email: "user@example.test", displayName: "User" } - const controller = controllerWith({ loadAccount: () => account }) - expect(controller.getAccount()).toEqual(account) + + test("a live access token skips the probe", async () => { + let probes = 0 + const controller = controllerWith({ + loadAccount: () => account, + loadAuth: () => auth, + probeSession: async () => { + probes++ + return "dead" + }, + }) + expect(await controller.getAccount()).toEqual(account) + expect(probes).toBe(0) + }) + + test("a dead probe hides the account", async () => { + const controller = controllerWith({ + loadAccount: () => account, + loadAuth: () => null, + probeSession: async () => "dead", + }) + expect(await controller.getAccount()).toBeNull() + }) + + test("an unknown probe stays optimistic and the verdict is cached", async () => { + let probes = 0 + const controller = controllerWith({ + loadAccount: () => account, + loadAuth: () => null, + probeSession: async () => { + probes++ + return "unknown" + }, + }) + expect(await controller.getAccount()).toEqual(account) + expect(await controller.getAccount()).toEqual(account) + expect(probes).toBe(1) + }) + + test("no identity on disk is null without probing", async () => { + let probes = 0 + const controller = controllerWith({ + loadAccount: () => null, + loadAuth: () => null, + probeSession: async () => { + probes++ + return "live" + }, + }) + expect(await controller.getAccount()).toBeNull() + expect(probes).toBe(0) + }) }) test("logout ends the session, clears the cookie, and resets state", async () => { diff --git a/packages/desktop/src/main/gateway-sso/controller.ts b/packages/desktop/src/main/gateway-sso/controller.ts index 1940e07f6..5e0ee559a 100644 --- a/packages/desktop/src/main/gateway-sso/controller.ts +++ b/packages/desktop/src/main/gateway-sso/controller.ts @@ -10,9 +10,11 @@ import { LEGACY_GATEWAY_PROVIDER_IDS, loadAccount, loadApiKey, + loadAuth, loadGatewayUrl, login, logout, + probeSession, saveApiKey, saveCodevConfig, silentSso, @@ -33,6 +35,11 @@ const RETRIES = 3 // /key/info round-trip is far under this; past it we cannot prove the key is // dead, so we leave it alone (mirrors codev-hub's launch pre-flight). const KEY_CHECK_TIMEOUT_MS = 2_500 +// Same shape for the account probe: past the cap we cannot prove the session +// is dead, so the avatar stays. The verdict is remembered briefly because +// getAccount refetches on every state event. +const ACCOUNT_PROBE_TIMEOUT_MS = 2_500 +const ACCOUNT_PROBE_TTL_MS = 60_000 type Deps = { openUrl: (url: string) => Promise @@ -60,7 +67,9 @@ type GatewayFns = { saveApiKey: typeof saveApiKey loadAccount: typeof loadAccount loadApiKey: typeof loadApiKey + loadAuth: typeof loadAuth loadGatewayUrl: typeof loadGatewayUrl + probeSession: typeof probeSession validateApiKey: typeof validateApiKey clearSkillhubCookie: typeof clearSkillhubCookie } @@ -81,7 +90,9 @@ export function createGatewaySsoController(deps: Deps) { saveApiKey, loadAccount, loadApiKey, + loadAuth, loadGatewayUrl, + probeSession, validateApiKey, clearSkillhubCookie, ...deps.gateway, @@ -110,6 +121,8 @@ export function createGatewaySsoController(deps: Deps) { let submitStash: ((pasted: string) => string | null) | undefined let session: AuthData | undefined + let probeAt = 0 + let probeVerdict: "live" | "dead" | "unknown" = "unknown" let apiKey: string | undefined let gatewayUrl: string | undefined let manualKey = false @@ -188,8 +201,25 @@ export function createGatewaySsoController(deps: Deps) { submitStash = undefined setState({ phase: "idle", status: "", error: undefined }) }, - getAccount(): GatewayAccount | null { - return gateway.loadAccount() + // The disk answer, hardened against a refresh token the wrapper has since + // rejected: an expired access token forces a bounded probe of the token + // endpoint, and only a definite rejection hides the avatar — an + // unreachable IdP must never log someone out visually. A live probe + // persists the refreshed tokens, so the next call short-circuits on the + // unexpired access token instead of re-probing. + async getAccount(): Promise { + const account = gateway.loadAccount() + if (!account) return null + if (gateway.loadAuth()) return account + if (Date.now() - probeAt > ACCOUNT_PROBE_TTL_MS) { + probeVerdict = await Promise.race([ + gateway.probeSession(fetchOpts).catch(() => "unknown" as const), + new Promise<"unknown">((resolve) => setTimeout(resolve, ACCOUNT_PROBE_TIMEOUT_MS, "unknown")), + ]) + probeAt = Date.now() + } + if (probeVerdict === "dead") return null + return account }, // Hub-parity sign-out (the desktop face of `codevhub logout`): the gateway // fn drops the SSO block from the shared auth file (api key and config @@ -204,6 +234,7 @@ export function createGatewaySsoController(deps: Deps) { loginRun = undefined submitStash = undefined session = undefined + probeAt = 0 await gateway.logout(fetchOpts) gateway.clearSkillhubCookie() // Wholesale reset rather than setState: its merge keeps a previous email.