mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
feat(team): let org admins reach PATCH /team/{team_id} like POST /team/update
Wire the coarse route gate so PATCH /team/{team_id} is reachable by exactly the roles that can call POST /team/update: proxy admins, org admins of the team's own organization, and JWT admins. Regular internal users and view-only proxy admins stay blocked, matching the existing endpoint
Because the team id lives in the path rather than the body, the org-context resolver now also reads it from path_team_id for the bare /team/{team_id} route, so an org admin's organization is resolved and injected the same way it already is for POST /team/update. /team/{team_id} is added to management_routes rather than the role-agnostic self_managed_routes; the latter would have opened POST /team/new to any authenticated user through the shared /team/{team_id} path pattern
This commit is contained in:
parent
e9246e924e
commit
8f0dcb23f4
4 changed files with 172 additions and 3 deletions
|
|
@ -589,6 +589,7 @@ class LiteLLMRoutes(enum.Enum):
|
|||
# team
|
||||
"/team/new",
|
||||
"/team/update",
|
||||
"/team/{team_id}",
|
||||
"/team/delete",
|
||||
"/team/list",
|
||||
"/v2/team/list",
|
||||
|
|
|
|||
|
|
@ -726,6 +726,7 @@ async def common_checks(
|
|||
route=route,
|
||||
request_body=request_body,
|
||||
fetch_team_org_id=_fetch_team_org_id,
|
||||
path_team_id=request.path_params.get("team_id"),
|
||||
)
|
||||
|
||||
_is_route_allowed = _is_api_route_allowed(
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
Auth Checks for Organizations
|
||||
"""
|
||||
|
||||
import re
|
||||
from typing import Awaitable, Callable, Dict, List, Optional, Tuple
|
||||
|
||||
from fastapi import status
|
||||
|
|
@ -173,12 +174,14 @@ def _user_is_org_admin(
|
|||
|
||||
|
||||
TEAM_ORG_CONTEXT_ROUTES = frozenset({"/team/update"})
|
||||
_TEAM_ID_PATH_ROUTE = re.compile(r"^/team/[^/]+$")
|
||||
|
||||
|
||||
async def add_team_org_context_to_request_body(
|
||||
route: str,
|
||||
request_body: dict,
|
||||
fetch_team_org_id: Callable[[str], Awaitable[Optional[str]]],
|
||||
path_team_id: Optional[str] = None,
|
||||
) -> dict:
|
||||
"""
|
||||
Return a copy of request_body with organization_id resolved from the target
|
||||
|
|
@ -188,12 +191,20 @@ async def add_team_org_context_to_request_body(
|
|||
the client having to send it. Returns request_body unchanged when it does
|
||||
not apply, so callers that already pass organization_id and non-team routes
|
||||
are untouched.
|
||||
|
||||
The team_id is taken from the body for TEAM_ORG_CONTEXT_ROUTES, or from the
|
||||
path (``path_team_id``) for the bare ``/team/{team_id}`` route.
|
||||
"""
|
||||
if route not in TEAM_ORG_CONTEXT_ROUTES:
|
||||
return request_body
|
||||
if request_body.get("organization_id"):
|
||||
return request_body
|
||||
team_id = request_body.get("team_id")
|
||||
|
||||
if route in TEAM_ORG_CONTEXT_ROUTES:
|
||||
team_id: Optional[str] = request_body.get("team_id")
|
||||
elif path_team_id and _TEAM_ID_PATH_ROUTE.match(route):
|
||||
team_id = path_team_id
|
||||
else:
|
||||
return request_body
|
||||
|
||||
if not isinstance(team_id, str) or not team_id:
|
||||
return request_body
|
||||
org_id = await fetch_team_org_id(team_id)
|
||||
|
|
|
|||
|
|
@ -2724,6 +2724,162 @@ def test_team_update_gate_rejects_cross_org_admin_with_resolved_org():
|
|||
)
|
||||
|
||||
|
||||
# ── PATCH /team/{team_id}: same org-context + role reach as POST /team/update ──
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_team_org_context_resolves_org_from_path_for_patch_route():
|
||||
"""PATCH /team/{team_id} carries team_id in the PATH, not the body. The target
|
||||
team's org is resolved from path_team_id and injected, so an org admin of that
|
||||
team's org clears the same gate they clear for POST /team/update."""
|
||||
|
||||
async def fetch(team_id: str):
|
||||
assert team_id == "team-1"
|
||||
return "org-1"
|
||||
|
||||
out = await add_team_org_context_to_request_body(
|
||||
route="/team/team-1",
|
||||
request_body={"metadata": {"cost_center": "x"}},
|
||||
fetch_team_org_id=fetch,
|
||||
path_team_id="team-1",
|
||||
)
|
||||
assert out == {"metadata": {"cost_center": "x"}, "organization_id": "org-1"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_team_org_context_path_noop_for_team_subresource():
|
||||
"""A sub-resource like /team/{team_id}/members/me is NOT the bare team route, so
|
||||
no org is injected even though a team_id path param is present."""
|
||||
|
||||
async def fetch(team_id: str):
|
||||
raise AssertionError("must not resolve for a team sub-resource route")
|
||||
|
||||
body = {"foo": "bar"}
|
||||
out = await add_team_org_context_to_request_body(
|
||||
route="/team/team-1/members/me",
|
||||
request_body=body,
|
||||
fetch_team_org_id=fetch,
|
||||
path_team_id="team-1",
|
||||
)
|
||||
assert out == body
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_team_org_context_path_noop_without_path_team_id():
|
||||
"""Routes with no team_id path param (e.g. POST /team/new, whose path also
|
||||
matches the bare shape) resolve nothing."""
|
||||
|
||||
async def fetch(team_id: str):
|
||||
raise AssertionError("must not resolve when there is no path team_id")
|
||||
|
||||
body = {"team_alias": "new team"}
|
||||
out = await add_team_org_context_to_request_body(
|
||||
route="/team/new",
|
||||
request_body=body,
|
||||
fetch_team_org_id=fetch,
|
||||
path_team_id=None,
|
||||
)
|
||||
assert out == body
|
||||
|
||||
|
||||
def test_patch_team_route_has_same_reach_as_team_update():
|
||||
"""/team/{team_id} is reachable by org admins (in org_admin_allowed_routes) but
|
||||
NOT by regular internal users or the role-agnostic self_managed_routes — the
|
||||
latter would open /team/new (the collision footgun) to any authenticated user."""
|
||||
from litellm.proxy._types import LiteLLMRoutes
|
||||
|
||||
assert RouteChecks.check_route_access(
|
||||
route="/team/abc-123", allowed_routes=LiteLLMRoutes.org_admin_allowed_routes.value
|
||||
)
|
||||
assert not RouteChecks.check_route_access(
|
||||
route="/team/abc-123", allowed_routes=LiteLLMRoutes.internal_user_routes.value
|
||||
)
|
||||
assert not RouteChecks.check_route_access(
|
||||
route="/team/abc-123", allowed_routes=LiteLLMRoutes.self_managed_routes.value
|
||||
)
|
||||
|
||||
|
||||
def _patch_team_request() -> MagicMock:
|
||||
request = MagicMock(spec=Request)
|
||||
request.method = "PATCH"
|
||||
request.query_params = {}
|
||||
return request
|
||||
|
||||
|
||||
def test_patch_team_gate_allows_org_admin_with_resolved_org():
|
||||
"""Post-resolution, an org admin of the team's org clears the coarse gate for
|
||||
PATCH /team/{team_id} — parity with /team/update."""
|
||||
user_obj = _make_org_admin_user("org-1")
|
||||
valid_token = UserAPIKeyAuth(user_id="org-admin-user", user_role=LitellmUserRoles.INTERNAL_USER.value)
|
||||
|
||||
RouteChecks.non_proxy_admin_allowed_routes_check(
|
||||
user_obj=user_obj,
|
||||
_user_role=LitellmUserRoles.INTERNAL_USER.value,
|
||||
route="/team/team-1",
|
||||
request=_patch_team_request(),
|
||||
valid_token=valid_token,
|
||||
request_data={"organization_id": "org-1"},
|
||||
)
|
||||
|
||||
|
||||
def test_patch_team_gate_rejects_regular_internal_user():
|
||||
"""A plain internal user (not an org admin) is rejected at the coarse gate for
|
||||
PATCH /team/{team_id}, even with the team's org resolved — injection alone is
|
||||
not access. Same outcome as /team/update."""
|
||||
user_obj = LiteLLM_UserTable(
|
||||
user_id="regular-user",
|
||||
user_role=LitellmUserRoles.INTERNAL_USER.value,
|
||||
organization_memberships=None,
|
||||
)
|
||||
valid_token = UserAPIKeyAuth(user_id="regular-user", user_role=LitellmUserRoles.INTERNAL_USER.value)
|
||||
|
||||
with pytest.raises(Exception):
|
||||
RouteChecks.non_proxy_admin_allowed_routes_check(
|
||||
user_obj=user_obj,
|
||||
_user_role=LitellmUserRoles.INTERNAL_USER.value,
|
||||
route="/team/team-1",
|
||||
request=_patch_team_request(),
|
||||
valid_token=valid_token,
|
||||
request_data={"organization_id": "org-1"},
|
||||
)
|
||||
|
||||
|
||||
def test_patch_team_gate_rejects_cross_org_admin():
|
||||
"""An org admin of a DIFFERENT org is rejected even after org resolution."""
|
||||
user_obj = _make_org_admin_user("org-1")
|
||||
valid_token = UserAPIKeyAuth(user_id="org-admin-user", user_role=LitellmUserRoles.INTERNAL_USER.value)
|
||||
|
||||
with pytest.raises(Exception):
|
||||
RouteChecks.non_proxy_admin_allowed_routes_check(
|
||||
user_obj=user_obj,
|
||||
_user_role=LitellmUserRoles.INTERNAL_USER.value,
|
||||
route="/team/team-1",
|
||||
request=_patch_team_request(),
|
||||
valid_token=valid_token,
|
||||
request_data={"organization_id": "org-2"},
|
||||
)
|
||||
|
||||
|
||||
def test_patch_team_gate_rejects_view_only_admin():
|
||||
"""A view-only proxy admin cannot PATCH a team (unsafe method), parity with the
|
||||
/team/update view-only block."""
|
||||
user_obj = LiteLLM_UserTable(
|
||||
user_id="viewer",
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value,
|
||||
)
|
||||
valid_token = UserAPIKeyAuth(user_id="viewer", user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value)
|
||||
|
||||
with pytest.raises(Exception):
|
||||
RouteChecks.non_proxy_admin_allowed_routes_check(
|
||||
user_obj=user_obj,
|
||||
_user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value,
|
||||
route="/team/team-1",
|
||||
request=_patch_team_request(),
|
||||
valid_token=valid_token,
|
||||
request_data={"organization_id": "org-1"},
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_initialize_pass_through_registers_wildcard_for_auth_subpath():
|
||||
"""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue