fix(auth): pass the challenge's resource_metadata URL to SDK auth()

An RFC 9728 `WWW-Authenticate: Bearer resource_metadata="…"` challenge was
parsed and then dropped by the normalized `AuthChallenge`, so every
challenge-driven `mcpAuth()` call ran discovery without it and probed
locations derived from the MCP server URL instead. OAuth failed outright
against any server whose metadata document lives off the well-known path.

The SDK handles this correctly inside its own transports, but the Inspector
never reaches that path — it intercepts the 401 and drives `auth()` itself
to capture the authorization URL and run the step-up scope union.

Carry the advertised URL on `AuthChallenge` as a string (it crosses the web
client's remote-backend boundary as JSON), convert it at the OAuth boundary,
and pass it to the standard challenge leg, the forced step-up leg, and CIMD
pre-registration — which runs before `auth()` and did its own discovery. A
malformed value is ignored rather than surfaced, matching the SDK's parser.
The callback leg needs nothing: SDK `auth()` persists the URL in its
discovery state.

Adds an `oauth.resourceMetadataPath` test-server mode that moves the
document off the well-known path entirely, so the integration test proves
discovery genuinely fails without the fix.

Closes #2071

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-24 19:48:40 -04:00
commit f8a46726b7
4 changed files with 469 additions and 6 deletions

View file

@ -1,6 +1,7 @@
import { describe, it, expect } from "vitest";
import {
AuthChallengeError,
challengeResourceMetadataUrl,
AuthRecoveryRequiredError,
findNestedAuthError,
isAuthChallengeError,
@ -286,6 +287,58 @@ describe("parseAuthChallengeFromError", () => {
});
});
describe("resource_metadata (RFC 9728) preservation — #2071", () => {
const METADATA_URL = "http://127.0.0.1:3001/custom/protected-resource";
const HEADER = `Bearer resource_metadata="${METADATA_URL}"`;
it("keeps the advertised URL when parsing a response challenge", () => {
const response = new Response(null, {
status: 401,
headers: { "WWW-Authenticate": HEADER },
});
expect(parseAuthChallengeFromResponse(response)).toEqual({
reason: "token_expired",
resourceMetadataUrl: METADATA_URL,
raw: { httpStatus: 401, wwwAuthenticate: HEADER },
});
});
it("keeps the advertised URL when parsing an error challenge", () => {
expect(
parseAuthChallengeFromError({ status: 401, wwwAuthenticate: HEADER }),
).toMatchObject({ resourceMetadataUrl: METADATA_URL });
});
it("omits the field entirely when the challenge does not advertise one", () => {
const response = new Response(null, {
status: 401,
headers: { "WWW-Authenticate": "Bearer" },
});
expect(parseAuthChallengeFromResponse(response)).not.toHaveProperty(
"resourceMetadataUrl",
);
});
it("converts the stored string to a URL at the OAuth boundary", () => {
expect(
challengeResourceMetadataUrl({ resourceMetadataUrl: METADATA_URL })?.href,
).toBe(METADATA_URL);
});
it.each([
["absent", undefined],
["empty", ""],
["whitespace", " "],
["not a URL", "not-a-url"],
])("ignores a %s resource_metadata value", (_label, value) => {
expect(
challengeResourceMetadataUrl({ resourceMetadataUrl: value }),
).toBeUndefined();
});
});
describe("isAuthChallengeError", () => {
it("detects AuthChallengeError instances", () => {
const err = new AuthChallengeError({ reason: "token_expired" }, 401);

View file

@ -1,4 +1,4 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { describe, it, expect, vi, afterEach, beforeEach } from "vitest";
import type { OAuthStorage } from "@inspector/core/auth/storage.js";
import { BaseOAuthClientProvider } from "@inspector/core/auth/providers.js";
import { ensureCimdClientRegistration } from "@inspector/core/auth/cimd.js";
@ -31,6 +31,10 @@ describe("ensureCimdClientRegistration", () => {
} as unknown as OAuthStorage;
});
afterEach(() => {
vi.unstubAllGlobals();
});
it("pre-registers URL-based client id when AS supports CIMD", async () => {
const fetchFn = vi.fn(async (input: RequestInfo | URL) => {
const url = String(input);
@ -97,6 +101,56 @@ describe("ensureCimdClientRegistration", () => {
expect(storage.saveClientInformation).not.toHaveBeenCalled();
});
it("discovers protected-resource metadata at the challenge-advertised URL (#2071)", async () => {
const metadataUrl = new URL(
"http://127.0.0.1:9999/custom/protected-resource",
);
// `ensureCimdClientRegistration` runs protected-resource discovery through
// the SDK's default fetch, so the advertised URL is observable only on the
// global — the AS-metadata leg is the one that takes `fetchFn`.
const globalFetch = vi.fn(async (input: RequestInfo | URL) =>
String(input) === metadataUrl.href
? new Response(
JSON.stringify({
resource: SERVER_URL,
authorization_servers: ["http://127.0.0.1:9999"],
}),
)
: new Response(null, { status: 404 }),
);
vi.stubGlobal("fetch", globalFetch);
const fetchFn = vi.fn(
async () =>
new Response(
JSON.stringify({
issuer: "http://127.0.0.1:9999",
authorization_endpoint: "http://127.0.0.1:9999/oauth/authorize",
token_endpoint: "http://127.0.0.1:9999/oauth/token",
response_types_supported: ["code"],
client_id_metadata_document_supported: true,
}),
),
);
await ensureCimdClientRegistration({
serverUrl: SERVER_URL,
provider: createProvider(storage),
fetchFn,
resourceMetadataUrl: metadataUrl,
});
expect(globalFetch).toHaveBeenCalled();
for (const [input] of globalFetch.mock.calls) {
expect(String(input)).toBe(metadataUrl.href);
}
expect(storage.saveClientInformation).toHaveBeenCalledWith(
SERVER_URL,
{ client_id: METADATA_URL },
{ registrationKind: "cimd" },
);
});
it("no-ops when client information is already stored", async () => {
storage.getClientInformation = vi.fn(async () => ({
client_id: "existing-client",

View file

@ -1729,6 +1729,98 @@ describe("OAuthManager", () => {
});
});
describe("resource_metadata forwarding (RFC 9728) — #2071", () => {
const METADATA_URL = "http://127.0.0.1:3001/custom/protected-resource";
it("forwards the advertised metadata URL to auth() as a URL", async () => {
mockedMcpAuth.mockResolvedValue("AUTHORIZED");
const manager = new OAuthManager(createMockParams());
await manager.handleAuthChallenge({
reason: "token_expired",
resourceMetadataUrl: METADATA_URL,
});
expect(mockedMcpAuth).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
resourceMetadataUrl: new URL(METADATA_URL),
}),
);
});
it("leaves the metadata URL undefined when the challenge advertises none", async () => {
mockedMcpAuth.mockResolvedValue("AUTHORIZED");
const manager = new OAuthManager(createMockParams());
await manager.handleAuthChallenge({ reason: "token_expired" });
expect(mockedMcpAuth).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ resourceMetadataUrl: undefined }),
);
});
it("ignores a malformed metadata URL rather than failing authorization", async () => {
mockedMcpAuth.mockResolvedValue("AUTHORIZED");
const manager = new OAuthManager(createMockParams());
const outcome = await manager.handleAuthChallenge({
reason: "token_expired",
resourceMetadataUrl: "not-a-url",
});
expect(outcome).toEqual({ kind: "satisfied" });
expect(mockedMcpAuth).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ resourceMetadataUrl: undefined }),
);
});
it("forwards it on the forced step-up reauthorization leg too", async () => {
mockedMcpAuth.mockResolvedValue("AUTHORIZED");
const params = createMockParams();
const noScopeTokens = {
access_token: "a",
token_type: "Bearer",
scope: "",
};
const midScopeTokens = {
access_token: "b",
token_type: "Bearer",
scope: "newscope",
};
const grantedTokens = {
access_token: "d",
token_type: "Bearer",
scope: "granted:scope",
};
storageOf(params)
.getTokens.mockResolvedValueOnce(undefined)
.mockResolvedValueOnce(undefined)
.mockResolvedValueOnce(noScopeTokens)
.mockResolvedValueOnce(noScopeTokens)
.mockResolvedValueOnce(midScopeTokens)
.mockResolvedValueOnce(grantedTokens);
const manager = new OAuthManager(params);
await manager.handleAuthChallenge({
reason: "insufficient_scope",
resourceMetadataUrl: METADATA_URL,
});
expect(mockedMcpAuth).toHaveBeenCalledTimes(2);
expect(mockedMcpAuth).toHaveBeenNthCalledWith(
2,
expect.anything(),
expect.objectContaining({
forceReauthorization: true,
resourceMetadataUrl: new URL(METADATA_URL),
}),
);
});
});
describe("handleAuthChallenge (additional branch coverage)", () => {
it("resolves via the second satisfaction check inside the mutex", async () => {
const params = createMockParams();

View file

@ -0,0 +1,106 @@
/**
* RFC 9728 `resource_metadata` end-to-end, against a real OAuth test server
* whose protected-resource metadata document lives at a non-default path and
* whose well-known locations deliberately 404 (#2071).
*
* The unit tests prove the URL is parsed and handed to `mcpAuth()`. This file
* proves the two halves that only a real server can: that the challenge the
* server actually emits parses into `AuthChallenge.resourceMetadataUrl`, and
* that SDK discovery genuinely cannot find the document without it — so the
* plumbing is load-bearing rather than merely present.
*/
import { describe, it, expect, beforeAll, afterAll } from "vitest";
import { fileURLToPath } from "node:url";
import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/client";
import {
TestServerHttp,
getDefaultServerConfig,
createOAuthTestServerConfig,
loadConfig,
resolveConfig,
waitForOAuthWellKnown,
} from "@modelcontextprotocol/inspector-test-server";
import {
challengeResourceMetadataUrl,
parseAuthChallengeFromResponse,
} from "@inspector/core/auth/challenge.js";
const METADATA_PATH = "/custom/protected-resource";
describe("OAuth challenge resource_metadata (RFC 9728)", () => {
let mcpServer: TestServerHttp | null = null;
let serverUrl = "";
beforeAll(async () => {
mcpServer = new TestServerHttp({
...getDefaultServerConfig(),
serverType: "streamable-http" as const,
...createOAuthTestServerConfig({
requireAuth: true,
resourceMetadataPath: METADATA_PATH,
}),
});
const port = await mcpServer.start();
serverUrl = `http://localhost:${port}`;
await waitForOAuthWellKnown(serverUrl);
}, 30_000);
afterAll(async () => {
await mcpServer?.stop();
mcpServer = null;
}, 30_000);
it("advertises the non-default metadata URL on the 401 challenge", async () => {
const response = await fetch(`${serverUrl}/mcp`, { method: "POST" });
expect(response.status).toBe(401);
const challenge = parseAuthChallengeFromResponse(response);
expect(challenge?.resourceMetadataUrl).toBe(`${serverUrl}${METADATA_PATH}`);
expect(challengeResourceMetadataUrl(challenge!)?.href).toBe(
`${serverUrl}${METADATA_PATH}`,
);
});
it("serves the metadata document only from the advertised path", async () => {
const advertised = await fetch(`${serverUrl}${METADATA_PATH}`);
expect(advertised.status).toBe(200);
await expect(advertised.json()).resolves.toMatchObject({
authorization_servers: [serverUrl],
});
const wellKnown = await fetch(
`${serverUrl}/.well-known/oauth-protected-resource`,
);
expect(wellKnown.status).toBe(404);
});
it("SDK discovery resolves with the advertised URL and fails without it", async () => {
await expect(
discoverOAuthProtectedResourceMetadata(serverUrl, {
resourceMetadataUrl: new URL(`${serverUrl}${METADATA_PATH}`),
}),
).resolves.toMatchObject({ authorization_servers: [serverUrl] });
// The pre-fix behavior: discovery derived from the MCP server URL alone.
await expect(
discoverOAuthProtectedResourceMetadata(serverUrl),
).rejects.toThrow();
});
it("keeps the showcase config valid and carrying the custom path", () => {
const configPath = fileURLToPath(
new URL(
"../../../../../../test-servers/configs/oauth-custom-resource-metadata-http.json",
import.meta.url,
),
);
// loadConfig validates as it parses, so this is the regression test for
// the checked-in showcase config as much as for the new field.
expect(resolveConfig(loadConfig(configPath)).oauth).toMatchObject({
enabled: true,
resourceMetadataPath: METADATA_PATH,
});
});
});