Skip to content

Commit 901d10c

Browse files
authored
Merge pull request #830 from kentcdodds/auth-fix
fix: enhance OAuth token handling in useConnection hook to prevent infinite auth loops
2 parents 1c35ee5 + 5b4edef commit 901d10c

File tree

3 files changed

+68
-5
lines changed

3 files changed

+68
-5
lines changed

client/src/App.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,12 @@ const App = () => {
164164
return migrateFromLegacyAuth(legacyToken, legacyHeaderName);
165165
}
166166

167-
// Default to Authorization: Bearer as the most common case
167+
// Default to empty array
168168
return [
169169
{
170170
name: "Authorization",
171171
value: "Bearer ",
172-
enabled: true,
172+
enabled: false,
173173
},
174174
];
175175
});

client/src/lib/hooks/__tests__/useConnection.test.tsx

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,10 @@ jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
9797
}));
9898

9999
// Mock the toast hook
100+
const mockToast = jest.fn();
100101
jest.mock("@/lib/hooks/useToast", () => ({
101102
useToast: () => ({
102-
toast: jest.fn(),
103+
toast: mockToast,
103104
}),
104105
}));
105106

@@ -913,6 +914,42 @@ describe("useConnection", () => {
913914
expect(headers).toHaveProperty("Authorization", "Bearer mock-token");
914915
});
915916

917+
test("warns of enabled empty Bearer token", async () => {
918+
// This test prevents regression of the bug where default "Bearer " header
919+
// prevented OAuth token injection, causing infinite auth loops
920+
const customHeaders: CustomHeaders = [
921+
{
922+
name: "Authorization",
923+
value: "Bearer ", // Empty Bearer token placeholder
924+
enabled: true, // enabled
925+
},
926+
];
927+
928+
const propsWithEmptyBearer = {
929+
...defaultProps,
930+
customHeaders,
931+
};
932+
933+
const { result } = renderHook(() => useConnection(propsWithEmptyBearer));
934+
935+
await act(async () => {
936+
await result.current.connect();
937+
});
938+
939+
const headers = mockSSETransport.options?.requestInit?.headers;
940+
941+
expect(headers).toHaveProperty("Authorization", "Bearer");
942+
// Should not have the x-custom-auth-headers since Authorization is standard
943+
expect(headers).not.toHaveProperty("x-custom-auth-headers");
944+
945+
// Should show toast notification for empty Authorization header
946+
expect(mockToast).toHaveBeenCalledWith({
947+
title: "Invalid Authorization Header",
948+
description: expect.any(String),
949+
variant: "destructive",
950+
});
951+
});
952+
916953
test("prioritizes custom headers over legacy auth", async () => {
917954
const customHeaders: CustomHeaders = [
918955
{ name: "Authorization", value: "Bearer custom-token", enabled: true },

client/src/lib/hooks/useConnection.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,11 +400,37 @@ export function useConnection({
400400
// Use custom headers (migration is handled in App.tsx)
401401
let finalHeaders: CustomHeaders = customHeaders || [];
402402

403-
// Add OAuth token if available and no custom headers are set
404-
if (finalHeaders.length === 0) {
403+
const isEmptyAuthHeader = (header: CustomHeaders[number]) =>
404+
header.name.trim().toLowerCase() === "authorization" &&
405+
header.value.trim().toLowerCase() === "bearer";
406+
407+
// Check for empty Authorization headers and show validation error
408+
const hasEmptyAuthHeader = finalHeaders.some(
409+
(header) => header.enabled && isEmptyAuthHeader(header),
410+
);
411+
412+
if (hasEmptyAuthHeader) {
413+
toast({
414+
title: "Invalid Authorization Header",
415+
description:
416+
"Authorization header is enabled but empty. Please add a token or disable the header.",
417+
variant: "destructive",
418+
});
419+
}
420+
421+
const needsOAuthToken = !finalHeaders.some(
422+
(header) =>
423+
header.enabled &&
424+
header.name.trim().toLowerCase() === "authorization",
425+
);
426+
427+
if (needsOAuthToken) {
405428
const oauthToken = (await serverAuthProvider.tokens())?.access_token;
406429
if (oauthToken) {
430+
// Add the OAuth token
407431
finalHeaders = [
432+
// Remove any existing Authorization headers with empty tokens
433+
...finalHeaders.filter((header) => !isEmptyAuthHeader(header)),
408434
{
409435
name: "Authorization",
410436
value: `Bearer ${oauthToken}`,

0 commit comments

Comments
 (0)