From abda15f4eb33d8511a7b29e23eff6d2689e47e85 Mon Sep 17 00:00:00 2001 From: janluedemann-esome Date: Sat, 23 May 2026 15:09:13 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=82=20fix:=20Detect=20OAuth=20Errors?= =?UTF-8?q?=20From=20HTTP=20400=20Responses=20(#11961)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mcp): detect non-standard OAuth errors from servers returning HTTP 400 * add tests for oauth error check * fix(mcp): align factory OAuth error detection --- packages/api/src/mcp/MCPConnectionFactory.ts | 8 +++ .../src/mcp/__tests__/MCPConnection.test.ts | 22 +++++++ .../__tests__/MCPConnectionFactory.test.ts | 66 +++++++++++++++++++ packages/api/src/mcp/connection.ts | 4 ++ 4 files changed, 100 insertions(+) diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index 68e3abd776..760325cbba 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -626,10 +626,18 @@ export class MCPConnectionFactory { if (message.includes('invalid_token')) { return true; } + // Check for invalid_grant (OAuth servers return this for expired/revoked grants) + if (message.includes('invalid_grant')) { + return true; + } // Check for authentication required if (message.includes('authentication required') || message.includes('unauthorized')) { return true; } + // Check for missing authorization values (e.g., Amazon Ads MCP returns HTTP 400 with this) + if (message.includes('no authorization')) { + return true; + } } return false; diff --git a/packages/api/src/mcp/__tests__/MCPConnection.test.ts b/packages/api/src/mcp/__tests__/MCPConnection.test.ts index 5cb5606d57..6ba9d92171 100644 --- a/packages/api/src/mcp/__tests__/MCPConnection.test.ts +++ b/packages/api/src/mcp/__tests__/MCPConnection.test.ts @@ -82,6 +82,10 @@ describe('MCPConnection Error Detection', () => { if (message.includes('authentication required') || message.includes('unauthorized')) { return true; } + // Check for missing authorization values (e.g., Amazon Ads MCP returns HTTP 400 with this) + if (message.includes('no authorization')) { + return true; + } } return false; @@ -171,6 +175,24 @@ describe('MCPConnection Error Detection', () => { }; expect(isOAuthError(error)).toBe(true); }); + + it('should detect OAuth error for "no authorization" in message (HTTP 400)', () => { + const error = { + message: + 'Either no authorization values are specified or it could not be derived from the request', + }; + expect(isOAuthError(error)).toBe(true); + }); + + it('should detect OAuth error for "No authorization" with different casing', () => { + const error = { message: 'No Authorization header provided' }; + expect(isOAuthError(error)).toBe(true); + }); + + it('should not detect OAuth error for unrelated 400 errors', () => { + const error = { code: 400, message: 'Bad request: missing required field' }; + expect(isOAuthError(error)).toBe(false); + }); }); describe('error type differentiation', () => { diff --git a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts index 5a0787868d..8018ec0a6f 100644 --- a/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts +++ b/packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts @@ -844,6 +844,72 @@ describe('MCPConnectionFactory', () => { expect.stringContaining('OAuth required, stopping connection attempts'), ); }); + + it('should identify "no authorization" errors as OAuth errors (HTTP 400)', async () => { + const basicOptions = { + serverName: 'test-server', + serverConfig: mockServerConfig, + }; + + const oauthOptions = { + useOAuth: true as const, + user: mockUser, + flowManager: mockFlowManager, + tokenMethods: { + findToken: jest.fn(), + createToken: jest.fn(), + updateToken: jest.fn(), + deleteTokens: jest.fn(), + }, + }; + + const noAuthError = new Error( + 'Either no authorization values are specified or it could not be derived from the request', + ); + + mockConnectionInstance.connect.mockRejectedValue(noAuthError); + mockConnectionInstance.isConnected.mockResolvedValue(false); + + await expect(MCPConnectionFactory.create(basicOptions, oauthOptions)).rejects.toThrow( + 'no authorization', + ); + expect(mockLogger.info).toHaveBeenCalledWith( + expect.stringContaining('OAuth required, stopping connection attempts'), + ); + }); + + it('should identify invalid_grant errors as OAuth errors', async () => { + const basicOptions = { + serverName: 'test-server', + serverConfig: mockServerConfig, + }; + + const oauthOptions = { + useOAuth: true as const, + user: mockUser, + flowManager: mockFlowManager, + tokenMethods: { + findToken: jest.fn(), + createToken: jest.fn(), + updateToken: jest.fn(), + deleteTokens: jest.fn(), + }, + }; + + const invalidGrantError = new Error( + 'Streamable HTTP error: Error POSTing to endpoint: {"error":"invalid_grant"}', + ); + + mockConnectionInstance.connect.mockRejectedValue(invalidGrantError); + mockConnectionInstance.isConnected.mockResolvedValue(false); + + await expect(MCPConnectionFactory.create(basicOptions, oauthOptions)).rejects.toThrow( + 'invalid_grant', + ); + expect(mockLogger.info).toHaveBeenCalledWith( + expect.stringContaining('OAuth required, stopping connection attempts'), + ); + }); }); describe('discoverTools static method', () => { diff --git a/packages/api/src/mcp/connection.ts b/packages/api/src/mcp/connection.ts index 8edc01f2f4..3049c33ad2 100644 --- a/packages/api/src/mcp/connection.ts +++ b/packages/api/src/mcp/connection.ts @@ -2326,6 +2326,10 @@ export class MCPConnection extends EventEmitter { if (message.includes('authentication required') || message.includes('unauthorized')) { return true; } + // Check for missing authorization values (e.g., Amazon Ads MCP returns HTTP 400 with this) + if (message.includes('no authorization')) { + return true; + } } return false;