mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
feat(mcp): persist admin-entered OAuth app credentials for the client-forwarded modes
This commit is contained in:
parent
bf02a4a47f
commit
931b617a51
5 changed files with 138 additions and 18 deletions
|
|
@ -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 (
|
||||
<div className="rounded-lg border border-dashed border-gray-300 p-4 space-y-2 mb-4">
|
||||
<p className="text-sm text-gray-600">
|
||||
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.
|
||||
</p>
|
||||
<Form.Item
|
||||
label={<span className="text-sm font-medium text-gray-700">OAuth Client ID (optional, not saved)</span>}
|
||||
label={<span className="text-sm font-medium text-gray-700">OAuth Client ID (optional, saved)</span>}
|
||||
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)."
|
||||
>
|
||||
<Input.Password
|
||||
placeholder="Leave blank to use dynamic client registration"
|
||||
|
|
@ -50,7 +52,7 @@ export default function PassthroughAuthorizeSection({
|
|||
/>
|
||||
</Form.Item>
|
||||
<Form.Item
|
||||
label={<span className="text-sm font-medium text-gray-700">OAuth Client Secret (optional, not saved)</span>}
|
||||
label={<span className="text-sm font-medium text-gray-700">OAuth Client Secret (optional, saved)</span>}
|
||||
name={["credentials", "client_secret"]}
|
||||
>
|
||||
<Input.Password
|
||||
|
|
@ -67,7 +69,8 @@ export default function PassthroughAuthorizeSection({
|
|||
{oauthFlow.error && <p className="text-sm text-red-500">{oauthFlow.error}</p>}
|
||||
{oauthFlow.status === "success" && oauthFlow.tokenResponse?.access_token && (
|
||||
<p className="text-sm text-green-600">
|
||||
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.
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
||||
|
|
|
|||
|
|
@ -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<CreateMCPServerProps> = ({
|
|||
// 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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
<MCPServerEdit
|
||||
mcpServer={{ ...interactiveOAuthServer, auth_type: authType }}
|
||||
accessToken="access-token"
|
||||
userID="user-1"
|
||||
onCancel={vi.fn()}
|
||||
onSuccess={vi.fn()}
|
||||
availableAccessGroups={[]}
|
||||
/>,
|
||||
);
|
||||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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<MCPServerEditProps> = ({
|
|||
};
|
||||
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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue