Skip to content

Commit 1017d14

Browse files
author
Heather Nelson
committed
fix(oauth): pass resource_metadata URL to auth() across all error sources
When Inspector handles an auth-related connection error, it must forward the `resource_metadata=...` URL advertised by the resource server (RFC 9728 §3) to the SDK's `auth()` and `discoverOAuthProtectedResourceMetadata()` calls. Without it, the SDK falls back to path-aware probes that 404 against resource servers exposing metadata at the suffix path (e.g. AWS AgentCore: `/runtimes/<arn>/invocations/.well-known/oauth-protected-resource`), then treats the resource server itself as the auth server, and redirects users to a bogus `<resource-host>/authorize`. This is one of several reports under #675. The proxy-mode path is particularly broken: the proxy unwraps upstream 401s into JSON-RPC errors with `data.upstream401.wwwAuthenticate`, so the SDK transport never sees a 401 and `onUnauthorized` doesn't fire. Adds a flexible extractor (`extractResourceMetadataUrlFromError`) covering three sources, in priority order: 1. `error.resourceMetadataUrl` (UnauthorizedError-style) 2. `error.response.headers` / `error.headers` WWW-Authenticate 3. `error.data.upstream401.wwwAuthenticate` (proxy-relayed envelope) Wires the result through `handleAuthError` in useConnection.ts so both `discoverOAuthProtectedResourceMetadata` and `auth()` get the verbatim URL when one is advertised. Lives in its own module so the unit tests don't pull in the SDK's CJS auth bundle (which transitively requires the ESM-only `pkce-challenge` package and breaks Jest module resolution). Tested: - New unit tests: 10/10 passing (covers all three sources, priority, and malformed inputs). - Manual e2e via Playwright against AWS AgentCore + Okta: vanilla v0.21.2 redirects to `<resource-host>/authorize`; patched build redirects to `https://extend.okta.com/oauth2/v1/authorize?...` with proper PKCE and `resource` parameter, completing OAuth + tools/list. Refs #675
1 parent f18775a commit 1017d14

4 files changed

Lines changed: 226 additions & 2 deletions

File tree

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import { describe, expect, it } from "@jest/globals";
2+
import { extractResourceMetadataUrlFromError } from "../extractResourceMetadataUrl";
3+
4+
const META_URL =
5+
"https://example.com/runtimes/abc/.well-known/oauth-protected-resource?qualifier=DEFAULT";
6+
7+
describe("extractResourceMetadataUrlFromError", () => {
8+
it("returns undefined for non-objects, plain Errors, empty objects", () => {
9+
expect(
10+
extractResourceMetadataUrlFromError(new Error("boom")),
11+
).toBeUndefined();
12+
expect(extractResourceMetadataUrlFromError(undefined)).toBeUndefined();
13+
expect(extractResourceMetadataUrlFromError(null)).toBeUndefined();
14+
expect(extractResourceMetadataUrlFromError({})).toBeUndefined();
15+
expect(extractResourceMetadataUrlFromError("string error")).toBeUndefined();
16+
});
17+
18+
it("extracts from .resourceMetadataUrl as URL", () => {
19+
expect(
20+
extractResourceMetadataUrlFromError({
21+
resourceMetadataUrl: new URL(META_URL),
22+
})?.toString(),
23+
).toBe(META_URL);
24+
});
25+
26+
it("extracts from .resourceMetadataUrl as string", () => {
27+
expect(
28+
extractResourceMetadataUrlFromError({
29+
resourceMetadataUrl: META_URL,
30+
})?.toString(),
31+
).toBe(META_URL);
32+
});
33+
34+
it("ignores .resourceMetadataUrl when not parseable, falls through", () => {
35+
expect(
36+
extractResourceMetadataUrlFromError({
37+
resourceMetadataUrl: "not a url",
38+
data: {
39+
upstream401: {
40+
wwwAuthenticate: `Bearer resource_metadata="${META_URL}"`,
41+
},
42+
},
43+
})?.toString(),
44+
).toBe(META_URL);
45+
});
46+
47+
it("extracts from .response.headers (quoted resource_metadata)", () => {
48+
const err = {
49+
response: {
50+
headers: new Headers({
51+
"www-authenticate": `Bearer realm="mcp", resource_metadata="${META_URL}"`,
52+
}),
53+
},
54+
};
55+
expect(extractResourceMetadataUrlFromError(err)?.toString()).toBe(META_URL);
56+
});
57+
58+
it("extracts from .headers (unquoted resource_metadata)", () => {
59+
const err = {
60+
headers: new Headers({
61+
"www-authenticate": `Bearer resource_metadata=${META_URL}`,
62+
}),
63+
};
64+
expect(extractResourceMetadataUrlFromError(err)?.toString()).toBe(META_URL);
65+
});
66+
67+
it("extracts from data.upstream401.wwwAuthenticate (proxy-mode)", () => {
68+
const err = {
69+
code: -32099,
70+
data: {
71+
httpStatus: 401,
72+
upstream401: {
73+
wwwAuthenticate: `Bearer realm="mcp", resource_metadata="${META_URL}"`,
74+
body: '{"error":"Missing Authentication Token"}',
75+
contentType: "application/json",
76+
},
77+
},
78+
};
79+
expect(extractResourceMetadataUrlFromError(err)?.toString()).toBe(META_URL);
80+
});
81+
82+
it("returns undefined for upstream401 without WWW-Authenticate", () => {
83+
expect(
84+
extractResourceMetadataUrlFromError({
85+
data: { upstream401: { body: "x" } },
86+
}),
87+
).toBeUndefined();
88+
});
89+
90+
it("returns undefined when resource_metadata is not a valid URL", () => {
91+
expect(
92+
extractResourceMetadataUrlFromError({
93+
data: {
94+
upstream401: {
95+
wwwAuthenticate: 'Bearer resource_metadata="bad url"',
96+
},
97+
},
98+
}),
99+
).toBeUndefined();
100+
});
101+
102+
it("priority: .resourceMetadataUrl beats both header and envelope", () => {
103+
const winner = "https://winner.example.com/.well-known/x";
104+
const err = {
105+
resourceMetadataUrl: winner,
106+
response: {
107+
headers: new Headers({
108+
"www-authenticate": `Bearer resource_metadata="${META_URL}"`,
109+
}),
110+
},
111+
data: {
112+
upstream401: {
113+
wwwAuthenticate: `Bearer resource_metadata="https://other"`,
114+
},
115+
},
116+
};
117+
expect(extractResourceMetadataUrlFromError(err)?.toString()).toBe(winner);
118+
});
119+
});

