Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
20 changes: 14 additions & 6 deletions src/transports/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
exchangeAuthorizationCode,
exchangeCodeForTokens,
getRegisteredClientCount,
isOAuthSessionId,
} from "../oauth.js";

export interface HttpTransportConfig {
Expand Down Expand Up @@ -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: {
Expand Down
65 changes: 65 additions & 0 deletions tests/unit/oauth-session-id-shape.test.ts
Original file line number Diff line number Diff line change
@@ -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,
);
});
});
Loading