@@ -57,13 +72,19 @@ export default function PassthroughAuthorizeSection({
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.
+ {appMayNotMatchUpstream && (
+
+ You changed the upstream URL or endpoints; the OAuth app entered here was registered for the previous upstream
+ and may not be valid. Update the client ID, or clear it to use dynamic client registration.
+
+ )}
OAuth Client ID (optional, saved)}
name={["credentials", "client_id"]}
- 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)."
+ extra={clientIdExtra}
>
@@ -73,7 +94,7 @@ export default function PassthroughAuthorizeSection({
name={["credentials", "client_secret"]}
>
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 a4668bddfc3..6d1d18b3902 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
@@ -609,6 +609,226 @@ describe("CreateMCPServer", () => {
expect(JSON.stringify(payload)).not.toContain("oauth2-minted-tok");
});
+ it("keeps the DCR-minted client out of form.credentials but reuses it via getCredentials", async () => {
+ await selectHttpTransport();
+ const user = userEvent.setup({ delay: null });
+ await user.type(getServerNameInput(), "DCR_Server");
+ await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp");
+ await selectAntOption("Authentication", "OAuth");
+
+ await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy());
+ await act(async () => {
+ oauthHook.onTokenReceived!(
+ { access_token: "oauth2-tok", token_type: "Bearer" },
+ { clientId: "dcr-client", clientSecret: "dcr-secret" },
+ );
+ });
+
+ // The DCR client must NOT be in the form store (or it could be collected as a CF server's app),
+ // but getCredentials merges it so a re-authorize reuses the registered client instead of re-DCRing.
+ expect(oauthHook.getCredentials?.()?.client_id).toBe("dcr-client");
+ });
+
+ it("persists the DCR client on an oauth2 submit via the ref", async () => {
+ await selectHttpTransport();
+ const user = userEvent.setup({ delay: null });
+ await user.type(getServerNameInput(), "DCR_Submit_Server");
+ await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp");
+ await selectAntOption("Authentication", "OAuth");
+
+ await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy());
+ await act(async () => {
+ oauthHook.onTokenReceived!(
+ { access_token: "oauth2-tok", token_type: "Bearer" },
+ { clientId: "dcr-client", clientSecret: "dcr-secret" },
+ );
+ });
+
+ const dcrSubmitServer = {
+ server_id: "dcr-submit",
+ server_name: "DCR_Submit_Server",
+ alias: "DCR_Submit_Server",
+ url: "https://example.com/mcp",
+ transport: "http",
+ auth_type: "oauth2",
+ 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(dcrSubmitServer);
+ 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.client_id).toBe("dcr-client");
+ expect(payload.credentials.client_secret).toBe("dcr-secret");
+ });
+
+ it("preserves the typed app across a switch between the two client-forwarded modes", async () => {
+ await selectHttpTransport();
+ const user = userEvent.setup({ delay: null });
+ await user.type(getServerNameInput(), "CF_Switch_Keep");
+ 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"), "app-id");
+ await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "app-secret");
+
+ await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy());
+ await act(async () => {
+ oauthHook.onTokenReceived!({ access_token: "cf-tok", token_type: "Bearer" }, undefined);
+ });
+
+ await selectAntOption("Authentication", "OAuth Delegate (client-supplied upstream token)");
+
+ const switched = {
+ server_id: "cf-switch-keep",
+ server_name: "CF_Switch_Keep",
+ alias: "CF_Switch_Keep",
+ url: "https://example.com/mcp",
+ transport: "http",
+ auth_type: "oauth_delegate",
+ 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(switched);
+ 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).toEqual({ client_id: "app-id", client_secret: "app-secret" });
+ });
+
+ it("preserves the typed app across a client-forwarded -> oauth2 -> client-forwarded round trip", async () => {
+ await selectHttpTransport();
+ const user = userEvent.setup({ delay: null });
+ await user.type(getServerNameInput(), "CF_Round");
+ 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"), "app-id");
+ await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "app-secret");
+
+ await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy());
+ await act(async () => {
+ oauthHook.onTokenReceived!({ access_token: "cf-tok", token_type: "Bearer" }, undefined);
+ });
+
+ await selectAntOption("Authentication", "OAuth");
+ await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)");
+
+ const cfRoundServer = {
+ server_id: "cf-round",
+ server_name: "CF_Round",
+ alias: "CF_Round",
+ 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",
+ };
+ vi.mocked(networking.createMCPServer).mockResolvedValue(cfRoundServer);
+ 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).toEqual({ client_id: "app-id", client_secret: "app-secret" });
+ });
+
+ it("keeps the typed app but warns when the URL changes after a client-forwarded authorize", async () => {
+ await selectHttpTransport();
+ const user = userEvent.setup({ delay: null });
+ await user.type(getServerNameInput(), "CF_Warn");
+ 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"), "app-id");
+
+ await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy());
+ await act(async () => {
+ oauthHook.onTokenReceived!({ access_token: "cf-tok", token_type: "Bearer" }, undefined);
+ });
+
+ await act(async () => {
+ fireEvent.change(screen.getByPlaceholderText("https://your-mcp-server.com"), {
+ target: { value: "https://other.example.com/mcp" },
+ });
+ });
+
+ // Keep + warn: the app stays in the field, and a non-blocking warning appears.
+ expect(screen.getByText(/OAuth app entered here was registered for the previous upstream/)).toBeInTheDocument();
+ });
+
+ it("keeps client_secret when only client_id is edited after a client-forwarded authorize", async () => {
+ await selectHttpTransport();
+ const user = userEvent.setup({ delay: null });
+ await user.type(getServerNameInput(), "CF_Keystroke");
+ 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"), "app-id");
+ await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "app-secret");
+
+ await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy());
+ await act(async () => {
+ oauthHook.onTokenReceived!({ access_token: "cf-tok", token_type: "Bearer" }, undefined);
+ });
+
+ // Editing only client_id fires an invalidation whose changedValues carries only the client_id
+ // sub-field; the preserve + deep-merge re-apply must keep client_secret from being dropped.
+ await user.type(screen.getByPlaceholderText("Leave blank to use dynamic client registration"), "2");
+
+ const cfKeystrokeServer = {
+ server_id: "cf-keystroke",
+ server_name: "CF_Keystroke",
+ alias: "CF_Keystroke",
+ 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",
+ };
+ vi.mocked(networking.createMCPServer).mockResolvedValue(cfKeystrokeServer);
+ 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).toEqual({ client_id: "app-id2", client_secret: "app-secret" });
+ });
+
+ it("replaces the token set on re-authorize instead of leaving stale siblings", async () => {
+ await selectHttpTransport();
+ const user = userEvent.setup({ delay: null });
+ await user.type(getServerNameInput(), "Reauth_Server");
+ await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp");
+ await selectAntOption("Authentication", "OAuth");
+
+ await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy());
+ const firstToken = { access_token: "T1", refresh_token: "R1", scope: "read", token_type: "Bearer" };
+ await act(async () => {
+ oauthHook.onTokenReceived!(firstToken, undefined);
+ });
+ await act(async () => {
+ oauthHook.onTokenReceived!({ access_token: "T2", token_type: "Bearer" }, undefined);
+ });
+
+ const creds = oauthHook.getCredentials?.() ?? {};
+ expect(creds.access_token).toBe("T2");
+ expect(creds.refresh_token).toBeUndefined();
+ expect(creds.scope).toBeUndefined();
+ });
+
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 a526bc93546..52abd40db93 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
@@ -19,6 +19,7 @@ import {
CLEARED_ON_INVALIDATION,
isHeldOAuthTokenStale,
preservedDeclaredAppCredentials,
+ withoutMintedTokenCredentials,
} from "./types";
import OAuthFormFields from "./OAuthFormFields";
import TruePassthroughWarning from "./TruePassthroughWarning";
@@ -109,6 +110,14 @@ const CreateMCPServer: React.FC = ({
// was fetched; undefined when no valid token is held. If any mint-relevant field diverges from this,
// the held token is stale and is discarded so the admin must re-authorize.
const [authorizedIdentity, setAuthorizedIdentity] = useState(undefined);
+ // The DCR-minted OAuth client from an interactive (oauth2) Authorize. Held OUT of form.credentials so
+ // it can never be collected as a client-forwarded server's declared app; injected into the payload
+ // only on an oauth2 submit (where persisting the registered client is correct), and cleared on any
+ // invalidation or modal close. An abandoned authorize leaves it null, which is the desired asymmetry.
+ const dcrClientRef = React.useRef<{ client_id: string; client_secret?: string } | null>(null);
+ // Set when the upstream identity (url/endpoints) changed while a declared app is present, so the
+ // section can warn that the saved app may not match the new upstream (the app is kept, not wiped).
+ const [appMayNotMatchUpstream, setAppMayNotMatchUpstream] = useState(false);
// Single hook call shared by MCPConnectionStatus and MCPToolConfiguration to avoid duplicate requests.
const {
@@ -150,6 +159,9 @@ const CreateMCPServer: React.FC = ({
searchValue,
aliasManuallyEdited,
logoUrl,
+ // Persist the identity so invalidation stays armed across the OAuth redirect round trip: a
+ // post-restore url/mode edit must still discard the held token instead of silently keeping it.
+ authorizedIdentity,
};
setSecureItem(CREATE_OAUTH_UI_STATE_KEY, JSON.stringify(uiState));
} catch (err) {
@@ -165,7 +177,12 @@ const CreateMCPServer: React.FC = ({
reset: resetOAuthFlow,
} = useMcpOAuthFlow({
accessToken,
- getCredentials: () => form.getFieldValue("credentials"),
+ // Merge the ref-held DCR client so a re-authorize reuses the registered client instead of
+ // re-registering; the form store itself never holds the DCR client (see onTokenReceived).
+ getCredentials: () => ({
+ ...((form.getFieldValue("credentials") as Record | undefined) ?? {}),
+ ...(dcrClientRef.current ?? {}),
+ }),
getTemporaryPayload: () => {
const values = form.getFieldsValue(true);
const transport = values.transport || transportType;
@@ -187,7 +204,9 @@ const CreateMCPServer: React.FC = ({
url,
transport: transport === TRANSPORT.OPENAPI ? "http" : transport,
auth_type: isClientForwardedTokenMode(values.auth_type) ? values.auth_type : AUTH_TYPE.OAUTH2,
- credentials: values.credentials,
+ credentials: isClientForwardedTokenMode(values.auth_type)
+ ? preservedDeclaredAppCredentials(values.credentials)
+ : values.credentials,
authorization_url: values.authorization_url,
token_url: values.token_url,
registration_url: values.registration_url,
@@ -217,18 +236,31 @@ const CreateMCPServer: React.FC = ({
return;
}
- const credentials = {
+ // The DCR-minted client is held in a ref, NOT written into form.credentials, so it can never be
+ // collected as a client-forwarded server's declared app; it is injected into the payload only on
+ // an oauth2 submit. An admin-typed client already lives in form.credentials and is left untouched.
+ dcrClientRef.current = registeredClient?.clientId
+ ? {
+ client_id: registeredClient.clientId,
+ ...(registeredClient.clientSecret && { client_secret: registeredClient.clientSecret }),
+ }
+ : null;
+
+ const current = (form.getFieldValue("credentials") as Record | undefined) ?? {};
+ const nextCredentials = {
+ ...(preservedDeclaredAppCredentials(current) ?? {}),
+ ...(current.scopes !== undefined && { scopes: current.scopes }),
access_token: token.access_token,
...(token.refresh_token && { refresh_token: token.refresh_token }),
...(token.expires_in && { expires_in: token.expires_in }),
...(token.scope && { scope: token.scope }),
- ...(registeredClient?.clientId && { client_id: registeredClient.clientId }),
- ...(registeredClient?.clientSecret && { client_secret: registeredClient.clientSecret }),
};
-
- form.setFieldsValue({ credentials });
- // Capture the identity AFTER writing the DCR'd credentials so the held token is not spuriously
- // invalidated by its own credential write.
+ // Path-replace (not deep-merge) so a re-authorize with fewer token fields does not leave stale
+ // siblings from the previous token behind; the admin-typed client keys and scopes are carried
+ // explicitly above.
+ form.setFieldValue("credentials", nextCredentials);
+ // Capture the identity AFTER writing the token so the held token is not spuriously invalidated by
+ // its own credential write.
setAuthorizedIdentity(getOAuthAuthorizationIdentity(form.getFieldsValue(true)));
NotificationsManager.success(
@@ -249,15 +281,17 @@ const CreateMCPServer: React.FC = ({
clearTools();
resetOAuthFlow();
setAuthorizedIdentity(undefined);
- const keptAppCredentials = preservedDeclaredAppCredentials(
- form.getFieldValue("auth_type"),
- "auth_type" in changedValues,
- form.getFieldValue("credentials"),
- );
+ dcrClientRef.current = null;
+ // Capture the admin-typed app before resetFields destroys it, then re-apply it: the app is
+ // upstream-scoped config, not minted material, so it survives every invalidation (the token is
+ // what gets discarded). Token-shaped keys are excluded by the helper's key filter.
+ const keptAppCredentials = preservedDeclaredAppCredentials(form.getFieldValue("credentials"));
form.resetFields([...CLEARED_ON_INVALIDATION]);
if (keptAppCredentials) {
form.setFieldsValue({ credentials: keptAppCredentials });
}
+ // Re-apply the in-flight edit last; rc-field-form deep-merges nested objects, so a changed
+ // credentials sub-field composes with the preserved sibling instead of replacing the object.
const preserved = Object.fromEntries(
CLEARED_ON_INVALIDATION.filter((key) => key in changedValues).map((key) => [key, changedValues[key]]),
);
@@ -285,7 +319,20 @@ const CreateMCPServer: React.FC = ({
setTransportType(restoredTransport);
}
if (parsed.formValues) {
- setPendingRestoredValues({ values: parsed.formValues, transport: restoredTransport });
+ // Strip minted token material from the restored credentials so a stale token never rehydrates
+ // into the form store (defense in depth for pre-fix snapshots); the declared app is kept.
+ const restoredValues = {
+ ...parsed.formValues,
+ ...(parsed.formValues.credentials
+ ? { credentials: withoutMintedTokenCredentials(parsed.formValues.credentials) }
+ : {}),
+ };
+ setPendingRestoredValues({ values: restoredValues, transport: restoredTransport });
+ }
+ if (typeof parsed.authorizedIdentity === "string") {
+ // Re-arm invalidation: without this the remounted form has authorizedIdentity=undefined, so a
+ // post-restore mode/url edit would never fire the stale-token discard.
+ setAuthorizedIdentity(parsed.authorizedIdentity);
}
if (parsed.costConfig) {
setCostConfig(parsed.costConfig);
@@ -511,8 +558,20 @@ const CreateMCPServer: React.FC = ({
const includeCredentials =
restValues.auth_type && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(restValues.auth_type);
- if (includeCredentials && credentialsPayload && Object.keys(credentialsPayload).length > 0) {
- payload.credentials = credentialsPayload;
+ // Client-forwarded rows persist ONLY the declared app; strip any token material that lingered in
+ // the form (e.g. from a prior oauth2 authorize on the same session) so it can never reach the row.
+ const submitCredentials = isClientForwardedTokenMode(restValues.auth_type)
+ ? preservedDeclaredAppCredentials(credentialsPayload)
+ : credentialsPayload;
+
+ if (includeCredentials && submitCredentials && Object.keys(submitCredentials).length > 0) {
+ payload.credentials = submitCredentials;
+ }
+
+ // An interactive (oauth2) create persists its DCR-minted client from the ref (kept out of the
+ // form store); reuse a re-authorize's registered client instead of re-registering.
+ if (restValues.auth_type === AUTH_TYPE.OAUTH2 && dcrClientRef.current) {
+ payload.credentials = { ...(payload.credentials ?? {}), ...dcrClientRef.current };
}
if (accessToken != null) {
@@ -587,6 +646,9 @@ const CreateMCPServer: React.FC = ({
setHasToolAllowlistInteraction(false);
setAliasManuallyEdited(false);
setLogoUrl(undefined);
+ setAuthorizedIdentity(undefined);
+ dcrClientRef.current = null;
+ setAppMayNotMatchUpstream(false);
setModalVisible(false);
};
@@ -674,12 +736,28 @@ const CreateMCPServer: React.FC = ({
const handleFormValuesChange = (changedValues: Record, allValues: Record) => {
// Any change to a mint-relevant field (url, auth_type, oauth_flow_type, client creds/scopes, or the
// authorization/token/registration endpoints — see getOAuthAuthorizationIdentity) makes a held token
- // stale, so discard it and force a fresh authorize. When that happens, formValues must be rebuilt
- // from the form's post-reset state, not the pre-reset allValues snapshot: the snapshot still holds
- // the discarded token in credentials, and useTestMCPConnection reads formValues for tool preview.
- if (isHeldOAuthTokenStale(allValues, authorizedIdentity)) {
+ // stale, so discard it and force a fresh authorize. The stale check reads getFieldsValue(true): the
+ // onValuesChange allValues argument holds only MOUNTED paths, so an unmounted identity field (e.g.
+ // an oauth_flow_type initialValue while in a client-forwarded mode) would compare as changed on
+ // every keystroke and churn the held token. When a clear happens, formValues is rebuilt from the
+ // form's post-reset state (not the pre-reset snapshot, which still holds the discarded token).
+ // Editing the client fields is the admin managing/acknowledging the app, so it always dismisses
+ // the "may not match upstream" warning regardless of the stale-token branch below.
+ if ("credentials" in changedValues) {
+ setAppMayNotMatchUpstream(false);
+ }
+ if (isHeldOAuthTokenStale(form.getFieldsValue(true), authorizedIdentity)) {
+ // A url/endpoint change while a declared app is present keeps the app but flags that it may not
+ // match the new upstream (the "keep + warn" behavior); a client-key edit is handled above.
+ const upstreamChanged = ["url", "spec_path", "authorization_url", "token_url", "registration_url"].some(
+ (key) => key in changedValues,
+ );
+ const hasDeclaredApp = preservedDeclaredAppCredentials(form.getFieldValue("credentials")) !== undefined;
clearHeldOAuthToken(changedValues);
- setFormValues({ ...form.getFieldsValue(true), ...changedValues });
+ if (upstreamChanged && hasDeclaredApp && !("credentials" in changedValues)) {
+ setAppMayNotMatchUpstream(true);
+ }
+ setFormValues(form.getFieldsValue(true));
return;
}
setFormValues(allValues);
@@ -1006,6 +1084,7 @@ const CreateMCPServer: React.FC = ({
error: oauthError,
tokenResponse: oauthTokenResponse,
}}
+ appMayNotMatchUpstream={appMayNotMatchUpstream}
/>
{shouldShowAuthValueField && (
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 5691023f79d..9cfe56f5fe2 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
@@ -1403,7 +1403,10 @@ describe("MCPServerEdit (OAuth token persistence on save)", () => {
screen.getByPlaceholderText("Leave blank to keep the currently saved app (if any)"),
"org-app-client-id",
);
- await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "org-app-secret");
+ await user.type(
+ screen.getByPlaceholderText("Leave blank to keep the currently saved secret (if any)"),
+ "org-app-secret",
+ );
await act(async () => {
fireEvent.click(screen.getAllByRole("button", { name: "Save Changes" })[0]);
@@ -1447,7 +1450,10 @@ describe("MCPServerEdit (OAuth token persistence on save)", () => {
screen.getByPlaceholderText("Leave blank to keep the currently saved app (if any)"),
"org-app-client-id",
);
- await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "org-app-secret");
+ await user.type(
+ screen.getByPlaceholderText("Leave blank to keep the currently saved secret (if any)"),
+ "org-app-secret",
+ );
act(() => {
mockOauth.onTokenReceived?.({ access_token: "cf-tok", token_type: "bearer" });
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 6d2d24356da..741b23a12fa 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
@@ -9,6 +9,7 @@ import {
CLEARED_ON_INVALIDATION,
isHeldOAuthTokenStale,
preservedDeclaredAppCredentials,
+ withoutMintedTokenCredentials,
OAUTH_FLOW,
MCP_OAUTH2_FLOW_M2M,
MCP_OAUTH2_FLOW_INTERACTIVE,
@@ -183,7 +184,9 @@ const MCPServerEdit: React.FC = ({
url,
transport,
auth_type: isClientForwardedTokenMode(values.auth_type) ? values.auth_type : AUTH_TYPE.OAUTH2,
- credentials: values.credentials,
+ credentials: isClientForwardedTokenMode(values.auth_type)
+ ? preservedDeclaredAppCredentials(values.credentials)
+ : values.credentials,
mcp_access_groups: values.mcp_access_groups || mcpServer.mcp_access_groups,
static_headers: staticHeaders,
command: values.command,
@@ -211,14 +214,18 @@ const MCPServerEdit: React.FC = ({
return;
}
- const credentials = {
+ const current = (form.getFieldValue("credentials") as Record | undefined) ?? {};
+ const nextCredentials = {
+ ...(preservedDeclaredAppCredentials(current) ?? {}),
+ ...(current.scopes !== undefined && { scopes: current.scopes }),
access_token: token.access_token,
...(token.refresh_token && { refresh_token: token.refresh_token }),
...(token.expires_in && { expires_in: token.expires_in }),
...(token.scope && { scope: token.scope }),
};
-
- form.setFieldsValue({ credentials });
+ // Path-replace (not deep-merge) so a re-authorize with fewer token fields does not leave stale
+ // siblings behind; the admin-typed client keys and scopes are carried explicitly above.
+ form.setFieldValue("credentials", nextCredentials);
// Re-capture after writing credentials so the token is not invalidated by its own credential write.
authorizedIdentityRef.current = getOAuthAuthorizationIdentity(form.getFieldsValue(true));
@@ -336,8 +343,19 @@ const MCPServerEdit: React.FC = ({
return;
}
if (parsed.formValues) {
- setPendingRestoredValues({ ...mcpServer, ...parsed.formValues });
+ // Strip minted token material from restored credentials so a stale token never rehydrates into
+ // the form store (defense in depth for pre-fix snapshots); the declared app is kept.
+ const restoredValues = {
+ ...mcpServer,
+ ...parsed.formValues,
+ ...(parsed.formValues.credentials
+ ? { credentials: withoutMintedTokenCredentials(parsed.formValues.credentials) }
+ : {}),
+ };
+ setPendingRestoredValues(restoredValues);
}
+ // The ref is re-armed by onTokenReceived when the redirect completes the code exchange, so there
+ // is no separate restore-side re-arm here (writing a ref inside an effect is disallowed).
if (parsed.costConfig) {
setCostConfig(parsed.costConfig);
}
@@ -411,11 +429,9 @@ const MCPServerEdit: React.FC = ({
}
setTools([]);
resetOAuthFlow();
- const keptAppCredentials = preservedDeclaredAppCredentials(
- getEffectiveAuthType(),
- "auth_type" in changedValues,
- form.getFieldValue("credentials"),
- );
+ // The admin-typed app is upstream-scoped config, not minted material, so it survives every
+ // invalidation; only the held token is discarded. Token-shaped keys are excluded by the filter.
+ const keptAppCredentials = preservedDeclaredAppCredentials(form.getFieldValue("credentials"));
form.resetFields([...CLEARED_ON_INVALIDATION]);
if (keptAppCredentials) {
form.setFieldsValue({ credentials: keptAppCredentials });
@@ -862,14 +878,20 @@ const MCPServerEdit: React.FC = ({
const includeCredentials =
restValues.auth_type && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(restValues.auth_type);
- if (includeCredentials && credentialsPayload && Object.keys(credentialsPayload).length > 0) {
- payload.credentials = credentialsPayload;
+ // Client-forwarded rows persist ONLY the declared app; strip any token material lingering in the
+ // form (e.g. from a prior oauth2 authorize this session) so it can never reach the row.
+ const submitCredentials = isClientForwardedTokenMode(restValues.auth_type)
+ ? preservedDeclaredAppCredentials(credentialsPayload)
+ : credentialsPayload;
+
+ if (includeCredentials && submitCredentials && Object.keys(submitCredentials).length > 0) {
+ payload.credentials = submitCredentials;
}
- // Explicit removal of a saved app for the client-forwarded modes. Blank fields are the
- // keep-existing convention (the backend merges partial credential updates), so removal must be
- // an explicit-null write: encrypt skips nulls and the merge overrides the stored keys, which
- // returns the server to dynamic client registration.
+ // Explicit removal of a saved app for the client-forwarded modes, applied AFTER the filter so it
+ // always wins. Blank fields are the keep-existing convention (the backend merges partial
+ // credential updates), so removal must be an explicit-null write: encrypt skips nulls and the
+ // merge overrides the stored keys, returning the server to dynamic client registration.
if (removeStoredApp && isClientForwardedTokenMode(restValues.auth_type)) {
payload.credentials = { client_id: null, client_secret: null };
}
@@ -1061,6 +1083,7 @@ const MCPServerEdit: React.FC = ({
tokenResponse: oauthTokenResponse,
}}
isEditing
+ savedAuthType={mcpServer.auth_type}
removeStoredApp={removeStoredApp}
onRemoveStoredAppChange={setRemoveStoredApp}
/>
diff --git a/ui/litellm-dashboard/src/components/mcp_tools/types.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/types.test.tsx
index c6faca1fb51..48f9fc2ac75 100644
--- a/ui/litellm-dashboard/src/components/mcp_tools/types.test.tsx
+++ b/ui/litellm-dashboard/src/components/mcp_tools/types.test.tsx
@@ -10,6 +10,9 @@ import {
getOAuthAuthorizationIdentity,
isHeldOAuthTokenStale,
oauth2FlowToFormValue,
+ preservedDeclaredAppCredentials,
+ withoutMintedTokenCredentials,
+ credentialAuthClass,
} from "./types";
describe("getOAuthAuthorizationIdentity", () => {
@@ -180,3 +183,45 @@ describe("oauth2FlowToFormValue", () => {
expect(oauth2FlowToFormValue(undefined)).toBeUndefined();
});
});
+
+describe("preservedDeclaredAppCredentials", () => {
+ it("keeps only non-empty string declared-app keys and never token-shaped keys", () => {
+ expect(preservedDeclaredAppCredentials(undefined)).toBeUndefined();
+ expect(preservedDeclaredAppCredentials({})).toBeUndefined();
+ expect(preservedDeclaredAppCredentials({ client_id: 123 })).toBeUndefined();
+ expect(preservedDeclaredAppCredentials({ client_id: "" })).toBeUndefined();
+ expect(preservedDeclaredAppCredentials({ client_id: "a", access_token: "t", scopes: ["s"] })).toEqual({
+ client_id: "a",
+ });
+ expect(preservedDeclaredAppCredentials({ client_secret: "s" })).toEqual({ client_secret: "s" });
+ expect(preservedDeclaredAppCredentials({ client_id: "a", client_secret: "b", refresh_token: "r" })).toEqual({
+ client_id: "a",
+ client_secret: "b",
+ });
+ });
+});
+
+describe("withoutMintedTokenCredentials", () => {
+ it("drops token keys and keeps the declared app and other config", () => {
+ expect(withoutMintedTokenCredentials(undefined)).toBeUndefined();
+ const mixed = {
+ client_id: "a",
+ client_secret: "b",
+ access_token: "t",
+ refresh_token: "r",
+ expires_in: 3600,
+ scope: "read",
+ scopes: ["read"],
+ };
+ expect(withoutMintedTokenCredentials(mixed)).toEqual({ client_id: "a", client_secret: "b", scopes: ["read"] });
+ });
+});
+
+describe("credentialAuthClass", () => {
+ it("collapses the client-forwarded modes to one class and leaves others distinct", () => {
+ expect(credentialAuthClass(AUTH_TYPE.TRUE_PASSTHROUGH)).toBe("client_forwarded");
+ expect(credentialAuthClass(AUTH_TYPE.OAUTH_DELEGATE)).toBe("client_forwarded");
+ expect(credentialAuthClass(AUTH_TYPE.OAUTH2)).toBe(AUTH_TYPE.OAUTH2);
+ expect(credentialAuthClass(null)).toBeNull();
+ });
+});
diff --git a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx
index e49fd5d57e4..29441cdb84c 100644
--- a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx
+++ b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx
@@ -96,23 +96,25 @@ 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.
+// The declared-app filter over form.credentials. It is a pure key filter with no mode/transition
+// guard because the surrounding code establishes that a client_id/client_secret in form.credentials
+// is ALWAYS admin-typed in every reachable state: the create form holds the DCR-minted client in a
+// ref and never writes it into the form store, the edit form's onTokenReceived never writes client
+// keys, and the invalidation reset clears the whole object atomically. So preserving the string
+// client keys across any invalidation (URL/endpoint edit, true_passthrough<->oauth_delegate switch,
+// or a round trip through another mode) is always legitimate, while the output key filter excludes
+// token-shaped keys so a preserve can never carry minted material through. Shared by both forms.
const DECLARED_APP_CREDENTIAL_KEYS = ["client_id", "client_secret"] as const;
+// Minted token material the oauth2 authorize path writes beside the app keys; stripped from restored
+// snapshots and from any credentials that transit to the temp-session preview so a stale token never
+// reaches the backend or a client-forwarded server row.
+export const MINTED_TOKEN_CREDENTIAL_KEYS = ["access_token", "refresh_token", "expires_in", "scope"] as const;
+
export const preservedDeclaredAppCredentials = (
- authType: string | null | undefined,
- authTypeChanged: boolean,
credentials: Record | null | undefined,
): Record | undefined => {
- if (!isClientForwardedTokenMode(authType) || authTypeChanged || !credentials) return undefined;
+ if (!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],
@@ -121,6 +123,24 @@ export const preservedDeclaredAppCredentials = (
return Object.keys(kept).length > 0 ? kept : undefined;
};
+// Drop minted token keys, keeping everything else (the declared app plus any non-token config).
+export const withoutMintedTokenCredentials = (
+ credentials: Record | null | undefined,
+): Record | undefined => {
+ if (!credentials) return undefined;
+ return Object.fromEntries(
+ Object.entries(credentials).filter(([key]) => !(MINTED_TOKEN_CREDENTIAL_KEYS as readonly string[]).includes(key)),
+ );
+};
+
+// The client-forwarded modes share one credential class (same declared app, same authorize relay), so
+// a switch between them must NOT be treated as an app change. Mirrors the backend _credential_auth_class
+// in db.py; kept in sync so the UI's keep-existing copy and the backend's merge cannot disagree.
+export const credentialAuthClass = (authType: string | null | undefined): string | null => {
+ if (authType === AUTH_TYPE.TRUE_PASSTHROUGH || authType === AUTH_TYPE.OAUTH_DELEGATE) return "client_forwarded";
+ return authType ?? null;
+};
+
// 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