fix(oauth): keep the server's path in the fallback authorization-server URL
When RFC 9728 protected-resource metadata names no authorization server, the
MCP server URL stands in as one. `getAuthorizationServerUrl` built that with
`new URL("/", serverUrl)`, which discards the path — exactly the part discovery
needs when the server is not hosted at the domain root. The SDK's
`buildDiscoveryUrls` derives the path-scoped well-known locations from that
pathname, so an origin-only value could only ever probe the domain root, which
404s for a server like `https://example.com/mod/minilesson/mcp.php`.
Keep the path (and strip query/fragment, which an RFC 8414 issuer never
carries). A straight swap would regress the case the origin-only value did
serve — a server that merely lives under a path while publishing its metadata
at the root — so the URL becomes the first of two candidates:
`getAuthorizationServerUrlCandidates` yields the path-scoped form then the bare
origin, and `discoverAuthorizationServerMetadataForServer` walks them, treating
a candidate's failure as non-fatal while another remains and rethrowing the
first error only when all of them fail.
`discoverScopes` and the CIMD pre-registration probe both go through the walk.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
ca2846de28
commit
afa0ba6eeb
1 changed files with 292 additions and 21 deletions
|
|
@ -2,6 +2,38 @@ import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/clien
|
|||
import type { OAuthProtectedResourceMetadata } from "@modelcontextprotocol/client";
|
||||
import { parseHttpUrl } from "./utils.js";
|
||||
|
||||
type AuthorizationServerMetadata = Awaited<
|
||||
ReturnType<typeof discoverAuthorizationServerMetadata>
|
||||
>;
|
||||
|
||||
/**
|
||||
* The MCP server's own URL, used as the authorization server when RFC 9728
|
||||
* protected-resource metadata named none.
|
||||
*
|
||||
* The path is kept. Reducing it to the origin (`new URL("/", serverUrl)`)
|
||||
* discards exactly the part that tells discovery where to look when the server
|
||||
* is not hosted at the domain root: the SDK's `buildDiscoveryUrls` derives the
|
||||
* path-scoped well-known locations — `/.well-known/oauth-authorization-server{path}`
|
||||
* and `{path}/.well-known/openid-configuration` — from this URL's pathname, so
|
||||
* an origin-only value can only ever probe the domain root (#2110).
|
||||
*
|
||||
* Query and fragment are stripped: an RFC 8414 issuer identifier carries
|
||||
* neither, and the SDK compares the discovered `issuer` against this URL.
|
||||
*/
|
||||
function serverUrlAsAuthorizationServer(serverUrl: string): URL {
|
||||
try {
|
||||
const url = new URL(serverUrl);
|
||||
url.search = "";
|
||||
url.hash = "";
|
||||
return url;
|
||||
} catch (err) {
|
||||
const detail = err instanceof Error ? err.message : String(err);
|
||||
throw new Error(`Invalid MCP server URL: "${serverUrl}" (${detail})`, {
|
||||
cause: err,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the URL to use for OAuth authorization server metadata discovery.
|
||||
* Uses resource metadata's authorization_servers[0] when present, otherwise the MCP server URL.
|
||||
|
|
@ -15,14 +47,69 @@ export function getAuthorizationServerUrl(
|
|||
if (first) {
|
||||
return parseHttpUrl(first, "protected resource authorization_servers[0]");
|
||||
}
|
||||
try {
|
||||
return new URL("/", serverUrl);
|
||||
} catch (err) {
|
||||
const detail = err instanceof Error ? err.message : String(err);
|
||||
throw new Error(`Invalid MCP server URL: "${serverUrl}" (${detail})`, {
|
||||
cause: err,
|
||||
});
|
||||
return serverUrlAsAuthorizationServer(serverUrl);
|
||||
}
|
||||
|
||||
/**
|
||||
* The authorization-server URLs to try, in order.
|
||||
*
|
||||
* When protected-resource metadata names an authorization server there is
|
||||
* exactly one candidate — the server told us where to look, and probing past it
|
||||
* would be guessing. Otherwise the MCP server URL itself stands in as the
|
||||
* authorization server, and a path-hosted server has two plausible answers: the
|
||||
* full URL, whose path-scoped well-known locations are what a server like
|
||||
* `https://example.com/mod/minilesson/mcp.php` publishes, and the bare origin,
|
||||
* used by a server that merely *lives* under a path while publishing its
|
||||
* metadata at the domain root. Trying the path-scoped form first fixes #2110
|
||||
* without regressing the root-hosted case, which is all the origin-only value
|
||||
* ever served.
|
||||
*/
|
||||
export function getAuthorizationServerUrlCandidates(
|
||||
serverUrl: string,
|
||||
resourceMetadata?: OAuthProtectedResourceMetadata | null,
|
||||
): URL[] {
|
||||
const primary = getAuthorizationServerUrl(serverUrl, resourceMetadata);
|
||||
const namedByMetadata = Boolean(resourceMetadata?.authorization_servers?.[0]);
|
||||
if (namedByMetadata || primary.pathname === "/") return [primary];
|
||||
return [primary, new URL("/", primary)];
|
||||
}
|
||||
|
||||
/**
|
||||
* Discovers authorization server metadata for an MCP server, walking the
|
||||
* candidates above until one answers.
|
||||
*
|
||||
* A candidate that fails — a 5xx, or an RFC 8414 §3.3 issuer mismatch, both of
|
||||
* which the SDK raises rather than returning `undefined` — is not fatal while
|
||||
* another candidate remains. If every candidate fails, the first error is
|
||||
* rethrown, so a caller still sees the original diagnosis rather than a bare
|
||||
* `undefined`.
|
||||
*/
|
||||
export async function discoverAuthorizationServerMetadataForServer(
|
||||
serverUrl: string,
|
||||
resourceMetadata?: OAuthProtectedResourceMetadata | null,
|
||||
fetchFn?: typeof fetch,
|
||||
): Promise<AuthorizationServerMetadata> {
|
||||
const candidates = getAuthorizationServerUrlCandidates(
|
||||
serverUrl,
|
||||
resourceMetadata,
|
||||
);
|
||||
let firstError: unknown;
|
||||
let sawError = false;
|
||||
for (const candidate of candidates) {
|
||||
try {
|
||||
const metadata = await discoverAuthorizationServerMetadata(candidate, {
|
||||
fetchFn,
|
||||
});
|
||||
if (metadata) return metadata;
|
||||
} catch (err) {
|
||||
if (!sawError) {
|
||||
firstError = err;
|
||||
sawError = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (sawError) throw firstError;
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -38,13 +125,11 @@ export const discoverScopes = async (
|
|||
fetchFn?: typeof fetch,
|
||||
): Promise<string | undefined> => {
|
||||
try {
|
||||
const authServerUrl = getAuthorizationServerUrl(
|
||||
const metadata = await discoverAuthorizationServerMetadataForServer(
|
||||
serverUrl,
|
||||
resourceMetadata,
|
||||
);
|
||||
const metadata = await discoverAuthorizationServerMetadata(authServerUrl, {
|
||||
fetchFn,
|
||||
});
|
||||
);
|
||||
|
||||
// Prefer resource metadata scopes, but fall back to OAuth metadata if empty
|
||||
const resourceScopes = resourceMetadata?.scopes_supported;
|
||||
|
|
|
|||
Loading…
Reference in a new issue