From 931b617a51f775327cde2d2cbb75dc3ce0873e55 Mon Sep 17 00:00:00 2001 From: Tin Date: Fri, 10 Jul 2026 00:45:38 -0700 Subject: [PATCH 01/10] feat(mcp): persist admin-entered OAuth app credentials for the client-forwarded modes --- .../mcp_tools/PassthroughAuthorizeSection.tsx | 31 ++++---- .../mcp_tools/create_mcp_server.test.tsx | 71 ++++++++++++++++++- .../mcp_tools/create_mcp_server.tsx | 4 +- .../mcp_tools/mcp_server_edit.test.tsx | 46 ++++++++++++ .../components/mcp_tools/mcp_server_edit.tsx | 4 +- 5 files changed, 138 insertions(+), 18 deletions(-) diff --git a/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx b/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx index af81f2713aec..314b45fff6c5 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx @@ -11,13 +11,14 @@ interface PassthroughOAuthFlow { /** * Browser-only Authorize & Fetch for the client-forwarded token modes - * (true_passthrough / oauth_delegate). LiteLLM never stores upstream - * credentials for these modes, so the token obtained here lives in this - * browser session only: it is forwarded per-server for the tools preview and - * allowlist configuration, and is never written to the server row or the - * per-user credential store. The optional client credentials cover IdPs - * without dynamic client registration (e.g. a pre-registered Slack app) and - * ride the temporary authorize session only. + * (true_passthrough / oauth_delegate). Tokens are never stored: the token + * obtained here lives in this browser session only, forwarded per-server for + * the tools preview and allowlist configuration, and is never written to the + * server row or the per-user credential store. The optional OAuth client + * credentials cover IdPs without dynamic client registration (e.g. a + * pre-registered Slack app); unlike the token they ARE saved onto the server + * as declared config, so internal users' Authorize relays through the org's + * app instead of dead-ending on upstreams that cannot mint clients. */ export default function PassthroughAuthorizeSection({ authType, @@ -35,14 +36,15 @@ export default function PassthroughAuthorizeSection({ return (

- Callers bring their own upstream token for this auth type, so LiteLLM stores no upstream credentials. To preview - tools and configure the tool allowlist, authorize against the upstream here: the token stays in this browser - session only and is never saved to LiteLLM. + Callers bring their own upstream token for this auth type, so LiteLLM never stores tokens. To preview tools and + configure the tool allowlist, authorize against the upstream here: the token stays in this browser session only + and is never saved to LiteLLM. An OAuth app configured below IS saved with the server, so internal users who + authorize from the Tools page go through it.

OAuth Client ID (optional, not saved)} + label={OAuth Client ID (optional, saved)} name={["credentials", "client_id"]} - extra="Only needed when the upstream does not support dynamic client registration (e.g. a pre-registered Slack app). Used for this browser authorization only." + extra="Set this to make everyone authorize through a specific app; required for upstreams without dynamic client registration (e.g. a pre-registered Slack app)." > OAuth Client Secret (optional, not saved)} + label={OAuth Client Secret (optional, saved)} name={["credentials", "client_secret"]} > {oauthFlow.error}

} {oauthFlow.status === "success" && oauthFlow.tokenResponse?.access_token && (

- Token held for this browser session. Tools can now be previewed and configured; nothing was saved to LiteLLM. + Token held for this browser session. Tools can now be previewed and configured; the token was not saved to + LiteLLM.

)}
diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx index 02374a3ffa47..0a3128c6e7fe 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx @@ -202,8 +202,8 @@ describe("CreateMCPServer", () => { await waitFor(() => { expect(screen.getByRole("button", { name: "Authorize & Fetch Tools (browser-only)" })).toBeInTheDocument(); }); - expect(screen.getByText("OAuth Client ID (optional, not saved)")).toBeInTheDocument(); - expect(screen.getByText("OAuth Client Secret (optional, not saved)")).toBeInTheDocument(); + expect(screen.getByText("OAuth Client ID (optional, saved)")).toBeInTheDocument(); + expect(screen.getByText("OAuth Client Secret (optional, saved)")).toBeInTheDocument(); }, ); @@ -436,6 +436,73 @@ describe("CreateMCPServer", () => { ); }); + it.each([ + ["true_passthrough", "True Passthrough (no LiteLLM auth)"], + ["oauth_delegate", "OAuth Delegate (client-supplied upstream token)"], + ])( + "persists admin-entered OAuth app credentials on create for %s while the token stays browser-held", + async (_authType, optionLabel) => { + oauthHook.tokenResponse = { access_token: "upstream-tok", token_type: "Bearer" }; + await selectHttpTransport(); + + const user = userEvent.setup({ delay: null }); + await user.type(getServerNameInput(), "CF_App_Server"); + await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp"); + + await selectAntOption("Authentication", optionLabel); + + // Admin declares the org's pre-registered upstream app; unlike the browser-authorized + // token, this is config and must survive onto the server row so internal users' + // Tools-page Authorize relays through it (required for non-DCR upstreams like Slack). + await user.type( + screen.getByPlaceholderText("Leave blank to use dynamic client registration"), + "org-app-client-id", + ); + await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "org-app-secret"); + + await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy()); + await act(async () => { + oauthHook.onTokenReceived!({ access_token: "upstream-tok", token_type: "Bearer" }, undefined); + }); + + const createdServer = { + server_id: "new-cf-app-server", + server_name: "CF_App_Server", + alias: "CF_App_Server", + url: "https://example.com/mcp", + transport: "http", + auth_type: _authType, + created_at: "2024-01-01T00:00:00Z", + created_by: "user-1", + updated_at: "2024-01-01T00:00:00Z", + updated_by: "user-1", + }; + vi.mocked(networking.createMCPServer).mockResolvedValue(createdServer); + + const submitButton = screen.getByRole("button", { name: "Add MCP Server" }); + await act(async () => { + fireEvent.click(submitButton); + }); + + await waitFor(() => expect(networking.createMCPServer).toHaveBeenCalledTimes(1)); + const [, payload] = vi.mocked(networking.createMCPServer).mock.calls[0]; + + // The declared app persists; the browser-authorized token still appears nowhere in the + // payload and no per-user DB credential is written. + expect(payload.credentials).toEqual({ + client_id: "org-app-client-id", + client_secret: "org-app-secret", + }); + expect(JSON.stringify(payload)).not.toContain("upstream-tok"); + expect(networking.storeMCPOAuthUserCredential).not.toHaveBeenCalled(); + expect(setToken).toHaveBeenCalledWith( + "new-cf-app-server", + expect.objectContaining({ access_token: "upstream-tok" }), + undefined, + ); + }, + ); + it("should not show auth value field when None auth type is selected", async () => { await selectHttpTransport(); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx index eb48fd024742..79db031008e6 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx @@ -60,6 +60,8 @@ const AUTH_TYPES_REQUIRING_CREDENTIALS = [ AUTH_TYPE.OAUTH2, AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE, AUTH_TYPE.AWS_SIGV4, + AUTH_TYPE.TRUE_PASSTHROUGH, + AUTH_TYPE.OAUTH_DELEGATE, ]; const CREATE_OAUTH_UI_STATE_KEY = "litellm-mcp-oauth-create-state"; @@ -209,7 +211,7 @@ const CreateMCPServer: React.FC = ({ // edit form's onTokenReceived early return. setAuthorizedIdentity(getOAuthAuthorizationIdentity(form.getFieldsValue(true))); NotificationsManager.success( - "Token held for this browser session. Tools can now be previewed and configured; nothing will be saved to LiteLLM.", + "Token held for this browser session. Tools can now be previewed and configured; the token is not saved to LiteLLM.", ); return; } 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 adb3e161da59..a1bad0307b0b 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 @@ -1,6 +1,7 @@ import React from "react"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { render, screen, waitFor, fireEvent, act } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import MCPServerEdit from "./mcp_server_edit"; import * as networking from "../networking"; import NotificationsManager from "../molecules/notifications_manager"; @@ -1377,6 +1378,51 @@ describe("MCPServerEdit (OAuth token persistence on save)", () => { }, ); + it.each([["true_passthrough"], ["oauth_delegate"]])( + "persists admin-entered OAuth app credentials in the update payload for the %s mode", + async (authType) => { + mockOauth.tokenResponse = { access_token: "cf-tok", expires_in: 1800, token_type: "bearer" }; + vi.mocked(networking.updateMCPServer).mockResolvedValue({ + ...interactiveOAuthServer, + auth_type: authType, + }); + + render( + , + ); + + const user = userEvent.setup({ delay: null }); + await user.type( + screen.getByPlaceholderText("Leave blank to use dynamic client registration"), + "org-app-client-id", + ); + await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "org-app-secret"); + + await act(async () => { + fireEvent.click(screen.getAllByRole("button", { name: "Save Changes" })[0]); + }); + + await waitFor(() => expect(networking.updateMCPServer).toHaveBeenCalledTimes(1)); + const [, payload] = vi.mocked(networking.updateMCPServer).mock.calls[0]; + + // The declared app is config and persists onto the row; the browser-held token still never + // reaches the payload or the per-user credential store. + expect(payload.credentials).toMatchObject({ + client_id: "org-app-client-id", + client_secret: "org-app-secret", + }); + expect(JSON.stringify(payload)).not.toContain("cf-tok"); + expect(networking.storeMCPOAuthUserCredential).not.toHaveBeenCalled(); + }, + ); + it("forwards a newly authorized browser-held token for tool loading before the form is saved", async () => { // Regression: fetchTools keyed the browser-held decision off the saved mcpServer.auth_type, so // after switching the form to true_passthrough and authorizing, the fresh token was not sent as 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 7446c96c40e7..9b2c5af861f4 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 @@ -56,6 +56,8 @@ const AUTH_TYPES_REQUIRING_CREDENTIALS = [ AUTH_TYPE.OAUTH2, AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE, AUTH_TYPE.AWS_SIGV4, + AUTH_TYPE.TRUE_PASSTHROUGH, + AUTH_TYPE.OAUTH_DELEGATE, ]; export const EDIT_OAUTH_UI_STATE_KEY = "litellm-mcp-oauth-edit-state"; @@ -202,7 +204,7 @@ const MCPServerEdit: React.FC = ({ }; setToken(mcpServer.server_id, browserHeldToken, userID); NotificationsManager.success( - "Token held for this browser session. Tools can now be loaded and configured; nothing was saved to LiteLLM.", + "Token held for this browser session. Tools can now be loaded and configured; the token is not saved to LiteLLM.", ); return; } From 57051d36d6cdf5d136fb152b3bd9e8b5502fad45 Mon Sep 17 00:00:00 2001 From: Tin Date: Fri, 10 Jul 2026 09:39:37 -0700 Subject: [PATCH 02/10] fix(mcp): keep admin-declared app credentials through OAuth invalidation for the client-forwarded modes --- ui/litellm-dashboard/eslint-metrics.json | 2 +- .../mcp_tools/create_mcp_server.test.tsx | 104 ++++++++++++++++++ .../mcp_tools/create_mcp_server.tsx | 9 ++ .../mcp_tools/mcp_server_edit.test.tsx | 54 +++++++++ .../components/mcp_tools/mcp_server_edit.tsx | 9 ++ .../src/components/mcp_tools/types.tsx | 25 +++++ 6 files changed, 202 insertions(+), 1 deletion(-) diff --git a/ui/litellm-dashboard/eslint-metrics.json b/ui/litellm-dashboard/eslint-metrics.json index 2e204c63a484..9431c2783879 100644 --- a/ui/litellm-dashboard/eslint-metrics.json +++ b/ui/litellm-dashboard/eslint-metrics.json @@ -1,7 +1,7 @@ { "@typescript-eslint/no-explicit-any": 1978, "complexity": 129, - "local/no-large-inline-object-arg": 512, + "local/no-large-inline-object-arg": 514, "local/no-long-condition-chain": 233, "max-depth": 59, "no-console": 15 diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx index 0a3128c6e7fe..cfd0ec1dfb40 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx @@ -503,6 +503,110 @@ describe("CreateMCPServer", () => { }, ); + it("preserves admin-entered app credentials when the URL changes after authorize for true_passthrough", async () => { + await selectHttpTransport(); + + const user = userEvent.setup({ delay: null }); + await user.type(getServerNameInput(), "CF_Keep_Server"); + await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp"); + + await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)"); + + await user.type( + screen.getByPlaceholderText("Leave blank to use dynamic client registration"), + "org-app-client-id", + ); + await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "org-app-secret"); + + await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy()); + await act(async () => { + oauthHook.onTokenReceived!({ access_token: "upstream-tok", token_type: "Bearer" }, undefined); + }); + + // Editing the URL after authorize invalidates the held token (identity change), but the + // declared app is config, not minted material: it must survive the invalidation instead of + // being silently reset, or the server would persist without the configured app. + await act(async () => { + fireEvent.change(screen.getByPlaceholderText("https://your-mcp-server.com"), { + target: { value: "https://other.example.com/mcp" }, + }); + }); + + vi.mocked(networking.createMCPServer).mockResolvedValue({ + server_id: "kept-app-server", + server_name: "CF_Keep_Server", + alias: "CF_Keep_Server", + url: "https://other.example.com/mcp", + transport: "http", + auth_type: "true_passthrough", + created_at: "2024-01-01T00:00:00Z", + created_by: "user-1", + updated_at: "2024-01-01T00:00:00Z", + updated_by: "user-1", + }); + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Add MCP Server" })); + }); + + await waitFor(() => expect(networking.createMCPServer).toHaveBeenCalledTimes(1)); + const [, payload] = vi.mocked(networking.createMCPServer).mock.calls[0]; + expect(payload.url).toBe("https://other.example.com/mcp"); + expect(payload.credentials).toEqual({ + client_id: "org-app-client-id", + client_secret: "org-app-secret", + }); + expect(JSON.stringify(payload)).not.toContain("upstream-tok"); + }); + + it("wipes oauth2-minted credentials when the auth type switches to a client-forwarded mode", async () => { + await selectHttpTransport(); + + const user = userEvent.setup({ delay: null }); + await user.type(getServerNameInput(), "Switch_Server"); + await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp"); + + await selectAntOption("Authentication", "OAuth"); + + // The oauth2 onTokenReceived branch writes the fetched token AND the DCR client into + // form.credentials; both are minted for the oauth2 identity. + await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy()); + await act(async () => { + oauthHook.onTokenReceived!( + { access_token: "oauth2-minted-tok", refresh_token: "oauth2-minted-refresh", token_type: "Bearer" }, + { clientId: "dcr-minted-client", clientSecret: "dcr-minted-secret" }, + ); + }); + + // Switching into a client-forwarded mode changes the identity with auth_type in the changed + // values, so the preserve carve-out must NOT apply: the minted material would otherwise ride + // into a mode that now persists credentials onto the server row. + await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)"); + + vi.mocked(networking.createMCPServer).mockResolvedValue({ + server_id: "switched-server", + server_name: "Switch_Server", + alias: "Switch_Server", + url: "https://example.com/mcp", + transport: "http", + auth_type: "true_passthrough", + created_at: "2024-01-01T00:00:00Z", + created_by: "user-1", + updated_at: "2024-01-01T00:00:00Z", + updated_by: "user-1", + }); + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Add MCP Server" })); + }); + + await waitFor(() => expect(networking.createMCPServer).toHaveBeenCalledTimes(1)); + const [, payload] = vi.mocked(networking.createMCPServer).mock.calls[0]; + expect(payload.credentials).toBeUndefined(); + expect(JSON.stringify(payload)).not.toContain("dcr-minted-client"); + expect(JSON.stringify(payload)).not.toContain("oauth2-minted-tok"); + }); + it("should not show auth value field when None auth type is selected", async () => { await selectHttpTransport(); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx index 79db031008e6..a526bc935461 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx @@ -18,6 +18,7 @@ import { getOAuthAuthorizationIdentity, CLEARED_ON_INVALIDATION, isHeldOAuthTokenStale, + preservedDeclaredAppCredentials, } from "./types"; import OAuthFormFields from "./OAuthFormFields"; import TruePassthroughWarning from "./TruePassthroughWarning"; @@ -248,7 +249,15 @@ const CreateMCPServer: React.FC = ({ clearTools(); resetOAuthFlow(); setAuthorizedIdentity(undefined); + const keptAppCredentials = preservedDeclaredAppCredentials( + form.getFieldValue("auth_type"), + "auth_type" in changedValues, + form.getFieldValue("credentials"), + ); form.resetFields([...CLEARED_ON_INVALIDATION]); + if (keptAppCredentials) { + form.setFieldsValue({ credentials: keptAppCredentials }); + } const preserved = Object.fromEntries( CLEARED_ON_INVALIDATION.filter((key) => key in changedValues).map((key) => [key, changedValues[key]]), ); 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 a1bad0307b0b..4e0129d4ba12 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 @@ -1423,6 +1423,60 @@ describe("MCPServerEdit (OAuth token persistence on save)", () => { }, ); + it.each([["true_passthrough"], ["oauth_delegate"]])( + "preserves admin-entered app credentials when the URL changes after authorize for the %s mode", + async (authType) => { + vi.mocked(networking.updateMCPServer).mockResolvedValue({ + ...interactiveOAuthServer, + auth_type: authType, + }); + + render( + , + ); + + const user = userEvent.setup({ delay: null }); + await user.type( + screen.getByPlaceholderText("Leave blank to use dynamic client registration"), + "org-app-client-id", + ); + await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "org-app-secret"); + + act(() => { + mockOauth.onTokenReceived?.({ access_token: "cf-tok", token_type: "bearer" }); + }); + + // The URL edit invalidates the held browser token (removeToken fires), but the declared app + // is config and must survive the invalidation into the update payload. + await act(async () => { + fireEvent.change(screen.getByPlaceholderText("https://your-mcp-server.com"), { + target: { value: "https://other.example.com/mcp" }, + }); + }); + expect(mockRemoveToken).toHaveBeenCalledWith("oauth_server_1", "user-1"); + + await act(async () => { + fireEvent.click(screen.getAllByRole("button", { name: "Save Changes" })[0]); + }); + + await waitFor(() => expect(networking.updateMCPServer).toHaveBeenCalledTimes(1)); + const [, payload] = vi.mocked(networking.updateMCPServer).mock.calls[0]; + expect(payload.url).toBe("https://other.example.com/mcp"); + expect(payload.credentials).toMatchObject({ + client_id: "org-app-client-id", + client_secret: "org-app-secret", + }); + expect(JSON.stringify(payload)).not.toContain("cf-tok"); + }, + ); + it("forwards a newly authorized browser-held token for tool loading before the form is saved", async () => { // Regression: fetchTools keyed the browser-held decision off the saved mcpServer.auth_type, so // after switching the form to true_passthrough and authorizing, the fresh token was not sent as 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 9b2c5af861f4..62f9b7e6931a 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 @@ -8,6 +8,7 @@ import { getOAuthAuthorizationIdentity, CLEARED_ON_INVALIDATION, isHeldOAuthTokenStale, + preservedDeclaredAppCredentials, OAUTH_FLOW, MCP_OAUTH2_FLOW_M2M, MCP_OAUTH2_FLOW_INTERACTIVE, @@ -409,7 +410,15 @@ const MCPServerEdit: React.FC = ({ } setTools([]); resetOAuthFlow(); + const keptAppCredentials = preservedDeclaredAppCredentials( + getEffectiveAuthType(), + "auth_type" in changedValues, + form.getFieldValue("credentials"), + ); form.resetFields([...CLEARED_ON_INVALIDATION]); + if (keptAppCredentials) { + form.setFieldsValue({ credentials: keptAppCredentials }); + } const preserved = Object.fromEntries( CLEARED_ON_INVALIDATION.filter((key) => key in changedValues).map((key) => [key, changedValues[key]]), ); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx index 3eba8b309685..e49fd5d57e46 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx @@ -96,6 +96,31 @@ export const getOAuthAuthorizationIdentity = (values: Record): // edit forms so what gets wiped cannot drift. export const CLEARED_ON_INVALIDATION = ["credentials"] as const; +// The carve-out to the wipe above for the client-forwarded token modes: their onTokenReceived branch +// never writes minted material into form.credentials, so for them the field only ever holds the +// admin-DECLARED upstream app (persisted as server config since the modes joined +// AUTH_TYPES_REQUIRING_CREDENTIALS), and an intra-mode identity change (e.g. a URL edit after +// Authorize) must not silently discard it. Two guards make the preserve safe: it never applies when +// auth_type itself changed (the previous mode's onTokenReceived may have written a fetched token or +// DCR client into the same field, and those are minted for the old mode), and it only ever keeps the +// declared-app keys, so token-shaped keys can never ride through a preserve. Shared by the create and +// edit forms so the carve-out cannot drift. +const DECLARED_APP_CREDENTIAL_KEYS = ["client_id", "client_secret"] as const; + +export const preservedDeclaredAppCredentials = ( + authType: string | null | undefined, + authTypeChanged: boolean, + credentials: Record | null | undefined, +): Record | undefined => { + if (!isClientForwardedTokenMode(authType) || authTypeChanged || !credentials) return undefined; + const kept = Object.fromEntries( + DECLARED_APP_CREDENTIAL_KEYS.filter((key) => typeof credentials[key] === "string" && credentials[key] !== "").map( + (key) => [key, credentials[key] as string], + ), + ); + return Object.keys(kept).length > 0 ? kept : undefined; +}; + // True when a token was authorized in this session (authorizedIdentity recorded at mint time) and the // form's current identity no longer matches it. Every invalidation decision in both forms goes through // this single check: onValuesChange for user edits, and an explicit recheck after any programmatic From cf1b407fbe0941890eb309e36d8c12f4ccb3b42d Mon Sep 17 00:00:00 2001 From: Tin Date: Fri, 10 Jul 2026 09:51:10 -0700 Subject: [PATCH 03/10] fix(mcp): state the keep-existing convention for blank client fields and add explicit app removal on edit --- .../mcp_tools/PassthroughAuthorizeSection.tsx | 29 +++++++++++++- .../mcp_tools/mcp_server_edit.test.tsx | 40 ++++++++++++++++++- .../components/mcp_tools/mcp_server_edit.tsx | 12 ++++++ 3 files changed, 77 insertions(+), 4 deletions(-) diff --git a/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx b/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx index 314b45fff6c5..8d53e8b86eea 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { Button, Form, Input } from "antd"; +import { Button, Checkbox, Form, Input } from "antd"; import { isClientForwardedTokenMode } from "./types"; interface PassthroughOAuthFlow { @@ -19,13 +19,26 @@ interface PassthroughOAuthFlow { * pre-registered Slack app); unlike the token they ARE saved onto the server * as declared config, so internal users' Authorize relays through the org's * app instead of dead-ending on upstreams that cannot mint clients. + * + * Blank fields follow the same convention as the M2M credential fields: on + * create they mean "no app configured" (dynamic client registration), while on + * edit the backend's partial update keeps whatever app is already stored, so + * blanks mean "keep existing". Removing a stored app is therefore an explicit + * action (the checkbox below, edit only), which saves an explicit-null + * credential write instead of omitting the field. */ export default function PassthroughAuthorizeSection({ authType, oauthFlow, + isEditing = false, + removeStoredApp = false, + onRemoveStoredAppChange, }: { authType?: string | null; oauthFlow: PassthroughOAuthFlow; + isEditing?: boolean; + removeStoredApp?: boolean; + onRemoveStoredAppChange?: (remove: boolean) => void; }) { if (!isClientForwardedTokenMode(authType)) return null; const authorizeButtonLabels: Record = { @@ -33,6 +46,9 @@ export default function PassthroughAuthorizeSection({ exchanging: "Exchanging authorization code...", }; const authorizeButtonLabel = authorizeButtonLabels[oauthFlow.status] ?? "Authorize & Fetch Tools (browser-only)"; + const blankMeaning = isEditing + ? "Leave blank to keep the currently saved app (if any)" + : "Leave blank to use dynamic client registration"; return (

@@ -47,7 +63,8 @@ export default function PassthroughAuthorizeSection({ extra="Set this to make everyone authorize through a specific app; required for upstreams without dynamic client registration (e.g. a pre-registered Slack app)." > @@ -57,9 +74,17 @@ export default function PassthroughAuthorizeSection({ > + {isEditing && onRemoveStoredAppChange && ( + onRemoveStoredAppChange(e.target.checked)}> + + Remove the saved OAuth app on save (the server goes back to dynamic client registration) + + + )}