mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-12 20:22:46 +00:00
fix(proxy): enforce per-target org authorization on /user/delete
Veria admin-queue finding E3NpkuAd, Audit-B #1. The route-level gate accepts this call when the caller is PROXY_ADMIN or ORG_ADMIN of any org named in request_data["organization_id"]/["organizations"]. The handler processes data.user_ids without cross-checking whether those users belong to the caller's administered orgs, so an org-admin of org-A could delete users in org-B via: {"user_ids": ["victim_in_org_B"], "organization_id": "org-A"} Add per-target authorization: org-admins may only delete users whose entire org membership is within their admin scope; targets with any org outside scope (or no org at all) require PROXY_ADMIN. Regression test confirms an ORG_ADMIN call fails with 403 and no cascade delete_many runs.
This commit is contained in:
@@ -2057,6 +2057,38 @@ async def delete_user(
|
||||
if data.user_ids is None:
|
||||
raise HTTPException(status_code=400, detail={"error": "No user id passed in"})
|
||||
|
||||
# Per-target authorization: the route-level gate accepts this call when
|
||||
# the caller is PROXY_ADMIN or an ORG_ADMIN of *any* org named in
|
||||
# request_data["organization_id"]/["organizations"]. That gate does NOT
|
||||
# cross-check data.user_ids against the caller's scope, so without this
|
||||
# loop an org-admin of org-A could delete users in org-B by supplying
|
||||
# {"user_ids": [victim_in_org_B], "organization_id": "org-A"}.
|
||||
caller_is_proxy_admin = (
|
||||
user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value
|
||||
)
|
||||
caller_admin_org_ids: set = set()
|
||||
if not caller_is_proxy_admin:
|
||||
caller_memberships = (
|
||||
await prisma_client.db.litellm_organizationmembership.find_many(
|
||||
where={
|
||||
"user_id": user_api_key_dict.user_id,
|
||||
"user_role": LitellmUserRoles.ORG_ADMIN.value,
|
||||
}
|
||||
)
|
||||
if user_api_key_dict.user_id
|
||||
else []
|
||||
)
|
||||
caller_admin_org_ids = {
|
||||
m.organization_id for m in caller_memberships if m.organization_id
|
||||
}
|
||||
if not caller_admin_org_ids:
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail={
|
||||
"error": "Only PROXY_ADMIN or ORG_ADMIN users may delete users."
|
||||
},
|
||||
)
|
||||
|
||||
# check that all teams passed exist
|
||||
for user_id in data.user_ids:
|
||||
user_row = await prisma_client.db.litellm_usertable.find_unique(
|
||||
@@ -2068,30 +2100,56 @@ async def delete_user(
|
||||
status_code=404,
|
||||
detail={"error": f"User not found, passed user_id={user_id}"},
|
||||
)
|
||||
else:
|
||||
# Enterprise Feature - Audit Logging. Enable with litellm.store_audit_logs = True
|
||||
# we do this after the first for loop, since first for loop is for validation. we only want this inserted after validation passes
|
||||
if litellm.store_audit_logs is True:
|
||||
# make an audit log for each team deleted
|
||||
_user_row = user_row.json(exclude_none=True)
|
||||
|
||||
asyncio.create_task(
|
||||
create_audit_log_for_update(
|
||||
request_data=LiteLLM_AuditLogs(
|
||||
id=str(uuid.uuid4()),
|
||||
updated_at=datetime.now(timezone.utc),
|
||||
changed_by=litellm_changed_by
|
||||
or user_api_key_dict.user_id
|
||||
or litellm_proxy_admin_name,
|
||||
changed_by_api_key=user_api_key_dict.api_key,
|
||||
table_name=LitellmTableNames.USER_TABLE_NAME,
|
||||
object_id=user_id,
|
||||
action="deleted",
|
||||
updated_values="{}",
|
||||
before_value=_user_row,
|
||||
if not caller_is_proxy_admin:
|
||||
target_memberships = (
|
||||
await prisma_client.db.litellm_organizationmembership.find_many(
|
||||
where={"user_id": user_id}
|
||||
)
|
||||
)
|
||||
target_org_ids = {
|
||||
m.organization_id for m in target_memberships if m.organization_id
|
||||
}
|
||||
# Org-admin may only delete users whose entire org membership is
|
||||
# within their admin scope. A target with ANY org outside the
|
||||
# caller's scope (or no org at all) requires PROXY_ADMIN.
|
||||
if not target_org_ids or not target_org_ids.issubset(
|
||||
caller_admin_org_ids
|
||||
):
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail={
|
||||
"error": (
|
||||
f"User {user_id} is not within your admin scope. "
|
||||
"Only PROXY_ADMIN may delete users outside your "
|
||||
"administered organizations."
|
||||
)
|
||||
},
|
||||
)
|
||||
|
||||
# Enterprise Feature - Audit Logging. Enable with litellm.store_audit_logs = True
|
||||
# we do this after the first for loop, since first for loop is for validation. we only want this inserted after validation passes
|
||||
if litellm.store_audit_logs is True:
|
||||
# make an audit log for each team deleted
|
||||
_user_row = user_row.json(exclude_none=True)
|
||||
|
||||
asyncio.create_task(
|
||||
create_audit_log_for_update(
|
||||
request_data=LiteLLM_AuditLogs(
|
||||
id=str(uuid.uuid4()),
|
||||
updated_at=datetime.now(timezone.utc),
|
||||
changed_by=litellm_changed_by
|
||||
or user_api_key_dict.user_id
|
||||
or litellm_proxy_admin_name,
|
||||
changed_by_api_key=user_api_key_dict.api_key,
|
||||
table_name=LitellmTableNames.USER_TABLE_NAME,
|
||||
object_id=user_id,
|
||||
action="deleted",
|
||||
updated_values="{}",
|
||||
before_value=_user_row,
|
||||
)
|
||||
)
|
||||
)
|
||||
|
||||
## CLEANUP MEMBERS_WITH_ROLES
|
||||
fetch_all_teams = await prisma_client.db.litellm_teamtable.find_many(
|
||||
|
||||
@@ -1878,6 +1878,77 @@ async def test_delete_user_cleans_up_created_by_invitation_links(mocker):
|
||||
assert condition[field] == {"in": ["admin-creator"]}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_user_rejects_org_admin_deleting_outside_scope(mocker):
|
||||
"""Regression: an org admin of org-A must not be able to delete a user
|
||||
whose org memberships include org-B.
|
||||
|
||||
Route-level gate accepts the request when the caller supplies an
|
||||
`organization_id` they administer; without per-user org authorization
|
||||
the handler would cascade-delete the victim's keys, memberships, and
|
||||
user row regardless of where the victim actually belongs.
|
||||
"""
|
||||
from fastapi import HTTPException
|
||||
|
||||
from litellm.proxy._types import DeleteUserRequest, UserAPIKeyAuth
|
||||
from litellm.proxy.management_endpoints.internal_user_endpoints import delete_user
|
||||
|
||||
mock_prisma_client = mocker.MagicMock()
|
||||
|
||||
# Target user exists and is a member of org-B only.
|
||||
mock_target_user = mocker.MagicMock()
|
||||
mock_target_user.user_id = "victim"
|
||||
mock_target_user.user_email = "victim@example.com"
|
||||
mock_target_user.teams = []
|
||||
mock_target_user.json.return_value = "{}"
|
||||
|
||||
async def mock_find_unique(*args, **kwargs):
|
||||
return mock_target_user
|
||||
|
||||
mock_prisma_client.db.litellm_usertable.find_unique = mocker.AsyncMock(
|
||||
side_effect=mock_find_unique
|
||||
)
|
||||
|
||||
# Caller (org_admin_user) administers org-A.
|
||||
caller_membership = mocker.MagicMock()
|
||||
caller_membership.organization_id = "org-A"
|
||||
|
||||
# Target user is a member of org-B (outside caller's scope).
|
||||
target_membership = mocker.MagicMock()
|
||||
target_membership.organization_id = "org-B"
|
||||
|
||||
async def mock_find_memberships(*args, **kwargs):
|
||||
where = kwargs.get("where") or (args[0] if args else {})
|
||||
user_id = where.get("user_id")
|
||||
if user_id == "org_admin_user":
|
||||
return [caller_membership]
|
||||
if user_id == "victim":
|
||||
return [target_membership]
|
||||
return []
|
||||
|
||||
mock_prisma_client.db.litellm_organizationmembership.find_many = mocker.AsyncMock(
|
||||
side_effect=mock_find_memberships
|
||||
)
|
||||
|
||||
mocker.patch("litellm.proxy.proxy_server.prisma_client", mock_prisma_client)
|
||||
|
||||
data = DeleteUserRequest(user_ids=["victim"])
|
||||
user_api_key_dict = UserAPIKeyAuth(
|
||||
user_id="org_admin_user", user_role=LitellmUserRoles.ORG_ADMIN
|
||||
)
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
await delete_user(data=data, user_api_key_dict=user_api_key_dict)
|
||||
assert exc.value.status_code == 403
|
||||
|
||||
# Critical: no delete_many calls should have executed.
|
||||
assert not hasattr(
|
||||
mock_prisma_client.db.litellm_verificationtoken.delete_many, "mock_calls"
|
||||
) or len(
|
||||
mock_prisma_client.db.litellm_verificationtoken.delete_many.mock_calls
|
||||
) == 0
|
||||
|
||||
|
||||
# =====================================================================
|
||||
# /v2/user/info endpoint tests
|
||||
# =====================================================================
|
||||
|
||||
Reference in New Issue
Block a user