fix: record an intercepted 401's status, headers and body (#2297)
The network tracker wrapped the auth-challenge interceptor, so a 401/403 never reached it as a response: the interceptor cancelled the body and threw, and the tracker logged only the thrown error. NetworkEntry renders its response section only when a status is present, so the body — and the WWW-Authenticate resource_metadata and scope — were never shown. Move the tracker below the interceptor so it records the real response and clones its body before the interceptor cancels it, and stop awaiting that cancel: with the tracker's clone as the other tee branch, the cancel settles only once the clone reads to the end, which would hang on an unending body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SG6q6YEDJyJB5H2Hr8nBDS Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
3e728a3b8c
commit
af4b574234
3 changed files with 105 additions and 28 deletions
|
|
@ -242,6 +242,85 @@ describe("Transport", () => {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("records an intercepted 401's status, WWW-Authenticate and body (#2297)", async () => {
|
||||||
|
const challengeBody = JSON.stringify({
|
||||||
|
error: "invalid_token",
|
||||||
|
error_description: "token expired",
|
||||||
|
});
|
||||||
|
const wwwAuthenticate =
|
||||||
|
'Bearer resource_metadata="https://mcp.example/.well-known/oauth-protected-resource", scope="files:read"';
|
||||||
|
const fetchFn: typeof fetch = async () =>
|
||||||
|
new Response(challengeBody, {
|
||||||
|
status: 401,
|
||||||
|
statusText: "Unauthorized",
|
||||||
|
headers: {
|
||||||
|
"content-type": "application/json",
|
||||||
|
"www-authenticate": wwwAuthenticate,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const fetchRequests: FetchRequestEntryBase[] = [];
|
||||||
|
const bodies = new Map<string, string>();
|
||||||
|
const result = createTransportNode(
|
||||||
|
{ type: "streamable-http", url: "https://mcp.example/mcp" },
|
||||||
|
{
|
||||||
|
fetchFn,
|
||||||
|
interceptAuthChallenges: true,
|
||||||
|
onFetchRequest: (entry) => fetchRequests.push(entry),
|
||||||
|
onFetchResponseBody: (id, body) => bodies.set(id, body),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
const client = new Client(
|
||||||
|
{ name: "test-client", version: "1.0.0" },
|
||||||
|
{ capabilities: {} },
|
||||||
|
);
|
||||||
|
await expect(client.connect(result.transport)).rejects.toThrow(
|
||||||
|
"MCP auth challenge (401)",
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(fetchRequests).toHaveLength(1);
|
||||||
|
const [entry] = fetchRequests;
|
||||||
|
expect(entry.error).toBeUndefined();
|
||||||
|
expect(entry.responseStatus).toBe(401);
|
||||||
|
expect(entry.responseHeaders?.["www-authenticate"]).toBe(wwwAuthenticate);
|
||||||
|
await vi.waitFor(() => expect(bodies.get(entry.id)).toBe(challengeBody));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("still throws an intercepted 401 whose body never ends (#2297)", async () => {
|
||||||
|
const fetchFn: typeof fetch = async () =>
|
||||||
|
new Response(
|
||||||
|
new ReadableStream<Uint8Array>({
|
||||||
|
start(controller) {
|
||||||
|
controller.enqueue(new TextEncoder().encode("partial"));
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
{
|
||||||
|
status: 401,
|
||||||
|
headers: { "www-authenticate": 'Bearer error="invalid_token"' },
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
const fetchRequests: FetchRequestEntryBase[] = [];
|
||||||
|
const result = createTransportNode(
|
||||||
|
{ type: "streamable-http", url: "https://mcp.example/mcp" },
|
||||||
|
{
|
||||||
|
fetchFn,
|
||||||
|
interceptAuthChallenges: true,
|
||||||
|
onFetchRequest: (entry) => fetchRequests.push(entry),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
const client = new Client(
|
||||||
|
{ name: "test-client", version: "1.0.0" },
|
||||||
|
{ capabilities: {} },
|
||||||
|
);
|
||||||
|
await expect(client.connect(result.transport)).rejects.toThrow(
|
||||||
|
"MCP auth challenge (401)",
|
||||||
|
);
|
||||||
|
expect(fetchRequests[0]?.responseStatus).toBe(401);
|
||||||
|
});
|
||||||
|
|
||||||
it("applies settings.headers to the outgoing streamable-http request", async () => {
|
it("applies settings.headers to the outgoing streamable-http request", async () => {
|
||||||
const server = createTestServerHttp({
|
const server = createTestServerHttp({
|
||||||
serverInfo: createTestServerInfo(),
|
serverInfo: createTestServerInfo(),
|
||||||
|
|
|
||||||
|
|
@ -24,8 +24,11 @@ export function createAuthChallengeInterceptFetch(
|
||||||
}
|
}
|
||||||
|
|
||||||
// Release the connection before throwing so the SDK transport is not left
|
// Release the connection before throwing so the SDK transport is not left
|
||||||
// with a half-read 401/403 body on streamable HTTP.
|
// with a half-read 401/403 body on streamable HTTP. Not awaited: when the
|
||||||
await response.body?.cancel().catch(() => {});
|
// network tracker sits below this wrapper (#2297) the body is one branch of
|
||||||
|
// a tee, and cancelling a branch settles only once the tracker's clone has
|
||||||
|
// read to the end — so a 401 whose body never ends would hang the throw.
|
||||||
|
void response.body?.cancel().catch(() => {});
|
||||||
|
|
||||||
throw new AuthChallengeError(
|
throw new AuthChallengeError(
|
||||||
challenge,
|
challenge,
|
||||||
|
|
|
||||||
|
|
@ -67,12 +67,27 @@ export function createTransportNode(
|
||||||
onAuthChallengeObserved
|
onAuthChallengeObserved
|
||||||
? createAuthChallengeObserverFetch(inner, onAuthChallengeObserved)
|
? createAuthChallengeObserverFetch(inner, onAuthChallengeObserved)
|
||||||
: inner;
|
: inner;
|
||||||
|
const withTracking = (inner: typeof fetch): typeof fetch =>
|
||||||
|
onFetchRequest
|
||||||
|
? createFetchTracker(inner, {
|
||||||
|
trackRequest: onFetchRequest,
|
||||||
|
updateResponseBody: onFetchResponseBody,
|
||||||
|
})
|
||||||
|
: inner;
|
||||||
// The observer sits *under* the interceptor so it still reports the
|
// The observer sits *under* the interceptor so it still reports the
|
||||||
// challenge on the path where interception throws — and, more to the point,
|
// challenge on the path where interception throws — and, more to the point,
|
||||||
// on the legacy first-auth path where interception is off entirely.
|
// on the legacy first-auth path where interception is off entirely.
|
||||||
|
//
|
||||||
|
// The network tracker sits under the interceptor too (#2297). Above it, the
|
||||||
|
// tracker never saw the 401/403 at all: the interceptor cancels the body and
|
||||||
|
// throws, so the entry was recorded from the thrown error alone — no status,
|
||||||
|
// no `WWW-Authenticate`, and no response body, which is exactly the detail
|
||||||
|
// someone debugging an OAuth challenge needs. Below it, the tracker records
|
||||||
|
// the real response and clones its body before the interceptor cancels it.
|
||||||
|
const trackedBase = withTracking(withChallengeObserver(baseFetch));
|
||||||
const fetchWithOptionalAuthIntercept = interceptAuthChallenges
|
const fetchWithOptionalAuthIntercept = interceptAuthChallenges
|
||||||
? createAuthChallengeInterceptFetch(withChallengeObserver(baseFetch))
|
? createAuthChallengeInterceptFetch(trackedBase)
|
||||||
: withChallengeObserver(baseFetch);
|
: trackedBase;
|
||||||
|
|
||||||
if (serverType === "stdio") {
|
if (serverType === "stdio") {
|
||||||
const stdioConfig = config as StdioServerConfig;
|
const stdioConfig = config as StdioServerConfig;
|
||||||
|
|
@ -111,15 +126,9 @@ export function createTransportNode(
|
||||||
const configuredSseFetch = sseConfig.eventSourceInit?.fetch as
|
const configuredSseFetch = sseConfig.eventSourceInit?.fetch as
|
||||||
| typeof fetch
|
| typeof fetch
|
||||||
| undefined;
|
| undefined;
|
||||||
const sseFetch = configuredSseFetch
|
const trackedFetch = configuredSseFetch
|
||||||
? withChallengeObserver(configuredSseFetch)
|
? withTracking(withChallengeObserver(configuredSseFetch))
|
||||||
: fetchWithOptionalAuthIntercept;
|
: fetchWithOptionalAuthIntercept;
|
||||||
const trackedFetch = onFetchRequest
|
|
||||||
? createFetchTracker(sseFetch, {
|
|
||||||
trackRequest: onFetchRequest,
|
|
||||||
updateResponseBody: onFetchResponseBody,
|
|
||||||
})
|
|
||||||
: sseFetch;
|
|
||||||
|
|
||||||
const headers = headersFromSettings(settings);
|
const headers = headersFromSettings(settings);
|
||||||
|
|
||||||
|
|
@ -134,18 +143,11 @@ export function createTransportNode(
|
||||||
...(headers && { headers }),
|
...(headers && { headers }),
|
||||||
};
|
};
|
||||||
|
|
||||||
const postFetch = onFetchRequest
|
|
||||||
? createFetchTracker(fetchWithOptionalAuthIntercept, {
|
|
||||||
trackRequest: onFetchRequest,
|
|
||||||
updateResponseBody: onFetchResponseBody,
|
|
||||||
})
|
|
||||||
: fetchWithOptionalAuthIntercept;
|
|
||||||
|
|
||||||
const transport = new SSEClientTransport(url, {
|
const transport = new SSEClientTransport(url, {
|
||||||
authProvider,
|
authProvider,
|
||||||
eventSourceInit,
|
eventSourceInit,
|
||||||
requestInit,
|
requestInit,
|
||||||
fetch: postFetch,
|
fetch: fetchWithOptionalAuthIntercept,
|
||||||
});
|
});
|
||||||
|
|
||||||
return { transport };
|
return { transport };
|
||||||
|
|
@ -161,17 +163,10 @@ export function createTransportNode(
|
||||||
...(headers && { headers }),
|
...(headers && { headers }),
|
||||||
};
|
};
|
||||||
|
|
||||||
const transportFetch = onFetchRequest
|
|
||||||
? createFetchTracker(fetchWithOptionalAuthIntercept, {
|
|
||||||
trackRequest: onFetchRequest,
|
|
||||||
updateResponseBody: onFetchResponseBody,
|
|
||||||
})
|
|
||||||
: fetchWithOptionalAuthIntercept;
|
|
||||||
|
|
||||||
const transport = new StreamableHTTPClientTransport(url, {
|
const transport = new StreamableHTTPClientTransport(url, {
|
||||||
authProvider,
|
authProvider,
|
||||||
requestInit,
|
requestInit,
|
||||||
fetch: transportFetch,
|
fetch: fetchWithOptionalAuthIntercept,
|
||||||
// SEP-2350: how the transport reacts to a `403 insufficient_scope`
|
// SEP-2350: how the transport reacts to a `403 insufficient_scope`
|
||||||
// challenge. Defaults to the SDK's `reauthorize` when unset.
|
// challenge. Defaults to the SDK's `reauthorize` when unset.
|
||||||
...(settings?.oauthOnInsufficientScope && {
|
...(settings?.oauthOnInsufficientScope && {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue