Merge pull request #830 from kentcdodds/auth-fix

fix: enhance OAuth token handling in useConnection hook to prevent infinite auth loops

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,
       },
     ];
   });

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 },

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}`,