mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
address greptile review feedback (greploop iteration 2)
- Default user_id to caller's own ID for non-admins instead of 403 when omitted, preserving backward compatibility for API consumers - Apply same fix to aggregated endpoint - Update test to verify defaulting behavior instead of expecting 403 - Add useEffect to sync selectedUserId when auth state settles in UsagePageView to handle async auth initialization Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
310cca1578
commit
df5e8d01a6
3 changed files with 24 additions and 22 deletions
|
|
@ -1965,12 +1965,7 @@ async def get_user_daily_activity(
|
|||
entity_id = user_id # None means global view, otherwise filter by user
|
||||
else:
|
||||
if user_id is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail={
|
||||
"error": "Non-admin users must provide a user_id. Global spend view is restricted to admins."
|
||||
},
|
||||
)
|
||||
user_id = user_api_key_dict.user_id
|
||||
if user_id != user_api_key_dict.user_id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
|
|
@ -2067,12 +2062,7 @@ async def get_user_daily_activity_aggregated(
|
|||
entity_id = user_id # None means global view, otherwise filter by user
|
||||
else:
|
||||
if user_id is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail={
|
||||
"error": "Non-admin users must provide a user_id. Global spend view is restricted to admins."
|
||||
},
|
||||
)
|
||||
user_id = user_api_key_dict.user_id
|
||||
if user_id != user_api_key_dict.user_id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
|
|
|
|||
|
|
@ -1175,9 +1175,9 @@ async def test_get_user_daily_activity_non_admin_cannot_view_other_users(monkeyp
|
|||
"""
|
||||
Test that non-admin users cannot view another user's daily activity data.
|
||||
The endpoint should raise 403 when user_id does not match the caller's own user_id.
|
||||
Also verifies that omitting user_id entirely is forbidden for non-admins.
|
||||
Also verifies that omitting user_id defaults to the caller's own user_id.
|
||||
"""
|
||||
from unittest.mock import MagicMock
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
from fastapi import HTTPException
|
||||
|
||||
|
|
@ -1197,9 +1197,7 @@ async def test_get_user_daily_activity_non_admin_cannot_view_other_users(monkeyp
|
|||
user_role=LitellmUserRoles.INTERNAL_USER,
|
||||
)
|
||||
|
||||
# Case 1: Non-admin tries to view a different user's data
|
||||
# The inner 403 HTTPException is caught by the outer except block and
|
||||
# re-raised as a 500, but the original message is preserved in the detail.
|
||||
# Case 1: Non-admin tries to view a different user's data — should get 403
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await get_user_daily_activity(
|
||||
start_date="2025-01-01",
|
||||
|
|
@ -1218,9 +1216,14 @@ async def test_get_user_daily_activity_non_admin_cannot_view_other_users(monkeyp
|
|||
exc_info.value.detail
|
||||
)
|
||||
|
||||
# Case 2: Non-admin omits user_id entirely (global view is admin-only)
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await get_user_daily_activity(
|
||||
# Case 2: Non-admin omits user_id — should default to their own user_id
|
||||
mock_response = MagicMock()
|
||||
with patch(
|
||||
"litellm.proxy.management_endpoints.internal_user_endpoints.get_daily_activity",
|
||||
new_callable=AsyncMock,
|
||||
return_value=mock_response,
|
||||
) as mock_get_daily:
|
||||
result = await get_user_daily_activity(
|
||||
start_date="2025-01-01",
|
||||
end_date="2025-01-31",
|
||||
model=None,
|
||||
|
|
@ -1232,8 +1235,10 @@ async def test_get_user_daily_activity_non_admin_cannot_view_other_users(monkeyp
|
|||
user_api_key_dict=non_admin_key_dict,
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 403
|
||||
assert "Non-admin users must provide a user_id" in str(exc_info.value.detail)
|
||||
# Verify it called get_daily_activity with the caller's own user_id
|
||||
mock_get_daily.assert_called_once()
|
||||
call_kwargs = mock_get_daily.call_args
|
||||
assert call_kwargs.kwargs["entity_id"] == "regular-user-123"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -165,6 +165,13 @@ const UsagePage: React.FC<UsagePageProps> = ({ teams, organizations }) => {
|
|||
getAllTags();
|
||||
}, [accessToken]);
|
||||
|
||||
// Sync selectedUserId when auth state settles (isAdmin/userID may be null on initial render)
|
||||
useEffect(() => {
|
||||
if (!isAdmin && userID) {
|
||||
setSelectedUserId(userID);
|
||||
}
|
||||
}, [isAdmin, userID]);
|
||||
|
||||
// Derived states from userSpendData
|
||||
const totalSpend = userSpendData.metadata?.total_spend || 0;
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue