From 17ace55bbfc6ec4cafb48626a15f5febad61ce7e Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Thu, 25 Sep 2025 15:29:26 -0600 Subject: [PATCH 1/5] fix: enhance OAuth token handling in useConnection hook to prevent infinite auth loops --- .../hooks/__tests__/useConnection.test.tsx | 29 +++++++++++++++++++ client/src/lib/hooks/useConnection.ts | 18 ++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/client/src/lib/hooks/__tests__/useConnection.test.tsx b/client/src/lib/hooks/__tests__/useConnection.test.tsx index 81bc4f45a..4946cddac 100644 --- a/client/src/lib/hooks/__tests__/useConnection.test.tsx +++ b/client/src/lib/hooks/__tests__/useConnection.test.tsx @@ -913,6 +913,35 @@ describe("useConnection", () => { expect(headers).toHaveProperty("Authorization", "Bearer mock-token"); }); + test("replaces empty Bearer token placeholder with OAuth 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, + }, + ]; + + const propsWithEmptyBearer = { + ...defaultProps, + customHeaders, + }; + + const { result } = renderHook(() => useConnection(propsWithEmptyBearer)); + + await act(async () => { + await result.current.connect(); + }); + + const headers = mockSSETransport.options?.requestInit?.headers; + // Should replace the empty "Bearer " with actual OAuth token + expect(headers).toHaveProperty("Authorization", "Bearer mock-token"); + // Should not have the x-custom-auth-headers since Authorization is standard + expect(headers).not.toHaveProperty("x-custom-auth-headers"); + }); + 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..70ae5acf2 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -400,11 +400,25 @@ 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) { + // Check if we need to inject OAuth token + // This handles both empty headers and default "Bearer " placeholder headers + const isEmptyAuthHeader = (header: CustomHeaders[number]) => + header.name.trim().toLowerCase() === "authorization" && + header.value.trim() === "Bearer"; + + const needsOAuthToken = + finalHeaders.length === 0 || + finalHeaders.some( + (header) => header.enabled && isEmptyAuthHeader(header), + ); + + 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}`, From 30f9855fd519b45d4161c041147fab1c47f79544 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Fri, 26 Sep 2025 13:24:28 -0600 Subject: [PATCH 2/5] fix: update authorization header handling to prevent empty values and show validation error --- client/src/App.tsx | 10 ++-------- .../lib/hooks/__tests__/useConnection.test.tsx | 10 +++++++++- client/src/lib/hooks/useConnection.ts | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/client/src/App.tsx b/client/src/App.tsx index 6c9ae3331..00fa6f46e 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -162,14 +162,8 @@ const App = () => { return migrateFromLegacyAuth(legacyToken, legacyHeaderName); } - // Default to Authorization: Bearer as the most common case - return [ - { - name: "Authorization", - value: "Bearer ", - enabled: true, - }, - ]; + // Default to empty array + return []; }); const [pendingSampleRequests, setPendingSampleRequests] = useState< diff --git a/client/src/lib/hooks/__tests__/useConnection.test.tsx b/client/src/lib/hooks/__tests__/useConnection.test.tsx index 4946cddac..8d8a1f8b0 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, }), })); @@ -940,6 +941,13 @@ describe("useConnection", () => { expect(headers).toHaveProperty("Authorization", "Bearer mock-token"); // 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 () => { diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 70ae5acf2..fa03496dd 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -400,6 +400,23 @@ export function useConnection({ // Use custom headers (migration is handled in App.tsx) let finalHeaders: CustomHeaders = customHeaders || []; + // Check for empty Authorization headers and show validation error + const hasEmptyAuthHeader = finalHeaders.some( + (header) => + header.enabled && + header.name.trim().toLowerCase() === "authorization" && + (!header.value.trim() || header.value.trim() === "Bearer"), + ); + + if (hasEmptyAuthHeader) { + toast({ + title: "Invalid Authorization Header", + description: + "Authorization header is enabled but empty. Please add a token or disable the header. It will be added automatically.", + variant: "destructive", + }); + } + // Check if we need to inject OAuth token // This handles both empty headers and default "Bearer " placeholder headers const isEmptyAuthHeader = (header: CustomHeaders[number]) => From 0da5bc2a2e6e8f39d97dbd4a3667bae2cea90c21 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Fri, 26 Sep 2025 13:26:46 -0600 Subject: [PATCH 3/5] refactor: streamline authorization header validation in useConnection hook --- client/src/lib/hooks/useConnection.ts | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index fa03496dd..d32c7ffa0 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -400,12 +400,13 @@ export function useConnection({ // Use custom headers (migration is handled in App.tsx) let finalHeaders: CustomHeaders = customHeaders || []; + const isEmptyAuthHeader = (header: CustomHeaders[number]) => + header.name.trim().toLowerCase() === "authorization" && + header.value.trim() === "Bearer"; + // Check for empty Authorization headers and show validation error const hasEmptyAuthHeader = finalHeaders.some( - (header) => - header.enabled && - header.name.trim().toLowerCase() === "authorization" && - (!header.value.trim() || header.value.trim() === "Bearer"), + (header) => header.enabled && isEmptyAuthHeader(header), ); if (hasEmptyAuthHeader) { @@ -417,17 +418,7 @@ export function useConnection({ }); } - // Check if we need to inject OAuth token - // This handles both empty headers and default "Bearer " placeholder headers - const isEmptyAuthHeader = (header: CustomHeaders[number]) => - header.name.trim().toLowerCase() === "authorization" && - header.value.trim() === "Bearer"; - - const needsOAuthToken = - finalHeaders.length === 0 || - finalHeaders.some( - (header) => header.enabled && isEmptyAuthHeader(header), - ); + const needsOAuthToken = finalHeaders.length === 0 || hasEmptyAuthHeader; if (needsOAuthToken) { const oauthToken = (await serverAuthProvider.tokens())?.access_token; From 34534ed062e55dd36376e291cdcf158a7294b5c2 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Tue, 30 Sep 2025 15:17:10 -0600 Subject: [PATCH 4/5] fix: update default authorization header to include empty Bearer token and improve validation logic --- client/src/App.tsx | 8 +++++++- client/src/lib/hooks/__tests__/useConnection.test.tsx | 8 ++++---- client/src/lib/hooks/useConnection.ts | 8 ++++++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/client/src/App.tsx b/client/src/App.tsx index 1cdea32c2..b7fc607b0 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -165,7 +165,13 @@ const App = () => { } // Default to empty array - return []; + return [ + { + name: "Authorization", + value: "Bearer ", + enabled: false, + }, + ]; }); const [pendingSampleRequests, setPendingSampleRequests] = useState< diff --git a/client/src/lib/hooks/__tests__/useConnection.test.tsx b/client/src/lib/hooks/__tests__/useConnection.test.tsx index 8d8a1f8b0..a9ba6825b 100644 --- a/client/src/lib/hooks/__tests__/useConnection.test.tsx +++ b/client/src/lib/hooks/__tests__/useConnection.test.tsx @@ -914,14 +914,14 @@ describe("useConnection", () => { expect(headers).toHaveProperty("Authorization", "Bearer mock-token"); }); - test("replaces empty Bearer token placeholder with OAuth token", async () => { + 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: true, // enabled }, ]; @@ -937,8 +937,8 @@ describe("useConnection", () => { }); const headers = mockSSETransport.options?.requestInit?.headers; - // Should replace the empty "Bearer " with actual OAuth token - expect(headers).toHaveProperty("Authorization", "Bearer mock-token"); + + 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"); diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index d32c7ffa0..d7b7f4b50 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -402,7 +402,7 @@ export function useConnection({ const isEmptyAuthHeader = (header: CustomHeaders[number]) => header.name.trim().toLowerCase() === "authorization" && - header.value.trim() === "Bearer"; + header.value.trim().toLowerCase() === "bearer"; // Check for empty Authorization headers and show validation error const hasEmptyAuthHeader = finalHeaders.some( @@ -418,7 +418,11 @@ export function useConnection({ }); } - const needsOAuthToken = finalHeaders.length === 0 || hasEmptyAuthHeader; + const needsOAuthToken = !finalHeaders.some( + (header) => + header.enabled && + header.name.trim().toLowerCase() === "authorization", + ); if (needsOAuthToken) { const oauthToken = (await serverAuthProvider.tokens())?.access_token; From 5b4edefbc711e4d63f2fd52b1c3b6ad7586b24cf Mon Sep 17 00:00:00 2001 From: Cliff Hall Date: Tue, 30 Sep 2025 19:06:19 -0400 Subject: [PATCH 5/5] Update client/src/lib/hooks/useConnection.ts --- client/src/lib/hooks/useConnection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index d7b7f4b50..37073e9b7 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -413,7 +413,7 @@ export function useConnection({ toast({ title: "Invalid Authorization Header", description: - "Authorization header is enabled but empty. Please add a token or disable the header. It will be added automatically.", + "Authorization header is enabled but empty. Please add a token or disable the header.", variant: "destructive", }); }