mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
fix(mcp): persist DCR client_id from on-create MCP OAuth Authorize & Fetch (#31920)
* fix(mcp): persist DCR client_id so interactive OAuth token refresh works
Interactive authorization_code MCP servers register an OAuth client via Dynamic
Client Registration (RFC 7591) during the authorize flow, but the minted
client_id and the discovered token_url were returned to the caller and never
written to the server row. The autonomous refresh_token grant reads client_id,
client_secret and token_url off the server, so an expired access token could not
be refreshed; the user was bounced back to re-authorize and tools/list returned
zero tools
Persist the DCR client_id (plus client_secret and token_endpoint_auth_method when
the registration returns them) and the discovered token_url onto the server row,
reusing the encrypt_credentials write that client_credentials and token exchange
already use, then refresh the in-memory registry so the value is live at refresh
time. Both the v1 refresher and the v2 AuthorizationCodeRefresher read those same
fields, so egress needs no change
* fix: reuse persisted MCP DCR clients
* fix(ui): persist DCR client_id from on-create MCP OAuth "Authorize & Fetch"
The interactive "Authorize & Fetch" flow on the create form registers an OAuth
client (RFC 7591) against a temporary server that has no DB row, then creates the
real server afterward. useMcpOAuthFlow captured the DCR client_id and client_secret
but passed only the token to onTokenReceived, so the create request dropped the
client identity and the created server could not refresh its access token; its row
had credentials={} and the refresh_token grant 401d at the upstream token endpoint
Forward the registered client to onTokenReceived and write client_id (and
client_secret when present) into the create form credentials, so the create request
carries them and the backend persists them through its existing encrypt_credentials
path. token_url is omitted because it is re-discovered on load (RFC 9728 then 8414);
token_endpoint_auth_method is unused because this flow only ever registers as
client_secret_post or none, never client_secret_basic
* fix(ui): prevent stale MCP OAuth credentials
* fix(ui): reset MCP OAuth authorization state
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
(cherry picked from commit 3235f4a499)
This commit is contained in:
parent
fe9fb0b81a
commit
a3c1ece4c4
4 changed files with 386 additions and 22 deletions
|
|
@ -7,6 +7,7 @@ import CreateMCPServer from "./create_mcp_server";
|
|||
|
||||
vi.mock("../networking", () => ({
|
||||
createMCPServer: vi.fn(),
|
||||
fetchOpenAPIRegistry: vi.fn().mockResolvedValue({ apis: [] }),
|
||||
registerMCPServer: vi.fn(),
|
||||
storeMCPOAuthUserCredential: vi.fn().mockResolvedValue({}),
|
||||
testMCPToolsListRequest: vi.fn().mockResolvedValue({ tools: [], error: null }),
|
||||
|
|
@ -16,15 +17,26 @@ vi.mock("@/utils/mcpTokenStore", () => ({
|
|||
setToken: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("./OpenAPIQuickPicker", () => ({
|
||||
default: () => null,
|
||||
}));
|
||||
|
||||
// Mutable holder so individual tests can simulate "Authorize & Fetch" having
|
||||
// produced a token before submit, and inspect the reset wiring.
|
||||
const oauthHook = vi.hoisted(() => ({
|
||||
tokenResponse: null as Record<string, unknown> | null,
|
||||
reset: vi.fn(),
|
||||
onTokenReceived: null as ((token: Record<string, unknown> | null) => void) | null,
|
||||
onTokenReceived: null as
|
||||
| ((token: Record<string, unknown> | null, registeredClient?: { clientId?: string; clientSecret?: string }) => void)
|
||||
| null,
|
||||
}));
|
||||
vi.mock("@/hooks/useMcpOAuthFlow", () => ({
|
||||
useMcpOAuthFlow: (opts: { onTokenReceived: (token: Record<string, unknown> | null) => void }) => {
|
||||
useMcpOAuthFlow: (opts: {
|
||||
onTokenReceived: (
|
||||
token: Record<string, unknown> | null,
|
||||
registeredClient?: { clientId?: string; clientSecret?: string },
|
||||
) => void;
|
||||
}) => {
|
||||
oauthHook.onTokenReceived = opts.onTokenReceived;
|
||||
return {
|
||||
startOAuthFlow: vi.fn(),
|
||||
|
|
@ -495,6 +507,170 @@ describe("CreateMCPServer", () => {
|
|||
expect(payload.token_validation).toEqual({ organization: "my-org", "team.id": "42" });
|
||||
});
|
||||
|
||||
it("invalidates the DCR client and OAuth flow when the MCP URL changes after Authorize & Fetch", async () => {
|
||||
await setupOAuthInteractive();
|
||||
|
||||
const nameInput = document.getElementById("server_name") as HTMLInputElement;
|
||||
await act(async () => {
|
||||
fireEvent.change(nameInput, { target: { value: "Url_Change_Server" } });
|
||||
});
|
||||
const urlInput = screen.getByPlaceholderText("https://your-mcp-server.com");
|
||||
await act(async () => {
|
||||
fireEvent.change(urlInput, { target: { value: "https://a.example.com/mcp" } });
|
||||
});
|
||||
|
||||
act(() => {
|
||||
oauthHook.onTokenReceived?.({ access_token: "tok-a" }, { clientId: "client-a", clientSecret: "secret-a" });
|
||||
});
|
||||
oauthHook.reset.mockClear();
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.change(urlInput, { target: { value: "https://b.example.com/mcp" } });
|
||||
});
|
||||
|
||||
await waitFor(() => expect(oauthHook.reset).toHaveBeenCalled());
|
||||
|
||||
vi.mocked(networking.createMCPServer).mockResolvedValue({
|
||||
server_id: "new-server-oauth",
|
||||
server_name: "Url_Change_Server",
|
||||
alias: "Url_Change_Server",
|
||||
url: "https://b.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",
|
||||
});
|
||||
|
||||
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];
|
||||
expect(payload.credentials?.client_id).toBeUndefined();
|
||||
expect(payload.credentials?.client_secret).toBeUndefined();
|
||||
});
|
||||
|
||||
it("invalidates the DCR client and OAuth flow when the OpenAPI spec URL changes after Authorize & Fetch", async () => {
|
||||
render(<CreateMCPServer {...defaultProps} />);
|
||||
await selectAntOption("Transport Type", "OpenAPI Spec");
|
||||
await waitFor(() => {
|
||||
expect(screen.getByPlaceholderText("https://petstore3.swagger.io/api/v3/openapi.json")).toBeInTheDocument();
|
||||
});
|
||||
await selectAntOption("Authentication", "OAuth");
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("OAuth Flow Type")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
const nameInput = document.getElementById("server_name") as HTMLInputElement;
|
||||
await act(async () => {
|
||||
fireEvent.change(nameInput, { target: { value: "OpenAPI_Server" } });
|
||||
});
|
||||
const specInput = screen.getByPlaceholderText("https://petstore3.swagger.io/api/v3/openapi.json");
|
||||
await act(async () => {
|
||||
fireEvent.change(specInput, { target: { value: "https://a.example.com/openapi.json" } });
|
||||
});
|
||||
|
||||
act(() => {
|
||||
oauthHook.onTokenReceived?.({ access_token: "tok-a" }, { clientId: "client-a", clientSecret: "secret-a" });
|
||||
});
|
||||
oauthHook.reset.mockClear();
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.change(specInput, { target: { value: "https://b.example.com/openapi.json" } });
|
||||
});
|
||||
|
||||
await waitFor(() => expect(oauthHook.reset).toHaveBeenCalled());
|
||||
|
||||
vi.mocked(networking.createMCPServer).mockResolvedValue({
|
||||
server_id: "new-openapi-server",
|
||||
server_name: "OpenAPI_Server",
|
||||
alias: "OpenAPI_Server",
|
||||
url: "https://b.example.com/openapi.json",
|
||||
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",
|
||||
});
|
||||
|
||||
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];
|
||||
expect(payload.spec_path).toBe("https://b.example.com/openapi.json");
|
||||
expect(payload.credentials?.client_id).toBeUndefined();
|
||||
expect(payload.credentials?.client_secret).toBeUndefined();
|
||||
});
|
||||
|
||||
it("invalidates the DCR client and OAuth flow when the transport changes after Authorize & Fetch", async () => {
|
||||
render(<CreateMCPServer {...defaultProps} />);
|
||||
await selectAntOption("Transport Type", "OpenAPI Spec");
|
||||
await waitFor(() => {
|
||||
expect(screen.getByPlaceholderText("https://petstore3.swagger.io/api/v3/openapi.json")).toBeInTheDocument();
|
||||
});
|
||||
await selectAntOption("Authentication", "OAuth");
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("OAuth Flow Type")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
const nameInput = document.getElementById("server_name") as HTMLInputElement;
|
||||
await act(async () => {
|
||||
fireEvent.change(nameInput, { target: { value: "Transport_Change_Server" } });
|
||||
});
|
||||
const specInput = screen.getByPlaceholderText("https://petstore3.swagger.io/api/v3/openapi.json");
|
||||
await act(async () => {
|
||||
fireEvent.change(specInput, { target: { value: "https://same.example.com/spec-or-mcp" } });
|
||||
});
|
||||
|
||||
act(() => {
|
||||
oauthHook.onTokenReceived?.({ access_token: "tok-a" }, { clientId: "client-a", clientSecret: "secret-a" });
|
||||
});
|
||||
oauthHook.reset.mockClear();
|
||||
|
||||
await selectAntOption("Transport Type", "Streamable HTTP");
|
||||
await waitFor(() => {
|
||||
expect(screen.getByPlaceholderText("https://your-mcp-server.com")).toBeInTheDocument();
|
||||
});
|
||||
const urlInput = screen.getByPlaceholderText("https://your-mcp-server.com");
|
||||
await act(async () => {
|
||||
fireEvent.change(urlInput, { target: { value: "https://same.example.com/spec-or-mcp" } });
|
||||
});
|
||||
|
||||
await waitFor(() => expect(oauthHook.reset).toHaveBeenCalled());
|
||||
|
||||
vi.mocked(networking.createMCPServer).mockResolvedValue({
|
||||
server_id: "new-transport-server",
|
||||
server_name: "Transport_Change_Server",
|
||||
alias: "Transport_Change_Server",
|
||||
url: "https://same.example.com/spec-or-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",
|
||||
});
|
||||
|
||||
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];
|
||||
expect(payload.url).toBe("https://same.example.com/spec-or-mcp");
|
||||
expect(payload.credentials?.client_id).toBeUndefined();
|
||||
expect(payload.credentials?.client_secret).toBeUndefined();
|
||||
});
|
||||
|
||||
it("omits token_validation from payload when token_validation_json is empty", async () => {
|
||||
vi.mocked(networking.createMCPServer).mockResolvedValue({
|
||||
server_id: "new-server-oauth",
|
||||
|
|
@ -667,11 +843,13 @@ describe("CreateMCPServer", () => {
|
|||
// Reopen for a brand-new server and enter a different URL without re-authorizing.
|
||||
rerender(<CreateMCPServer {...defaultProps} isModalVisible={true} />);
|
||||
const reopenedUrlInput = screen.getByPlaceholderText("https://your-mcp-server.com");
|
||||
oauthHook.reset.mockClear();
|
||||
await act(async () => {
|
||||
fireEvent.change(reopenedUrlInput, { target: { value: "https://server-b.example.com/mcp" } });
|
||||
});
|
||||
|
||||
// The previous server's token must never be replayed for the new session.
|
||||
expect(oauthHook.reset).not.toHaveBeenCalled();
|
||||
expect(usedToken("stale-token-A")).toBe(false);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -89,6 +89,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
const [oauthAccessToken, setOauthAccessToken] = useState<string | null>(null);
|
||||
const [logoUrl, setLogoUrl] = useState<string | undefined>(undefined);
|
||||
const [oauthDocsUrl, setOauthDocsUrl] = useState<string | null>(null);
|
||||
const [authorizedUrl, setAuthorizedUrl] = useState<string | undefined>(undefined);
|
||||
|
||||
// Single hook call shared by MCPConnectionStatus and MCPToolConfiguration to avoid duplicate requests.
|
||||
const { tools, isLoadingTools, toolsError, toolsErrorStackTrace, canFetchTools, fetchTools, clearTools } =
|
||||
|
|
@ -105,6 +106,12 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
const isAwsSigV4AuthType = authType === AUTH_TYPE.AWS_SIGV4;
|
||||
const isM2MFlow = isOAuthAuthType && formValues.oauth_flow_type === OAUTH_FLOW.M2M;
|
||||
|
||||
const getOAuthAuthorizationTarget = (values: Record<string, unknown>): string | undefined => {
|
||||
const transport = values.transport || transportType;
|
||||
const target = transport === TRANSPORT.OPENAPI ? values.spec_path : values.url;
|
||||
return typeof target === "string" ? target : undefined;
|
||||
};
|
||||
|
||||
const persistCreateUiState = () => {
|
||||
if (typeof window === "undefined") {
|
||||
return;
|
||||
|
|
@ -171,7 +178,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
env: values.env,
|
||||
};
|
||||
},
|
||||
onTokenReceived: (token) => {
|
||||
onTokenReceived: (token, registeredClient) => {
|
||||
setOauthAccessToken(token?.access_token ?? null);
|
||||
|
||||
if (token?.access_token) {
|
||||
|
|
@ -180,9 +187,12 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
...(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 });
|
||||
setAuthorizedUrl(getOAuthAuthorizationTarget(form.getFieldsValue(true)));
|
||||
|
||||
NotificationsManager.success(
|
||||
"OAuth authorization successful! Please click 'Create MCP Server' to save the configuration.",
|
||||
|
|
@ -193,6 +203,15 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
flowSource: "create",
|
||||
});
|
||||
|
||||
const clearAuthorizedOAuthState = (values: Record<string, unknown>) => {
|
||||
form.resetFields(["credentials", "authorization_url", "token_url", "registration_url"]);
|
||||
form.setFieldsValue(values);
|
||||
setOauthAccessToken(null);
|
||||
clearTools();
|
||||
resetOAuthFlow();
|
||||
setAuthorizedUrl(undefined);
|
||||
};
|
||||
|
||||
React.useEffect(() => {
|
||||
if (typeof window === "undefined") {
|
||||
return;
|
||||
|
|
@ -506,12 +525,28 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
const handleTransportChange = (value: string) => {
|
||||
setTransportType(value);
|
||||
// Clear fields that are not relevant for the selected transport
|
||||
if (value === "stdio") {
|
||||
form.setFieldsValue({ url: undefined, spec_path: undefined, auth_type: undefined, credentials: undefined });
|
||||
} else if (value === TRANSPORT.OPENAPI) {
|
||||
form.setFieldsValue({ url: undefined, command: undefined, args: undefined, env: undefined });
|
||||
const transportValues =
|
||||
value === "stdio"
|
||||
? { url: undefined, spec_path: undefined, auth_type: undefined, credentials: undefined }
|
||||
: value === TRANSPORT.OPENAPI
|
||||
? { url: undefined, command: undefined, args: undefined, env: undefined }
|
||||
: { spec_path: undefined, command: undefined, args: undefined, env: undefined };
|
||||
|
||||
const nextValues =
|
||||
authorizedUrl === undefined
|
||||
? transportValues
|
||||
: {
|
||||
...transportValues,
|
||||
credentials: undefined,
|
||||
authorization_url: undefined,
|
||||
token_url: undefined,
|
||||
registration_url: undefined,
|
||||
};
|
||||
|
||||
if (authorizedUrl !== undefined) {
|
||||
clearAuthorizedOAuthState(nextValues);
|
||||
} else {
|
||||
form.setFieldsValue({ spec_path: undefined, command: undefined, args: undefined, env: undefined });
|
||||
form.setFieldsValue(nextValues);
|
||||
}
|
||||
};
|
||||
|
||||
|
|
@ -567,11 +602,32 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
setOauthAccessToken(null);
|
||||
clearTools();
|
||||
resetOAuthFlow();
|
||||
setAuthorizedUrl(undefined);
|
||||
}
|
||||
}, [isModalVisible, form, clearTools, resetOAuthFlow]);
|
||||
|
||||
const isAdmin = isAdminRole(userRole);
|
||||
|
||||
const handleFormValuesChange = (changedValues: Record<string, unknown>, allValues: Record<string, unknown>) => {
|
||||
const changedAuthorizationTarget = "url" in changedValues || "spec_path" in changedValues;
|
||||
if (
|
||||
changedAuthorizationTarget &&
|
||||
authorizedUrl !== undefined &&
|
||||
getOAuthAuthorizationTarget(allValues) !== authorizedUrl
|
||||
) {
|
||||
const invalidated = {
|
||||
credentials: undefined,
|
||||
authorization_url: changedValues.authorization_url,
|
||||
token_url: changedValues.token_url,
|
||||
registration_url: changedValues.registration_url,
|
||||
};
|
||||
clearAuthorizedOAuthState(invalidated);
|
||||
setFormValues({ ...allValues, ...invalidated });
|
||||
return;
|
||||
}
|
||||
setFormValues(allValues);
|
||||
};
|
||||
|
||||
// rendering
|
||||
return (
|
||||
<Modal
|
||||
|
|
@ -616,7 +672,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
<Form
|
||||
form={form}
|
||||
onFinish={handleCreate}
|
||||
onValuesChange={(_, allValues) => setFormValues(allValues)}
|
||||
onValuesChange={handleFormValuesChange}
|
||||
layout="vertical"
|
||||
className="space-y-6"
|
||||
>
|
||||
|
|
@ -736,7 +792,9 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
<OpenAPIFormSection
|
||||
form={form}
|
||||
accessToken={isModalVisible ? accessToken : null}
|
||||
onValuesChange={(updates) => setFormValues((prev) => ({ ...prev, ...updates }))}
|
||||
onValuesChange={(updates) =>
|
||||
handleFormValuesChange(updates, { ...form.getFieldsValue(true), ...updates })
|
||||
}
|
||||
onKeyToolsChange={setKeyTools}
|
||||
onLogoUrlChange={setLogoUrl}
|
||||
onOAuthDocsUrlChange={setOauthDocsUrl}
|
||||
|
|
|
|||
|
|
@ -77,7 +77,7 @@ describe("useMcpOAuthFlow reset", () => {
|
|||
|
||||
await waitFor(() => expect(result.current.status).toBe("success"));
|
||||
expect(result.current.tokenResponse).toEqual(token);
|
||||
expect(onTokenReceived).toHaveBeenCalledWith(token);
|
||||
expect(onTokenReceived).toHaveBeenCalledWith(token, expect.objectContaining({ clientId: "client-1" }));
|
||||
|
||||
act(() => {
|
||||
result.current.reset();
|
||||
|
|
@ -88,6 +88,34 @@ describe("useMcpOAuthFlow reset", () => {
|
|||
expect(result.current.error).toBeNull();
|
||||
});
|
||||
|
||||
it("ignores an in-flight exchange result after reset", async () => {
|
||||
const token = { access_token: "stale-token" };
|
||||
let resolveExchange: (value: typeof token) => void = () => undefined;
|
||||
const exchangePromise = new Promise<typeof token>((resolve) => {
|
||||
resolveExchange = resolve;
|
||||
});
|
||||
vi.mocked(networking.exchangeMcpOAuthToken).mockReturnValueOnce(exchangePromise);
|
||||
seedCompletedRedirect();
|
||||
|
||||
const onTokenReceived = vi.fn();
|
||||
const { result } = renderFlow(onTokenReceived);
|
||||
|
||||
await waitFor(() => expect(result.current.status).toBe("exchanging"));
|
||||
|
||||
act(() => {
|
||||
result.current.reset();
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
resolveExchange(token);
|
||||
await exchangePromise;
|
||||
});
|
||||
|
||||
expect(onTokenReceived).not.toHaveBeenCalled();
|
||||
expect(result.current.status).toBe("idle");
|
||||
expect(result.current.tokenResponse).toBeNull();
|
||||
});
|
||||
|
||||
it("clears the in-flight guard so a callback after a mid-exchange close is not swallowed", async () => {
|
||||
// First exchange hangs, mimicking the modal being closed while the token
|
||||
// endpoint is still in flight. processingRef is left true at that point.
|
||||
|
|
@ -112,6 +140,92 @@ describe("useMcpOAuthFlow reset", () => {
|
|||
const onTokenReceived2 = vi.fn();
|
||||
rerender({ onTokenReceived: onTokenReceived2 });
|
||||
|
||||
await waitFor(() => expect(onTokenReceived2).toHaveBeenCalledWith(token));
|
||||
await waitFor(() =>
|
||||
expect(onTokenReceived2).toHaveBeenCalledWith(token, expect.objectContaining({ clientId: "client-1" })),
|
||||
);
|
||||
});
|
||||
|
||||
it("passes the DCR-registered client_id and client_secret to onTokenReceived so the created server persists them", async () => {
|
||||
const token = { access_token: "tok-xyz", refresh_token: "ref-xyz", expires_in: 3600 };
|
||||
vi.mocked(networking.exchangeMcpOAuthToken).mockResolvedValue(token);
|
||||
setSecureItem(RESULT_KEY, JSON.stringify({ state: "state-1", code: "code-1" }));
|
||||
setSecureItem(
|
||||
FLOW_STATE_KEY,
|
||||
JSON.stringify({
|
||||
state: "state-1",
|
||||
codeVerifier: "verifier-1",
|
||||
serverId: "server-1",
|
||||
clientId: "dcr-client-xyz",
|
||||
clientSecret: "dcr-secret-abc",
|
||||
redirectUri: "https://app.example.com/ui/mcp/oauth/callback",
|
||||
flowSource: "create",
|
||||
}),
|
||||
);
|
||||
|
||||
const onTokenReceived = vi.fn();
|
||||
const { result } = renderFlow(onTokenReceived);
|
||||
|
||||
await waitFor(() => expect(result.current.status).toBe("success"));
|
||||
expect(onTokenReceived).toHaveBeenCalledWith(token, {
|
||||
clientId: "dcr-client-xyz",
|
||||
clientSecret: "dcr-secret-abc",
|
||||
});
|
||||
});
|
||||
|
||||
it("reuses an existing client_id and does not register a new client (second Authorize & Fetch, same server)", async () => {
|
||||
vi.mocked(networking.cacheTemporaryMcpServer).mockResolvedValue({ server_id: "server-1" });
|
||||
vi.mocked(networking.buildMcpOAuthAuthorizeUrl).mockReturnValue("https://idp.example.com/authorize");
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useMcpOAuthFlow({
|
||||
accessToken: "admin-token",
|
||||
getCredentials: () => ({ client_id: "existing-client" }),
|
||||
getTemporaryPayload: () => ({
|
||||
url: "https://server-1.example.com/mcp",
|
||||
transport: "http",
|
||||
credentials: { client_id: "existing-client" },
|
||||
}),
|
||||
onTokenReceived: vi.fn(),
|
||||
flowSource: "create",
|
||||
}),
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.startOAuthFlow();
|
||||
});
|
||||
|
||||
expect(networking.registerMcpOAuthClient).not.toHaveBeenCalled();
|
||||
expect(networking.buildMcpOAuthAuthorizeUrl).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ clientId: "existing-client" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("registers a fresh client when no client_id is present (new URL after the derived client is cleared)", async () => {
|
||||
vi.mocked(networking.cacheTemporaryMcpServer).mockResolvedValue({ server_id: "server-2" });
|
||||
vi.mocked(networking.registerMcpOAuthClient).mockResolvedValue({ client_id: "fresh-client" });
|
||||
vi.mocked(networking.buildMcpOAuthAuthorizeUrl).mockReturnValue("https://idp.example.com/authorize");
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useMcpOAuthFlow({
|
||||
accessToken: "admin-token",
|
||||
getCredentials: () => ({}),
|
||||
getTemporaryPayload: () => ({
|
||||
url: "https://server-2.example.com/mcp",
|
||||
transport: "http",
|
||||
credentials: {},
|
||||
}),
|
||||
onTokenReceived: vi.fn(),
|
||||
flowSource: "create",
|
||||
}),
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.startOAuthFlow();
|
||||
});
|
||||
|
||||
expect(networking.registerMcpOAuthClient).toHaveBeenCalledTimes(1);
|
||||
expect(networking.buildMcpOAuthAuthorizeUrl).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ clientId: "fresh-client" }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -26,7 +26,10 @@ interface UseMcpOAuthFlowOptions {
|
|||
}
|
||||
| undefined;
|
||||
getTemporaryPayload: () => Record<string, any> | null;
|
||||
onTokenReceived: (tokenResponse: Record<string, any>) => void;
|
||||
onTokenReceived: (
|
||||
tokenResponse: Record<string, any>,
|
||||
registeredClient?: { clientId?: string; clientSecret?: string },
|
||||
) => void;
|
||||
onBeforeRedirect?: () => void;
|
||||
// Distinguishes which form started the flow (e.g. "create" vs "edit"). Both forms
|
||||
// mount this hook with shared storage keys, so the return handler only processes a
|
||||
|
|
@ -55,6 +58,7 @@ export const useMcpOAuthFlow = ({
|
|||
const [error, setError] = useState<string | null>(null);
|
||||
const [tokenResponse, setTokenResponse] = useState<Record<string, any> | null>(null);
|
||||
const processingRef = useRef(false);
|
||||
const resetVersionRef = useRef(0);
|
||||
|
||||
const FLOW_STATE_KEY = "litellm-mcp-oauth-flow-state";
|
||||
const RESULT_KEY = "litellm-mcp-oauth-result";
|
||||
|
|
@ -144,9 +148,7 @@ export const useMcpOAuthFlow = ({
|
|||
}
|
||||
|
||||
let registeredClient: { clientId?: string; clientSecret?: string } = {};
|
||||
const hasPreconfiguredCredentials = Boolean(
|
||||
temporaryPayload.credentials?.client_id && temporaryPayload.credentials?.client_secret,
|
||||
);
|
||||
const hasPreconfiguredCredentials = Boolean(temporaryPayload.credentials?.client_id);
|
||||
|
||||
if (!hasPreconfiguredCredentials) {
|
||||
const registration = await registerMcpOAuthClient(accessToken, serverId, {
|
||||
|
|
@ -286,6 +288,8 @@ export const useMcpOAuthFlow = ({
|
|||
}
|
||||
}
|
||||
|
||||
const resetVersion = resetVersionRef.current;
|
||||
|
||||
try {
|
||||
if (!flowState || !flowState.state || !flowState.codeVerifier || !flowState.serverId) {
|
||||
throw new Error(
|
||||
|
|
@ -314,22 +318,31 @@ export const useMcpOAuthFlow = ({
|
|||
accessToken,
|
||||
});
|
||||
|
||||
onTokenReceived(token);
|
||||
if (resetVersion !== resetVersionRef.current) {
|
||||
return;
|
||||
}
|
||||
|
||||
onTokenReceived(token, { clientId: flowState.clientId, clientSecret: flowState.clientSecret });
|
||||
setTokenResponse(token);
|
||||
setStatus("success");
|
||||
setError(null);
|
||||
NotificationsManager.success("OAuth token retrieved successfully");
|
||||
} catch (err) {
|
||||
if (resetVersion !== resetVersionRef.current) {
|
||||
return;
|
||||
}
|
||||
const message = extractErrorMessage(err);
|
||||
setError(message);
|
||||
setStatus("error");
|
||||
NotificationsManager.error(message);
|
||||
} finally {
|
||||
clearStoredFlow();
|
||||
// Reset processing flag after a delay to allow UI updates
|
||||
setTimeout(() => {
|
||||
processingRef.current = false;
|
||||
}, 1000);
|
||||
if (resetVersion === resetVersionRef.current) {
|
||||
clearStoredFlow();
|
||||
// Reset processing flag after a delay to allow UI updates
|
||||
setTimeout(() => {
|
||||
processingRef.current = false;
|
||||
}, 1000);
|
||||
}
|
||||
}
|
||||
}, [onTokenReceived]);
|
||||
|
||||
|
|
@ -338,6 +351,7 @@ export const useMcpOAuthFlow = ({
|
|||
}, [resumeOAuthFlow]);
|
||||
|
||||
const reset = useCallback(() => {
|
||||
resetVersionRef.current += 1;
|
||||
setStatus("idle");
|
||||
setError(null);
|
||||
setTokenResponse(null);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue