diff --git a/client/src/App.tsx b/client/src/App.tsx index 04794df05..b7fc607b0 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -164,12 +164,12 @@ const App = () => { return migrateFromLegacyAuth(legacyToken, legacyHeaderName); } - // Default to Authorization: Bearer as the most common case + // Default to empty array return [ { name: "Authorization", value: "Bearer ", - enabled: true, + enabled: false, }, ]; }); diff --git a/client/src/lib/hooks/__tests__/useConnection.test.tsx b/client/src/lib/hooks/__tests__/useConnection.test.tsx index 81bc4f45a..a9ba6825b 100644 --- a/client/src/lib/hooks/__tests__/useConnection.test.tsx +++ b/client/src/lib/hooks/__tests__/useConnection.test.tsx @@ -97,9 +97,10 @@ jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ })); // Mock the toast hook +const mockToast = jest.fn(); jest.mock("@/lib/hooks/useToast", () => ({ useToast: () => ({ - toast: jest.fn(), + toast: mockToast, }), })); @@ -913,6 +914,42 @@ describe("useConnection", () => { expect(headers).toHaveProperty("Authorization", "Bearer mock-token"); }); + test("warns of enabled empty Bearer token", async () => { + // This test prevents regression of the bug where default "Bearer " header + // prevented OAuth token injection, causing infinite auth loops + const customHeaders: CustomHeaders = [ + { + name: "Authorization", + value: "Bearer ", // Empty Bearer token placeholder + enabled: true, // enabled + }, + ]; + + const propsWithEmptyBearer = { + ...defaultProps, + customHeaders, + }; + + const { result } = renderHook(() => useConnection(propsWithEmptyBearer)); + + await act(async () => { + await result.current.connect(); + }); + + const headers = mockSSETransport.options?.requestInit?.headers; + + expect(headers).toHaveProperty("Authorization", "Bearer"); + // Should not have the x-custom-auth-headers since Authorization is standard + expect(headers).not.toHaveProperty("x-custom-auth-headers"); + + // Should show toast notification for empty Authorization header + expect(mockToast).toHaveBeenCalledWith({ + title: "Invalid Authorization Header", + description: expect.any(String), + variant: "destructive", + }); + }); + test("prioritizes custom headers over legacy auth", async () => { const customHeaders: CustomHeaders = [ { name: "Authorization", value: "Bearer custom-token", enabled: true }, diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 163231eb4..37073e9b7 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -400,11 +400,37 @@ export function useConnection({ // Use custom headers (migration is handled in App.tsx) let finalHeaders: CustomHeaders = customHeaders || []; - // Add OAuth token if available and no custom headers are set - if (finalHeaders.length === 0) { + const isEmptyAuthHeader = (header: CustomHeaders[number]) => + header.name.trim().toLowerCase() === "authorization" && + header.value.trim().toLowerCase() === "bearer"; + + // Check for empty Authorization headers and show validation error + const hasEmptyAuthHeader = finalHeaders.some( + (header) => header.enabled && isEmptyAuthHeader(header), + ); + + if (hasEmptyAuthHeader) { + toast({ + title: "Invalid Authorization Header", + description: + "Authorization header is enabled but empty. Please add a token or disable the header.", + variant: "destructive", + }); + } + + const needsOAuthToken = !finalHeaders.some( + (header) => + header.enabled && + header.name.trim().toLowerCase() === "authorization", + ); + + if (needsOAuthToken) { const oauthToken = (await serverAuthProvider.tokens())?.access_token; if (oauthToken) { + // Add the OAuth token finalHeaders = [ + // Remove any existing Authorization headers with empty tokens + ...finalHeaders.filter((header) => !isEmptyAuthHeader(header)), { name: "Authorization", value: `Bearer ${oauthToken}`,