Merge remote-tracking branch 'origin/v2/fix/2123-root-union-schemas' into v2/fix/2123-root-union-schemas
This commit is contained in:
cliffhall
commit
a0659b20d1
8 changed files with 578 additions and 56 deletions
28
AGENTS.md
28
AGENTS.md
|
|
@ -73,6 +73,34 @@ v2/main/
|
|||
│ │ # the advertised document rather than a
|
||||
│ │ # location derived from the MCP server
|
||||
│ │ # URL — #2071;
|
||||
│ │ # discovery.ts the authorization-server
|
||||
│ │ # URL POLICY — when protected-resource
|
||||
│ │ # metadata names no authorization
|
||||
│ │ # server, the MCP server URL stands in,
|
||||
│ │ # and its PATH is load-bearing: the
|
||||
│ │ # SDK's buildDiscoveryUrls derives the
|
||||
│ │ # path-scoped well-known locations from
|
||||
│ │ # that pathname, so reducing it to the
|
||||
│ │ # origin can only ever probe the domain
|
||||
│ │ # root. getAuthorizationServerUrlCandidates
|
||||
│ │ # therefore yields the path-scoped URL
|
||||
│ │ # FIRST and the bare origin second, and
|
||||
│ │ # discoverAuthorizationServerMetadataForServer
|
||||
│ │ # walks them — the second candidate is
|
||||
│ │ # what keeps a server that merely lives
|
||||
│ │ # under a path while publishing metadata
|
||||
│ │ # at the root working, so do not drop it
|
||||
│ │ # in favour of a straight swap. EVERY
|
||||
│ │ # consumer of the fallback must walk:
|
||||
│ │ # discoverScopes, the CIMD probe, and
|
||||
│ │ # the CLI's refreshStoredAuthToken,
|
||||
│ │ # which is why the walk is also exposed
|
||||
│ │ # as ...FromCandidates(candidates,
|
||||
│ │ # discover) — the CLI injects its own
|
||||
│ │ # discovery function as a test seam and
|
||||
│ │ # needs to know WHICH candidate
|
||||
│ │ # answered, since that is the base its
|
||||
│ │ # token request is made against — #2110;
|
||||
│ │ # scopes.ts SEP-2350 scope union, oauthUx.ts
|
||||
│ │ # shared copy, mcpAuth.ts force-reauthorization,
|
||||
│ │ # issuerBinding.ts SEP-2352 callback-leg failure
|
||||
|
|
|
|||
|
|
@ -158,6 +158,97 @@ describe("refreshStoredAuthToken", () => {
|
|||
}
|
||||
});
|
||||
|
||||
// #2110: with no stored metadata the MCP server URL stands in as the
|
||||
// authorization server. A path-hosted server has two plausible answers, and
|
||||
// both must keep working — the path-scoped URL for a server that publishes
|
||||
// its metadata under its own path, the bare origin for one that merely lives
|
||||
// under a path. The candidate that answers is also the base the token request
|
||||
// is made against, so the walk has to report *which* one it was.
|
||||
it("prefers the path-scoped authorization server for a path-hosted MCP server", async () => {
|
||||
const path = writeOAuthFixture({
|
||||
[SERVER]: {
|
||||
tokens: { refresh_token: "old-refresh", token_type: "Bearer" },
|
||||
clientInformation: { client_id: "cid" },
|
||||
},
|
||||
});
|
||||
try {
|
||||
const refresh = vi.fn().mockResolvedValue(freshTokens);
|
||||
const pathMetadata = {
|
||||
issuer: "https://api.example/mcp",
|
||||
token_endpoint: "https://api.example/mcp/token",
|
||||
};
|
||||
const discover = vi.fn().mockResolvedValue(pathMetadata);
|
||||
|
||||
await refreshStoredAuthToken(SERVER, path, { refresh, discover });
|
||||
|
||||
expect(discover).toHaveBeenCalledTimes(1);
|
||||
expect(discover).toHaveBeenCalledWith(new URL("https://api.example/mcp"));
|
||||
const [authServerUrl, opts] = refresh.mock.calls[0]!;
|
||||
expect(authServerUrl).toEqual(new URL("https://api.example/mcp"));
|
||||
expect(opts.metadata).toEqual(pathMetadata);
|
||||
} finally {
|
||||
rmSync(path, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("falls back to the origin when a path-hosted server publishes metadata at the root", async () => {
|
||||
const path = writeOAuthFixture({
|
||||
[SERVER]: {
|
||||
tokens: { refresh_token: "old-refresh", token_type: "Bearer" },
|
||||
clientInformation: { client_id: "cid" },
|
||||
},
|
||||
});
|
||||
try {
|
||||
const refresh = vi.fn().mockResolvedValue(freshTokens);
|
||||
const rootMetadata = {
|
||||
issuer: "https://api.example",
|
||||
token_endpoint: "https://api.example/token",
|
||||
};
|
||||
const discover = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(undefined)
|
||||
.mockResolvedValueOnce(rootMetadata);
|
||||
|
||||
const token = await refreshStoredAuthToken(SERVER, path, {
|
||||
refresh,
|
||||
discover,
|
||||
});
|
||||
|
||||
expect(token).toBe("refreshed-access-token");
|
||||
expect(discover).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
new URL("https://api.example/"),
|
||||
);
|
||||
// The candidate that answered is the one the token request targets.
|
||||
const [authServerUrl, opts] = refresh.mock.calls[0]!;
|
||||
expect(authServerUrl).toEqual(new URL("https://api.example/"));
|
||||
expect(opts.metadata).toEqual(rootMetadata);
|
||||
} finally {
|
||||
rmSync(path, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps the path-scoped candidate as the token-request base when no candidate answers", async () => {
|
||||
const path = writeOAuthFixture({
|
||||
[SERVER]: {
|
||||
tokens: { refresh_token: "old-refresh", token_type: "Bearer" },
|
||||
clientInformation: { client_id: "cid" },
|
||||
},
|
||||
});
|
||||
try {
|
||||
const refresh = vi.fn().mockResolvedValue(freshTokens);
|
||||
const discover = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await refreshStoredAuthToken(SERVER, path, { refresh, discover });
|
||||
|
||||
expect(discover).toHaveBeenCalledTimes(2);
|
||||
const [authServerUrl] = refresh.mock.calls[0]!;
|
||||
expect(authServerUrl).toEqual(new URL("https://api.example/mcp"));
|
||||
} finally {
|
||||
rmSync(path, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("uses the distinct no_client_information code when client info is missing", async () => {
|
||||
const path = writeOAuthFixture({
|
||||
[SERVER]: {
|
||||
|
|
|
|||
|
|
@ -47,7 +47,11 @@ import {
|
|||
serializeOAuthPersistBlob,
|
||||
type OAuthPersistSnapshot,
|
||||
} from "@inspector/core/auth/oauth-persist.js";
|
||||
import { getAuthorizationServerUrl } from "@inspector/core/auth/discovery.js";
|
||||
import {
|
||||
discoverAuthorizationServerMetadataFromCandidates,
|
||||
getAuthorizationServerUrl,
|
||||
getAuthorizationServerUrlCandidates,
|
||||
} from "@inspector/core/auth/discovery.js";
|
||||
import { writeStoreFile } from "@inspector/core/storage/store-io.js";
|
||||
import {
|
||||
refreshAuthorization,
|
||||
|
|
@ -347,11 +351,27 @@ export async function refreshStoredAuthToken(
|
|||
);
|
||||
}
|
||||
|
||||
const authServerUrl = found.state.serverMetadata?.issuer
|
||||
? new URL(found.state.serverMetadata.issuer)
|
||||
: getAuthorizationServerUrl(serverUrl);
|
||||
const metadata =
|
||||
found.state.serverMetadata ?? (await discover(authServerUrl)) ?? undefined;
|
||||
// With stored metadata the issuer settles it. Without, the MCP server URL
|
||||
// stands in as the authorization server — and a path-hosted server has two
|
||||
// plausible answers, so walk them rather than committing to the path-scoped
|
||||
// one: a server that merely lives under a path while publishing its metadata
|
||||
// at the domain root must keep working (#2110). The candidate that *answered*
|
||||
// becomes `authServerUrl`, since it is also the base the token request below
|
||||
// is made against.
|
||||
let authServerUrl: URL;
|
||||
let metadata = found.state.serverMetadata ?? undefined;
|
||||
if (found.state.serverMetadata?.issuer) {
|
||||
authServerUrl = new URL(found.state.serverMetadata.issuer);
|
||||
} else {
|
||||
const discovered = await discoverAuthorizationServerMetadataFromCandidates(
|
||||
getAuthorizationServerUrlCandidates(serverUrl),
|
||||
discover,
|
||||
);
|
||||
authServerUrl =
|
||||
discovered?.authorizationServerUrl ??
|
||||
getAuthorizationServerUrl(serverUrl);
|
||||
metadata = discovered?.metadata;
|
||||
}
|
||||
|
||||
let tokens: OAuthTokens;
|
||||
try {
|
||||
|
|
|
|||
86
clients/web/src/test/core/mcp/remote/parseSSE.test.ts
Normal file
86
clients/web/src/test/core/mcp/remote/parseSSE.test.ts
Normal file
|
|
@ -0,0 +1,86 @@
|
|||
/**
|
||||
* `parseSSE` cross-chunk frame loss.
|
||||
*
|
||||
* The reproduction needs no browser and no network: `parseSSE` takes a reader,
|
||||
* so a test supplies the chunk boundary directly instead of hoping a real
|
||||
* transport happens to produce one. That is what makes this defect testable at
|
||||
* all — on the wire the split is a function of payload size, TCP segmentation
|
||||
* and any proxy in between, none of which a test can steer.
|
||||
*
|
||||
* The bug: `currentEvent` / `currentData` are declared INSIDE the read loop, so
|
||||
* a frame whose `data:` line arrives in one chunk and whose terminating blank
|
||||
* line arrives in the next is discarded. `buffer` already carries a partial
|
||||
* *line* across reads; nothing carries a partial *frame*.
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { parseSSE } from "@inspector/core/mcp/remote/remoteClientTransport";
|
||||
|
||||
/** A reader that yields exactly the chunks given, in order. */
|
||||
function readerFrom(chunks: string[]): ReadableStreamDefaultReader<Uint8Array> {
|
||||
const encoder = new TextEncoder();
|
||||
let i = 0;
|
||||
return {
|
||||
read: async () =>
|
||||
i < chunks.length
|
||||
? { done: false as const, value: encoder.encode(chunks[i++]!) }
|
||||
: { done: true as const, value: undefined },
|
||||
} as ReadableStreamDefaultReader<Uint8Array>;
|
||||
}
|
||||
|
||||
async function collect(chunks: string[]) {
|
||||
const out: { event: string; data: string }[] = [];
|
||||
for await (const frame of parseSSE(readerFrom(chunks))) out.push(frame);
|
||||
return out;
|
||||
}
|
||||
|
||||
const FRAME = 'event: message\ndata: {"id":1,"result":"payload"}\n\n';
|
||||
|
||||
describe("parseSSE chunk boundaries", () => {
|
||||
it("delivers a frame that arrives whole (the case that hides the bug)", async () => {
|
||||
// Chromium and Firefox happen to produce this shape at our payload sizes,
|
||||
// which is why the defect below has never been observed in practice.
|
||||
expect(await collect([FRAME])).toEqual([
|
||||
{ event: "message", data: '{"id":1,"result":"payload"}' },
|
||||
]);
|
||||
});
|
||||
|
||||
it("delivers a frame split between its data line and its terminator", async () => {
|
||||
// THE BUG. Chunk 1 ends after the data line's newline; chunk 2 carries only
|
||||
// the blank line that closes the frame. Nothing is malformed — this is one
|
||||
// valid frame, delivered in two reads.
|
||||
const split = ['event: message\ndata: {"id":1,"result":"payload"}\n', "\n"];
|
||||
expect(split.join("")).toBe(FRAME); // same bytes as above, different reads
|
||||
expect(await collect(split)).toEqual([
|
||||
{ event: "message", data: '{"id":1,"result":"payload"}' },
|
||||
]);
|
||||
});
|
||||
|
||||
it("delivers a frame split mid-payload", async () => {
|
||||
const split = ['event: message\ndata: {"id":1,"res', 'ult":"payload"}\n\n'];
|
||||
expect(split.join("")).toBe(FRAME);
|
||||
expect(await collect(split)).toEqual([
|
||||
{ event: "message", data: '{"id":1,"result":"payload"}' },
|
||||
]);
|
||||
});
|
||||
|
||||
it("delivers every frame when a burst is split at each boundary", async () => {
|
||||
// Bytes for three frames, re-cut so each split lands between a data line
|
||||
// and its terminator — the shape a large response most plausibly produces.
|
||||
const chunks: string[] = [];
|
||||
for (const id of [1, 2, 3]) {
|
||||
chunks.push(`event: message\ndata: {"id":${id}}\n`, "\n");
|
||||
}
|
||||
expect(await collect(chunks)).toEqual([
|
||||
{ event: "message", data: '{"id":1}' },
|
||||
{ event: "message", data: '{"id":2}' },
|
||||
{ event: "message", data: '{"id":3}' },
|
||||
]);
|
||||
});
|
||||
|
||||
it("flushes a final frame when the stream ends without a blank line", async () => {
|
||||
expect(await collect(['event: message\ndata: {"id":9}\n'])).toEqual([
|
||||
{ event: "message", data: '{"id":9}' },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,7 +1,9 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import {
|
||||
discoverAuthorizationServerMetadataForServer,
|
||||
discoverScopes,
|
||||
getAuthorizationServerUrl,
|
||||
getAuthorizationServerUrlCandidates,
|
||||
} from "@inspector/core/auth/discovery.js";
|
||||
import type { OAuthProtectedResourceMetadata } from "@modelcontextprotocol/client";
|
||||
|
||||
|
|
@ -335,3 +337,172 @@ describe("getAuthorizationServerUrl", () => {
|
|||
expect(captured?.message).toMatch(/Invalid MCP server URL: "" \(.+\)/);
|
||||
});
|
||||
});
|
||||
|
||||
// #2110: an MCP server hosted under a path (Moodle, a WordPress plugin, any
|
||||
// PHP endpoint) publishes its OAuth metadata under that same path. Reducing the
|
||||
// fallback authorization-server URL to the origin threw that path away, so
|
||||
// discovery could only ever probe the domain root and 404'd.
|
||||
describe("path-hosted MCP servers (#2110)", () => {
|
||||
const pathServerUrl = "https://misc.example.com/mod/minilesson/mcp.php";
|
||||
|
||||
describe("getAuthorizationServerUrl", () => {
|
||||
it("keeps the server's path in the fallback URL", () => {
|
||||
expect(getAuthorizationServerUrl(pathServerUrl)).toEqual(
|
||||
new URL(pathServerUrl),
|
||||
);
|
||||
});
|
||||
|
||||
it("strips query and fragment, which an RFC 8414 issuer never carries", () => {
|
||||
expect(
|
||||
getAuthorizationServerUrl(`${pathServerUrl}?session=abc#frag`),
|
||||
).toEqual(new URL(pathServerUrl));
|
||||
});
|
||||
});
|
||||
|
||||
describe("getAuthorizationServerUrlCandidates", () => {
|
||||
it("tries the path-scoped URL before the bare origin", () => {
|
||||
expect(getAuthorizationServerUrlCandidates(pathServerUrl)).toEqual([
|
||||
new URL(pathServerUrl),
|
||||
new URL("https://misc.example.com/"),
|
||||
]);
|
||||
});
|
||||
|
||||
it("offers a single candidate for a root-hosted server", () => {
|
||||
expect(
|
||||
getAuthorizationServerUrlCandidates("https://mcp.example.com"),
|
||||
).toEqual([new URL("https://mcp.example.com/")]);
|
||||
});
|
||||
|
||||
it("does not probe past an authorization server named by resource metadata", () => {
|
||||
const resourceMetadata: OAuthProtectedResourceMetadata = {
|
||||
resource: pathServerUrl,
|
||||
authorization_servers: ["https://auth.example.com/realms/r"],
|
||||
};
|
||||
expect(
|
||||
getAuthorizationServerUrlCandidates(pathServerUrl, resourceMetadata),
|
||||
).toEqual([new URL("https://auth.example.com/realms/r")]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("discoverAuthorizationServerMetadataForServer", () => {
|
||||
const metadata = {
|
||||
issuer: pathServerUrl,
|
||||
authorization_endpoint: "https://misc.example.com/mod/authorize.php",
|
||||
token_endpoint: "https://misc.example.com/mod/token.php",
|
||||
response_types_supported: ["code"],
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("returns the path-scoped result without probing the origin", async () => {
|
||||
const { discoverAuthorizationServerMetadata } =
|
||||
await import("@modelcontextprotocol/client");
|
||||
vi.mocked(discoverAuthorizationServerMetadata).mockResolvedValue(
|
||||
metadata,
|
||||
);
|
||||
|
||||
await expect(
|
||||
discoverAuthorizationServerMetadataForServer(pathServerUrl),
|
||||
).resolves.toEqual(metadata);
|
||||
expect(discoverAuthorizationServerMetadata).toHaveBeenCalledTimes(1);
|
||||
expect(discoverAuthorizationServerMetadata).toHaveBeenCalledWith(
|
||||
new URL(pathServerUrl),
|
||||
{ fetchFn: undefined },
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the origin when the path-scoped candidate has no metadata", async () => {
|
||||
const { discoverAuthorizationServerMetadata } =
|
||||
await import("@modelcontextprotocol/client");
|
||||
vi.mocked(discoverAuthorizationServerMetadata)
|
||||
.mockResolvedValueOnce(undefined)
|
||||
.mockResolvedValueOnce(metadata);
|
||||
|
||||
await expect(
|
||||
discoverAuthorizationServerMetadataForServer(pathServerUrl),
|
||||
).resolves.toEqual(metadata);
|
||||
expect(discoverAuthorizationServerMetadata).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
new URL("https://misc.example.com/"),
|
||||
{ fetchFn: undefined },
|
||||
);
|
||||
});
|
||||
|
||||
it("survives a throwing candidate while another remains", async () => {
|
||||
const { discoverAuthorizationServerMetadata } =
|
||||
await import("@modelcontextprotocol/client");
|
||||
vi.mocked(discoverAuthorizationServerMetadata)
|
||||
.mockRejectedValueOnce(new Error("issuer mismatch"))
|
||||
.mockResolvedValueOnce(metadata);
|
||||
|
||||
await expect(
|
||||
discoverAuthorizationServerMetadataForServer(pathServerUrl),
|
||||
).resolves.toEqual(metadata);
|
||||
});
|
||||
|
||||
it("rethrows the first error when every candidate fails", async () => {
|
||||
const { discoverAuthorizationServerMetadata } =
|
||||
await import("@modelcontextprotocol/client");
|
||||
vi.mocked(discoverAuthorizationServerMetadata)
|
||||
.mockRejectedValueOnce(new Error("path-scoped failed"))
|
||||
.mockRejectedValueOnce(new Error("origin failed"));
|
||||
|
||||
await expect(
|
||||
discoverAuthorizationServerMetadataForServer(pathServerUrl),
|
||||
).rejects.toThrow("path-scoped failed");
|
||||
});
|
||||
|
||||
it("resolves undefined when every candidate answers with no metadata", async () => {
|
||||
const { discoverAuthorizationServerMetadata } =
|
||||
await import("@modelcontextprotocol/client");
|
||||
vi.mocked(discoverAuthorizationServerMetadata).mockResolvedValue(
|
||||
undefined,
|
||||
);
|
||||
|
||||
await expect(
|
||||
discoverAuthorizationServerMetadataForServer(pathServerUrl),
|
||||
).resolves.toBeUndefined();
|
||||
expect(discoverAuthorizationServerMetadata).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("passes the caller's fetch through to every candidate", async () => {
|
||||
const { discoverAuthorizationServerMetadata } =
|
||||
await import("@modelcontextprotocol/client");
|
||||
vi.mocked(discoverAuthorizationServerMetadata).mockResolvedValue(
|
||||
undefined,
|
||||
);
|
||||
const fetchFn = vi.fn<typeof fetch>();
|
||||
|
||||
await discoverAuthorizationServerMetadataForServer(
|
||||
pathServerUrl,
|
||||
undefined,
|
||||
fetchFn,
|
||||
);
|
||||
|
||||
expect(discoverAuthorizationServerMetadata).toHaveBeenCalledWith(
|
||||
new URL(pathServerUrl),
|
||||
{ fetchFn },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("discoverScopes reaches a path-hosted authorization server", async () => {
|
||||
const { discoverAuthorizationServerMetadata } =
|
||||
await import("@modelcontextprotocol/client");
|
||||
vi.mocked(discoverAuthorizationServerMetadata).mockResolvedValue({
|
||||
issuer: pathServerUrl,
|
||||
authorization_endpoint: "https://misc.example.com/mod/authorize.php",
|
||||
token_endpoint: "https://misc.example.com/mod/token.php",
|
||||
response_types_supported: ["code"],
|
||||
scopes_supported: ["read"],
|
||||
});
|
||||
|
||||
await expect(discoverScopes(pathServerUrl)).resolves.toBe("read");
|
||||
expect(discoverAuthorizationServerMetadata).toHaveBeenCalledWith(
|
||||
new URL(pathServerUrl),
|
||||
{ fetchFn: undefined },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,9 +1,6 @@
|
|||
import {
|
||||
discoverAuthorizationServerMetadata,
|
||||
discoverOAuthProtectedResourceMetadata,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/client";
|
||||
import type { OAuthClientInformation } from "@modelcontextprotocol/client";
|
||||
import { getAuthorizationServerUrl } from "./discovery.js";
|
||||
import { discoverAuthorizationServerMetadataForServer } from "./discovery.js";
|
||||
import type { BaseOAuthClientProvider } from "./providers.js";
|
||||
|
||||
/**
|
||||
|
|
@ -49,14 +46,14 @@ export async function ensureCimdClientRegistration(params: {
|
|||
resourceMetadata = undefined;
|
||||
}
|
||||
|
||||
const authServerUrl = getAuthorizationServerUrl(
|
||||
// Walks the path-scoped authorization-server URL before the bare origin, so a
|
||||
// server hosted under a path is probed where it actually publishes its
|
||||
// metadata rather than only at the domain root (#2110).
|
||||
const metadata = await discoverAuthorizationServerMetadataForServer(
|
||||
params.serverUrl,
|
||||
resourceMetadata,
|
||||
params.fetchFn,
|
||||
);
|
||||
|
||||
const metadata = await discoverAuthorizationServerMetadata(authServerUrl, {
|
||||
...(params.fetchFn && { fetchFn: params.fetchFn }),
|
||||
});
|
||||
if (!metadata?.client_id_metadata_document_supported) return;
|
||||
|
||||
const clientInformation: OAuthClientInformation = {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,38 @@ import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/clien
|
|||
import type { OAuthProtectedResourceMetadata } from "@modelcontextprotocol/client";
|
||||
import { parseHttpUrl } from "./utils.js";
|
||||
|
||||
type AuthorizationServerMetadata = Awaited<
|
||||
ReturnType<typeof discoverAuthorizationServerMetadata>
|
||||
>;
|
||||
|
||||
/**
|
||||
* The MCP server's own URL, used as the authorization server when RFC 9728
|
||||
* protected-resource metadata named none.
|
||||
*
|
||||
* The path is kept. Reducing it to the origin (`new URL("/", serverUrl)`)
|
||||
* discards exactly the part that tells discovery where to look when the server
|
||||
* is not hosted at the domain root: the SDK's `buildDiscoveryUrls` derives the
|
||||
* path-scoped well-known locations — `/.well-known/oauth-authorization-server{path}`
|
||||
* and `{path}/.well-known/openid-configuration` — from this URL's pathname, so
|
||||
* an origin-only value can only ever probe the domain root (#2110).
|
||||
*
|
||||
* Query and fragment are stripped: an RFC 8414 issuer identifier carries
|
||||
* neither, and the SDK compares the discovered `issuer` against this URL.
|
||||
*/
|
||||
function serverUrlAsAuthorizationServer(serverUrl: string): URL {
|
||||
try {
|
||||
const url = new URL(serverUrl);
|
||||
url.search = "";
|
||||
url.hash = "";
|
||||
return url;
|
||||
} catch (err) {
|
||||
const detail = err instanceof Error ? err.message : String(err);
|
||||
throw new Error(`Invalid MCP server URL: "${serverUrl}" (${detail})`, {
|
||||
cause: err,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the URL to use for OAuth authorization server metadata discovery.
|
||||
* Uses resource metadata's authorization_servers[0] when present, otherwise the MCP server URL.
|
||||
|
|
@ -15,14 +47,87 @@ export function getAuthorizationServerUrl(
|
|||
if (first) {
|
||||
return parseHttpUrl(first, "protected resource authorization_servers[0]");
|
||||
}
|
||||
try {
|
||||
return new URL("/", serverUrl);
|
||||
} catch (err) {
|
||||
const detail = err instanceof Error ? err.message : String(err);
|
||||
throw new Error(`Invalid MCP server URL: "${serverUrl}" (${detail})`, {
|
||||
cause: err,
|
||||
});
|
||||
return serverUrlAsAuthorizationServer(serverUrl);
|
||||
}
|
||||
|
||||
/**
|
||||
* The authorization-server URLs to try, in order.
|
||||
*
|
||||
* When protected-resource metadata names an authorization server there is
|
||||
* exactly one candidate — the server told us where to look, and probing past it
|
||||
* would be guessing. Otherwise the MCP server URL itself stands in as the
|
||||
* authorization server, and a path-hosted server has two plausible answers: the
|
||||
* full URL, whose path-scoped well-known locations are what a server like
|
||||
* `https://example.com/mod/minilesson/mcp.php` publishes, and the bare origin,
|
||||
* used by a server that merely *lives* under a path while publishing its
|
||||
* metadata at the domain root. Trying the path-scoped form first fixes #2110
|
||||
* without regressing the root-hosted case, which is all the origin-only value
|
||||
* ever served.
|
||||
*/
|
||||
export function getAuthorizationServerUrlCandidates(
|
||||
serverUrl: string,
|
||||
resourceMetadata?: OAuthProtectedResourceMetadata | null,
|
||||
): URL[] {
|
||||
const primary = getAuthorizationServerUrl(serverUrl, resourceMetadata);
|
||||
const namedByMetadata = Boolean(resourceMetadata?.authorization_servers?.[0]);
|
||||
if (namedByMetadata || primary.pathname === "/") return [primary];
|
||||
return [primary, new URL("/", primary)];
|
||||
}
|
||||
|
||||
export interface DiscoveredAuthorizationServer {
|
||||
/** The candidate that answered — not necessarily the first one tried. */
|
||||
authorizationServerUrl: URL;
|
||||
metadata: NonNullable<AuthorizationServerMetadata>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Walks `candidates` until one answers, and reports **which** one did.
|
||||
*
|
||||
* A candidate that fails — a 5xx, or an RFC 8414 §3.3 issuer mismatch, both of
|
||||
* which the SDK raises rather than returning `undefined` — is not fatal while
|
||||
* another candidate remains. If every candidate fails, the first error is
|
||||
* rethrown, so a caller still sees the original diagnosis rather than a bare
|
||||
* `undefined`.
|
||||
*
|
||||
* `discover` is injected rather than closed over so a caller that already owns
|
||||
* a discovery function — the CLI's `refreshStoredAuthToken`, which takes one as
|
||||
* a test seam — can walk the same candidates without giving that seam up.
|
||||
*/
|
||||
export async function discoverAuthorizationServerMetadataFromCandidates(
|
||||
candidates: URL[],
|
||||
discover: (url: URL) => Promise<AuthorizationServerMetadata>,
|
||||
): Promise<DiscoveredAuthorizationServer | undefined> {
|
||||
let firstError: unknown;
|
||||
let sawError = false;
|
||||
for (const candidate of candidates) {
|
||||
try {
|
||||
const metadata = await discover(candidate);
|
||||
if (metadata) return { authorizationServerUrl: candidate, metadata };
|
||||
} catch (err) {
|
||||
if (!sawError) {
|
||||
firstError = err;
|
||||
sawError = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (sawError) throw firstError;
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Discovers authorization server metadata for an MCP server, walking the
|
||||
* candidates above until one answers.
|
||||
*/
|
||||
export async function discoverAuthorizationServerMetadataForServer(
|
||||
serverUrl: string,
|
||||
resourceMetadata?: OAuthProtectedResourceMetadata | null,
|
||||
fetchFn?: typeof fetch,
|
||||
): Promise<AuthorizationServerMetadata> {
|
||||
const found = await discoverAuthorizationServerMetadataFromCandidates(
|
||||
getAuthorizationServerUrlCandidates(serverUrl, resourceMetadata),
|
||||
(candidate) => discoverAuthorizationServerMetadata(candidate, { fetchFn }),
|
||||
);
|
||||
return found?.metadata;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -38,13 +143,11 @@ export const discoverScopes = async (
|
|||
fetchFn?: typeof fetch,
|
||||
): Promise<string | undefined> => {
|
||||
try {
|
||||
const authServerUrl = getAuthorizationServerUrl(
|
||||
const metadata = await discoverAuthorizationServerMetadataForServer(
|
||||
serverUrl,
|
||||
resourceMetadata,
|
||||
);
|
||||
const metadata = await discoverAuthorizationServerMetadata(authServerUrl, {
|
||||
fetchFn,
|
||||
});
|
||||
);
|
||||
|
||||
// Prefer resource metadata scopes, but fall back to OAuth metadata if empty
|
||||
const resourceScopes = resourceMetadata?.scopes_supported;
|
||||
|
|
|
|||
|
|
@ -165,14 +165,55 @@ function legacySessionId(json: RemoteConnectResponse): string | undefined {
|
|||
}
|
||||
|
||||
/**
|
||||
* Parse SSE stream from a ReadableStream.
|
||||
* Yields { event, data } for each SSE message.
|
||||
* Parse an SSE stream from a ReadableStream.
|
||||
* Yields `{ event, data }` for each complete SSE message.
|
||||
*
|
||||
* ⚠️ **The in-progress event must live OUTSIDE the read loop.** A `read()`
|
||||
* returns an arbitrary slice of bytes, not a whole frame, so a frame's `data:`
|
||||
* line can arrive in one chunk and the blank line terminating it in the next.
|
||||
* Declaring `currentEvent` / `currentData` inside the loop — which this did
|
||||
* until #2134 — discards the accumulated payload at that boundary: the frame is
|
||||
* never yielded, the JSON-RPC response it carried never settles, and the caller
|
||||
* hangs forever with no error on any channel. `buffer` already carries a partial
|
||||
* *line* across reads; these carry the partial *frame*.
|
||||
*
|
||||
* It survived because Chromium and Firefox happen to deliver whole frames at the
|
||||
* payload sizes this transport sees. That is luck rather than a guarantee —
|
||||
* chunk boundaries are a function of payload size, TCP segmentation and any
|
||||
* intermediary — and the exposure grows with payload size, so a large
|
||||
* `resources/read` is the most likely first victim.
|
||||
*
|
||||
* Exported for `clients/web/src/test/core/mcp/remote/parseSSE.test.ts`, which is
|
||||
* the only way to reach the defect deterministically: driving a real transport
|
||||
* cannot steer where the chunk boundary lands, but a test that supplies the
|
||||
* reader can put it exactly on the terminator.
|
||||
*/
|
||||
async function* parseSSE(
|
||||
export async function* parseSSE(
|
||||
reader: ReadableStreamDefaultReader<Uint8Array>,
|
||||
): AsyncGenerator<{ event: string; data: string }> {
|
||||
const decoder = new TextDecoder();
|
||||
let buffer = "";
|
||||
// Carried ACROSS reads, not per chunk — see the header.
|
||||
let currentEvent = "message";
|
||||
let currentData: string[] = [];
|
||||
|
||||
const consume = (line: string): { event: string; data: string } | null => {
|
||||
if (line.startsWith("event:")) {
|
||||
currentEvent = line.slice(6).trim();
|
||||
} else if (line.startsWith("data:")) {
|
||||
currentData.push(line.slice(5).trimStart());
|
||||
} else if (line === "") {
|
||||
const complete =
|
||||
currentData.length > 0
|
||||
? { event: currentEvent, data: currentData.join("\n") }
|
||||
: null;
|
||||
currentEvent = "message";
|
||||
currentData = [];
|
||||
return complete;
|
||||
}
|
||||
// Anything else (a `:` keep-alive comment, `id:`, `retry:`) is ignored.
|
||||
return null;
|
||||
};
|
||||
|
||||
while (true) {
|
||||
const { done, value } = await reader.read();
|
||||
|
|
@ -183,36 +224,21 @@ async function* parseSSE(
|
|||
/* v8 ignore next -- String.split always returns a non-empty array, so pop() is never undefined; the ?? "" fallback is unreachable. */
|
||||
buffer = lines.pop() ?? "";
|
||||
|
||||
let currentEvent = "message";
|
||||
let currentData: string[] = [];
|
||||
|
||||
for (const line of lines) {
|
||||
if (line.startsWith("event:")) {
|
||||
currentEvent = line.slice(6).trim();
|
||||
} else if (line.startsWith("data:")) {
|
||||
currentData.push(line.slice(5).trimStart());
|
||||
} else if (line === "") {
|
||||
if (currentData.length > 0) {
|
||||
yield { event: currentEvent, data: currentData.join("\n") };
|
||||
}
|
||||
currentEvent = "message";
|
||||
currentData = [];
|
||||
}
|
||||
const complete = consume(line);
|
||||
if (complete) yield complete;
|
||||
}
|
||||
}
|
||||
|
||||
if (buffer.trim()) {
|
||||
const lines = buffer.split("\n");
|
||||
let currentEvent = "message";
|
||||
const currentData: string[] = [];
|
||||
for (const line of lines) {
|
||||
if (line.startsWith("event:")) currentEvent = line.slice(6).trim();
|
||||
else if (line.startsWith("data:"))
|
||||
currentData.push(line.slice(5).trimStart());
|
||||
}
|
||||
if (currentData.length > 0) {
|
||||
yield { event: currentEvent, data: currentData.join("\n") };
|
||||
}
|
||||
// Stream ended: flush the trailing partial line, then anything still held. A
|
||||
// server that closes without a final blank line has still delivered a complete
|
||||
// frame, and dropping it would lose the last message of every such stream.
|
||||
if (buffer) {
|
||||
const complete = consume(buffer);
|
||||
if (complete) yield complete;
|
||||
}
|
||||
if (currentData.length > 0) {
|
||||
yield { event: currentEvent, data: currentData.join("\n") };
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue