mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-02 07:13:02 +00:00
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.
This commit is contained in:
parent
0db7a19a38
commit
f998fb05cb
4 changed files with 159 additions and 69 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -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<UserListResponse>({
|
||||
|
|
@ -87,17 +60,16 @@ export const useUserEmailLookup = (userIds: readonly string[]) => {
|
|||
|
||||
export const useUserLookup = (userId: string | null) => {
|
||||
const { accessToken, userRole } = useAuthorized();
|
||||
return useQuery<UserInfo | null>({
|
||||
return useQuery<UserInfoV2Response | null>({
|
||||
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),
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue