Merge pull request #36062 from BerriAI/litellm_/lucid-pike-b1ee0e

fix(proxy): allow non-admins to reach /user/daily/activity/aggregated
This commit is contained in:
yuneng-jiang 2026-08-06 10:54:20 -07:00 committed by GitHub
commit 63c639f18b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 122 additions and 0 deletions

View file

@ -781,6 +781,7 @@ class LiteLLMRoutes(enum.Enum):
"/model/update",
"/model/delete",
"/user/daily/activity",
"/user/daily/activity/aggregated",
"/user/available_roles", # read-only role metadata; any authenticated user may read
"/user/list", # org admins checked in endpoint; non-admins get 403
"/model/{model_id}/update",

View file

@ -3246,3 +3246,55 @@ def test_internal_user_still_blocked_from_another_users_info():
assert exc_info.value.status_code == 403
assert "key not allowed to access this user's info" in str(exc_info.value.detail)
@pytest.mark.parametrize(
"route",
[
"/user/daily/activity",
"/user/daily/activity/aggregated",
],
)
@pytest.mark.parametrize(
"user_role",
[
LitellmUserRoles.INTERNAL_USER.value,
LitellmUserRoles.INTERNAL_USER_VIEW_ONLY.value,
],
)
def test_user_daily_activity_routes_reachable_by_non_admin(route, user_role):
"""Both /user/daily/activity and its /aggregated sibling power the default
"Your Usage" dashboard view, and both handlers self-scope to the caller
(_user_has_admin_view -> require_caller_user_id_for_non_admin -> 403 on a
user_id mismatch). self_managed_routes is the ONLY list that grants either
route to a non-admin, so dropping one from it 401s every internal user's
main Usage page before the handler ever runs.
"""
user_obj = LiteLLM_UserTable(
user_id="test_user",
user_email="test@example.com",
user_role=user_role,
)
valid_token = UserAPIKeyAuth(user_id="test_user", user_role=user_role)
request = MagicMock(spec=Request)
request.query_params = {}
RouteChecks.non_proxy_admin_allowed_routes_check(
user_obj=user_obj,
_user_role=user_role,
route=route,
request=request,
valid_token=valid_token,
request_data={},
)
def test_user_daily_activity_aggregated_not_covered_by_prefix_match():
"""check_route_access is exact-match plus explicit wildcards, so listing the
parent /user/daily/activity does not implicitly cover the /aggregated
sub-path. Pins the reason the sibling needs its own entry.
"""
assert not RouteChecks.check_route_access(
route="/user/daily/activity/aggregated",
allowed_routes=["/user/daily/activity"],
)

View file

@ -2294,6 +2294,75 @@ async def test_get_user_daily_activity_aggregated_admin_global_view(monkeypatch)
)
@pytest.mark.asyncio
async def test_get_user_daily_activity_aggregated_non_admin_cannot_view_other_users(
monkeypatch,
):
"""
Same scoping contract as
test_get_user_daily_activity_non_admin_cannot_view_other_users, on the
aggregated route. Non-admins reach this handler now that the route is in
self_managed_routes, so the 403-on-mismatch and default-to-self behaviour
has to hold here too: opening the route must not widen access.
"""
from unittest.mock import AsyncMock, MagicMock, patch
from fastapi import HTTPException
from litellm.proxy.management_endpoints.internal_user_endpoints import (
get_user_daily_activity_aggregated,
)
mock_prisma_client = MagicMock()
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma_client)
non_admin_key_dict = UserAPIKeyAuth(
user_id="regular-user-123",
user_role=LitellmUserRoles.INTERNAL_USER,
)
# Case 1: Non-admin targets another user's data — 403, helper never reached
with patch(
"litellm.proxy.management_endpoints.internal_user_endpoints.get_daily_activity_aggregated",
new_callable=AsyncMock,
) as mock_get_daily_agg:
with pytest.raises(HTTPException) as exc_info:
await get_user_daily_activity_aggregated(
start_date="2025-01-01",
end_date="2025-01-31",
model=None,
api_key=None,
user_id="other-user-456",
timezone=None,
user_api_key_dict=non_admin_key_dict,
)
assert exc_info.value.status_code == 403
assert "Non-admin users can only view their own spend data" in str(exc_info.value.detail)
mock_get_daily_agg.assert_not_called()
# Case 2: Non-admin omits user_id — scoped to their own user_id, not global
mock_response = MagicMock()
with patch(
"litellm.proxy.management_endpoints.internal_user_endpoints.get_daily_activity_aggregated",
new_callable=AsyncMock,
return_value=mock_response,
) as mock_get_daily_agg:
result = await get_user_daily_activity_aggregated(
start_date="2025-01-01",
end_date="2025-01-31",
model=None,
api_key=None,
user_id=None,
timezone=None,
user_api_key_dict=non_admin_key_dict,
)
assert result is mock_response
mock_get_daily_agg.assert_called_once()
assert mock_get_daily_agg.call_args.kwargs["entity_id"] == "regular-user-123"
@pytest.mark.asyncio
async def test_delete_user_cleans_up_created_by_invitation_links(mocker):
"""