diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index 3a50c0f6cf..9e3ed7a351 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -315,7 +315,6 @@ describe('MCP Routes', () => { it('should fail the flow when OAuth error is received with valid CSRF cookie', async () => { const flowId = 'test-user-id:test-server'; const mockFlowManager = { - getFlowState: jest.fn().mockResolvedValue({ status: 'PENDING', createdAt: Date.now() }), failFlow: jest.fn().mockResolvedValue(true), }; @@ -342,10 +341,38 @@ describe('MCP Routes', () => { ); }); + it('should fail the flow when OAuth error is received with valid session cookie', async () => { + const flowId = 'test-user-id:test-server'; + const mockFlowManager = { + failFlow: jest.fn().mockResolvedValue(true), + }; + + getLogStores.mockReturnValueOnce({}); + require('~/config').getFlowStateManager.mockReturnValueOnce(mockFlowManager); + MCPOAuthHandler.resolveStateToFlowId.mockResolvedValueOnce(flowId); + + const sessionToken = generateTestCsrfToken('test-user-id'); + const response = await request(app) + .get('/api/mcp/test-server/oauth/callback') + .set('Cookie', [`oauth_session=${sessionToken}`]) + .query({ + error: 'invalid_client', + state: flowId, + }); + const basePath = getBasePath(); + + expect(response.status).toBe(302); + expect(response.headers.location).toBe(`${basePath}/oauth/error?error=invalid_client`); + expect(mockFlowManager.failFlow).toHaveBeenCalledWith( + flowId, + 'mcp_oauth', + 'invalid_client', + ); + }); + it('should NOT fail the flow when OAuth error is received without cookies (DoS prevention)', async () => { const flowId = 'test-user-id:test-server'; const mockFlowManager = { - getFlowState: jest.fn().mockResolvedValue({ status: 'PENDING', createdAt: Date.now() }), failFlow: jest.fn(), }; diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index d6c3b5290c..c517140b8a 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -495,7 +495,8 @@ export class MCPConnectionFactory { /** * Checks whether an error indicates the OAuth client registration was rejected. * Includes RFC 6749 ยง5.2 standard codes (`invalid_client`, `unauthorized_client`) - * and known vendor-specific patterns (Okta: `client_id mismatch`, Auth0: `client not found`). + * and known vendor-specific patterns (Okta: `client_id mismatch`, Auth0: `client not found`, + * generic: `unknown client`). */ static isClientRejection(error: unknown): boolean { if (!error || typeof error !== 'object') { diff --git a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts index 5983710f65..d90ca5b345 100644 --- a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts +++ b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts @@ -354,8 +354,8 @@ describe('MCPConnectionFactory', () => { await oauthRequiredHandler!({ serverUrl: 'https://api.example.com' }); - // Wait for the background .catch() handler to run - await new Promise((r) => setTimeout(r, 50)); + // Drain microtasks so the background .catch() handler completes + await new Promise((r) => setImmediate(r)); // deleteClientRegistration should have been called via clearStaleClientIfRejected expect(mockMCPTokenStorage.deleteClientRegistration).toHaveBeenCalledWith(