From 467166fdd7c26b89ea73cf418e419881e25fa1fe Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 17 Apr 2026 00:11:00 +0000 Subject: [PATCH] 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. --- .../internal_user_endpoints.py | 98 +++++++++++++++---- .../test_internal_user_endpoints.py | 71 ++++++++++++++ 2 files changed, 149 insertions(+), 20 deletions(-) diff --git a/litellm/proxy/management_endpoints/internal_user_endpoints.py b/litellm/proxy/management_endpoints/internal_user_endpoints.py index 1772da3d15..be0f02439b 100644 --- a/litellm/proxy/management_endpoints/internal_user_endpoints.py +++ b/litellm/proxy/management_endpoints/internal_user_endpoints.py @@ -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( diff --git a/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py index 0f90d236ae..104071c3e6 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py @@ -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 # =====================================================================