mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-15 04:19:08 +00:00
refactor(spend): model the request_id spend-log lookup as an explicit point lookup (LIT-3981)
The date-window relaxation for request_id lookups rode an apply_date_window flag threaded through the date validation and parsing. Model the two intents directly instead. A UI request_id query is a point lookup on the @id primary key that drops the time window and authorizes by row ownership; every other query, including the public /spend/logs/v2 route, takes the range-scan path that still requires a window Because the ownership check fully authorizes the single row, the general user/team scoping is now skipped for id lookups rather than layered on top redundantly. The confusing `is_v2 or request_id is None` guard is gone, and moving the date requirement into the range-scan branch lets the type checker narrow the dates it parses Behavior is preserved: the v2 contract still requires dates even when a request_id is supplied, and a non-owner is still rejected with 403. A regression test covers the non-admin owner id lookup, which resolves across all time and filters by the primary key alone
This commit is contained in:
parent
93ba298842
commit
7a85bf5320
2 changed files with 113 additions and 35 deletions
|
|
@ -1754,20 +1754,6 @@ async def ui_view_spend_logs(
|
|||
|
||||
is_v2 = "/spend/logs/v2" in get_request_route(request)
|
||||
|
||||
# request_id is the @id primary key, so it identifies a single row and needs
|
||||
# no time window. The internal UI always sends a 24h window, which is why a
|
||||
# log id copied from an older page could not be found; drop the window for an
|
||||
# id lookup on the UI route so it resolves across all time (LIT-3981).
|
||||
apply_date_window = is_v2 or request_id is None
|
||||
|
||||
if apply_date_window and (start_date is None or end_date is None):
|
||||
raise ProxyException(
|
||||
message="Start date and end date are required",
|
||||
type="bad_request",
|
||||
param="None",
|
||||
code=status.HTTP_400_BAD_REQUEST,
|
||||
)
|
||||
|
||||
# Validate sort_by and sort_order
|
||||
valid_sort_fields = {
|
||||
"spend",
|
||||
|
|
@ -1794,23 +1780,42 @@ async def ui_view_spend_logs(
|
|||
)
|
||||
|
||||
try:
|
||||
formats = ["%Y-%m-%d %H:%M:%S", "%Y-%m-%d"] if is_v2 else ["%Y-%m-%d %H:%M:%S"]
|
||||
is_admin_view = _is_admin_view_safe(user_api_key_dict=user_api_key_dict)
|
||||
is_request_id_lookup = request_id is not None and not is_v2
|
||||
|
||||
def parse_date(date_str: str) -> datetime:
|
||||
date_str = date_str.strip()
|
||||
for fmt in formats:
|
||||
try:
|
||||
return datetime.strptime(date_str, fmt).replace(tzinfo=timezone.utc)
|
||||
except ValueError:
|
||||
continue
|
||||
expected = "'YYYY-MM-DD' or 'YYYY-MM-DD HH:MM:SS'" if is_v2 else "'YYYY-MM-DD HH:MM:SS'"
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail=f"Invalid date format: {date_str}. Expected: {expected}",
|
||||
)
|
||||
if is_request_id_lookup:
|
||||
# request_id is the @id primary key: it identifies a single row, so a
|
||||
# time window is meaningless. The dashboard always sends a default 24h
|
||||
# window, which hid ids copied from an older page (LIT-3981). Drop the
|
||||
# window for the id lookup so it resolves across all time; every other
|
||||
# query, including the public v2 route, still requires one (below).
|
||||
start_date_obj: datetime | None = None
|
||||
end_date_obj: datetime | None = None
|
||||
else:
|
||||
if start_date is None or end_date is None:
|
||||
raise ProxyException(
|
||||
message="Start date and end date are required",
|
||||
type="bad_request",
|
||||
param="None",
|
||||
code=status.HTTP_400_BAD_REQUEST,
|
||||
)
|
||||
formats = ["%Y-%m-%d %H:%M:%S", "%Y-%m-%d"] if is_v2 else ["%Y-%m-%d %H:%M:%S"]
|
||||
|
||||
start_date_obj = parse_date(start_date) if apply_date_window else None
|
||||
end_date_obj = parse_date(end_date) if apply_date_window else None
|
||||
def parse_date(date_str: str) -> datetime:
|
||||
date_str = date_str.strip()
|
||||
for fmt in formats:
|
||||
try:
|
||||
return datetime.strptime(date_str, fmt).replace(tzinfo=timezone.utc)
|
||||
except ValueError:
|
||||
continue
|
||||
expected = "'YYYY-MM-DD' or 'YYYY-MM-DD HH:MM:SS'" if is_v2 else "'YYYY-MM-DD HH:MM:SS'"
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail=f"Invalid date format: {date_str}. Expected: {expected}",
|
||||
)
|
||||
|
||||
start_date_obj = parse_date(start_date)
|
||||
end_date_obj = parse_date(end_date)
|
||||
|
||||
# Build where conditions
|
||||
where_conditions: dict[str, Any] = {}
|
||||
|
|
@ -1887,18 +1892,19 @@ async def ui_view_spend_logs(
|
|||
where_conditions["spend"]["gte"] = min_spend
|
||||
if max_spend is not None:
|
||||
where_conditions["spend"]["lte"] = max_spend
|
||||
is_admin_view = _is_admin_view_safe(user_api_key_dict=user_api_key_dict)
|
||||
# The UI request_id lookup drops the date window above, so a non-admin could
|
||||
# otherwise reach any single row by id; require they own it, mirroring the
|
||||
# detail endpoint. Scoped to the UI route so the public v2 contract is unchanged.
|
||||
if request_id is not None and not is_admin_view and not is_v2:
|
||||
# A request_id lookup drops the date window, so a non-admin could otherwise
|
||||
# reach any single row by id; require they own it, mirroring the detail
|
||||
# endpoint. That ownership check fully authorizes the one row, so the
|
||||
# general scoping below is skipped for id lookups. Scoped to the UI route
|
||||
# so the public v2 contract is unchanged.
|
||||
if request_id is not None and not is_v2 and not is_admin_view:
|
||||
await _assert_user_can_view_request_id(
|
||||
prisma_client=prisma_client,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
request_id=request_id,
|
||||
)
|
||||
permitted_team_ids: List[str] | None = None
|
||||
if not is_admin_view:
|
||||
if not is_request_id_lookup and not is_admin_view:
|
||||
if team_id is not None:
|
||||
can_view_team = await _can_team_member_view_log(
|
||||
prisma_client=prisma_client,
|
||||
|
|
|
|||
|
|
@ -1640,6 +1640,78 @@ async def test_ui_view_spend_logs_request_id_blocks_non_owner(client, monkeypatc
|
|||
app.dependency_overrides.pop(ps.user_api_key_auth, None)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ui_view_spend_logs_request_id_owner_scoped_by_id_only(
|
||||
client, monkeypatch
|
||||
):
|
||||
"""A non-admin owner looking up their own request_id resolves across all time.
|
||||
The ownership check authorizes the single row, so the query drops both the date
|
||||
window and the general user/team scoping and filters by the primary key alone;
|
||||
without that skip an internal user would have a `user`/`OR` clause added."""
|
||||
today = datetime.datetime.now(timezone.utc)
|
||||
mock_spend_logs = [
|
||||
{
|
||||
"id": "log_old",
|
||||
"request_id": "req-old",
|
||||
"api_key": "sk-test-key",
|
||||
"user": "user_1",
|
||||
"team_id": "team1",
|
||||
"spend": 0.05,
|
||||
"startTime": (today - datetime.timedelta(days=90)).isoformat(),
|
||||
"model": "gpt-4",
|
||||
},
|
||||
]
|
||||
|
||||
captured: dict = {}
|
||||
|
||||
def filter_fn(where):
|
||||
captured["where"] = where
|
||||
rows = _filter_logs_by_date_range(mock_spend_logs, where)
|
||||
if where.get("request_id"):
|
||||
rows = [r for r in rows if r["request_id"] == where["request_id"]]
|
||||
return rows
|
||||
|
||||
mock_prisma = make_ui_spend_logs_mock_prisma(mock_spend_logs, filter_fn)
|
||||
|
||||
class _OwnedRow:
|
||||
user = "user_1"
|
||||
team_id = "team1"
|
||||
|
||||
async def _find_unique(where, include=None):
|
||||
return _OwnedRow()
|
||||
|
||||
mock_prisma.db.find_unique = _find_unique
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma)
|
||||
|
||||
# A 5-day window that EXCLUDES the 90-day-old log, as the dashboard sends.
|
||||
start_date = (today - datetime.timedelta(days=5)).strftime("%Y-%m-%d %H:%M:%S")
|
||||
end_date = today.strftime("%Y-%m-%d %H:%M:%S")
|
||||
|
||||
app.dependency_overrides[ps.user_api_key_auth] = lambda: UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.INTERNAL_USER, user_id="user_1"
|
||||
)
|
||||
try:
|
||||
response = client.get(
|
||||
"/spend/logs/ui",
|
||||
params={
|
||||
"request_id": "req-old",
|
||||
"start_date": start_date,
|
||||
"end_date": end_date,
|
||||
},
|
||||
headers={"Authorization": "Bearer sk-test"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["total"] == 1
|
||||
assert data["data"][0]["request_id"] == "req-old"
|
||||
assert "startTime" not in captured["where"]
|
||||
assert captured["where"]["request_id"] == "req-old"
|
||||
assert "user" not in captured["where"]
|
||||
assert "OR" not in captured["where"]
|
||||
finally:
|
||||
app.dependency_overrides.pop(ps.user_api_key_auth, None)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ui_view_spend_logs_unauthorized(client):
|
||||
# Test without authorization header
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue