mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
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>
This commit is contained in:
parent
7b6e16cfd3
commit
0c39090403
4 changed files with 122 additions and 87 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"})),
|
||||
]
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue