modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(oauth): walk the candidates in the CLI refresh path too

Browse source 
Copilot review round 1.

`refreshStoredAuthToken` consumes the same fallback and made a single SDK
discovery call, so preserving the path there — without the walk the other two
consumers got — would have regressed exactly the case the walk exists to keep:
a path-hosted MCP server whose authorization metadata is published at the
domain root.

It injects its own discovery function as a test seam, so the walk is now also
exposed as `discoverAuthorizationServerMetadataFromCandidates(candidates,
discover)`, which takes that function and reports *which* candidate answered —
the CLI needs the winning URL, since it is the base the token request is made
against, not just the metadata.

Also drops the `as unknown as typeof fetch` from the new test in favour of
`vi.fn<typeof fetch>()`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-26 00:34:27 -04:00
commit 2bb0b6245c
5 changed files with 163 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

12
AGENTS.md
View file

@ -90,7 +90,17 @@ v2/main/
│ │ # 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 — #2110;
│ │ # 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

91
clients/cli/__tests__/stored-auth.test.ts
View file

@ -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]: {

32
clients/cli/src/cli.ts
View file

@ -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 {

4
clients/web/src/test/integration/auth/discovery.test.ts
View file

@ -473,12 +473,12 @@ describe("path-hosted MCP servers (#2110)", () => {
vi.mocked(discoverAuthorizationServerMetadata).mockResolvedValue(
undefined,
);
const fetchFn = vi.fn();
const fetchFn = vi.fn<typeof fetch>();
await discoverAuthorizationServerMetadataForServer(
pathServerUrl,
undefined,
fetchFn as unknown as typeof fetch,
fetchFn,
);
expect(discoverAuthorizationServerMetadata).toHaveBeenCalledWith(

48
core/auth/discovery.ts
View file

@ -74,33 +74,35 @@ export function getAuthorizationServerUrlCandidates(
return [primary, new URL("/", primary)];
}
export interface DiscoveredAuthorizationServer {
/** The candidate that answered — not necessarily the first one tried. */
authorizationServerUrl: URL;
metadata: NonNullable<AuthorizationServerMetadata>;
}
/**
* Discovers authorization server metadata for an MCP server, walking the
* candidates above until one answers.
* 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 discoverAuthorizationServerMetadataForServer(
serverUrl: string,
resourceMetadata?: OAuthProtectedResourceMetadata | null,
fetchFn?: typeof fetch,
): Promise<AuthorizationServerMetadata> {
const candidates = getAuthorizationServerUrlCandidates(
serverUrl,
resourceMetadata,
);
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 discoverAuthorizationServerMetadata(candidate, {
fetchFn,
});
if (metadata) return metadata;
const metadata = await discover(candidate);
if (metadata) return { authorizationServerUrl: candidate, metadata };
} catch (err) {
if (!sawError) {
firstError = err;
@ -112,6 +114,22 @@ export async function discoverAuthorizationServerMetadataForServer(
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;
}
/**
* Discovers OAuth scopes from server metadata, with preference for resource metadata scopes
* @param serverUrl - The MCP server URL