From b62b30bac0d575425da7e484cbd4f93360847b6a Mon Sep 17 00:00:00 2001 From: Tin Date: Wed, 8 Jul 2026 16:10:30 -0700 Subject: [PATCH] fix(ui): edit-form browser-authorize payload uses the selected auth_type The edit form's getTemporaryPayload read the server's stored auth_type instead of the value the admin selected in the dropdown, so an admin who switched an existing oauth2 server to true_passthrough (or oauth_delegate) and ran the browser authorize flow built the temporary OAuth-relay server as oauth2. That made needs_user_oauth_token true and persisted the token to the DB, contrary to the mode's browser-held contract, and left it inconsistent with onTokenReceived and the submit payload, both of which already read the form value. It now reads values.auth_type, matching the create form. --- .../mcp_tools/mcp_server_edit.test.tsx | 44 ++++++++++++++++--- .../components/mcp_tools/mcp_server_edit.tsx | 4 +- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx index f071e247466..0579a3208d1 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx @@ -19,14 +19,20 @@ vi.mock("../molecules/notifications_manager", () => ({ }, })); -const mockOauth: { tokenResponse: any } = { tokenResponse: null }; +const mockOauth: { + tokenResponse: any; + getTemporaryPayload: (() => Record | null) | null; +} = { tokenResponse: null, getTemporaryPayload: null }; vi.mock("@/hooks/useMcpOAuthFlow", () => ({ - useMcpOAuthFlow: () => ({ - startOAuthFlow: vi.fn(), - status: "idle", - error: null, - tokenResponse: mockOauth.tokenResponse, - }), + useMcpOAuthFlow: (opts: { getTemporaryPayload?: () => Record | null }) => { + mockOauth.getTemporaryPayload = opts?.getTemporaryPayload ?? null; + return { + startOAuthFlow: vi.fn(), + status: "idle", + error: null, + tokenResponse: mockOauth.tokenResponse, + }; + }, })); vi.mock("./mcp_server_cost_config", () => ({ @@ -344,6 +350,30 @@ describe("MCPServerEdit (true passthrough warning)", () => { screen.queryByText("True Passthrough disables LiteLLM authentication for this server"), ).not.toBeInTheDocument(); }); + + it("browser-authorize temp payload uses the selected auth_type, not the stored one", async () => { + // Stored server is oauth2; the admin switches the dropdown to true_passthrough before saving. + // The temp OAuth-relay payload must reflect the selection so the exchange is treated as + // browser-held (no DB persistence), matching onTokenReceived and the create form. + render( + , + ); + + await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)"); + + await waitFor(() => { + expect(mockOauth.getTemporaryPayload).toBeTruthy(); + }); + const payload = mockOauth.getTemporaryPayload!(); + expect(payload).toBeTruthy(); + expect(payload?.auth_type).toBe("true_passthrough"); + }); }); describe("MCPServerEdit (auth type switch)", () => { diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx index 7d9316e1baa..0a00f05ab53 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx @@ -164,8 +164,8 @@ const MCPServerEdit: React.FC = ({ url, transport, auth_type: - mcpServer.auth_type === AUTH_TYPE.TRUE_PASSTHROUGH || mcpServer.auth_type === AUTH_TYPE.OAUTH_DELEGATE - ? mcpServer.auth_type + values.auth_type === AUTH_TYPE.TRUE_PASSTHROUGH || values.auth_type === AUTH_TYPE.OAUTH_DELEGATE + ? values.auth_type : AUTH_TYPE.OAUTH2, credentials: values.credentials, mcp_access_groups: values.mcp_access_groups || mcpServer.mcp_access_groups,