diff --git a/src/mcp/add-server.test.ts b/src/mcp/add-server.test.ts index 5a0001dc2..492447139 100644 --- a/src/mcp/add-server.test.ts +++ b/src/mcp/add-server.test.ts @@ -291,7 +291,11 @@ describe("persistMCPServerEnabled", () => { mcpServers: [linearHTTP], }), ); - await saveAuthState(linearAuth, { codeVerifier: "keep-me" }, home); + await saveAuthState( + linearAuth, + { tokens: { access_token: "keep-me", token_type: "bearer" } }, + home, + ); const writer = createGlobalSettingsWriter(path); expect( @@ -306,7 +310,7 @@ describe("persistMCPServerEnabled", () => { showPromptCost: true, mcpServers: [{ ...linearHTTP, enabled: false }], }); - expect((await loadAuthState(linearAuth, home)).codeVerifier).toBe( + expect((await loadAuthState(linearAuth, home)).tokens?.access_token).toBe( "keep-me", ); @@ -388,7 +392,11 @@ describe("persistMCPServerRemoved", () => { ], }), ); - await saveAuthState(linearAuth, { codeVerifier: "linear-secret" }, home); + await saveAuthState( + linearAuth, + { tokens: { access_token: "linear-secret", token_type: "bearer" } }, + home, + ); const writer = createGlobalSettingsWriter(path); const result = await persistMCPServerRemoved(writer, "linear", home); @@ -480,7 +488,11 @@ describe("persistMCPServerRemoved", () => { path, JSON.stringify({ providers: {}, mcpServers: [custom] }), ); - await saveAuthState(identity, { codeVerifier: "custom-exa" }, home); + await saveAuthState( + identity, + { tokens: { access_token: "custom-exa", token_type: "bearer" } }, + home, + ); const writer = createGlobalSettingsWriter(path); expect(await persistMCPServerRemoved(writer, "exa", home)).toMatchObject({ @@ -501,7 +513,11 @@ describe("persistMCPServerRemoved", () => { path, JSON.stringify({ providers: {}, mcpServers: [linearHTTP] }), ); - await saveAuthState(linearAuth, { codeVerifier: "linear-secret" }, home); + await saveAuthState( + linearAuth, + { tokens: { access_token: "linear-secret", token_type: "bearer" } }, + home, + ); const authDir = mcpAuthDir(home); await chmod(authDir, 0o000); const writer = createGlobalSettingsWriter(path); @@ -538,7 +554,11 @@ describe("local MCP persist", () => { localPath, JSON.stringify({ provider: "a", mcpServers: [linearHTTP] }), ); - await saveAuthState(linearAuth, { codeVerifier: "local-linear" }, home); + await saveAuthState( + linearAuth, + { tokens: { access_token: "local-linear", token_type: "bearer" } }, + home, + ); const writer = createLocalSettingsWriter(localPath); expect( @@ -582,7 +602,11 @@ describe("local MCP persist", () => { path, JSON.stringify({ provider: "a", mcpServers: [linearHTTP] }), ); - await saveAuthState(linearAuth, { codeVerifier: "local-linear" }, home); + await saveAuthState( + linearAuth, + { tokens: { access_token: "local-linear", token_type: "bearer" } }, + home, + ); const authDir = mcpAuthDir(home); await chmod(authDir, 0o000); const writer = createLocalSettingsWriter(path); diff --git a/src/mcp/auth-store.test.ts b/src/mcp/auth-store.test.ts index 5a7ef057e..4fa165f0f 100644 --- a/src/mcp/auth-store.test.ts +++ b/src/mcp/auth-store.test.ts @@ -1,8 +1,9 @@ import { describe, expect, test } from "bun:test"; import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { + authFilePath, deleteAuthState, loadAuthState, saveAuthState, @@ -33,9 +34,10 @@ describe("mcp auth-store", () => { home, ); - // Reproduce the bug: one writer saves tokens while another overwrites with a - // fresh codeVerifier from a concurrent OAuth start. With full-snapshot - // persists, the verifier write wiped tokens; with updateAuthState, both land. + // Reproduce the bug: one writer saves tokens while another updates the + // client registration from a concurrent OAuth start. With full-snapshot + // persists, the registration write wiped tokens; with updateAuthState, + // both land. const writes = await Promise.all([ updateAuthState( linear, @@ -52,7 +54,12 @@ describe("mcp auth-store", () => { updateAuthState( linear, (state) => { - state.codeVerifier = "verifier-from-other-session"; + state.clientInformation = { + client_id: "c1", + redirect_uris: ["http://127.0.0.1:1/callback"], + client_id_issued_at: 1, + client_name: "other-session", + }; }, home, ), @@ -60,7 +67,7 @@ describe("mcp auth-store", () => { const final = await loadAuthState(linear, home); expect(final.tokens?.access_token).toBe("tok"); - expect(final.codeVerifier).toBe("verifier-from-other-session"); + expect(final.clientInformation?.client_name).toBe("other-session"); expect(final.clientInformation?.client_id).toBe("c1"); expect( writes[0].tokens?.access_token === "tok" || @@ -68,7 +75,7 @@ describe("mcp auth-store", () => { ).toBe(true); }); - test("overlapping updateAuthState from two processes keeps tokens and PKCE", async () => { + test("overlapping updateAuthState from two processes keeps tokens and client registration", async () => { const home = await tempHome(); await saveAuthState( linear, @@ -103,7 +110,12 @@ describe("mcp auth-store", () => { refresh_token: "ref", }; } else { - state.codeVerifier = "verifier-from-other-session"; + state.clientInformation = { + client_id: "c1", + redirect_uris: ["http://127.0.0.1:1/callback"], + client_id_issued_at: 1, + client_name: "other-session", + }; } }, home, @@ -118,7 +130,7 @@ describe("mcp auth-store", () => { }, ), Bun.spawn( - [process.execPath, "-e", script, "--", home, "verifier", barrier], + [process.execPath, "-e", script, "--", home, "client", barrier], { stdout: "ignore", stderr: "pipe", @@ -135,7 +147,7 @@ describe("mcp auth-store", () => { const final = await loadAuthState(linear, home); expect(final.tokens?.access_token).toBe("tok"); - expect(final.codeVerifier).toBe("verifier-from-other-session"); + expect(final.clientInformation?.client_name).toBe("other-session"); expect(final.clientInformation?.client_id).toBe("c1"); }); @@ -143,11 +155,17 @@ describe("mcp auth-store", () => { const home = await tempHome(); await Promise.all( Array.from({ length: 20 }, (_, i) => - saveAuthState(linear, { codeVerifier: `v${String(i)}` }, home), + saveAuthState( + linear, + { + tokens: { access_token: `v${String(i)}`, token_type: "bearer" }, + }, + home, + ), ), ); const final = await loadAuthState(linear, home); - expect(final.codeVerifier?.startsWith("v")).toBe(true); + expect(final.tokens?.access_token?.startsWith("v")).toBe(true); // No leftover temp files from failed renames. const dir = join(home, ".corbits", "mcp-auth"); const files = await Array.fromAsync( @@ -155,7 +173,7 @@ describe("mcp auth-store", () => { ); expect(files).toHaveLength(1); const raw = await readFile(join(dir, files[0] ?? "missing"), "utf8"); - expect(JSON.parse(raw).codeVerifier).toBe(final.codeVerifier); + expect(JSON.parse(raw).tokens).toEqual(final.tokens); }); test("scopes credentials to normalized endpoint identity", async () => { @@ -175,13 +193,21 @@ describe("mcp auth-store", () => { serverURL: "https://ONE.example:443/mcp#ignored", }; - await saveAuthState(originA, { codeVerifier: "only-a" }, home); + await saveAuthState( + originA, + { tokens: { access_token: "only-a", token_type: "bearer" } }, + home, + ); - expect((await loadAuthState(originA, home)).codeVerifier).toBe("only-a"); + expect((await loadAuthState(originA, home)).tokens?.access_token).toBe( + "only-a", + ); expect(await loadAuthState(originB, home)).toEqual({}); expect(await loadAuthState(pathB, home)).toEqual({}); expect(await loadAuthState(queryB, home)).toEqual({}); - expect((await loadAuthState(equivalent, home)).codeVerifier).toBe("only-a"); + expect((await loadAuthState(equivalent, home)).tokens?.access_token).toBe( + "only-a", + ); }); test("ignores legacy name-only auth state without modifying it", async () => { @@ -204,6 +230,34 @@ describe("mcp auth-store", () => { }); }); + test("updateAuthState strips a legacy PKCE verifier instead of persisting it", async () => { + const home = await tempHome(); + const path = authFilePath(linear, home); + await mkdir(dirname(path), { recursive: true }); + await writeFile( + path, + JSON.stringify({ + tokens: { access_token: "tok", token_type: "bearer" }, + codeVerifier: "legacy-secret", + }), + ); + + const written = await updateAuthState( + linear, + (state) => { + state.tokens = { access_token: "tok2", token_type: "bearer" }; + }, + home, + ); + + expect(written.tokens?.access_token).toBe("tok2"); + expect("codeVerifier" in written).toBe(false); + expect(await readFile(path, "utf8")).not.toContain("codeVerifier"); + expect((await loadAuthState(linear, home)).tokens?.access_token).toBe( + "tok2", + ); + }); + test("bounds the display slug without weakening scoped identity", async () => { const home = await tempHome(); const dir = join(home, ".corbits", "mcp-auth"); @@ -211,7 +265,7 @@ describe("mcp auth-store", () => { const longName = `${prefixName}/long`; await saveAuthState( { serverName: longName, serverURL: "https://long.example/mcp" }, - { codeVerifier: "scoped-secret" }, + { tokens: { access_token: "scoped-secret", token_type: "bearer" } }, home, ); @@ -225,8 +279,14 @@ describe("mcp auth-store", () => { test("deleteAuthState removes an existing file", async () => { const home = await tempHome(); - await saveAuthState(linear, { codeVerifier: "secret" }, home); - expect((await loadAuthState(linear, home)).codeVerifier).toBe("secret"); + await saveAuthState( + linear, + { tokens: { access_token: "secret", token_type: "bearer" } }, + home, + ); + expect((await loadAuthState(linear, home)).tokens?.access_token).toBe( + "secret", + ); await deleteAuthState(linear, home); expect(await loadAuthState(linear, home)).toEqual({}); @@ -241,13 +301,17 @@ describe("mcp auth-store", () => { test("concurrent updateAuthState and deleteAuthState do not throw ENOENT on rename", async () => { const home = await tempHome(); - await saveAuthState(linear, { codeVerifier: "v0" }, home); + await saveAuthState( + linear, + { tokens: { access_token: "v0", token_type: "bearer" } }, + home, + ); await Promise.all([ updateAuthState( linear, (state) => { - state.codeVerifier = "v1"; + state.tokens = { access_token: "v1", token_type: "bearer" }; }, home, ), @@ -256,7 +320,7 @@ describe("mcp auth-store", () => { const final = await loadAuthState(linear, home); expect( - final.codeVerifier === undefined || final.codeVerifier === "v1", + final.tokens === undefined || final.tokens.access_token === "v1", ).toBe(true); }); }); diff --git a/src/mcp/auth-store.ts b/src/mcp/auth-store.ts index aa55de7a7..86f937ca1 100644 --- a/src/mcp/auth-store.ts +++ b/src/mcp/auth-store.ts @@ -18,14 +18,14 @@ import type { } from "@modelcontextprotocol/sdk/shared/auth.js"; import { SETTINGS_DIR_NAME } from "../branding.js"; -// Per-server OAuth state persisted between sessions. Holding the PKCE verifier is -// necessary because authorization spans a process boundary (browser round-trip); -// tokens and dynamically-registered client info let later sessions reconnect -// without any user interaction. +// Per-server OAuth state persisted between sessions. Only the durable pair — +// dynamically-registered client info and tokens — reaches disk; the PKCE +// verifier is instance-local transient memory in the OAuth provider, since the +// browser flow starts and finishes inside one provider episode. Legacy files +// may still carry a codeVerifier key, which is stripped on write. export interface MCPAuthState { clientInformation?: OAuthClientInformationFull; tokens?: OAuthTokens; - codeVerifier?: string; } export interface MCPAuthIdentity { @@ -67,7 +67,7 @@ export function authFilePath( ); } -function isEnoent(err: unknown): boolean { +export function isEnoent(err: unknown): boolean { return ( typeof err === "object" && err !== null && @@ -150,7 +150,7 @@ let tmpWriteCounter = 0; // Serialize read-modify-write per auth file so two OAuth provider instances for // the same server cannot clobber each other's fields (classic lost-update: one -// session's saveCodeVerifier wiping another's just-written tokens). +// session's saveTokens wiping another's just-saved client registration). const updateChains = new Map>(); const LOCK_STALE_MS = 5_000; @@ -220,10 +220,22 @@ function enqueueAuthFileOp(path: string, op: () => Promise): Promise { return run; } +// Legacy auth files may carry a codeVerifier key from when the PKCE verifier +// flowed through disk. The verifier is instance-local transient memory now — +// strip the key so it never reaches disk again. +function withoutLegacyVerifier(state: MCPAuthState): MCPAuthState { + if (!("codeVerifier" in state)) return state; + const copy: Record = { ...state }; + delete copy.codeVerifier; + return copy as MCPAuthState; +} + async function writeAuthFile(path: string, state: MCPAuthState): Promise { await mkdir(dirname(path), { recursive: true, mode: 0o700 }); const tmp = `${path}.${process.pid}.${(tmpWriteCounter += 1)}.tmp`; - await writeFile(tmp, JSON.stringify(state, null, 2), { mode: 0o600 }); + await writeFile(tmp, JSON.stringify(withoutLegacyVerifier(state), null, 2), { + mode: 0o600, + }); await rename(tmp, path); } @@ -250,8 +262,9 @@ export async function updateAuthState( return enqueueAuthFileOp(path, async () => { const state = await loadAuthState(identity, home); mutator(state); - await writeAuthFile(path, state); - return state; + const durable = withoutLegacyVerifier(state); + await writeAuthFile(path, durable); + return durable; }); } diff --git a/src/mcp/oauth-provider.test.ts b/src/mcp/oauth-provider.test.ts index 47237ec33..463f33112 100644 --- a/src/mcp/oauth-provider.test.ts +++ b/src/mcp/oauth-provider.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test"; import { readFileSync } from "node:fs"; import { chmod, mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js"; import { authFilePath, @@ -111,13 +111,16 @@ async function expectStoredClient( describe("createOAuthProvider", () => { test("drops stale DCR client when redirect port changed and no tokens exist", async () => { const home = await tempHome(); - await saveAuthState( - linear, - { + // Legacy file: stale registration plus a PKCE verifier from when the + // verifier flowed through disk. Both must be scrubbed, not persisted. + const path = authFilePath(linear, home); + await mkdir(dirname(path), { recursive: true }); + await writeFile( + path, + JSON.stringify({ clientInformation: clientInfo(60435), codeVerifier: "old-verifier", - }, - home, + }), ); const provider = await createOAuthProvider({ @@ -129,9 +132,9 @@ describe("createOAuthProvider", () => { }); expect(await syncValue(provider.clientInformation())).toBeUndefined(); - const disk = await loadAuthState(linear, home); - expect(disk.clientInformation).toBeUndefined(); - expect(disk.codeVerifier).toBeUndefined(); + const raw = await readFile(path, "utf8"); + expect(raw).not.toContain("client-on-60435"); + expect(raw).not.toContain("codeVerifier"); }); test("keeps registered client and tokens when only the loopback port changed", async () => { @@ -164,7 +167,7 @@ describe("createOAuthProvider", () => { expect((await syncValue(provider.tokens()))?.access_token).toBe("live"); }); - test("concurrent saveTokens and saveCodeVerifier from two providers keep both fields", async () => { + test("concurrent saveTokens and saveCodeVerifier keep tokens on disk and the verifier in memory", async () => { const home = await tempHome(); await saveAuthState(linear, { clientInformation: clientInfo(1) }, home); @@ -193,9 +196,141 @@ describe("createOAuthProvider", () => { b.saveCodeVerifier("verifier-b"), ]); + // The verifier is instance-local: b reads its own, a has none, and the + // concurrent memory-only save cannot clobber the disk tokens. + expect(b.codeVerifier()).toBe("verifier-b"); + expect(() => a.codeVerifier()).toThrow("No PKCE code verifier saved"); const disk = await loadAuthState(linear, home); expect(disk.tokens?.access_token).toBe("tok-a"); - expect(disk.codeVerifier).toBe("verifier-b"); + expect("codeVerifier" in disk).toBe(false); + }); + + test("never writes the PKCE verifier to disk across a full browser-flow save sequence", async () => { + const home = await tempHome(); + const provider = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + // The SDK browser flow persists DCR client info, then the PKCE verifier, + // then the exchanged tokens — in that order. + await saveClient(provider, clientInfo(62000)); + await provider.saveCodeVerifier("pkce-one-time"); + await provider.saveTokens({ + access_token: "tok", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref", + }); + + expect(provider.codeVerifier()).toBe("pkce-one-time"); + const raw = await readFile(authFilePath(linear, home), "utf8"); + expect(raw).not.toContain("codeVerifier"); + expect(raw).not.toContain("pkce-one-time"); + expect(Object.keys(JSON.parse(raw)).sort()).toEqual([ + "clientInformation", + "tokens", + ]); + }); + + test("drops in-memory tokens once the auth file is deleted", async () => { + const home = await tempHome(); + const provider = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(provider, clientInfo(1)); + await provider.saveTokens({ access_token: "tok", token_type: "bearer" }); + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + + await deleteAuthState(linear, home); + + expect(await syncValue(provider.tokens())).toBeUndefined(); + }); + + test("keeps live tokens when the auth file is unreadable but drops them on delete", async () => { + const home = await tempHome(); + const provider = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(provider, clientInfo(1)); + await provider.saveTokens({ access_token: "tok", token_type: "bearer" }); + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + + // A transient stat failure (EACCES here, file still present) must not + // discard live credentials — only a real deletion (ENOENT) does. + const dir = dirname(authFilePath(linear, home)); + await chmod(dir, 0o000); + try { + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + } finally { + await chmod(dir, 0o700); + } + + await deleteAuthState(linear, home); + + expect(await syncValue(provider.tokens())).toBeUndefined(); + }); + + test("two overlapping logins on different ports both complete without clobbering verifiers", async () => { + const home = await tempHome(); + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + + await saveClient(a, clientInfo(62000)); + await a.saveCodeVerifier("pkce-a"); + await saveClient(b, clientInfo(60435)); + await b.saveCodeVerifier("pkce-b"); + + await a.saveTokens({ + access_token: "tok-a", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref-a", + }); + await b.saveTokens({ + access_token: "tok-b", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref-b", + }); + + // Each episode keeps its own verifier; neither clobbers the other. + expect(a.codeVerifier()).toBe("pkce-a"); + expect(b.codeVerifier()).toBe("pkce-b"); + // The verifier is transient: the shared file holds the completed auth only. + const raw = await readFile(authFilePath(linear, home), "utf8"); + expect(raw).not.toContain("codeVerifier"); + expect(raw).not.toContain("pkce-a"); + expect(raw).not.toContain("pkce-b"); + // Last completer wins the durable tokens; the sibling picks them up. + expect((await loadAuthState(linear, home)).tokens?.access_token).toBe( + "tok-b", + ); + expect((await syncValue(a.tokens()))?.access_token).toBe("tok-b"); + expect((await syncValue(b.tokens()))?.access_token).toBe("tok-b"); + await expectClientOnPort(b, 60435); }); test("resetAuthorization clears client when redirect no longer matches registration", async () => { @@ -210,7 +345,6 @@ describe("createOAuthProvider", () => { expires_in: 1, refresh_token: "r", }, - codeVerifier: "v", }, home, ); @@ -511,7 +645,7 @@ describe("createOAuthProvider", () => { await expectStoredClient(home, "client-on-62000-v2", 62000); }); - test("sync getters fall back to the in-memory mirror when the auth file disappears", async () => { + test("drops in-memory tokens when the auth file disappears and adopts a later sibling save", async () => { const home = await tempHome(); const provider = await createOAuthProvider({ serverName: "linear", @@ -523,10 +657,18 @@ describe("createOAuthProvider", () => { await provider.saveTokens({ access_token: "tok", token_type: "bearer" }); expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + // Server-side removal must not leave orphaned credentials behind. await deleteAuthState(linear, home); - expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); - expect(await syncValue(provider.clientInformation())).toBeUndefined(); + expect(await syncValue(provider.tokens())).toBeUndefined(); + + // The provider stays live: a sibling session's fresh auth is adopted. + await saveAuthState( + linear, + { tokens: { access_token: "reauthed", token_type: "bearer" } }, + home, + ); + expect((await syncValue(provider.tokens()))?.access_token).toBe("reauthed"); }); test("keeps live tokens when the auth file is corrupt", async () => { diff --git a/src/mcp/oauth-provider.ts b/src/mcp/oauth-provider.ts index 1878843b8..4b26f0ef4 100644 --- a/src/mcp/oauth-provider.ts +++ b/src/mcp/oauth-provider.ts @@ -20,6 +20,7 @@ import type { import type { FetchLike } from "@modelcontextprotocol/sdk/shared/transport.js"; import { authFilePath, + isEnoent, tryLoadAuthStateSync, updateAuthState, type MCPAuthIdentity, @@ -70,7 +71,6 @@ function dropStaleClientRegistration( if (state.tokens !== undefined) return; if (redirectUrisInclude(state.clientInformation, redirectUrl)) return; delete state.clientInformation; - delete state.codeVerifier; } function shouldAdoptClient( @@ -162,26 +162,43 @@ export async function createOAuthProvider( // failed/unreadable file is retried on the next getter call. const authPath = authFilePath(identity, home); let seenStamp: string | undefined; + // True once this provider has committed a disk observation. A transient stat + // failure resets the stamp so the next getter retries, but must not erase + // the fact that durable state was seen — otherwise a later real deletion + // (ENOENT) would look like "never saw the file" and orphan live tokens. + let observedDurable = false; const refreshDurableFromDisk = (): void => { let stamp: string | undefined; try { const stat = statSync(authPath); stamp = `${String(stat.mtimeMs)}:${String(stat.size)}`; - } catch { - if (seenStamp === undefined) return; + } catch (err) { + if (!observedDurable) return; seenStamp = undefined; + if (!isEnoent(err)) return; + // The auth file is gone (server removed, logged out elsewhere). Hold no + // orphaned credentials: drop the in-memory tokens too. A later sibling + // save is adopted on the next read via the stamp check above. + observedDurable = false; + delete stored.tokens; return; } if (stamp === seenStamp) return; const next = tryLoadAuthStateSync(identity, home); if (next === undefined) return; seenStamp = stamp; + observedDurable = true; const adoptClient = shouldAdoptClient(stored, next, opts.redirectUrl); assignTokens(stored, next); if (adoptClient) assignClient(stored, next); }; let oauthState: string | undefined; + // PKCE verifier: instance-local transient memory only, never persisted. The + // browser flow starts and finishes inside this provider episode, so the + // secret binding them must not be shared through the auth file — a + // different-port sibling must neither see nor clobber it. + let pkceVerifier: string | undefined; let authorizationServerMetadata: AuthorizationServerMetadata | undefined; let authorizationServerUrl: string | undefined; let resource: URL | undefined; @@ -228,18 +245,17 @@ export async function createOAuthProvider( opts.onAuthURL(opts.serverName, authorizationUrl.toString()); }, saveCodeVerifier(codeVerifier: string): Promise { - stored.codeVerifier = codeVerifier; - return apply((state) => { - state.codeVerifier = codeVerifier; - }); + pkceVerifier = codeVerifier; + return Promise.resolve(); }, codeVerifier(): string { - if (stored.codeVerifier === undefined) + if (pkceVerifier === undefined) throw new Error("No PKCE code verifier saved for this authorization."); - return stored.codeVerifier; + return pkceVerifier; }, async resetAuthorization(): Promise { oauthState = undefined; + pkceVerifier = undefined; // Snapshot before the disk refresh so a session that never held tokens // cannot adopt a sibling's credentials and then delete them. const previous = stored.tokens?.access_token; @@ -248,13 +264,11 @@ export async function createOAuthProvider( if (state.tokens?.access_token === previous) { delete state.tokens; } - delete state.codeVerifier; // Next browser flow needs a client registered for *this* loopback port. if (!redirectUrisInclude(state.clientInformation, opts.redirectUrl)) { delete state.clientInformation; } }); - delete stored.codeVerifier; }, refreshToken: async (refreshToken: string): Promise => { try {