From 460d02ab2fbfd63268bc10aed2e163a29904c28f Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Thu, 27 Aug 2026 17:50:06 -0700 Subject: [PATCH 1/4] 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": {}}, From 128b74b5a3b342971449237b8c92c5af1004460e Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Thu, 27 Aug 2026 21:10:24 -0700 Subject: [PATCH 2/4] fix(management-v1): keep the user PATCH within the lint budgets and regenerate the specs The patch request and item models now annotate their collections read-only, and the merge-patch resolvers hand back MappingProxyType rather than plain dicts, so the only mutable dict left is the one the shared write path stamps into before Prisma takes it. The resolver seam is a Protocol instead of a Callable alias. Also regenerates _lazy_openapi_snapshot.json, which had drifted on the base branch, and schema.d.ts, which the new route belongs in. --- .../internal_user_endpoints.py | 26 ++- .../management_v1/users.py | 77 +++--- ui/litellm-dashboard/src/lib/http/schema.d.ts | 221 ++++++++++++++++++ 3 files changed, 282 insertions(+), 42 deletions(-) diff --git a/litellm/proxy/management_endpoints/internal_user_endpoints.py b/litellm/proxy/management_endpoints/internal_user_endpoints.py index be8b55ee6cb..9e5fc6e7aff 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, Callable, Mapping, Sequence +from collections.abc import Awaitable, Mapping, Sequence from datetime import datetime, timezone -from typing import Any, Final, Literal, TypeAlias, cast +from typing import Any, Final, Literal, Protocol, cast import fastapi from fastapi import APIRouter, Depends, Header, HTTPException, Request, status @@ -1181,13 +1181,20 @@ def _process_keys_for_user_info( return returned_keys -UserUpdateFieldResolver: TypeAlias = Callable[ - [dict, "UpdateUserRequest | UpdateUserRequestNoUserIDorEmail"], - dict, -] +class UserUpdateFieldResolver(Protocol): + """Decides which fields a user-update request writes; see `update_single_user`.""" + + def __call__( + self, + data_json: Mapping[str, object], + data: "UpdateUserRequest | UpdateUserRequestNoUserIDorEmail", + ) -> Mapping[str, object]: ... -def _update_internal_user_params(data_json: dict, data: UpdateUserRequest | UpdateUserRequestNoUserIDorEmail) -> dict: +def _update_internal_user_params( + data_json: Mapping[str, object], + data: UpdateUserRequest | UpdateUserRequestNoUserIDorEmail, +) -> dict: # mutable-ok: `/user/bulk_update` pops identity fields off this result before writing it """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 @@ -1284,7 +1291,7 @@ def _check_user_update_authz( # 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()) + "user_role" in (user_request.fields_set() if hasattr(user_request, "fields_set") else frozenset()) or user_request.user_role is not None ) if sends_role and user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN.value: @@ -1400,7 +1407,8 @@ async def update_single_user( ) data_json: Final[dict] = user_request.model_dump(exclude_unset=True) - non_default_values = resolve_fields(data_json, user_request) + resolved_fields: Final = resolve_fields(data_json, user_request) + non_default_values = dict(resolved_fields) # mutable-ok: the write path stamps into this before prisma takes it _hash_password_in_dict(non_default_values) existing_user_row: BaseModel | None = None diff --git a/litellm/proxy/management_endpoints/management_v1/users.py b/litellm/proxy/management_endpoints/management_v1/users.py index 4364f365549..3fecabed55a 100644 --- a/litellm/proxy/management_endpoints/management_v1/users.py +++ b/litellm/proxy/management_endpoints/management_v1/users.py @@ -1,6 +1,6 @@ """`PATCH /management/v1/users/{user_id}`.""" -from collections.abc import Callable, Mapping +from collections.abc import Callable, Mapping, Sequence from datetime import datetime from types import MappingProxyType from typing import TYPE_CHECKING, Annotated, Final, Literal @@ -48,6 +48,8 @@ _CLEARS_TO_EMPTY: Final[Mapping[str, Callable[[], object]]] = MappingProxyType( {"models": list, "metadata": dict, "model_max_budget": dict} ) +_NO_FIELDS: Final[Mapping[str, object]] = MappingProxyType({}) + class UserPatchRequest(BaseModel): """Body of `PATCH /management/v1/users/{user_id}`, read as an RFC 7396 JSON merge patch. @@ -70,14 +72,14 @@ class UserPatchRequest(BaseModel): ] | None ) = None - models: list[str] | None = None + models: Sequence[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 + metadata: Mapping[str, JsonValue] | None = None + model_max_budget: Mapping[str, JsonValue] | None = None object_permission: LiteLLM_ObjectPermissionBase | None = None @@ -94,7 +96,7 @@ class UserItem(BaseModel): user_email: str | None = None user_alias: str | None = None user_role: str | None = None - models: list[str] = Field(default_factory=list) + models: Sequence[str] = Field(default_factory=list) spend: float = 0.0 max_budget: float | None = None budget_duration: str | None = None @@ -102,18 +104,18 @@ class UserItem(BaseModel): 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) + metadata: Mapping[str, JsonValue] = Field(default_factory=dict) + model_max_budget: Mapping[str, JsonValue] = Field(default_factory=dict) object_permission_id: str | None = None - teams: list[str] = Field(default_factory=list) + teams: Sequence[str] = Field(default_factory=list) created_at: datetime | None = None updated_at: datetime | None = None def resolve_user_patch_fields( - data_json: dict, + data_json: Mapping[str, object], data: UpdateUserRequest | UpdateUserRequestNoUserIDorEmail, -) -> dict: +) -> Mapping[str, object]: """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. @@ -123,29 +125,36 @@ def resolve_user_patch_fields( `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)} + resolved: Final[Mapping[str, object]] = MappingProxyType( + { + 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) + } + ) + budget_duration: Final = resolved.get("budget_duration") + derived: Final = ( + _budget_reset_fields(budget_duration if isinstance(budget_duration, str) else None) + if "budget_duration" in resolved + else _NO_FIELDS + ) + return MappingProxyType({**resolved, **derived, **_internal_user_role_defaults(data, resolved)}) -def _budget_reset_fields(budget_duration: str | None) -> dict: +def _budget_reset_fields(budget_duration: str | None) -> Mapping[str, object]: """`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} + return MappingProxyType({"budget_reset_at": None}) validate_budget_duration(budget_duration) - return {"budget_reset_at": get_budget_reset_time(budget_duration=budget_duration)} + return MappingProxyType({"budget_reset_at": get_budget_reset_time(budget_duration=budget_duration)}) def _internal_user_role_defaults( data: UpdateUserRequest | UpdateUserRequestNoUserIDorEmail, - resolved: dict, -) -> dict: + resolved: Mapping[str, object], +) -> Mapping[str, object]: """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 @@ -153,19 +162,21 @@ def _internal_user_role_defaults( 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} + return _NO_FIELDS + budget: Final[Mapping[str, object]] = ( + MappingProxyType({"max_budget": litellm.max_internal_user_budget}) if "max_budget" not in resolved and litellm.max_internal_user_budget is not None - else {} + else _NO_FIELDS ) 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), - } + return MappingProxyType( + { + **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: @@ -199,7 +210,7 @@ def _problem_from_http_exception(exc: HTTPException) -> ManagementProblem: @router.patch( "/users/{user_id}", - tags=["Internal User management"], + tags=("Internal User management",), dependencies=(Depends(user_api_key_auth),), response_model=ItemResponse[UserItem], responses=problem_responses(403, 404, 422, 500, 503), @@ -277,4 +288,4 @@ async def patch_user( 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}) + return await table.find_first(where={"user_id": user_id}) # mutable-ok: prisma query filters are dict-shaped diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index f8f4a2b7059..c7c6e6bdc1d 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -8126,6 +8126,42 @@ export interface paths { patch?: never; trace?: never; }; + "/management/v1/users/{user_id}": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + /** + * Patch User + * @description 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}' + * ``` + */ + patch: operations["patch_user_management_v1_users__user_id__patch"]; + trace?: never; + }; "/mcp": { parameters: { query?: never; @@ -27267,6 +27303,10 @@ export interface components { /** Is Accepted */ is_accepted: boolean; }; + /** ItemResponse[UserItem] */ + ItemResponse_UserItem_: { + data: components["schemas"]["UserItem"]; + }; /** JWTKeyMappingResponse */ JWTKeyMappingResponse: { /** @@ -27294,6 +27334,7 @@ export interface components { /** Updated By */ updated_by?: string | null; }; + JsonValue: unknown; /** KeyHealthResponse */ KeyHealthResponse: { /** @@ -33095,6 +33136,25 @@ export interface components { */ version_status: string; }; + /** + * ProblemDetail + * @description RFC 9457 problem details, served as `application/problem+json`. + */ + ProblemDetail: { + /** + * Allowed + * @default null + */ + allowed: string[] | null; + /** Detail */ + detail: string; + /** Status */ + status: number; + /** Title */ + title: string; + /** Type */ + type: string; + }; /** Prompt */ Prompt: { litellm_params: components["schemas"]["PromptLiteLLMParams"]; @@ -37691,6 +37751,58 @@ export interface components { /** User Role */ user_role?: string | null; }; + /** + * UserItem + * @description 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. + */ + UserItem: { + /** Budget Duration */ + budget_duration?: string | null; + /** Budget Reset At */ + budget_reset_at?: string | null; + /** Created At */ + created_at?: string | null; + /** Max Budget */ + max_budget?: number | null; + /** Max Parallel Requests */ + max_parallel_requests?: number | null; + /** Metadata */ + metadata?: { + [key: string]: components["schemas"]["JsonValue"]; + }; + /** Model Max Budget */ + model_max_budget?: { + [key: string]: components["schemas"]["JsonValue"]; + }; + /** Models */ + models?: string[]; + /** Object Permission Id */ + object_permission_id?: string | null; + /** Rpm Limit */ + rpm_limit?: number | null; + /** + * Spend + * @default 0 + */ + spend: number; + /** Teams */ + teams?: string[]; + /** Tpm Limit */ + tpm_limit?: number | null; + /** Updated At */ + updated_at?: string | null; + /** User Alias */ + user_alias?: string | null; + /** User Email */ + user_email?: string | null; + /** User Id */ + user_id: string; + /** User Role */ + user_role?: string | null; + }; /** * UserListResponse * @description Response model for the user list endpoint @@ -37707,6 +37819,43 @@ export interface components { /** Users */ users: components["schemas"]["LiteLLM_UserTableWithKeyCount"][]; }; + /** + * UserPatchRequest + * @description 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. + */ + UserPatchRequest: { + /** Budget Duration */ + budget_duration?: string | null; + /** Max Budget */ + max_budget?: number | null; + /** Max Parallel Requests */ + max_parallel_requests?: number | null; + /** Metadata */ + metadata?: { + [key: string]: components["schemas"]["JsonValue"]; + } | null; + /** Model Max Budget */ + model_max_budget?: { + [key: string]: components["schemas"]["JsonValue"]; + } | null; + /** Models */ + models?: string[] | null; + object_permission?: components["schemas"]["LiteLLM_ObjectPermissionBase"] | null; + /** Rpm Limit */ + rpm_limit?: number | null; + /** Tpm Limit */ + tpm_limit?: number | null; + /** User Alias */ + user_alias?: string | null; + /** User Email */ + user_email?: string | null; + /** User Role */ + user_role?: ("proxy_admin" | "proxy_admin_viewer" | "internal_user" | "internal_user_viewer") | null; + }; /** * UserUpdateResult * @description Result of a single user update operation @@ -48772,6 +48921,78 @@ export interface operations { }; }; }; + patch_user_management_v1_users__user_id__patch: { + parameters: { + query?: never; + header?: never; + path: { + /** @description The id of the user to update. */ + user_id: string; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["UserPatchRequest"]; + }; + }; + responses: { + /** @description Successful Response */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ItemResponse_UserItem_"]; + }; + }; + /** @description Forbidden */ + 403: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["ProblemDetail"]; + }; + }; + /** @description Not found */ + 404: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["ProblemDetail"]; + }; + }; + /** @description Invalid request body */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["ProblemDetail"]; + }; + }; + /** @description Internal server error */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["ProblemDetail"]; + }; + }; + /** @description Database not connected */ + 503: { + headers: { + [name: string]: unknown; + }; + content: { + "application/problem+json": components["schemas"]["ProblemDetail"]; + }; + }; + }; + }; aggregate_mcp_route_mcp_get: { parameters: { query?: never; From 423563ed9c18764cc7a1d0ea63bacf393e343333 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 28 Aug 2026 13:11:07 -0700 Subject: [PATCH 3/4] test: patch the prisma client once per user test instead of once per collaborator The management_v1 fixture reached into UserRepository.table and the spend-counter helper to stand the route up. Seeding prisma_client.db is enough, since the repository reads the table off it, and the other two patches were doing nothing: the admin-name patch set the value the constant already had, and the counter only fires when the payload carries spend. The user_info_v2 tests each built their own prisma double, so a fixture now seeds the rows the group reads and the tests just say which users exist. --- .../management_v1/test_users.py | 21 ++- .../test_internal_user_endpoints.py | 124 +++++++++--------- 2 files changed, 74 insertions(+), 71 deletions(-) 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 index 2e66729194c..06cca8e376f 100644 --- a/tests/test_litellm/proxy/management_endpoints/management_v1/test_users.py +++ b/tests/test_litellm/proxy/management_endpoints/management_v1/test_users.py @@ -107,21 +107,20 @@ def caller(): @pytest.fixture def prisma(mocker): - """A prisma double whose `update_data` call is what the assertions inspect.""" + """A prisma double whose `update_data` call is what the assertions inspect. + + `UserRepository.table` reads straight off `prisma_client.db`, so seeding that is enough and the + repository itself stays real. The module global is the one seam the route has: it imports + `prisma_client` from `proxy_server` at call time. + """ + table = MagicMock() + table.find_first = AsyncMock(return_value=_user_row()) 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.db.litellm_usertable = table client.table = table + mocker.patch("litellm.proxy.proxy_server.prisma_client", client) return client 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 dbebf460d3e..d8aea3268c8 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 @@ -2589,8 +2589,32 @@ async def test_user_update_rejects_silent_create_for_non_proxy_admin(mocker): # ===================================================================== +@pytest.fixture +def user_rows(mocker): + """Seed the user table `/v2/user/info` reads, keyed by user_id, and return nothing for the rest. + + One seam for the whole `user_info_v2` group, so a test says which rows exist and nothing else. + """ + + def _seed(**rows): + prisma_client = mocker.MagicMock() + + async def find_unique(*args, **kwargs): + row = rows.get(kwargs.get("where", {}).get("user_id")) + if row is None: + return None + stub = mocker.MagicMock() + stub.model_dump.return_value = row + return stub + + prisma_client.db.litellm_usertable.find_unique = mocker.AsyncMock(side_effect=find_unique) + mocker.patch("litellm.proxy.proxy_server.prisma_client", prisma_client) + + return _seed + + @pytest.mark.asyncio -async def test_user_info_v2_proxy_admin_can_query_any_user(mocker): +async def test_user_info_v2_proxy_admin_can_query_any_user(mocker, user_rows): """ Test that proxy admin can query any user via /v2/user/info. """ @@ -2599,37 +2623,27 @@ async def test_user_info_v2_proxy_admin_can_query_any_user(mocker): from litellm.proxy._types import UserInfoV2Response from litellm.proxy.management_endpoints.internal_user_endpoints import user_info_v2 - mock_prisma_client = mocker.MagicMock() - - mock_user_row = mocker.MagicMock() - mock_user_row.model_dump.return_value = { - "user_id": "target-user-123", - "user_email": "target@example.com", - "user_alias": "Target User", - "user_role": "internal_user", - "spend": 42.5, - "max_budget": 100.0, - "models": ["gpt-4"], - "budget_duration": "30d", - "budget_reset_at": None, - "metadata": {"team": "engineering"}, - "created_at": datetime(2024, 1, 1, tzinfo=timezone.utc), - "updated_at": datetime(2024, 6, 1, tzinfo=timezone.utc), - "sso_user_id": "sso-abc", - "teams": ["team-1", "team-2"], - } - - async def mock_find_unique(*args, **kwargs): - if kwargs.get("where", {}).get("user_id") == "target-user-123": - return mock_user_row - return None - - mock_prisma_client.db.litellm_usertable.find_unique = mocker.AsyncMock( - side_effect=mock_find_unique + user_rows( + **{ + "target-user-123": { + "user_id": "target-user-123", + "user_email": "target@example.com", + "user_alias": "Target User", + "user_role": "internal_user", + "spend": 42.5, + "max_budget": 100.0, + "models": ["gpt-4"], + "budget_duration": "30d", + "budget_reset_at": None, + "metadata": {"team": "engineering"}, + "created_at": datetime(2024, 1, 1, tzinfo=timezone.utc), + "updated_at": datetime(2024, 6, 1, tzinfo=timezone.utc), + "sso_user_id": "sso-abc", + "teams": ["team-1", "team-2"], + } + } ) - mocker.patch("litellm.proxy.proxy_server.prisma_client", mock_prisma_client) - mock_request = mocker.MagicMock(spec=Request) admin_key = UserAPIKeyAuth( @@ -2744,7 +2758,7 @@ def test_build_user_info_response_redacts_scim_enterprise_metadata(): @pytest.mark.asyncio -async def test_user_info_v2_internal_user_can_query_self(mocker): +async def test_user_info_v2_internal_user_can_query_self(mocker, user_rows): """ Test that an internal user can query their own info. """ @@ -2753,37 +2767,27 @@ async def test_user_info_v2_internal_user_can_query_self(mocker): from litellm.proxy._types import UserInfoV2Response from litellm.proxy.management_endpoints.internal_user_endpoints import user_info_v2 - mock_prisma_client = mocker.MagicMock() - - mock_user_row = mocker.MagicMock() - mock_user_row.model_dump.return_value = { - "user_id": "self-user", - "user_email": "self@example.com", - "user_alias": None, - "user_role": "internal_user", - "spend": 10.0, - "max_budget": None, - "models": [], - "budget_duration": None, - "budget_reset_at": None, - "metadata": None, - "created_at": None, - "updated_at": None, - "sso_user_id": None, - "teams": [], - } - - async def mock_find_unique(*args, **kwargs): - if kwargs.get("where", {}).get("user_id") == "self-user": - return mock_user_row - return None - - mock_prisma_client.db.litellm_usertable.find_unique = mocker.AsyncMock( - side_effect=mock_find_unique + user_rows( + **{ + "self-user": { + "user_id": "self-user", + "user_email": "self@example.com", + "user_alias": None, + "user_role": "internal_user", + "spend": 10.0, + "max_budget": None, + "models": [], + "budget_duration": None, + "budget_reset_at": None, + "metadata": None, + "created_at": None, + "updated_at": None, + "sso_user_id": None, + "teams": [], + } + } ) - mocker.patch("litellm.proxy.proxy_server.prisma_client", mock_prisma_client) - mock_request = mocker.MagicMock(spec=Request) user_key = UserAPIKeyAuth( From 196468c69327dfaf5dc287a9d2e6582e1a3c65a8 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 28 Aug 2026 13:36:52 -0700 Subject: [PATCH 4/4] fix(management-v1): suppress the tags tuple type error on the user PATCH route FastAPI annotates tags as a list, so the tuple the route passes is a reportArgumentType error even though any sequence works. Suppressed rather than switched to a list literal, which LIT002 rejects. --- litellm/proxy/management_endpoints/management_v1/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/litellm/proxy/management_endpoints/management_v1/users.py b/litellm/proxy/management_endpoints/management_v1/users.py index 3fecabed55a..86df612d006 100644 --- a/litellm/proxy/management_endpoints/management_v1/users.py +++ b/litellm/proxy/management_endpoints/management_v1/users.py @@ -210,7 +210,7 @@ def _problem_from_http_exception(exc: HTTPException) -> ManagementProblem: @router.patch( "/users/{user_id}", - tags=("Internal User management",), + tags=("Internal User management",), # pyright: ignore[reportArgumentType] # annotated list, takes a sequence dependencies=(Depends(user_api_key_auth),), response_model=ItemResponse[UserItem], responses=problem_responses(403, 404, 422, 500, 503),