From f998fb05cbc657e4246badec6233ba9198df2c9e Mon Sep 17 00:00:00 2001 From: Jesus Velarde Date: Sat, 31 Oct 2026 18:21:07 -0700 Subject: [PATCH] fix(proxy): let org admins read their org members in /v2/user/info Org admins could list a user through /user/list but got a 404 from /v2/user/info for that same user, so the dashboard fell back to a substring /user/list lookup that could miss the user. /v2/user/info now allows an org admin to read users in organizations they administer, and the dashboard's single-user lookup calls only that endpoint. Expected 4xx responses from /v2/user/info no longer log a traceback. --- .../internal_user_endpoints.py | 31 ++++- .../test_internal_user_endpoints.py | 119 ++++++++++++++++++ .../(dashboard)/hooks/users/useUsers.test.ts | 40 +----- .../app/(dashboard)/hooks/users/useUsers.ts | 38 +----- 4 files changed, 159 insertions(+), 69 deletions(-) diff --git a/litellm/proxy/management_endpoints/internal_user_endpoints.py b/litellm/proxy/management_endpoints/internal_user_endpoints.py index 54a2d4062d2..95f30279bc8 100644 --- a/litellm/proxy/management_endpoints/internal_user_endpoints.py +++ b/litellm/proxy/management_endpoints/internal_user_endpoints.py @@ -47,7 +47,7 @@ from litellm.proxy.auth.password_policy import ( ) from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.common_utils.auth_cache_invalidation_pubsub import evict_and_broadcast -from litellm.proxy.common_utils.openai_error_payload import proxy_exception_for +from litellm.proxy.common_utils.openai_error_payload import openai_shaped_proxy_exception, proxy_exception_for from litellm.proxy.common_utils.user_api_key_cache import ( AUTH_OBJECTS_TARGET, object_permission_cache_key, @@ -1048,6 +1048,7 @@ async def _check_user_info_v2_access( 1. Proxy admins / proxy admin viewers can access any user 2. User can access their own info 3. Team admins can access info of users in their teams + 4. Org admins can access info of users in their organizations Raises on unexpected DB errors so they surface as 500s, not silent 404s. """ @@ -1090,9 +1091,35 @@ async def _check_user_info_v2_access( if team.team_id in (target_user.teams or []): return target_user + if user_api_key_dict.user_id is not None and await _target_in_caller_admin_org( + prisma_client=prisma_client, + caller_user_id=user_api_key_dict.user_id, + target_user_id=target_user_id, + ): + return await _fetch_target_user() + return None +async def _target_in_caller_admin_org( + prisma_client: "PrismaClient", + caller_user_id: str, + target_user_id: str, +) -> bool: + admin_memberships: Final = await prisma_client.db.litellm_organizationmembership.find_many( + where={"user_id": caller_user_id, "user_role": LitellmUserRoles.ORG_ADMIN.value} + ) + if not admin_memberships: + return False + shared_membership: Final = await prisma_client.db.litellm_organizationmembership.find_first( + where={ + "user_id": target_user_id, + "organization_id": {"in": [membership.organization_id for membership in admin_memberships]}, + } + ) + return shared_membership is not None + + @router.get( "/v2/user/info", tags=["Internal User management"], @@ -1197,6 +1224,8 @@ async def user_info_v2( ), ) except Exception as e: + if isinstance(e, HTTPException) and e.status_code < 500: + raise openai_shaped_proxy_exception(e, str(e.detail), e.status_code) verbose_proxy_logger.exception("litellm.proxy.proxy_server.user_info_v2(): Exception occured - %s", e) raise handle_exception_on_proxy(e) diff --git a/tests/unit/proxy/management_endpoints/test_internal_user_endpoints.py b/tests/unit/proxy/management_endpoints/test_internal_user_endpoints.py index 39586a1a204..6d6b6f8d4b4 100644 --- a/tests/unit/proxy/management_endpoints/test_internal_user_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_internal_user_endpoints.py @@ -3352,6 +3352,8 @@ async def test_user_info_v2_internal_user_cannot_query_other(mocker): mock_prisma_client.db.litellm_usertable.find_unique = mocker.AsyncMock(side_effect=mock_find_unique) + mock_prisma_client.db.litellm_organizationmembership.find_many = mocker.AsyncMock(return_value=[]) + mocker.patch("litellm.proxy.proxy_server.prisma_client", mock_prisma_client) mock_request = mocker.MagicMock(spec=Request) @@ -3456,6 +3458,40 @@ async def test_user_info_v2_nonexistent_user_returns_404(mocker): assert "nonexistent-user-id" in str(exc_info.value.message) +@pytest.mark.parametrize( + ("find_unique", "expected_code", "logs_traceback"), + [ + (AsyncMock(return_value=None), "404", False), + (AsyncMock(side_effect=RuntimeError("db down")), "500", True), + ], +) +@pytest.mark.asyncio +async def test_user_info_v2_logs_traceback_only_for_server_errors( + mocker: MockerFixture, + caplog: pytest.LogCaptureFixture, + find_unique: AsyncMock, + expected_code: str, + logs_traceback: bool, +) -> None: + from fastapi import Request + + from litellm.proxy.management_endpoints.internal_user_endpoints import user_info_v2 + + prisma: Final = mocker.MagicMock() + prisma.db.litellm_usertable.find_unique = find_unique + mocker.patch("litellm.proxy.proxy_server.prisma_client", prisma) + + with caplog.at_level(logging.ERROR), pytest.raises(ProxyException) as exc_info: + await user_info_v2( + request=mocker.MagicMock(spec=Request), + user_id="some-user", + user_api_key_dict=UserAPIKeyAuth(user_id="admin-user", user_role=LitellmUserRoles.PROXY_ADMIN), + ) + + assert exc_info.value.code == expected_code + assert any(record.exc_info for record in caplog.records) is logs_traceback + + @pytest.mark.asyncio async def test_user_info_v2_response_shape(mocker): """ @@ -3674,6 +3710,8 @@ async def test_user_info_v2_team_admin_cannot_query_non_team_member(mocker): mock_prisma_client.db.litellm_teamtable.find_many = mocker.AsyncMock(side_effect=mock_find_many_teams) + mock_prisma_client.db.litellm_organizationmembership.find_many = mocker.AsyncMock(return_value=[]) + mocker.patch("litellm.proxy.proxy_server.prisma_client", mock_prisma_client) mock_request = mocker.MagicMock(spec=Request) @@ -3690,6 +3728,87 @@ async def test_user_info_v2_team_admin_cannot_query_non_team_member(mocker): assert exc_info.value.code == "404" +_ORG_MEMBERSHIPS: Final = ( + SimpleNamespace(user_id="org-admin-user", organization_id="org-a", user_role="org_admin"), + SimpleNamespace(user_id="org-admin-user", organization_id="org-c", user_role="internal_user"), + SimpleNamespace(user_id="member-of-a", organization_id="org-a", user_role="internal_user"), + SimpleNamespace(user_id="member-of-b", organization_id="org-b", user_role="internal_user"), + SimpleNamespace(user_id="member-of-c", organization_id="org-c", user_role="internal_user"), +) + + +def _membership_matches(membership: SimpleNamespace, where: Mapping[str, object]) -> bool: + org_filter: Final = where.get("organization_id") + return ( + membership.user_id == where["user_id"] + and ("user_role" not in where or membership.user_role == where["user_role"]) + and (not isinstance(org_filter, dict) or membership.organization_id in org_filter["in"]) + ) + + +def _org_admin_prisma(mocker: MockerFixture) -> MagicMock: + def user_row(user_id: str) -> MagicMock: + row: Final = mocker.MagicMock() + row.teams = [] + row.model_dump.return_value = {"user_id": user_id, "user_email": f"{user_id}@example.com", "teams": []} + return row + + async def find_unique(**kwargs: dict[str, str]) -> MagicMock | None: + user_id: Final = kwargs["where"]["user_id"] + return user_row(user_id) if user_id in {m.user_id for m in _ORG_MEMBERSHIPS} else None + + async def find_many(**kwargs: Mapping[str, object]) -> list[SimpleNamespace]: + return [m for m in _ORG_MEMBERSHIPS if _membership_matches(m, kwargs["where"])] + + async def find_first(**kwargs: Mapping[str, object]) -> SimpleNamespace | None: + return next((m for m in _ORG_MEMBERSHIPS if _membership_matches(m, kwargs["where"])), None) + + prisma: Final = mocker.MagicMock() + prisma.db.litellm_usertable.find_unique = mocker.AsyncMock(side_effect=find_unique) + prisma.db.litellm_organizationmembership.find_many = mocker.AsyncMock(side_effect=find_many) + prisma.db.litellm_organizationmembership.find_first = mocker.AsyncMock(side_effect=find_first) + return prisma + + +@pytest.mark.asyncio +async def test_user_info_v2_org_admin_can_query_member_of_their_org(mocker: MockerFixture) -> None: + from fastapi import Request + + from litellm.proxy.management_endpoints.internal_user_endpoints import user_info_v2 + + mocker.patch("litellm.proxy.proxy_server.prisma_client", _org_admin_prisma(mocker)) + + response: Final = await user_info_v2( + request=mocker.MagicMock(spec=Request), + user_id="member-of-a", + user_api_key_dict=UserAPIKeyAuth(user_id="org-admin-user", user_role=LitellmUserRoles.INTERNAL_USER), + ) + + assert response.user_id == "member-of-a" + assert response.user_email == "member-of-a@example.com" + + +@pytest.mark.parametrize("target_user_id", ["member-of-b", "member-of-c"]) +@pytest.mark.asyncio +async def test_user_info_v2_org_admin_cannot_query_user_outside_their_admin_orgs( + mocker: MockerFixture, target_user_id: str +) -> None: + from fastapi import Request + + from litellm.proxy.management_endpoints.internal_user_endpoints import user_info_v2 + + mocker.patch("litellm.proxy.proxy_server.prisma_client", _org_admin_prisma(mocker)) + + with pytest.raises(ProxyException) as exc_info: + await user_info_v2( + request=mocker.MagicMock(spec=Request), + user_id=target_user_id, + user_api_key_dict=UserAPIKeyAuth(user_id="org-admin-user", user_role=LitellmUserRoles.INTERNAL_USER), + ) + + assert exc_info.value.code == "404" + + @pytest.mark.asyncio async def test_user_info_v2_url_encoding_plus_character(mocker): """ diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.test.ts index 6c806613f21..36f49615317 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.test.ts @@ -330,54 +330,24 @@ describe("useUserLookup", () => { expect(result.current.data?.user_id).toBe("user-1-0"); }); - it("falls back when user info returns a different user id", async () => { - const response = buildUserListResponse(1, 1); - const exactUser = { ...response.users[0], user_id: "a b", user_alias: "Right User" }; + it("resolves to no user when user info returns a different user id", async () => { vi.mocked(userGetInfoV2).mockResolvedValue(buildUserInfoV2Response("a+b")); - vi.mocked(userListCall).mockResolvedValue({ - ...response, - page_size: 100, - users: [exactUser], - }); const { result } = renderHook(() => useUserLookup("a b"), { wrapper }); await waitFor(() => expect(result.current.isSuccess).toBe(true)); - expect(result.current.data).toMatchObject({ user_id: "a b", user_alias: "Right User" }); - expect(userListCall).toHaveBeenCalledWith("test-access-token", ["a b"], 1, 100); + expect(result.current.data).toBeNull(); + expect(userListCall).not.toHaveBeenCalled(); }); - it("falls back to the exact user list match when the user info endpoint returns a 404", async () => { - const response = buildUserListResponse(1, 1); - const exactUser = { ...response.users[1], user_id: "bob", user_alias: "Bob" }; - vi.mocked(userListCall).mockResolvedValue({ - ...response, - page_size: 100, - users: [{ ...response.users[0], user_id: "bob2" }, exactUser], - }); - vi.mocked(userGetInfoV2).mockRejectedValue(new ApiError("Not Found", 404, {})); - - const { result } = renderHook(() => useUserLookup("bob"), { wrapper }); - - await waitFor(() => expect(result.current.isSuccess).toBe(true)); - expect(result.current.data).toMatchObject(exactUser); - expect(userListCall).toHaveBeenCalledWith("test-access-token", ["bob"], 1, 100); - }); - - it("resolves to null when the user list contains only a substring match", async () => { - const response = buildUserListResponse(1, 1); - vi.mocked(userListCall).mockResolvedValue({ - ...response, - page_size: 100, - users: [{ ...response.users[0], user_id: "bob2" }], - }); + it("resolves to no user when the user info endpoint returns a 404", async () => { vi.mocked(userGetInfoV2).mockRejectedValue(new ApiError("Not Found", 404, {})); const { result } = renderHook(() => useUserLookup("bob"), { wrapper }); await waitFor(() => expect(result.current.isSuccess).toBe(true)); expect(result.current.data).toBeNull(); - expect(userListCall).toHaveBeenCalledWith("test-access-token", ["bob"], 1, 100); + expect(userListCall).not.toHaveBeenCalled(); }); it("rethrows errors other than 404", async () => { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.ts index 91699283835..20df7388ca1 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.ts @@ -1,5 +1,5 @@ import { userGetInfoV2, userListCall } from "@/components/networking"; -import type { UserInfo, UserInfoV2Response, UserListResponse } from "@/components/networking"; +import type { UserInfoV2Response, UserListResponse } from "@/components/networking"; import { useInfiniteQuery, useQuery } from "@tanstack/react-query"; import { createQueryKeys } from "../common/queryKeysFactory"; import { canListUsers } from "@/utils/roles"; @@ -11,33 +11,6 @@ const userLookupKeys = createQueryKeys("userLookup"); const DEFAULT_PAGE_SIZE = 50; -const mapUserInfoV2ToUserInfo = (user: UserInfoV2Response): UserInfo => { - const userInfo: UserInfo = { - user_id: user.user_id, - user_email: user.user_email, - user_alias: user.user_alias, - user_role: user.user_role, - spend: user.spend, - max_budget: user.max_budget, - models: user.models, - budget_duration: user.budget_duration, - budget_reset_at: user.budget_reset_at, - metadata: user.metadata, - created_at: user.created_at, - updated_at: user.updated_at, - sso_user_id: user.sso_user_id, - teams: user.teams, - tpm_limit: user.tpm_limit, - rpm_limit: user.rpm_limit, - allowed_cache_controls: [], - key_count: 0, - model_max_budget: null, - model_spend: null, - policies: [], - }; - return userInfo; -}; - export const useInfiniteUsers = (pageSize: number = DEFAULT_PAGE_SIZE, searchEmail?: string) => { const { accessToken, userRole } = useAuthorized(); return useInfiniteQuery({ @@ -87,17 +60,16 @@ export const useUserEmailLookup = (userIds: readonly string[]) => { export const useUserLookup = (userId: string | null) => { const { accessToken, userRole } = useAuthorized(); - return useQuery({ + return useQuery({ queryKey: userLookupKeys.detail(userId ?? ""), queryFn: async () => { try { const user = await userGetInfoV2(accessToken!, userId!); - if (user.user_id === userId) return mapUserInfoV2ToUserInfo(user); + return user.user_id === userId ? user : null; } catch (error) { - if (!(error instanceof ApiError && error.status === 404)) throw error; + if (error instanceof ApiError && error.status === 404) return null; + throw error; } - const response = await userListCall(accessToken!, [userId!], 1, USER_LIST_MAX_PAGE_SIZE); - return response.users.find((user) => user.user_id === userId) ?? null; }, enabled: Boolean(accessToken) && Boolean(userId) && canListUsers(userRole), });