From 561dbe5390f06a4d03b4a5a7af6376b08e662bf1 Mon Sep 17 00:00:00 2001 From: Bob Dickinson Date: Mon, 16 Mar 2026 18:11:06 -0700 Subject: [PATCH 1/4] Fix OAuth: use authorization_servers URL from resource metadata for scope discovery - discoverScopes now uses resourceMetadata.authorization_servers[0] when present; otherwise falls back to the MCP server URL. - Add tests: different-domain auth server, path preserved, empty list fallback. Backport of PR 1133 (core/auth layout). Made-with: Cursor --- core/__tests__/auth/discovery.test.ts | 88 +++++++++++++++++++++++++++ core/auth/discovery.ts | 12 ++-- 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/core/__tests__/auth/discovery.test.ts b/core/__tests__/auth/discovery.test.ts index 591701291..4a7080866 100644 --- a/core/__tests__/auth/discovery.test.ts +++ b/core/__tests__/auth/discovery.test.ts @@ -176,4 +176,92 @@ describe("OAuth Scope Discovery", () => { { fetchFn: mockFetchFn }, ); }); + + it("should use authorization_servers URL from resource metadata for discovery (different domain)", async () => { + const { discoverAuthorizationServerMetadata } = + await import("@modelcontextprotocol/sdk/client/auth.js"); + vi.mocked(discoverAuthorizationServerMetadata).mockResolvedValue({ + issuer: "https://auth-server.com", + authorization_endpoint: "https://auth-server.com/authorize", + token_endpoint: "https://auth-server.com/token", + response_types_supported: ["code"], + scopes_supported: ["read", "write"], + }); + + const resourceMetadata: OAuthProtectedResourceMetadata = { + resource: "https://mcp-server.com", + authorization_servers: ["https://auth-server.com/"], + scopes_supported: ["read", "write"], + }; + + const scopes = await discoverScopes( + "https://mcp-server.com", + resourceMetadata, + ); + + expect(scopes).toBe("read write"); + expect(discoverAuthorizationServerMetadata).toHaveBeenCalledWith( + new URL("https://auth-server.com/"), + { fetchFn: undefined }, + ); + }); + + it("should preserve full path in authorization_servers URL", async () => { + const { discoverAuthorizationServerMetadata } = + await import("@modelcontextprotocol/sdk/client/auth.js"); + vi.mocked(discoverAuthorizationServerMetadata).mockResolvedValue({ + issuer: "https://auth-server.com/realms/my-realm", + authorization_endpoint: + "https://auth-server.com/realms/my-realm/authorize", + token_endpoint: "https://auth-server.com/realms/my-realm/token", + response_types_supported: ["code"], + scopes_supported: ["read", "write"], + }); + + const resourceMetadata: OAuthProtectedResourceMetadata = { + resource: "https://mcp-server.com", + authorization_servers: ["https://auth-server.com/realms/my-realm/"], + scopes_supported: ["read", "write"], + }; + + const scopes = await discoverScopes( + "https://mcp-server.com", + resourceMetadata, + ); + + expect(scopes).toBe("read write"); + expect(discoverAuthorizationServerMetadata).toHaveBeenCalledWith( + new URL("https://auth-server.com/realms/my-realm/"), + { fetchFn: undefined }, + ); + }); + + it("should fall back to serverUrl when authorization_servers is empty", async () => { + const { discoverAuthorizationServerMetadata } = + await import("@modelcontextprotocol/sdk/client/auth.js"); + vi.mocked(discoverAuthorizationServerMetadata).mockResolvedValue({ + issuer: "https://mcp-server.com", + authorization_endpoint: "https://mcp-server.com/authorize", + token_endpoint: "https://mcp-server.com/token", + response_types_supported: ["code"], + scopes_supported: ["read", "write"], + }); + + const resourceMetadata: OAuthProtectedResourceMetadata = { + resource: "https://mcp-server.com", + authorization_servers: [], + scopes_supported: ["read", "write"], + }; + + const scopes = await discoverScopes( + "https://mcp-server.com", + resourceMetadata, + ); + + expect(scopes).toBe("read write"); + expect(discoverAuthorizationServerMetadata).toHaveBeenCalledWith( + new URL("/", "https://mcp-server.com"), + { fetchFn: undefined }, + ); + }); }); diff --git a/core/auth/discovery.ts b/core/auth/discovery.ts index ae1b33ef6..da03bef79 100644 --- a/core/auth/discovery.ts +++ b/core/auth/discovery.ts @@ -14,10 +14,14 @@ export const discoverScopes = async ( fetchFn?: typeof fetch, ): Promise => { try { - const metadata = await discoverAuthorizationServerMetadata( - new URL("/", serverUrl), - { fetchFn }, - ); + // Use the authorization server URL from resource metadata if available, + // otherwise fall back to the MCP server URL + const authServerUrl = resourceMetadata?.authorization_servers?.length + ? new URL(resourceMetadata.authorization_servers[0]) + : new URL("/", serverUrl); + const metadata = await discoverAuthorizationServerMetadata(authServerUrl, { + fetchFn, + }); // Prefer resource metadata scopes, but fall back to OAuth metadata if empty const resourceScopes = resourceMetadata?.scopes_supported; From 150d59ba94c0614b71aae2076541276592bfaa47 Mon Sep 17 00:00:00 2001 From: Bob Dickinson Date: Mon, 16 Mar 2026 18:15:27 -0700 Subject: [PATCH 2/4] Fixed TypeScript error --- core/auth/discovery.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/auth/discovery.ts b/core/auth/discovery.ts index da03bef79..a1800e6af 100644 --- a/core/auth/discovery.ts +++ b/core/auth/discovery.ts @@ -16,9 +16,11 @@ export const discoverScopes = async ( try { // Use the authorization server URL from resource metadata if available, // otherwise fall back to the MCP server URL - const authServerUrl = resourceMetadata?.authorization_servers?.length - ? new URL(resourceMetadata.authorization_servers[0]) - : new URL("/", serverUrl); + const firstAuthServer = resourceMetadata?.authorization_servers?.[0]; + const authServerUrl = + firstAuthServer !== undefined + ? new URL(firstAuthServer) + : new URL("/", serverUrl); const metadata = await discoverAuthorizationServerMetadata(authServerUrl, { fetchFn, }); From abcbef751c73a1858a8cbcbdad1dcbab6cb136fb Mon Sep 17 00:00:00 2001 From: Bob Dickinson Date: Mon, 16 Mar 2026 18:38:10 -0700 Subject: [PATCH 3/4] Fixed: OAuth: use authorization_servers URL from resource metadata (including centralizing getAuthorizationServerUrl) --- core/auth/discovery.ts | 24 +++++++++++++++++------- core/auth/state-machine.ts | 15 ++++++--------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/core/auth/discovery.ts b/core/auth/discovery.ts index a1800e6af..f2d9194b7 100644 --- a/core/auth/discovery.ts +++ b/core/auth/discovery.ts @@ -1,6 +1,19 @@ import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/sdk/client/auth.js"; import type { OAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/shared/auth.js"; +/** + * Returns the URL to use for OAuth authorization server metadata discovery. + * Uses resource metadata's authorization_servers[0] when present, otherwise the MCP server URL. + */ +export function getAuthorizationServerUrl( + serverUrl: string, + resourceMetadata?: OAuthProtectedResourceMetadata | null, +): URL { + const first = resourceMetadata?.authorization_servers?.[0]; + // Use truthy check to match original state-machine: empty string falls back to serverUrl + return first ? new URL(first) : new URL("/", serverUrl); +} + /** * Discovers OAuth scopes from server metadata, with preference for resource metadata scopes * @param serverUrl - The MCP server URL @@ -14,13 +27,10 @@ export const discoverScopes = async ( fetchFn?: typeof fetch, ): Promise => { try { - // Use the authorization server URL from resource metadata if available, - // otherwise fall back to the MCP server URL - const firstAuthServer = resourceMetadata?.authorization_servers?.[0]; - const authServerUrl = - firstAuthServer !== undefined - ? new URL(firstAuthServer) - : new URL("/", serverUrl); + const authServerUrl = getAuthorizationServerUrl( + serverUrl, + resourceMetadata, + ); const metadata = await discoverAuthorizationServerMetadata(authServerUrl, { fetchFn, }); diff --git a/core/auth/state-machine.ts b/core/auth/state-machine.ts index 49f950718..4f2958e79 100644 --- a/core/auth/state-machine.ts +++ b/core/auth/state-machine.ts @@ -1,6 +1,6 @@ import type { OAuthStep, AuthGuidedState } from "./types.js"; import type { BaseOAuthClientProvider } from "./providers.js"; -import { discoverScopes } from "./discovery.js"; +import { discoverScopes, getAuthorizationServerUrl } from "./discovery.js"; import { discoverAuthorizationServerMetadata, registerClient, @@ -32,20 +32,12 @@ export const oauthTransitions: Record = { metadata_discovery: { canTransition: async () => true, execute: async (context) => { - // Default to discovering from the server's URL - let authServerUrl: URL = new URL("/", context.serverUrl); let resourceMetadata: OAuthProtectedResourceMetadata | null = null; let resourceMetadataError: Error | null = null; try { resourceMetadata = await discoverOAuthProtectedResourceMetadata( context.serverUrl as string | URL, ); - if (resourceMetadata?.authorization_servers?.length) { - const firstServer = resourceMetadata.authorization_servers[0]; - if (firstServer) { - authServerUrl = new URL(firstServer); - } - } } catch (e) { if (e instanceof Error) { resourceMetadataError = e; @@ -54,6 +46,11 @@ export const oauthTransitions: Record = { } } + const authServerUrl = getAuthorizationServerUrl( + context.serverUrl, + resourceMetadata, + ); + const resource: URL | undefined = resourceMetadata ? await selectResourceURL( context.serverUrl, From d5f81124d94922e0ad906bc3a005248a3dac729d Mon Sep 17 00:00:00 2001 From: Bob Dickinson Date: Mon, 16 Mar 2026 18:58:31 -0700 Subject: [PATCH 4/4] Added tests recommend by Claude review --- core/__tests__/auth/discovery.test.ts | 52 ++++++++++++++++++++++- core/__tests__/auth/state-machine.test.ts | 38 +++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/core/__tests__/auth/discovery.test.ts b/core/__tests__/auth/discovery.test.ts index 4a7080866..7ce0225a4 100644 --- a/core/__tests__/auth/discovery.test.ts +++ b/core/__tests__/auth/discovery.test.ts @@ -1,5 +1,8 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { discoverScopes } from "../../auth/discovery.js"; +import { + discoverScopes, + getAuthorizationServerUrl, +} from "../../auth/discovery.js"; import type { OAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/shared/auth.js"; // Mock SDK functions @@ -265,3 +268,50 @@ describe("OAuth Scope Discovery", () => { ); }); }); + +describe("getAuthorizationServerUrl", () => { + const serverUrl = "https://mcp.example.com"; + + it("returns server URL when resourceMetadata is null", () => { + expect(getAuthorizationServerUrl(serverUrl, null)).toEqual( + new URL("/", serverUrl), + ); + }); + + it("returns server URL when resourceMetadata is undefined", () => { + expect(getAuthorizationServerUrl(serverUrl)).toEqual( + new URL("/", serverUrl), + ); + }); + + it("returns server URL when authorization_servers is empty array", () => { + const resourceMetadata: OAuthProtectedResourceMetadata = { + resource: serverUrl, + authorization_servers: [], + }; + expect(getAuthorizationServerUrl(serverUrl, resourceMetadata)).toEqual( + new URL("/", serverUrl), + ); + }); + + it("falls back to server URL when authorization_servers[0] is empty string", () => { + const resourceMetadata: OAuthProtectedResourceMetadata = { + resource: serverUrl, + authorization_servers: [""], + }; + expect(getAuthorizationServerUrl(serverUrl, resourceMetadata)).toEqual( + new URL("/", serverUrl), + ); + }); + + it("returns authorization_servers[0] when present and truthy", () => { + const authUrl = "https://auth.example.com/"; + const resourceMetadata: OAuthProtectedResourceMetadata = { + resource: serverUrl, + authorization_servers: [authUrl], + }; + expect(getAuthorizationServerUrl(serverUrl, resourceMetadata)).toEqual( + new URL(authUrl), + ); + }); +}); diff --git a/core/__tests__/auth/state-machine.test.ts b/core/__tests__/auth/state-machine.test.ts index 3fb153ae0..9f91caf94 100644 --- a/core/__tests__/auth/state-machine.test.ts +++ b/core/__tests__/auth/state-machine.test.ts @@ -167,6 +167,44 @@ describe("OAuthStateMachine", () => { ); }); + it("should use authorization_servers URL from resource metadata for auth server discovery", async () => { + const authServerUrl = "https://auth-server.com/"; + const resourceMetaDifferentAuth: OAuthProtectedResourceMetadata = { + resource: serverUrl, + authorization_servers: [authServerUrl], + scopes_supported: ["read", "write"], + }; + const selectedResource = new URL(serverUrl); + const { + discoverOAuthProtectedResourceMetadata, + discoverAuthorizationServerMetadata, + selectResourceURL, + } = await import("@modelcontextprotocol/sdk/client/auth.js"); + vi.mocked(discoverOAuthProtectedResourceMetadata).mockResolvedValue( + resourceMetaDifferentAuth, + ); + vi.mocked(selectResourceURL).mockResolvedValue(selectedResource); + + const stateMachine = new OAuthStateMachine( + serverUrl, + mockProvider, + updateState, + ); + await stateMachine.executeStep(state); + + expect(discoverAuthorizationServerMetadata).toHaveBeenCalledWith( + new URL(authServerUrl), + expect.any(Object), + ); + expect(updateState).toHaveBeenCalledWith( + expect.objectContaining({ + resourceMetadata: resourceMetaDifferentAuth, + authServerUrl: new URL(authServerUrl), + oauthStep: "client_registration", + }), + ); + }); + it("should call selectResourceURL only when resource metadata is present", async () => { const { discoverOAuthProtectedResourceMetadata, selectResourceURL } = await import("@modelcontextprotocol/sdk/client/auth.js");