mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(mcp): preserve a stored client on OAuth-resume restore, add the edit upstream-mismatch warning, and type the credentials field
This commit is contained in:
parent
1be846680a
commit
86506775c9
4 changed files with 52 additions and 11 deletions
|
|
@ -322,13 +322,11 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
setTransportType(restoredTransport);
|
||||
}
|
||||
if (parsed.formValues) {
|
||||
// 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.
|
||||
// Assign the cleaned credentials (strip minted token material so a stale token never rehydrates);
|
||||
// the declared app the admin typed is kept. Create has no server-side stored app to merge.
|
||||
const restoredValues = {
|
||||
...parsed.formValues,
|
||||
...(parsed.formValues.credentials
|
||||
? { credentials: withoutMintedTokenCredentials(parsed.formValues.credentials) }
|
||||
: {}),
|
||||
credentials: withoutMintedTokenCredentials(parsed.formValues.credentials),
|
||||
};
|
||||
setPendingRestoredValues({ values: restoredValues, transport: restoredTransport });
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,7 +2,8 @@ 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 MCPServerEdit, { EDIT_OAUTH_UI_STATE_KEY } from "./mcp_server_edit";
|
||||
import { setSecureItem } from "@/utils/secureStorage";
|
||||
import * as networking from "../networking";
|
||||
import NotificationsManager from "../molecules/notifications_manager";
|
||||
import { selectAntOption } from "./testUtils";
|
||||
|
|
@ -1548,6 +1549,39 @@ describe("MCPServerEdit (OAuth token persistence on save)", () => {
|
|||
expect(screen.getByText(/registered for the previous upstream/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("preserves a stored client_id on OAuth-resume restore even when the saved snapshot is token-only", async () => {
|
||||
// Post-redirect restore: the sessionStorage snapshot carries only a minted token (no client keys),
|
||||
// while the loaded server has a stored client_id. The restore must merge the server's declared app
|
||||
// under the snapshot before stripping tokens, so the stored client_id is never cleared to blank.
|
||||
setSecureItem(
|
||||
EDIT_OAUTH_UI_STATE_KEY,
|
||||
JSON.stringify({
|
||||
serverId: "oauth_server_1",
|
||||
formValues: { auth_type: "true_passthrough", credentials: { access_token: "leftover-token" } },
|
||||
}),
|
||||
);
|
||||
|
||||
render(
|
||||
<MCPServerEdit
|
||||
mcpServer={{
|
||||
...interactiveOAuthServer,
|
||||
auth_type: "true_passthrough",
|
||||
credentials: { client_id: "stored-client" },
|
||||
}}
|
||||
accessToken="access-token"
|
||||
userID="user-1"
|
||||
onCancel={vi.fn()}
|
||||
onSuccess={vi.fn()}
|
||||
availableAccessGroups={[]}
|
||||
/>,
|
||||
);
|
||||
|
||||
const clientIdField = await screen.findByPlaceholderText("Leave blank to keep the currently saved app (if any)");
|
||||
await waitFor(() => expect((clientIdField as HTMLInputElement).value).toBe("stored-client"));
|
||||
// The leftover minted token must not have rehydrated anywhere.
|
||||
expect(document.body.innerHTML).not.toContain("leftover-token");
|
||||
});
|
||||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -309,6 +309,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
}
|
||||
syncedServerIdRef.current = mcpServer.server_id;
|
||||
form.setFieldsValue(initialValues);
|
||||
setAppMayNotMatchUpstream(false);
|
||||
}, [mcpServer.server_id, initialValues, form]);
|
||||
|
||||
// Initialize cost config from existing server data
|
||||
|
|
@ -346,14 +347,19 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
return;
|
||||
}
|
||||
if (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.
|
||||
// Rebuild credentials from the declared app in EITHER the loaded server or the saved snapshot,
|
||||
// then strip minted token material. Merging the two (server under snapshot) before stripping is
|
||||
// what guarantees a token-only snapshot never clears a stored client_id/client_secret: the
|
||||
// server's declared app survives and only the token keys drop. Assigning the cleaned result (not
|
||||
// spreading the raw snapshot) also ensures a stale token can never rehydrate into the form.
|
||||
const restoredCredentials = withoutMintedTokenCredentials({
|
||||
...(mcpServer.credentials ?? {}),
|
||||
...((parsed.formValues.credentials as Record<string, unknown> | undefined) ?? {}),
|
||||
});
|
||||
const restoredValues = {
|
||||
...mcpServer,
|
||||
...parsed.formValues,
|
||||
...(parsed.formValues.credentials
|
||||
? { credentials: withoutMintedTokenCredentials(parsed.formValues.credentials) }
|
||||
: {}),
|
||||
credentials: restoredCredentials,
|
||||
};
|
||||
setPendingRestoredValues(restoredValues);
|
||||
}
|
||||
|
|
@ -955,6 +961,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
}
|
||||
|
||||
NotificationsManager.success("MCP Server updated successfully");
|
||||
setAppMayNotMatchUpstream(false);
|
||||
onSuccess(updated);
|
||||
} catch (error: any) {
|
||||
NotificationsManager.fromBackend("Failed to update MCP Server" + (error?.message ? `: ${error.message}` : ""));
|
||||
|
|
|
|||
|
|
@ -368,6 +368,8 @@ export interface MCPServer {
|
|||
delegate_auth_to_upstream?: boolean;
|
||||
oauth_passthrough?: boolean;
|
||||
max_concurrent_requests?: number | null;
|
||||
/** Redacted to null in server responses; present when constructing a server locally. */
|
||||
credentials?: Record<string, unknown> | null;
|
||||
|
||||
/** Stdio-only fields (present when transport === 'stdio') */
|
||||
command?: string | null;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue