From eea292abbabed6a9b4bb86630c60efa74fd7e9a5 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Wed, 5 Aug 2026 23:24:37 -0700 Subject: [PATCH] fix(proxy): allow non-admins to reach /user/daily/activity/aggregated The aggregated route was missing from LiteLLMRoutes.self_managed_routes while its paginated sibling /user/daily/activity was listed, so auth rejected every internal user with a 401 before the handler ran. That route backs the default "Your Usage" view in the dashboard, which left the main Usage page broken for non-admin users. The handler already self-scopes: it checks admin view first, then falls back to require_caller_user_id_for_non_admin, defaults a missing user_id to the caller's own, and returns 403 when a non-admin asks for someone else's data. Listing the route restores reachability without widening what a caller can read. check_route_access matches exactly (plus explicit wildcards), so the parent entry never covered the /aggregated sub-path. --- litellm/proxy/_types.py | 1 + .../proxy/auth/test_route_checks.py | 52 ++++++++++++++ .../test_internal_user_endpoints.py | 69 +++++++++++++++++++ 3 files changed, 122 insertions(+) diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 7bc8ed59a6a..5b1134650f2 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -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", diff --git a/tests/test_litellm/proxy/auth/test_route_checks.py b/tests/test_litellm/proxy/auth/test_route_checks.py index 9285b997efc..0bfb10320f7 100644 --- a/tests/test_litellm/proxy/auth/test_route_checks.py +++ b/tests/test_litellm/proxy/auth/test_route_checks.py @@ -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"], + ) diff --git a/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py index aab9a0b4fd0..056c2d3657a 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py @@ -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): """