From 39a32561b2dcc1a34b2f51897383ccd955ddf100 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 14 Jul 2026 11:58:23 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=9D=20fix:=20Discover=20MCP=20OAuth=20?= =?UTF-8?q?Exchange=20Methods=20(#14256)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: discover MCP OAuth exchange methods * fix: bound configured OAuth discovery * fix: preserve configured OAuth resource discovery * test: model absent OAuth resource metadata --- .../MCPServerDialog/hooks/useMCPServerForm.ts | 34 +- .../MCPServerDialog/sections/AuthSection.tsx | 42 ++- .../MCPServerDialog/utils/oauth.spec.ts | 37 +++ .../MCPBuilder/MCPServerDialog/utils/oauth.ts | 32 ++ .../api/src/mcp/__tests__/handler.test.ts | 180 +++++++++++ packages/api/src/mcp/oauth/handler.ts | 298 ++++++++++++++---- 6 files changed, 541 insertions(+), 82 deletions(-) create mode 100644 client/src/components/SidePanel/MCPBuilder/MCPServerDialog/utils/oauth.spec.ts create mode 100644 client/src/components/SidePanel/MCPBuilder/MCPServerDialog/utils/oauth.ts diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/hooks/useMCPServerForm.ts b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/hooks/useMCPServerForm.ts index 7f42eee19b..b677ed8bfc 100644 --- a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/hooks/useMCPServerForm.ts +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/hooks/useMCPServerForm.ts @@ -1,15 +1,16 @@ import { useEffect, useMemo, useCallback, useState } from 'react'; import { useForm } from 'react-hook-form'; -import type { MCPServerCreateParams } from 'librechat-data-provider'; +import { useToastContext } from '@librechat/client'; +import type { MCPServerCreateParams, TokenExchangeMethodEnum } from 'librechat-data-provider'; +import type { MCPServerDefinition } from '~/hooks'; import { useCreateMCPServerMutation, useUpdateMCPServerMutation, useDeleteMCPServerMutation, } from '~/data-provider/MCP'; -import { useToastContext } from '@librechat/client'; -import { useLocalize } from '~/hooks'; import { extractServerNameFromUrl, isValidUrl, normalizeUrl } from '../utils/urlUtils'; -import type { MCPServerDefinition } from '~/hooks'; +import { getOAuthConfig } from '../utils/oauth'; +import { useLocalize } from '~/hooks'; // Auth type enum export enum AuthTypeEnum { @@ -38,6 +39,7 @@ export interface AuthConfig { oauth_authorization_url?: string; oauth_token_url?: string; oauth_scope?: string; + oauth_token_exchange_method?: TokenExchangeMethodEnum; obo_scopes?: string; server_id?: string; } @@ -108,6 +110,7 @@ export function useMCPServerForm({ server, onSuccess, onClose }: UseMCPServerFor oauth_authorization_url: server.config.oauth?.authorization_url || '', oauth_token_url: server.config.oauth?.token_url || '', oauth_scope: server.config.oauth?.scope || '', + oauth_token_exchange_method: server.config.oauth?.token_exchange_method, obo_scopes: 'obo' in server.config && server.config.obo ? server.config.obo.scopes : '', server_id: server.serverName, }, @@ -132,6 +135,7 @@ export function useMCPServerForm({ server, onSuccess, onClose }: UseMCPServerFor oauth_authorization_url: '', oauth_token_url: '', oauth_scope: '', + oauth_token_exchange_method: undefined, obo_scopes: '', }, trust: false, @@ -189,25 +193,9 @@ export function useMCPServerForm({ server, onSuccess, onClose }: UseMCPServerFor }; // Add OAuth configuration - if ( - formData.auth.auth_type === AuthTypeEnum.OAuth && - (formData.auth.oauth_client_id || - formData.auth.oauth_client_secret || - formData.auth.oauth_authorization_url || - formData.auth.oauth_token_url || - formData.auth.oauth_scope) - ) { - config.oauth = { - ...(formData.auth.oauth_client_id && { client_id: formData.auth.oauth_client_id }), - ...(formData.auth.oauth_client_secret && { - client_secret: formData.auth.oauth_client_secret, - }), - ...(formData.auth.oauth_authorization_url && { - authorization_url: formData.auth.oauth_authorization_url, - }), - ...(formData.auth.oauth_token_url && { token_url: formData.auth.oauth_token_url }), - ...(formData.auth.oauth_scope && { scope: formData.auth.oauth_scope }), - }; + const oauthConfig = getOAuthConfig(formData.auth); + if (oauthConfig) { + config.oauth = oauthConfig; } // Add API Key configuration diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx index 216a68151e..12133fce5f 100644 --- a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx @@ -1,7 +1,7 @@ import { useMemo, useState } from 'react'; import { Copy, CopyCheck } from 'lucide-react'; import { useFormContext, useWatch } from 'react-hook-form'; -import { Permissions, PermissionTypes } from 'librechat-data-provider'; +import { Permissions, PermissionTypes, TokenExchangeMethodEnum } from 'librechat-data-provider'; import { Label, Input, Checkbox, SecretInput, Radio, useToastContext } from '@librechat/client'; import type { MCPServerFormData } from '../hooks/useMCPServerForm'; import { AuthTypeEnum, AuthorizationTypeEnum } from '../hooks/useMCPServerForm'; @@ -14,6 +14,8 @@ interface AuthSectionProps { serverName?: string; } +const AUTO_TOKEN_EXCHANGE_METHOD = 'auto'; + export default function AuthSection({ isEditMode, serverName }: AuthSectionProps) { const localize = useLocalize(); const { showToast } = useToastContext(); @@ -42,6 +44,10 @@ export default function AuthSection({ isEditMode, serverName }: AuthSectionProps name: 'auth.api_key_authorization_type', }) as AuthorizationTypeEnum; + const tokenExchangeMethod = useWatch({ + name: 'auth.oauth_token_exchange_method', + }); + const redirectUri = serverName ? `${window.location.origin}/api/mcp/${serverName}/oauth/callback` : ''; @@ -265,6 +271,40 @@ export default function AuthSection({ isEditMode, serverName }: AuthSectionProps + {/* Token exchange method */} +
+ + + + + setValue( + 'auth.oauth_token_exchange_method', + value === AUTO_TOKEN_EXCHANGE_METHOD + ? undefined + : (value as TokenExchangeMethodEnum), + { shouldDirty: true }, + ) + } + fullWidth + aria-labelledby="oauth-token-exchange-method-label" + /> +
+ {/* Redirect URI */} {isEditMode && redirectUri && (
diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/utils/oauth.spec.ts b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/utils/oauth.spec.ts new file mode 100644 index 0000000000..8e202b5e77 --- /dev/null +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/utils/oauth.spec.ts @@ -0,0 +1,37 @@ +import { TokenExchangeMethodEnum } from 'librechat-data-provider'; +import { getOAuthConfig } from './oauth'; + +describe('getOAuthConfig', () => { + it('serializes an explicit token exchange method', () => { + expect( + getOAuthConfig({ + auth_type: 'oauth', + oauth_client_id: 'client-id', + oauth_token_exchange_method: TokenExchangeMethodEnum.DefaultPost, + }), + ).toEqual({ + client_id: 'client-id', + token_exchange_method: TokenExchangeMethodEnum.DefaultPost, + }); + }); + + it('omits the exchange method when automatic discovery is selected', () => { + expect( + getOAuthConfig({ + auth_type: 'oauth', + oauth_client_id: 'client-id', + oauth_token_exchange_method: undefined, + }), + ).toEqual({ client_id: 'client-id' }); + }); + + it('does not create OAuth config for another authentication type', () => { + expect( + getOAuthConfig({ + auth_type: 'none', + oauth_client_id: 'stale-client-id', + oauth_token_exchange_method: TokenExchangeMethodEnum.BasicAuthHeader, + }), + ).toBeUndefined(); + }); +}); diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/utils/oauth.ts b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/utils/oauth.ts new file mode 100644 index 0000000000..323a724363 --- /dev/null +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/utils/oauth.ts @@ -0,0 +1,32 @@ +import type { MCPOptions, TokenExchangeMethodEnum } from 'librechat-data-provider'; + +interface OAuthFormConfig { + auth_type: string; + oauth_client_id?: string; + oauth_client_secret?: string; + oauth_authorization_url?: string; + oauth_token_url?: string; + oauth_scope?: string; + oauth_token_exchange_method?: TokenExchangeMethodEnum; +} + +export function getOAuthConfig( + auth: OAuthFormConfig, +): NonNullable | undefined { + if (auth.auth_type !== 'oauth') { + return undefined; + } + + const oauth: NonNullable = { + ...(auth.oauth_client_id && { client_id: auth.oauth_client_id }), + ...(auth.oauth_client_secret && { client_secret: auth.oauth_client_secret }), + ...(auth.oauth_authorization_url && { authorization_url: auth.oauth_authorization_url }), + ...(auth.oauth_token_url && { token_url: auth.oauth_token_url }), + ...(auth.oauth_scope && { scope: auth.oauth_scope }), + ...(auth.oauth_token_exchange_method && { + token_exchange_method: auth.oauth_token_exchange_method, + }), + }; + + return Object.keys(oauth).length > 0 ? oauth : undefined; +} diff --git a/packages/api/src/mcp/__tests__/handler.test.ts b/packages/api/src/mcp/__tests__/handler.test.ts index 34c32bfbbb..8ab6999775 100644 --- a/packages/api/src/mcp/__tests__/handler.test.ts +++ b/packages/api/src/mcp/__tests__/handler.test.ts @@ -111,6 +111,186 @@ describe('MCPOAuthHandler - Configurable OAuth Metadata', () => { client_secret: 'test-client-secret', }; + it('should discover client_secret_post for a pre-registered confidential client', async () => { + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: mockServerUrl, + authorization_servers: ['https://auth.example.com'], + }); + mockDiscoverAuthorizationServerMetadata.mockResolvedValueOnce({ + issuer: 'https://auth.example.com', + authorization_endpoint: baseConfig.authorization_url, + token_endpoint: baseConfig.token_url, + token_endpoint_auth_methods_supported: ['client_secret_post'], + response_types_supported: ['code'], + grant_types_supported: ['authorization_code', 'refresh_token'], + code_challenge_methods_supported: ['S256'], + } as AuthorizationServerMetadata); + + const result = await MCPOAuthHandler.initiateOAuthFlow( + mockServerName, + mockServerUrl, + mockUserId, + {}, + baseConfig, + ); + + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalled(); + expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith( + new URL('https://auth.example.com'), + expect.objectContaining({ fetchFn: expect.any(Function) }), + ); + expect(mockStartAuthorization).toHaveBeenCalledWith( + mockServerUrl, + expect.objectContaining({ + metadata: expect.objectContaining({ + token_endpoint_auth_methods_supported: ['client_secret_post'], + code_challenge_methods_supported: ['S256'], + }), + clientInformation: expect.objectContaining({ + token_endpoint_auth_method: 'client_secret_post', + }), + }), + ); + expect(result.authorizationUrl).toContain('resource=https%3A%2F%2Fexample.com%2Fmcp'); + expect(result.flowMetadata.resourceMetadata).toEqual( + expect.objectContaining({ resource: mockServerUrl }), + ); + }); + + it('should discover capabilities from the configured authorization server origin', async () => { + mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValueOnce( + new Error('No resource metadata'), + ); + mockDiscoverAuthorizationServerMetadata.mockResolvedValueOnce({ + issuer: 'https://auth.example.com', + authorization_endpoint: baseConfig.authorization_url, + token_endpoint: baseConfig.token_url, + token_endpoint_auth_methods_supported: ['client_secret_post'], + response_types_supported: ['code'], + } as AuthorizationServerMetadata); + + await MCPOAuthHandler.initiateOAuthFlow( + mockServerName, + mockServerUrl, + mockUserId, + {}, + baseConfig, + ); + + expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith( + new URL('https://auth.example.com'), + expect.objectContaining({ fetchFn: expect.any(Function) }), + ); + expect(mockStartAuthorization).toHaveBeenCalledWith( + mockServerUrl, + expect.objectContaining({ + clientInformation: expect.objectContaining({ + token_endpoint_auth_method: 'client_secret_post', + }), + }), + ); + }); + + it('should preserve resource discovery while preferring an explicit exchange method', async () => { + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: mockServerUrl, + authorization_servers: ['https://auth.example.com'], + }); + + const result = await MCPOAuthHandler.initiateOAuthFlow( + mockServerName, + mockServerUrl, + mockUserId, + {}, + { + ...baseConfig, + token_exchange_method: TokenExchangeMethodEnum.BasicAuthHeader, + }, + ); + + expect(mockStartAuthorization).toHaveBeenCalledWith( + mockServerUrl, + expect.objectContaining({ + clientInformation: expect.objectContaining({ + token_endpoint_auth_method: 'client_secret_basic', + }), + }), + ); + expect(mockDiscoverOAuthProtectedResourceMetadata).toHaveBeenCalled(); + expect(mockDiscoverAuthorizationServerMetadata).not.toHaveBeenCalled(); + expect(result.authorizationUrl).toContain('resource=https%3A%2F%2Fexample.com%2Fmcp'); + expect(result.flowMetadata.resourceMetadata).toEqual( + expect.objectContaining({ resource: mockServerUrl }), + ); + }); + + it('should fall back when pre-configured metadata discovery times out', async () => { + mockDiscoverOAuthProtectedResourceMetadata.mockImplementationOnce( + () => new Promise(() => undefined), + ); + + await expect( + MCPOAuthHandler.initiateOAuthFlow( + mockServerName, + mockServerUrl, + mockUserId, + {}, + baseConfig, + ), + ).resolves.toEqual( + expect.objectContaining({ + authorizationUrl: expect.stringContaining('state='), + }), + ); + expect(mockStartAuthorization).toHaveBeenCalledWith( + mockServerUrl, + expect.objectContaining({ + clientInformation: expect.objectContaining({ + token_endpoint_auth_method: 'client_secret_basic', + }), + }), + ); + }); + + it('should not apply metadata from a different token endpoint', async () => { + mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValueOnce({ + resource: mockServerUrl, + authorization_servers: ['https://auth.example.com'], + }); + mockDiscoverAuthorizationServerMetadata.mockResolvedValueOnce({ + issuer: 'https://auth.example.com', + authorization_endpoint: baseConfig.authorization_url, + token_endpoint: 'https://untrusted.example.com/oauth/token', + token_endpoint_auth_methods_supported: ['client_secret_post'], + response_types_supported: ['code'], + } as AuthorizationServerMetadata); + + const result = await MCPOAuthHandler.initiateOAuthFlow( + mockServerName, + mockServerUrl, + mockUserId, + {}, + baseConfig, + ); + + expect(mockStartAuthorization).toHaveBeenCalledWith( + mockServerUrl, + expect.objectContaining({ + metadata: expect.objectContaining({ + token_endpoint: baseConfig.token_url, + token_endpoint_auth_methods_supported: ['client_secret_basic', 'client_secret_post'], + }), + clientInformation: expect.objectContaining({ + token_endpoint_auth_method: 'client_secret_basic', + }), + }), + ); + expect(result.authorizationUrl).toContain('resource=https%3A%2F%2Fexample.com%2Fmcp'); + expect(result.flowMetadata.resourceMetadata).toEqual( + expect.objectContaining({ resource: mockServerUrl }), + ); + }); + it('should use default values when OAuth metadata fields are not configured', async () => { await MCPOAuthHandler.initiateOAuthFlow( mockServerName, diff --git a/packages/api/src/mcp/oauth/handler.ts b/packages/api/src/mcp/oauth/handler.ts index 30d497b6d3..f26d508456 100644 --- a/packages/api/src/mcp/oauth/handler.ts +++ b/packages/api/src/mcp/oauth/handler.ts @@ -40,6 +40,24 @@ import { getOAuthUrlPort } from './url'; /** Type for the OAuth metadata from the SDK */ type SDKOAuthMetadata = Parameters[1]['metadata']; +type OAuthDiscoveryResult = { + metadata: OAuthMetadata; + resourceMetadata?: OAuthProtectedResourceMetadata; + authServerUrl: URL; +}; + +type OAuthResourceDiscoveryResult = { + resourceMetadata?: OAuthProtectedResourceMetadata; + authServerUrl?: URL; +}; + +type PreconfiguredOAuthDiscoveryResult = { + metadata?: OAuthMetadata; + resourceMetadata?: OAuthProtectedResourceMetadata; +}; + +const PRECONFIGURED_DISCOVERY_TIMEOUT_MS = 5_000; + export class MCPOAuthHandler { private static readonly FLOW_TYPE = 'mcp_oauth'; @@ -141,25 +159,76 @@ export class MCPOAuthHandler { oauthHeaders: Record, allowedDomains?: string[] | null, allowedAddresses?: string[] | null, - ): Promise<{ - metadata: OAuthMetadata; - resourceMetadata?: OAuthProtectedResourceMetadata; - authServerUrl: URL; - }> { + signal?: AbortSignal, + ): Promise { logger.debug( `[MCPOAuth] discoverMetadata called with serverUrl: ${sanitizeUrlForLogging(serverUrl)}`, ); - let authServerUrl = new URL(serverUrl); - let resourceMetadata: OAuthProtectedResourceMetadata | undefined; - const fetchFn = this.createOAuthFetch( oauthHeaders, undefined, allowedDomains, allowedAddresses, + signal, ); + const resourceDiscovery = await this.discoverResourceMetadata( + serverUrl, + fetchFn, + allowedDomains, + allowedAddresses, + ); + const resourceMetadata = resourceDiscovery.resourceMetadata; + const authServerUrl = resourceDiscovery.authServerUrl ?? new URL(serverUrl); + const metadata = await this.discoverAuthorizationMetadata( + authServerUrl, + fetchFn, + allowedDomains, + allowedAddresses, + ); + + if (metadata) { + return { metadata, resourceMetadata, authServerUrl }; + } + + /** + * No metadata discovered - create fallback metadata using default OAuth endpoint paths. + * This mirrors the MCP SDK's behavior where it falls back to /authorize, /token, /register + * when metadata discovery fails (e.g., servers without .well-known endpoints). + * See: https://github.com/modelcontextprotocol/sdk/blob/main/src/client/auth.ts + */ + logger.warn( + `[MCPOAuth] No OAuth metadata discovered from ${sanitizeUrlForLogging(authServerUrl)}, using legacy fallback endpoints`, + ); + + const fallbackMetadata: OAuthMetadata = { + issuer: authServerUrl.toString(), + authorization_endpoint: new URL('/authorize', authServerUrl).toString(), + token_endpoint: new URL('/token', authServerUrl).toString(), + registration_endpoint: new URL('/register', authServerUrl).toString(), + response_types_supported: ['code'], + grant_types_supported: ['authorization_code', 'refresh_token'], + code_challenge_methods_supported: ['S256', 'plain'], + token_endpoint_auth_methods_supported: ['client_secret_basic', 'client_secret_post', 'none'], + }; + + logger.debug(`[MCPOAuth] Using fallback metadata:`, fallbackMetadata); + return { + metadata: fallbackMetadata, + resourceMetadata, + authServerUrl, + }; + } + + private static async discoverResourceMetadata( + serverUrl: string, + fetchFn: FetchLike, + allowedDomains?: string[] | null, + allowedAddresses?: string[] | null, + ): Promise { + let resourceMetadata: OAuthProtectedResourceMetadata | undefined; + /** * RFC 9728 ยง5.1: when the server's 401 `WWW-Authenticate` header advertises a * `resource_metadata` URL, use that URL as the authoritative source. Path-aware @@ -235,53 +304,32 @@ export class MCPOAuthHandler { allowedDomains, allowedAddresses, ); - authServerUrl = new URL(discoveredAuthServer); + const authServerUrl = new URL(discoveredAuthServer); logger.debug( `[MCPOAuth] Found authorization server from resource metadata: ${authServerUrl}`, ); + return { resourceMetadata, authServerUrl }; } else { logger.debug(`[MCPOAuth] No authorization servers found in resource metadata`); } } - // Discover OAuth metadata + return { resourceMetadata }; + } + + private static async discoverAuthorizationMetadata( + authServerUrl: URL, + fetchFn: FetchLike, + allowedDomains?: string[] | null, + allowedAddresses?: string[] | null, + ): Promise { logger.debug( `[MCPOAuth] Discovering OAuth metadata from ${sanitizeUrlForLogging(authServerUrl)}`, ); const rawMetadata = await this.discoverWithOriginFallback(authServerUrl, fetchFn); if (!rawMetadata) { - /** - * No metadata discovered - create fallback metadata using default OAuth endpoint paths. - * This mirrors the MCP SDK's behavior where it falls back to /authorize, /token, /register - * when metadata discovery fails (e.g., servers without .well-known endpoints). - * See: https://github.com/modelcontextprotocol/sdk/blob/main/src/client/auth.ts - */ - logger.warn( - `[MCPOAuth] No OAuth metadata discovered from ${sanitizeUrlForLogging(authServerUrl)}, using legacy fallback endpoints`, - ); - - const fallbackMetadata: OAuthMetadata = { - issuer: authServerUrl.toString(), - authorization_endpoint: new URL('/authorize', authServerUrl).toString(), - token_endpoint: new URL('/token', authServerUrl).toString(), - registration_endpoint: new URL('/register', authServerUrl).toString(), - response_types_supported: ['code'], - grant_types_supported: ['authorization_code', 'refresh_token'], - code_challenge_methods_supported: ['S256', 'plain'], - token_endpoint_auth_methods_supported: [ - 'client_secret_basic', - 'client_secret_post', - 'none', - ], - }; - - logger.debug(`[MCPOAuth] Using fallback metadata:`, fallbackMetadata); - return { - metadata: fallbackMetadata, - resourceMetadata, - authServerUrl, - }; + return undefined; } logger.debug(`[MCPOAuth] OAuth metadata discovered successfully`); @@ -313,11 +361,73 @@ export class MCPOAuthHandler { } logger.debug(`[MCPOAuth] OAuth metadata parsed successfully`); - return { - metadata: metadata as unknown as OAuthMetadata, - resourceMetadata, - authServerUrl, - }; + return metadata as unknown as OAuthMetadata; + } + + private static discoverPreconfiguredMetadataWithTimeout( + serverUrl: string, + authorizationUrl: string, + discoverCapabilities: boolean, + oauthHeaders: Record, + allowedDomains?: string[] | null, + allowedAddresses?: string[] | null, + ): Promise { + const controller = new AbortController(); + let partialResult: PreconfiguredOAuthDiscoveryResult = {}; + + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + controller.abort(); + logger.warn( + `[MCPOAuth] Pre-configured OAuth metadata discovery timed out after ${PRECONFIGURED_DISCOVERY_TIMEOUT_MS}ms; using available metadata and configured defaults.`, + ); + resolve(partialResult); + }, PRECONFIGURED_DISCOVERY_TIMEOUT_MS); + + const fetchFn = this.createOAuthFetch( + oauthHeaders, + undefined, + allowedDomains, + allowedAddresses, + controller.signal, + ); + + void this.discoverResourceMetadata(serverUrl, fetchFn, allowedDomains, allowedAddresses) + .then(async (resourceDiscovery) => { + partialResult = { resourceMetadata: resourceDiscovery.resourceMetadata }; + if (!discoverCapabilities) { + return partialResult; + } + + const authServerUrl = + resourceDiscovery.authServerUrl ?? new URL(new URL(authorizationUrl).origin); + try { + const metadata = await this.discoverAuthorizationMetadata( + authServerUrl, + fetchFn, + allowedDomains, + allowedAddresses, + ); + return { ...partialResult, metadata }; + } catch (error) { + logger.warn( + `[MCPOAuth] Authorization server metadata discovery failed for pre-configured client; using configured endpoints and defaults`, + { error }, + ); + return partialResult; + } + }) + .then( + (result) => { + clearTimeout(timeout); + resolve(result); + }, + (error) => { + clearTimeout(timeout); + reject(error); + }, + ); + }); } /** @@ -530,6 +640,61 @@ export class MCPOAuthHandler { this.validateOAuthUrl(config.token_url, 'token_url', allowedDomains, allowedAddresses), ]); + let discoveredMetadata: OAuthMetadata | undefined; + let resourceMetadata: OAuthProtectedResourceMetadata | undefined; + const shouldDiscoverCapabilities = + !!config.client_secret && + config.token_exchange_method === undefined && + config.token_endpoint_auth_methods_supported === undefined; + + try { + const discovery = await this.discoverPreconfiguredMetadataWithTimeout( + serverUrl, + config.authorization_url, + shouldDiscoverCapabilities, + oauthHeaders, + allowedDomains, + allowedAddresses, + ); + resourceMetadata = discovery.resourceMetadata; + + if (shouldDiscoverCapabilities && discovery.metadata) { + const discoveredTokenEndpoint = discovery.metadata.token_endpoint; + const configuredTokenEndpoint = new URL(config.token_url).href; + + /** + * Pre-registered credentials are bound to the configured token endpoint. Metadata + * discovery may supply capabilities for that endpoint, but must never redirect the + * client secret to a different endpoint. + */ + if ( + discoveredTokenEndpoint && + new URL(discoveredTokenEndpoint).href === configuredTokenEndpoint + ) { + discoveredMetadata = discovery.metadata; + logger.debug( + `[MCPOAuth] Using discovered OAuth capabilities with pre-configured endpoints for ${serverName}`, + ); + } else { + logger.warn( + `[MCPOAuth] Ignoring discovered OAuth capabilities for ${serverName} because the token endpoint does not match the configured endpoint`, + { + configuredTokenEndpoint: sanitizeUrlForLogging(configuredTokenEndpoint), + discoveredTokenEndpoint: discoveredTokenEndpoint + ? sanitizeUrlForLogging(discoveredTokenEndpoint) + : undefined, + }, + ); + } + } + } catch (error) { + /** Preserve compatibility with OAuth providers that do not publish metadata. */ + logger.warn( + `[MCPOAuth] OAuth metadata discovery failed for pre-configured client ${serverName}; using configured endpoints and defaults`, + { error }, + ); + } + const skipCodeChallengeCheck = config?.skip_code_challenge_check === true || process.env.MCP_SKIP_CODE_CHALLENGE_CHECK === 'true'; @@ -543,7 +708,10 @@ export class MCPOAuthHandler { `[MCPOAuth] Code challenge check skip enabled, forcing S256 support for ${serverName}`, ); } else { - codeChallengeMethodsSupported = ['S256', 'plain']; + codeChallengeMethodsSupported = discoveredMetadata?.code_challenge_methods_supported ?? [ + 'S256', + 'plain', + ]; } /** Metadata based on pre-configured settings */ @@ -551,10 +719,14 @@ export class MCPOAuthHandler { if (!config.client_secret) { tokenEndpointAuthMethod = 'none'; } else { - // When token_exchange_method is undefined or not DefaultPost, default to using - // client_secret_basic (Basic Auth header) for token endpoint authentication. tokenEndpointAuthMethod = - getForcedTokenEndpointAuthMethod(config.token_exchange_method) ?? 'client_secret_basic'; + resolveTokenEndpointAuthMethod({ + tokenExchangeMethod: config.token_exchange_method, + tokenAuthMethods: + config.token_endpoint_auth_methods_supported ?? + discoveredMetadata?.token_endpoint_auth_methods_supported ?? + [], + }) ?? 'client_secret_basic'; } let defaultTokenAuthMethods: string[]; @@ -569,15 +741,16 @@ export class MCPOAuthHandler { const metadata: OAuthMetadata = { authorization_endpoint: config.authorization_url, token_endpoint: config.token_url, - issuer: serverUrl, - scopes_supported: config.scope?.split(' ') ?? [], - grant_types_supported: config?.grant_types_supported ?? [ - 'authorization_code', - 'refresh_token', - ], + issuer: discoveredMetadata?.issuer ?? serverUrl, + scopes_supported: config.scope?.split(' ') ?? discoveredMetadata?.scopes_supported ?? [], + grant_types_supported: config?.grant_types_supported ?? + discoveredMetadata?.grant_types_supported ?? ['authorization_code', 'refresh_token'], token_endpoint_auth_methods_supported: - config?.token_endpoint_auth_methods_supported ?? defaultTokenAuthMethods, - response_types_supported: config?.response_types_supported ?? ['code'], + config?.token_endpoint_auth_methods_supported ?? + discoveredMetadata?.token_endpoint_auth_methods_supported ?? + defaultTokenAuthMethods, + response_types_supported: config?.response_types_supported ?? + discoveredMetadata?.response_types_supported ?? ['code'], code_challenge_methods_supported: codeChallengeMethodsSupported, }; logger.debug(`[MCPOAuth] metadata for "${serverName}": ${JSON.stringify(metadata)}`); @@ -602,6 +775,14 @@ export class MCPOAuthHandler { authorizationUrl.searchParams.set('state', state); logger.debug(`[MCPOAuth] Added state parameter to authorization URL`); + if (resourceMetadata?.resource) { + const canonicalResource = new URL(resourceMetadata.resource).href; + authorizationUrl.searchParams.set('resource', canonicalResource); + logger.debug( + `[MCPOAuth] Added resource parameter to pre-configured authorization URL: ${canonicalResource}`, + ); + } + /** * Auth0/Cognito-style `audience` parameter. Forwarded as-is; the provider * decides whether it accepts RFC 8707 `resource`, the legacy `audience`, @@ -620,6 +801,7 @@ export class MCPOAuthHandler { codeVerifier, clientInfo, metadata, + resourceMetadata, ...(allowedDomains !== undefined && { allowedDomains }), ...(allowedAddresses !== undefined && { allowedAddresses }), ...(Object.keys(oauthHeaders).length > 0 && { oauthHeaders }),