diff --git a/src/oauth.ts b/src/oauth.ts index bbcca01..07994e5 100644 --- a/src/oauth.ts +++ b/src/oauth.ts @@ -429,6 +429,22 @@ export async function completeOAuthFlow( return { sessionId, session }; } +/** + * Does this bearer token have the shape of a session ID we issued? + * + * Session IDs are `randomBytes(32).toString("hex")` - 64 lowercase hex chars, + * no dashes. Digital Samba developer keys are UUIDs, so the two never collide. + * + * The HTTP transport uses this to tell "expired OAuth session, tell the client + * to re-authenticate" apart from "legacy developer key, pass it through". Get + * it wrong and an expired session is sent to the API as if it were a key, so + * the client sees the API's "Unauthenticated" rather than a 401 and never + * learns to re-authorise. + */ +export function isOAuthSessionId(token: string): boolean { + return /^[0-9a-f]{64}$/.test(token); +} + /** * Get session by ID */ diff --git a/src/transports/http.ts b/src/transports/http.ts index db9ff27..61e8332 100644 --- a/src/transports/http.ts +++ b/src/transports/http.ts @@ -43,6 +43,7 @@ import { exchangeAuthorizationCode, exchangeCodeForTokens, getRegisteredClientCount, + isOAuthSessionId, } from "../oauth.js"; export interface HttpTransportConfig { @@ -132,18 +133,25 @@ function authMiddleware(requireAuth: boolean) { let sessionId: string | null = null; if (token.startsWith("oauth:")) { sessionId = token.substring(6); - } else { - // Try to use token directly as session ID (Claude Desktop DCR flow) - const directSession = await getAccessTokenFromSession(token); - if (directSession) { - sessionId = token; - } + } else if (isOAuthSessionId(token)) { + // Shaped like a session ID we issued, so treat it as one even when the + // session is gone. Falling through to the developer-key branch here + // would send a dead session ID to the API as if it were a key: the + // client gets a confusing "Unauthenticated" from the API instead of a + // 401, never learns to re-authenticate, and can never recover. + sessionId = token; } if (sessionId) { const accessToken = await getAccessTokenFromSession(sessionId); if (!accessToken) { + // Point the client at the OAuth metadata so it can re-authorize + // itself rather than needing the connector removed and re-added. + res.setHeader( + "WWW-Authenticate", + 'Bearer realm="mcp", error="invalid_token", error_description="The OAuth session has expired", resource_metadata="/.well-known/oauth-protected-resource"', + ); res.status(401).json({ jsonrpc: "2.0", error: { diff --git a/tests/unit/oauth-session-id-shape.test.ts b/tests/unit/oauth-session-id-shape.test.ts new file mode 100644 index 0000000..23032ef --- /dev/null +++ b/tests/unit/oauth-session-id-shape.test.ts @@ -0,0 +1,65 @@ +/** + * Unit tests for isOAuthSessionId (src/oauth.ts) + * + * This predicate decides whether an unrecognised bearer token is treated as an + * expired OAuth session (401 + WWW-Authenticate, so the client re-authorises) + * or as a legacy direct developer key (passed to the API as-is). + * + * Getting it wrong in either direction is user-visible: + * - too narrow: expired sessions are sent to the API as developer keys, the + * client sees "Unauthenticated" instead of a 401, and can never recover. + * - too broad: legacy developer keys are rejected as expired sessions. + * + * @module tests/unit/oauth-session-id-shape + */ + +import { randomBytes } from "node:crypto"; +import { isOAuthSessionId } from "../../src/oauth.js"; + +describe("isOAuthSessionId", () => { + it("accepts session IDs generated the way oauth.ts generates them", () => { + for (let i = 0; i < 20; i++) { + const sessionId = randomBytes(32).toString("hex"); + expect(isOAuthSessionId(sessionId)).toBe(true); + } + }); + + it("rejects Digital Samba developer keys, which are UUIDs", () => { + // Real-world shape, from docs/digital-samba-api.md + expect(isOAuthSessionId("fc16892a-556b-4c2d-a522-82e6b3e884cc")).toBe( + false, + ); + expect(isOAuthSessionId("57670ebd-0de2-4f92-8bce-661bec142dde")).toBe( + false, + ); + }); + + it("rejects tokens of the wrong length", () => { + expect(isOAuthSessionId("a".repeat(63))).toBe(false); + expect(isOAuthSessionId("a".repeat(65))).toBe(false); + expect(isOAuthSessionId(randomBytes(16).toString("hex"))).toBe(false); + }); + + it("rejects non-hex characters", () => { + // 64 chars but 'g' is not hex + expect(isOAuthSessionId("g".repeat(64))).toBe(false); + expect(isOAuthSessionId(`${"a".repeat(63)}Z`)).toBe(false); + }); + + it("rejects uppercase hex, which we never emit", () => { + const upper = randomBytes(32).toString("hex").toUpperCase(); + expect(isOAuthSessionId(upper)).toBe(false); + }); + + it("rejects empty and whitespace-padded tokens", () => { + expect(isOAuthSessionId("")).toBe(false); + expect(isOAuthSessionId(` ${"a".repeat(64)}`)).toBe(false); + expect(isOAuthSessionId(`${"a".repeat(64)}\n`)).toBe(false); + }); + + it("rejects the oauth: prefixed form, which is handled separately", () => { + expect(isOAuthSessionId(`oauth:${randomBytes(32).toString("hex")}`)).toBe( + false, + ); + }); +});