From 0c39090403774d021e9fafe1a1e5ff6726e34f34 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 01:33:03 +0000 Subject: [PATCH] fix(team_endpoints): scope an unfiltered /team/list to the caller A non-admin listing teams without a user_id filter meant "my teams", but both /team/list and /v2/team/list ran the admin route check before falling back to the caller's own user_id, so the request 401'd. Self-scope first, so only an explicit request for another user's teams is rejected. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../management_endpoints/team_endpoints.py | 40 +++--- .../management/test_team_list.py | 28 +++-- .../management/test_team_list_v2.py | 22 ++-- .../test_team_endpoints.py | 119 ++++++++++-------- 4 files changed, 122 insertions(+), 87 deletions(-) diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index 95632d7cb35..525fd4ee151 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -4763,17 +4763,17 @@ async def _enforce_list_team_v2_access( user_id, ) else: - # Not an org admin — fall back to standard route check - if not allowed_route_check_inside_route(user_api_key_dict=user_api_key_dict, requested_user_id=user_id): + # An unscoped query from a regular user means "my teams"; only an explicit + # request for another user's teams is rejected + scoped_user_id: Final = user_id if user_id is not None else user_api_key_dict.user_id + if not allowed_route_check_inside_route(user_api_key_dict=user_api_key_dict, requested_user_id=scoped_user_id): raise HTTPException( status_code=401, detail={ "error": f"Only admin users can query all teams/other teams. Your user role={user_api_key_dict.user_role}" }, ) - # Regular user — auto-inject caller's user_id - if user_id is None: - user_id = user_api_key_dict.user_id + return scoped_user_id, org_admin_org_ids return user_id, org_admin_org_ids @@ -4973,21 +4973,25 @@ async def _authorize_and_filter_teams( prisma_client: PrismaClient, user_api_key_cache: UserApiKeyCache, proxy_logging_obj: ProxyLogging, -) -> list: +) -> tuple[list, str | None]: """ - Authorize the /team/list request and return filtered teams. + Authorize the /team/list request and return the filtered teams plus the user_id they were scoped to. - Proxy admins: all teams (or filtered by user_id if provided). - Org admins: teams from their orgs (scoped to user_id if provided). - - Own query (user_id matches caller): teams the user is a member of. + - Own query (user_id omitted, or matching the caller): teams the user is a member of. - Others: 401. """ is_proxy_admin: Final = _user_has_admin_view(user_api_key_dict) allowed_org_ids: list[str] | None = None + # An unscoped query from a non-admin means "my teams" + scoped_user_id: Final = user_id if (is_proxy_admin or user_id is not None) else user_api_key_dict.user_id if not is_proxy_admin: is_own_query: Final = ( - user_id is not None and user_api_key_dict.user_id is not None and user_api_key_dict.user_id == user_id + scoped_user_id is not None + and user_api_key_dict.user_id is not None + and user_api_key_dict.user_id == scoped_user_id ) # Check if user is an org admin (even for own queries, so they see org teams) @@ -5023,14 +5027,14 @@ async def _authorize_and_filter_teams( include={"litellm_model_table": True}, ) if not user_id: - return list(org_teams) + return list(org_teams), None # Filter org teams to only those where the target user is a member return [ team for team in org_teams if team.members_with_roles and any(m.get("user_id") == user_id for m in team.members_with_roles) - ] - elif user_id: + ], user_id + elif scoped_user_id: # Regular user: fetch all and filter by membership (Prisma can't filter JSON arrays) response: Final = await _raw_team_db(TeamRepository(prisma_client)).find_many( include={"litellm_model_table": True} @@ -5038,11 +5042,13 @@ async def _authorize_and_filter_teams( return [ team for team in response - if team.members_with_roles and any(m.get("user_id") == user_id for m in team.members_with_roles) - ] + if team.members_with_roles and any(m.get("user_id") == scoped_user_id for m in team.members_with_roles) + ], scoped_user_id else: # Proxy admin: all teams - return list(await _raw_team_db(TeamRepository(prisma_client)).find_many(include={"litellm_model_table": True})) + return list( + await _raw_team_db(TeamRepository(prisma_client)).find_many(include={"litellm_model_table": True}) + ), None @router.get("/team/list", tags=["team management"], dependencies=[Depends(user_api_key_auth)]) @@ -5075,7 +5081,7 @@ async def list_team( detail={"error": CommonProxyErrors.db_not_connected_error.value}, ) - filtered_response: Final = await _authorize_and_filter_teams( + filtered_response, scoped_user_id = await _authorize_and_filter_teams( user_api_key_dict=user_api_key_dict, user_id=user_id, prisma_client=prisma_client, @@ -5084,7 +5090,7 @@ async def list_team( ) _team_ids: Final = [team.team_id for team in filtered_response] - returned_tm: Final = await get_all_team_memberships(prisma_client, _team_ids, user_id=user_id) + returned_tm: Final = await get_all_team_memberships(prisma_client, _team_ids, user_id=scoped_user_id) returned_responses: list[TeamListResponseObject] = [] for team in filtered_response: diff --git a/tests/proxy_behavior/management/test_team_list.py b/tests/proxy_behavior/management/test_team_list.py index 2bd106dd2d0..3add80a9269 100644 --- a/tests/proxy_behavior/management/test_team_list.py +++ b/tests/proxy_behavior/management/test_team_list.py @@ -23,17 +23,18 @@ def _seeded_visible(resp_json, world) -> set: # Family 1 — bare GET /team/list (no query params). _authorize_and_filter_teams -# authorizes only an admin view (proxy admin) or an org admin; everyone else -# is 401. An org admin sees every team in its org(s). +# gives a proxy admin every team, an org admin every team in its org(s), and +# everyone else the teams they belong to (LIT-5384: a bare query from a +# regular user used to be 401). _BARE = [ ("proxy_admin", Actor.PROXY_ADMIN, 200, {"alpha", "beta", "gamma"}), ("org_admin", Actor.ORG_ADMIN, 200, {"alpha", "gamma"}), - ("team_admin", Actor.TEAM_ADMIN, 401, None), - ("internal_user", Actor.INTERNAL_USER, 401, None), - ("owner", Actor.OWNER, 401, None), - ("unrelated_same_org", Actor.UNRELATED_SAME_ORG, 401, None), - ("cross_org_user", Actor.CROSS_ORG_USER, 401, None), - ("service_account", Actor.SERVICE_ACCOUNT, 401, None), + ("team_admin", Actor.TEAM_ADMIN, 200, {"alpha"}), + ("internal_user", Actor.INTERNAL_USER, 200, {"alpha"}), + ("owner", Actor.OWNER, 200, {"alpha"}), + ("unrelated_same_org", Actor.UNRELATED_SAME_ORG, 200, {"alpha"}), + ("cross_org_user", Actor.CROSS_ORG_USER, 200, {"beta"}), + ("service_account", Actor.SERVICE_ACCOUNT, 200, {"alpha"}), ("org_b_admin", Actor.ORG_B_ADMIN, 200, {"beta"}), ] @@ -103,3 +104,14 @@ async def test_team_list_own_query( assert visible == set(expected_visible), ( f"{actor.value}: expected {sorted(expected_visible)}, " f"got {sorted(visible)}" ) + + +async def test_team_list_other_user_id_query_is_401(proxy_client, world): + """A regular user filtering by another user's user_id is still rejected 401.""" + resp = await proxy_client.get( + f"/team/list?user_id={world.keys[Actor.CROSS_ORG_USER].user_id}", + headers={ + "Authorization": f"Bearer {world.keys[Actor.INTERNAL_USER].cleartext}" + }, + ) + assert resp.status_code == 401, resp.text diff --git a/tests/proxy_behavior/management/test_team_list_v2.py b/tests/proxy_behavior/management/test_team_list_v2.py index 81178ad73c0..98bad1a3b66 100644 --- a/tests/proxy_behavior/management/test_team_list_v2.py +++ b/tests/proxy_behavior/management/test_team_list_v2.py @@ -37,21 +37,21 @@ async def _v2_team_ids(proxy_client, caller_cleartext: str, extra: str = "") -> page += 1 -# GET /v2/team/list is an info route reachable by every actor, but -# _enforce_list_team_v2_access still gates a BARE query: a proxy admin sees -# all teams, an org admin sees its orgs' teams, and a regular user — who has -# passed no user_id filter — is rejected 401 ("only admins can query all -# teams"). A regular user must scope the query to its own user_id. +# GET /v2/team/list is an info route reachable by every actor. +# _enforce_list_team_v2_access scopes a BARE query: a proxy admin sees all +# teams, an org admin sees its orgs' teams, and a regular user — who has +# passed no user_id filter — sees the teams it belongs to (LIT-5384: this +# used to be a 401, which broke the Admin UI key edit flow). _BARE = [ ("proxy_admin", Actor.PROXY_ADMIN, 200, frozenset({"alpha", "beta", "gamma"})), ("org_admin", Actor.ORG_ADMIN, 200, frozenset({"alpha", "gamma"})), ("org_b_admin", Actor.ORG_B_ADMIN, 200, frozenset({"beta"})), - ("team_admin", Actor.TEAM_ADMIN, 401, None), - ("internal_user", Actor.INTERNAL_USER, 401, None), - ("owner", Actor.OWNER, 401, None), - ("unrelated_same_org", Actor.UNRELATED_SAME_ORG, 401, None), - ("cross_org_user", Actor.CROSS_ORG_USER, 401, None), - ("service_account", Actor.SERVICE_ACCOUNT, 401, None), + ("team_admin", Actor.TEAM_ADMIN, 200, frozenset({"alpha"})), + ("internal_user", Actor.INTERNAL_USER, 200, frozenset({"alpha"})), + ("owner", Actor.OWNER, 200, frozenset({"alpha"})), + ("unrelated_same_org", Actor.UNRELATED_SAME_ORG, 200, frozenset({"alpha"})), + ("cross_org_user", Actor.CROSS_ORG_USER, 200, frozenset({"beta"})), + ("service_account", Actor.SERVICE_ACCOUNT, 200, frozenset({"alpha"})), ] diff --git a/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py index db39fcd3799..c07992690bb 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py @@ -3076,56 +3076,6 @@ async def test_bulk_team_member_add_no_db_connection(): assert "DB not connected" in str(exc_info.value.detail) -@pytest.mark.asyncio -async def test_list_team_v2_security_check_non_admin_user(): - """ - Test that list_team_v2 properly checks route permissions for non-admin users. - Non-admin users should only be able to query their own teams. - """ - from unittest.mock import AsyncMock, Mock, patch - - from fastapi import HTTPException, Request - - from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth - from litellm.proxy.management_endpoints.team_endpoints import list_team_v2 - - # Mock request - mock_request = Mock(spec=Request) - - # Test Case 1: Non-admin user trying to query all teams (user_id=None) - mock_user_api_key_dict_non_admin = UserAPIKeyAuth( - user_role=LitellmUserRoles.INTERNAL_USER, - user_id="non_admin_user_123", - ) - - with ( - patch("litellm.proxy.proxy_server.prisma_client") as mock_prisma_client, - patch("litellm.proxy.proxy_server.user_api_key_cache"), - patch("litellm.proxy.proxy_server.proxy_logging_obj"), - patch( - "litellm.proxy.management_endpoints.team_endpoints.get_user_object", - new_callable=AsyncMock, - return_value=None, - ), - ): - mock_prisma_client.return_value = MagicMock() # Mock non-None prisma client - - # Should raise HTTPException with 401 status - with pytest.raises(HTTPException) as exc_info: - await list_team_v2( - http_request=mock_request, - user_id=None, # Non-admin trying to query all teams - user_api_key_dict=mock_user_api_key_dict_non_admin, - status=None, - ) - - assert exc_info.value.status_code == 401 - assert "Only admin users can query all teams/other teams" in str( - exc_info.value.detail - ) - assert LitellmUserRoles.INTERNAL_USER.value in str(exc_info.value.detail) - - @pytest.mark.asyncio async def test_list_team_v2_security_check_non_admin_user_other_user(): """ @@ -8947,7 +8897,7 @@ async def test_list_team_v1_batches_key_queries(): patch( "litellm.proxy.management_endpoints.team_endpoints._authorize_and_filter_teams", new_callable=AsyncMock, - return_value=[team1, team2], + return_value=([team1, team2], None), ), patch( "litellm.proxy.management_endpoints.team_endpoints.get_all_team_memberships", @@ -12209,3 +12159,70 @@ async def test_invalidate_access_group_cache_deletes_the_cached_object(): "user_api_key_cache": cache, "proxy_logging_obj": logging_obj, } + + +@pytest.mark.asyncio +async def test_enforce_list_team_v2_access_scopes_bare_query_to_caller(): + """A non-admin listing teams without a user_id filter means "my teams" (LIT-5384), + and only an explicit request for another user's teams is rejected.""" + from litellm.proxy.management_endpoints.team_endpoints import ( + _enforce_list_team_v2_access, + ) + + caller = UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER, user_id="alice") + args = { + "organization_id": None, + "prisma_client": MagicMock(), + "user_api_key_cache": MagicMock(), + "proxy_logging_obj": MagicMock(), + } + + with patch( + "litellm.proxy.management_endpoints.team_endpoints._get_org_admin_org_ids", + new_callable=AsyncMock, + return_value=None, + ): + assert await _enforce_list_team_v2_access( + user_api_key_dict=caller, user_id=None, **args + ) == ("alice", None) + + with pytest.raises(HTTPException) as exc: + await _enforce_list_team_v2_access( + user_api_key_dict=caller, user_id="bob", **args + ) + + assert exc.value.status_code == 401 + + +@pytest.mark.asyncio +async def test_authorize_and_filter_teams_scopes_bare_query_to_caller(): + """/team/list without a user_id filter returns the caller's own teams rather than + 401'ing a non-admin (LIT-5384).""" + from litellm.proxy.management_endpoints.team_endpoints import ( + _authorize_and_filter_teams, + ) + + caller = UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER, user_id="alice") + prisma_client = MagicMock() + prisma_client.db.litellm_teamtable.find_many = AsyncMock(return_value=[]) + args = { + "prisma_client": prisma_client, + "user_api_key_cache": MagicMock(), + "proxy_logging_obj": MagicMock(), + } + + with patch( + "litellm.proxy.management_endpoints.team_endpoints.get_user_object", + new_callable=AsyncMock, + return_value=None, + ): + assert await _authorize_and_filter_teams( + user_api_key_dict=caller, user_id=None, **args + ) == ([], "alice") + + with pytest.raises(HTTPException) as exc: + await _authorize_and_filter_teams( + user_api_key_dict=caller, user_id="bob", **args + ) + + assert exc.value.status_code == 401