Merge pull request #2113 from modelcontextprotocol/v2/fix/2071-oauth-resource-metadata
fix(auth): pass the challenge's resource_metadata URL to SDK auth()
This commit is contained in:
commit
71ce0877dd
19 changed files with 927 additions and 13 deletions
14
AGENTS.md
14
AGENTS.md
|
|
@ -61,7 +61,19 @@ v2/main/
|
||||||
├── core/ # Shared core code (no package.json — consumed via the `@inspector/core` vite alias)
|
├── core/ # Shared core code (no package.json — consumed via the `@inspector/core` vite alias)
|
||||||
│ ├── auth/ # OAuth: providers, discovery, OAuthStorage + persist backends;
|
│ ├── auth/ # OAuth: providers, discovery, OAuthStorage + persist backends;
|
||||||
│ │ # mid-session recovery (challenge.ts WWW-Authenticate
|
│ │ # mid-session recovery (challenge.ts WWW-Authenticate
|
||||||
│ │ # parsing, scopes.ts SEP-2350 scope union, oauthUx.ts
|
│ │ # parsing — including the RFC 9728
|
||||||
|
│ │ # `resource_metadata` URL, carried on
|
||||||
|
│ │ # AuthChallenge as a STRING because the
|
||||||
|
│ │ # web client's challenge crosses the
|
||||||
|
│ │ # remote-backend boundary as JSON, and
|
||||||
|
│ │ # converted to a URL at the OAuth
|
||||||
|
│ │ # boundary by challengeResourceMetadataUrl
|
||||||
|
│ │ # (malformed values ignored, matching the
|
||||||
|
│ │ # SDK's own parser) so discovery targets
|
||||||
|
│ │ # the advertised document rather than a
|
||||||
|
│ │ # location derived from the MCP server
|
||||||
|
│ │ # URL — #2071;
|
||||||
|
│ │ # scopes.ts SEP-2350 scope union, oauthUx.ts
|
||||||
│ │ # shared copy, mcpAuth.ts force-reauthorization,
|
│ │ # shared copy, mcpAuth.ts force-reauthorization,
|
||||||
│ │ # issuerBinding.ts SEP-2352 callback-leg failure
|
│ │ # issuerBinding.ts SEP-2352 callback-leg failure
|
||||||
│ │ # classification — separates a recoverable
|
│ │ # classification — separates a recoverable
|
||||||
|
|
|
||||||
18
README.md
18
README.md
|
|
@ -154,6 +154,7 @@ Each config below is a ready-made server for exercising one feature by hand. Loa
|
||||||
| `nullable-fields-http.json` | Tools tab: nullable (`anyOf` + `null`) arguments | [#1928](https://github.com/modelcontextprotocol/inspector/issues/1928) |
|
| `nullable-fields-http.json` | Tools tab: nullable (`anyOf` + `null`) arguments | [#1928](https://github.com/modelcontextprotocol/inspector/issues/1928) |
|
||||||
| `rfc6570-templates-http.json` | Resources tab: RFC 6570 resource-template expansion | [#1919](https://github.com/modelcontextprotocol/inspector/issues/1919) |
|
| `rfc6570-templates-http.json` | Resources tab: RFC 6570 resource-template expansion | [#1919](https://github.com/modelcontextprotocol/inspector/issues/1919) |
|
||||||
| `advertised-extensions-http.json` | Tool registration gated on advertised extensions | [#1739](https://github.com/modelcontextprotocol/inspector/issues/1739) |
|
| `advertised-extensions-http.json` | Tool registration gated on advertised extensions | [#1739](https://github.com/modelcontextprotocol/inspector/issues/1739) |
|
||||||
|
| `oauth-custom-resource-metadata-http.json` **(legacy era)** | OAuth discovery driven by the challenge's `resource_metadata` | [#2071](https://github.com/modelcontextprotocol/inspector/issues/2071) |
|
||||||
| `logging-{legacy,modern}-http.json` | Logging, both eras | [#1629](https://github.com/modelcontextprotocol/inspector/issues/1629) |
|
| `logging-{legacy,modern}-http.json` | Logging, both eras | [#1629](https://github.com/modelcontextprotocol/inspector/issues/1629) |
|
||||||
| `subscriptions-{legacy,modern}-http.json` | Resource subscriptions, both eras | [#1630](https://github.com/modelcontextprotocol/inspector/issues/1630) |
|
| `subscriptions-{legacy,modern}-http.json` | Resource subscriptions, both eras | [#1630](https://github.com/modelcontextprotocol/inspector/issues/1630) |
|
||||||
| `tasks-{legacy,modern}-http.json` | Tasks, both eras | [#1631](https://github.com/modelcontextprotocol/inspector/issues/1631) |
|
| `tasks-{legacy,modern}-http.json` | Tasks, both eras | [#1631](https://github.com/modelcontextprotocol/inspector/issues/1631) |
|
||||||
|
|
@ -327,6 +328,23 @@ Requiredness is a property of the **expression**, not the variable: RFC 6570 dro
|
||||||
|
|
||||||
This is the debugging knob for a server legitimately changing tool registration based on what the client advertises. Legacy stateful leg only — the modern per-request leg has no persistent `oninitialized`.
|
This is the debugging knob for a server legitimately changing tool registration based on what the client advertises. Legacy stateful leg only — the modern per-request leg has no persistent `oninitialized`.
|
||||||
|
|
||||||
|
#### OAuth `resource_metadata` at a non-default path
|
||||||
|
|
||||||
|
`oauth-custom-resource-metadata-http.json` is an OAuth-protected server (combined AS + resource, DCR enabled) that serves its RFC 9728 protected-resource metadata document **only** from `/custom/protected-resource`, and advertises it on every 401:
|
||||||
|
|
||||||
|
```http
|
||||||
|
HTTP/1.1 401 Unauthorized
|
||||||
|
WWW-Authenticate: Bearer resource_metadata="http://127.0.0.1:8082/custom/protected-resource"
|
||||||
|
```
|
||||||
|
|
||||||
|
The default `/.well-known/oauth-protected-resource` route is deliberately left unserved, so a client that ignores the advertised URL cannot discover the document at all. Plain streamable-HTTP — connect with the **default (legacy)** protocol era.
|
||||||
|
|
||||||
|
Add the server, click **Connect**, and watch the Inspector's first protected-resource metadata request in the Network tab: it must go to `/custom/protected-resource`. On the broken build the challenge's `resource_metadata` was parsed and then dropped before the SDK's `auth()` ever saw it ([#2071](https://github.com/modelcontextprotocol/inspector/issues/2071)), so discovery probed locations derived from the MCP server URL, 404'd, and authorization failed for any server that puts the document somewhere other than the well-known path.
|
||||||
|
|
||||||
|
The same server is worth running against `--cli` / `--tui`, which reach it by a different route: with no stored token in the legacy era the Inspector connects with no auth provider (so the SDK cannot open a browser before the callback server is listening), the 401 surfaces as the SDK's headerless `UnauthorizedError`, and the client calls `authenticate()` with no challenge in hand. The transport therefore *observes* every 401/403 passively, so the advertised URL is still available on that path.
|
||||||
|
|
||||||
|
The value now rides the normalized `AuthChallenge` as a string — it has to be serializable, because the web client's challenge crosses the remote-backend boundary as JSON — and is converted to a `URL` at the OAuth boundary, where it is handed to `auth()` as `resourceMetadataUrl` and to the CIMD pre-registration probe, which runs *before* `auth()` and would otherwise do its own default-location discovery. A malformed value is ignored rather than surfaced, matching the SDK's own `WWW-Authenticate` parser: discovery falls back to the default locations instead of failing the whole authorization on a bad header. The callback leg needs nothing extra — SDK `auth()` persists the URL in its discovery state, so it survives both the web full-page redirect and the CLI/TUI loopback callback.
|
||||||
|
|
||||||
#### Logging, both eras
|
#### Logging, both eras
|
||||||
|
|
||||||
`logging-legacy-http.json` and `logging-modern-http.json` both serve `logging: true` plus a `send_notification` tool that emits a `notifications/message` at a chosen level. The legacy one is a plain streamable-HTTP server; the modern one sets `transport.modern: true`.
|
`logging-legacy-http.json` and `logging-modern-http.json` both serve `logging: true` plus a `send_notification` tool that emits a `notifications/message` at a chosen level. The legacy one is a plain streamable-HTTP server; the modern one sets `transport.modern: true`.
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
import { describe, it, expect } from "vitest";
|
import { describe, it, expect } from "vitest";
|
||||||
import {
|
import {
|
||||||
AuthChallengeError,
|
AuthChallengeError,
|
||||||
|
challengeResourceMetadataUrl,
|
||||||
AuthRecoveryRequiredError,
|
AuthRecoveryRequiredError,
|
||||||
findNestedAuthError,
|
findNestedAuthError,
|
||||||
isAuthChallengeError,
|
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", () => {
|
describe("isAuthChallengeError", () => {
|
||||||
it("detects AuthChallengeError instances", () => {
|
it("detects AuthChallengeError instances", () => {
|
||||||
const err = new AuthChallengeError({ reason: "token_expired" }, 401);
|
const err = new AuthChallengeError({ reason: "token_expired" }, 401);
|
||||||
|
|
|
||||||
|
|
@ -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 type { OAuthStorage } from "@inspector/core/auth/storage.js";
|
||||||
import { BaseOAuthClientProvider } from "@inspector/core/auth/providers.js";
|
import { BaseOAuthClientProvider } from "@inspector/core/auth/providers.js";
|
||||||
import { ensureCimdClientRegistration } from "@inspector/core/auth/cimd.js";
|
import { ensureCimdClientRegistration } from "@inspector/core/auth/cimd.js";
|
||||||
|
|
@ -31,6 +31,10 @@ describe("ensureCimdClientRegistration", () => {
|
||||||
} as unknown as OAuthStorage;
|
} as unknown as OAuthStorage;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.unstubAllGlobals();
|
||||||
|
});
|
||||||
|
|
||||||
it("pre-registers URL-based client id when AS supports CIMD", async () => {
|
it("pre-registers URL-based client id when AS supports CIMD", async () => {
|
||||||
const fetchFn = vi.fn(async (input: RequestInfo | URL) => {
|
const fetchFn = vi.fn(async (input: RequestInfo | URL) => {
|
||||||
const url = String(input);
|
const url = String(input);
|
||||||
|
|
@ -97,6 +101,63 @@ describe("ensureCimdClientRegistration", () => {
|
||||||
expect(storage.saveClientInformation).not.toHaveBeenCalled();
|
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",
|
||||||
|
);
|
||||||
|
// Both discovery legs must run through the configured fetch — on web that
|
||||||
|
// is the backend proxy, so a leg that fell back to the global would fail
|
||||||
|
// on CORS and be swallowed.
|
||||||
|
const fetchFn = vi.fn(async (input: RequestInfo | URL) => {
|
||||||
|
const url = String(input);
|
||||||
|
if (url === metadataUrl.href) {
|
||||||
|
return new Response(
|
||||||
|
JSON.stringify({
|
||||||
|
resource: SERVER_URL,
|
||||||
|
authorization_servers: ["http://127.0.0.1:9999"],
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
if (url.includes("/.well-known/oauth-authorization-server")) {
|
||||||
|
return 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,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return new Response(null, { status: 404 });
|
||||||
|
});
|
||||||
|
// Nothing may reach the network directly; a call here means a leg fell
|
||||||
|
// back to the global fetch.
|
||||||
|
const globalFetch = vi.fn();
|
||||||
|
vi.stubGlobal("fetch", globalFetch);
|
||||||
|
|
||||||
|
await ensureCimdClientRegistration({
|
||||||
|
serverUrl: SERVER_URL,
|
||||||
|
provider: createProvider(storage),
|
||||||
|
fetchFn,
|
||||||
|
resourceMetadataUrl: metadataUrl,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(globalFetch).not.toHaveBeenCalled();
|
||||||
|
const requested = fetchFn.mock.calls.map(([input]) => String(input));
|
||||||
|
expect(requested).toContain(metadataUrl.href);
|
||||||
|
expect(
|
||||||
|
requested.filter((url) =>
|
||||||
|
url.includes("/.well-known/oauth-protected-resource"),
|
||||||
|
),
|
||||||
|
).toEqual([]);
|
||||||
|
expect(storage.saveClientInformation).toHaveBeenCalledWith(
|
||||||
|
SERVER_URL,
|
||||||
|
{ client_id: METADATA_URL },
|
||||||
|
{ registrationKind: "cimd" },
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
it("no-ops when client information is already stored", async () => {
|
it("no-ops when client information is already stored", async () => {
|
||||||
storage.getClientInformation = vi.fn(async () => ({
|
storage.getClientInformation = vi.fn(async () => ({
|
||||||
client_id: "existing-client",
|
client_id: "existing-client",
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,8 @@
|
||||||
import { describe, it, expect, vi } from "vitest";
|
import { describe, it, expect, vi } from "vitest";
|
||||||
import { createAuthChallengeInterceptFetch } from "@inspector/core/mcp/node/authChallengeFetch.js";
|
import {
|
||||||
|
createAuthChallengeInterceptFetch,
|
||||||
|
createAuthChallengeObserverFetch,
|
||||||
|
} from "@inspector/core/mcp/node/authChallengeFetch.js";
|
||||||
import { AuthChallengeError } from "@inspector/core/auth/challenge.js";
|
import { AuthChallengeError } from "@inspector/core/auth/challenge.js";
|
||||||
|
|
||||||
describe("createAuthChallengeInterceptFetch", () => {
|
describe("createAuthChallengeInterceptFetch", () => {
|
||||||
|
|
@ -84,3 +87,63 @@ describe("createAuthChallengeInterceptFetch", () => {
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("createAuthChallengeObserverFetch", () => {
|
||||||
|
const METADATA_URL = "http://127.0.0.1:3001/custom/protected-resource";
|
||||||
|
|
||||||
|
it("reports a challenge without altering the response or its body", async () => {
|
||||||
|
const baseFetch = vi.fn(
|
||||||
|
async () =>
|
||||||
|
new Response("payload", {
|
||||||
|
status: 401,
|
||||||
|
headers: {
|
||||||
|
"WWW-Authenticate": `Bearer resource_metadata="${METADATA_URL}"`,
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
const onChallenge = vi.fn();
|
||||||
|
const fetchFn = createAuthChallengeObserverFetch(baseFetch, onChallenge);
|
||||||
|
|
||||||
|
const res = await fetchFn("https://example.com/mcp");
|
||||||
|
|
||||||
|
expect(res.status).toBe(401);
|
||||||
|
// The body must survive for the transport (or the interceptor above).
|
||||||
|
expect(await res.text()).toBe("payload");
|
||||||
|
expect(onChallenge).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({ resourceMetadataUrl: METADATA_URL }),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reports a 403 challenge too", async () => {
|
||||||
|
const baseFetch = vi.fn(
|
||||||
|
async () =>
|
||||||
|
new Response(null, {
|
||||||
|
status: 403,
|
||||||
|
headers: { "WWW-Authenticate": 'Bearer error="insufficient_scope"' },
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
const onChallenge = vi.fn();
|
||||||
|
|
||||||
|
await createAuthChallengeObserverFetch(
|
||||||
|
baseFetch,
|
||||||
|
onChallenge,
|
||||||
|
)("https://example.com/mcp");
|
||||||
|
|
||||||
|
expect(onChallenge).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({ reason: "insufficient_scope" }),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("stays silent on a non-challenge response", async () => {
|
||||||
|
const baseFetch = vi.fn(async () => new Response("ok", { status: 200 }));
|
||||||
|
const onChallenge = vi.fn();
|
||||||
|
|
||||||
|
const res = await createAuthChallengeObserverFetch(
|
||||||
|
baseFetch,
|
||||||
|
onChallenge,
|
||||||
|
)("https://example.com/mcp");
|
||||||
|
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(onChallenge).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
|
||||||
|
|
@ -1768,6 +1768,153 @@ 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("uses the last observed challenge on the plain authenticate() path", async () => {
|
||||||
|
// The legacy first-authorization path: no challenge is passed in, so the
|
||||||
|
// one the transport observed is the only source (#2071).
|
||||||
|
mockedMcpAuth.mockResolvedValue("REDIRECT");
|
||||||
|
const params = createMockParams();
|
||||||
|
const manager = new OAuthManager(params);
|
||||||
|
const captureSpy = vi
|
||||||
|
.spyOn(
|
||||||
|
(await import("@inspector/core/auth/providers.js"))
|
||||||
|
.BaseOAuthClientProvider.prototype,
|
||||||
|
"getCapturedAuthUrl",
|
||||||
|
)
|
||||||
|
.mockReturnValue(new URL("https://auth.example.com/authorize?state=x"));
|
||||||
|
|
||||||
|
manager.noteObservedAuthChallenge({
|
||||||
|
reason: "unauthorized",
|
||||||
|
resourceMetadataUrl: METADATA_URL,
|
||||||
|
});
|
||||||
|
await manager.authenticate();
|
||||||
|
|
||||||
|
expect(mockedMcpAuth).toHaveBeenCalledWith(
|
||||||
|
expect.anything(),
|
||||||
|
expect.objectContaining({
|
||||||
|
resourceMetadataUrl: new URL(METADATA_URL),
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
captureSpy.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clears the observed URL when a later challenge advertises none", async () => {
|
||||||
|
mockedMcpAuth.mockResolvedValue("REDIRECT");
|
||||||
|
const params = createMockParams();
|
||||||
|
const manager = new OAuthManager(params);
|
||||||
|
const captureSpy = vi
|
||||||
|
.spyOn(
|
||||||
|
(await import("@inspector/core/auth/providers.js"))
|
||||||
|
.BaseOAuthClientProvider.prototype,
|
||||||
|
"getCapturedAuthUrl",
|
||||||
|
)
|
||||||
|
.mockReturnValue(new URL("https://auth.example.com/authorize?state=x"));
|
||||||
|
|
||||||
|
manager.noteObservedAuthChallenge({
|
||||||
|
reason: "unauthorized",
|
||||||
|
resourceMetadataUrl: METADATA_URL,
|
||||||
|
});
|
||||||
|
manager.noteObservedAuthChallenge({ reason: "unauthorized" });
|
||||||
|
await manager.authenticate();
|
||||||
|
|
||||||
|
expect(mockedMcpAuth).toHaveBeenCalledWith(
|
||||||
|
expect.anything(),
|
||||||
|
expect.objectContaining({ resourceMetadataUrl: undefined }),
|
||||||
|
);
|
||||||
|
captureSpy.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
|
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)", () => {
|
describe("handleAuthChallenge (additional branch coverage)", () => {
|
||||||
it("resolves via the second satisfaction check inside the mutex", async () => {
|
it("resolves via the second satisfaction check inside the mutex", async () => {
|
||||||
const params = createMockParams();
|
const params = createMockParams();
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,266 @@
|
||||||
|
/**
|
||||||
|
* 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, vi } from "vitest";
|
||||||
|
import { createServer } from "node:http";
|
||||||
|
import { fileURLToPath } from "node:url";
|
||||||
|
import { mkdtempSync, rmSync } from "node:fs";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
import { join } from "node:path";
|
||||||
|
import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/client";
|
||||||
|
import {
|
||||||
|
TestServerHttp,
|
||||||
|
getDefaultServerConfig,
|
||||||
|
createOAuthTestServerConfig,
|
||||||
|
isOriginRelativePath,
|
||||||
|
loadConfig,
|
||||||
|
resolveConfig,
|
||||||
|
waitForOAuthWellKnown,
|
||||||
|
} from "@modelcontextprotocol/inspector-test-server";
|
||||||
|
import {
|
||||||
|
challengeResourceMetadataUrl,
|
||||||
|
parseAuthChallengeFromResponse,
|
||||||
|
} from "@inspector/core/auth/challenge.js";
|
||||||
|
import type { AuthChallenge } from "@inspector/core/auth/challenge.js";
|
||||||
|
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||||
|
import { createTransportNode } from "@inspector/core/mcp/node/transport.js";
|
||||||
|
import {
|
||||||
|
ConsoleNavigation,
|
||||||
|
type RedirectUrlProvider,
|
||||||
|
} from "@inspector/core/auth/providers.js";
|
||||||
|
import { NodeOAuthStorage } from "@inspector/core/auth/node/storage-node.js";
|
||||||
|
|
||||||
|
const METADATA_PATH = "/custom/protected-resource";
|
||||||
|
const STATIC_CLIENT_ID = "test-2071-resource-metadata";
|
||||||
|
const STATIC_CLIENT_SECRET = "test-2071-secret";
|
||||||
|
const REDIRECT_URL = "http://localhost:3000/oauth/callback";
|
||||||
|
/** Fixed, since this one is asserted against a server of its own. */
|
||||||
|
const METADATA_URL_LITERAL = "http://127.0.0.1:3001/custom/protected-resource";
|
||||||
|
|
||||||
|
describe("OAuth challenge resource_metadata (RFC 9728)", () => {
|
||||||
|
let mcpServer: TestServerHttp | null = null;
|
||||||
|
let serverUrl = "";
|
||||||
|
let storageDir = "";
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
mcpServer = new TestServerHttp({
|
||||||
|
...getDefaultServerConfig(),
|
||||||
|
serverType: "streamable-http" as const,
|
||||||
|
...createOAuthTestServerConfig({
|
||||||
|
requireAuth: true,
|
||||||
|
resourceMetadataPath: METADATA_PATH,
|
||||||
|
// Static rather than DCR: this suite is about *where the metadata
|
||||||
|
// document is found*, so registration should not be a second thing
|
||||||
|
// that can fail.
|
||||||
|
staticClients: [
|
||||||
|
{
|
||||||
|
clientId: STATIC_CLIENT_ID,
|
||||||
|
clientSecret: STATIC_CLIENT_SECRET,
|
||||||
|
redirectUris: [REDIRECT_URL],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
const port = await mcpServer.start();
|
||||||
|
serverUrl = `http://localhost:${port}`;
|
||||||
|
storageDir = mkdtempSync(join(tmpdir(), "mcp-inspector-2071-"));
|
||||||
|
await waitForOAuthWellKnown(serverUrl);
|
||||||
|
}, 30_000);
|
||||||
|
|
||||||
|
afterAll(async () => {
|
||||||
|
await mcpServer?.stop();
|
||||||
|
mcpServer = null;
|
||||||
|
if (storageDir) {
|
||||||
|
rmSync(storageDir, { recursive: true, force: true });
|
||||||
|
storageDir = "";
|
||||||
|
}
|
||||||
|
}, 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("drives first-time legacy authorization off the advertised URL (#2071)", async () => {
|
||||||
|
// The gap the challenge-driven plumbing alone does not close: with no
|
||||||
|
// stored token in the default legacy era, `InspectorClient` builds the
|
||||||
|
// transport with no `authProvider` and no challenge interception, so the
|
||||||
|
// 401 reaches the client as the SDK's headerless `UnauthorizedError` and
|
||||||
|
// the CLI/TUI runner calls `authenticate()` with nothing in hand. The
|
||||||
|
// observed-challenge path is what carries the URL across that gap.
|
||||||
|
const requested: string[] = [];
|
||||||
|
const spyFetch: typeof fetch = async (input, init) => {
|
||||||
|
requested.push(input instanceof Request ? input.url : String(input));
|
||||||
|
return fetch(input, init);
|
||||||
|
};
|
||||||
|
|
||||||
|
const redirectUrlProvider: RedirectUrlProvider = {
|
||||||
|
getRedirectUrl: () => REDIRECT_URL,
|
||||||
|
};
|
||||||
|
|
||||||
|
const client = new InspectorClient(
|
||||||
|
{ type: "streamable-http", url: `${serverUrl}/mcp` },
|
||||||
|
{
|
||||||
|
environment: {
|
||||||
|
transport: createTransportNode,
|
||||||
|
fetch: spyFetch,
|
||||||
|
oauth: {
|
||||||
|
// A fresh path per run. SDK `auth()` persists its discovery
|
||||||
|
// state — the advertised metadata URL included — so a reused file
|
||||||
|
// would let a watch-mode rerun satisfy discovery from the cache
|
||||||
|
// and pass even with the observer plumbing removed (Copilot).
|
||||||
|
storage: new NodeOAuthStorage(join(storageDir, "oauth.json")),
|
||||||
|
navigation: new ConsoleNavigation(),
|
||||||
|
redirectUrlProvider,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
oauth: {
|
||||||
|
clientId: STATIC_CLIENT_ID,
|
||||||
|
clientSecret: STATIC_CLIENT_SECRET,
|
||||||
|
scope: "mcp",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect(client.connect()).rejects.toThrow();
|
||||||
|
|
||||||
|
// Reaching an authorization URL at all means discovery resolved — which it
|
||||||
|
// can only have done through the advertised path, since the well-known one
|
||||||
|
// 404s on this server.
|
||||||
|
const authUrl = await client.authenticate();
|
||||||
|
expect(authUrl?.href).toContain("/oauth/authorize");
|
||||||
|
|
||||||
|
expect(requested).toContain(`${serverUrl}${METADATA_PATH}`);
|
||||||
|
expect(
|
||||||
|
requested.filter((url) =>
|
||||||
|
url.includes("/.well-known/oauth-protected-resource"),
|
||||||
|
),
|
||||||
|
).toEqual([]);
|
||||||
|
|
||||||
|
await client.disconnect();
|
||||||
|
}, 30_000);
|
||||||
|
|
||||||
|
it("observes the SSE challenge through a caller-supplied eventSourceInit.fetch", async () => {
|
||||||
|
// That explicit fetch is deliberately not proxy- or intercept-wrapped, so
|
||||||
|
// it used to bypass the observer entirely and a first-time legacy SSE
|
||||||
|
// authorization lost the advertised URL the same way (Copilot).
|
||||||
|
const sseServer = createServer((_req, res) => {
|
||||||
|
res.writeHead(401, {
|
||||||
|
"WWW-Authenticate": `Bearer resource_metadata="${METADATA_URL_LITERAL}"`,
|
||||||
|
});
|
||||||
|
res.end();
|
||||||
|
});
|
||||||
|
await new Promise<void>((resolve) => sseServer.listen(0, resolve));
|
||||||
|
const address = sseServer.address();
|
||||||
|
/* v8 ignore next 3 -- listen(0) on a TCP socket always yields an AddressInfo */
|
||||||
|
if (address === null || typeof address === "string") {
|
||||||
|
throw new Error("expected a TCP address");
|
||||||
|
}
|
||||||
|
|
||||||
|
const explicitFetch = vi.fn<typeof fetch>((input, init) =>
|
||||||
|
fetch(input, init),
|
||||||
|
);
|
||||||
|
const observed: AuthChallenge[] = [];
|
||||||
|
|
||||||
|
try {
|
||||||
|
const { transport } = createTransportNode(
|
||||||
|
{
|
||||||
|
type: "sse",
|
||||||
|
url: `http://127.0.0.1:${address.port}/sse`,
|
||||||
|
eventSourceInit: { fetch: explicitFetch },
|
||||||
|
},
|
||||||
|
{
|
||||||
|
onAuthChallengeObserved: (challenge) => observed.push(challenge),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect(transport.start()).rejects.toThrow();
|
||||||
|
await transport.close().catch(() => {});
|
||||||
|
} finally {
|
||||||
|
await new Promise<void>((resolve) => sseServer.close(() => resolve()));
|
||||||
|
}
|
||||||
|
|
||||||
|
expect(explicitFetch).toHaveBeenCalled();
|
||||||
|
expect(observed).toContainEqual(
|
||||||
|
expect.objectContaining({ resourceMetadataUrl: METADATA_URL_LITERAL }),
|
||||||
|
);
|
||||||
|
}, 30_000);
|
||||||
|
|
||||||
|
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,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
["/custom/protected-resource", true],
|
||||||
|
["/a", true],
|
||||||
|
// Both re-point the origin when resolved against the request base while
|
||||||
|
// Express still registers the route locally, so the server would advertise
|
||||||
|
// a document it does not serve (Copilot).
|
||||||
|
["//other-host/doc", false],
|
||||||
|
["/\\other-host/doc", false],
|
||||||
|
["https://other-host/doc", false],
|
||||||
|
["custom/protected-resource", false],
|
||||||
|
["/has a space", false],
|
||||||
|
// `href` preserves both, so these pass an origin+href comparison — but
|
||||||
|
// Express matches on the path alone and a fragment never reaches the
|
||||||
|
// server, so the advertised URL could not resolve to the route.
|
||||||
|
["/doc?version=1", false],
|
||||||
|
["/doc#section", false],
|
||||||
|
["", false],
|
||||||
|
])("origin-relative path check: %j -> %s", (value, expected) => {
|
||||||
|
expect(isOriginRelativePath(value)).toBe(expected);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -26,6 +26,17 @@ export interface AuthChallenge {
|
||||||
/** Resource authorization server audience when known. */
|
/** Resource authorization server audience when known. */
|
||||||
audience?: string;
|
audience?: string;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* RFC 9728 `resource_metadata` advertised by the `WWW-Authenticate` challenge.
|
||||||
|
*
|
||||||
|
* Kept as a string so the challenge stays serializable — it crosses the web
|
||||||
|
* client's remote-backend boundary as JSON. Converted to a `URL` (and
|
||||||
|
* discarded if malformed) at the OAuth boundary, where it is handed to the
|
||||||
|
* SDK as `resourceMetadataUrl` so protected-resource discovery targets the
|
||||||
|
* advertised document instead of a location derived from the MCP server URL.
|
||||||
|
*/
|
||||||
|
resourceMetadataUrl?: string;
|
||||||
|
|
||||||
/** Optional human-readable detail from server or SDK (for UI, not parsing). */
|
/** Optional human-readable detail from server or SDK (for UI, not parsing). */
|
||||||
message?: string;
|
message?: string;
|
||||||
|
|
||||||
|
|
@ -193,6 +204,30 @@ export function parseWwwAuthenticateBearer(
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Convert a challenge's advertised RFC 9728 `resource_metadata` value into the
|
||||||
|
* `URL` the SDK's `auth()` takes as `resourceMetadataUrl`.
|
||||||
|
*
|
||||||
|
* A malformed value is ignored rather than surfaced, matching the SDK's own
|
||||||
|
* `WWW-Authenticate` parser (`extractWWWAuthenticateParams`), which drops a
|
||||||
|
* value `new URL()` rejects. Discovery then falls back to the default
|
||||||
|
* RFC 9728 locations derived from the MCP server URL — the pre-existing
|
||||||
|
* behavior — instead of failing the whole authorization on a bad header.
|
||||||
|
*/
|
||||||
|
export function challengeResourceMetadataUrl(
|
||||||
|
challenge: Pick<AuthChallenge, "resourceMetadataUrl">,
|
||||||
|
): URL | undefined {
|
||||||
|
const value = challenge.resourceMetadataUrl?.trim();
|
||||||
|
if (!value) {
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
return new URL(value);
|
||||||
|
} catch {
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/** Split an OAuth scope string into individual scopes (space-separated). */
|
/** Split an OAuth scope string into individual scopes (space-separated). */
|
||||||
export function parseScopeString(scope: string | undefined): string[] {
|
export function parseScopeString(scope: string | undefined): string[] {
|
||||||
if (!scope?.trim()) {
|
if (!scope?.trim()) {
|
||||||
|
|
@ -275,6 +310,9 @@ export function parseAuthChallengeFromResponse(
|
||||||
return {
|
return {
|
||||||
reason: reasonFromHttpResponse(status, bearer),
|
reason: reasonFromHttpResponse(status, bearer),
|
||||||
...(requiredScopes.length > 0 ? { requiredScopes } : {}),
|
...(requiredScopes.length > 0 ? { requiredScopes } : {}),
|
||||||
|
...(bearer.resourceMetadata
|
||||||
|
? { resourceMetadataUrl: bearer.resourceMetadata }
|
||||||
|
: {}),
|
||||||
...(bearer.errorDescription ? { message: bearer.errorDescription } : {}),
|
...(bearer.errorDescription ? { message: bearer.errorDescription } : {}),
|
||||||
...(context ? { context } : {}),
|
...(context ? { context } : {}),
|
||||||
raw: {
|
raw: {
|
||||||
|
|
@ -337,6 +375,9 @@ export function parseAuthChallengeFromError(
|
||||||
return {
|
return {
|
||||||
reason: reasonFromHttpResponse(status, bearer),
|
reason: reasonFromHttpResponse(status, bearer),
|
||||||
...(requiredScopes.length > 0 ? { requiredScopes } : {}),
|
...(requiredScopes.length > 0 ? { requiredScopes } : {}),
|
||||||
|
...(bearer.resourceMetadata
|
||||||
|
? { resourceMetadataUrl: bearer.resourceMetadata }
|
||||||
|
: {}),
|
||||||
...(context ? { context } : {}),
|
...(context ? { context } : {}),
|
||||||
raw: {
|
raw: {
|
||||||
httpStatus: status,
|
httpStatus: status,
|
||||||
|
|
|
||||||
|
|
@ -14,11 +14,18 @@ import type { BaseOAuthClientProvider } from "./providers.js";
|
||||||
* accepts an already-stored `client_id` (including `http://` URLs used by local
|
* accepts an already-stored `client_id` (including `http://` URLs used by local
|
||||||
* dev/test metadata servers). Production CIMD metadata documents should still
|
* dev/test metadata servers). Production CIMD metadata documents should still
|
||||||
* use HTTPS per SEP-991.
|
* use HTTPS per SEP-991.
|
||||||
|
*
|
||||||
|
* `resourceMetadataUrl` is the RFC 9728 document advertised by the
|
||||||
|
* `WWW-Authenticate` challenge, when the caller has one. It matters here and
|
||||||
|
* not only in `auth()`: this runs *before* SDK `auth()`, so without it the
|
||||||
|
* pre-registration probe would do its own default-location discovery and miss
|
||||||
|
* a document served from a non-default path (#2071).
|
||||||
*/
|
*/
|
||||||
export async function ensureCimdClientRegistration(params: {
|
export async function ensureCimdClientRegistration(params: {
|
||||||
serverUrl: string;
|
serverUrl: string;
|
||||||
provider: BaseOAuthClientProvider;
|
provider: BaseOAuthClientProvider;
|
||||||
fetchFn?: typeof fetch;
|
fetchFn?: typeof fetch;
|
||||||
|
resourceMetadataUrl?: URL;
|
||||||
}): Promise<void> {
|
}): Promise<void> {
|
||||||
const clientMetadataUrl = params.provider.clientMetadataUrl?.trim();
|
const clientMetadataUrl = params.provider.clientMetadataUrl?.trim();
|
||||||
if (!clientMetadataUrl) return;
|
if (!clientMetadataUrl) return;
|
||||||
|
|
@ -30,6 +37,13 @@ export async function ensureCimdClientRegistration(params: {
|
||||||
try {
|
try {
|
||||||
resourceMetadata = await discoverOAuthProtectedResourceMetadata(
|
resourceMetadata = await discoverOAuthProtectedResourceMetadata(
|
||||||
params.serverUrl,
|
params.serverUrl,
|
||||||
|
{ resourceMetadataUrl: params.resourceMetadataUrl },
|
||||||
|
// The same fetch the AS-metadata leg below uses. On web that is
|
||||||
|
// `createRemoteFetch`, which proxies through the backend to sidestep
|
||||||
|
// CORS — on the global `fetch` this leg would fail in the browser, be
|
||||||
|
// swallowed by the catch, and leave CIMD probing the wrong
|
||||||
|
// authorization server (Copilot).
|
||||||
|
params.fetchFn,
|
||||||
);
|
);
|
||||||
} catch {
|
} catch {
|
||||||
resourceMetadata = undefined;
|
resourceMetadata = undefined;
|
||||||
|
|
|
||||||
|
|
@ -2025,6 +2025,16 @@ export class InspectorClient extends InspectorClientEventTarget {
|
||||||
...(this.serverSettings && { settings: this.serverSettings }),
|
...(this.serverSettings && { settings: this.serverSettings }),
|
||||||
};
|
};
|
||||||
if (this.isHttpOAuthConfig() && oauthManager) {
|
if (this.isHttpOAuthConfig() && oauthManager) {
|
||||||
|
// Record every 401/403 the transport sees, whatever else happens to
|
||||||
|
// it. The legacy first-authorization path deliberately runs with no
|
||||||
|
// authProvider and no challenge interception (see below), so the SDK
|
||||||
|
// raises a headerless `UnauthorizedError` and the client calls
|
||||||
|
// `authenticate()` with nothing in hand — this is the only place the
|
||||||
|
// challenge's RFC 9728 `resource_metadata` still exists (#2071).
|
||||||
|
const manager = oauthManager;
|
||||||
|
transportOptions.onAuthChallengeObserved = (challenge) => {
|
||||||
|
manager.noteObservedAuthChallenge(challenge);
|
||||||
|
};
|
||||||
if (oauthManager.isEnterpriseManaged()) {
|
if (oauthManager.isEnterpriseManaged()) {
|
||||||
await oauthManager.trySilentEnterpriseManagedAuth();
|
await oauthManager.trySilentEnterpriseManagedAuth();
|
||||||
const provider = await oauthManager.createOAuthProviderForTransport();
|
const provider = await oauthManager.createOAuthProviderForTransport();
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ import {
|
||||||
AuthChallengeError,
|
AuthChallengeError,
|
||||||
parseAuthChallengeFromResponse,
|
parseAuthChallengeFromResponse,
|
||||||
} from "../../auth/challenge.js";
|
} from "../../auth/challenge.js";
|
||||||
|
import type { AuthChallenge } from "../../auth/challenge.js";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Wrap fetch so MCP HTTP 401/403 responses become {@link AuthChallengeError}
|
* Wrap fetch so MCP HTTP 401/403 responses become {@link AuthChallengeError}
|
||||||
|
|
@ -33,3 +34,37 @@ export function createAuthChallengeInterceptFetch(
|
||||||
);
|
);
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Wrap fetch so every MCP HTTP 401/403 is *reported* — control flow untouched,
|
||||||
|
* response returned as-is.
|
||||||
|
*
|
||||||
|
* This is the passive sibling of {@link createAuthChallengeInterceptFetch},
|
||||||
|
* and it exists for the one case that one cannot cover: a first-time
|
||||||
|
* authorization in the default legacy era. There, `InspectorClient`
|
||||||
|
* deliberately builds the transport with no `authProvider` (so the SDK does
|
||||||
|
* not open a browser before the callback server is listening) and therefore
|
||||||
|
* leaves interception off, so the CLI/TUI runner catches a plain
|
||||||
|
* `UnauthorizedError` and calls `authenticate()` itself. That SDK error
|
||||||
|
* carries no headers, so the challenge's RFC 9728 `resource_metadata` would be
|
||||||
|
* lost before discovery ever starts (#2071) — the observer keeps it.
|
||||||
|
*
|
||||||
|
* Only the status line and headers are read; the body is left untouched for
|
||||||
|
* the transport (or the interceptor layered above this one) to consume.
|
||||||
|
*/
|
||||||
|
export function createAuthChallengeObserverFetch(
|
||||||
|
baseFetch: typeof fetch,
|
||||||
|
onChallenge: (challenge: AuthChallenge) => void,
|
||||||
|
): typeof fetch {
|
||||||
|
return async (input, init) => {
|
||||||
|
const response = await baseFetch(input, init);
|
||||||
|
if (response.status === 401 || response.status === 403) {
|
||||||
|
const challenge = parseAuthChallengeFromResponse(response);
|
||||||
|
/* v8 ignore next 3 -- parseAuthChallengeFromResponse only returns undefined for non-401/403, which the status guard above already excludes */
|
||||||
|
if (challenge) {
|
||||||
|
onChallenge(challenge);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return response;
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -12,7 +12,10 @@ import { StdioClientTransport } from "@modelcontextprotocol/client/stdio";
|
||||||
import { SSEClientTransport } from "@modelcontextprotocol/client";
|
import { SSEClientTransport } from "@modelcontextprotocol/client";
|
||||||
import { StreamableHTTPClientTransport } from "@modelcontextprotocol/client";
|
import { StreamableHTTPClientTransport } from "@modelcontextprotocol/client";
|
||||||
import { createFetchTracker } from "../fetchTracking.js";
|
import { createFetchTracker } from "../fetchTracking.js";
|
||||||
import { createAuthChallengeInterceptFetch } from "./authChallengeFetch.js";
|
import {
|
||||||
|
createAuthChallengeInterceptFetch,
|
||||||
|
createAuthChallengeObserverFetch,
|
||||||
|
} from "./authChallengeFetch.js";
|
||||||
import { createProxyFetch } from "./proxyFetch.js";
|
import { createProxyFetch } from "./proxyFetch.js";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -49,6 +52,7 @@ export function createTransportNode(
|
||||||
authProvider,
|
authProvider,
|
||||||
settings,
|
settings,
|
||||||
interceptAuthChallenges = false,
|
interceptAuthChallenges = false,
|
||||||
|
onAuthChallengeObserved,
|
||||||
} = options;
|
} = options;
|
||||||
|
|
||||||
// `optionsFetchFn` is the caller's whole fetch stack and already sits on top of
|
// `optionsFetchFn` is the caller's whole fetch stack and already sits on top of
|
||||||
|
|
@ -57,9 +61,18 @@ export function createTransportNode(
|
||||||
// that. The fallback is for the one caller that supplies nothing: the web
|
// that. The fallback is for the one caller that supplies nothing: the web
|
||||||
// backend (`core/mcp/remote/node/server.ts`), which calls this directly.
|
// backend (`core/mcp/remote/node/server.ts`), which calls this directly.
|
||||||
const baseFetch = optionsFetchFn ?? createProxyFetch() ?? globalThis.fetch;
|
const baseFetch = optionsFetchFn ?? createProxyFetch() ?? globalThis.fetch;
|
||||||
|
// Purely passive, so — unlike proxying and interception — it is safe to
|
||||||
|
// apply to any fetch, including a caller's explicit one.
|
||||||
|
const withChallengeObserver = (inner: typeof fetch): typeof fetch =>
|
||||||
|
onAuthChallengeObserved
|
||||||
|
? createAuthChallengeObserverFetch(inner, onAuthChallengeObserved)
|
||||||
|
: inner;
|
||||||
|
// The observer sits *under* the interceptor so it still reports the
|
||||||
|
// challenge on the path where interception throws — and, more to the point,
|
||||||
|
// on the legacy first-auth path where interception is off entirely.
|
||||||
const fetchWithOptionalAuthIntercept = interceptAuthChallenges
|
const fetchWithOptionalAuthIntercept = interceptAuthChallenges
|
||||||
? createAuthChallengeInterceptFetch(baseFetch)
|
? createAuthChallengeInterceptFetch(withChallengeObserver(baseFetch))
|
||||||
: baseFetch;
|
: withChallengeObserver(baseFetch);
|
||||||
|
|
||||||
if (serverType === "stdio") {
|
if (serverType === "stdio") {
|
||||||
const stdioConfig = config as StdioServerConfig;
|
const stdioConfig = config as StdioServerConfig;
|
||||||
|
|
@ -92,9 +105,15 @@ export function createTransportNode(
|
||||||
// A caller-supplied eventSourceInit.fetch wins as-is (explicit fetch is not
|
// A caller-supplied eventSourceInit.fetch wins as-is (explicit fetch is not
|
||||||
// re-wrapped for proxying); the default path uses fetchWithOptionalAuthIntercept
|
// re-wrapped for proxying); the default path uses fetchWithOptionalAuthIntercept
|
||||||
// (the proxy-aware baseFetch plus the optional auth-challenge intercept).
|
// (the proxy-aware baseFetch plus the optional auth-challenge intercept).
|
||||||
const sseFetch =
|
// The challenge observer is applied either way: it is passive, and without
|
||||||
(sseConfig.eventSourceInit?.fetch as typeof fetch) ||
|
// it a first-time legacy SSE authorization through an explicit fetch would
|
||||||
fetchWithOptionalAuthIntercept;
|
// lose the challenge's `resource_metadata` the same way (Copilot).
|
||||||
|
const configuredSseFetch = sseConfig.eventSourceInit?.fetch as
|
||||||
|
| typeof fetch
|
||||||
|
| undefined;
|
||||||
|
const sseFetch = configuredSseFetch
|
||||||
|
? withChallengeObserver(configuredSseFetch)
|
||||||
|
: fetchWithOptionalAuthIntercept;
|
||||||
const trackedFetch = onFetchRequest
|
const trackedFetch = onFetchRequest
|
||||||
? createFetchTracker(sseFetch, {
|
? createFetchTracker(sseFetch, {
|
||||||
trackRequest: onFetchRequest,
|
trackRequest: onFetchRequest,
|
||||||
|
|
|
||||||
|
|
@ -39,6 +39,7 @@ import type {
|
||||||
HandleAuthChallengeOptions,
|
HandleAuthChallengeOptions,
|
||||||
} from "../auth/challenge.js";
|
} from "../auth/challenge.js";
|
||||||
import {
|
import {
|
||||||
|
challengeResourceMetadataUrl,
|
||||||
parseScopeString,
|
parseScopeString,
|
||||||
unionAuthorizationScopes,
|
unionAuthorizationScopes,
|
||||||
} from "../auth/challenge.js";
|
} from "../auth/challenge.js";
|
||||||
|
|
@ -81,6 +82,12 @@ export class OAuthManager {
|
||||||
private oauthFlowState: OAuthFlowState | null = null;
|
private oauthFlowState: OAuthFlowState | null = null;
|
||||||
/** SEP-2350 union scope pending until interactive step-up completes. */
|
/** SEP-2350 union scope pending until interactive step-up completes. */
|
||||||
private pendingAuthorizationScope: string | undefined;
|
private pendingAuthorizationScope: string | undefined;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The RFC 9728 metadata URL from the last 401/403 the transport observed.
|
||||||
|
* See {@link noteObservedAuthChallenge}.
|
||||||
|
*/
|
||||||
|
private observedResourceMetadataUrl: URL | undefined;
|
||||||
private authChallengeMutex: Promise<void> = Promise.resolve();
|
private authChallengeMutex: Promise<void> = Promise.resolve();
|
||||||
|
|
||||||
constructor(params: OAuthManagerParams) {
|
constructor(params: OAuthManagerParams) {
|
||||||
|
|
@ -262,6 +269,20 @@ export class OAuthManager {
|
||||||
return authorizationUrl;
|
return authorizationUrl;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Record a challenge the transport saw but did not raise as one.
|
||||||
|
*
|
||||||
|
* The legacy first-authorization path connects with no `authProvider` and no
|
||||||
|
* challenge interception, so the client calls {@link authenticate} off a
|
||||||
|
* headerless SDK `UnauthorizedError` — this is what carries the advertised
|
||||||
|
* RFC 9728 metadata URL across that gap (#2071). Latest-wins: every
|
||||||
|
* challenge comes from the same server, so the most recent one is what that
|
||||||
|
* server currently says, including when it says nothing.
|
||||||
|
*/
|
||||||
|
noteObservedAuthChallenge(challenge: AuthChallenge): void {
|
||||||
|
this.observedResourceMetadataUrl = challengeResourceMetadataUrl(challenge);
|
||||||
|
}
|
||||||
|
|
||||||
async authenticate(): Promise<URL | undefined> {
|
async authenticate(): Promise<URL | undefined> {
|
||||||
if (this.isEnterpriseManaged()) {
|
if (this.isEnterpriseManaged()) {
|
||||||
return this.authenticateEnterpriseManaged();
|
return this.authenticateEnterpriseManaged();
|
||||||
|
|
@ -272,16 +293,23 @@ export class OAuthManager {
|
||||||
|
|
||||||
provider.clearCapturedAuthUrl();
|
provider.clearCapturedAuthUrl();
|
||||||
|
|
||||||
|
// No challenge is passed to this entry point — it is reached from a plain
|
||||||
|
// unauthorized error — so the last one the transport observed is the only
|
||||||
|
// source for the advertised metadata URL.
|
||||||
|
const resourceMetadataUrl = this.observedResourceMetadataUrl;
|
||||||
|
|
||||||
await ensureCimdClientRegistration({
|
await ensureCimdClientRegistration({
|
||||||
serverUrl,
|
serverUrl,
|
||||||
provider,
|
provider,
|
||||||
fetchFn: this.params.effectiveAuthFetch,
|
fetchFn: this.params.effectiveAuthFetch,
|
||||||
|
resourceMetadataUrl,
|
||||||
});
|
});
|
||||||
|
|
||||||
const result = await mcpAuth(provider, {
|
const result = await mcpAuth(provider, {
|
||||||
serverUrl,
|
serverUrl,
|
||||||
scope: provider.scope,
|
scope: provider.scope,
|
||||||
fetchFn: this.params.effectiveAuthFetch,
|
fetchFn: this.params.effectiveAuthFetch,
|
||||||
|
resourceMetadataUrl,
|
||||||
});
|
});
|
||||||
|
|
||||||
if (result === "AUTHORIZED") {
|
if (result === "AUTHORIZED") {
|
||||||
|
|
@ -733,10 +761,17 @@ export class OAuthManager {
|
||||||
|
|
||||||
provider.clearCapturedAuthUrl();
|
provider.clearCapturedAuthUrl();
|
||||||
|
|
||||||
|
// RFC 9728: honor the metadata document the challenge advertised rather
|
||||||
|
// than probing the default locations derived from the MCP server URL
|
||||||
|
// (#2071). The SDK persists it in its discovery state, so the callback leg
|
||||||
|
// recovers it without our passing it again.
|
||||||
|
const resourceMetadataUrl = challengeResourceMetadataUrl(enriched);
|
||||||
|
|
||||||
await ensureCimdClientRegistration({
|
await ensureCimdClientRegistration({
|
||||||
serverUrl,
|
serverUrl,
|
||||||
provider,
|
provider,
|
||||||
fetchFn: this.params.effectiveAuthFetch,
|
fetchFn: this.params.effectiveAuthFetch,
|
||||||
|
resourceMetadataUrl,
|
||||||
});
|
});
|
||||||
|
|
||||||
const scopeForAuth =
|
const scopeForAuth =
|
||||||
|
|
@ -754,6 +789,7 @@ export class OAuthManager {
|
||||||
serverUrl,
|
serverUrl,
|
||||||
scope: scopeForAuth,
|
scope: scopeForAuth,
|
||||||
fetchFn: this.params.effectiveAuthFetch,
|
fetchFn: this.params.effectiveAuthFetch,
|
||||||
|
resourceMetadataUrl,
|
||||||
...(enriched.reason === "insufficient_scope" && {
|
...(enriched.reason === "insufficient_scope" && {
|
||||||
forceReauthorization: isStrictScopeSuperset(
|
forceReauthorization: isStrictScopeSuperset(
|
||||||
scopeForAuth,
|
scopeForAuth,
|
||||||
|
|
@ -883,6 +919,7 @@ export class OAuthManager {
|
||||||
serverUrl,
|
serverUrl,
|
||||||
scope: scopeForAuth,
|
scope: scopeForAuth,
|
||||||
fetchFn: this.params.effectiveAuthFetch,
|
fetchFn: this.params.effectiveAuthFetch,
|
||||||
|
resourceMetadataUrl: challengeResourceMetadataUrl(enriched),
|
||||||
forceReauthorization: true,
|
forceReauthorization: true,
|
||||||
});
|
});
|
||||||
} finally {
|
} finally {
|
||||||
|
|
|
||||||
|
|
@ -38,6 +38,7 @@ import type {
|
||||||
RedirectUrlProvider,
|
RedirectUrlProvider,
|
||||||
} from "../auth/providers.js";
|
} from "../auth/providers.js";
|
||||||
import type { OAuthStorage } from "../auth/storage.js";
|
import type { OAuthStorage } from "../auth/storage.js";
|
||||||
|
import type { AuthChallenge } from "../auth/challenge.js";
|
||||||
|
|
||||||
// Stdio transport config
|
// Stdio transport config
|
||||||
export interface StdioServerConfig {
|
export interface StdioServerConfig {
|
||||||
|
|
@ -903,6 +904,15 @@ export interface CreateTransportOptions {
|
||||||
* become {@link AuthChallengeError} before the SDK calls `auth()` on a frozen provider.
|
* become {@link AuthChallengeError} before the SDK calls `auth()` on a frozen provider.
|
||||||
*/
|
*/
|
||||||
interceptAuthChallenges?: boolean;
|
interceptAuthChallenges?: boolean;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Called for every HTTP 401/403 the transport sees, without altering control
|
||||||
|
* flow. Unlike {@link interceptAuthChallenges} this never throws, so it also
|
||||||
|
* covers the legacy first-time-authorization path where interception is
|
||||||
|
* deliberately off and the SDK's headerless `UnauthorizedError` would
|
||||||
|
* otherwise lose the challenge's `resource_metadata` (#2071).
|
||||||
|
*/
|
||||||
|
onAuthChallengeObserved?: (challenge: AuthChallenge) => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface CreateTransportResult {
|
export interface CreateTransportResult {
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,19 @@
|
||||||
|
{
|
||||||
|
"serverInfo": {
|
||||||
|
"name": "oauth-custom-resource-metadata",
|
||||||
|
"version": "1.0.0"
|
||||||
|
},
|
||||||
|
"tools": [{ "preset": "echo" }],
|
||||||
|
"oauth": {
|
||||||
|
"enabled": true,
|
||||||
|
"mode": "combined",
|
||||||
|
"requireAuth": true,
|
||||||
|
"resourceMetadataPath": "/custom/protected-resource",
|
||||||
|
"scopesSupported": ["mcp"],
|
||||||
|
"supportDCR": true
|
||||||
|
},
|
||||||
|
"transport": {
|
||||||
|
"type": "streamable-http",
|
||||||
|
"port": 8082
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -573,6 +573,19 @@ export interface ServerConfig {
|
||||||
*/
|
*/
|
||||||
resource?: string;
|
resource?: string;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Serve the RFC 9728 protected-resource metadata document from this
|
||||||
|
* non-default path *instead of* `/.well-known/oauth-protected-resource`,
|
||||||
|
* and advertise it via `WWW-Authenticate: Bearer resource_metadata="…"`
|
||||||
|
* on every 401.
|
||||||
|
*
|
||||||
|
* The default well-known routes are deliberately left unserved, so a
|
||||||
|
* client that ignores the advertised URL cannot discover the document at
|
||||||
|
* all — which is what makes this a regression test for #2071 rather than
|
||||||
|
* a path that merely happens to work.
|
||||||
|
*/
|
||||||
|
resourceMetadataPath?: string;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* OAuth authorization server issuer URL (combined mode AS metadata).
|
* OAuth authorization server issuer URL (combined mode AS metadata).
|
||||||
* If not provided, defaults to the test server's base URL.
|
* If not provided, defaults to the test server's base URL.
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,7 @@
|
||||||
import { readFileSync } from "fs";
|
import { readFileSync } from "fs";
|
||||||
import path from "path";
|
import path from "path";
|
||||||
import YAML from "yaml";
|
import YAML from "yaml";
|
||||||
|
import { isOriginRelativePath } from "./test-server-oauth.js";
|
||||||
|
|
||||||
export interface PresetRef {
|
export interface PresetRef {
|
||||||
preset: string;
|
preset: string;
|
||||||
|
|
@ -19,6 +20,8 @@ export interface ConfigFileOAuth {
|
||||||
mode?: "combined" | "protected-resource";
|
mode?: "combined" | "protected-resource";
|
||||||
authorizationServers?: string[];
|
authorizationServers?: string[];
|
||||||
resource?: string;
|
resource?: string;
|
||||||
|
/** Serve RFC 9728 metadata from this path and advertise it on 401 (#2071). */
|
||||||
|
resourceMetadataPath?: string;
|
||||||
issuerUrl?: string;
|
issuerUrl?: string;
|
||||||
accessTokenIssuers?: string[];
|
accessTokenIssuers?: string[];
|
||||||
jwksUri?: string;
|
jwksUri?: string;
|
||||||
|
|
@ -200,6 +203,12 @@ function validateConfig(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
const metadataPath = oauth.resourceMetadataPath;
|
||||||
|
if (metadataPath !== undefined && !isOriginRelativePath(metadataPath)) {
|
||||||
|
throw new Error(
|
||||||
|
`Invalid config in ${filePath}: oauth.resourceMetadataPath must be an origin-relative path (e.g. "/custom/protected-resource") — a value such as "//host/doc" would advertise a document the server does not serve`,
|
||||||
|
);
|
||||||
|
}
|
||||||
if (transportType === "stdio" && oauth.enabled === true) {
|
if (transportType === "stdio" && oauth.enabled === true) {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
`Invalid config in ${filePath}: oauth requires streamable-http or sse transport`,
|
`Invalid config in ${filePath}: oauth requires streamable-http or sse transport`,
|
||||||
|
|
|
||||||
|
|
@ -2910,6 +2910,11 @@ export function createOAuthTestServerConfig(options: {
|
||||||
supportCIMD?: boolean;
|
supportCIMD?: boolean;
|
||||||
tokenExpirationSeconds?: number;
|
tokenExpirationSeconds?: number;
|
||||||
supportRefreshTokens?: boolean;
|
supportRefreshTokens?: boolean;
|
||||||
|
/**
|
||||||
|
* Move the RFC 9728 metadata document off the well-known path and advertise
|
||||||
|
* it via `WWW-Authenticate: Bearer resource_metadata="…"` (#2071).
|
||||||
|
*/
|
||||||
|
resourceMetadataPath?: string;
|
||||||
}): Partial<ServerConfig> {
|
}): Partial<ServerConfig> {
|
||||||
return {
|
return {
|
||||||
oauth: {
|
oauth: {
|
||||||
|
|
@ -2917,6 +2922,12 @@ export function createOAuthTestServerConfig(options: {
|
||||||
mode: "combined",
|
mode: "combined",
|
||||||
requireAuth: options.requireAuth ?? false,
|
requireAuth: options.requireAuth ?? false,
|
||||||
scopesSupported: options.scopesSupported ?? ["mcp"],
|
scopesSupported: options.scopesSupported ?? ["mcp"],
|
||||||
|
// `!== undefined`, not truthiness: an explicitly supplied `""` is
|
||||||
|
// invalid, and dropping it here would silently fall back to the
|
||||||
|
// well-known route instead of reporting the bad fixture (Copilot).
|
||||||
|
...(options.resourceMetadataPath !== undefined
|
||||||
|
? { resourceMetadataPath: options.resourceMetadataPath }
|
||||||
|
: {}),
|
||||||
staticClients: options.staticClients,
|
staticClients: options.staticClients,
|
||||||
supportDCR: options.supportDCR ?? false,
|
supportDCR: options.supportDCR ?? false,
|
||||||
supportCIMD: options.supportCIMD ?? false,
|
supportCIMD: options.supportCIMD ?? false,
|
||||||
|
|
|
||||||
|
|
@ -27,6 +27,79 @@ export function getOAuthMode(
|
||||||
return config.mode ?? "combined";
|
return config.mode ?? "combined";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const PATH_VALIDATION_BASE = "http://config.invalid";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* True for a path that resolves under its own origin — the only shape safe to
|
||||||
|
* use as both an Express route and a `resource_metadata` value.
|
||||||
|
*
|
||||||
|
* A leading-slash check is not enough: `//other-host/doc` and `/\other-host/doc`
|
||||||
|
* both re-point the origin when resolved against the request base (the URL
|
||||||
|
* parser folds a backslash into a slash for special schemes), while Express
|
||||||
|
* still registers the route locally — so the server would advertise a document
|
||||||
|
* it does not serve (Copilot). Comparing the resolved href against the literal
|
||||||
|
* also rejects anything the parser would rewrite (spaces, unescaped
|
||||||
|
* characters), which an Express route would not match either.
|
||||||
|
*
|
||||||
|
* A query or fragment is rejected for the same reason from the other
|
||||||
|
* direction: `href` preserves both, so `/doc?v=1` and `/doc#s` would pass the
|
||||||
|
* comparison above, yet Express matches on the path alone (and treats `?` as a
|
||||||
|
* pattern character) and a fragment is never sent on the wire at all — so the
|
||||||
|
* advertised URL could not reach the registered route (Copilot).
|
||||||
|
*/
|
||||||
|
export function isOriginRelativePath(value: unknown): value is string {
|
||||||
|
if (typeof value !== "string" || !value.startsWith("/")) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
const resolved = new URL(value, PATH_VALIDATION_BASE);
|
||||||
|
return (
|
||||||
|
resolved.origin === PATH_VALIDATION_BASE &&
|
||||||
|
resolved.search === "" &&
|
||||||
|
resolved.hash === "" &&
|
||||||
|
resolved.href === `${PATH_VALIDATION_BASE}${value}`
|
||||||
|
);
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The configured metadata path, validated. Throws at server-setup time rather
|
||||||
|
* than serving a route that contradicts the challenge — the JSON-config path
|
||||||
|
* is validated earlier by `load-config`, so this covers a `ServerConfig`
|
||||||
|
* built programmatically.
|
||||||
|
*/
|
||||||
|
function resourceMetadataPath(config: OAuthConfig): string | undefined {
|
||||||
|
const path = config.resourceMetadataPath;
|
||||||
|
if (path === undefined) {
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
if (!isOriginRelativePath(path)) {
|
||||||
|
throw new Error(
|
||||||
|
`oauth.resourceMetadataPath must be an origin-relative path (got ${JSON.stringify(path)})`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return path;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The `WWW-Authenticate` challenge sent with every 401.
|
||||||
|
*
|
||||||
|
* RFC 9728 §5.1: a resource server advertises where its protected-resource
|
||||||
|
* metadata lives via the `resource_metadata` parameter. Only emitted when the
|
||||||
|
* config moves that document off the well-known path — otherwise the bare
|
||||||
|
* `Bearer` challenge keeps the existing fixtures byte-identical.
|
||||||
|
*/
|
||||||
|
function bearerChallenge(config: OAuthConfig, req: Request): string {
|
||||||
|
const path = resourceMetadataPath(config);
|
||||||
|
if (!path) {
|
||||||
|
return "Bearer";
|
||||||
|
}
|
||||||
|
const requestBaseUrl = `${req.protocol}://${req.get("host")}`;
|
||||||
|
return `Bearer resource_metadata="${new URL(path, requestBaseUrl).href}"`;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Set up OAuth routes on an Express application
|
* Set up OAuth routes on an Express application
|
||||||
* This adds all OAuth endpoints (authorization, token, metadata, etc.)
|
* This adds all OAuth endpoints (authorization, token, metadata, etc.)
|
||||||
|
|
@ -76,7 +149,7 @@ export function createBearerTokenMiddleware(
|
||||||
// For streamable-http, the SDK checks response status and throws StreamableHTTPError with code 401
|
// For streamable-http, the SDK checks response status and throws StreamableHTTPError with code 401
|
||||||
res.status(401);
|
res.status(401);
|
||||||
res.setHeader("Content-Type", "application/json");
|
res.setHeader("Content-Type", "application/json");
|
||||||
res.setHeader("WWW-Authenticate", "Bearer");
|
res.setHeader("WWW-Authenticate", bearerChallenge(config, req));
|
||||||
// Return a JSON-RPC error response format that the SDK will recognize
|
// Return a JSON-RPC error response format that the SDK will recognize
|
||||||
res.json({
|
res.json({
|
||||||
jsonrpc: "2.0",
|
jsonrpc: "2.0",
|
||||||
|
|
@ -113,7 +186,7 @@ export function createBearerTokenMiddleware(
|
||||||
// Return 401 - the SDK's transport should detect this and throw an error
|
// Return 401 - the SDK's transport should detect this and throw an error
|
||||||
res.status(401);
|
res.status(401);
|
||||||
res.setHeader("Content-Type", "application/json");
|
res.setHeader("Content-Type", "application/json");
|
||||||
res.setHeader("WWW-Authenticate", "Bearer");
|
res.setHeader("WWW-Authenticate", bearerChallenge(config, req));
|
||||||
// Return a JSON-RPC error response format that the SDK will recognize
|
// Return a JSON-RPC error response format that the SDK will recognize
|
||||||
res.json({
|
res.json({
|
||||||
jsonrpc: "2.0",
|
jsonrpc: "2.0",
|
||||||
|
|
@ -184,9 +257,12 @@ function setupMetadataEndpoints(
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// OAuth Protected Resource Metadata
|
// OAuth Protected Resource Metadata. `resourceMetadataPath` moves the
|
||||||
|
// document off the well-known path entirely (rather than serving both), so
|
||||||
|
// a client that ignores the advertised `resource_metadata` URL gets a 404
|
||||||
|
// — see the field's doc comment.
|
||||||
app.get(
|
app.get(
|
||||||
"/.well-known/oauth-protected-resource",
|
resourceMetadataPath(config) ?? "/.well-known/oauth-protected-resource",
|
||||||
(req: Request, res: Response) => {
|
(req: Request, res: Response) => {
|
||||||
const requestBaseUrl = `${req.protocol}://${req.get("host")}`;
|
const requestBaseUrl = `${req.protocol}://${req.get("host")}`;
|
||||||
const resourceUrl = config.resource ?? new URL("/", requestBaseUrl).href;
|
const resourceUrl = config.resource ?? new URL("/", requestBaseUrl).href;
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue