From a9bc5549b2616d06bb4a824940b941e0d91812c3 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Wed, 29 Apr 2026 22:51:59 +0000 Subject: [PATCH] fix(team): also gate organization-scoped endpoints behind _verify_org_access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Variant analysis on the team-callback IDOR (GHSA-xxv2-fprq-9x93) surfaced the same shape on organization-scoped endpoints. Each takes ``organization_id`` from the request body, looks up the org, and performs reads or writes — without checking whether the caller can manage that org. Affected endpoints: * ``PATCH /organization/update`` — any authenticated key holder could rewrite any org's metadata, budgets, and object permissions. * ``POST /organization/member_add`` — docstring promises "Only proxy_admin or org_admin allowed" but the code never enforced it; any caller could add members to any org. * ``POST /organization/member_update`` — only the ``modify-PROXY_ADMIN-target-only`` defense was in place; non-admin members in any org could be re-roled by any caller. * ``POST /organization/member_delete`` — no access check at all; any caller could remove any user from any org. Each handler now runs the existing ``_verify_org_access`` helper (proxy-admin / org-admin hierarchy already used by ``GET /organization/info`` and ``POST /organization/info``) before the read or write. Tests: - ``test_organization_member_add_rejects_unauthorized_caller`` — internal user not on the org gets 403; DB write never happens. - ``test_organization_member_update_rejects_unauthorized_caller`` — same. - ``test_organization_member_delete_rejects_unauthorized_caller`` — same. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../organization_endpoints.py | 38 +++++ .../test_organization_endpoints.py | 130 ++++++++++++++++++ 2 files changed, 168 insertions(+) diff --git a/litellm/proxy/management_endpoints/organization_endpoints.py b/litellm/proxy/management_endpoints/organization_endpoints.py index 442fae2a4fa..79b9c459e4b 100644 --- a/litellm/proxy/management_endpoints/organization_endpoints.py +++ b/litellm/proxy/management_endpoints/organization_endpoints.py @@ -500,6 +500,15 @@ async def update_organization( if data.updated_by is None: data.updated_by = user_api_key_dict.user_id + # IDOR guard: only proxy admins / org admins of THIS org may update + # it. Without this, any authenticated key holder could rewrite + # another organization's metadata, budgets, and object permissions. + await _verify_org_access( + organization_id=data.organization_id, + user_api_key_dict=user_api_key_dict, + prisma_client=prisma_client, + ) + existing_organization_row = ( await prisma_client.db.litellm_organizationtable.find_unique( where={"organization_id": data.organization_id}, @@ -909,6 +918,16 @@ async def organization_member_add( if prisma_client is None: raise HTTPException(status_code=500, detail={"error": "No db connected"}) + # IDOR guard: docstring says "Only proxy_admin or org_admin of + # organization, allowed to access this endpoint" — but the code + # never enforced that. Any authenticated key holder could add + # members to any org. Now gated explicitly. + await _verify_org_access( + organization_id=data.organization_id, + user_api_key_dict=user_api_key_dict, + prisma_client=prisma_client, + ) + # Check if organization exists existing_organization_row = ( await prisma_client.db.litellm_organizationtable.find_unique( @@ -1018,6 +1037,16 @@ async def organization_member_update( detail={"error": CommonProxyErrors.db_not_connected_error.value}, ) + # IDOR guard: only proxy admins / org admins of THIS org may + # update member roles. The PROXY_ADMIN-target check below was + # the only access control; without this, any authenticated user + # could change any non-admin member's role in any org. + await _verify_org_access( + organization_id=data.organization_id, + user_api_key_dict=user_api_key_dict, + prisma_client=prisma_client, + ) + # Check if organization exists existing_organization_row = ( await prisma_client.db.litellm_organizationtable.find_unique( @@ -1179,6 +1208,15 @@ async def organization_member_delete( detail={"error": CommonProxyErrors.db_not_connected_error.value}, ) + # IDOR guard: only proxy admins / org admins of THIS org may + # delete members. Without this, any authenticated key holder + # could remove any user from any org. + await _verify_org_access( + organization_id=data.organization_id, + user_api_key_dict=user_api_key_dict, + prisma_client=prisma_client, + ) + if data.user_email is not None and data.user_id is None: existing_user_email_row = await find_member_if_email( data.user_email, prisma_client 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 4501cc76636..f4470e7e83d 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_organization_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_organization_endpoints.py @@ -565,3 +565,133 @@ async def test_organization_info_includes_user_email(monkeypatch): membership = LiteLLM_OrganizationMembershipTable(**raw_membership) assert membership.user_email == "alice@example.com" + + +# Regression tests for IDOR fixes on org-scoped endpoints. Sibling cluster +# to GHSA-xxv2-fprq-9x93 (team callback IDOR): the same shape of "any +# authenticated key holder reaches an endpoint that takes an +# organization_id from the request body without an access guard." The +# fix routes ``update_organization``, ``organization_member_add``, +# ``organization_member_update``, and ``organization_member_delete`` +# through the existing ``_verify_org_access`` helper. + + +@pytest.fixture +def unauthorized_caller(): + from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth + + return UserAPIKeyAuth( + user_role=LitellmUserRoles.INTERNAL_USER, + user_id="random_authenticated_user", + api_key="sk-random", + ) + + +@pytest.fixture +def patched_org_prisma(): + """Mock prisma so that find_unique returns a victim org and + get_user_object reports the caller has no org membership — so + _verify_org_access raises 403.""" + victim_row = MagicMock() + victim_row.organization_id = "org-victim" + victim_row.metadata = {} + victim_row.model_dump.return_value = {"organization_id": "org-victim"} + + caller_user = MagicMock() + caller_user.organization_memberships = [] # no admin role anywhere + + with ( + patch("litellm.proxy.proxy_server.prisma_client") as mock_prisma, + patch( + "litellm.proxy.management_endpoints.organization_endpoints.get_user_object", + new_callable=AsyncMock, + return_value=caller_user, + ), + patch( + "litellm.proxy.proxy_server.user_api_key_cache", + ), + patch("litellm.proxy.proxy_server.proxy_logging_obj"), + ): + mock_prisma.db.litellm_organizationtable.find_unique = AsyncMock( + return_value=victim_row + ) + yield mock_prisma + + +@pytest.mark.asyncio +async def test_organization_member_add_rejects_unauthorized_caller( + patched_org_prisma, unauthorized_caller +): + # ``organization_member_add`` catches HTTPException in its + # catch-all and re-wraps as ProxyException with the original status + # code preserved. + from litellm.proxy._types import ( + OrganizationMemberAddRequest, + OrgMember, + ProxyException, + ) + from litellm.proxy.management_endpoints.organization_endpoints import ( + organization_member_add, + ) + from unittest.mock import Mock + + from fastapi import Request + + data = OrganizationMemberAddRequest( + organization_id="org-victim", + member=OrgMember(role="internal_user", user_id="attacker-user"), + ) + + with pytest.raises((HTTPException, ProxyException)) as exc: + await organization_member_add( + data=data, + http_request=Mock(spec=Request), + user_api_key_dict=unauthorized_caller, + ) + code = getattr(exc.value, "status_code", None) or getattr(exc.value, "code", None) + assert int(code) == 403 + + +@pytest.mark.asyncio +async def test_organization_member_update_rejects_unauthorized_caller( + patched_org_prisma, unauthorized_caller +): + from litellm.proxy._types import OrganizationMemberUpdateRequest + from litellm.proxy.management_endpoints.organization_endpoints import ( + organization_member_update, + ) + + data = OrganizationMemberUpdateRequest( + organization_id="org-victim", + user_id="some-other-user", + role="org_admin", + ) + + with pytest.raises(HTTPException) as exc: + await organization_member_update( + data=data, + user_api_key_dict=unauthorized_caller, + ) + assert exc.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_organization_member_delete_rejects_unauthorized_caller( + patched_org_prisma, unauthorized_caller +): + from litellm.proxy._types import OrganizationMemberDeleteRequest + from litellm.proxy.management_endpoints.organization_endpoints import ( + organization_member_delete, + ) + + data = OrganizationMemberDeleteRequest( + organization_id="org-victim", + user_id="some-other-user", + ) + + with pytest.raises(HTTPException) as exc: + await organization_member_delete( + data=data, + user_api_key_dict=unauthorized_caller, + ) + assert exc.value.status_code == 403