mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-21 00:21:49 +00:00
fix(ui): hide MCP disconnect and revoke controls from view-only admin sessions
The auth hook normalizes proxy_admin_viewer to Admin for page access, so the role check alone let a view-only admin see Disconnect and Revoke buttons that the backend refuses with 403. Thread isViewOnly from useAuthorized into the MCP servers page and gate both mutation controls on it Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
ea37596b88
commit
aea33b4b50
6 changed files with 110 additions and 17 deletions
|
|
@ -1,7 +1,9 @@
|
|||
import { render, screen } from "@testing-library/react";
|
||||
import { render, screen, within } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { MCPServerView } from "./mcp_server_view";
|
||||
import * as networking from "@/components/networking";
|
||||
import type { MCPServer } from "@/components/mcp_tools/types";
|
||||
|
||||
vi.mock(".", () => ({
|
||||
|
|
@ -13,6 +15,12 @@ vi.mock("./mcp_server_edit", () => ({
|
|||
EDIT_OAUTH_UI_STATE_KEY: "litellm-mcp-oauth-edit-state",
|
||||
}));
|
||||
|
||||
vi.mock("@/components/networking", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("@/components/networking")>()),
|
||||
fetchMCPServerUserCredentials: vi.fn(),
|
||||
revokeMCPServerUserCredential: vi.fn(),
|
||||
}));
|
||||
|
||||
const baseServer = {
|
||||
server_id: "srv-1",
|
||||
server_name: "demo server",
|
||||
|
|
@ -25,19 +33,38 @@ const baseServer = {
|
|||
|
||||
const renderView = (overrides: Partial<MCPServer> = {}, props: Record<string, unknown> = {}) =>
|
||||
render(
|
||||
<MCPServerView
|
||||
mcpServer={{ ...baseServer, ...overrides } as MCPServer}
|
||||
onBack={vi.fn()}
|
||||
isProxyAdmin
|
||||
isEditing={false}
|
||||
accessToken="tok"
|
||||
userRole="Admin"
|
||||
userID="u1"
|
||||
availableAccessGroups={[]}
|
||||
{...props}
|
||||
/>,
|
||||
<QueryClientProvider client={new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } })}>
|
||||
<MCPServerView
|
||||
mcpServer={{ ...baseServer, ...overrides } as MCPServer}
|
||||
onBack={vi.fn()}
|
||||
isProxyAdmin
|
||||
isEditing={false}
|
||||
accessToken="tok"
|
||||
userRole="Admin"
|
||||
userID="u1"
|
||||
availableAccessGroups={[]}
|
||||
{...props}
|
||||
/>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
|
||||
const openUserCredentials = async (props: Record<string, unknown>) => {
|
||||
vi.mocked(networking.fetchMCPServerUserCredentials).mockResolvedValue([
|
||||
{
|
||||
user_id: "alice",
|
||||
credential_type: "byok",
|
||||
expires_at: null,
|
||||
connected_at: null,
|
||||
updated_at: "2026-01-01T00:00:00+00:00",
|
||||
},
|
||||
]);
|
||||
renderView({}, props);
|
||||
await userEvent.click(screen.getByRole("tab", { name: "User Credentials" }));
|
||||
return within(await screen.findByRole("region", { name: "Stored user credentials" })).getByRole("row", {
|
||||
name: /alice/,
|
||||
});
|
||||
};
|
||||
|
||||
describe("MCPServerView", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
|
@ -149,4 +176,15 @@ describe("MCPServerView", () => {
|
|||
|
||||
expect(await screen.findByText("All tools enabled")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("lets a full admin revoke a stored user credential", async () => {
|
||||
const row = await openUserCredentials({});
|
||||
expect(within(row).getByRole("button", { name: "Revoke credential for user alice" })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows stored credentials to a view-only admin session without a revoke control", async () => {
|
||||
const row = await openUserCredentials({ isViewOnly: true });
|
||||
expect(row).toHaveTextContent("BYOK API key");
|
||||
expect(within(row).queryByRole("button", { name: /^Revoke credential/ })).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ interface MCPServerViewProps {
|
|||
accessToken: string | null;
|
||||
userRole: string | null;
|
||||
userID: string | null;
|
||||
isViewOnly?: boolean;
|
||||
availableAccessGroups: string[];
|
||||
initialTabIndex?: number;
|
||||
}
|
||||
|
|
@ -55,6 +56,7 @@ export const MCPServerView: React.FC<MCPServerViewProps> = ({
|
|||
accessToken,
|
||||
userRole,
|
||||
userID,
|
||||
isViewOnly = false,
|
||||
availableAccessGroups,
|
||||
initialTabIndex = 0,
|
||||
}) => {
|
||||
|
|
@ -66,7 +68,7 @@ export const MCPServerView: React.FC<MCPServerViewProps> = ({
|
|||
const [copiedStates, setCopiedStates] = useState<Record<string, boolean>>({});
|
||||
const [selectedTabIndex, setSelectedTabIndex] = useState(returningFromEditOAuth ? 2 : initialTabIndex);
|
||||
const canViewUserCredentials = userRole !== null && isProxyAdminTierRole(userRole);
|
||||
const canRevokeUserCredentials = userRole !== null && isProxyAdminRole(userRole);
|
||||
const canRevokeUserCredentials = userRole !== null && isProxyAdminRole(userRole) && !isViewOnly;
|
||||
|
||||
const handleSuccess = (updated: MCPServer) => {
|
||||
setEditing(false);
|
||||
|
|
|
|||
|
|
@ -17,6 +17,8 @@ vi.mock("@/components/networking", () => ({
|
|||
updateConfigFieldSetting: vi.fn().mockResolvedValue(undefined),
|
||||
deleteConfigFieldSetting: vi.fn().mockResolvedValue(undefined),
|
||||
listMCPUserEnvVarStatus: vi.fn().mockResolvedValue([]),
|
||||
fetchMCPGatewaySessions: vi.fn(),
|
||||
terminateMCPGatewaySessions: vi.fn(),
|
||||
}));
|
||||
|
||||
const createQueryClient = () =>
|
||||
|
|
@ -400,4 +402,50 @@ describe("MCPServers", () => {
|
|||
// The server list refresh must NOT trigger a second health check
|
||||
expect(networking.fetchMCPServerHealth).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
const liveSessionsReport = {
|
||||
worker_pid: 4242,
|
||||
total_sessions: 1,
|
||||
by_client: [{ label: "claude-code", count: 1 }],
|
||||
by_user: [{ label: "alice", count: 1 }],
|
||||
sessions: [
|
||||
{
|
||||
session_id_prefix: "aaaa1111",
|
||||
client_name: "claude-code",
|
||||
client_version: "1.0.0",
|
||||
user_id: "alice",
|
||||
user_email: "alice@example.com",
|
||||
key_alias: "alice-key",
|
||||
team_id: null,
|
||||
team_alias: null,
|
||||
client_ip: "10.0.0.1",
|
||||
idle_seconds: 5,
|
||||
in_flight_requests: 0,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const openLiveConnections = async (props: { isViewOnly?: boolean }) => {
|
||||
vi.mocked(networking.fetchMCPServers).mockResolvedValue([]);
|
||||
vi.mocked(networking.fetchMCPGatewaySessions).mockResolvedValue(liveSessionsReport);
|
||||
render(
|
||||
<QueryClientProvider client={createQueryClient()}>
|
||||
<MCPServers {...defaultProps} {...props} />
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
await userEvent.click(await screen.findByRole("tab", { name: "Live Connections" }));
|
||||
return within(await screen.findByRole("region", { name: "Live sessions" })).getByRole("row", { name: /aaaa1111/ });
|
||||
};
|
||||
|
||||
it("lets a full admin disconnect a live session", async () => {
|
||||
const row = await openLiveConnections({ isViewOnly: false });
|
||||
expect(within(row).getByRole("button", { name: "Disconnect session aaaa1111" })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows live sessions to a view-only admin session without any disconnect control", async () => {
|
||||
const row = await openLiveConnections({ isViewOnly: true });
|
||||
expect(row).toHaveTextContent("alice@example.com");
|
||||
expect(within(row).queryByRole("button", { name: /^Disconnect/ })).not.toBeInTheDocument();
|
||||
expect(screen.queryByRole("button", { name: /^Disconnect all/ })).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -109,7 +109,7 @@ const readToolsOAuthServerId = (): string | null => {
|
|||
}
|
||||
};
|
||||
|
||||
const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID }) => {
|
||||
const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, isViewOnly = false }) => {
|
||||
const { data: mcpServers, isLoading: isLoadingServers, refetch } = useMCPServers();
|
||||
|
||||
// Fetch health status for all servers
|
||||
|
|
@ -578,6 +578,7 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID })
|
|||
accessToken={accessToken}
|
||||
userID={userID}
|
||||
userRole={userRole}
|
||||
isViewOnly={isViewOnly}
|
||||
availableAccessGroups={uniqueMcpAccessGroups}
|
||||
initialTabIndex={selectedServerId === toolsTabServerId ? 1 : 0}
|
||||
/>
|
||||
|
|
@ -755,7 +756,10 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID })
|
|||
)}
|
||||
{isProxyAdminTierRole(userRole) && (
|
||||
<TabsContent value="connections">
|
||||
<MCPGatewaySessionsTab accessToken={accessToken} canTerminate={isProxyAdminRole(userRole)} />
|
||||
<MCPGatewaySessionsTab
|
||||
accessToken={accessToken}
|
||||
canTerminate={isProxyAdminRole(userRole) && !isViewOnly}
|
||||
/>
|
||||
</TabsContent>
|
||||
)}
|
||||
</Tabs>
|
||||
|
|
|
|||
|
|
@ -4,6 +4,6 @@ import { MCPServers } from "./_components";
|
|||
import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized";
|
||||
|
||||
export default function McpServers() {
|
||||
const { accessToken, userRole, userId } = useAuthorized();
|
||||
return <MCPServers accessToken={accessToken} userRole={userRole} userID={userId} />;
|
||||
const { accessToken, userRole, userId, isViewOnly } = useAuthorized();
|
||||
return <MCPServers accessToken={accessToken} userRole={userRole} userID={userId} isViewOnly={isViewOnly} />;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -517,6 +517,7 @@ export interface MCPServerProps {
|
|||
accessToken: string | null;
|
||||
userRole: string | null;
|
||||
userID: string | null;
|
||||
isViewOnly?: boolean;
|
||||
}
|
||||
|
||||
export interface MCPToolsetTool {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue