From be90ac5b86ac8b7821766139562aa75914d18f5e Mon Sep 17 00:00:00 2001 From: Xiao Yijun Date: Wed, 18 Jun 2025 10:53:20 +0800 Subject: [PATCH 01/16] feat: support oidc discovery in client sdk --- .claude/settings.local.json | 11 + src/client/auth.test.ts | 120 ++++++++-- src/client/auth.ts | 453 +++++++++++++++++++++++++++--------- src/client/sse.test.ts | 11 +- src/shared/auth.ts | 187 +++++++++++---- 5 files changed, 603 insertions(+), 179 deletions(-) create mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 000000000..0609f59bf --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,11 @@ +{ + "permissions": { + "allow": [ + "Bash(npm test:*)", + "Bash(npm run lint)", + "WebFetch(domain:github.com)", + "WebFetch(domain:openid.net)" + ], + "deny": [] + } +} \ No newline at end of file diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index ce0cc7081..3fd7bc6a8 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -359,7 +359,7 @@ describe("OAuth Authorization", () => { code_challenge_methods_supported: ["S256"], }; - it("returns metadata when discovery succeeds", async () => { + it("returns metadata when oauth-authorization-server discovery succeeds", async () => { mockFetch.mockResolvedValueOnce({ ok: true, status: 200, @@ -377,6 +377,28 @@ describe("OAuth Authorization", () => { }); }); + it("returns metadata when oidc discovery succeeds", async () => { + mockFetch.mockImplementation((url) => { + if (url.toString().includes('openid-configuration')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => validMetadata, + }); + } + return Promise.resolve({ + ok: false, + status: 404, + }); + }); + + const metadata = await discoverOAuthMetadata("https://auth.example.com"); + expect(metadata).toEqual(validMetadata); + expect(mockFetch).toHaveBeenCalledTimes(2); + expect(mockFetch.mock.calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + expect(mockFetch.mock.calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration"); + }); + it("returns metadata when discovery succeeds with path", async () => { mockFetch.mockResolvedValueOnce({ ok: true, @@ -395,14 +417,14 @@ describe("OAuth Authorization", () => { }); }); - it("falls back to root discovery when path-aware discovery returns 404", async () => { - // First call (path-aware) returns 404 + it("tries discovery endpoints in new spec order for URLs with path", async () => { + // First call (OAuth with path insertion) returns 404 mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); - // Second call (root fallback) succeeds + // Second call (OIDC with path insertion) succeeds mockFetch.mockResolvedValueOnce({ ok: true, status: 200, @@ -415,29 +437,35 @@ describe("OAuth Authorization", () => { const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); - // First call should be path-aware + // First call should be OAuth with path insertion const [firstUrl, firstOptions] = calls[0]; expect(firstUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/path/name"); expect(firstOptions.headers).toEqual({ "MCP-Protocol-Version": LATEST_PROTOCOL_VERSION }); - // Second call should be root fallback + // Second call should be OIDC with path insertion const [secondUrl, secondOptions] = calls[1]; - expect(secondUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + expect(secondUrl.toString()).toBe("https://auth.example.com/.well-known/openid-configuration/path/name"); expect(secondOptions.headers).toEqual({ "MCP-Protocol-Version": LATEST_PROTOCOL_VERSION }); }); - it("returns undefined when both path-aware and root discovery return 404", async () => { - // First call (path-aware) returns 404 + it("returns undefined when all discovery endpoints return 404", async () => { + // First call (OAuth with path insertion) returns 404 mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); - // Second call (root fallback) also returns 404 + // Second call (OIDC with path insertion) returns 404 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // Third call (OIDC with path appending) returns 404 mockFetch.mockResolvedValueOnce({ ok: false, status: 404, @@ -447,7 +475,33 @@ describe("OAuth Authorization", () => { expect(metadata).toBeUndefined(); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(2); + expect(calls.length).toBe(3); + }); + + it("tries all endpoints in correct order for URLs with path", async () => { + // All calls return 404 to test the order + mockFetch.mockResolvedValue({ + ok: false, + status: 404, + }); + + const metadata = await discoverOAuthMetadata("https://auth.example.com/tenant1"); + expect(metadata).toBeUndefined(); + + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(3); + + // First call should be OAuth 2.0 Authorization Server Metadata with path insertion + const [firstUrl] = calls[0]; + expect(firstUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); + + // Second call should be OpenID Connect Discovery 1.0 with path insertion + const [secondUrl] = calls[1]; + expect(secondUrl.toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); + + // Third call should be OpenID Connect Discovery 1.0 path appending + const [thirdUrl] = calls[2]; + expect(thirdUrl.toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration"); }); it("does not fallback when the original URL is already at root path", async () => { @@ -457,11 +511,17 @@ describe("OAuth Authorization", () => { status: 404, }); + // Second call (OIDC discovery) also returns 404 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + const metadata = await discoverOAuthMetadata("https://auth.example.com/"); expect(metadata).toBeUndefined(); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(1); // Should not attempt fallback + expect(calls.length).toBe(2); // Should not attempt fallback but will try OIDC const [url] = calls[0]; expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); @@ -474,27 +534,42 @@ describe("OAuth Authorization", () => { status: 404, }); + // Second call (OIDC discovery) also returns 404 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + const metadata = await discoverOAuthMetadata("https://auth.example.com"); expect(metadata).toBeUndefined(); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(1); // Should not attempt fallback + expect(calls.length).toBe(2); // Should not attempt fallback but will try OIDC const [url] = calls[0]; expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); }); - it("falls back when path-aware discovery encounters CORS error", async () => { - // First call (path-aware) fails with TypeError (CORS) + it("tries all endpoints when discovery encounters CORS error", async () => { + // First call (OAuth with path insertion) fails with TypeError (CORS) mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError("CORS error"))); - // Retry path-aware without headers (simulating CORS retry) + // Retry OAuth with path insertion without headers (simulating CORS retry) mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); - // Second call (root fallback) succeeds + // Second call (OIDC with path insertion) fails with TypeError (CORS) + mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError("CORS error"))); + + // Retry OIDC with path insertion without headers (simulating CORS retry) + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // Third call (OIDC with path appending) succeeds mockFetch.mockResolvedValueOnce({ ok: true, status: 200, @@ -505,11 +580,11 @@ describe("OAuth Authorization", () => { expect(metadata).toEqual(validMetadata); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(3); + expect(calls.length).toBe(5); - // Final call should be root fallback - const [lastUrl, lastOptions] = calls[2]; - expect(lastUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + // Final call should be OIDC with path appending + const [lastUrl, lastOptions] = calls[4]; + expect(lastUrl.toString()).toBe("https://auth.example.com/deep/path/.well-known/openid-configuration"); expect(lastOptions.headers).toEqual({ "MCP-Protocol-Version": LATEST_PROTOCOL_VERSION }); @@ -587,13 +662,14 @@ describe("OAuth Authorization", () => { }); it("returns undefined when discovery endpoint returns 404", async () => { - mockFetch.mockResolvedValueOnce({ + mockFetch.mockResolvedValue({ ok: false, status: 404, }); const metadata = await discoverOAuthMetadata("https://auth.example.com"); expect(metadata).toBeUndefined(); + expect(mockFetch).toHaveBeenCalledTimes(2); }); it("throws on non-404 errors", async () => { diff --git a/src/client/auth.ts b/src/client/auth.ts index 4a8bbe2d2..416965285 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -7,19 +7,36 @@ import { OAuthMetadata, OAuthClientInformationFull, OAuthProtectedResourceMetadata, - OAuthErrorResponseSchema + OAuthErrorResponseSchema, + OpenIdProviderMetadata, + AuthorizationServerMetadata, } from "../shared/auth.js"; -import { OAuthClientInformationFullSchema, OAuthMetadataSchema, OAuthProtectedResourceMetadataSchema, OAuthTokensSchema } from "../shared/auth.js"; -import { checkResourceAllowed, resourceUrlFromServerUrl } from "../shared/auth-utils.js"; +import { + OAuthClientInformationFullSchema, + OAuthMetadataSchema, + OAuthProtectedResourceMetadataSchema, + OAuthTokensSchema, + OpenIdProviderMetadataSchema, +} from "../shared/auth.js"; +import { + checkResourceAllowed, + resourceUrlFromServerUrl, +} from "../shared/auth-utils.js"; import { InvalidClientError, InvalidGrantError, OAUTH_ERRORS, OAuthError, ServerError, - UnauthorizedClientError + UnauthorizedClientError, } from "../server/auth/errors.js"; +const wellKnownPaths = { + oauthAuthorizationServer: "/.well-known/oauth-authorization-server", + oauthProtectedResource: "/.well-known/oauth-protected-resource", + openidConfiguration: "/.well-known/openid-configuration", +} as const; + /** * Implements an end-to-end OAuth client to be used with one MCP server. * @@ -48,7 +65,10 @@ export interface OAuthClientProvider { * server, or returns `undefined` if the client is not registered with the * server. */ - clientInformation(): OAuthClientInformation | undefined | Promise; + clientInformation(): + | OAuthClientInformation + | undefined + | Promise; /** * If implemented, this permits the OAuth client to dynamically register with @@ -58,7 +78,9 @@ export interface OAuthClientProvider { * This method is not required to be implemented if client information is * statically known (e.g., pre-registered). */ - saveClientInformation?(clientInformation: OAuthClientInformationFull): void | Promise; + saveClientInformation?( + clientInformation: OAuthClientInformationFull + ): void | Promise; /** * Loads any existing OAuth tokens for the current session, or returns @@ -107,7 +129,12 @@ export interface OAuthClientProvider { * @param url - The token endpoint URL being called * @param metadata - Optional OAuth metadata for the server, which may include supported authentication methods */ - addClientAuthentication?(headers: Headers, params: URLSearchParams, url: string | URL, metadata?: OAuthMetadata): void | Promise; + addClientAuthentication?( + headers: Headers, + params: URLSearchParams, + url: string | URL, + metadata?: OAuthMetadata + ): void | Promise; /** * If defined, overrides the selection and validation of the @@ -116,14 +143,19 @@ export interface OAuthClientProvider { * * Implementations must verify the returned resource matches the MCP server. */ - validateResourceURL?(serverUrl: string | URL, resource?: string): Promise; + validateResourceURL?( + serverUrl: string | URL, + resource?: string + ): Promise; /** * If implemented, provides a way for the client to invalidate (e.g. delete) the specified * credentials, in the case where the server has indicated that they are no longer valid. * This avoids requiring the user to intervene manually. */ - invalidateCredentials?(scope: 'all' | 'client' | 'tokens' | 'verifier'): void | Promise; + invalidateCredentials?( + scope: "all" | "client" | "tokens" | "verifier" + ): void | Promise; } export type AuthResult = "AUTHORIZED" | "REDIRECT"; @@ -134,7 +166,7 @@ export class UnauthorizedError extends Error { } } -type ClientAuthMethod = 'client_secret_basic' | 'client_secret_post' | 'none'; +type ClientAuthMethod = "client_secret_basic" | "client_secret_post" | "none"; /** * Determines the best client authentication method to use based on server support and client configuration. @@ -216,9 +248,15 @@ function applyClientAuthentication( /** * Applies HTTP Basic authentication (RFC 6749 Section 2.3.1) */ -function applyBasicAuth(clientId: string, clientSecret: string | undefined, headers: Headers): void { +function applyBasicAuth( + clientId: string, + clientSecret: string | undefined, + headers: Headers +): void { if (!clientSecret) { - throw new Error("client_secret_basic authentication requires a client_secret"); + throw new Error( + "client_secret_basic authentication requires a client_secret" + ); } const credentials = btoa(`${clientId}:${clientSecret}`); @@ -228,7 +266,11 @@ function applyBasicAuth(clientId: string, clientSecret: string | undefined, head /** * Applies POST body authentication (RFC 6749 Section 2.3.1) */ -function applyPostAuth(clientId: string, clientSecret: string | undefined, params: URLSearchParams): void { +function applyPostAuth( + clientId: string, + clientSecret: string | undefined, + params: URLSearchParams +): void { params.set("client_id", clientId); if (clientSecret) { params.set("client_secret", clientSecret); @@ -253,7 +295,9 @@ function applyPublicAuth(clientId: string, params: URLSearchParams): void { * @param input - A Response object or string containing the error response * @returns A Promise that resolves to an OAuthError instance */ -export async function parseErrorResponse(input: Response | string): Promise { +export async function parseErrorResponse( + input: Response | string +): Promise { const statusCode = input instanceof Response ? input.status : undefined; const body = input instanceof Response ? await input.text() : input; @@ -261,10 +305,12 @@ export async function parseErrorResponse(input: Response | string): Promise { - + resourceMetadataUrl?: URL; + } +): Promise { try { return await authInternal(provider, options); } catch (error) { // Handle recoverable error types by invalidating credentials and retrying - if (error instanceof InvalidClientError || error instanceof UnauthorizedClientError) { - await provider.invalidateCredentials?.('all'); + if ( + error instanceof InvalidClientError || + error instanceof UnauthorizedClientError + ) { + await provider.invalidateCredentials?.("all"); return await authInternal(provider, options); } else if (error instanceof InvalidGrantError) { - await provider.invalidateCredentials?.('tokens'); + await provider.invalidateCredentials?.("tokens"); return await authInternal(provider, options); } // Throw otherwise - throw error + throw error; } } async function authInternal( provider: OAuthClientProvider, - { serverUrl, + { + serverUrl, authorizationCode, scope, - resourceMetadataUrl + resourceMetadataUrl, }: { serverUrl: string | URL; authorizationCode?: string; scope?: string; - resourceMetadataUrl?: URL - }): Promise { - + resourceMetadataUrl?: URL; + } +): Promise { let resourceMetadata: OAuthProtectedResourceMetadata | undefined; let authorizationServerUrl = serverUrl; try { - resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }); - if (resourceMetadata.authorization_servers && resourceMetadata.authorization_servers.length > 0) { + resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl, { + resourceMetadataUrl, + }); + if ( + resourceMetadata.authorization_servers && + resourceMetadata.authorization_servers.length > 0 + ) { authorizationServerUrl = resourceMetadata.authorization_servers[0]; } } catch { // Ignore errors and fall back to /.well-known/oauth-authorization-server } - const resource: URL | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata); + const resource: URL | undefined = await selectResourceURL( + serverUrl, + provider, + resourceMetadata + ); const metadata = await discoverOAuthMetadata(serverUrl, { - authorizationServerUrl + authorizationServerUrl, }); // Handle client registration if needed let clientInformation = await Promise.resolve(provider.clientInformation()); if (!clientInformation) { if (authorizationCode !== undefined) { - throw new Error("Existing OAuth client information is required when exchanging an authorization code"); + throw new Error( + "Existing OAuth client information is required when exchanging an authorization code" + ); } if (!provider.saveClientInformation) { - throw new Error("OAuth client information must be saveable for dynamic registration"); + throw new Error( + "OAuth client information must be saveable for dynamic registration" + ); } const fullInformation = await registerClient(authorizationServerUrl, { @@ -364,7 +428,7 @@ async function authInternal( }); await provider.saveTokens(tokens); - return "AUTHORIZED" + return "AUTHORIZED"; } const tokens = await provider.tokens(); @@ -382,7 +446,7 @@ async function authInternal( }); await provider.saveTokens(newTokens); - return "AUTHORIZED" + return "AUTHORIZED"; } catch (error) { // If this is a ServerError, or an unknown type, log it out and try to continue. Otherwise, escalate so we can fix things and retry. if (!(error instanceof OAuthError) || error instanceof ServerError) { @@ -397,26 +461,36 @@ async function authInternal( const state = provider.state ? await provider.state() : undefined; // Start new authorization flow - const { authorizationUrl, codeVerifier } = await startAuthorization(authorizationServerUrl, { - metadata, - clientInformation, - state, - redirectUrl: provider.redirectUrl, - scope: scope || provider.clientMetadata.scope, - resource, - }); + const { authorizationUrl, codeVerifier } = await startAuthorization( + authorizationServerUrl, + { + metadata, + clientInformation, + state, + redirectUrl: provider.redirectUrl, + scope: scope || provider.clientMetadata.scope, + resource, + } + ); await provider.saveCodeVerifier(codeVerifier); await provider.redirectToAuthorization(authorizationUrl); - return "REDIRECT" + return "REDIRECT"; } -export async function selectResourceURL(serverUrl: string | URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise { +export async function selectResourceURL( + serverUrl: string | URL, + provider: OAuthClientProvider, + resourceMetadata?: OAuthProtectedResourceMetadata +): Promise { const defaultResource = resourceUrlFromServerUrl(serverUrl); // If provider has custom validation, delegate to it if (provider.validateResourceURL) { - return await provider.validateResourceURL(defaultResource, resourceMetadata?.resource); + return await provider.validateResourceURL( + defaultResource, + resourceMetadata?.resource + ); } // Only include resource parameter when Protected Resource Metadata is present @@ -425,8 +499,15 @@ export async function selectResourceURL(serverUrl: string | URL, provider: OAuth } // Validate that the metadata's resource is compatible with our request - if (!checkResourceAllowed({ requestedResource: defaultResource, configuredResource: resourceMetadata.resource })) { - throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${defaultResource} (or origin)`); + if ( + !checkResourceAllowed({ + requestedResource: defaultResource, + configuredResource: resourceMetadata.resource, + }) + ) { + throw new Error( + `Protected resource ${resourceMetadata.resource} does not match expected ${defaultResource} (or origin)` + ); } // Prefer the resource from metadata since it's what the server is telling us to request return new URL(resourceMetadata.resource); @@ -436,14 +517,13 @@ export async function selectResourceURL(serverUrl: string | URL, provider: OAuth * Extract resource_metadata from response header. */ export function extractResourceMetadataUrl(res: Response): URL | undefined { - const authenticateHeader = res.headers.get("WWW-Authenticate"); if (!authenticateHeader) { return undefined; } - const [type, scheme] = authenticateHeader.split(' '); - if (type.toLowerCase() !== 'bearer' || !scheme) { + const [type, scheme] = authenticateHeader.split(" "); + if (type.toLowerCase() !== "bearer" || !scheme) { return undefined; } const regex = /resource_metadata="([^"]*)"/; @@ -468,24 +548,26 @@ export function extractResourceMetadataUrl(res: Response): URL | undefined { */ export async function discoverOAuthProtectedResourceMetadata( serverUrl: string | URL, - opts?: { protocolVersion?: string, resourceMetadataUrl?: string | URL }, + opts?: { protocolVersion?: string; resourceMetadataUrl?: string | URL } ): Promise { const response = await discoverMetadataWithFallback( serverUrl, - 'oauth-protected-resource', + "oauth-protected-resource", { protocolVersion: opts?.protocolVersion, metadataUrl: opts?.resourceMetadataUrl, - }, + } ); if (!response || response.status === 404) { - throw new Error(`Resource server does not implement OAuth 2.0 Protected Resource Metadata.`); + throw new Error( + `Resource server does not implement OAuth 2.0 Protected Resource Metadata.` + ); } if (!response.ok) { throw new Error( - `HTTP ${response.status} trying to load well-known OAuth protected resource metadata.`, + `HTTP ${response.status} trying to load well-known OAuth protected resource metadata.` ); } return OAuthProtectedResourceMetadataSchema.parse(await response.json()); @@ -496,7 +578,7 @@ export async function discoverOAuthProtectedResourceMetadata( */ async function fetchWithCorsRetry( url: URL, - headers?: Record, + headers?: Record ): Promise { try { return await fetch(url, { headers }); @@ -504,10 +586,10 @@ async function fetchWithCorsRetry( if (error instanceof TypeError) { if (headers) { // CORS errors come back as TypeError, retry without headers - return fetchWithCorsRetry(url) + return fetchWithCorsRetry(url); } else { // We're getting CORS errors on retry too, return undefined - return undefined + return undefined; } } throw error; @@ -519,7 +601,7 @@ async function fetchWithCorsRetry( */ function buildWellKnownPath(wellKnownPrefix: string, pathname: string): string { let wellKnownPath = `/.well-known/${wellKnownPrefix}${pathname}`; - if (pathname.endsWith('/')) { + if (pathname.endsWith("/")) { // Strip trailing slash from pathname to avoid double slashes wellKnownPath = wellKnownPath.slice(0, -1); } @@ -531,10 +613,10 @@ function buildWellKnownPath(wellKnownPrefix: string, pathname: string): string { */ async function tryMetadataDiscovery( url: URL, - protocolVersion: string, + protocolVersion: string ): Promise { const headers = { - "MCP-Protocol-Version": protocolVersion + "MCP-Protocol-Version": protocolVersion, }; return await fetchWithCorsRetry(url, headers); } @@ -542,8 +624,11 @@ async function tryMetadataDiscovery( /** * Determines if fallback to root discovery should be attempted */ -function shouldAttemptFallback(response: Response | undefined, pathname: string): boolean { - return !response || response.status === 404 && pathname !== '/'; +function shouldAttemptFallback( + response: Response | undefined, + pathname: string +): boolean { + return !response || (response.status === 404 && pathname !== "/"); } /** @@ -551,8 +636,12 @@ function shouldAttemptFallback(response: Response | undefined, pathname: string) */ async function discoverMetadataWithFallback( serverUrl: string | URL, - wellKnownType: 'oauth-authorization-server' | 'oauth-protected-resource', - opts?: { protocolVersion?: string; metadataUrl?: string | URL, metadataServerUrl?: string | URL }, + wellKnownType: "oauth-authorization-server" | "oauth-protected-resource", + opts?: { + protocolVersion?: string; + metadataUrl?: string | URL; + metadataServerUrl?: string | URL; + } ): Promise { const issuer = new URL(serverUrl); const protocolVersion = opts?.protocolVersion ?? LATEST_PROTOCOL_VERSION; @@ -579,52 +668,185 @@ async function discoverMetadataWithFallback( } /** - * Looks up RFC 8414 OAuth 2.0 Authorization Server Metadata. + * Discovers OAuth 2.0 Authorization Server Metadata using RFC 8414 specification. * - * If the server returns a 404 for the well-known endpoint, this function will + * This function attempts to discover OAuth 2.0 metadata with path-aware discovery + * and fallback to root discovery for compatibility. + */ +async function discoverOAuth2AuthorizationServerMetadata( + issuer: URL, + protocolVersion: string +): Promise { + const hasPathname = issuer.pathname !== "/"; + + const url = hasPathname + ? new URL( + `${wellKnownPaths.oauthAuthorizationServer}${issuer.pathname}`, + issuer.origin + ) + : new URL(wellKnownPaths.oauthAuthorizationServer, issuer.origin); + + const response = await fetchWithCorsRetry(url, { + "MCP-Protocol-Version": protocolVersion, + }); + + if (response && response.ok) { + return OAuthMetadataSchema.parse(await response.json()); + } + + // Check for non-404 errors and throw them + if (response && !response.ok && response.status !== 404) { + throw new Error( + `HTTP ${ + response.status + } trying to load OAuth 2.0 Authorization Server Metadata from ${url.toString()}` + ); + } + + return undefined; +} + +/** + * Discovers OpenID Connect Provider Metadata using OIDC Discovery 1.0 specification. + * + * This function implements the three discovery URL patterns as per MCP specification: + * 1. Root discovery: /.well-known/openid-configuration + * 2. Path insertion: /.well-known/openid-configuration/path + * 3. Path appending: /path/.well-known/openid-configuration + */ +async function discoverOpenIDProviderMetadata( + issuer: URL, + protocolVersion: string +): Promise { + const hasPathname = issuer.pathname !== "/"; + + const discoverUrls = !hasPathname + ? [ + // 1. Root discovery: /.well-known/openid-configuration + new URL(wellKnownPaths.openidConfiguration, issuer.origin), + ] + : [ + // 2. Path insertion: /.well-known/openid-configuration/path + new URL( + wellKnownPaths.openidConfiguration + issuer.pathname, + issuer.origin + ), + // 3. Path appending: /path/.well-known/openid-configuration + new URL( + issuer.pathname + wellKnownPaths.openidConfiguration, + issuer.origin + ), + ]; + + for (const url of discoverUrls) { + const response = await fetchWithCorsRetry(url, { + "MCP-Protocol-Version": protocolVersion, + }); + + if (response && response.ok) { + return await parseOpenIDMetadata(response); + } + + // Check for non-404 errors and throw them + if (response && !response.ok && response.status !== 404) { + throw new Error( + `HTTP ${ + response.status + } trying to load OpenID Connect Provider Metadata from ${url.toString()}` + ); + } + } + + return undefined; +} + +/** + * Helper function to parse OpenID Provider Metadata with fallback to OAuth2 metadata + */ +async function parseOpenIDMetadata( + response: Response +): Promise { + const data = await response.json(); + try { + return OpenIdProviderMetadataSchema.parse(data); + } catch { + // If OpenID parsing fails, try OAuth2 parsing as fallback + // This handles cases where OIDC endpoint returns OAuth2-compatible metadata + return OAuthMetadataSchema.parse(data); + } +} + +/** + * Looks up authorization server metadata from an MCP-compliant server. + * + * Per the MCP specification, clients **MUST** support both OAuth 2.0 + * Authorization Server Metadata ([RFC8414](https://datatracker.ietf.org/doc/html/rfc8414)) + * and [OpenID Connect Discovery 1.0](https://openid.net/specs/openid-connect-discovery-1_0-final.html). + * This function implements this requirement by checking the well-known + * discovery endpoints for both standards. + * + * The function can parse responses from both types of endpoints because OIDC + * discovery metadata is a superset of the metadata defined in RFC 8414. + * + * If the server returns a 404 for all known endpoints, this function will * return `undefined`. Any other errors will be thrown as exceptions. */ -export async function discoverOAuthMetadata( +export async function discoverAuthorizationServerMetadata( issuer: string | URL, { authorizationServerUrl, protocolVersion, }: { - authorizationServerUrl?: string | URL, - protocolVersion?: string, - } = {}, -): Promise { - if (typeof issuer === 'string') { + authorizationServerUrl?: string | URL; + protocolVersion?: string; + } = {} +): Promise { + if (typeof issuer === "string") { issuer = new URL(issuer); } if (!authorizationServerUrl) { authorizationServerUrl = issuer; } - if (typeof authorizationServerUrl === 'string') { + if (typeof authorizationServerUrl === "string") { authorizationServerUrl = new URL(authorizationServerUrl); } protocolVersion ??= LATEST_PROTOCOL_VERSION; - const response = await discoverMetadataWithFallback( + // First try OAuth 2.0 Authorization Server Metadata discovery + const oauthMetadata = await discoverOAuth2AuthorizationServerMetadata( authorizationServerUrl, - 'oauth-authorization-server', - { - protocolVersion, - metadataServerUrl: authorizationServerUrl, - }, + protocolVersion ); - - if (!response || response.status === 404) { - return undefined; + if (oauthMetadata) { + return oauthMetadata; } - if (!response.ok) { - throw new Error( - `HTTP ${response.status} trying to load well-known OAuth metadata`, - ); - } + // If that fails, try OpenID Connect Provider discovery + return await discoverOpenIDProviderMetadata( + authorizationServerUrl, + protocolVersion + ); +} - return OAuthMetadataSchema.parse(await response.json()); +/** + * @deprecated Use discoverAuthorizationServerMetadata instead + */ +export async function discoverOAuthMetadata( + issuer: string | URL, + { + authorizationServerUrl, + protocolVersion, + }: { + authorizationServerUrl?: string | URL; + protocolVersion?: string; + } = {} +): Promise { + const result = await discoverAuthorizationServerMetadata(issuer, { + authorizationServerUrl, + protocolVersion, + }); + + return result as OAuthMetadata | undefined; } /** @@ -646,7 +868,7 @@ export async function startAuthorization( scope?: string; state?: string; resource?: URL; - }, + } ): Promise<{ authorizationUrl: URL; codeVerifier: string }> { const responseType = "code"; const codeChallengeMethod = "S256"; @@ -657,7 +879,7 @@ export async function startAuthorization( if (!metadata.response_types_supported.includes(responseType)) { throw new Error( - `Incompatible auth server: does not support response type ${responseType}`, + `Incompatible auth server: does not support response type ${responseType}` ); } @@ -666,7 +888,7 @@ export async function startAuthorization( !metadata.code_challenge_methods_supported.includes(codeChallengeMethod) ) { throw new Error( - `Incompatible auth server: does not support code challenge method ${codeChallengeMethod}`, + `Incompatible auth server: does not support code challenge method ${codeChallengeMethod}` ); } } else { @@ -683,7 +905,7 @@ export async function startAuthorization( authorizationUrl.searchParams.set("code_challenge", codeChallenge); authorizationUrl.searchParams.set( "code_challenge_method", - codeChallengeMethod, + codeChallengeMethod ); authorizationUrl.searchParams.set("redirect_uri", String(redirectUrl)); @@ -730,7 +952,7 @@ export async function exchangeAuthorization( codeVerifier, redirectUri, resource, - addClientAuthentication + addClientAuthentication, }: { metadata?: OAuthMetadata; clientInformation: OAuthClientInformation; @@ -739,20 +961,20 @@ export async function exchangeAuthorization( redirectUri: string | URL; resource?: URL; addClientAuthentication?: OAuthClientProvider["addClientAuthentication"]; - }, + } ): Promise { const grantType = "authorization_code"; const tokenUrl = metadata?.token_endpoint - ? new URL(metadata.token_endpoint) - : new URL("/token", authorizationServerUrl); + ? new URL(metadata.token_endpoint) + : new URL("/token", authorizationServerUrl); if ( - metadata?.grant_types_supported && - !metadata.grant_types_supported.includes(grantType) + metadata?.grant_types_supported && + !metadata.grant_types_supported.includes(grantType) ) { throw new Error( - `Incompatible auth server: does not support grant type ${grantType}`, + `Incompatible auth server: does not support grant type ${grantType}` ); } @@ -771,8 +993,12 @@ export async function exchangeAuthorization( addClientAuthentication(headers, params, authorizationServerUrl, metadata); } else { // Determine and apply client authentication method - const supportedMethods = metadata?.token_endpoint_auth_methods_supported ?? []; - const authMethod = selectClientAuthMethod(clientInformation, supportedMethods); + const supportedMethods = + metadata?.token_endpoint_auth_methods_supported ?? []; + const authMethod = selectClientAuthMethod( + clientInformation, + supportedMethods + ); applyClientAuthentication(authMethod, clientInformation, headers, params); } @@ -833,7 +1059,7 @@ export async function refreshAuthorization( !metadata.grant_types_supported.includes(grantType) ) { throw new Error( - `Incompatible auth server: does not support grant type ${grantType}`, + `Incompatible auth server: does not support grant type ${grantType}` ); } } else { @@ -853,8 +1079,12 @@ export async function refreshAuthorization( addClientAuthentication(headers, params, authorizationServerUrl, metadata); } else { // Determine and apply client authentication method - const supportedMethods = metadata?.token_endpoint_auth_methods_supported ?? []; - const authMethod = selectClientAuthMethod(clientInformation, supportedMethods); + const supportedMethods = + metadata?.token_endpoint_auth_methods_supported ?? []; + const authMethod = selectClientAuthMethod( + clientInformation, + supportedMethods + ); applyClientAuthentication(authMethod, clientInformation, headers, params); } @@ -872,7 +1102,10 @@ export async function refreshAuthorization( throw await parseErrorResponse(response); } - return OAuthTokensSchema.parse({ refresh_token: refreshToken, ...(await response.json()) }); + return OAuthTokensSchema.parse({ + refresh_token: refreshToken, + ...(await response.json()), + }); } /** @@ -886,13 +1119,15 @@ export async function registerClient( }: { metadata?: OAuthMetadata; clientMetadata: OAuthClientMetadata; - }, + } ): Promise { let registrationUrl: URL; if (metadata) { if (!metadata.registration_endpoint) { - throw new Error("Incompatible auth server: does not support dynamic client registration"); + throw new Error( + "Incompatible auth server: does not support dynamic client registration" + ); } registrationUrl = new URL(metadata.registration_endpoint); diff --git a/src/client/sse.test.ts b/src/client/sse.test.ts index 2cc4a1dd7..bd5075946 100644 --- a/src/client/sse.test.ts +++ b/src/client/sse.test.ts @@ -352,6 +352,11 @@ describe("SSEClientTransport", () => { }); describe("auth handling", () => { + const authServerMetadataUrls = [ + "/.well-known/oauth-authorization-server", + "/.well-known/openid-configuration", + ]; + let mockAuthProvider: jest.Mocked; beforeEach(() => { @@ -608,7 +613,7 @@ describe("SSEClientTransport", () => { authServer.close(); authServer = createServer((req, res) => { - if (req.url === "/.well-known/oauth-authorization-server") { + if (req.url && authServerMetadataUrls.includes(req.url)) { res.writeHead(404).end(); return; } @@ -730,7 +735,7 @@ describe("SSEClientTransport", () => { authServer.close(); authServer = createServer((req, res) => { - if (req.url === "/.well-known/oauth-authorization-server") { + if (req.url && authServerMetadataUrls.includes(req.url)) { res.writeHead(404).end(); return; } @@ -875,7 +880,7 @@ describe("SSEClientTransport", () => { authServer.close(); authServer = createServer((req, res) => { - if (req.url === "/.well-known/oauth-authorization-server") { + if (req.url && authServerMetadataUrls.includes(req.url)) { res.writeHead(404).end(); return; } diff --git a/src/shared/auth.ts b/src/shared/auth.ts index 467680a56..32eb63445 100644 --- a/src/shared/auth.ts +++ b/src/shared/auth.ts @@ -56,6 +56,68 @@ export const OAuthMetadataSchema = z }) .passthrough(); +/** + * OpenID Connect Discovery 1.0 Provider Metadata + * see: https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata + */ +export const OpenIdProviderMetadataSchema = z + .object({ + issuer: z.string(), + authorization_endpoint: z.string(), + token_endpoint: z.string(), + userinfo_endpoint: z.string().optional(), + jwks_uri: z.string(), + registration_endpoint: z.string().optional(), + scopes_supported: z.array(z.string()).optional(), + response_types_supported: z.array(z.string()), + response_modes_supported: z.array(z.string()).optional(), + grant_types_supported: z.array(z.string()).optional(), + acr_values_supported: z.array(z.string()).optional(), + subject_types_supported: z.array(z.string()), + id_token_signing_alg_values_supported: z.array(z.string()), + id_token_encryption_alg_values_supported: z.array(z.string()).optional(), + id_token_encryption_enc_values_supported: z.array(z.string()).optional(), + userinfo_signing_alg_values_supported: z.array(z.string()).optional(), + userinfo_encryption_alg_values_supported: z.array(z.string()).optional(), + userinfo_encryption_enc_values_supported: z.array(z.string()).optional(), + request_object_signing_alg_values_supported: z.array(z.string()).optional(), + request_object_encryption_alg_values_supported: z + .array(z.string()) + .optional(), + request_object_encryption_enc_values_supported: z + .array(z.string()) + .optional(), + token_endpoint_auth_methods_supported: z.array(z.string()).optional(), + token_endpoint_auth_signing_alg_values_supported: z + .array(z.string()) + .optional(), + display_values_supported: z.array(z.string()).optional(), + claim_types_supported: z.array(z.string()).optional(), + claims_supported: z.array(z.string()).optional(), + service_documentation: z.string().optional(), + claims_locales_supported: z.array(z.string()).optional(), + ui_locales_supported: z.array(z.string()).optional(), + claims_parameter_supported: z.boolean().optional(), + request_parameter_supported: z.boolean().optional(), + request_uri_parameter_supported: z.boolean().optional(), + require_request_uri_registration: z.boolean().optional(), + op_policy_uri: z.string().optional(), + op_tos_uri: z.string().optional(), + }) + .passthrough(); + +/** + * OpenID Connect Discovery metadata that may include OAuth 2.0 fields + * This schema represents the real-world scenario where OIDC providers + * return a mix of OpenID Connect and OAuth 2.0 metadata fields + */ +export const OpenIdProviderDiscoveryMetadataSchema = + OpenIdProviderMetadataSchema.merge( + OAuthMetadataSchema.pick({ + code_challenge_methods_supported: true, + }) + ); + /** * OAuth 2.1 token response */ @@ -73,73 +135,108 @@ export const OAuthTokensSchema = z /** * OAuth 2.1 error response */ -export const OAuthErrorResponseSchema = z - .object({ - error: z.string(), - error_description: z.string().optional(), - error_uri: z.string().optional(), - }); +export const OAuthErrorResponseSchema = z.object({ + error: z.string(), + error_description: z.string().optional(), + error_uri: z.string().optional(), +}); /** * RFC 7591 OAuth 2.0 Dynamic Client Registration metadata */ -export const OAuthClientMetadataSchema = z.object({ - redirect_uris: z.array(z.string()).refine((uris) => uris.every((uri) => URL.canParse(uri)), { message: "redirect_uris must contain valid URLs" }), - token_endpoint_auth_method: z.string().optional(), - grant_types: z.array(z.string()).optional(), - response_types: z.array(z.string()).optional(), - client_name: z.string().optional(), - client_uri: z.string().optional(), - logo_uri: z.string().optional(), - scope: z.string().optional(), - contacts: z.array(z.string()).optional(), - tos_uri: z.string().optional(), - policy_uri: z.string().optional(), - jwks_uri: z.string().optional(), - jwks: z.any().optional(), - software_id: z.string().optional(), - software_version: z.string().optional(), - software_statement: z.string().optional(), -}).strip(); +export const OAuthClientMetadataSchema = z + .object({ + redirect_uris: z + .array(z.string()) + .refine((uris) => uris.every((uri) => URL.canParse(uri)), { + message: "redirect_uris must contain valid URLs", + }), + token_endpoint_auth_method: z.string().optional(), + grant_types: z.array(z.string()).optional(), + response_types: z.array(z.string()).optional(), + client_name: z.string().optional(), + client_uri: z.string().optional(), + logo_uri: z.string().optional(), + scope: z.string().optional(), + contacts: z.array(z.string()).optional(), + tos_uri: z.string().optional(), + policy_uri: z.string().optional(), + jwks_uri: z.string().optional(), + jwks: z.any().optional(), + software_id: z.string().optional(), + software_version: z.string().optional(), + software_statement: z.string().optional(), + }) + .strip(); /** * RFC 7591 OAuth 2.0 Dynamic Client Registration client information */ -export const OAuthClientInformationSchema = z.object({ - client_id: z.string(), - client_secret: z.string().optional(), - client_id_issued_at: z.number().optional(), - client_secret_expires_at: z.number().optional(), -}).strip(); +export const OAuthClientInformationSchema = z + .object({ + client_id: z.string(), + client_secret: z.string().optional(), + client_id_issued_at: z.number().optional(), + client_secret_expires_at: z.number().optional(), + }) + .strip(); /** * RFC 7591 OAuth 2.0 Dynamic Client Registration full response (client information plus metadata) */ -export const OAuthClientInformationFullSchema = OAuthClientMetadataSchema.merge(OAuthClientInformationSchema); +export const OAuthClientInformationFullSchema = OAuthClientMetadataSchema.merge( + OAuthClientInformationSchema +); /** * RFC 7591 OAuth 2.0 Dynamic Client Registration error response */ -export const OAuthClientRegistrationErrorSchema = z.object({ - error: z.string(), - error_description: z.string().optional(), -}).strip(); +export const OAuthClientRegistrationErrorSchema = z + .object({ + error: z.string(), + error_description: z.string().optional(), + }) + .strip(); /** * RFC 7009 OAuth 2.0 Token Revocation request */ -export const OAuthTokenRevocationRequestSchema = z.object({ - token: z.string(), - token_type_hint: z.string().optional(), -}).strip(); - +export const OAuthTokenRevocationRequestSchema = z + .object({ + token: z.string(), + token_type_hint: z.string().optional(), + }) + .strip(); export type OAuthMetadata = z.infer; +export type OpenIdProviderMetadata = z.infer< + typeof OpenIdProviderMetadataSchema +>; + +export type OpenIdProviderDiscoveryMetadata = z.infer< + typeof OpenIdProviderDiscoveryMetadataSchema +>; + export type OAuthTokens = z.infer; export type OAuthErrorResponse = z.infer; export type OAuthClientMetadata = z.infer; -export type OAuthClientInformation = z.infer; -export type OAuthClientInformationFull = z.infer; -export type OAuthClientRegistrationError = z.infer; -export type OAuthTokenRevocationRequest = z.infer; -export type OAuthProtectedResourceMetadata = z.infer; +export type OAuthClientInformation = z.infer< + typeof OAuthClientInformationSchema +>; +export type OAuthClientInformationFull = z.infer< + typeof OAuthClientInformationFullSchema +>; +export type OAuthClientRegistrationError = z.infer< + typeof OAuthClientRegistrationErrorSchema +>; +export type OAuthTokenRevocationRequest = z.infer< + typeof OAuthTokenRevocationRequestSchema +>; +export type OAuthProtectedResourceMetadata = z.infer< + typeof OAuthProtectedResourceMetadataSchema +>; + +// Unified type for authorization server metadata +export type AuthorizationServerMetadata = + | OAuthMetadata + | OpenIdProviderDiscoveryMetadata; From 5375f46c5477f79c5840ad3d6e12d8c4b546431e Mon Sep 17 00:00:00 2001 From: Xiao Yijun Date: Sun, 20 Jul 2025 00:32:02 +0800 Subject: [PATCH 02/16] feat: apply draft spec --- src/client/auth.ts | 264 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 248 insertions(+), 16 deletions(-) diff --git a/src/client/auth.ts b/src/client/auth.ts index b5a3a6a43..185d42efd 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -7,7 +7,10 @@ import { OAuthMetadata, OAuthClientInformationFull, OAuthProtectedResourceMetadata, - OAuthErrorResponseSchema + OAuthErrorResponseSchema, + OpenIdProviderDiscoveryMetadata, + AuthorizationServerMetadata, + OpenIdProviderDiscoveryMetadataSchema } from "../shared/auth.js"; import { OAuthClientInformationFullSchema, OAuthMetadataSchema, OAuthProtectedResourceMetadataSchema, OAuthTokensSchema } from "../shared/auth.js"; import { checkResourceAllowed, resourceUrlFromServerUrl } from "../shared/auth-utils.js"; @@ -108,7 +111,7 @@ export interface OAuthClientProvider { * @param url - The token endpoint URL being called * @param metadata - Optional OAuth metadata for the server, which may include supported authentication methods */ - addClientAuthentication?(headers: Headers, params: URLSearchParams, url: string | URL, metadata?: OAuthMetadata): void | Promise; + addClientAuthentication?(headers: Headers, params: URLSearchParams, url: string | URL, metadata?: AuthorizationServerMetadata): void | Promise; /** * If defined, overrides the selection and validation of the @@ -319,7 +322,7 @@ async function authInternal( ): Promise { let resourceMetadata: OAuthProtectedResourceMetadata | undefined; - let authorizationServerUrl = serverUrl; + let authorizationServerUrl: string | URL | undefined; try { resourceMetadata = await discoverOAuthProtectedResourceMetadata(serverUrl, { resourceMetadataUrl }, fetchFn); if (resourceMetadata.authorization_servers && resourceMetadata.authorization_servers.length > 0) { @@ -331,9 +334,17 @@ async function authInternal( const resource: URL | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata); - const metadata = await discoverOAuthMetadata(serverUrl, { - authorizationServerUrl - }, fetchFn); + const metadata = await discoverAuthorizationServerMetadata(serverUrl, authorizationServerUrl, { + fetchFn, + }); + + /** + * If we don't get a valid authorization server metadata from protected resource metadata, + * fallback to the legacy MCP spec's implementation (version 2025-03-26): MCP server acts as the Authorization server. + */ + if (!authorizationServerUrl) { + authorizationServerUrl = serverUrl; + } // Handle client registration if needed let clientInformation = await Promise.resolve(provider.clientInformation()); @@ -524,15 +535,21 @@ async function fetchWithCorsRetry( } /** - * Constructs the well-known path for OAuth metadata discovery + * Constructs the well-known path for auth-related metadata discovery */ -function buildWellKnownPath(wellKnownPrefix: string, pathname: string): string { - let wellKnownPath = `/.well-known/${wellKnownPrefix}${pathname}`; +function buildWellKnownPath( + wellKnownPrefix: 'oauth-authorization-server' | 'oauth-protected-resource' | 'openid-configuration', + pathname: string = '', + options: { prependPathname?: boolean } = {} +): string { + // Strip trailing slash from pathname to avoid double slashes if (pathname.endsWith('/')) { - // Strip trailing slash from pathname to avoid double slashes - wellKnownPath = wellKnownPath.slice(0, -1); + pathname = pathname.slice(0, -1); } - return wellKnownPath; + + return options.prependPathname + ? `${pathname}/.well-known/${wellKnownPrefix}` + : `/.well-known/${wellKnownPrefix}${pathname}`; } /** @@ -594,6 +611,8 @@ async function discoverMetadataWithFallback( * * If the server returns a 404 for the well-known endpoint, this function will * return `undefined`. Any other errors will be thrown as exceptions. + * + * @deprecated This function is deprecated in favor of `discoverAuthorizationServerMetadata`. */ export async function discoverOAuthMetadata( issuer: string | URL, @@ -640,6 +659,219 @@ export async function discoverOAuthMetadata( return OAuthMetadataSchema.parse(await response.json()); } +/** + * Discovers authorization server metadata with support for RFC 8414 OAuth 2.0 Authorization Server Metadata + * and OpenID Connect Discovery 1.0 specifications. + * + * This function implements a fallback strategy for authorization server discovery: + * 1. If `authorizationServerUrl` is provided, attempts RFC 8414 OAuth metadata discovery first + * 2. If OAuth discovery fails, falls back to OpenID Connect Discovery + * 3. If `authorizationServerUrl` is not provided, uses legacy MCP specification behavior + * + * @param serverUrl - The MCP Server URL, used for legacy specification support where the MCP server + * acts as both the resource server and authorization server + * @param authorizationServerUrl - The authorization server URL obtained from the MCP Server's + * protected resource metadata. If this parameter is `undefined`, + * it indicates that protected resource metadata was not successfully + * retrieved, triggering legacy fallback behavior + * @param options - Configuration options + * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch + * @param options.protocolVersion - MCP protocol version to use, defaults to LATEST_PROTOCOL_VERSION + * @returns Promise resolving to authorization server metadata, or undefined if discovery fails + */ +export async function discoverAuthorizationServerMetadata( + serverUrl: string | URL, + authorizationServerUrl?: string | URL, + { + fetchFn = fetch, + protocolVersion = LATEST_PROTOCOL_VERSION, + }: { + fetchFn?: FetchLike; + protocolVersion?: string; + } = {} +): Promise { + if (!authorizationServerUrl) { + // Legacy support: MCP servers act as the Auth server. + return retrieveOAuthMetadataFromMcpServer(serverUrl, { + fetchFn, + protocolVersion, + }); + } + + const oauthMetadata = await retrieveOAuthMetadataFromAuthorizationServer(authorizationServerUrl, { + fetchFn, + protocolVersion, + }); + + if (oauthMetadata) { + return oauthMetadata; + } + + return retrieveOpenIdProviderMetadataFromAuthorizationServer(authorizationServerUrl, { + fetchFn, + protocolVersion, + }); +} + +/** + * Legacy implementation where the MCP server acts as the Auth server. + * According to MCP spec version 2025-03-26. + * + * @param serverUrl - The MCP Server URL + * @param options - Configuration options + * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch + * @param options.protocolVersion - MCP protocol version to use (required) + * @returns Promise resolving to OAuth metadata + */ +async function retrieveOAuthMetadataFromMcpServer( + serverUrl: string | URL, + { + fetchFn = fetch, + protocolVersion, + }: { + fetchFn?: FetchLike; + protocolVersion: string; + } +): Promise { + const serverOrigin = typeof serverUrl === 'string' ? new URL(serverUrl).origin : serverUrl.origin; + + const metadataEndpoint = new URL(buildWellKnownPath('oauth-authorization-server'), serverOrigin); + + const response = await fetchWithCorsRetry(metadataEndpoint, getProtocolVersionHeader(protocolVersion), fetchFn); + + if (!response) { + throw new Error(`CORS error trying to load OAuth metadata from ${metadataEndpoint}`); + } + + if (!response.ok) { + if (response.status === 404) { + /** + * The MCP server does not implement OAuth 2.0 Authorization Server Metadata + * + * Return fallback OAuth 2.0 Authorization Server Metadata + */ + return { + issuer: serverOrigin, + authorization_endpoint: new URL('/authorize', serverOrigin).href, + token_endpoint: new URL('/token', serverOrigin).href, + registration_endpoint: new URL('/register', serverOrigin).href, + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'], + }; + } + + throw new Error(`HTTP ${response.status} trying to load OAuth metadata from ${metadataEndpoint}`); + } + + return OAuthMetadataSchema.parse(await response.json()); +} + +/** + * Retrieves RFC 8414 OAuth 2.0 Authorization Server Metadata from the authorization server. + * + * @param authorizationServerUrl - The authorization server URL + * @param options - Configuration options + * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch + * @param options.protocolVersion - MCP protocol version to use (required) + * @returns Promise resolving to OAuth metadata, or undefined if discovery fails + */ +async function retrieveOAuthMetadataFromAuthorizationServer( + authorizationServerUrl: string | URL, + { + fetchFn = fetch, + protocolVersion, + }: { + fetchFn?: FetchLike; + protocolVersion: string; + } +): Promise { + const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl; + + const hasPath = url.pathname !== '/'; + + const metadataEndpoint = new URL( + buildWellKnownPath('oauth-authorization-server', hasPath ? url.pathname : ''), + url.origin + ); + + const response = await fetchWithCorsRetry(metadataEndpoint, getProtocolVersionHeader(protocolVersion), fetchFn); + + if (!response) { + throw new Error(`CORS error trying to load OAuth metadata from ${metadataEndpoint}`); + } + + if (!response.ok) { + if (response.status === 404) { + return undefined; + } + + throw new Error(`HTTP ${response.status} trying to load OAuth metadata from ${metadataEndpoint}`); + } + + return OAuthMetadataSchema.parse(await response.json()); +} + +/** + * Retrieves OpenID Connect Discovery 1.0 metadata from the authorization server. + * + * @param authorizationServerUrl - The authorization server URL + * @param options - Configuration options + * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch + * @param options.protocolVersion - MCP protocol version to use (required) + * @returns Promise resolving to OpenID provider metadata, or undefined if discovery fails + */ +async function retrieveOpenIdProviderMetadataFromAuthorizationServer( + authorizationServerUrl: string | URL, + { + fetchFn = fetch, + protocolVersion, + }: { + fetchFn?: FetchLike; + protocolVersion: string; + } +): Promise { + const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl; + const hasPath = url.pathname !== '/'; + + const potentialMetadataEndpoints = hasPath + ? [ + // https://example.com/.well-known/openid-configuration/tenant1 + new URL(buildWellKnownPath('openid-configuration', url.pathname), url.origin), + // https://example.com/tenant1/.well-known/openid-configuration + new URL(buildWellKnownPath('openid-configuration', url.pathname, { prependPathname: true }), `${url.origin}`), + ] + : [ + // https://example.com/.well-known/openid-configuration + new URL(buildWellKnownPath('openid-configuration'), url.origin), + ]; + + for (const endpoint of potentialMetadataEndpoints) { + const response = await fetchWithCorsRetry(endpoint, getProtocolVersionHeader(protocolVersion), fetchFn); + + if (!response) { + throw new Error(`CORS error trying to load OpenID provider metadata from ${endpoint}`); + } + + if (!response.ok) { + if (response.status === 404) { + continue; + } + + throw new Error(`HTTP ${response.status} trying to load OpenID provider metadata from ${endpoint}`); + } + + return OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); + } + + return undefined; +} + +function getProtocolVersionHeader(protocolVersion: string): Record { + return { + 'MCP-Protocol-Version': protocolVersion, + }; +} + /** * Begins the authorization flow with the given server, by generating a PKCE challenge and constructing the authorization URL. */ @@ -653,7 +885,7 @@ export async function startAuthorization( state, resource, }: { - metadata?: OAuthMetadata; + metadata?: AuthorizationServerMetadata; clientInformation: OAuthClientInformation; redirectUrl: string | URL; scope?: string; @@ -746,7 +978,7 @@ export async function exchangeAuthorization( addClientAuthentication, fetchFn, }: { - metadata?: OAuthMetadata; + metadata?: AuthorizationServerMetadata; clientInformation: OAuthClientInformation; authorizationCode: string; codeVerifier: string; @@ -831,7 +1063,7 @@ export async function refreshAuthorization( addClientAuthentication, fetchFn, }: { - metadata?: OAuthMetadata; + metadata?: AuthorizationServerMetadata; clientInformation: OAuthClientInformation; refreshToken: string; resource?: URL; @@ -902,7 +1134,7 @@ export async function registerClient( clientMetadata, fetchFn, }: { - metadata?: OAuthMetadata; + metadata?: AuthorizationServerMetadata; clientMetadata: OAuthClientMetadata; fetchFn?: FetchLike; }, From 997b85074b9ef3f166a8fe16ff74ab0f60693f80 Mon Sep 17 00:00:00 2001 From: Xiao Yijun Date: Sun, 20 Jul 2025 00:57:17 +0800 Subject: [PATCH 03/16] chore: revert tests --- src/client/auth.test.ts | 120 ++++++++-------------------------------- 1 file changed, 22 insertions(+), 98 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 48b747f2e..b0ea8d1e8 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -388,7 +388,7 @@ describe("OAuth Authorization", () => { code_challenge_methods_supported: ["S256"], }; - it("returns metadata when oauth-authorization-server discovery succeeds", async () => { + it("returns metadata when discovery succeeds", async () => { mockFetch.mockResolvedValueOnce({ ok: true, status: 200, @@ -406,28 +406,6 @@ describe("OAuth Authorization", () => { }); }); - it("returns metadata when oidc discovery succeeds", async () => { - mockFetch.mockImplementation((url) => { - if (url.toString().includes('openid-configuration')) { - return Promise.resolve({ - ok: true, - status: 200, - json: async () => validMetadata, - }); - } - return Promise.resolve({ - ok: false, - status: 404, - }); - }); - - const metadata = await discoverOAuthMetadata("https://auth.example.com"); - expect(metadata).toEqual(validMetadata); - expect(mockFetch).toHaveBeenCalledTimes(2); - expect(mockFetch.mock.calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); - expect(mockFetch.mock.calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration"); - }); - it("returns metadata when discovery succeeds with path", async () => { mockFetch.mockResolvedValueOnce({ ok: true, @@ -446,14 +424,14 @@ describe("OAuth Authorization", () => { }); }); - it("tries discovery endpoints in new spec order for URLs with path", async () => { - // First call (OAuth with path insertion) returns 404 + it("falls back to root discovery when path-aware discovery returns 404", async () => { + // First call (path-aware) returns 404 mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); - // Second call (OIDC with path insertion) succeeds + // Second call (root fallback) succeeds mockFetch.mockResolvedValueOnce({ ok: true, status: 200, @@ -466,35 +444,29 @@ describe("OAuth Authorization", () => { const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); - // First call should be OAuth with path insertion + // First call should be path-aware const [firstUrl, firstOptions] = calls[0]; expect(firstUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/path/name"); expect(firstOptions.headers).toEqual({ "MCP-Protocol-Version": LATEST_PROTOCOL_VERSION }); - // Second call should be OIDC with path insertion + // Second call should be root fallback const [secondUrl, secondOptions] = calls[1]; - expect(secondUrl.toString()).toBe("https://auth.example.com/.well-known/openid-configuration/path/name"); + expect(secondUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); expect(secondOptions.headers).toEqual({ "MCP-Protocol-Version": LATEST_PROTOCOL_VERSION }); }); - it("returns undefined when all discovery endpoints return 404", async () => { - // First call (OAuth with path insertion) returns 404 - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - // Second call (OIDC with path insertion) returns 404 + it("returns undefined when both path-aware and root discovery return 404", async () => { + // First call (path-aware) returns 404 mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); - // Third call (OIDC with path appending) returns 404 + // Second call (root fallback) also returns 404 mockFetch.mockResolvedValueOnce({ ok: false, status: 404, @@ -504,33 +476,7 @@ describe("OAuth Authorization", () => { expect(metadata).toBeUndefined(); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(3); - }); - - it("tries all endpoints in correct order for URLs with path", async () => { - // All calls return 404 to test the order - mockFetch.mockResolvedValue({ - ok: false, - status: 404, - }); - - const metadata = await discoverOAuthMetadata("https://auth.example.com/tenant1"); - expect(metadata).toBeUndefined(); - - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(3); - - // First call should be OAuth 2.0 Authorization Server Metadata with path insertion - const [firstUrl] = calls[0]; - expect(firstUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - - // Second call should be OpenID Connect Discovery 1.0 with path insertion - const [secondUrl] = calls[1]; - expect(secondUrl.toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); - - // Third call should be OpenID Connect Discovery 1.0 path appending - const [thirdUrl] = calls[2]; - expect(thirdUrl.toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration"); + expect(calls.length).toBe(2); }); it("does not fallback when the original URL is already at root path", async () => { @@ -540,17 +486,11 @@ describe("OAuth Authorization", () => { status: 404, }); - // Second call (OIDC discovery) also returns 404 - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - const metadata = await discoverOAuthMetadata("https://auth.example.com/"); expect(metadata).toBeUndefined(); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(2); // Should not attempt fallback but will try OIDC + expect(calls.length).toBe(1); // Should not attempt fallback const [url] = calls[0]; expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); @@ -563,42 +503,27 @@ describe("OAuth Authorization", () => { status: 404, }); - // Second call (OIDC discovery) also returns 404 - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - const metadata = await discoverOAuthMetadata("https://auth.example.com"); expect(metadata).toBeUndefined(); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(2); // Should not attempt fallback but will try OIDC + expect(calls.length).toBe(1); // Should not attempt fallback const [url] = calls[0]; expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); }); - it("tries all endpoints when discovery encounters CORS error", async () => { - // First call (OAuth with path insertion) fails with TypeError (CORS) - mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError("CORS error"))); - - // Retry OAuth with path insertion without headers (simulating CORS retry) - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - // Second call (OIDC with path insertion) fails with TypeError (CORS) + it("falls back when path-aware discovery encounters CORS error", async () => { + // First call (path-aware) fails with TypeError (CORS) mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError("CORS error"))); - // Retry OIDC with path insertion without headers (simulating CORS retry) + // Retry path-aware without headers (simulating CORS retry) mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); - // Third call (OIDC with path appending) succeeds + // Second call (root fallback) succeeds mockFetch.mockResolvedValueOnce({ ok: true, status: 200, @@ -609,11 +534,11 @@ describe("OAuth Authorization", () => { expect(metadata).toEqual(validMetadata); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(5); + expect(calls.length).toBe(3); - // Final call should be OIDC with path appending - const [lastUrl, lastOptions] = calls[4]; - expect(lastUrl.toString()).toBe("https://auth.example.com/deep/path/.well-known/openid-configuration"); + // Final call should be root fallback + const [lastUrl, lastOptions] = calls[2]; + expect(lastUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); expect(lastOptions.headers).toEqual({ "MCP-Protocol-Version": LATEST_PROTOCOL_VERSION }); @@ -691,14 +616,13 @@ describe("OAuth Authorization", () => { }); it("returns undefined when discovery endpoint returns 404", async () => { - mockFetch.mockResolvedValue({ + mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); const metadata = await discoverOAuthMetadata("https://auth.example.com"); expect(metadata).toBeUndefined(); - expect(mockFetch).toHaveBeenCalledTimes(2); }); it("throws on non-404 errors", async () => { From 69619b9385cc60c12a5e1ffd26d3cefde9358730 Mon Sep 17 00:00:00 2001 From: Xiao Yijun Date: Sun, 20 Jul 2025 01:01:28 +0800 Subject: [PATCH 04/16] test: add unit tests --- .claude/settings.local.json | 11 -- src/client/auth.test.ts | 303 +++++++++++++++++++++++++++++++++++- 2 files changed, 300 insertions(+), 14 deletions(-) delete mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json deleted file mode 100644 index 0609f59bf..000000000 --- a/.claude/settings.local.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "permissions": { - "allow": [ - "Bash(npm test:*)", - "Bash(npm run lint)", - "WebFetch(domain:github.com)", - "WebFetch(domain:openid.net)" - ], - "deny": [] - } -} \ No newline at end of file diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index b0ea8d1e8..9aca72216 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -1,6 +1,7 @@ import { LATEST_PROTOCOL_VERSION } from '../types.js'; import { discoverOAuthMetadata, + discoverAuthorizationServerMetadata, startAuthorization, exchangeAuthorization, refreshAuthorization, @@ -11,7 +12,7 @@ import { type OAuthClientProvider, } from "./auth.js"; import {ServerError} from "../server/auth/errors.js"; -import { OAuthMetadata } from '../shared/auth.js'; +import { AuthorizationServerMetadata } from '../shared/auth.js'; // Mock fetch globally const mockFetch = jest.fn(); @@ -683,6 +684,302 @@ describe("OAuth Authorization", () => { }); }); + describe("discoverAuthorizationServerMetadata", () => { + const validOAuthMetadata = { + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/authorize", + token_endpoint: "https://auth.example.com/token", + registration_endpoint: "https://auth.example.com/register", + response_types_supported: ["code"], + code_challenge_methods_supported: ["S256"], + }; + + const validOpenIdMetadata = { + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/authorize", + token_endpoint: "https://auth.example.com/token", + jwks_uri: "https://auth.example.com/jwks", + subject_types_supported: ["public"], + id_token_signing_alg_values_supported: ["RS256"], + response_types_supported: ["code"], + code_challenge_methods_supported: ["S256"], + }; + + it("returns OAuth metadata when authorizationServerUrl is provided and OAuth discovery succeeds", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOAuthMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com" + ); + + expect(metadata).toEqual(validOAuthMetadata); + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(1); + const [url, options] = calls[0]; + expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + expect(options.headers).toEqual({ + "MCP-Protocol-Version": LATEST_PROTOCOL_VERSION + }); + }); + + it("falls back to OpenID Connect discovery when OAuth discovery fails", async () => { + // First call (OAuth) returns 404 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // Second call (OpenID Connect) succeeds + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOpenIdMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com" + ); + + expect(metadata).toEqual(validOpenIdMetadata); + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(2); + + // First call should be OAuth discovery + expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + + // Second call should be OpenID Connect discovery + expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration"); + }); + + it("returns undefined when authorizationServerUrl is provided but both discoveries fail", async () => { + // Both calls return 404 + mockFetch.mockResolvedValue({ + ok: false, + status: 404, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com" + ); + + expect(metadata).toBeUndefined(); + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(2); + }); + + it("handles authorization server URL with path in OAuth discovery", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOAuthMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com/tenant1" + ); + + expect(metadata).toEqual(validOAuthMetadata); + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(1); + const [url] = calls[0]; + expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); + }); + + it("handles authorization server URL with path in OpenID Connect discovery", async () => { + // OAuth discovery fails + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // OpenID Connect discovery succeeds with path insertion + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOpenIdMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com/tenant1" + ); + + expect(metadata).toEqual(validOpenIdMetadata); + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(2); + + // First call should be OAuth with path + expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); + + // Second call should be OpenID Connect with path insertion + expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); + }); + + it("tries multiple OpenID Connect endpoints when path is present", async () => { + // OAuth discovery fails + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // First OpenID Connect attempt (path insertion) fails + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // Second OpenID Connect attempt (path prepending) succeeds + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOpenIdMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com/tenant1" + ); + + expect(metadata).toEqual(validOpenIdMetadata); + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(3); + + // First call should be OAuth with path + expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); + + // Second call should be OpenID Connect with path insertion + expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); + + // Third call should be OpenID Connect with path prepending + expect(calls[2][0].toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration"); + }); + + it("falls back to legacy MCP server when authorizationServerUrl is undefined", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOAuthMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + undefined + ); + + expect(metadata).toEqual(validOAuthMetadata); + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(1); + const [url] = calls[0]; + expect(url.toString()).toBe("https://mcp.example.com/.well-known/oauth-authorization-server"); + }); + + it("returns fallback metadata when legacy MCP server returns 404", async () => { + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + undefined + ); + + expect(metadata).toEqual({ + issuer: "https://mcp.example.com", + authorization_endpoint: "https://mcp.example.com/authorize", + token_endpoint: "https://mcp.example.com/token", + registration_endpoint: "https://mcp.example.com/register", + response_types_supported: ["code"], + code_challenge_methods_supported: ["S256"], + }); + }); + + it("throws on non-404 errors in legacy mode", async () => { + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 500, + }); + + await expect( + discoverAuthorizationServerMetadata("https://mcp.example.com", undefined) + ).rejects.toThrow("HTTP 500"); + }); + + it("handles CORS errors with retry", async () => { + // First call fails with CORS + mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError("CORS error"))); + + // Retry without headers succeeds + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOAuthMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com" + ); + + expect(metadata).toEqual(validOAuthMetadata); + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(2); + + // First call should have headers + expect(calls[0][1]?.headers).toHaveProperty("MCP-Protocol-Version"); + + // Second call should not have headers (CORS retry) + expect(calls[1][1]?.headers).toBeUndefined(); + }); + + it("supports custom fetch function", async () => { + const customFetch = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => validOAuthMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com", + { fetchFn: customFetch } + ); + + expect(metadata).toEqual(validOAuthMetadata); + expect(customFetch).toHaveBeenCalledTimes(1); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("supports custom protocol version", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOAuthMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com", + { protocolVersion: "2025-01-01" } + ); + + expect(metadata).toEqual(validOAuthMetadata); + const calls = mockFetch.mock.calls; + const [, options] = calls[0]; + expect(options.headers).toEqual({ + "MCP-Protocol-Version": "2025-01-01" + }); + }); + }); + describe("startAuthorization", () => { const validMetadata = { issuer: "https://auth.example.com", @@ -909,7 +1206,7 @@ describe("OAuth Authorization", () => { authorizationCode: "code123", codeVerifier: "verifier123", redirectUri: "http://localhost:3000/callback", - addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL, metadata: OAuthMetadata) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL, metadata: AuthorizationServerMetadata) => { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); params.set("example_url", typeof url === 'string' ? url : url.toString()); params.set("example_metadata", metadata.authorization_endpoint); @@ -1091,7 +1388,7 @@ describe("OAuth Authorization", () => { metadata: validMetadata, clientInformation: validClientInfo, refreshToken: "refresh123", - addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL, metadata?: OAuthMetadata) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL, metadata?: AuthorizationServerMetadata) => { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); params.set("example_url", typeof url === 'string' ? url : url.toString()); params.set("example_metadata", metadata?.authorization_endpoint ?? '?'); From c148ef1f297bb811bd250ac5ad73baa877dd2037 Mon Sep 17 00:00:00 2001 From: Xiao Yijun Date: Sun, 20 Jul 2025 01:28:26 +0800 Subject: [PATCH 05/16] chore: revert format changes --- src/shared/auth.ts | 124 +++++++++++++++++---------------------------- 1 file changed, 47 insertions(+), 77 deletions(-) diff --git a/src/shared/auth.ts b/src/shared/auth.ts index 32eb63445..47eba9ac5 100644 --- a/src/shared/auth.ts +++ b/src/shared/auth.ts @@ -135,108 +135,78 @@ export const OAuthTokensSchema = z /** * OAuth 2.1 error response */ -export const OAuthErrorResponseSchema = z.object({ - error: z.string(), - error_description: z.string().optional(), - error_uri: z.string().optional(), -}); +export const OAuthErrorResponseSchema = z + .object({ + error: z.string(), + error_description: z.string().optional(), + error_uri: z.string().optional(), + }); /** * RFC 7591 OAuth 2.0 Dynamic Client Registration metadata */ -export const OAuthClientMetadataSchema = z - .object({ - redirect_uris: z - .array(z.string()) - .refine((uris) => uris.every((uri) => URL.canParse(uri)), { - message: "redirect_uris must contain valid URLs", - }), - token_endpoint_auth_method: z.string().optional(), - grant_types: z.array(z.string()).optional(), - response_types: z.array(z.string()).optional(), - client_name: z.string().optional(), - client_uri: z.string().optional(), - logo_uri: z.string().optional(), - scope: z.string().optional(), - contacts: z.array(z.string()).optional(), - tos_uri: z.string().optional(), - policy_uri: z.string().optional(), - jwks_uri: z.string().optional(), - jwks: z.any().optional(), - software_id: z.string().optional(), - software_version: z.string().optional(), - software_statement: z.string().optional(), - }) - .strip(); +export const OAuthClientMetadataSchema = z.object({ + redirect_uris: z.array(z.string()).refine((uris) => uris.every((uri) => URL.canParse(uri)), { message: "redirect_uris must contain valid URLs" }), + token_endpoint_auth_method: z.string().optional(), + grant_types: z.array(z.string()).optional(), + response_types: z.array(z.string()).optional(), + client_name: z.string().optional(), + client_uri: z.string().optional(), + logo_uri: z.string().optional(), + scope: z.string().optional(), + contacts: z.array(z.string()).optional(), + tos_uri: z.string().optional(), + policy_uri: z.string().optional(), + jwks_uri: z.string().optional(), + jwks: z.any().optional(), + software_id: z.string().optional(), + software_version: z.string().optional(), + software_statement: z.string().optional(), +}).strip(); /** * RFC 7591 OAuth 2.0 Dynamic Client Registration client information */ -export const OAuthClientInformationSchema = z - .object({ - client_id: z.string(), - client_secret: z.string().optional(), - client_id_issued_at: z.number().optional(), - client_secret_expires_at: z.number().optional(), - }) - .strip(); +export const OAuthClientInformationSchema = z.object({ + client_id: z.string(), + client_secret: z.string().optional(), + client_id_issued_at: z.number().optional(), + client_secret_expires_at: z.number().optional(), +}).strip(); /** * RFC 7591 OAuth 2.0 Dynamic Client Registration full response (client information plus metadata) */ -export const OAuthClientInformationFullSchema = OAuthClientMetadataSchema.merge( - OAuthClientInformationSchema -); +export const OAuthClientInformationFullSchema = OAuthClientMetadataSchema.merge(OAuthClientInformationSchema); /** * RFC 7591 OAuth 2.0 Dynamic Client Registration error response */ -export const OAuthClientRegistrationErrorSchema = z - .object({ - error: z.string(), - error_description: z.string().optional(), - }) - .strip(); +export const OAuthClientRegistrationErrorSchema = z.object({ + error: z.string(), + error_description: z.string().optional(), +}).strip(); /** * RFC 7009 OAuth 2.0 Token Revocation request */ -export const OAuthTokenRevocationRequestSchema = z - .object({ - token: z.string(), - token_type_hint: z.string().optional(), - }) - .strip(); +export const OAuthTokenRevocationRequestSchema = z.object({ + token: z.string(), + token_type_hint: z.string().optional(), +}).strip(); export type OAuthMetadata = z.infer; -export type OpenIdProviderMetadata = z.infer< - typeof OpenIdProviderMetadataSchema ->; - -export type OpenIdProviderDiscoveryMetadata = z.infer< - typeof OpenIdProviderDiscoveryMetadataSchema ->; +export type OpenIdProviderMetadata = z.infer; +export type OpenIdProviderDiscoveryMetadata = z.infer; export type OAuthTokens = z.infer; export type OAuthErrorResponse = z.infer; export type OAuthClientMetadata = z.infer; -export type OAuthClientInformation = z.infer< - typeof OAuthClientInformationSchema ->; -export type OAuthClientInformationFull = z.infer< - typeof OAuthClientInformationFullSchema ->; -export type OAuthClientRegistrationError = z.infer< - typeof OAuthClientRegistrationErrorSchema ->; -export type OAuthTokenRevocationRequest = z.infer< - typeof OAuthTokenRevocationRequestSchema ->; -export type OAuthProtectedResourceMetadata = z.infer< - typeof OAuthProtectedResourceMetadataSchema ->; +export type OAuthClientInformation = z.infer; +export type OAuthClientInformationFull = z.infer; +export type OAuthClientRegistrationError = z.infer; +export type OAuthTokenRevocationRequest = z.infer; +export type OAuthProtectedResourceMetadata = z.infer; // Unified type for authorization server metadata -export type AuthorizationServerMetadata = - | OAuthMetadata - | OpenIdProviderDiscoveryMetadata; +export type AuthorizationServerMetadata = OAuthMetadata | OpenIdProviderDiscoveryMetadata; From ecb76f146d23dba6876202807564c641415538e7 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Wed, 23 Jul 2025 10:51:32 +0100 Subject: [PATCH 06/16] feat: add S256 PKCE validation for OIDC discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds validation during OIDC provider discovery to ensure S256 code challenge method is supported as required by MCP specification. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/client/auth.test.ts | 93 ++++++++++++++++++++++++++--------------- src/client/auth.ts | 11 ++++- 2 files changed, 70 insertions(+), 34 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 9aca72216..eb6f623a8 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -217,7 +217,7 @@ describe("OAuth Authorization", () => { ok: false, status: 404, }); - + // Second call (root fallback) succeeds mockFetch.mockResolvedValueOnce({ ok: true, @@ -227,17 +227,17 @@ describe("OAuth Authorization", () => { const metadata = await discoverOAuthProtectedResourceMetadata("https://resource.example.com/path/name"); expect(metadata).toEqual(validMetadata); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); - + // First call should be path-aware const [firstUrl, firstOptions] = calls[0]; expect(firstUrl.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource/path/name"); expect(firstOptions.headers).toEqual({ "MCP-Protocol-Version": LATEST_PROTOCOL_VERSION }); - + // Second call should be root fallback const [secondUrl, secondOptions] = calls[1]; expect(secondUrl.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource"); @@ -252,7 +252,7 @@ describe("OAuth Authorization", () => { ok: false, status: 404, }); - + // Second call (root fallback) also returns 404 mockFetch.mockResolvedValueOnce({ ok: false, @@ -261,7 +261,7 @@ describe("OAuth Authorization", () => { await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com/path/name")) .rejects.toThrow("Resource server does not implement OAuth 2.0 Protected Resource Metadata."); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); }); @@ -275,10 +275,10 @@ describe("OAuth Authorization", () => { await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com/")) .rejects.toThrow("Resource server does not implement OAuth 2.0 Protected Resource Metadata."); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(1); // Should not attempt fallback - + const [url] = calls[0]; expect(url.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource"); }); @@ -292,10 +292,10 @@ describe("OAuth Authorization", () => { await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com")) .rejects.toThrow("Resource server does not implement OAuth 2.0 Protected Resource Metadata."); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(1); // Should not attempt fallback - + const [url] = calls[0]; expect(url.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource"); }); @@ -303,13 +303,13 @@ describe("OAuth Authorization", () => { it("falls back when path-aware discovery encounters CORS error", async () => { // First call (path-aware) fails with TypeError (CORS) mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError("CORS error"))); - + // Retry path-aware without headers (simulating CORS retry) mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); - + // Second call (root fallback) succeeds mockFetch.mockResolvedValueOnce({ ok: true, @@ -319,10 +319,10 @@ describe("OAuth Authorization", () => { const metadata = await discoverOAuthProtectedResourceMetadata("https://resource.example.com/deep/path"); expect(metadata).toEqual(validMetadata); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(3); - + // Final call should be root fallback const [lastUrl, lastOptions] = calls[2]; expect(lastUrl.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource"); @@ -341,10 +341,10 @@ describe("OAuth Authorization", () => { await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com/path", { resourceMetadataUrl: "https://custom.example.com/metadata" })).rejects.toThrow("Resource server does not implement OAuth 2.0 Protected Resource Metadata."); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(1); // Should not attempt fallback when explicit URL is provided - + const [url] = calls[0]; expect(url.toString()).toBe("https://custom.example.com/metadata"); }); @@ -749,10 +749,10 @@ describe("OAuth Authorization", () => { expect(metadata).toEqual(validOpenIdMetadata); const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); - + // First call should be OAuth discovery expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); - + // Second call should be OpenID Connect discovery expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration"); }); @@ -815,10 +815,10 @@ describe("OAuth Authorization", () => { expect(metadata).toEqual(validOpenIdMetadata); const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); - + // First call should be OAuth with path expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - + // Second call should be OpenID Connect with path insertion expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); }); @@ -851,17 +851,44 @@ describe("OAuth Authorization", () => { expect(metadata).toEqual(validOpenIdMetadata); const calls = mockFetch.mock.calls; expect(calls.length).toBe(3); - + // First call should be OAuth with path expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - + // Second call should be OpenID Connect with path insertion expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); - + // Third call should be OpenID Connect with path prepending expect(calls[2][0].toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration"); }); + it("throws error when OIDC provider does not support S256 PKCE", async () => { + // OAuth discovery fails + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // OpenID Connect discovery succeeds but without S256 support + const invalidOpenIdMetadata = { + ...validOpenIdMetadata, + code_challenge_methods_supported: ["plain"], // Missing S256 + }; + + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => invalidOpenIdMetadata, + }); + + await expect( + discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com" + ) + ).rejects.toThrow("does not support S256 code challenge method required by MCP specification"); + }); + it("falls back to legacy MCP server when authorizationServerUrl is undefined", async () => { mockFetch.mockResolvedValueOnce({ ok: true, @@ -916,7 +943,7 @@ describe("OAuth Authorization", () => { it("handles CORS errors with retry", async () => { // First call fails with CORS mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError("CORS error"))); - + // Retry without headers succeeds mockFetch.mockResolvedValueOnce({ ok: true, @@ -932,10 +959,10 @@ describe("OAuth Authorization", () => { expect(metadata).toEqual(validOAuthMetadata); const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); - + // First call should have headers expect(calls[0][1]?.headers).toHaveProperty("MCP-Protocol-Version"); - + // Second call should not have headers (CORS retry) expect(calls[1][1]?.headers).toBeUndefined(); }); @@ -2216,17 +2243,17 @@ describe("OAuth Authorization", () => { // Verify the correct URLs were fetched const calls = mockFetch.mock.calls; - + // First call should be to PRM expect(calls[0][0].toString()).toBe("https://my.resource.com/.well-known/oauth-protected-resource/path/name"); - + // Second call should be to AS metadata with the path from authorization server expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/oauth"); }); it("supports overriding the fetch function used for requests", async () => { const customFetch = jest.fn(); - + // Mock PRM discovery customFetch.mockResolvedValueOnce({ ok: true, @@ -2236,7 +2263,7 @@ describe("OAuth Authorization", () => { authorization_servers: ["https://auth.example.com"], }), }); - + // Mock AS metadata discovery customFetch.mockResolvedValueOnce({ ok: true, @@ -2253,7 +2280,7 @@ describe("OAuth Authorization", () => { const mockProvider: OAuthClientProvider = { get redirectUrl() { return "http://localhost:3000/callback"; }, - get clientMetadata() { + get clientMetadata() { return { client_name: "Test Client", redirect_uris: ["http://localhost:3000/callback"], @@ -2278,10 +2305,10 @@ describe("OAuth Authorization", () => { expect(result).toBe("REDIRECT"); expect(customFetch).toHaveBeenCalledTimes(2); expect(mockFetch).not.toHaveBeenCalled(); - + // Verify custom fetch was called for PRM discovery expect(customFetch.mock.calls[0][0].toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource"); - + // Verify custom fetch was called for AS metadata discovery expect(customFetch.mock.calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); }); diff --git a/src/client/auth.ts b/src/client/auth.ts index 185d42efd..96ef2dd25 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -860,7 +860,16 @@ async function retrieveOpenIdProviderMetadataFromAuthorizationServer( throw new Error(`HTTP ${response.status} trying to load OpenID provider metadata from ${endpoint}`); } - return OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); + const metadata = OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); + + // MCP spec requires OIDC providers to support S256 PKCE + if (!metadata.code_challenge_methods_supported?.includes('S256')) { + throw new Error( + `Incompatible OIDC provider at ${endpoint}: does not support S256 code challenge method required by MCP specification` + ); + } + + return metadata; } return undefined; From 773412454c0b4128d0bed654cf83d216a7931527 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Wed, 23 Jul 2025 10:53:56 +0100 Subject: [PATCH 07/16] refactor: restore original fallback behavior in discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves fallback logic back to individual functions instead of generating "fake" metadata during discovery. Discovery now returns undefined on 404 like the original code, maintaining the existing architectural pattern where functions handle their own URL fallbacks. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/client/auth.test.ts | 11 ++--------- src/client/auth.ts | 18 +++--------------- 2 files changed, 5 insertions(+), 24 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index eb6f623a8..1b6283ecb 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -908,7 +908,7 @@ describe("OAuth Authorization", () => { expect(url.toString()).toBe("https://mcp.example.com/.well-known/oauth-authorization-server"); }); - it("returns fallback metadata when legacy MCP server returns 404", async () => { + it("returns undefined when legacy MCP server returns 404", async () => { mockFetch.mockResolvedValueOnce({ ok: false, status: 404, @@ -919,14 +919,7 @@ describe("OAuth Authorization", () => { undefined ); - expect(metadata).toEqual({ - issuer: "https://mcp.example.com", - authorization_endpoint: "https://mcp.example.com/authorize", - token_endpoint: "https://mcp.example.com/token", - registration_endpoint: "https://mcp.example.com/register", - response_types_supported: ["code"], - code_challenge_methods_supported: ["S256"], - }); + expect(metadata).toBeUndefined(); }); it("throws on non-404 errors in legacy mode", async () => { diff --git a/src/client/auth.ts b/src/client/auth.ts index 96ef2dd25..7f7d6555d 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -721,7 +721,7 @@ export async function discoverAuthorizationServerMetadata( * @param options - Configuration options * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch * @param options.protocolVersion - MCP protocol version to use (required) - * @returns Promise resolving to OAuth metadata + * @returns Promise resolving to OAuth metadata, or undefined if discovery fails */ async function retrieveOAuthMetadataFromMcpServer( serverUrl: string | URL, @@ -732,7 +732,7 @@ async function retrieveOAuthMetadataFromMcpServer( fetchFn?: FetchLike; protocolVersion: string; } -): Promise { +): Promise { const serverOrigin = typeof serverUrl === 'string' ? new URL(serverUrl).origin : serverUrl.origin; const metadataEndpoint = new URL(buildWellKnownPath('oauth-authorization-server'), serverOrigin); @@ -745,19 +745,7 @@ async function retrieveOAuthMetadataFromMcpServer( if (!response.ok) { if (response.status === 404) { - /** - * The MCP server does not implement OAuth 2.0 Authorization Server Metadata - * - * Return fallback OAuth 2.0 Authorization Server Metadata - */ - return { - issuer: serverOrigin, - authorization_endpoint: new URL('/authorize', serverOrigin).href, - token_endpoint: new URL('/token', serverOrigin).href, - registration_endpoint: new URL('/register', serverOrigin).href, - response_types_supported: ['code'], - code_challenge_methods_supported: ['S256'], - }; + return undefined; } throw new Error(`HTTP ${response.status} trying to load OAuth metadata from ${metadataEndpoint}`); From 4f6b5ada3050beefe535f73179e7e83104dc7cc3 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Wed, 23 Jul 2025 12:03:24 +0100 Subject: [PATCH 08/16] feat: add root OAuth discovery fallback with RFC compliance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements root OAuth discovery fallback when path-aware discovery fails, restoring compatibility behavior from original code. Adds comprehensive RFC documentation for OAuth 2.0 Authorization Server Metadata (RFC 8414) and OpenID Connect Discovery 1.0 path handling rules. Discovery sequence for issuer with path components: 1. OAuth with path insertion (RFC 8414 Section 3.1) 2. OIDC with path insertion (RFC 8414 Section 5 compatibility) 3. OIDC with path appending (OIDC Discovery 1.0 Section 4.1) 4. OAuth root fallback for compatibility 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/client/auth.test.ts | 46 +++++++++++++++++++++++++++++++++++++++++ src/client/auth.ts | 35 +++++++++++++++++++++++++++---- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 1b6283ecb..541377724 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -774,6 +774,52 @@ describe("OAuth Authorization", () => { expect(calls.length).toBe(2); }); + it("should fall back to root OAuth discovery when path-aware discovery fails", async () => { + // First call (OAuth with path) returns 404 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // Second call (OIDC with path insertion) returns 404 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // Third call (OIDC with path appending) returns 404 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // Fourth call should be OAuth root fallback + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOAuthMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com/tenant1" + ); + + expect(metadata).toEqual(validOAuthMetadata); + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(4); + + // Should try OAuth with path first + expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); + + // Should try OIDC discoveries next + expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); + expect(calls[2][0].toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration"); + + // Should finally fall back to OAuth root discovery + expect(calls[3][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + }); + it("handles authorization server URL with path in OAuth discovery", async () => { mockFetch.mockResolvedValueOnce({ ok: true, diff --git a/src/client/auth.ts b/src/client/auth.ts index 7f7d6555d..dea3f6d61 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -698,6 +698,9 @@ export async function discoverAuthorizationServerMetadata( }); } + const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl; + const hasPath = url.pathname !== '/'; + const oauthMetadata = await retrieveOAuthMetadataFromAuthorizationServer(authorizationServerUrl, { fetchFn, protocolVersion, @@ -707,10 +710,26 @@ export async function discoverAuthorizationServerMetadata( return oauthMetadata; } - return retrieveOpenIdProviderMetadataFromAuthorizationServer(authorizationServerUrl, { + const oidcMetadata = await retrieveOpenIdProviderMetadataFromAuthorizationServer(authorizationServerUrl, { fetchFn, protocolVersion, }); + + if (oidcMetadata) { + return oidcMetadata; + } + + // If both path-aware discoveries failed and the issuer has a path component, + // try OAuth discovery at the root as a final fallback for compatibility + if (hasPath) { + const rootUrl = new URL(url.origin); + return retrieveOAuthMetadataFromAuthorizationServer(rootUrl, { + fetchFn, + protocolVersion, + }); + } + + return undefined; } /** @@ -756,8 +775,12 @@ async function retrieveOAuthMetadataFromMcpServer( /** * Retrieves RFC 8414 OAuth 2.0 Authorization Server Metadata from the authorization server. + * + * Per RFC 8414 Section 3.1, when the issuer identifier contains path components, + * the well-known URI is constructed by inserting "/.well-known/oauth-authorization-server" + * before the path component. * - * @param authorizationServerUrl - The authorization server URL + * @param authorizationServerUrl - The authorization server URL (issuer identifier) * @param options - Configuration options * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch * @param options.protocolVersion - MCP protocol version to use (required) @@ -774,7 +797,6 @@ async function retrieveOAuthMetadataFromAuthorizationServer( } ): Promise { const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl; - const hasPath = url.pathname !== '/'; const metadataEndpoint = new URL( @@ -801,8 +823,13 @@ async function retrieveOAuthMetadataFromAuthorizationServer( /** * Retrieves OpenID Connect Discovery 1.0 metadata from the authorization server. + * + * Per RFC 8414 Section 5 compatibility notes and OpenID Connect Discovery 1.0 Section 4.1, + * when the issuer identifier contains path components, discovery endpoints are tried in order: + * 1. RFC 8414 style: Insert /.well-known/openid-configuration before the path + * 2. OIDC Discovery 1.0 style: Append /.well-known/openid-configuration after the path * - * @param authorizationServerUrl - The authorization server URL + * @param authorizationServerUrl - The authorization server URL (issuer identifier) * @param options - Configuration options * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch * @param options.protocolVersion - MCP protocol version to use (required) From 7065c386a512812c411ca842fc50e76bb0202c1f Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Wed, 23 Jul 2025 14:19:54 +0100 Subject: [PATCH 09/16] refactor: revise discovery order for consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes discovery sequence to be more logical and consistent: 1. Path-aware OAuth discovery 2. Root OAuth discovery (if path exists) 3. OIDC discovery methods This ensures OAuth methods are exhausted before trying OIDC, providing more predictable fallback behavior. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/client/auth.test.ts | 91 +++++++++++++++++++++++++++++------------ src/client/auth.ts | 24 +++++------ 2 files changed, 76 insertions(+), 39 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 541377724..0359fdd92 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -727,7 +727,7 @@ describe("OAuth Authorization", () => { }); }); - it("falls back to OpenID Connect discovery when OAuth discovery fails", async () => { + it("falls back to OpenID Connect discovery when OAuth discovery fails (no path component)", async () => { // First call (OAuth) returns 404 mockFetch.mockResolvedValueOnce({ ok: false, @@ -781,23 +781,47 @@ describe("OAuth Authorization", () => { status: 404, }); - // Second call (OIDC with path insertion) returns 404 + // Second call should be OAuth root fallback + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOAuthMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + "https://auth.example.com/tenant1" + ); + + expect(metadata).toEqual(validOAuthMetadata); + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(2); + + // Should try OAuth with path first + expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); + + // Should fall back to OAuth root discovery + expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + }); + + it("should try OIDC discovery after OAuth attempts fail", async () => { + // First call (OAuth with path) returns 404 mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); - // Third call (OIDC with path appending) returns 404 + // Second call (OAuth root) returns 404 mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); - // Fourth call should be OAuth root fallback + // Third call (OIDC with path insertion) succeeds mockFetch.mockResolvedValueOnce({ ok: true, status: 200, - json: async () => validOAuthMetadata, + json: async () => validOpenIdMetadata, }); const metadata = await discoverAuthorizationServerMetadata( @@ -805,19 +829,16 @@ describe("OAuth Authorization", () => { "https://auth.example.com/tenant1" ); - expect(metadata).toEqual(validOAuthMetadata); + expect(metadata).toEqual(validOpenIdMetadata); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(4); - - // Should try OAuth with path first + expect(calls.length).toBe(3); + + // Should try OAuth attempts first expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - - // Should try OIDC discoveries next - expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); - expect(calls[2][0].toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration"); - - // Should finally fall back to OAuth root discovery - expect(calls[3][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + + // Then try OIDC discovery + expect(calls[2][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); }); it("handles authorization server URL with path in OAuth discovery", async () => { @@ -840,7 +861,13 @@ describe("OAuth Authorization", () => { }); it("handles authorization server URL with path in OpenID Connect discovery", async () => { - // OAuth discovery fails + // OAuth discovery with path fails + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // OAuth discovery at root fails mockFetch.mockResolvedValueOnce({ ok: false, status: 404, @@ -860,17 +887,26 @@ describe("OAuth Authorization", () => { expect(metadata).toEqual(validOpenIdMetadata); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(2); + expect(calls.length).toBe(3); // First call should be OAuth with path expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - // Second call should be OpenID Connect with path insertion - expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); + // Second call should be OAuth at root + expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + + // Third call should be OpenID Connect with path insertion + expect(calls[2][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); }); it("tries multiple OpenID Connect endpoints when path is present", async () => { - // OAuth discovery fails + // OAuth discovery with path fails + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // OAuth discovery at root fails mockFetch.mockResolvedValueOnce({ ok: false, status: 404, @@ -896,16 +932,19 @@ describe("OAuth Authorization", () => { expect(metadata).toEqual(validOpenIdMetadata); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(3); + expect(calls.length).toBe(4); // First call should be OAuth with path expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - // Second call should be OpenID Connect with path insertion - expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); + // Second call should be OAuth at root + expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); + + // Third call should be OpenID Connect with path insertion + expect(calls[2][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); - // Third call should be OpenID Connect with path prepending - expect(calls[2][0].toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration"); + // Fourth call should be OpenID Connect with path prepending + expect(calls[3][0].toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration"); }); it("throws error when OIDC provider does not support S256 PKCE", async () => { diff --git a/src/client/auth.ts b/src/client/auth.ts index dea3f6d61..911eba237 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -710,26 +710,24 @@ export async function discoverAuthorizationServerMetadata( return oauthMetadata; } - const oidcMetadata = await retrieveOpenIdProviderMetadataFromAuthorizationServer(authorizationServerUrl, { - fetchFn, - protocolVersion, - }); - - if (oidcMetadata) { - return oidcMetadata; - } - - // If both path-aware discoveries failed and the issuer has a path component, - // try OAuth discovery at the root as a final fallback for compatibility if (hasPath) { const rootUrl = new URL(url.origin); - return retrieveOAuthMetadataFromAuthorizationServer(rootUrl, { + const rootOauthMetadata = await retrieveOAuthMetadataFromAuthorizationServer(rootUrl, { fetchFn, protocolVersion, }); + + if (rootOauthMetadata) { + return rootOauthMetadata; + } } - return undefined; + const oidcMetadata = await retrieveOpenIdProviderMetadataFromAuthorizationServer(authorizationServerUrl, { + fetchFn, + protocolVersion, + }); + + return oidcMetadata; } /** From fdc147e039f81b8777c146c8b10e88cbce2b5499 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Wed, 23 Jul 2025 16:18:39 +0100 Subject: [PATCH 10/16] test: remove redundant discovery tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes tests that are now covered by the parameterized discovery sequence tests, eliminating duplication while maintaining full coverage of all discovery scenarios. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/client/auth.test.ts | 345 +++++++++++++--------------------------- 1 file changed, 110 insertions(+), 235 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 0359fdd92..c62a897a9 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -705,247 +705,122 @@ describe("OAuth Authorization", () => { code_challenge_methods_supported: ["S256"], }; - it("returns OAuth metadata when authorizationServerUrl is provided and OAuth discovery succeeds", async () => { - mockFetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => validOAuthMetadata, - }); - - const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", - "https://auth.example.com" - ); - - expect(metadata).toEqual(validOAuthMetadata); - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(1); - const [url, options] = calls[0]; - expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); - expect(options.headers).toEqual({ - "MCP-Protocol-Version": LATEST_PROTOCOL_VERSION - }); - }); - - it("falls back to OpenID Connect discovery when OAuth discovery fails (no path component)", async () => { - // First call (OAuth) returns 404 - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - // Second call (OpenID Connect) succeeds - mockFetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => validOpenIdMetadata, - }); - - const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", - "https://auth.example.com" - ); - - expect(metadata).toEqual(validOpenIdMetadata); - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(2); - - // First call should be OAuth discovery - expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); - - // Second call should be OpenID Connect discovery - expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration"); - }); - - it("returns undefined when authorizationServerUrl is provided but both discoveries fail", async () => { - // Both calls return 404 - mockFetch.mockResolvedValue({ - ok: false, - status: 404, - }); - - const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", - "https://auth.example.com" - ); - - expect(metadata).toBeUndefined(); - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(2); - }); - - it("should fall back to root OAuth discovery when path-aware discovery fails", async () => { - // First call (OAuth with path) returns 404 - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - // Second call should be OAuth root fallback - mockFetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => validOAuthMetadata, - }); - - const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", - "https://auth.example.com/tenant1" - ); - - expect(metadata).toEqual(validOAuthMetadata); - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(2); - - // Should try OAuth with path first - expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - - // Should fall back to OAuth root discovery - expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); - }); - - it("should try OIDC discovery after OAuth attempts fail", async () => { - // First call (OAuth with path) returns 404 - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - // Second call (OAuth root) returns 404 - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - // Third call (OIDC with path insertion) succeeds - mockFetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => validOpenIdMetadata, - }); - - const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", - "https://auth.example.com/tenant1" - ); - - expect(metadata).toEqual(validOpenIdMetadata); - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(3); - - // Should try OAuth attempts first - expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); - - // Then try OIDC discovery - expect(calls[2][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); - }); - - it("handles authorization server URL with path in OAuth discovery", async () => { - mockFetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => validOAuthMetadata, - }); - - const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", - "https://auth.example.com/tenant1" - ); - - expect(metadata).toEqual(validOAuthMetadata); - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(1); - const [url] = calls[0]; - expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - }); + describe("discovery sequence", () => { + const testCases = [ + { + description: "no path - direct OAuth success", + serverUrl: "https://auth.example.com", + responses: [{ success: true, metadata: validOAuthMetadata }], + expectedPaths: [ + "https://auth.example.com/.well-known/oauth-authorization-server" + ] + }, + { + description: "no path - OAuth fails, OIDC succeeds", + serverUrl: "https://auth.example.com", + responses: [ + { success: false, status: 404 }, + { success: true, metadata: validOpenIdMetadata } + ], + expectedPaths: [ + "https://auth.example.com/.well-known/oauth-authorization-server", + "https://auth.example.com/.well-known/openid-configuration" + ] + }, + { + description: "with path - path OAuth succeeds", + serverUrl: "https://auth.example.com/tenant1", + responses: [{ success: true, metadata: validOAuthMetadata }], + expectedPaths: [ + "https://auth.example.com/.well-known/oauth-authorization-server/tenant1" + ] + }, + { + description: "with path - path OAuth fails, root OAuth succeeds", + serverUrl: "https://auth.example.com/tenant1", + responses: [ + { success: false, status: 404 }, + { success: true, metadata: validOAuthMetadata } + ], + expectedPaths: [ + "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", + "https://auth.example.com/.well-known/oauth-authorization-server" + ] + }, + { + description: "with path - OAuth fails, OIDC path insertion succeeds", + serverUrl: "https://auth.example.com/tenant1", + responses: [ + { success: false, status: 404 }, // OAuth path + { success: false, status: 404 }, // OAuth root + { success: true, metadata: validOpenIdMetadata } // OIDC path insertion + ], + expectedPaths: [ + "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", + "https://auth.example.com/.well-known/oauth-authorization-server", + "https://auth.example.com/.well-known/openid-configuration/tenant1" + ] + }, + { + description: "with path - OAuth fails, OIDC path appending succeeds", + serverUrl: "https://auth.example.com/tenant1", + responses: [ + { success: false, status: 404 }, // OAuth path + { success: false, status: 404 }, // OAuth root + { success: false, status: 404 }, // OIDC path insertion + { success: true, metadata: validOpenIdMetadata } // OIDC path appending + ], + expectedPaths: [ + "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", + "https://auth.example.com/.well-known/oauth-authorization-server", + "https://auth.example.com/.well-known/openid-configuration/tenant1", + "https://auth.example.com/tenant1/.well-known/openid-configuration" + ] + } + ]; + + testCases.forEach(({ description, serverUrl, responses, expectedPaths }) => { + it(description, async () => { + // Set up mock responses + responses.forEach(response => { + if (response.success) { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => response.metadata + }); + } else { + mockFetch.mockResolvedValueOnce({ + ok: false, + status: response.status || 404 + }); + } + }); - it("handles authorization server URL with path in OpenID Connect discovery", async () => { - // OAuth discovery with path fails - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); + const metadata = await discoverAuthorizationServerMetadata( + "https://mcp.example.com", + serverUrl + ); + + // Verify result + const successResponse = responses.find(r => r.success); + if (successResponse) { + expect(metadata).toEqual(successResponse.metadata); + } else { + expect(metadata).toBeUndefined(); + } - // OAuth discovery at root fails - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); + // Verify discovery sequence + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(expectedPaths.length); - // OpenID Connect discovery succeeds with path insertion - mockFetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => validOpenIdMetadata, + expectedPaths.forEach((expectedPath, index) => { + expect(calls[index][0].toString()).toBe(expectedPath); + }); + }); }); - - const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", - "https://auth.example.com/tenant1" - ); - - expect(metadata).toEqual(validOpenIdMetadata); - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(3); - - // First call should be OAuth with path - expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - - // Second call should be OAuth at root - expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); - - // Third call should be OpenID Connect with path insertion - expect(calls[2][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); }); - it("tries multiple OpenID Connect endpoints when path is present", async () => { - // OAuth discovery with path fails - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - // OAuth discovery at root fails - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - // First OpenID Connect attempt (path insertion) fails - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - // Second OpenID Connect attempt (path prepending) succeeds - mockFetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => validOpenIdMetadata, - }); - - const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", - "https://auth.example.com/tenant1" - ); - - expect(metadata).toEqual(validOpenIdMetadata); - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(4); - - // First call should be OAuth with path - expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); - - // Second call should be OAuth at root - expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); - - // Third call should be OpenID Connect with path insertion - expect(calls[2][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1"); - - // Fourth call should be OpenID Connect with path prepending - expect(calls[3][0].toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration"); - }); it("throws error when OIDC provider does not support S256 PKCE", async () => { // OAuth discovery fails From c8ccd0359df7b72d5b5b3a142b9492287420489b Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Wed, 23 Jul 2025 16:52:27 +0100 Subject: [PATCH 11/16] clean up unused param --- src/client/auth.test.ts | 54 ++++++++------------------ src/client/auth.ts | 84 ++++++++--------------------------------- 2 files changed, 31 insertions(+), 107 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index c62a897a9..1e6b93b4e 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -776,6 +776,22 @@ describe("OAuth Authorization", () => { "https://auth.example.com/.well-known/openid-configuration/tenant1", "https://auth.example.com/tenant1/.well-known/openid-configuration" ] + }, + { + description: "with path - all fails, returns undefined", + serverUrl: "https://auth.example.com/tenant1", + responses: [ + { success: false, status: 404 }, // OAuth path + { success: false, status: 404 }, // OAuth root + { success: false, status: 404 }, // OIDC path insertion + { success: false, status: 404 }, // OIDC path appending + ], + expectedPaths: [ + "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", + "https://auth.example.com/.well-known/oauth-authorization-server", + "https://auth.example.com/.well-known/openid-configuration/tenant1", + "https://auth.example.com/tenant1/.well-known/openid-configuration" + ] } ]; @@ -798,7 +814,6 @@ describe("OAuth Authorization", () => { }); const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", serverUrl ); @@ -843,45 +858,11 @@ describe("OAuth Authorization", () => { await expect( discoverAuthorizationServerMetadata( - "https://mcp.example.com", "https://auth.example.com" ) ).rejects.toThrow("does not support S256 code challenge method required by MCP specification"); }); - it("falls back to legacy MCP server when authorizationServerUrl is undefined", async () => { - mockFetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => validOAuthMetadata, - }); - - const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", - undefined - ); - - expect(metadata).toEqual(validOAuthMetadata); - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(1); - const [url] = calls[0]; - expect(url.toString()).toBe("https://mcp.example.com/.well-known/oauth-authorization-server"); - }); - - it("returns undefined when legacy MCP server returns 404", async () => { - mockFetch.mockResolvedValueOnce({ - ok: false, - status: 404, - }); - - const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", - undefined - ); - - expect(metadata).toBeUndefined(); - }); - it("throws on non-404 errors in legacy mode", async () => { mockFetch.mockResolvedValueOnce({ ok: false, @@ -905,7 +886,6 @@ describe("OAuth Authorization", () => { }); const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", "https://auth.example.com" ); @@ -928,7 +908,6 @@ describe("OAuth Authorization", () => { }); const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", "https://auth.example.com", { fetchFn: customFetch } ); @@ -946,7 +925,6 @@ describe("OAuth Authorization", () => { }); const metadata = await discoverAuthorizationServerMetadata( - "https://mcp.example.com", "https://auth.example.com", { protocolVersion: "2025-01-01" } ); diff --git a/src/client/auth.ts b/src/client/auth.ts index 911eba237..68f6597c7 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -332,12 +332,6 @@ async function authInternal( // Ignore errors and fall back to /.well-known/oauth-authorization-server } - const resource: URL | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata); - - const metadata = await discoverAuthorizationServerMetadata(serverUrl, authorizationServerUrl, { - fetchFn, - }); - /** * If we don't get a valid authorization server metadata from protected resource metadata, * fallback to the legacy MCP spec's implementation (version 2025-03-26): MCP server acts as the Authorization server. @@ -346,6 +340,12 @@ async function authInternal( authorizationServerUrl = serverUrl; } + const resource: URL | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata); + + const metadata = await discoverAuthorizationServerMetadata(authorizationServerUrl, { + fetchFn, + }); + // Handle client registration if needed let clientInformation = await Promise.resolve(provider.clientInformation()); if (!clientInformation) { @@ -664,24 +664,19 @@ export async function discoverOAuthMetadata( * and OpenID Connect Discovery 1.0 specifications. * * This function implements a fallback strategy for authorization server discovery: - * 1. If `authorizationServerUrl` is provided, attempts RFC 8414 OAuth metadata discovery first + * 1. Attempts RFC 8414 OAuth metadata discovery first * 2. If OAuth discovery fails, falls back to OpenID Connect Discovery - * 3. If `authorizationServerUrl` is not provided, uses legacy MCP specification behavior * - * @param serverUrl - The MCP Server URL, used for legacy specification support where the MCP server - * acts as both the resource server and authorization server * @param authorizationServerUrl - The authorization server URL obtained from the MCP Server's - * protected resource metadata. If this parameter is `undefined`, - * it indicates that protected resource metadata was not successfully - * retrieved, triggering legacy fallback behavior + * protected resource metadata, or the MCP server's URL if the + * metadata was not found. * @param options - Configuration options * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch * @param options.protocolVersion - MCP protocol version to use, defaults to LATEST_PROTOCOL_VERSION * @returns Promise resolving to authorization server metadata, or undefined if discovery fails */ export async function discoverAuthorizationServerMetadata( - serverUrl: string | URL, - authorizationServerUrl?: string | URL, + authorizationServerUrl: string | URL, { fetchFn = fetch, protocolVersion = LATEST_PROTOCOL_VERSION, @@ -690,18 +685,10 @@ export async function discoverAuthorizationServerMetadata( protocolVersion?: string; } = {} ): Promise { - if (!authorizationServerUrl) { - // Legacy support: MCP servers act as the Auth server. - return retrieveOAuthMetadataFromMcpServer(serverUrl, { - fetchFn, - protocolVersion, - }); - } - const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl; const hasPath = url.pathname !== '/'; - const oauthMetadata = await retrieveOAuthMetadataFromAuthorizationServer(authorizationServerUrl, { + const oauthMetadata = await fetchOAuthMetadata(authorizationServerUrl, { fetchFn, protocolVersion, }); @@ -712,7 +699,7 @@ export async function discoverAuthorizationServerMetadata( if (hasPath) { const rootUrl = new URL(url.origin); - const rootOauthMetadata = await retrieveOAuthMetadataFromAuthorizationServer(rootUrl, { + const rootOauthMetadata = await fetchOAuthMetadata(rootUrl, { fetchFn, protocolVersion, }); @@ -730,50 +717,9 @@ export async function discoverAuthorizationServerMetadata( return oidcMetadata; } -/** - * Legacy implementation where the MCP server acts as the Auth server. - * According to MCP spec version 2025-03-26. - * - * @param serverUrl - The MCP Server URL - * @param options - Configuration options - * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch - * @param options.protocolVersion - MCP protocol version to use (required) - * @returns Promise resolving to OAuth metadata, or undefined if discovery fails - */ -async function retrieveOAuthMetadataFromMcpServer( - serverUrl: string | URL, - { - fetchFn = fetch, - protocolVersion, - }: { - fetchFn?: FetchLike; - protocolVersion: string; - } -): Promise { - const serverOrigin = typeof serverUrl === 'string' ? new URL(serverUrl).origin : serverUrl.origin; - - const metadataEndpoint = new URL(buildWellKnownPath('oauth-authorization-server'), serverOrigin); - - const response = await fetchWithCorsRetry(metadataEndpoint, getProtocolVersionHeader(protocolVersion), fetchFn); - - if (!response) { - throw new Error(`CORS error trying to load OAuth metadata from ${metadataEndpoint}`); - } - - if (!response.ok) { - if (response.status === 404) { - return undefined; - } - - throw new Error(`HTTP ${response.status} trying to load OAuth metadata from ${metadataEndpoint}`); - } - - return OAuthMetadataSchema.parse(await response.json()); -} - /** * Retrieves RFC 8414 OAuth 2.0 Authorization Server Metadata from the authorization server. - * + * * Per RFC 8414 Section 3.1, when the issuer identifier contains path components, * the well-known URI is constructed by inserting "/.well-known/oauth-authorization-server" * before the path component. @@ -784,7 +730,7 @@ async function retrieveOAuthMetadataFromMcpServer( * @param options.protocolVersion - MCP protocol version to use (required) * @returns Promise resolving to OAuth metadata, or undefined if discovery fails */ -async function retrieveOAuthMetadataFromAuthorizationServer( +async function fetchOAuthMetadata( authorizationServerUrl: string | URL, { fetchFn = fetch, @@ -821,7 +767,7 @@ async function retrieveOAuthMetadataFromAuthorizationServer( /** * Retrieves OpenID Connect Discovery 1.0 metadata from the authorization server. - * + * * Per RFC 8414 Section 5 compatibility notes and OpenID Connect Discovery 1.0 Section 4.1, * when the issuer identifier contains path components, discovery endpoints are tried in order: * 1. RFC 8414 style: Insert /.well-known/openid-configuration before the path From 90f6ecac6fae075010476036704914b6b0afd01c Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Thu, 24 Jul 2025 10:57:05 +0100 Subject: [PATCH 12/16] simplify fetching logic --- src/client/auth.test.ts | 2 +- src/client/auth.ts | 212 +++++++++++++++------------------------- 2 files changed, 78 insertions(+), 136 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 1e6b93b4e..2f7f1d85a 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -863,7 +863,7 @@ describe("OAuth Authorization", () => { ).rejects.toThrow("does not support S256 code challenge method required by MCP specification"); }); - it("throws on non-404 errors in legacy mode", async () => { + it("throws on non-404 errors", async () => { mockFetch.mockResolvedValueOnce({ ok: false, status: 500, diff --git a/src/client/auth.ts b/src/client/auth.ts index 68f6597c7..6d48ee760 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -634,7 +634,7 @@ export async function discoverOAuthMetadata( if (typeof authorizationServerUrl === 'string') { authorizationServerUrl = new URL(authorizationServerUrl); } - protocolVersion ??= LATEST_PROTOCOL_VERSION; + protocolVersion ??= LATEST_PROTOCOL_VERSION ; const response = await discoverMetadataWithFallback( authorizationServerUrl, @@ -675,162 +675,104 @@ export async function discoverOAuthMetadata( * @param options.protocolVersion - MCP protocol version to use, defaults to LATEST_PROTOCOL_VERSION * @returns Promise resolving to authorization server metadata, or undefined if discovery fails */ -export async function discoverAuthorizationServerMetadata( - authorizationServerUrl: string | URL, - { - fetchFn = fetch, - protocolVersion = LATEST_PROTOCOL_VERSION, - }: { - fetchFn?: FetchLike; - protocolVersion?: string; - } = {} -): Promise { +/** + * Builds a list of discovery URLs to try for authorization server metadata. + * URLs are returned in priority order: + * 1. OAuth metadata at the given URL + * 2. OAuth metadata at root (if URL has path) + * 3. OIDC metadata endpoints + */ +function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: URL; type: 'oauth' | 'oidc' }[] { const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl; const hasPath = url.pathname !== '/'; - - const oauthMetadata = await fetchOAuthMetadata(authorizationServerUrl, { - fetchFn, - protocolVersion, + const urlsToTry: { url: URL; type: 'oauth' | 'oidc' }[] = []; + + // 1. OAuth metadata at the given URL + urlsToTry.push({ + url: new URL( + buildWellKnownPath('oauth-authorization-server', hasPath ? url.pathname : ''), + url.origin + ), + type: 'oauth' }); - - if (oauthMetadata) { - return oauthMetadata; - } - + + // 2. OAuth metadata at root (if URL has path) if (hasPath) { - const rootUrl = new URL(url.origin); - const rootOauthMetadata = await fetchOAuthMetadata(rootUrl, { - fetchFn, - protocolVersion, + urlsToTry.push({ + url: new URL(buildWellKnownPath('oauth-authorization-server'), url.origin), + type: 'oauth' }); - - if (rootOauthMetadata) { - return rootOauthMetadata; - } - } - - const oidcMetadata = await retrieveOpenIdProviderMetadataFromAuthorizationServer(authorizationServerUrl, { - fetchFn, - protocolVersion, - }); - - return oidcMetadata; -} - -/** - * Retrieves RFC 8414 OAuth 2.0 Authorization Server Metadata from the authorization server. - * - * Per RFC 8414 Section 3.1, when the issuer identifier contains path components, - * the well-known URI is constructed by inserting "/.well-known/oauth-authorization-server" - * before the path component. - * - * @param authorizationServerUrl - The authorization server URL (issuer identifier) - * @param options - Configuration options - * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch - * @param options.protocolVersion - MCP protocol version to use (required) - * @returns Promise resolving to OAuth metadata, or undefined if discovery fails - */ -async function fetchOAuthMetadata( - authorizationServerUrl: string | URL, - { - fetchFn = fetch, - protocolVersion, - }: { - fetchFn?: FetchLike; - protocolVersion: string; - } -): Promise { - const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl; - const hasPath = url.pathname !== '/'; - - const metadataEndpoint = new URL( - buildWellKnownPath('oauth-authorization-server', hasPath ? url.pathname : ''), - url.origin - ); - - const response = await fetchWithCorsRetry(metadataEndpoint, getProtocolVersionHeader(protocolVersion), fetchFn); - - if (!response) { - throw new Error(`CORS error trying to load OAuth metadata from ${metadataEndpoint}`); } - - if (!response.ok) { - if (response.status === 404) { - return undefined; - } - - throw new Error(`HTTP ${response.status} trying to load OAuth metadata from ${metadataEndpoint}`); + + // 3. OIDC metadata endpoints + if (hasPath) { + // RFC 8414 style: Insert /.well-known/openid-configuration before the path + urlsToTry.push({ + url: new URL(buildWellKnownPath('openid-configuration', url.pathname), url.origin), + type: 'oidc' + }); + // OIDC Discovery 1.0 style: Append /.well-known/openid-configuration after the path + urlsToTry.push({ + url: new URL(buildWellKnownPath('openid-configuration', url.pathname, { prependPathname: true }), url.origin), + type: 'oidc' + }); + } else { + urlsToTry.push({ + url: new URL(buildWellKnownPath('openid-configuration'), url.origin), + type: 'oidc' + }); } - - return OAuthMetadataSchema.parse(await response.json()); + + return urlsToTry; } -/** - * Retrieves OpenID Connect Discovery 1.0 metadata from the authorization server. - * - * Per RFC 8414 Section 5 compatibility notes and OpenID Connect Discovery 1.0 Section 4.1, - * when the issuer identifier contains path components, discovery endpoints are tried in order: - * 1. RFC 8414 style: Insert /.well-known/openid-configuration before the path - * 2. OIDC Discovery 1.0 style: Append /.well-known/openid-configuration after the path - * - * @param authorizationServerUrl - The authorization server URL (issuer identifier) - * @param options - Configuration options - * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch - * @param options.protocolVersion - MCP protocol version to use (required) - * @returns Promise resolving to OpenID provider metadata, or undefined if discovery fails - */ -async function retrieveOpenIdProviderMetadataFromAuthorizationServer( +export async function discoverAuthorizationServerMetadata( authorizationServerUrl: string | URL, { fetchFn = fetch, - protocolVersion, + protocolVersion = LATEST_PROTOCOL_VERSION, }: { fetchFn?: FetchLike; - protocolVersion: string; - } -): Promise { - const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl; - const hasPath = url.pathname !== '/'; - - const potentialMetadataEndpoints = hasPath - ? [ - // https://example.com/.well-known/openid-configuration/tenant1 - new URL(buildWellKnownPath('openid-configuration', url.pathname), url.origin), - // https://example.com/tenant1/.well-known/openid-configuration - new URL(buildWellKnownPath('openid-configuration', url.pathname, { prependPathname: true }), `${url.origin}`), - ] - : [ - // https://example.com/.well-known/openid-configuration - new URL(buildWellKnownPath('openid-configuration'), url.origin), - ]; - - for (const endpoint of potentialMetadataEndpoints) { - const response = await fetchWithCorsRetry(endpoint, getProtocolVersionHeader(protocolVersion), fetchFn); - + protocolVersion?: string; + } = {} +): Promise { + const headers = { 'MCP-Protocol-Version': protocolVersion }; + + // Get the list of URLs to try + const urlsToTry = buildDiscoveryUrls(authorizationServerUrl); + + // Try each URL in order + for (const { url: endpointUrl, type } of urlsToTry) { + const response = await fetchWithCorsRetry(endpointUrl, headers, fetchFn); + if (!response) { - throw new Error(`CORS error trying to load OpenID provider metadata from ${endpoint}`); + throw new Error(`CORS error trying to load ${type === 'oauth' ? 'OAuth' : 'OpenID provider'} metadata from ${endpointUrl}`); } - + if (!response.ok) { if (response.status === 404) { - continue; + continue; // Try next URL } - - throw new Error(`HTTP ${response.status} trying to load OpenID provider metadata from ${endpoint}`); + throw new Error(`HTTP ${response.status} trying to load ${type === 'oauth' ? 'OAuth' : 'OpenID provider'} metadata from ${endpointUrl}`); } - - const metadata = OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); - - // MCP spec requires OIDC providers to support S256 PKCE - if (!metadata.code_challenge_methods_supported?.includes('S256')) { - throw new Error( - `Incompatible OIDC provider at ${endpoint}: does not support S256 code challenge method required by MCP specification` - ); + + // Parse and validate based on type + if (type === 'oauth') { + return OAuthMetadataSchema.parse(await response.json()); + } else { + const metadata = OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); + + // MCP spec requires OIDC providers to support S256 PKCE + if (!metadata.code_challenge_methods_supported?.includes('S256')) { + throw new Error( + `Incompatible OIDC provider at ${endpointUrl}: does not support S256 code challenge method required by MCP specification` + ); + } + + return metadata; } - - return metadata; } - + return undefined; } From 49b51f5d7d2d5802f88aa6c491ca028398b30ec5 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Thu, 24 Jul 2025 11:02:30 +0100 Subject: [PATCH 13/16] test discovery urls list --- src/client/auth.test.ts | 201 +++++++++++++++------------------------- src/client/auth.ts | 2 +- 2 files changed, 74 insertions(+), 129 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 2f7f1d85a..0de3c492c 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -2,6 +2,7 @@ import { LATEST_PROTOCOL_VERSION } from '../types.js'; import { discoverOAuthMetadata, discoverAuthorizationServerMetadata, + buildDiscoveryUrls, startAuthorization, exchangeAuthorization, refreshAuthorization, @@ -684,6 +685,55 @@ describe("OAuth Authorization", () => { }); }); + describe("buildDiscoveryUrls", () => { + it("generates correct URLs for server without path", () => { + const urls = buildDiscoveryUrls("https://auth.example.com"); + + expect(urls).toHaveLength(2); + expect(urls.map(u => ({ url: u.url.toString(), type: u.type }))).toEqual([ + { + url: "https://auth.example.com/.well-known/oauth-authorization-server", + type: "oauth" + }, + { + url: "https://auth.example.com/.well-known/openid-configuration", + type: "oidc" + } + ]); + }); + + it("generates correct URLs for server with path", () => { + const urls = buildDiscoveryUrls("https://auth.example.com/tenant1"); + + expect(urls).toHaveLength(4); + expect(urls.map(u => ({ url: u.url.toString(), type: u.type }))).toEqual([ + { + url: "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", + type: "oauth" + }, + { + url: "https://auth.example.com/.well-known/oauth-authorization-server", + type: "oauth" + }, + { + url: "https://auth.example.com/.well-known/openid-configuration/tenant1", + type: "oidc" + }, + { + url: "https://auth.example.com/tenant1/.well-known/openid-configuration", + type: "oidc" + } + ]); + }); + + it("handles URL object input", () => { + const urls = buildDiscoveryUrls(new URL("https://auth.example.com/tenant1")); + + expect(urls).toHaveLength(4); + expect(urls[0].url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); + }); + }); + describe("discoverAuthorizationServerMetadata", () => { const validOAuthMetadata = { issuer: "https://auth.example.com", @@ -705,138 +755,33 @@ describe("OAuth Authorization", () => { code_challenge_methods_supported: ["S256"], }; - describe("discovery sequence", () => { - const testCases = [ - { - description: "no path - direct OAuth success", - serverUrl: "https://auth.example.com", - responses: [{ success: true, metadata: validOAuthMetadata }], - expectedPaths: [ - "https://auth.example.com/.well-known/oauth-authorization-server" - ] - }, - { - description: "no path - OAuth fails, OIDC succeeds", - serverUrl: "https://auth.example.com", - responses: [ - { success: false, status: 404 }, - { success: true, metadata: validOpenIdMetadata } - ], - expectedPaths: [ - "https://auth.example.com/.well-known/oauth-authorization-server", - "https://auth.example.com/.well-known/openid-configuration" - ] - }, - { - description: "with path - path OAuth succeeds", - serverUrl: "https://auth.example.com/tenant1", - responses: [{ success: true, metadata: validOAuthMetadata }], - expectedPaths: [ - "https://auth.example.com/.well-known/oauth-authorization-server/tenant1" - ] - }, - { - description: "with path - path OAuth fails, root OAuth succeeds", - serverUrl: "https://auth.example.com/tenant1", - responses: [ - { success: false, status: 404 }, - { success: true, metadata: validOAuthMetadata } - ], - expectedPaths: [ - "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", - "https://auth.example.com/.well-known/oauth-authorization-server" - ] - }, - { - description: "with path - OAuth fails, OIDC path insertion succeeds", - serverUrl: "https://auth.example.com/tenant1", - responses: [ - { success: false, status: 404 }, // OAuth path - { success: false, status: 404 }, // OAuth root - { success: true, metadata: validOpenIdMetadata } // OIDC path insertion - ], - expectedPaths: [ - "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", - "https://auth.example.com/.well-known/oauth-authorization-server", - "https://auth.example.com/.well-known/openid-configuration/tenant1" - ] - }, - { - description: "with path - OAuth fails, OIDC path appending succeeds", - serverUrl: "https://auth.example.com/tenant1", - responses: [ - { success: false, status: 404 }, // OAuth path - { success: false, status: 404 }, // OAuth root - { success: false, status: 404 }, // OIDC path insertion - { success: true, metadata: validOpenIdMetadata } // OIDC path appending - ], - expectedPaths: [ - "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", - "https://auth.example.com/.well-known/oauth-authorization-server", - "https://auth.example.com/.well-known/openid-configuration/tenant1", - "https://auth.example.com/tenant1/.well-known/openid-configuration" - ] - }, - { - description: "with path - all fails, returns undefined", - serverUrl: "https://auth.example.com/tenant1", - responses: [ - { success: false, status: 404 }, // OAuth path - { success: false, status: 404 }, // OAuth root - { success: false, status: 404 }, // OIDC path insertion - { success: false, status: 404 }, // OIDC path appending - ], - expectedPaths: [ - "https://auth.example.com/.well-known/oauth-authorization-server/tenant1", - "https://auth.example.com/.well-known/oauth-authorization-server", - "https://auth.example.com/.well-known/openid-configuration/tenant1", - "https://auth.example.com/tenant1/.well-known/openid-configuration" - ] - } - ]; - - testCases.forEach(({ description, serverUrl, responses, expectedPaths }) => { - it(description, async () => { - // Set up mock responses - responses.forEach(response => { - if (response.success) { - mockFetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => response.metadata - }); - } else { - mockFetch.mockResolvedValueOnce({ - ok: false, - status: response.status || 404 - }); - } - }); - - const metadata = await discoverAuthorizationServerMetadata( - serverUrl - ); - - // Verify result - const successResponse = responses.find(r => r.success); - if (successResponse) { - expect(metadata).toEqual(successResponse.metadata); - } else { - expect(metadata).toBeUndefined(); - } + it("tries URLs in order and returns first successful metadata", async () => { + // First OAuth URL fails with 404 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + }); + + // Second OAuth URL (root) succeeds + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOAuthMetadata, + }); - // Verify discovery sequence - const calls = mockFetch.mock.calls; - expect(calls.length).toBe(expectedPaths.length); + const metadata = await discoverAuthorizationServerMetadata( + "https://auth.example.com/tenant1" + ); - expectedPaths.forEach((expectedPath, index) => { - expect(calls[index][0].toString()).toBe(expectedPath); - }); - }); - }); + expect(metadata).toEqual(validOAuthMetadata); + + // Verify it tried the URLs in the correct order + const calls = mockFetch.mock.calls; + expect(calls.length).toBe(2); + expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); + expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); }); - it("throws error when OIDC provider does not support S256 PKCE", async () => { // OAuth discovery fails mockFetch.mockResolvedValueOnce({ diff --git a/src/client/auth.ts b/src/client/auth.ts index 6d48ee760..7651e4840 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -682,7 +682,7 @@ export async function discoverOAuthMetadata( * 2. OAuth metadata at root (if URL has path) * 3. OIDC metadata endpoints */ -function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: URL; type: 'oauth' | 'oidc' }[] { +export function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: URL; type: 'oauth' | 'oidc' }[] { const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl; const hasPath = url.pathname !== '/'; const urlsToTry: { url: URL; type: 'oauth' | 'oidc' }[] = []; From 0b2c0b12aa9b2919ff38b9444051052f9dbd6507 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Thu, 24 Jul 2025 11:11:08 +0100 Subject: [PATCH 14/16] cleanup --- src/client/auth.ts | 64 +++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/src/client/auth.ts b/src/client/auth.ts index 7651e4840..106423021 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -8,7 +8,6 @@ import { OAuthClientInformationFull, OAuthProtectedResourceMetadata, OAuthErrorResponseSchema, - OpenIdProviderDiscoveryMetadata, AuthorizationServerMetadata, OpenIdProviderDiscoveryMetadataSchema } from "../shared/auth.js"; @@ -659,22 +658,7 @@ export async function discoverOAuthMetadata( return OAuthMetadataSchema.parse(await response.json()); } -/** - * Discovers authorization server metadata with support for RFC 8414 OAuth 2.0 Authorization Server Metadata - * and OpenID Connect Discovery 1.0 specifications. - * - * This function implements a fallback strategy for authorization server discovery: - * 1. Attempts RFC 8414 OAuth metadata discovery first - * 2. If OAuth discovery fails, falls back to OpenID Connect Discovery - * - * @param authorizationServerUrl - The authorization server URL obtained from the MCP Server's - * protected resource metadata, or the MCP server's URL if the - * metadata was not found. - * @param options - Configuration options - * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch - * @param options.protocolVersion - MCP protocol version to use, defaults to LATEST_PROTOCOL_VERSION - * @returns Promise resolving to authorization server metadata, or undefined if discovery fails - */ + /** * Builds a list of discovery URLs to try for authorization server metadata. * URLs are returned in priority order: @@ -686,7 +670,7 @@ export function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl; const hasPath = url.pathname !== '/'; const urlsToTry: { url: URL; type: 'oauth' | 'oidc' }[] = []; - + // 1. OAuth metadata at the given URL urlsToTry.push({ url: new URL( @@ -695,7 +679,7 @@ export function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: ), type: 'oauth' }); - + // 2. OAuth metadata at root (if URL has path) if (hasPath) { urlsToTry.push({ @@ -703,7 +687,7 @@ export function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: type: 'oauth' }); } - + // 3. OIDC metadata endpoints if (hasPath) { // RFC 8414 style: Insert /.well-known/openid-configuration before the path @@ -722,10 +706,26 @@ export function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: type: 'oidc' }); } - + return urlsToTry; } +/** + * Discovers authorization server metadata with support for RFC 8414 OAuth 2.0 Authorization Server Metadata + * and OpenID Connect Discovery 1.0 specifications. + * + * This function implements a fallback strategy for authorization server discovery: + * 1. Attempts RFC 8414 OAuth metadata discovery first + * 2. If OAuth discovery fails, falls back to OpenID Connect Discovery + * + * @param authorizationServerUrl - The authorization server URL obtained from the MCP Server's + * protected resource metadata, or the MCP server's URL if the + * metadata was not found. + * @param options - Configuration options + * @param options.fetchFn - Optional fetch function for making HTTP requests, defaults to global fetch + * @param options.protocolVersion - MCP protocol version to use, defaults to LATEST_PROTOCOL_VERSION + * @returns Promise resolving to authorization server metadata, or undefined if discovery fails + */ export async function discoverAuthorizationServerMetadata( authorizationServerUrl: string | URL, { @@ -737,49 +737,43 @@ export async function discoverAuthorizationServerMetadata( } = {} ): Promise { const headers = { 'MCP-Protocol-Version': protocolVersion }; - + // Get the list of URLs to try const urlsToTry = buildDiscoveryUrls(authorizationServerUrl); - + // Try each URL in order for (const { url: endpointUrl, type } of urlsToTry) { const response = await fetchWithCorsRetry(endpointUrl, headers, fetchFn); - + if (!response) { throw new Error(`CORS error trying to load ${type === 'oauth' ? 'OAuth' : 'OpenID provider'} metadata from ${endpointUrl}`); } - + if (!response.ok) { if (response.status === 404) { continue; // Try next URL } throw new Error(`HTTP ${response.status} trying to load ${type === 'oauth' ? 'OAuth' : 'OpenID provider'} metadata from ${endpointUrl}`); } - + // Parse and validate based on type if (type === 'oauth') { return OAuthMetadataSchema.parse(await response.json()); } else { const metadata = OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); - + // MCP spec requires OIDC providers to support S256 PKCE if (!metadata.code_challenge_methods_supported?.includes('S256')) { throw new Error( `Incompatible OIDC provider at ${endpointUrl}: does not support S256 code challenge method required by MCP specification` ); } - + return metadata; } } - - return undefined; -} -function getProtocolVersionHeader(protocolVersion: string): Record { - return { - 'MCP-Protocol-Version': protocolVersion, - }; + return undefined; } /** From ab060e17e1d096c9775c20946a77fbdec88ad5a6 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Thu, 24 Jul 2025 11:16:59 +0100 Subject: [PATCH 15/16] simplify path construction --- src/client/auth.ts | 64 +++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/src/client/auth.ts b/src/client/auth.ts index 106423021..26777cb91 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -671,42 +671,54 @@ export function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: const hasPath = url.pathname !== '/'; const urlsToTry: { url: URL; type: 'oauth' | 'oidc' }[] = []; - // 1. OAuth metadata at the given URL - urlsToTry.push({ - url: new URL( - buildWellKnownPath('oauth-authorization-server', hasPath ? url.pathname : ''), - url.origin - ), - type: 'oauth' - }); - // 2. OAuth metadata at root (if URL has path) - if (hasPath) { + if (!hasPath) { + // Root path: https://example.com/.well-known/oauth-authorization-server urlsToTry.push({ - url: new URL(buildWellKnownPath('oauth-authorization-server'), url.origin), + url: new URL('/.well-known/oauth-authorization-server', url.origin), type: 'oauth' }); - } - // 3. OIDC metadata endpoints - if (hasPath) { - // RFC 8414 style: Insert /.well-known/openid-configuration before the path - urlsToTry.push({ - url: new URL(buildWellKnownPath('openid-configuration', url.pathname), url.origin), - type: 'oidc' - }); - // OIDC Discovery 1.0 style: Append /.well-known/openid-configuration after the path + // OIDC: https://example.com/.well-known/openid-configuration urlsToTry.push({ - url: new URL(buildWellKnownPath('openid-configuration', url.pathname, { prependPathname: true }), url.origin), - type: 'oidc' - }); - } else { - urlsToTry.push({ - url: new URL(buildWellKnownPath('openid-configuration'), url.origin), + url: new URL(`/.well-known/openid-configuration`, url.origin), type: 'oidc' }); + + return urlsToTry; + } + + // Strip trailing slash from pathname to avoid double slashes + let pathname = url.pathname; + if (pathname.endsWith('/')) { + pathname = pathname.slice(0, -1); } + // 1. OAuth metadata at the given URL + // Insert well-known before the path: https://example.com/.well-known/oauth-authorization-server/tenant1 + urlsToTry.push({ + url: new URL(`/.well-known/oauth-authorization-server${pathname}`, url.origin), + type: 'oauth' + }); + + // Root path: https://example.com/.well-known/oauth-authorization-server + urlsToTry.push({ + url: new URL('/.well-known/oauth-authorization-server', url.origin), + type: 'oauth' + }); + + // 3. OIDC metadata endpoints + // RFC 8414 style: Insert /.well-known/openid-configuration before the path + urlsToTry.push({ + url: new URL(`/.well-known/openid-configuration${pathname}`, url.origin), + type: 'oidc' + }); + // OIDC Discovery 1.0 style: Append /.well-known/openid-configuration after the path + urlsToTry.push({ + url: new URL(`${pathname}/.well-known/openid-configuration`, url.origin), + type: 'oidc' + }); + return urlsToTry; } From a6fd8f5903700fb3027e13530d015ffeb45174d3 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Thu, 24 Jul 2025 11:26:47 +0100 Subject: [PATCH 16/16] permit all 4xx status codes to continue probing --- src/client/auth.test.ts | 32 +++++++++++++++++++++++++------- src/client/auth.ts | 3 ++- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 0de3c492c..c3049124e 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -688,7 +688,7 @@ describe("OAuth Authorization", () => { describe("buildDiscoveryUrls", () => { it("generates correct URLs for server without path", () => { const urls = buildDiscoveryUrls("https://auth.example.com"); - + expect(urls).toHaveLength(2); expect(urls.map(u => ({ url: u.url.toString(), type: u.type }))).toEqual([ { @@ -704,7 +704,7 @@ describe("OAuth Authorization", () => { it("generates correct URLs for server with path", () => { const urls = buildDiscoveryUrls("https://auth.example.com/tenant1"); - + expect(urls).toHaveLength(4); expect(urls.map(u => ({ url: u.url.toString(), type: u.type }))).toEqual([ { @@ -728,7 +728,7 @@ describe("OAuth Authorization", () => { it("handles URL object input", () => { const urls = buildDiscoveryUrls(new URL("https://auth.example.com/tenant1")); - + expect(urls).toHaveLength(4); expect(urls[0].url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1"); }); @@ -761,7 +761,7 @@ describe("OAuth Authorization", () => { ok: false, status: 404, }); - + // Second OAuth URL (root) succeeds mockFetch.mockResolvedValueOnce({ ok: true, @@ -774,7 +774,7 @@ describe("OAuth Authorization", () => { ); expect(metadata).toEqual(validOAuthMetadata); - + // Verify it tried the URLs in the correct order const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); @@ -808,14 +808,32 @@ describe("OAuth Authorization", () => { ).rejects.toThrow("does not support S256 code challenge method required by MCP specification"); }); - it("throws on non-404 errors", async () => { + it("continues on 4xx errors", async () => { + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 400, + }); + + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOpenIdMetadata, + }); + + const metadata = await discoverAuthorizationServerMetadata("https://mcp.example.com"); + + expect(metadata).toEqual(validOpenIdMetadata); + + }); + + it("throws on non-4xx errors", async () => { mockFetch.mockResolvedValueOnce({ ok: false, status: 500, }); await expect( - discoverAuthorizationServerMetadata("https://mcp.example.com", undefined) + discoverAuthorizationServerMetadata("https://mcp.example.com") ).rejects.toThrow("HTTP 500"); }); diff --git a/src/client/auth.ts b/src/client/auth.ts index 26777cb91..0f8e84b35 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -762,7 +762,8 @@ export async function discoverAuthorizationServerMetadata( } if (!response.ok) { - if (response.status === 404) { + // Continue looking for any 4xx response code. + if (response.status >= 400 && response.status < 500) { continue; // Try next URL } throw new Error(`HTTP ${response.status} trying to load ${type === 'oauth' ? 'OAuth' : 'OpenID provider'} metadata from ${endpointUrl}`);