From 1f131d39c0c3362ab73cdbdb90273dc86080ce1f Mon Sep 17 00:00:00 2001 From: Tony Liu Date: Fri, 5 Jun 2026 11:43:03 +1200 Subject: [PATCH] fix: use the auth provider's current token instead of a connect-time snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When relying on OAuth, useConnection read the access token from the provider once at connection setup and baked `Authorization: Bearer ` into `requestInit.headers`. The SDK transports already receive the same provider as their `authProvider` and add a fresh `Authorization` from it on every request, refreshing it on 401. But the SDK's `_commonHeaders()` spreads `requestInit.headers` after the provider token, so the connect-time snapshot always won. The effect: after the access token expires, the SDK refreshes successfully (token endpoint returns 200 and the provider stores the new token), but the retried request still carries the stale snapshot token, gets another 401, and the SDK's circuit breaker throws "Server returned 401 after successful authentication" — a 401 loop the session can't recover from. Stop snapshotting the OAuth token into requestInit and let the provider be the single source of truth. A user-supplied static Authorization header still flows through the custom-headers path and intentionally overrides the provider. Update the related test to assert the token is no longer baked in, and add direct SSE / Streamable HTTP tests covering it. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../hooks/__tests__/useConnection.test.tsx | 66 ++++++++++++++++++- client/src/lib/hooks/useConnection.ts | 32 +++------ 2 files changed, 73 insertions(+), 25 deletions(-) diff --git a/client/src/lib/hooks/__tests__/useConnection.test.tsx b/client/src/lib/hooks/__tests__/useConnection.test.tsx index 875c9e387..a8ac79ee1 100644 --- a/client/src/lib/hooks/__tests__/useConnection.test.tsx +++ b/client/src/lib/hooks/__tests__/useConnection.test.tsx @@ -1313,7 +1313,7 @@ describe("useConnection", () => { ); }); - test("uses OAuth token when no custom headers or legacy auth provided", async () => { + test("relies on the auth provider for the OAuth token instead of baking it into requestInit", async () => { const propsWithoutAuth = { ...defaultProps, }; @@ -1324,8 +1324,12 @@ describe("useConnection", () => { await result.current.connect(); }); - const headers = mockSSETransport.options?.requestInit?.headers; - expect(headers).toHaveProperty("Authorization", "Bearer mock-token"); + const options = mockSSETransport.options; + // The SDK's authProvider supplies (and refreshes) the OAuth token on every + // request, so the Inspector must not snapshot it into requestInit — doing + // so would shadow the refreshed token and 401-loop after expiry. + expect(options?.authProvider).toBeDefined(); + expect(options?.requestInit?.headers).not.toHaveProperty("Authorization"); }); test("warns of enabled empty Bearer token", async () => { @@ -1794,4 +1798,60 @@ describe("useConnection", () => { ).toBeNull(); }); }); + + describe("OAuth access token is supplied by the provider, not snapshotted", () => { + // The mocked InspectorOAuthClientProvider always returns a token, so the + // pre-fix code would have baked "Bearer mock-token" into requestInit. These + // tests assert it no longer does, for the direct transports where the SDK's + // authProvider is the single, always-current source of the token. + beforeEach(() => { + jest.clearAllMocks(); + mockSSETransport.options = undefined; + mockStreamableHTTPTransport.options = undefined; + }); + + test("direct streamable-http does not bake an Authorization header into requestInit", async () => { + const props = { + ...defaultProps, + connectionType: "direct" as const, + transportType: "streamable-http" as const, + sseUrl: "http://localhost:8080", + }; + + const { result } = renderHook(() => useConnection(props)); + await act(async () => { + await result.current.connect(); + }); + + const options = mockStreamableHTTPTransport.options; + expect(options).toBeDefined(); + // The provider is the single source of truth for the OAuth token. + expect(options?.authProvider).toBeDefined(); + // No connect-time snapshot that would shadow the provider's refreshed + // token (the SDK lets requestInit override the provider) and cause a + // 401 loop after the access token expires. + expect(options?.requestInit?.headers).not.toHaveProperty("Authorization"); + expect(options?.requestInit?.headers).not.toHaveProperty("authorization"); + }); + + test("direct SSE does not bake an Authorization header into requestInit", async () => { + const props = { + ...defaultProps, + connectionType: "direct" as const, + transportType: "sse" as const, + sseUrl: "http://localhost:8080", + }; + + const { result } = renderHook(() => useConnection(props)); + await act(async () => { + await result.current.connect(); + }); + + const options = mockSSETransport.options; + expect(options).toBeDefined(); + expect(options?.authProvider).toBeDefined(); + expect(options?.requestInit?.headers).not.toHaveProperty("Authorization"); + expect(options?.requestInit?.headers).not.toHaveProperty("authorization"); + }); + }); }); diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 9694b891f..b9953e5ab 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -499,7 +499,7 @@ export function useConnection({ const serverAuthProvider = new InspectorOAuthClientProvider(sseUrl); // Use custom headers (migration is handled in App.tsx) - let finalHeaders: CustomHeaders = customHeaders || []; + const finalHeaders: CustomHeaders = customHeaders || []; const isEmptyAuthHeader = (header: CustomHeaders[number]) => header.name.trim().toLowerCase() === "authorization" && @@ -519,27 +519,15 @@ export function useConnection({ }); } - 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}`, - enabled: true, - }, - ]; - } - } + // Do not inject the OAuth access token here. The transports below receive + // `serverAuthProvider` as their authProvider, so the SDK adds a fresh + // `Authorization` from the provider on every request and replaces it after + // a refresh. Baking a connect-time snapshot into requestInit.headers would + // shadow that: the SDK's _commonHeaders lets requestInit override the + // provider token, so the stale token would keep being sent and the session + // would 401-loop once the access token expires. A user-supplied static + // Authorization header still flows through the custom-headers path below + // and intentionally overrides the provider. // Process all enabled custom headers const customHeaderNames: string[] = [];