diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 97b7bf97a4e..8c4abec2584 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -811,6 +811,9 @@ class LiteLLMRoutes(enum.Enum): "/model/delete", "/user/daily/activity", "/user/daily/activity/aggregated", + # Endpoint restricts results to organizations the caller is ORG_ADMIN + # of; a caller who administers none gets an empty result set. + "/organization/daily/activity", "/user/available_roles", # read-only role metadata; any authenticated user may read "/user/list", # org admins checked in endpoint; non-admins get 403 "/model/{model_id}/update", diff --git a/litellm/proxy/management_endpoints/organization_endpoints.py b/litellm/proxy/management_endpoints/organization_endpoints.py index 3ae871b476e..ffca858c0ce 100644 --- a/litellm/proxy/management_endpoints/organization_endpoints.py +++ b/litellm/proxy/management_endpoints/organization_endpoints.py @@ -559,7 +559,7 @@ async def get_organization_daily_activity( # Fetch organization aliases for metadata where_condition: Final = _STR_OBJECT_DICT_ADAPTER.validate_python({}) - if org_ids_list: + if org_ids_list is not None: where_condition["organization_id"] = {"in": list(org_ids_list)} org_aliases: Final = await _table(OrganizationRepository(prisma_client)).find_many(where=where_condition) diff --git a/tests/test_litellm/proxy/auth/test_route_checks.py b/tests/test_litellm/proxy/auth/test_route_checks.py index 0bfb10320f7..21511c74154 100644 --- a/tests/test_litellm/proxy/auth/test_route_checks.py +++ b/tests/test_litellm/proxy/auth/test_route_checks.py @@ -1,5 +1,6 @@ import os import sys +from datetime import datetime from unittest.mock import MagicMock, patch sys.path.insert( @@ -9,7 +10,14 @@ sys.path.insert( import pytest from fastapi import HTTPException, Request -from litellm.proxy._types import LiteLLM_UserTable, LitellmUserRoles, UserAPIKeyAuth +from litellm.proxy._types import ( + LiteLLM_OrganizationMembershipTable, + LiteLLM_UserTable, + LiteLLMRoutes, + LitellmUserRoles, + UserAPIKeyAuth, +) +from litellm.proxy.auth.auth_checks_organization import _user_is_org_admin from litellm.proxy.auth.route_checks import RouteChecks @@ -3298,3 +3306,76 @@ def test_user_daily_activity_aggregated_not_covered_by_prefix_match(): route="/user/daily/activity/aggregated", allowed_routes=["/user/daily/activity"], ) + + +@pytest.mark.parametrize( + "user_role", + [ + LitellmUserRoles.INTERNAL_USER.value, + LitellmUserRoles.INTERNAL_USER_VIEW_ONLY.value, + ], +) +def test_organization_daily_activity_reachable_by_non_admin_roles(user_role): + """The Organization Usage dashboard calls /organization/daily/activity, whose + handler restricts results to organizations the caller is ORG_ADMIN of (and + 403s on any other org). That scoping is unreachable unless the route layer + lets a non-proxy-admin through first: the route belongs to no info / + management / org_admin_only list, so self_managed_routes is the only entry + granting it, and dropping it 401s every org admin's Organization Usage view + before the handler ever runs. + """ + user_obj = LiteLLM_UserTable( + user_id="test_user", + user_email="test@example.com", + user_role=user_role, + ) + valid_token = UserAPIKeyAuth(user_id="test_user", user_role=user_role) + request = MagicMock(spec=Request) + request.query_params = {} + + RouteChecks.non_proxy_admin_allowed_routes_check( + user_obj=user_obj, + _user_role=user_role, + route="/organization/daily/activity", + request=request, + valid_token=valid_token, + request_data={}, + ) + + +def test_organization_daily_activity_not_granted_by_org_admin_request_data_branch(): + """The org-admin branch of the route gate cannot grant this route, so the + self_managed_routes entry is load-bearing rather than redundant. + + Query params do reach request_data, so the reason is not body-vs-query: it + is the key name. _user_is_org_admin reads ``organization_id`` (singular) and + ``organizations``, while this endpoint's filter is ``organization_ids`` + (plural), and the dashboard's first page load sends no organization filter + at all. Both shapes are pinned below because renaming the query param would + otherwise silently change which gate is doing the work. + """ + user_obj = LiteLLM_UserTable( + user_id="test_user", + user_email="test@example.com", + user_role=LitellmUserRoles.INTERNAL_USER.value, + organization_memberships=[ + LiteLLM_OrganizationMembershipTable( + user_id="test_user", + organization_id="org-a", + user_role=LitellmUserRoles.ORG_ADMIN.value, + created_at=datetime.now(), + updated_at=datetime.now(), + ) + ], + ) + + # The dashboard's default page load: no organization filter at all. + assert not _user_is_org_admin(request_data={}, user_object=user_obj) + # The filtered load, naming an org this user really does administer. + assert not _user_is_org_admin(request_data={"organization_ids": "org-a"}, user_object=user_obj) + # The key name the helper would have had to see to grant it. + assert _user_is_org_admin(request_data={"organization_id": "org-a"}, user_object=user_obj) + assert not RouteChecks.check_route_access( + route="/organization/daily/activity", + allowed_routes=LiteLLMRoutes.org_admin_only_routes.value, + ) diff --git a/tests/test_litellm/proxy/management_endpoints/test_organization_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_organization_endpoints.py index 7ed123f6cdf..3061da336f6 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_organization_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_organization_endpoints.py @@ -992,3 +992,51 @@ def test_build_budget_write_data_clears_reset_at_with_null_duration(): data = build_budget_write_data({"budget_duration": None}, "admin-1") assert data["budget_duration"] is None assert data["budget_reset_at"] is None + + +@pytest.mark.asyncio +async def test_get_organization_daily_activity_non_admin_without_org_admin_role_sees_nothing( + monkeypatch, +): + """A caller who is ORG_ADMIN of no organization must resolve to an EMPTY id + list, never to None. None means "no entity filter" downstream, i.e. every + organization's spend, so the natural simplification of falling back to None + on an empty membership set turns a scoping rule into a proxy-wide leak. The + organization-alias lookup must be scoped by that same empty list rather than + reading the whole table. + """ + from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth + from litellm.proxy.management_endpoints import organization_endpoints + from litellm.proxy.management_endpoints.organization_endpoints import ( + get_organization_daily_activity, + ) + + mock_prisma_client = AsyncMock() + org_table_find_many = AsyncMock(return_value=[]) + mock_prisma_client.db.litellm_organizationtable.find_many = org_table_find_many + mock_prisma_client.db.litellm_organizationmembership.find_many = AsyncMock(return_value=[]) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma_client) + + monkeypatch.setattr( + "litellm.proxy.management_endpoints.organization_endpoints._user_has_admin_view", + lambda _: False, + ) + + get_daily_activity_mock = AsyncMock(return_value=MagicMock(name="SpendAnalyticsPaginatedResponse")) + monkeypatch.setattr(organization_endpoints, "get_daily_activity", get_daily_activity_mock) + + auth = UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER, user_id="no-orgs-user") + await get_organization_daily_activity( + organization_ids=None, + start_date="2024-04-01", + end_date="2024-04-30", + model=None, + api_key=None, + page=1, + page_size=10, + exclude_organization_ids=None, + user_api_key_dict=auth, + ) + + assert get_daily_activity_mock.call_args.kwargs["entity_id"] == [] + assert org_table_find_many.call_args.kwargs["where"] == {"organization_id": {"in": []}} diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.test.tsx index 7055e36b194..666172947d1 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.test.tsx @@ -939,6 +939,26 @@ describe("EntityUsage", () => { expect(call()).not.toHaveBeenCalled(); }); + // An org admin's session role is "Internal User", so the row above cannot + // distinguish them. Gating the fetch on the session role alone left the + // Organization Usage panel rendered but permanently empty, because the + // request was never issued even though the proxy would have served it. + it.each([ + ["organization", () => mockOrganizationDailyActivityCall, true], + ["agent", () => mockAgentDailyActivityCall, false], + ] as const)("fetches %s activity for an org admin: %s", async (entityType, call, expected) => { + render(); + + if (expected) { + await waitFor(() => { + expect(call()).toHaveBeenCalled(); + }); + } else { + expect(await screen.findByText("Agent Spend Overview")).toBeInTheDocument(); + expect(call()).not.toHaveBeenCalled(); + } + }); + it("keeps the team breakdown but drops its agent sub-fetch for an internal user", async () => { render(); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.tsx b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.tsx index 7343dcaaec5..87c6dd2a369 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.tsx @@ -86,6 +86,7 @@ interface EntityUsageProps { entityList: EntityList[] | null; premiumUser: boolean; dateValue: DateRangePickerValue; + isOrgAdmin?: boolean; } const ENTITY_FETCH_FNS: Record Promise> = { @@ -115,6 +116,7 @@ const EntityUsage: React.FC = ({ entityList, userRole, dateValue, + isOrgAdmin = false, }) => { const { teams } = useTeams(); const [selectedTags, setSelectedTags] = useState([]); @@ -135,7 +137,7 @@ const EntityUsage: React.FC = ({ const fetchFn = ENTITY_FETCH_FNS[entityType]; const aggregatedFetchFn = ENTITY_AGGREGATED_FETCH_FNS[entityType]; const entityCapability = ENTITY_CAPABILITIES[entityType]; - const canViewEntity = entityCapability === undefined || hasCapability(userRole, entityCapability); + const canViewEntity = entityCapability === undefined || hasCapability(userRole, entityCapability, isOrgAdmin); const showAgentBreakdown = entityType === "team" && hasCapability(userRole, "viewAgentUsage"); const hasRequestWindow = !!accessToken && !!startTime && !!endTime; const enabled = hasRequestWindow && canViewEntity; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.test.tsx index f560d3c2c84..a6f46ee8242 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.test.tsx @@ -1,6 +1,7 @@ import { useAgents } from "@/app/(dashboard)/hooks/agents/useAgents"; import { useCustomers } from "@/app/(dashboard)/hooks/customers/useCustomers"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; +import useIsOrgAdmin from "@/app/(dashboard)/hooks/useIsOrgAdmin"; import { useCurrentUser } from "@/app/(dashboard)/hooks/users/useCurrentUser"; import { useInfiniteUsers } from "@/app/(dashboard)/hooks/users/useUsers"; import { act, fireEvent, screen, waitFor } from "@testing-library/react"; @@ -141,6 +142,11 @@ vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ default: vi.fn(), })); +vi.mock("@/app/(dashboard)/hooks/useIsOrgAdmin", () => ({ + __esModule: true, + default: vi.fn(() => false), +})); + vi.mock("@/app/(dashboard)/hooks/users/useCurrentUser", () => ({ useCurrentUser: vi.fn(), })); @@ -621,6 +627,49 @@ describe("UsagePage", () => { }); }); + // Org-admin membership comes from the server, so it can be revoked while the + // page is open. The Organization Usage option and its panel both disappear, + // and without a fallback the selector keeps a value it no longer offers, + // leaving the user on a blank trigger over a blank panel with nothing to + // click. An internal user is used because that is the session role an org + // admin actually carries. + it("should leave the organization view when org-admin membership is revoked mid-session", async () => { + const mockUseIsOrgAdmin = vi.mocked(useIsOrgAdmin); + mockUseIsOrgAdmin.mockReturnValue(true); + mockUseAuthorized.mockReturnValue({ + isLoading: false, + isAuthorized: true, + token: "mock-token", + accessToken: "test-token", + userId: "user-123", + userEmail: "test@example.com", + userRole: "Internal User", + premiumUser: true, + disabledPersonalKeyCreation: false, + showSSOBanner: false, + } as any); + + const { rerender } = renderWithProviders(); + + const usageSelect = screen.getByTestId("usage-view-select"); + act(() => { + fireEvent.change(usageSelect, { target: { value: "organization" } }); + }); + await waitFor(() => { + expect(screen.getAllByText("Entity Usage").length).toBeGreaterThan(0); + }); + expect((usageSelect as HTMLSelectElement).value).toBe("organization"); + + mockUseIsOrgAdmin.mockReturnValue(false); + act(() => { + rerender(); + }); + + await waitFor(() => { + expect((screen.getByTestId("usage-view-select") as HTMLSelectElement).value).toBe("global"); + }); + }); + it("should show customer usage view for admins", async () => { mockUseCustomers.mockReturnValue({ data: mockCustomers, diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.tsx b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.tsx index 9558b9a7f76..311b10e9ac0 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.tsx @@ -20,6 +20,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip import { useAgents } from "@/app/(dashboard)/hooks/agents/useAgents"; import { useCustomers } from "@/app/(dashboard)/hooks/customers/useCustomers"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; +import useIsOrgAdmin from "@/app/(dashboard)/hooks/useIsOrgAdmin"; import { useCurrentUser } from "@/app/(dashboard)/hooks/users/useCurrentUser"; import { hasCapability } from "@/utils/capabilities"; import { formatNumberWithCommas } from "@/utils/dataUtils"; @@ -100,7 +101,8 @@ const UsagePage: React.FC = ({ teams, organizations }) => { const { data: currentUser } = useCurrentUser(); const isAdmin = all_admin_roles.includes(userRole || ""); const canViewTagUsage = isAdmin || internalUserRoles.includes(userRole || ""); - const canViewOrganizationUsage = hasCapability(userRole, "viewOrganizationUsage"); + const isOrgAdmin = useIsOrgAdmin(); + const canViewOrganizationUsage = hasCapability(userRole, "viewOrganizationUsage", isOrgAdmin); const canViewAgentUsage = hasCapability(userRole, "viewAgentUsage"); // For admins: null means global view (all users), a string means filter by that user @@ -110,7 +112,14 @@ const UsagePage: React.FC = ({ teams, organizations }) => { const [isCloudZeroModalOpen, setIsCloudZeroModalOpen] = useState(false); const [isGlobalExportModalOpen, setIsGlobalExportModalOpen] = useState(false); const [isAiChatOpen, setIsAiChatOpen] = useState(false); - const [usageView, setUsageView] = useState("global"); + const [selectedUsageView, setUsageView] = useState("global"); + // Org-admin membership is read from the server, so unlike the other usage + // views this one can be revoked while the page is open. Derive the view in + // render rather than storing it, so the fallback lands on the same paint and + // the selector never holds a value it no longer offers. + const usageView: UsageOption = + selectedUsageView === "organization" && !canViewOrganizationUsage ? "global" : selectedUsageView; + const [showCredentialBanner, setShowCredentialBanner] = useState(true); const [topKeysLimit, setTopKeysLimit] = useState(5); const [topModelsLimit, setTopModelsLimit] = useState(5); @@ -460,6 +469,7 @@ const UsagePage: React.FC = ({ teams, organizations }) => { onChange={(value) => setUsageView(value)} userRole={userRole} canViewTagUsage={canViewTagUsage} + isOrgAdmin={isOrgAdmin} /> @@ -899,6 +909,7 @@ const UsagePage: React.FC = ({ teams, organizations }) => { entityType="organization" userID={userID} userRole={userRole} + isOrgAdmin={isOrgAdmin} dateValue={dateValue} entityList={ organizations?.map((organization) => ({ diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsageViewSelect/UsageViewSelect.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsageViewSelect/UsageViewSelect.test.tsx index 80005a20d05..3d835c98f52 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsageViewSelect/UsageViewSelect.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsageViewSelect/UsageViewSelect.test.tsx @@ -79,6 +79,24 @@ describe("UsageViewSelect", () => { expect(offers(container, optionName)).toBe(false); }); + // An org admin's session role is "Internal User" — org-admin-ness lives in the + // membership table — so the two rows above cannot tell them apart from a plain + // internal user. Organization Usage must open for them, and only that option: + // the proxy serves them /organization/daily/activity scoped to the orgs they + // administer, but still refuses the agent usage route. + it.each([ + ["Organization Usage", true], + ["Agent Usage (A2A)", false], + ] as const)("should offer %s to an org admin: %s", async (optionName, expected) => { + const user = userEvent.setup(); + const { container } = render( + , + ); + + await openMenu(user); + expect(offers(container, optionName)).toBe(expected); + }); + it.each(["Team Usage", "Tag Usage"])("should keep %s available to an internal user", async (optionName) => { const user = userEvent.setup(); const { container } = render( diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsageViewSelect/UsageViewSelect.tsx b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsageViewSelect/UsageViewSelect.tsx index 3fa58fc9da8..cfdbf16a57d 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsageViewSelect/UsageViewSelect.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsageViewSelect/UsageViewSelect.tsx @@ -19,6 +19,7 @@ export interface UsageViewSelectProps { onChange: (value: UsageOption) => void; userRole: string | null; canViewTagUsage?: boolean; + isOrgAdmin?: boolean; title?: string; description?: string; "data-id"?: string; @@ -108,6 +109,7 @@ export const UsageViewSelect: React.FC = ({ onChange, userRole, canViewTagUsage = false, + isOrgAdmin = false, title = "Usage View", description = "Select the usage data you want to view", "data-id": dataId, @@ -116,7 +118,7 @@ export const UsageViewSelect: React.FC = ({ const getFilteredOptions = () => { return OPTIONS.filter((option) => { if (option.capability) { - return hasCapability(userRole, option.capability); + return hasCapability(userRole, option.capability, isOrgAdmin); } if (option.value === "tag" && canViewTagUsage) { return true; diff --git a/ui/litellm-dashboard/src/utils/capabilities.test.ts b/ui/litellm-dashboard/src/utils/capabilities.test.ts index 3f5d4b4efbe..25314a3f46a 100644 --- a/ui/litellm-dashboard/src/utils/capabilities.test.ts +++ b/ui/litellm-dashboard/src/utils/capabilities.test.ts @@ -49,6 +49,7 @@ const SESSION_ROLE_AN_ORG_ADMIN_ACTUALLY_CARRIES = "Internal User"; const ORG_ADMIN_BACKEND_ACCESS: ReadonlyArray = [ ["viewDeletedTeams", "GET /v2/team/list?status=deleted -> 200 (scoped to their orgs)", true], + ["viewOrganizationUsage", "GET /organization/daily/activity -> 200 (scoped to orgs they administer)", true], ["viewToolPolicies", "GET /v1/tool/list -> 401", false], ["viewPolicies", "GET /policies/list -> 401", false], ["viewPrompts", "GET /prompts/list -> 401", false], @@ -64,6 +65,14 @@ describe("hasCapability for organization admins", () => { expect(hasCapability(role, "viewDeletedTeams", true)).toBe(true); }); + // An org admin is an internal_user whose org-admin-ness lives in the + // membership table, so their session role never distinguishes them. Gating + // the Organization Usage view on the session role alone hid the whole view + // from them and left the tab's data fetch disabled. + it.each(NON_ADMIN_ROLES)("grants viewOrganizationUsage to an org admin whose session role is %s", (role) => { + expect(hasCapability(role, "viewOrganizationUsage", true)).toBe(true); + }); + it.each(ADMIN_ONLY_CAPABILITIES)("leaves %s denied when the caller is not an org admin", (capability) => { expect(hasCapability(SESSION_ROLE_AN_ORG_ADMIN_ACTUALLY_CARRIES, capability, false)).toBe(false); expect(hasCapability(SESSION_ROLE_AN_ORG_ADMIN_ACTUALLY_CARRIES, capability)).toBe(false); @@ -73,7 +82,7 @@ describe("hasCapability for organization admins", () => { const orgAdminCapabilities = ADMIN_ONLY_CAPABILITIES.filter((capability) => hasCapability(SESSION_ROLE_AN_ORG_ADMIN_ACTUALLY_CARRIES, capability, true), ); - expect(orgAdminCapabilities).toEqual(["viewDeletedTeams"]); + expect(orgAdminCapabilities).toEqual(["viewDeletedTeams", "viewOrganizationUsage"]); }); it("does not let the org-admin allowance reopen the proxy-admin-only viewGlobalSpend gate", () => { diff --git a/ui/litellm-dashboard/src/utils/capabilities.ts b/ui/litellm-dashboard/src/utils/capabilities.ts index dabbc827bcb..ec41531f2ac 100644 --- a/ui/litellm-dashboard/src/utils/capabilities.ts +++ b/ui/litellm-dashboard/src/utils/capabilities.ts @@ -19,7 +19,10 @@ const CAPABILITY_ROLES = { export type Capability = keyof typeof CAPABILITY_ROLES; -const ORG_ADMIN_CAPABILITIES: ReadonlySet = new Set(["viewDeletedTeams"]); +const ORG_ADMIN_CAPABILITIES: ReadonlySet = new Set([ + "viewDeletedTeams", + "viewOrganizationUsage", +]); export const hasCapability = ( userRole: string | null | undefined,