diff --git a/api/server/routes/mcp.js b/api/server/routes/mcp.js index 8d877417ad..39c4f4fa43 100644 --- a/api/server/routes/mcp.js +++ b/api/server/routes/mcp.js @@ -429,13 +429,23 @@ router.get('/connection/status', requireJwtAuth, async (req, res) => { const connectionStatus = {}; for (const [serverName] of Object.entries(mcpConfig)) { - connectionStatus[serverName] = await getServerConnectionStatus( - user.id, - serverName, - appConnections, - userConnections, - oauthServers, - ); + try { + connectionStatus[serverName] = await getServerConnectionStatus( + user.id, + serverName, + appConnections, + userConnections, + oauthServers, + ); + } catch (error) { + const message = `Failed to get status for server "${serverName}"`; + logger.error(`[MCP Connection Status] ${message},`, error); + connectionStatus[serverName] = { + connectionState: 'error', + requiresOAuth: oauthServers.has(serverName), + error: message, + }; + } } res.json({ diff --git a/packages/api/src/mcp/__tests__/detectOAuth.integration.dev.ts b/packages/api/src/mcp/__tests__/detectOAuth.integration.dev.ts index 7881b9bd65..01dd8a5d28 100644 --- a/packages/api/src/mcp/__tests__/detectOAuth.integration.dev.ts +++ b/packages/api/src/mcp/__tests__/detectOAuth.integration.dev.ts @@ -25,7 +25,7 @@ describe('OAuth Detection Integration Tests', () => { name: 'GitHub Copilot MCP Server', url: 'https://api.githubcopilot.com/mcp', expectedOAuth: true, - expectedMethod: '401-challenge-metadata', + expectedMethod: 'protected-resource-metadata', withMeta: true, }, { @@ -42,6 +42,13 @@ describe('OAuth Detection Integration Tests', () => { expectedMethod: 'protected-resource-metadata', withMeta: true, }, + { + name: 'StackOverflow MCP (HEAD=405, POST=401+Bearer)', + url: 'https://mcp.stackoverflow.com', + expectedOAuth: true, + expectedMethod: '401-challenge-metadata', + withMeta: false, + }, { name: 'HTTPBin (Non-OAuth)', url: 'https://httpbin.org', diff --git a/packages/api/src/mcp/__tests__/handler.test.ts b/packages/api/src/mcp/__tests__/handler.test.ts index 24e8c5ddb4..f7347b8bbe 100644 --- a/packages/api/src/mcp/__tests__/handler.test.ts +++ b/packages/api/src/mcp/__tests__/handler.test.ts @@ -992,4 +992,147 @@ describe('MCPOAuthHandler - Configurable OAuth Metadata', () => { expect(headers.get('foo')).toBe('bar'); }); }); + + describe('Fallback OAuth Metadata (Legacy Server Support)', () => { + const originalFetch = global.fetch; + const mockFetch = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + global.fetch = mockFetch as unknown as typeof fetch; + }); + + afterAll(() => { + global.fetch = originalFetch; + }); + + it('should use fallback metadata when discoverAuthorizationServerMetadata returns undefined', async () => { + // Mock resource metadata discovery to fail + mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValueOnce( + new Error('No resource metadata'), + ); + + // Mock authorization server metadata discovery to return undefined (no .well-known) + mockDiscoverAuthorizationServerMetadata.mockResolvedValueOnce(undefined); + + // Mock client registration to succeed + mockRegisterClient.mockResolvedValueOnce({ + client_id: 'dynamic-client-id', + client_secret: 'dynamic-client-secret', + redirect_uris: ['http://localhost:3080/api/mcp/test-server/oauth/callback'], + }); + + // Mock startAuthorization to return a successful response + mockStartAuthorization.mockResolvedValueOnce({ + authorizationUrl: new URL('https://mcp.example.com/authorize?client_id=dynamic-client-id'), + codeVerifier: 'test-code-verifier', + }); + + await MCPOAuthHandler.initiateOAuthFlow( + 'test-server', + 'https://mcp.example.com', + 'user-123', + {}, + undefined, + ); + + // Verify registerClient was called with fallback metadata + expect(mockRegisterClient).toHaveBeenCalledWith( + 'https://mcp.example.com/', + expect.objectContaining({ + metadata: expect.objectContaining({ + 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'], + 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', + ], + }), + }), + ); + }); + + it('should use fallback /token endpoint for refresh when metadata discovery fails', async () => { + const metadata = { + serverName: 'test-server', + serverUrl: 'https://mcp.example.com', + clientInfo: { + client_id: 'test-client-id', + client_secret: 'test-client-secret', + }, + }; + + // Mock metadata discovery to return undefined (no .well-known) + mockDiscoverAuthorizationServerMetadata.mockResolvedValueOnce(undefined); + + // Mock successful token refresh + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ + access_token: 'new-access-token', + refresh_token: 'new-refresh-token', + expires_in: 3600, + }), + } as Response); + + const result = await MCPOAuthHandler.refreshOAuthTokens( + 'test-refresh-token', + metadata, + {}, + {}, + ); + + // Verify fetch was called with fallback /token endpoint + expect(mockFetch).toHaveBeenCalledWith( + 'https://mcp.example.com/token', + expect.objectContaining({ + method: 'POST', + }), + ); + + expect(result.access_token).toBe('new-access-token'); + }); + + it('should use fallback auth methods when metadata discovery fails during refresh', async () => { + const metadata = { + serverName: 'test-server', + serverUrl: 'https://mcp.example.com', + clientInfo: { + client_id: 'test-client-id', + client_secret: 'test-client-secret', + }, + }; + + // Mock metadata discovery to return undefined + mockDiscoverAuthorizationServerMetadata.mockResolvedValueOnce(undefined); + + // Mock successful token refresh + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ + access_token: 'new-access-token', + expires_in: 3600, + }), + } as Response); + + await MCPOAuthHandler.refreshOAuthTokens('test-refresh-token', metadata, {}, {}); + + // Verify it uses client_secret_basic (first in fallback auth methods) + const expectedAuth = `Basic ${Buffer.from('test-client-id:test-client-secret').toString('base64')}`; + expect(mockFetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: expectedAuth, + }), + }), + ); + }); + }); }); diff --git a/packages/api/src/mcp/oauth/detectOAuth.test.ts b/packages/api/src/mcp/oauth/detectOAuth.test.ts new file mode 100644 index 0000000000..8164adddaf --- /dev/null +++ b/packages/api/src/mcp/oauth/detectOAuth.test.ts @@ -0,0 +1,267 @@ +import { detectOAuthRequirement } from './detectOAuth'; + +jest.mock('@modelcontextprotocol/sdk/client/auth.js', () => ({ + discoverOAuthProtectedResourceMetadata: jest.fn(), +})); + +import { discoverOAuthProtectedResourceMetadata } from '@modelcontextprotocol/sdk/client/auth.js'; + +const mockDiscoverOAuthProtectedResourceMetadata = + discoverOAuthProtectedResourceMetadata as jest.MockedFunction< + typeof discoverOAuthProtectedResourceMetadata + >; + +describe('detectOAuthRequirement', () => { + const originalFetch = global.fetch; + const mockFetch = jest.fn() as unknown as jest.MockedFunction; + + beforeEach(() => { + jest.clearAllMocks(); + global.fetch = mockFetch; + mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue( + new Error('No protected resource metadata'), + ); + }); + + afterAll(() => { + global.fetch = originalFetch; + }); + + describe('POST fallback when HEAD fails', () => { + it('should try POST when HEAD returns 405 Method Not Allowed', async () => { + // HEAD returns 405 (Method Not Allowed) + mockFetch.mockResolvedValueOnce({ + status: 405, + headers: new Headers(), + } as Response); + + // POST returns 401 with Bearer + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Bearer' }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('401-challenge-metadata'); + expect(mockFetch).toHaveBeenCalledTimes(2); + + // Verify HEAD was called first + expect(mockFetch.mock.calls[0][1]).toEqual(expect.objectContaining({ method: 'HEAD' })); + + // Verify POST was called second with proper headers and body + expect(mockFetch.mock.calls[1][1]).toEqual( + expect.objectContaining({ + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({}), + }), + ); + }); + + it('should try POST when HEAD returns non-401 status', async () => { + // HEAD returns 200 OK (no auth required for HEAD) + mockFetch.mockResolvedValueOnce({ + status: 200, + headers: new Headers(), + } as Response); + + // POST returns 401 with Bearer + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Bearer' }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('should not try POST if HEAD returns 401', async () => { + // HEAD returns 401 with Bearer + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Bearer' }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + // Only HEAD should be called since it returned 401 + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + }); + + describe('Bearer detection without resource_metadata URL', () => { + it('should detect OAuth when 401 has WWW-Authenticate: Bearer (case insensitive)', async () => { + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'bearer' }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('401-challenge-metadata'); + expect(result.metadata).toBeNull(); + }); + + it('should detect OAuth when 401 has WWW-Authenticate: BEARER (uppercase)', async () => { + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'BEARER' }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('401-challenge-metadata'); + }); + + it('should detect OAuth when Bearer is part of a larger header value', async () => { + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Bearer realm="api"' }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + }); + + it('should not detect OAuth when 401 has no WWW-Authenticate header', async () => { + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers(), + } as Response); + + // POST also returns 401 without header + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers(), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(false); + expect(result.method).toBe('no-metadata-found'); + }); + + it('should not detect OAuth when 401 has non-Bearer auth scheme', async () => { + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Basic realm="api"' }), + } as Response); + + // POST also returns 401 with Basic + mockFetch.mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Basic realm="api"' }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(false); + }); + }); + + describe('resource_metadata URL in WWW-Authenticate', () => { + it('should prefer resource_metadata URL when provided with Bearer', async () => { + const metadataUrl = 'https://auth.example.com/.well-known/oauth-protected-resource'; + + mockFetch + // HEAD request - 401 with resource_metadata URL + .mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${metadataUrl}"`, + }), + } as Response) + // Metadata fetch + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ + authorization_servers: ['https://auth.example.com'], + }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('401-challenge-metadata'); + expect(result.metadata).toEqual({ + authorization_servers: ['https://auth.example.com'], + }); + }); + + it('should fall back to Bearer detection if metadata fetch fails', async () => { + const metadataUrl = 'https://auth.example.com/.well-known/oauth-protected-resource'; + + mockFetch + // HEAD request - 401 with resource_metadata URL + .mockResolvedValueOnce({ + status: 401, + headers: new Headers({ + 'www-authenticate': `Bearer resource_metadata="${metadataUrl}"`, + }), + } as Response) + // Metadata fetch fails + .mockRejectedValueOnce(new Error('Network error')); + + const result = await detectOAuthRequirement('https://mcp.example.com'); + + // Should still detect OAuth via Bearer + expect(result.requiresOAuth).toBe(true); + expect(result.metadata).toBeNull(); + }); + }); + + describe('StackOverflow-like server behavior', () => { + it('should detect OAuth for servers that return 405 for HEAD and 401+Bearer for POST', async () => { + // This mimics StackOverflow's actual behavior: + // HEAD -> 405 Method Not Allowed + // POST -> 401 with WWW-Authenticate: Bearer + + mockFetch + // HEAD returns 405 + .mockResolvedValueOnce({ + status: 405, + headers: new Headers(), + } as Response) + // POST returns 401 with Bearer + .mockResolvedValueOnce({ + status: 401, + headers: new Headers({ 'www-authenticate': 'Bearer' }), + } as Response); + + const result = await detectOAuthRequirement('https://mcp.stackoverflow.com'); + + expect(result.requiresOAuth).toBe(true); + expect(result.method).toBe('401-challenge-metadata'); + expect(result.metadata).toBeNull(); + }); + }); + + describe('error handling', () => { + it('should return no OAuth required when all checks fail', async () => { + mockFetch.mockRejectedValue(new Error('Network error')); + + const result = await detectOAuthRequirement('https://unreachable.example.com'); + + expect(result.requiresOAuth).toBe(false); + expect(result.method).toBe('no-metadata-found'); + }); + + it('should handle timeout gracefully', async () => { + mockFetch.mockImplementation( + () => new Promise((_, reject) => setTimeout(() => reject(new Error('Timeout')), 100)), + ); + + const result = await detectOAuthRequirement('https://slow.example.com'); + + expect(result.requiresOAuth).toBe(false); + }); + }); +}); diff --git a/packages/api/src/mcp/oauth/detectOAuth.ts b/packages/api/src/mcp/oauth/detectOAuth.ts index f22f5e4cd2..84d2066f4c 100644 --- a/packages/api/src/mcp/oauth/detectOAuth.ts +++ b/packages/api/src/mcp/oauth/detectOAuth.ts @@ -66,32 +66,81 @@ async function checkProtectedResourceMetadata( } } -// Checks for OAuth using 401 challenge with resource metadata URL +/** + * Checks for OAuth using 401 challenge with resource metadata URL or Bearer token. + * Tries HEAD first, then falls back to POST if HEAD doesn't return 401. + * Some servers (like StackOverflow) only return 401 for POST requests. + */ async function check401ChallengeMetadata(serverUrl: string): Promise { + // Try HEAD first (lighter weight) + const headResult = await check401WithMethod(serverUrl, 'HEAD'); + if (headResult) return headResult; + + // Fall back to POST if HEAD didn't return 401 (some servers don't support HEAD) + const postResult = await check401WithMethod(serverUrl, 'POST'); + if (postResult) return postResult; + + return null; +} + +async function check401WithMethod( + serverUrl: string, + method: 'HEAD' | 'POST', +): Promise { try { - const response = await fetch(serverUrl, { - method: 'HEAD', + const fetchOptions: RequestInit = { + method, signal: AbortSignal.timeout(mcpConfig.OAUTH_DETECTION_TIMEOUT), - }); + }; + + // POST requests need headers and body for MCP servers + if (method === 'POST') { + fetchOptions.headers = { 'Content-Type': 'application/json' }; + fetchOptions.body = JSON.stringify({}); + } + + const response = await fetch(serverUrl, fetchOptions); if (response.status !== 401) return null; const wwwAuth = response.headers.get('www-authenticate'); const metadataUrl = wwwAuth?.match(/resource_metadata="([^"]+)"/)?.[1]; - if (!metadataUrl) return null; - const metadataResponse = await fetch(metadataUrl, { - signal: AbortSignal.timeout(mcpConfig.OAUTH_DETECTION_TIMEOUT), - }); - const metadata = await metadataResponse.json(); + if (metadataUrl) { + try { + // Try to fetch resource metadata from the provided URL + const metadataResponse = await fetch(metadataUrl, { + signal: AbortSignal.timeout(mcpConfig.OAUTH_DETECTION_TIMEOUT), + }); + const metadata = await metadataResponse.json(); - if (!metadata?.authorization_servers?.length) return null; + if (metadata?.authorization_servers?.length) { + return { + requiresOAuth: true, + method: '401-challenge-metadata', + metadata, + }; + } + } catch { + // Metadata fetch failed, continue to Bearer check below + } + } - return { - requiresOAuth: true, - method: '401-challenge-metadata', - metadata, - }; + /** + * If we got a 401 with WWW-Authenticate containing "Bearer" (case-insensitive), + * the server requires OAuth authentication even without discovery metadata. + * This handles "legacy" OAuth servers (like StackOverflow's MCP) that use standard + * OAuth endpoints (/authorize, /token, /register) without .well-known metadata. + */ + if (wwwAuth && /bearer/i.test(wwwAuth)) { + return { + requiresOAuth: true, + method: '401-challenge-metadata', + metadata: null, + }; + } + + return null; } catch { return null; } diff --git a/packages/api/src/mcp/oauth/handler.ts b/packages/api/src/mcp/oauth/handler.ts index 2357e0c606..894ad09250 100644 --- a/packages/api/src/mcp/oauth/handler.ts +++ b/packages/api/src/mcp/oauth/handler.ts @@ -93,10 +93,37 @@ export class MCPOAuthHandler { }); if (!rawMetadata) { - logger.error( - `[MCPOAuth] Failed to discover OAuth metadata from ${sanitizeUrlForLogging(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`, ); - throw new Error('Failed to discover OAuth metadata'); + + 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, + }; } logger.debug(`[MCPOAuth] OAuth metadata discovered successfully`); @@ -562,13 +589,21 @@ export class MCPOAuthHandler { fetchFn: this.createOAuthFetch(oauthHeaders), }); if (!oauthMetadata) { - throw new Error('Failed to discover OAuth metadata for token refresh'); - } - if (!oauthMetadata.token_endpoint) { + /** + * No metadata discovered - use fallback /token endpoint. + * This mirrors the MCP SDK's behavior for legacy servers without .well-known endpoints. + */ + logger.warn( + `[MCPOAuth] No OAuth metadata discovered for token refresh, using fallback /token endpoint`, + ); + tokenUrl = new URL('/token', metadata.serverUrl).toString(); + authMethods = ['client_secret_basic', 'client_secret_post', 'none']; + } else if (!oauthMetadata.token_endpoint) { throw new Error('No token endpoint found in OAuth metadata'); + } else { + tokenUrl = oauthMetadata.token_endpoint; + authMethods = oauthMetadata.token_endpoint_auth_methods_supported; } - tokenUrl = oauthMetadata.token_endpoint; - authMethods = oauthMetadata.token_endpoint_auth_methods_supported; } const body = new URLSearchParams({ @@ -741,12 +776,20 @@ export class MCPOAuthHandler { fetchFn: this.createOAuthFetch(oauthHeaders), }); + let tokenUrl: URL; if (!oauthMetadata?.token_endpoint) { - throw new Error('No token endpoint found in OAuth metadata'); + /** + * No metadata or token_endpoint discovered - use fallback /token endpoint. + * This mirrors the MCP SDK's behavior for legacy servers without .well-known endpoints. + */ + logger.warn( + `[MCPOAuth] No OAuth metadata or token endpoint found, using fallback /token endpoint`, + ); + tokenUrl = new URL('/token', metadata.serverUrl); + } else { + tokenUrl = new URL(oauthMetadata.token_endpoint); } - const tokenUrl = new URL(oauthMetadata.token_endpoint); - const body = new URLSearchParams({ grant_type: 'refresh_token', refresh_token: refreshToken, diff --git a/packages/api/src/mcp/oauth/types.ts b/packages/api/src/mcp/oauth/types.ts index 2ea4916807..178e20e35b 100644 --- a/packages/api/src/mcp/oauth/types.ts +++ b/packages/api/src/mcp/oauth/types.ts @@ -18,6 +18,8 @@ export interface OAuthMetadata { token_endpoint_auth_methods_supported?: string[]; /** Code challenge methods supported */ code_challenge_methods_supported?: string[]; + /** Dynamic client registration endpoint (RFC 7591) */ + registration_endpoint?: string; /** Revocation endpoint */ revocation_endpoint?: string; /** Revocation endpoint auth methods supported */ diff --git a/packages/data-provider/src/types/queries.ts b/packages/data-provider/src/types/queries.ts index 62b033f6ba..ec0a70f9a8 100644 --- a/packages/data-provider/src/types/queries.ts +++ b/packages/data-provider/src/types/queries.ts @@ -185,8 +185,8 @@ export interface MCPConnectionStatusResponse { export interface MCPServerConnectionStatusResponse { success: boolean; serverName: string; - connectionStatus: string; requiresOAuth: boolean; + connectionStatus: 'disconnected' | 'connecting' | 'connected' | 'error'; } export interface MCPAuthValuesResponse {