mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
fix(team): also gate organization-scoped endpoints behind _verify_org_access
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) <noreply@anthropic.com>
This commit is contained in:
parent
578846e57d
commit
a9bc5549b2
2 changed files with 168 additions and 0 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue