diff --git a/src/mcp/auth-store.ts b/src/mcp/auth-store.ts index 2b53e1433..37207ff92 100644 --- a/src/mcp/auth-store.ts +++ b/src/mcp/auth-store.ts @@ -1,5 +1,6 @@ import { createHash } from "node:crypto"; import { mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises"; +import { readFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join } from "node:path"; import type { @@ -41,7 +42,7 @@ export function normalizeMCPServerURL(serverURL: string): string { return url.toString(); } -function authFilePath(identity: MCPAuthIdentity, home: string): string { +export function authFilePath(identity: MCPAuthIdentity, home: string = homedir()): string { const normalizedURL = normalizeMCPServerURL(identity.serverURL); const digest = createHash("sha256") .update(JSON.stringify([identity.serverName, normalizedURL])) @@ -49,32 +50,80 @@ function authFilePath(identity: MCPAuthIdentity, home: string): string { return join(mcpAuthDir(home), `${serverDisplaySlug(identity.serverName)}-${digest}.json`); } -export async function loadAuthState( - identity: MCPAuthIdentity, - home: string = homedir(), -): Promise { - let raw: string; +function isEnoent(err: unknown): boolean { + return ( + typeof err === "object" && + err !== null && + "code" in err && + (err as { code?: unknown }).code === "ENOENT" + ); +} + +function parseAuthState(raw: string): MCPAuthState | undefined { + try { + const parsed: unknown = JSON.parse(raw); + if (typeof parsed === "object" && parsed !== null) return parsed as MCPAuthState; + } catch { + // A corrupt auth file should not wedge the session; treat it as no state and + // let a fresh authorization overwrite it. + } + return undefined; +} + +function stateFromRaw(raw: string | undefined): MCPAuthState { + if (raw === undefined) return {}; + return parseAuthState(raw) ?? {}; +} + +function readAuthFileSync(path: string): string | undefined { try { - raw = await readFile(authFilePath(identity, home), "utf8"); + return readFileSync(path, "utf8"); } catch (err) { - if ( - typeof err === "object" && - err !== null && - "code" in err && - (err as { code?: unknown }).code === "ENOENT" - ) { - return {}; - } + if (isEnoent(err)) return undefined; throw err; } +} + +async function readAuthFile(path: string): Promise { try { - const parsed = JSON.parse(raw); - if (typeof parsed === "object" && parsed !== null) return parsed as MCPAuthState; + return await readFile(path, "utf8"); + } catch (err) { + if (isEnoent(err)) return undefined; + throw err; + } +} + +// Synchronous connect-contract load. Tolerates a missing (ENOENT) or corrupt +// file with empty state, matching loadAuthState; other read errors propagate. +export function loadAuthStateSync( + identity: MCPAuthIdentity, + home: string = homedir(), +): MCPAuthState { + return stateFromRaw(readAuthFileSync(authFilePath(identity, home))); +} + +export async function loadAuthState( + identity: MCPAuthIdentity, + home: string = homedir(), +): Promise { + return stateFromRaw(await readAuthFile(authFilePath(identity, home))); +} + +// Cache refresh for a live provider: missing, unreadable, or corrupt files +// return undefined so the caller keeps its in-memory mirror. Empty-on-corrupt +// is loadAuthState's connect contract, not cache invalidation. +export function tryLoadAuthStateSync( + identity: MCPAuthIdentity, + home: string = homedir(), +): MCPAuthState | undefined { + let raw: string | undefined; + try { + raw = readAuthFileSync(authFilePath(identity, home)); } catch { - // A corrupt auth file should not wedge the session; treat it as no state and - // let a fresh authorization overwrite it. + return undefined; } - return {}; + if (raw === undefined) return undefined; + return parseAuthState(raw); } // pid alone is not unique per call — concurrent saves in one process must not @@ -174,14 +223,7 @@ async function unlinkAuthFile(path: string): Promise { try { await unlink(path); } catch (err) { - if ( - typeof err === "object" && - err !== null && - "code" in err && - (err as { code?: unknown }).code === "ENOENT" - ) { - return; - } + if (isEnoent(err)) return; throw err; } } diff --git a/src/mcp/oauth-provider.test.ts b/src/mcp/oauth-provider.test.ts index 9e8d8bc1d..0696f8c06 100644 --- a/src/mcp/oauth-provider.test.ts +++ b/src/mcp/oauth-provider.test.ts @@ -1,8 +1,10 @@ -import { describe, expect, test } from "bun:test"; -import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { describe, expect, spyOn, test } from "bun:test"; +import * as fs from "node:fs"; +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 { loadAuthState, saveAuthState } from "./auth-store.js"; +import { authFilePath, loadAuthState, saveAuthState, deleteAuthState } from "./auth-store.js"; import { createOAuthProvider } from "./oauth-provider.js"; async function tempHome(): Promise { @@ -25,6 +27,15 @@ async function syncValue(value: T | Promise): Promise { return await value; } +async function saveClient( + provider: Awaited>, + info: ReturnType, +): Promise { + const save = provider.saveClientInformation; + if (save === undefined) throw new Error("saveClientInformation is required"); + await save(info); +} + describe("createOAuthProvider", () => { test("drops stale DCR client when redirect port changed and no tokens exist", async () => { const home = await tempHome(); @@ -208,6 +219,357 @@ describe("createOAuthProvider", () => { expect(await readFile(join(dir, ".json"), "utf8")).toBe(legacy); }); + test("propagates tokens saved by one provider to an existing sibling provider", async () => { + const home = await tempHome(); + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + expect(await syncValue(b.tokens())).toBeUndefined(); + + await a.saveTokens({ + access_token: "fresh", + token_type: "bearer", + expires_in: 3600, + refresh_token: "fresh-refresh", + }); + + expect((await syncValue(b.tokens()))?.access_token).toBe("fresh"); + }); + + test("does not replace an in-progress PKCE verifier from a different-port sibling", 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, + }); + await saveClient(a, clientInfo(62000)); + await a.saveCodeVerifier("pkce-a"); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + expect(a.codeVerifier()).toBe("pkce-a"); + + await b.saveCodeVerifier("pkce-b"); + expect(a.codeVerifier()).toBe("pkce-a"); + expect(b.codeVerifier()).toBe("pkce-b"); + }); + + test("does not adopt a different-port sibling DCR client without tokens", 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, + }); + await saveClient(a, clientInfo(62000)); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + expect((await syncValue(a.clientInformation()))?.client_id).toBe("client-on-62000"); + + await saveClient(b, clientInfo(60435)); + const info = await syncValue(a.clientInformation()); + expect(info?.client_id).toBe("client-on-62000"); + expect(info && "redirect_uris" in info ? info.redirect_uris : undefined).toEqual([ + "http://127.0.0.1:62000/callback", + ]); + }); + + test("saveTokens after a different-port sibling construct keeps this session's DCR", 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, + }); + await saveClient(a, clientInfo(62000)); + await a.saveCodeVerifier("pkce-a"); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + expect(await syncValue(b.clientInformation())).toBeUndefined(); + + await a.saveTokens({ + access_token: "tok-a", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref-a", + }); + + const info = await syncValue(a.clientInformation()); + expect(info?.client_id).toBe("client-on-62000"); + expect(info && "redirect_uris" in info ? info.redirect_uris : undefined).toEqual([ + "http://127.0.0.1:62000/callback", + ]); + const disk = await loadAuthState(linear, home); + expect(disk.clientInformation?.client_id).toBe("client-on-62000"); + expect(disk.clientInformation?.redirect_uris).toEqual(["http://127.0.0.1:62000/callback"]); + }); + + test("saveTokens after a different-port sibling saveClient keeps this session's DCR", 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, + }); + await saveClient(a, clientInfo(62000)); + await a.saveCodeVerifier("pkce-a"); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(b, clientInfo(60435)); + + await a.saveTokens({ + access_token: "tok-a", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref-a", + }); + + const info = await syncValue(a.clientInformation()); + expect(info?.client_id).toBe("client-on-62000"); + expect(info && "redirect_uris" in info ? info.redirect_uris : undefined).toEqual([ + "http://127.0.0.1:62000/callback", + ]); + const disk = await loadAuthState(linear, home); + expect(disk.clientInformation?.client_id).toBe("client-on-62000"); + expect(disk.clientInformation?.redirect_uris).toEqual(["http://127.0.0.1:62000/callback"]); + expect(disk.tokens?.access_token).toBe("tok-a"); + expect((await syncValue(a.tokens()))?.access_token).toBe("tok-a"); + }); + + test("saveCodeVerifier after a different-port sibling saveClient does not adopt that client", 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, + }); + await saveClient(a, clientInfo(62000)); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(b, clientInfo(60435)); + await a.saveCodeVerifier("pkce-a"); + + const info = await syncValue(a.clientInformation()); + expect(info?.client_id).toBe("client-on-62000"); + expect(info && "redirect_uris" in info ? info.redirect_uris : undefined).toEqual([ + "http://127.0.0.1:62000/callback", + ]); + }); + + test("idle tokens getter adopts a sibling's completed auth without rewriting matching DCR", 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, + }); + await saveClient(a, clientInfo(62000)); + await a.saveTokens({ + access_token: "tok-a", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref-a", + }); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(b, clientInfo(60435)); + await b.saveTokens({ + access_token: "tok-b", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref-b", + }); + + expect((await syncValue(a.tokens()))?.access_token).toBe("tok-b"); + const disk = await loadAuthState(linear, home); + expect(disk.tokens?.access_token).toBe("tok-b"); + expect(disk.clientInformation?.client_id).toBe("client-on-60435"); + expect(disk.clientInformation?.redirect_uris).toEqual(["http://127.0.0.1:60435/callback"]); + }); + + test("same-port DCR rotation after a different-port sibling saveClient keeps the new client", async () => { + const home = await tempHome(); + const v1 = { ...clientInfo(62000), client_id: "client-on-62000-v1" }; + const v2 = { ...clientInfo(62000), client_id: "client-on-62000-v2" }; + const a = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(a, v1); + + const b = await createOAuthProvider({ + serverName: "linear", + serverURL: linear.serverURL, + redirectUrl: "http://127.0.0.1:60435/callback", + onAuthURL: () => undefined, + home, + }); + await saveClient(b, clientInfo(60435)); + await saveClient(a, v2); + + const info = await syncValue(a.clientInformation()); + expect(info?.client_id).toBe("client-on-62000-v2"); + expect(info && "redirect_uris" in info ? info.redirect_uris : undefined).toEqual([ + "http://127.0.0.1:62000/callback", + ]); + const disk = await loadAuthState(linear, home); + expect(disk.clientInformation?.client_id).toBe("client-on-62000-v2"); + expect(disk.clientInformation?.redirect_uris).toEqual(["http://127.0.0.1:62000/callback"]); + }); + + test("sync getters fall back to the in-memory mirror when the auth file disappears", 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 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()))?.access_token).toBe("tok"); + expect(await syncValue(provider.clientInformation())).toBeUndefined(); + }); + + test("keeps live tokens when the auth file is corrupt", 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 provider.saveTokens({ access_token: "tok", token_type: "bearer" }); + + const path = authFilePath(linear, home); + await writeFile(path, "{not-json"); + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + }); + + test("keeps the mirror through an unreadable auth file and recovers once readable", 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 provider.saveTokens({ access_token: "tok", token_type: "bearer" }); + + const path = authFilePath(linear, home); + await chmod(path, 0o000); + try { + try { + readFileSync(path, "utf8"); + // Owner-read still succeeds (root or platforms that ignore mode) — skip. + return; + } catch (err) { + expect( + typeof err === "object" && + err !== null && + "code" in err && + (err as { code?: unknown }).code === "EACCES", + ).toBe(true); + } + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + } finally { + await chmod(path, 0o600); + } + await saveAuthState(linear, { tokens: { access_token: "fresh", token_type: "bearer" } }, home); + expect((await syncValue(provider.tokens()))?.access_token).toBe("fresh"); + }); + + test("sync getters skip reading the auth file when mtime and size are unchanged", 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 provider.saveTokens({ access_token: "tok", token_type: "bearer" }); + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + + const read = spyOn(fs, "readFileSync"); + try { + expect((await syncValue(provider.tokens()))?.access_token).toBe("tok"); + expect(await syncValue(provider.clientInformation())).toBeUndefined(); + expect(read).not.toHaveBeenCalled(); + } finally { + read.mockRestore(); + } + }); + test("does not delete scoped state whose filename stem is another provider name", async () => { const home = await tempHome(); const dir = join(home, ".corbits", "mcp-auth"); diff --git a/src/mcp/oauth-provider.ts b/src/mcp/oauth-provider.ts index 4519f27d1..7c58b2b1a 100644 --- a/src/mcp/oauth-provider.ts +++ b/src/mcp/oauth-provider.ts @@ -1,3 +1,5 @@ +import { statSync } from "node:fs"; +import { homedir } from "node:os"; import type { OAuthClientProvider } from "@modelcontextprotocol/sdk/client/auth.js"; import type { OAuthClientInformationFull, @@ -5,7 +7,13 @@ import type { OAuthClientMetadata, OAuthTokens, } from "@modelcontextprotocol/sdk/shared/auth.js"; -import { updateAuthState, type MCPAuthIdentity, type MCPAuthState } from "./auth-store.js"; +import { + authFilePath, + tryLoadAuthStateSync, + updateAuthState, + type MCPAuthIdentity, + type MCPAuthState, +} from "./auth-store.js"; import { MCP_CLIENT_NAME } from "../branding.js"; export interface OAuthProviderOptions { @@ -38,13 +46,41 @@ function dropStaleClientRegistration(state: MCPAuthState, redirectUrl: string): delete state.codeVerifier; } -function replaceStored(stored: MCPAuthState, next: MCPAuthState): void { - delete stored.clientInformation; - delete stored.tokens; - delete stored.codeVerifier; - if (next.clientInformation !== undefined) stored.clientInformation = next.clientInformation; +function shouldAdoptClient(stored: MCPAuthState, next: MCPAuthState, redirectUrl: string): boolean { + if (next.clientInformation === undefined) return false; + if (redirectUrisInclude(next.clientInformation, redirectUrl)) return true; + // Other-port DCR is a sibling's in-progress registration unless they also + // published new tokens (completed re-auth). + return next.tokens !== undefined && next.tokens.access_token !== stored.tokens?.access_token; +} + +function assignTokens(stored: MCPAuthState, next: MCPAuthState): void { if (next.tokens !== undefined) stored.tokens = next.tokens; - if (next.codeVerifier !== undefined) stored.codeVerifier = next.codeVerifier; + else delete stored.tokens; +} + +function assignClient(stored: MCPAuthState, next: MCPAuthState): void { + if (next.clientInformation !== undefined) stored.clientInformation = next.clientInformation; + else delete stored.clientInformation; +} + +function matchingLiveClient( + stored: MCPAuthState, + redirectUrl: string, +): OAuthClientInformationFull | undefined { + const live = stored.clientInformation; + if (live === undefined || !redirectUrisInclude(live, redirectUrl)) return undefined; + return live; +} + +function persistMatchingLiveClient( + stored: MCPAuthState, + next: MCPAuthState, + redirectUrl: string, +): void { + const live = matchingLiveClient(stored, redirectUrl); + if (live === undefined) return; + next.clientInformation = live; } export async function createOAuthProvider( @@ -54,20 +90,57 @@ export async function createOAuthProvider( serverName: opts.serverName, serverURL: opts.serverURL, }; + const home = opts.home ?? homedir(); // Load + scrub stale DCR under the per-file chain so concurrent providers see - // the same cleaned state. Mutations always re-read disk; this in-memory mirror - // only serves the SDK's sync getters (tokens / clientInformation / codeVerifier). + // the same cleaned state. Mutations always re-read disk; tokens and matching + // DCR are observed from disk so a sibling session's completed auth is picked + // up. PKCE stays instance-local after this snapshot — a different-port sibling + // must not clobber an in-progress verifier. const stored: MCPAuthState = await updateAuthState( identity, (state) => { dropStaleClientRegistration(state, opts.redirectUrl); }, - opts.home, + home, ); const apply = async (mutator: (state: MCPAuthState) => void): Promise => { - const next = await updateAuthState(identity, mutator, opts.home); - replaceStored(stored, next); + const next = await updateAuthState( + identity, + (state) => { + mutator(state); + persistMatchingLiveClient(stored, state, opts.redirectUrl); + }, + home, + ); + assignTokens(stored, next); + if (matchingLiveClient(stored, opts.redirectUrl) === undefined) { + assignClient(stored, next); + } + }; + + // Cheap staleness guard: statSync per getter, sync read only when the file's + // mtime or size changed. Stamp commits only after a successful read so a + // failed/unreadable file is retried on the next getter call. + const authPath = authFilePath(identity, home); + let seenStamp: string | undefined; + const refreshDurableFromDisk = (): void => { + let stamp: string | undefined; + try { + const stat = statSync(authPath); + stamp = `${String(stat.mtimeMs)}:${String(stat.size)}`; + } catch { + if (seenStamp === undefined) return; + seenStamp = undefined; + return; + } + if (stamp === seenStamp) return; + const next = tryLoadAuthStateSync(identity, home); + if (next === undefined) return; + seenStamp = stamp; + const adoptClient = shouldAdoptClient(stored, next, opts.redirectUrl); + assignTokens(stored, next); + if (adoptClient) assignClient(stored, next); }; let oauthState: string | undefined; @@ -89,14 +162,17 @@ export async function createOAuthProvider( }; }, clientInformation(): OAuthClientInformationMixed | undefined { + refreshDurableFromDisk(); return stored.clientInformation; }, saveClientInformation(info: OAuthClientInformationMixed): Promise { + stored.clientInformation = info as OAuthClientInformationFull; return apply((state) => { state.clientInformation = info as OAuthClientInformationFull; }); }, tokens(): OAuthTokens | undefined { + refreshDurableFromDisk(); return stored.tokens; }, saveTokens(tokens: OAuthTokens): Promise { @@ -110,6 +186,7 @@ export async function createOAuthProvider( opts.onAuthURL(opts.serverName, authorizationUrl.toString()); }, saveCodeVerifier(codeVerifier: string): Promise { + stored.codeVerifier = codeVerifier; return apply((state) => { state.codeVerifier = codeVerifier; }); @@ -119,9 +196,9 @@ export async function createOAuthProvider( throw new Error("No PKCE code verifier saved for this authorization."); return stored.codeVerifier; }, - resetAuthorization(): Promise { + async resetAuthorization(): Promise { oauthState = undefined; - return apply((state) => { + await apply((state) => { delete state.tokens; delete state.codeVerifier; // Next browser flow needs a client registered for *this* loopback port. @@ -129,6 +206,7 @@ export async function createOAuthProvider( delete state.clientInformation; } }); + delete stored.codeVerifier; }, }; }