From 8f0dcb23f4e598265e2661b2bd98670c4432bffd Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 10 Jul 2026 23:14:01 -0700 Subject: [PATCH] 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 --- litellm/proxy/_types.py | 1 + litellm/proxy/auth/auth_checks.py | 1 + .../proxy/auth/auth_checks_organization.py | 17 +- .../proxy/auth/test_route_checks.py | 156 ++++++++++++++++++ 4 files changed, 172 insertions(+), 3 deletions(-) diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index f441a1d3f84..23fe7730c17 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -589,6 +589,7 @@ class LiteLLMRoutes(enum.Enum): # team "/team/new", "/team/update", + "/team/{team_id}", "/team/delete", "/team/list", "/v2/team/list", diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index cd8103abf5e..fab0338c7a8 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -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( diff --git a/litellm/proxy/auth/auth_checks_organization.py b/litellm/proxy/auth/auth_checks_organization.py index b4caff9b8ee..bb82b3a142d 100644 --- a/litellm/proxy/auth/auth_checks_organization.py +++ b/litellm/proxy/auth/auth_checks_organization.py @@ -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) diff --git a/tests/test_litellm/proxy/auth/test_route_checks.py b/tests/test_litellm/proxy/auth/test_route_checks.py index 204e6a671e3..404701bb8ac 100644 --- a/tests/test_litellm/proxy/auth/test_route_checks.py +++ b/tests/test_litellm/proxy/auth/test_route_checks.py @@ -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(): """