client/src/lib/connectionAuthErrors.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ export function mcpProxyTransportErrorDataIndicatesUnauthorized(
2020
return typeof status === "number" && status === 401;
2121
}
2222

23+
export { extractResourceMetadataUrlFromError } from "./extractResourceMetadataUrl";
24+
2325
/**
2426
* Whether `handleAuthError` / OAuth recovery should run for this failure.
2527
*/
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/**
2+
* RFC 9728 helper: extract `resource_metadata=...` URL from auth-related
3+
* errors, regardless of which transport / connection mode produced them.
4+
*
5+
* Why: Inspector's proxy mode unwraps upstream 401s into JSON-RPC errors with
6+
* `data.upstream401.wwwAuthenticate`. The SDK transport never sees a 401, so
7+
* its `onUnauthorized` hook can't fire. Without the verbatim metadata URL,
8+
* `auth()` falls back to path-aware probes that 404 against resource servers
9+
* like AWS AgentCore (which exposes RFC 9728 metadata at the "suffix" path)
10+
* and the user is redirected to a bogus `<resource-host>/authorize`.
11+
*
12+
* Sources, in priority order:
13+
* 1. `error.resourceMetadataUrl` — set by the SDK transport on
14+
* `UnauthorizedError` for direct 401s.
15+
* 2. `error.response.headers` or `error.headers` — `WWW-Authenticate` header
16+
* on Sse/StreamableHTTP errors (duck-typed so this module is independent
17+
* of the SDK's CJS auth bundle, which has Jest resolution issues).
18+
* 3. `error.data.upstream401.wwwAuthenticate` — the proxy-relayed envelope
19+
* used when `connectionType === "proxy"`.
20+
*
21+
* Returns `undefined` when no advertised URL is available; callers fall back
22+
* to default discovery.
23+
*/
24+
25+
function parseResourceMetadataFromWwwAuthenticate(
26+
wwwAuthenticate: unknown,
27+
): URL | undefined {
28+
if (typeof wwwAuthenticate !== "string") return undefined;
29+
// RFC 9728 / RFC 6750: quoted-string preferred; bare token tolerated.
30+
const match = wwwAuthenticate.match(
31+
/resource_metadata\s*=\s*(?:"([^"]+)"|([^\s,]+))/i,
32+
);
33+
const raw = match?.[1] ?? match?.[2];
34+
if (!raw) return undefined;
35+
try {
36+
return new URL(raw);
37+
} catch {
38+
return undefined;
39+
}
40+
}
41+
42+
function isPlainObject(value: unknown): value is Record<string, unknown> {
43+
return typeof value === "object" && value !== null && !Array.isArray(value);
44+
}
45+
46+
export function extractResourceMetadataUrlFromError(
47+
error: unknown,
48+
): URL | undefined {
49+
if (!error || typeof error !== "object") return undefined;
50+
const e = error as {
51+
resourceMetadataUrl?: unknown;
52+
response?: { headers?: { get?: (k: string) => string | null } };
53+
headers?: Headers | unknown;
54+
data?: unknown;
55+
};
56+
57+
// 1. SDK UnauthorizedError carries it directly
58+
const direct = e.resourceMetadataUrl;
59+
if (direct instanceof URL) return direct;
60+
if (typeof direct === "string") {
61+
try {
62+
return new URL(direct);
63+
} catch {
64+
/* fall through */
65+
}
66+
}
67+
68+
// 2. WWW-Authenticate on the error (Sse/StreamableHTTP/fetch-style)
69+
const wwwAuth =
70+
e.response?.headers?.get?.("www-authenticate") ??
71+
(e.headers instanceof Headers
72+
? e.headers.get("www-authenticate")
73+
: undefined);
74+
const fromHeader = parseResourceMetadataFromWwwAuthenticate(wwwAuth);
75+
if (fromHeader) return fromHeader;
76+
77+
// 3. Proxy-relayed `upstream401` envelope (Inspector's proxy mode)
78+
if (
79+
isPlainObject(e.data) &&
80+
"upstream401" in e.data &&
81+
isPlainObject(e.data.upstream401)
82+
) {
83+
return parseResourceMetadataFromWwwAuthenticate(
84+
e.data.upstream401.wwwAuthenticate,
85+
);
86+
}
87+
88+
return undefined;
89+
}

client/src/lib/hooks/useConnection.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ import { RequestOptions } from "@modelcontextprotocol/sdk/shared/protocol.js";
4949
import { useEffect, useRef, useState } from "react";
5050
import { useToast } from "@/lib/hooks/useToast";
5151
import { ConnectionStatus, CLIENT_IDENTITY } from "../constants";
52-
import { isConnectionAuthError } from "../connectionAuthErrors";
52+
import {
53+
extractResourceMetadataUrlFromError,
54+
isConnectionAuthError,
55+
} from "../connectionAuthErrors";
5356
import { Notification } from "../notificationTypes";
5457
import {
5558
auth,
@@ -392,13 +395,23 @@ export function useConnection({
392395
const fetchFn =
393396
connectionType === "proxy" ? createProxyFetch(config) : undefined;
394397

398+
// RFC 9728: the resource server advertises its protected-resource
399+
// metadata location via `WWW-Authenticate: Bearer resource_metadata=...`.
400+
// Use that URL verbatim instead of letting the SDK probe path-style
401+
// heuristics — those don't match resource servers (e.g. AWS AgentCore)
402+
// that expose metadata at the suffix path defined in RFC 9728 §3.
403+
// Without this, discovery 404s, the SDK falls back to treating the
404+
// resource server as the auth server, and the user is redirected to
405+
// a bogus `<resource-host>/authorize`. See issue #675.
406+
const resourceMetadataUrl = extractResourceMetadataUrlFromError(error);
407+
395408
if (!scope) {
396409
// Only discover resource metadata when we need to discover scopes
397410
let resourceMetadata;
398411
try {
399412
resourceMetadata = await discoverOAuthProtectedResourceMetadata(
400413
new URL("/", sseUrl),
401-
{},
414+
resourceMetadataUrl ? { resourceMetadataUrl } : {},
402415
fetchFn,
403416
);
404417
} catch {
@@ -414,6 +427,7 @@ export function useConnection({
414427
const result = await auth(serverAuthProvider, {
415428
serverUrl: sseUrl,
416429
scope,
430+
...(resourceMetadataUrl ? { resourceMetadataUrl } : {}),
417431
...(fetchFn && { fetchFn }),
418432
});
419433
return result === "AUTHORIZED";

0 commit comments

Comments
 (0)