From 626dae184e59e134ab5ae59d4f33765ecfbfcd52 Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Tue, 1 Jul 2025 20:39:40 -0700 Subject: [PATCH 1/3] Add authorizationServerUrl and metadata arguments to addClientAuthentication. --- src/client/auth.ts | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/client/auth.ts b/src/client/auth.ts index c4b3c920f..87a8423a6 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -72,25 +72,25 @@ export interface OAuthClientProvider { * the authorization result. */ codeVerifier(): string | Promise; - + /** * Adds custom client authentication to OAuth token requests. - * + * * This optional method allows implementations to customize how client credentials * are included in token exchange and refresh requests. When provided, this method * is called instead of the default authentication logic, giving full control over * the authentication mechanism. - * + * * Common use cases include: * - Supporting authentication methods beyond the standard OAuth 2.0 methods * - Adding custom headers for proprietary authentication schemes * - Implementing client assertion-based authentication (e.g., JWT bearer tokens) - * + * * @param url - The token endpoint URL being called * @param headers - The request headers (can be modified to add authentication) * @param params - The request body parameters (can be modified to add credentials) */ - addClientAuthentication?(url: URL, headers: Headers, params: URLSearchParams): void | Promise; + addClientAuthentication?(headers: Headers, params: URLSearchParams, url: string | URL, metadata?: OAuthMetadata): void | Promise; /** * If defined, overrides the selection and validation of the @@ -112,12 +112,12 @@ export class UnauthorizedError extends Error { /** * Determines the best client authentication method to use based on server support and client configuration. - * + * * Priority order (highest to lowest): * 1. client_secret_basic (if client secret is available) * 2. client_secret_post (if client secret is available) * 3. none (for public clients) - * + * * @param clientInformation - OAuth client information containing credentials * @param supportedMethods - Authentication methods supported by the authorization server * @returns The selected authentication method @@ -127,7 +127,7 @@ function selectClientAuthMethod( supportedMethods: string[] ): string { const hasClientSecret = !!clientInformation.client_secret; - + // If server doesn't specify supported methods, use RFC 6749 defaults if (supportedMethods.length === 0) { return hasClientSecret ? "client_secret_post" : "none"; @@ -137,11 +137,11 @@ function selectClientAuthMethod( if (hasClientSecret && supportedMethods.includes("client_secret_basic")) { return "client_secret_basic"; } - + if (hasClientSecret && supportedMethods.includes("client_secret_post")) { return "client_secret_post"; } - + if (supportedMethods.includes("none")) { return "none"; } @@ -152,12 +152,12 @@ function selectClientAuthMethod( /** * Applies client authentication to the request based on the specified method. - * + * * Implements OAuth 2.1 client authentication methods: * - client_secret_basic: HTTP Basic authentication (RFC 6749 Section 2.3.1) * - client_secret_post: Credentials in request body (RFC 6749 Section 2.3.1) * - none: Public client authentication (RFC 6749 Section 2.1) - * + * * @param method - The authentication method to use * @param clientInformation - OAuth client information containing credentials * @param headers - HTTP headers object to modify @@ -197,7 +197,7 @@ function applyBasicAuth(clientId: string, clientSecret: string | undefined, head if (!clientSecret) { throw new Error("client_secret_basic authentication requires a client_secret"); } - + const credentials = btoa(`${clientId}:${clientSecret}`); headers.set("Authorization", `Basic ${credentials}`); } @@ -593,11 +593,11 @@ export async function startAuthorization( /** * Exchanges an authorization code for an access token with the given server. - * + * * Supports multiple client authentication methods as specified in OAuth 2.1: * - Automatically selects the best authentication method based on server support * - Falls back to appropriate defaults when server metadata is unavailable - * + * * @param authorizationServerUrl - The authorization server's base URL * @param options - Configuration object containing client info, auth code, etc. * @returns Promise resolving to OAuth tokens @@ -650,12 +650,12 @@ export async function exchangeAuthorization( }); if (addClientAuthentication) { - addClientAuthentication(tokenUrl, headers, params); + 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); - + applyClientAuthentication(authMethod, clientInformation, headers, params); } @@ -678,11 +678,11 @@ export async function exchangeAuthorization( /** * Exchange a refresh token for an updated access token. - * + * * Supports multiple client authentication methods as specified in OAuth 2.1: * - Automatically selects the best authentication method based on server support * - Preserves the original refresh token if a new one is not returned - * + * * @param authorizationServerUrl - The authorization server's base URL * @param options - Configuration object containing client info, refresh token, etc. * @returns Promise resolving to OAuth tokens (preserves original refresh_token if not replaced) @@ -732,12 +732,12 @@ export async function refreshAuthorization( }); if (addClientAuthentication) { - addClientAuthentication(tokenUrl, headers, params); + 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); - + applyClientAuthentication(authMethod, clientInformation, headers, params); } From 6dcda4375f8a40f05a3a069d687f31fc4fd4456e Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Tue, 1 Jul 2025 20:47:59 -0700 Subject: [PATCH 2/3] Fix tests. --- src/client/auth.test.ts | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index e4f5bc9ae..eb09a2adb 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -232,7 +232,7 @@ describe("OAuth Authorization", () => { ok: false, status: 404, }); - + // Second call (root fallback) succeeds mockFetch.mockResolvedValueOnce({ ok: true, @@ -242,17 +242,17 @@ describe("OAuth Authorization", () => { const metadata = await discoverOAuthMetadata("https://auth.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://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 const [secondUrl, secondOptions] = calls[1]; expect(secondUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); @@ -267,7 +267,7 @@ describe("OAuth Authorization", () => { ok: false, status: 404, }); - + // Second call (root fallback) also returns 404 mockFetch.mockResolvedValueOnce({ ok: false, @@ -276,7 +276,7 @@ describe("OAuth Authorization", () => { const metadata = await discoverOAuthMetadata("https://auth.example.com/path/name"); expect(metadata).toBeUndefined(); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); }); @@ -290,10 +290,10 @@ describe("OAuth Authorization", () => { 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 - + const [url] = calls[0]; expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); }); @@ -307,10 +307,10 @@ describe("OAuth Authorization", () => { 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 - + const [url] = calls[0]; expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); }); @@ -318,13 +318,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, @@ -334,10 +334,10 @@ describe("OAuth Authorization", () => { const metadata = await discoverOAuthMetadata("https://auth.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://auth.example.com/.well-known/oauth-authorization-server"); @@ -645,9 +645,9 @@ describe("OAuth Authorization", () => { authorizationCode: "code123", codeVerifier: "verifier123", redirectUri: "http://localhost:3000/callback", - addClientAuthentication: (url: URL, headers: Headers, params: URLSearchParams) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL) => { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); - params.set("example_url", url.toString()); + params.set("example_url", typeof url === 'string' ? url : url.toString()); params.set("example_param", "example_value"); }, }); @@ -671,7 +671,7 @@ describe("OAuth Authorization", () => { expect(body.get("code_verifier")).toBe("verifier123"); expect(body.get("client_id")).toBeNull(); expect(body.get("redirect_uri")).toBe("http://localhost:3000/callback"); - expect(body.get("example_url")).toBe("https://auth.example.com/token"); + expect(body.get("example_url")).toBe("https://auth.example.com"); expect(body.get("example_param")).toBe("example_value"); expect(body.get("client_secret")).toBeNull(); }); @@ -775,9 +775,9 @@ describe("OAuth Authorization", () => { const tokens = await refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", - addClientAuthentication: (url: URL, headers: Headers, params: URLSearchParams) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL) => { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); - params.set("example_url", url.toString()); + params.set("example_url", typeof url === 'string' ? url : url.toString()); params.set("example_param", "example_value"); }, }); @@ -799,7 +799,7 @@ describe("OAuth Authorization", () => { expect(body.get("grant_type")).toBe("refresh_token"); expect(body.get("refresh_token")).toBe("refresh123"); expect(body.get("client_id")).toBeNull(); - expect(body.get("example_url")).toBe("https://auth.example.com/token"); + expect(body.get("example_url")).toBe("https://auth.example.com"); expect(body.get("example_param")).toBe("example_value"); expect(body.get("client_secret")).toBeNull(); }); From 8cce4a568fc8572f37abe6be264b63234562460b Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Tue, 1 Jul 2025 20:58:21 -0700 Subject: [PATCH 3/3] Update addClientAuthentication tests for metadata. --- src/client/auth.test.ts | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index eb09a2adb..3285c37bc 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -11,6 +11,7 @@ import { auth, type OAuthClientProvider, } from "./auth.js"; +import { OAuthMetadata } from 'src/shared/auth.js'; // Mock fetch globally const mockFetch = jest.fn(); @@ -588,6 +589,13 @@ describe("OAuth Authorization", () => { refresh_token: "refresh123", }; + const validMetadata = { + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/authorize", + token_endpoint: "https://auth.example.com/token", + response_types_supported: ["code"] + }; + const validClientInfo = { client_id: "client123", client_secret: "secret123", @@ -641,13 +649,15 @@ describe("OAuth Authorization", () => { }); const tokens = await exchangeAuthorization("https://auth.example.com", { + metadata: validMetadata, clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", redirectUri: "http://localhost:3000/callback", - addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL, metadata: OAuthMetadata) => { 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); params.set("example_param", "example_value"); }, }); @@ -672,6 +682,7 @@ describe("OAuth Authorization", () => { expect(body.get("client_id")).toBeNull(); expect(body.get("redirect_uri")).toBe("http://localhost:3000/callback"); expect(body.get("example_url")).toBe("https://auth.example.com"); + expect(body.get("example_metadata")).toBe("https://auth.example.com/authorize"); expect(body.get("example_param")).toBe("example_value"); expect(body.get("client_secret")).toBeNull(); }); @@ -724,6 +735,13 @@ describe("OAuth Authorization", () => { refresh_token: "newrefresh123", }; + const validMetadata = { + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/authorize", + token_endpoint: "https://auth.example.com/token", + response_types_supported: ["code"] + }; + const validClientInfo = { client_id: "client123", client_secret: "secret123", @@ -773,11 +791,13 @@ describe("OAuth Authorization", () => { }); const tokens = await refreshAuthorization("https://auth.example.com", { + metadata: validMetadata, clientInformation: validClientInfo, refreshToken: "refresh123", - addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL, metadata: OAuthMetadata) => { 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); params.set("example_param", "example_value"); }, }); @@ -800,6 +820,7 @@ describe("OAuth Authorization", () => { expect(body.get("refresh_token")).toBe("refresh123"); expect(body.get("client_id")).toBeNull(); expect(body.get("example_url")).toBe("https://auth.example.com"); + expect(body.get("example_metadata")).toBe("https://auth.example.com/authorize"); expect(body.get("example_param")).toBe("example_value"); expect(body.get("client_secret")).toBeNull(); });