From 460d02ab2fbfd63268bc10aed2e163a29904c28f Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Thu, 27 Aug 2026 17:50:06 -0700 Subject: [PATCH] feat(management-v1): add PATCH /management/v1/users/{user_id} with merge-patch semantics POST /user/update cannot clear a user setting. Its field resolution drops any null (and any [] or {}), so a request asking to remove a tpm limit gets a 200 back with the old value still in the database and nothing to say the clear was discarded. The filter is a leftover. It was written when the request was serialized with data.json(), which emitted every unset field at its non-None default (models=[], metadata={}, spend=0), so without it every update wiped them. exclude_unset=True has done that job since #10993, leaving the filter to swallow deliberate clears only, which is why max_budget needed its own fields_set carve-out to become clearable at all. Rather than change what /user/update means for existing callers, the clearable contract goes on a new control-plane route. An omitted field is left alone, an explicit null clears the setting, unknown body keys are refused with a 422 problem instead of ignored, and a user id that does not exist is a 404 rather than the silent create the underlying upsert would otherwise do. Only field resolution is new. Authorization, the self-escalation guard, metadata merging, the entitlement upsert, audit logging and cache invalidation stay in the one shared write path, now taking the resolver as a parameter, so the two surfaces cannot drift on the parts that matter. That path's role guard also moves from "user_role is not None" to presence, since a merge-patch caller clearing their own role to null would otherwise have slipped past it. Also adds the mutation machinery the surface was missing: an ItemResponse envelope, problem_responses() for declaring RFC 9457 failures in OpenAPI, and a 422 branch in the control plane's validation handler so a rejected body is no longer reported as a query-parameter fault. --- .../internal_user_endpoints.py | 42 ++- .../management_v1/__init__.py | 4 + .../management_v1/common.py | 68 ++++- .../management_v1/users.py | 280 ++++++++++++++++++ litellm/proxy/proxy_server.py | 26 +- .../management_endpoints/management_v1.py | 6 + .../management_v1/test_users.py | 252 ++++++++++++++++ .../test_internal_user_endpoints.py | 60 ++-- 8 files changed, 693 insertions(+), 45 deletions(-) create mode 100644 litellm/proxy/management_endpoints/management_v1/users.py create mode 100644 tests/test_litellm/proxy/management_endpoints/management_v1/test_users.py diff --git a/litellm/proxy/management_endpoints/internal_user_endpoints.py b/litellm/proxy/management_endpoints/internal_user_endpoints.py index 9a98bdbb6b1..be8b55ee6cb 100644 --- a/litellm/proxy/management_endpoints/internal_user_endpoints.py +++ b/litellm/proxy/management_endpoints/internal_user_endpoints.py @@ -15,9 +15,9 @@ These are members of a Team on LiteLLM import asyncio import json import traceback -from collections.abc import Awaitable, Mapping, Sequence +from collections.abc import Awaitable, Callable, Mapping, Sequence from datetime import datetime, timezone -from typing import Any, Final, Literal, cast +from typing import Any, Final, Literal, TypeAlias, cast import fastapi from fastapi import APIRouter, Depends, Header, HTTPException, Request, status @@ -1181,7 +1181,22 @@ def _process_keys_for_user_info( return returned_keys +UserUpdateFieldResolver: TypeAlias = Callable[ + [dict, "UpdateUserRequest | UpdateUserRequestNoUserIDorEmail"], + dict, +] + + def _update_internal_user_params(data_json: dict, data: UpdateUserRequest | UpdateUserRequestNoUserIDorEmail) -> dict: + """Legacy `/user/update` field resolution: a null (or `[]`/`{}`) means "not sent", so it is dropped. + + Retained verbatim for backwards compatibility. It dates to when `data_json` came from + `data.json()`, which serialized every unset field at its non-None default (`models=[]`, + `metadata={}`, `spend=0`), so without this filter every update wiped them. `exclude_unset=True` + has done that job since #10993, leaving the filter to do nothing but swallow deliberate clears, + which is why `max_budget` needed a `fields_set` carve-out to become clearable at all. New + surfaces should use the merge-patch resolver in `management_v1/users.py` instead. + """ non_default_values: Final = {} fields_set: Final = data.fields_set() if hasattr(data, "fields_set") else set() @@ -1266,7 +1281,13 @@ def _check_user_update_authz( existing_user_row: BaseModel | None, ) -> None: """Authorization checks for /user/update — raises HTTPException on failure.""" - if user_request.user_role is not None and user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN.value: + # Presence, not truthiness: a merge-patch caller clearing their own role to null would otherwise + # slip past a `is not None` check and demote themselves out of whatever an admin assigned. + sends_role: Final = ( + "user_role" in (user_request.fields_set() if hasattr(user_request, "fields_set") else set()) + or user_request.user_role is not None + ) + if sends_role and user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN.value: raise HTTPException(status_code=403, detail="Only proxy admins can modify user roles.") if existing_user_row is not None: @@ -1347,15 +1368,22 @@ async def _invalidate_cached_user_entitlement(user_id: str | None, object_permis verbose_proxy_logger.warning("Failed to invalidate cached entitlement key %r: %s", key, e) -async def _update_single_user_helper( +async def update_single_user( user_request: UpdateUserRequest, user_api_key_dict: UserAPIKeyAuth, litellm_changed_by: str | None = None, + resolve_fields: UserUpdateFieldResolver = _update_internal_user_params, ) -> dict[str, Any]: """ Helper function to update a single user. Used by both user_update and bulk_user_update endpoints. + Everything past field resolution is policy the two surfaces must share: authorization, the + self-escalation guard, metadata merging, entitlement upsert, audit logging and cache + invalidation. Only the question of *which* fields a request writes differs, so that is the one + step injected: `/user/update` keeps its legacy drop-nulls rule, while `/management/v1` passes a + merge-patch resolver where an explicit null clears. + Returns the updated user data or raises an exception on failure. """ from litellm.proxy.proxy_server import litellm_proxy_admin_name, prisma_client @@ -1372,7 +1400,7 @@ async def _update_single_user_helper( ) data_json: Final[dict] = user_request.model_dump(exclude_unset=True) - non_default_values = _update_internal_user_params(data_json=data_json, data=user_request) + non_default_values = resolve_fields(data_json, user_request) _hash_password_in_dict(non_default_values) existing_user_row: BaseModel | None = None @@ -1588,7 +1616,7 @@ async def user_update( try: verbose_proxy_logger.debug("/user/update: Received data = %s", data) - response: Final = await _update_single_user_helper( + response: Final = await update_single_user( user_request=data, user_api_key_dict=user_api_key_dict, ) @@ -1626,7 +1654,7 @@ async def bulk_update_processed_users( try: for user_request in users_to_update: try: - response = await _update_single_user_helper( + response = await update_single_user( user_request=user_request, user_api_key_dict=user_api_key_dict, litellm_changed_by=litellm_changed_by, diff --git a/litellm/proxy/management_endpoints/management_v1/__init__.py b/litellm/proxy/management_endpoints/management_v1/__init__.py index 342e6525cda..a2172162dae 100644 --- a/litellm/proxy/management_endpoints/management_v1/__init__.py +++ b/litellm/proxy/management_endpoints/management_v1/__init__.py @@ -10,9 +10,13 @@ from litellm.proxy.management_endpoints.management_v1.budgets import ( from litellm.proxy.management_endpoints.management_v1.spend_logs import ( router as spend_logs_router, ) +from litellm.proxy.management_endpoints.management_v1.users import ( + router as users_router, +) router: Final = APIRouter() router.include_router(budgets_router) router.include_router(spend_logs_router) +router.include_router(users_router) __all__ = ["router"] diff --git a/litellm/proxy/management_endpoints/management_v1/common.py b/litellm/proxy/management_endpoints/management_v1/common.py index 5ecaacbe170..f8f38d85736 100644 --- a/litellm/proxy/management_endpoints/management_v1/common.py +++ b/litellm/proxy/management_endpoints/management_v1/common.py @@ -3,6 +3,72 @@ The contract machinery every list route shares lives in `litellm.proxy.list_api`. """ -from typing import Final +from collections.abc import Mapping, MutableMapping +from types import MappingProxyType +from typing import Final, TypeAlias + +from litellm.proxy.list_api.common import PROBLEM_CONTENT_TYPE, PROBLEM_TYPE_BASE +from litellm.types.proxy.management_endpoints.management_v1 import ProblemDetail MANAGEMENT_V1_PREFIX: Final = "/management/v1" +PROBLEM_DETAIL_SCHEMA_NAME: Final = "ProblemDetail" +PROBLEM_DETAIL_REF: Final = f"#/components/schemas/{PROBLEM_DETAIL_SCHEMA_NAME}" + +_PROBLEM_TITLES: Final[Mapping[int, str]] = MappingProxyType( + { + 403: "Forbidden", + 404: "Not found", + 409: "Conflict", + 422: "Invalid request body", + 500: "Internal server error", + 503: "Database not connected", + } +) + + +def add_problem_detail_component( + openapi_schema: MutableMapping[str, object], # mutable-ok: fastapi's generated schema is a plain nested dict +) -> None: + """Register `ProblemDetail` as an OpenAPI component so `problem_responses()` can `$ref` it. + + FastAPI only emits components for models reachable from a route's `response_model`, and a problem + document never is one: it is the failure shape, declared out of band. + """ + components: Final = openapi_schema.setdefault("components", {}) # mutable-ok: seeds a branch of fastapi's own dict + if not isinstance(components, MutableMapping): + return + schemas: Final = components.setdefault("schemas", {}) # mutable-ok: seeds a branch of fastapi's own dict + if isinstance(schemas, MutableMapping): + schemas.setdefault(PROBLEM_DETAIL_SCHEMA_NAME, ProblemDetail.model_json_schema()) + + +# FastAPI declares `responses=` as a dict of dicts and rewrites copies of the entries as it renders +# the schema, so every layer below has to be a plain mutable dict. +ProblemResponses: TypeAlias = dict[int | str, dict[str, object]] # mutable-ok: fastapi's `responses=` contract + + +def problem_responses(*statuses: int) -> ProblemResponses: + """OpenAPI `responses=` entries declaring each status as an RFC 9457 problem document. + + Spelled as a raw `$ref` rather than `model=`, because FastAPI renders a `model=` entry under the + route's own media type and would document these as `application/json`. + """ + return {status: _problem_entry(status) for status in statuses} # mutable-ok: fastapi's `responses=` contract + + +def _problem_entry(status: int) -> dict[str, object]: # mutable-ok: fastapi's `responses=` contract + media_type: Final = {"schema": {"$ref": PROBLEM_DETAIL_REF}} # mutable-ok: fastapi's `responses=` contract + return { # mutable-ok: fastapi's `responses=` contract + "description": _PROBLEM_TITLES.get(status, "Error"), + "content": {PROBLEM_CONTENT_TYPE: media_type}, # mutable-ok: fastapi's `responses=` contract + } + + +def validation_problem(detail: str) -> ProblemDetail: + """A rejected request body, as opposed to `unknown_query_param_problem` for the query string.""" + return ProblemDetail( + type=f"{PROBLEM_TYPE_BASE}invalid-request-body", + title=_PROBLEM_TITLES[422], + status=422, + detail=detail, + ) diff --git a/litellm/proxy/management_endpoints/management_v1/users.py b/litellm/proxy/management_endpoints/management_v1/users.py new file mode 100644 index 00000000000..4364f365549 --- /dev/null +++ b/litellm/proxy/management_endpoints/management_v1/users.py @@ -0,0 +1,280 @@ +"""`PATCH /management/v1/users/{user_id}`.""" + +from collections.abc import Callable, Mapping +from datetime import datetime +from types import MappingProxyType +from typing import TYPE_CHECKING, Annotated, Final, Literal + +from fastapi import APIRouter, Depends, HTTPException, Path +from pydantic import BaseModel, ConfigDict, Field, JsonValue, TypeAdapter + +import litellm +from litellm._logging import verbose_proxy_logger +from litellm.proxy._types import ( + CommonProxyErrors, + LiteLLM_ObjectPermissionBase, + LitellmUserRoles, + UpdateUserRequest, + UpdateUserRequestNoUserIDorEmail, + UserAPIKeyAuth, +) +from litellm.proxy.auth.user_api_key_auth import user_api_key_auth +from litellm.proxy.list_api.common import PROBLEM_TYPE_BASE, ManagementProblem +from litellm.proxy.management_endpoints.common_utils import validate_budget_duration +from litellm.proxy.management_endpoints.management_v1.common import ( + MANAGEMENT_V1_PREFIX, + problem_responses, +) +from litellm.proxy.utils import PrismaClient +from litellm.repositories.prisma_protocols import TableActions +from litellm.repositories.user_repository import UserRepository +from litellm.types.proxy.management_endpoints.management_v1 import ( + ItemResponse, + ProblemDetail, +) + +if TYPE_CHECKING: + from prisma import models as prisma_models + +router: Final = APIRouter(prefix=MANAGEMENT_V1_PREFIX) + +# Starlette leaves `HTTPException.status_code` untyped; validate rather than widen the call site. +_HTTP_STATUS: Final = TypeAdapter(int) + +# Columns the schema declares NOT NULL with a default. A merge-patch null on one of these means +# "back to empty", which is the default, not SQL NULL; writing NULL would be rejected by the engine. +# Held as factories so each call gets its own empty container to hand to the query engine. +_CLEARS_TO_EMPTY: Final[Mapping[str, Callable[[], object]]] = MappingProxyType( + {"models": list, "metadata": dict, "model_max_budget": dict} +) + + +class UserPatchRequest(BaseModel): + """Body of `PATCH /management/v1/users/{user_id}`, read as an RFC 7396 JSON merge patch. + + Every field is optional and nullable, and the two are not the same thing: an omitted field is + left alone, an explicit `null` clears the setting. `extra="forbid"` is what makes that promise + keepable, since a misspelled key would otherwise read as "omitted" and silently do nothing. + """ + + model_config = ConfigDict(extra="forbid") + + user_email: str | None = None + user_alias: str | None = None + user_role: ( + Literal[ + LitellmUserRoles.PROXY_ADMIN, + LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY, + LitellmUserRoles.INTERNAL_USER, + LitellmUserRoles.INTERNAL_USER_VIEW_ONLY, + ] + | None + ) = None + models: list[str] | None = None + max_budget: float | None = None + budget_duration: str | None = None + tpm_limit: int | None = None + rpm_limit: int | None = None + max_parallel_requests: int | None = None + metadata: dict[str, JsonValue] | None = None + model_max_budget: dict[str, JsonValue] | None = None + object_permission: LiteLLM_ObjectPermissionBase | None = None + + +class UserItem(BaseModel): + """One internal user as the control plane returns it, read back off the row the write produced. + + Re-reading rather than echoing the request is the point of the endpoint: a caller can tell a + clear that landed from one that was dropped by looking at the response. + """ + + model_config = ConfigDict(from_attributes=True) + + user_id: str + user_email: str | None = None + user_alias: str | None = None + user_role: str | None = None + models: list[str] = Field(default_factory=list) + spend: float = 0.0 + max_budget: float | None = None + budget_duration: str | None = None + budget_reset_at: datetime | None = None + tpm_limit: int | None = None + rpm_limit: int | None = None + max_parallel_requests: int | None = None + metadata: dict[str, JsonValue] = Field(default_factory=dict) + model_max_budget: dict[str, JsonValue] = Field(default_factory=dict) + object_permission_id: str | None = None + teams: list[str] = Field(default_factory=list) + created_at: datetime | None = None + updated_at: datetime | None = None + + +def resolve_user_patch_fields( + data_json: dict, + data: UpdateUserRequest | UpdateUserRequestNoUserIDorEmail, +) -> dict: + """Merge-patch field resolution: whatever the caller sent is written, nulls included. + + The inverse of `_update_internal_user_params`, which drops nulls and so cannot express a clear. + Only three adjustments are made to the raw body, and each is a property of the schema rather + than a policy choice: NOT NULL columns clear to their default instead of SQL NULL, + `budget_reset_at` follows `budget_duration` because it is derived from it, and an + `object_permission` clear is left for the caller of this resolver, which drops the entitlement + link rather than writing a column. + """ + resolved: Final[dict] = { + key: (_CLEARS_TO_EMPTY[key]() if value is None and key in _CLEARS_TO_EMPTY else value) + for key, value in data_json.items() + if not (key == "object_permission" and value is None) + } + derived: Final = _budget_reset_fields(resolved["budget_duration"]) if "budget_duration" in resolved else {} + return {**resolved, **derived, **_internal_user_role_defaults(data, resolved)} + + +def _budget_reset_fields(budget_duration: str | None) -> dict: + """`budget_reset_at` is derived from `budget_duration`, so the two only ever move together.""" + from litellm.proxy.common_utils.timezone_utils import get_budget_reset_time + + if budget_duration is None: + return {"budget_reset_at": None} + validate_budget_duration(budget_duration) + return {"budget_reset_at": get_budget_reset_time(budget_duration=budget_duration)} + + +def _internal_user_role_defaults( + data: UpdateUserRequest | UpdateUserRequestNoUserIDorEmail, + resolved: dict, +) -> dict: + """Apply the proxy-wide internal-user budget caps, but only to fields the caller left alone. + + Mirrors `/user/update`, so promoting someone to internal user still lands them under + `max_internal_user_budget`. Guarded on absence rather than falsiness, so an explicit clear is + never overwritten by a global default the caller was trying to get out from under. + """ + if data.user_role != LitellmUserRoles.INTERNAL_USER: + return {} + budget: Final = ( + {"max_budget": litellm.max_internal_user_budget} + if "max_budget" not in resolved and litellm.max_internal_user_budget is not None + else {} + ) + if "budget_duration" in resolved or litellm.internal_user_budget_duration is None: + return budget + return { + **budget, + "budget_duration": litellm.internal_user_budget_duration, + **_budget_reset_fields(litellm.internal_user_budget_duration), + } + + +def _problem(status: int, slug: str, title: str, detail: str) -> ManagementProblem: + return ManagementProblem( + ProblemDetail(type=f"{PROBLEM_TYPE_BASE}{slug}", title=title, status=status, detail=detail) + ) + + +_PROBLEM_BY_STATUS: Final[Mapping[int, tuple[str, str]]] = MappingProxyType( + {403: ("forbidden", "Forbidden"), 404: ("user-not-found", "Not found")} +) + + +def _problem_from_http_exception(exc: HTTPException) -> ManagementProblem: + """Re-dress the shared write path's `HTTPException` as a problem document. + + `update_single_user` is shared with `/user/update` and raises that endpoint's error + shape. Translating here keeps the control plane's contract without forking the authorization + checks, which is the one thing that must not drift between the two surfaces. + """ + status: Final[int] = _HTTP_STATUS.validate_python(exc.status_code) + detail: Final[object] = exc.detail + slug, title = _PROBLEM_BY_STATUS.get(status, ("user-update-failed", "User update failed")) + return _problem( + status=status, + slug=slug, + title=title, + detail=str(detail.get("error", detail)) if isinstance(detail, Mapping) else str(detail), + ) + + +@router.patch( + "/users/{user_id}", + tags=["Internal User management"], + dependencies=(Depends(user_api_key_auth),), + response_model=ItemResponse[UserItem], + responses=problem_responses(403, 404, 422, 500, 503), +) +async def patch_user( + user_api_key_dict: Annotated[UserAPIKeyAuth, Depends(user_api_key_auth)], + body: UserPatchRequest, + user_id: Annotated[str, Path(description="The id of the user to update.")], +) -> ItemResponse[UserItem]: + """ + Partially update one internal user, as an RFC 7396 JSON merge patch. + + An omitted field is left alone and an explicit `null` clears the setting, which is the whole + reason this route exists: `POST /user/update` drops nulls, so it answers `200` to a clear it + silently discarded, and only `max_budget` was ever made clearable. Unknown body keys are + refused with a `422` rather than ignored. `null` on `models`, `metadata` or `model_max_budget` + resets the column to empty, since the schema declares those NOT NULL. + + Requires a proxy admin: the route is in no non-admin allowlist, so everyone else is refused at + the route gate, and the shared write path's self-service guards stand behind that as defense in + depth. Unlike `/user/update`, a user id that does not exist is a `404` rather than a silent + create, since the underlying write is an upsert. + + Example curl, clearing a rate limit and setting another: + ``` + curl --location --request PATCH 'http://0.0.0.0:4000/management/v1/users/user123' \ + --header 'Authorization: Bearer sk-1234' \ + --header 'Content-Type: application/json' \ + --data '{"tpm_limit": null, "rpm_limit": 60}' + ``` + """ + from litellm.proxy.management_endpoints.internal_user_endpoints import ( + update_single_user, + ) + from litellm.proxy.proxy_server import prisma_client + + if prisma_client is None: + raise _problem( + 503, + "database-not-connected", + "Database not connected", + CommonProxyErrors.db_not_connected_error.value, + ) + + # `update_data` upserts, so a missing row would otherwise be created by an admin's PATCH. + if await _find_user(prisma_client, user_id) is None: + raise _problem(404, "user-not-found", "Not found", f"No user with id {user_id!r} exists.") + + try: + await update_single_user( + user_request=UpdateUserRequest(user_id=user_id, **body.model_dump(exclude_unset=True)), + user_api_key_dict=user_api_key_dict, + resolve_fields=resolve_user_patch_fields, + ) + except ManagementProblem: + raise + except HTTPException as e: + raise _problem_from_http_exception(e) from e + except Exception as e: # noqa: BLE001 # a driver error answers as a problem document, not the OpenAI error shape + verbose_proxy_logger.exception( + "litellm.proxy.management_endpoints.management_v1.users.patch_user(): Exception occured - %s", e + ) + raise _problem(500, "internal-server-error", "Internal server error", str(e)) from e + + updated: Final = await _find_user(prisma_client, user_id) + if updated is None: + raise _problem( + 500, + "internal-server-error", + "Internal server error", + "The updated user could not be read back.", + ) + return ItemResponse[UserItem](data=UserItem.model_validate(updated)) + + +async def _find_user(prisma_client: PrismaClient, user_id: str) -> "prisma_models.LiteLLM_UserTable | None": + table: Final[TableActions[prisma_models.LiteLLM_UserTable]] = UserRepository(prisma_client).table + return await table.find_first(where={"user_id": user_id}) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 597acdf661f..a89c541cc13 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -493,7 +493,11 @@ from litellm.proxy.management_endpoints.key_management_endpoints import ( from litellm.proxy.management_endpoints.management_v1 import ( router as management_v1_router, ) -from litellm.proxy.management_endpoints.management_v1.common import MANAGEMENT_V1_PREFIX +from litellm.proxy.management_endpoints.management_v1.common import ( + MANAGEMENT_V1_PREFIX, + add_problem_detail_component, + validation_problem, +) from litellm.proxy.management_endpoints.model_access_group_management_endpoints import ( router as model_access_group_management_router, ) @@ -1525,6 +1529,9 @@ def get_openapi_schema(): openapi_schema = inject_lazy_stubs(openapi_schema, loaded_lazy_modules(app)) openapi_schema = ensure_unique_openapi_operation_ids(openapi_schema) + # `/management/v1` problem responses `$ref` this; nothing reachable from a response_model does. + add_problem_detail_component(openapi_schema) + # Fix Swagger UI execute path error when server_root_path is set if server_root_path: openapi_schema["servers"] = [{"url": "/" + server_root_path.strip("/")}] @@ -1708,17 +1715,22 @@ class _ValidationErrorDetail(TypedDict): @app.exception_handler(RequestValidationError) async def otel_request_validation_exception_handler(request: Request, exc: RequestValidationError): if request.url.path.startswith(MANAGEMENT_V1_PREFIX): - _close_dangling_otel_server_span(request, 400, exc=exc) validation_errors: Final[Sequence[_ValidationErrorDetail]] = exc.errors() + # A rejected body is a 422 and a rejected query string a 400; conflating them would report an + # unknown body key (which `extra="forbid"` mutation models refuse) as a query-parameter fault. + from_body: Final = any(error["loc"][:1] == ("body",) for error in validation_errors) + summary: Final = "; ".join( + f"{'.'.join(str(part) for part in error['loc'][1:])}: {error['msg']}" for error in validation_errors + ) + _close_dangling_otel_server_span(request, 422 if from_body else 400, exc=exc) return problem_response( - ProblemDetail( + validation_problem(summary or "The request body is invalid.") + if from_body + else ProblemDetail( type=f"{PROBLEM_TYPE_BASE}invalid-query-parameter", title="Invalid query parameter", status=400, - detail="; ".join( - f"{'.'.join(str(part) for part in error['loc'][1:])}: {error['msg']}" for error in validation_errors - ) - or "The request query parameters are invalid.", + detail=summary or "The request query parameters are invalid.", ) ) _close_dangling_otel_server_span(request, 422, exc=exc) diff --git a/litellm/types/proxy/management_endpoints/management_v1.py b/litellm/types/proxy/management_endpoints/management_v1.py index b2244f6eb9b..39ccffaf9ee 100644 --- a/litellm/types/proxy/management_endpoints/management_v1.py +++ b/litellm/types/proxy/management_endpoints/management_v1.py @@ -71,3 +71,9 @@ class ListResponse(BaseModel, Generic[TOut]): data: list[TOut] meta: ListMeta links: ListLinks + + +class ItemResponse(BaseModel, Generic[TOut]): + """Single-entity envelope. Shares `ListResponse`'s `data` key so a client unwraps both the same way.""" + + data: TOut diff --git a/tests/test_litellm/proxy/management_endpoints/management_v1/test_users.py b/tests/test_litellm/proxy/management_endpoints/management_v1/test_users.py new file mode 100644 index 00000000000..2e66729194c --- /dev/null +++ b/tests/test_litellm/proxy/management_endpoints/management_v1/test_users.py @@ -0,0 +1,252 @@ +"""`PATCH /management/v1/users/{user_id}`. + +The point of the endpoint is that an explicit `null` clears a setting, so most of what follows is +about telling "the caller sent null" apart from "the caller sent nothing" all the way down to the +values handed to the query engine. +""" + +from datetime import datetime, timezone +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest +from fastapi import FastAPI, Request +from fastapi.exceptions import RequestValidationError +from fastapi.testclient import TestClient +from pydantic import ValidationError + +import litellm +from litellm.proxy._types import ( + LitellmUserRoles, + UpdateUserRequest, + UserAPIKeyAuth, +) +from litellm.proxy.auth.user_api_key_auth import user_api_key_auth +from litellm.proxy.list_api.common import ManagementProblem, problem_response +from litellm.proxy.management_endpoints.internal_user_endpoints import ( + _update_internal_user_params, +) +from litellm.proxy.management_endpoints.management_v1 import router +from litellm.proxy.management_endpoints.management_v1.common import ( + MANAGEMENT_V1_PREFIX, + validation_problem, +) +from litellm.proxy.management_endpoints.management_v1.users import ( + UserPatchRequest, + resolve_user_patch_fields, +) + +app = FastAPI() + + +@app.exception_handler(ManagementProblem) +async def _management_problem_handler(request: Request, exc: ManagementProblem): + return problem_response(exc.problem) + + +@app.exception_handler(RequestValidationError) +async def _validation_handler(request: Request, exc: RequestValidationError): + from_body = any(error["loc"][:1] == ("body",) for error in exc.errors()) + if not from_body: + raise exc + return problem_response(validation_problem("; ".join(error["msg"] for error in exc.errors()))) + + +app.include_router(router) + +ADMIN = UserAPIKeyAuth(user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN.value) +TARGET_ID = "user-1" +PATCH_PATH = f"{MANAGEMENT_V1_PREFIX}/users/{TARGET_ID}" + + +def _resolve(**body: Any) -> dict[str, Any]: + """Run a patch body through the endpoint's field resolution, as the write path would.""" + request = UpdateUserRequest( + user_id=TARGET_ID, **UserPatchRequest.model_validate(body).model_dump(exclude_unset=True) + ) + return resolve_user_patch_fields(request.model_dump(exclude_unset=True), request) + + +def _user_row(**overrides: Any) -> MagicMock: + row = MagicMock() + row.model_dump.return_value = { + "user_id": TARGET_ID, + "user_email": "u1@example.com", + "user_alias": None, + "user_role": LitellmUserRoles.INTERNAL_USER.value, + "models": [], + "spend": 0.0, + "max_budget": None, + "budget_duration": None, + "budget_reset_at": None, + "tpm_limit": 500, + "rpm_limit": None, + "max_parallel_requests": None, + "metadata": {}, + "model_max_budget": {}, + "object_permission_id": None, + "teams": [], + "created_at": datetime(2026, 8, 1, tzinfo=timezone.utc), + "updated_at": datetime(2026, 8, 2, tzinfo=timezone.utc), + **overrides, + } + for key, value in row.model_dump.return_value.items(): + setattr(row, key, value) + return row + + +@pytest.fixture +def caller(): + """Whoever the route should authenticate as. Rebind `.value` inside a test to change it.""" + holder = MagicMock() + holder.value = ADMIN + app.dependency_overrides[user_api_key_auth] = lambda: holder.value + yield holder + app.dependency_overrides.clear() + + +@pytest.fixture +def prisma(mocker): + """A prisma double whose `update_data` call is what the assertions inspect.""" + client = MagicMock() + client.update_data = AsyncMock(return_value={"user_id": TARGET_ID, "data": _user_row()}) + client.get_data = AsyncMock(return_value=[]) + mocker.patch("litellm.proxy.proxy_server.prisma_client", client) + mocker.patch("litellm.proxy.proxy_server.litellm_proxy_admin_name", "default_user_id") + mocker.patch("litellm.proxy.proxy_server._invalidate_spend_counter", AsyncMock()) + table = MagicMock() + table.find_first = AsyncMock(return_value=_user_row()) + mocker.patch( + "litellm.repositories.user_repository.UserRepository.table", + new_callable=mocker.PropertyMock, + return_value=table, + ) + client.table = table + return client + + +def _written(prisma) -> dict[str, Any]: + assert prisma.update_data.await_count == 1, "expected exactly one user write" + return prisma.update_data.await_args.kwargs["data"] + + +# --- field resolution: null clears, absent is left alone ----------------------------------------- + + +def test_explicit_null_clears_a_nullable_column(): + """The bug this endpoint exists for: `/user/update` drops this null and answers 200 unchanged.""" + assert _resolve(tpm_limit=None)["tpm_limit"] is None + + +def test_omitted_field_is_not_written(): + resolved = _resolve(rpm_limit=60) + assert resolved["rpm_limit"] == 60 + assert "tpm_limit" not in resolved + assert "max_budget" not in resolved + + +@pytest.mark.parametrize( + ("field", "empty"), + [("models", []), ("metadata", {}), ("model_max_budget", {})], +) +def test_null_on_a_not_null_column_clears_to_empty(field, empty): + """These columns are NOT NULL in the schema, so the clear is the default, not SQL NULL.""" + assert _resolve(**{field: None})[field] == empty + + +def test_null_budget_duration_also_clears_the_reset_time(): + resolved = _resolve(budget_duration=None) + assert resolved["budget_duration"] is None + assert resolved["budget_reset_at"] is None + + +def test_setting_budget_duration_derives_a_reset_time(): + resolved = _resolve(budget_duration="30d") + assert resolved["budget_duration"] == "30d" + assert isinstance(resolved["budget_reset_at"], datetime) + + +def test_object_permission_null_is_left_for_the_entitlement_unlink(): + """Passing it through would make the shared path's upsert branch swallow the clear.""" + assert "object_permission" not in _resolve(object_permission=None) + + +def test_internal_user_default_budget_does_not_override_an_explicit_clear(monkeypatch): + monkeypatch.setattr(litellm, "max_internal_user_budget", 25.0) + assert _resolve(user_role=LitellmUserRoles.INTERNAL_USER, max_budget=None)["max_budget"] is None + + +def test_internal_user_default_budget_still_applies_when_unmentioned(monkeypatch): + monkeypatch.setattr(litellm, "max_internal_user_budget", 25.0) + assert _resolve(user_role=LitellmUserRoles.INTERNAL_USER)["max_budget"] == 25.0 + + +def test_legacy_user_update_still_drops_nulls(): + """The old contract is unchanged: callers relying on null-as-no-op are not broken by this PR.""" + request = UpdateUserRequest(user_id=TARGET_ID, tpm_limit=None, rpm_limit=60) + resolved = _update_internal_user_params(request.model_dump(exclude_unset=True), request) + assert "tpm_limit" not in resolved + assert resolved["rpm_limit"] == 60 + + +# --- request validation -------------------------------------------------------------------------- + + +def test_unknown_body_key_is_rejected(): + """Ignoring it would read as "omitted", i.e. a typo would silently do nothing.""" + with pytest.raises(ValidationError): + UserPatchRequest.model_validate({"tpm_limitt": 5}) + + +def test_unknown_body_key_answers_422_problem(caller, prisma): + response = TestClient(app).patch(PATCH_PATH, json={"tpm_limitt": 5}) + assert response.status_code == 422 + assert response.json()["type"].endswith("invalid-request-body") + assert prisma.update_data.await_count == 0 + + +# --- route behaviour ----------------------------------------------------------------------------- + + +def test_patch_writes_the_null_through_to_the_database(caller, prisma): + response = TestClient(app).patch(PATCH_PATH, json={"tpm_limit": None, "rpm_limit": 60}) + assert response.status_code == 200 + written = _written(prisma) + assert written["tpm_limit"] is None + assert written["rpm_limit"] == 60 + + +def test_patch_returns_the_row_read_back_after_the_write(caller, prisma): + body = TestClient(app).patch(PATCH_PATH, json={"rpm_limit": 60}).json() + assert body["data"]["user_id"] == TARGET_ID + assert body["data"]["tpm_limit"] == 500 + + +def test_missing_user_is_404_and_writes_nothing(caller, prisma): + prisma.table.find_first = AsyncMock(return_value=None) + response = TestClient(app).patch(PATCH_PATH, json={"rpm_limit": 60}) + assert response.status_code == 404 + assert response.json()["type"].endswith("user-not-found") + # `update_data` upserts, so a create here would be a silent user creation. + assert prisma.update_data.await_count == 0 + + +def test_non_admin_cannot_clear_their_own_role(caller, prisma): + caller.value = UserAPIKeyAuth(user_id=TARGET_ID, user_role=LitellmUserRoles.INTERNAL_USER.value) + response = TestClient(app).patch(PATCH_PATH, json={"user_role": None}) + assert response.status_code == 403 + assert prisma.update_data.await_count == 0 + + +def test_non_admin_cannot_clear_their_own_budget(caller, prisma): + caller.value = UserAPIKeyAuth(user_id=TARGET_ID, user_role=LitellmUserRoles.INTERNAL_USER.value) + response = TestClient(app).patch(PATCH_PATH, json={"max_budget": None}) + assert response.status_code == 403 + assert prisma.update_data.await_count == 0 + + +def test_non_admin_cannot_patch_someone_else(caller, prisma): + caller.value = UserAPIKeyAuth(user_id="other", user_role=LitellmUserRoles.INTERNAL_USER.value) + response = TestClient(app).patch(PATCH_PATH, json={"rpm_limit": 60}) + assert response.status_code == 403 + assert prisma.update_data.await_count == 0 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 8bce967b316..dbebf460d3e 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 @@ -1208,14 +1208,14 @@ async def test_new_user_admin_can_set_permissions(mocker): @pytest.mark.asyncio async def test_update_single_user_non_admin_permissions_rejected(mocker): - """`_update_single_user_helper` rejects a non-admin when `permissions` + """`update_single_user` rejects a non-admin when `permissions` is present in the request body. Covers both `/user/update` and `/user/bulk_update`, which share this helper.""" from fastapi import HTTPException from litellm.proxy._types import UpdateUserRequest from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = mocker.MagicMock() @@ -1231,7 +1231,7 @@ async def test_update_single_user_non_admin_permissions_rejected(mocker): ) with pytest.raises(HTTPException) as exc_info: - await _update_single_user_helper( + await update_single_user( user_request=data, user_api_key_dict=caller ) assert exc_info.value.status_code == 403 @@ -1240,13 +1240,13 @@ async def test_update_single_user_non_admin_permissions_rejected(mocker): @pytest.mark.asyncio async def test_update_single_user_non_admin_permissions_explicit_empty_rejected(mocker): - """`_update_single_user_helper` rejects a non-admin when `permissions` + """`update_single_user` rejects a non-admin when `permissions` is present as `{}` in the request body.""" from fastapi import HTTPException from litellm.proxy._types import UpdateUserRequest from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = mocker.MagicMock() @@ -1260,7 +1260,7 @@ async def test_update_single_user_non_admin_permissions_explicit_empty_rejected( ) with pytest.raises(HTTPException) as exc_info: - await _update_single_user_helper( + await update_single_user( user_request=data, user_api_key_dict=caller ) assert exc_info.value.status_code == 403 @@ -2557,7 +2557,7 @@ async def test_user_update_rejects_silent_create_for_non_proxy_admin(mocker): from litellm.proxy._types import UpdateUserRequest, UserAPIKeyAuth from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = mocker.MagicMock() @@ -2578,7 +2578,7 @@ async def test_user_update_rejects_silent_create_for_non_proxy_admin(mocker): ) with pytest.raises(HTTPException) as exc: - await _update_single_user_helper( + await update_single_user( user_request=user_request, user_api_key_dict=org_admin ) assert exc.value.status_code == 404 @@ -3436,7 +3436,7 @@ async def test_ghsa_wvg4_non_admin_cannot_self_escalate_max_budget(mocker): from fastapi import HTTPException from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = mocker.MagicMock() @@ -3461,7 +3461,7 @@ async def test_ghsa_wvg4_non_admin_cannot_self_escalate_max_budget(mocker): ) with pytest.raises(HTTPException) as exc: - await _update_single_user_helper( + await update_single_user( user_request=user_request, user_api_key_dict=caller ) assert exc.value.status_code == 403 @@ -3474,7 +3474,7 @@ async def test_ghsa_wvg4_non_admin_cannot_self_escalate_spend(mocker): from fastapi import HTTPException from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = mocker.MagicMock() @@ -3499,7 +3499,7 @@ async def test_ghsa_wvg4_non_admin_cannot_self_escalate_spend(mocker): ) with pytest.raises(HTTPException) as exc: - await _update_single_user_helper( + await update_single_user( user_request=user_request, user_api_key_dict=caller ) assert exc.value.status_code == 403 @@ -3510,7 +3510,7 @@ async def test_ghsa_wvg4_non_admin_cannot_self_escalate_spend(mocker): async def test_ghsa_wvg4_proxy_admin_can_update_user_budget(mocker): """PROXY_ADMIN must still be able to modify another user's budget.""" from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = mocker.MagicMock() @@ -3539,7 +3539,7 @@ async def test_ghsa_wvg4_proxy_admin_can_update_user_budget(mocker): user_role=LitellmUserRoles.PROXY_ADMIN, ) - result = await _update_single_user_helper( + result = await update_single_user( user_request=user_request, user_api_key_dict=admin_caller ) assert result is not None @@ -3550,7 +3550,7 @@ async def test_admin_user_update_spend_invalidates_counter(mocker): """A direct /user/update spend change must invalidate the cross-pod spend counter so enforcement re-reads the new DB value.""" from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = mocker.MagicMock() @@ -3581,7 +3581,7 @@ async def test_admin_user_update_spend_invalidates_counter(mocker): user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN ) - await _update_single_user_helper( + await update_single_user( user_request=user_request, user_api_key_dict=admin_caller ) mock_invalidate.assert_awaited_once_with(counter_key="spend:user:target-user") @@ -3593,7 +3593,7 @@ async def test_user_update_rejects_non_finite_spend(mocker): from fastapi import HTTPException from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = mocker.MagicMock() @@ -3617,7 +3617,7 @@ async def test_user_update_rejects_non_finite_spend(mocker): ) with pytest.raises(HTTPException) as exc: - await _update_single_user_helper( + await update_single_user( user_request=user_request, user_api_key_dict=admin_caller ) assert exc.value.status_code == 400 @@ -3871,7 +3871,7 @@ async def test_user_update_persists_mcp_entitlement_and_links_it(mocker): would not even be a column. """ from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = _object_permission_mocks(mocker) @@ -3879,7 +3879,7 @@ async def test_user_update_persists_mcp_entitlement_and_links_it(mocker): cache.async_delete_cache = mocker.AsyncMock() mocker.patch("litellm.proxy.proxy_server.user_api_key_cache", cache) - await _update_single_user_helper( + await update_single_user( user_request=UpdateUserRequest( user_id="target-user", object_permission={ @@ -3910,7 +3910,7 @@ async def test_user_update_invalidates_the_cached_entitlement(mocker): (which carries a "no entitlement" sentinel), and the cached user row. """ from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) _object_permission_mocks(mocker) @@ -3918,7 +3918,7 @@ async def test_user_update_invalidates_the_cached_entitlement(mocker): cache.async_delete_cache = mocker.AsyncMock() mocker.patch("litellm.proxy.proxy_server.user_api_key_cache", cache) - await _update_single_user_helper( + await update_single_user( user_request=UpdateUserRequest( user_id="target-user", object_permission={"mcp_tool_permissions": {"github": []}}, @@ -3948,7 +3948,7 @@ async def test_admin_can_clear_a_users_mcp_entitlement(mocker): the gateway would keep enforcing the cleared grants until the cache expired. """ from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = _object_permission_mocks(mocker, "perm-existing") @@ -3956,7 +3956,7 @@ async def test_admin_can_clear_a_users_mcp_entitlement(mocker): cache.async_delete_cache = mocker.AsyncMock() mocker.patch("litellm.proxy.proxy_server.user_api_key_cache", cache) - await _update_single_user_helper( + await update_single_user( user_request=UpdateUserRequest(user_id="target-user", object_permission={}), user_api_key_dict=UserAPIKeyAuth( user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN @@ -3984,7 +3984,7 @@ async def test_user_update_invalidates_both_the_old_and_new_permission_rows(mock grants, so anything still resolving that id keeps reading them. """ from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) _object_permission_mocks(mocker, "perm-existing") @@ -3992,7 +3992,7 @@ async def test_user_update_invalidates_both_the_old_and_new_permission_rows(mock cache.async_delete_cache = mocker.AsyncMock() mocker.patch("litellm.proxy.proxy_server.user_api_key_cache", cache) - await _update_single_user_helper( + await update_single_user( user_request=UpdateUserRequest( user_id="target-user", object_permission={"mcp_tool_permissions": {"github": ["list_issues"]}}, @@ -4019,7 +4019,7 @@ async def test_non_admin_cannot_clear_their_own_mcp_entitlement(mocker): from fastapi import HTTPException from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = _object_permission_mocks(mocker, "perm-existing") @@ -4028,7 +4028,7 @@ async def test_non_admin_cannot_clear_their_own_mcp_entitlement(mocker): mocker.patch("litellm.proxy.proxy_server.user_api_key_cache", cache) with pytest.raises(HTTPException) as exc: - await _update_single_user_helper( + await update_single_user( user_request=UpdateUserRequest(user_id="target-user", object_permission={}), user_api_key_dict=UserAPIKeyAuth( user_id="target-user", user_role=LitellmUserRoles.INTERNAL_USER @@ -4046,7 +4046,7 @@ async def test_non_admin_cannot_rewrite_their_own_mcp_entitlement(mocker): from fastapi import HTTPException from litellm.proxy.management_endpoints.internal_user_endpoints import ( - _update_single_user_helper, + update_single_user, ) mock_prisma_client = _object_permission_mocks(mocker, "perm-existing") @@ -4055,7 +4055,7 @@ async def test_non_admin_cannot_rewrite_their_own_mcp_entitlement(mocker): mocker.patch("litellm.proxy.proxy_server.user_api_key_cache", cache) with pytest.raises(HTTPException) as exc: - await _update_single_user_helper( + await update_single_user( user_request=UpdateUserRequest( user_id="target-user", object_permission={"mcp_servers": [], "mcp_tool_permissions": {}},