mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-05 02:41:56 +00:00
fix(proxy): pass only a team admin's changed fields on to the team update
This commit is contained in:
parent
36eb9cdb35
commit
81ae5caa7e
4 changed files with 77 additions and 17 deletions
|
|
@ -35,6 +35,7 @@ _SETTINGS_LOCATION: Final = "Settings > UI > Team admin editable fields"
|
|||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class TeamAdminEditAllowed:
|
||||
request: UpdateTeamRequest
|
||||
kind: Literal["allowed"] = "allowed"
|
||||
|
||||
|
||||
|
|
@ -143,6 +144,15 @@ def changed_team_fields(data: UpdateTeamRequest, existing_row: LiteLLM_TeamTable
|
|||
return column_changes | _metadata_changes(data, submitted, existing)
|
||||
|
||||
|
||||
def _only_changes(data: UpdateTeamRequest, changed: frozenset[str]) -> UpdateTeamRequest:
|
||||
"""The request without the values it resends unchanged, which would otherwise still trigger derived writes
|
||||
such as a resent budget_duration pushing budget_reset_at back."""
|
||||
sent: Final = frozenset(data.model_fields_set)
|
||||
via_metadata: Final = frozenset({"metadata"}) if changed - sent else frozenset()
|
||||
kept: Final = frozenset({"team_id"}) | (changed & sent) | via_metadata
|
||||
return UpdateTeamRequest.model_validate(data.model_dump(include=MappingProxyType({field: True for field in kept})))
|
||||
|
||||
|
||||
def team_admin_edit_verdict(
|
||||
data: UpdateTeamRequest,
|
||||
existing: LiteLLM_TeamTable,
|
||||
|
|
@ -150,16 +160,17 @@ def team_admin_edit_verdict(
|
|||
) -> TeamAdminEditVerdict:
|
||||
if not permitted:
|
||||
return TeamAdminEditingDisabled()
|
||||
blocked: Final = sorted(changed_team_fields(data, existing) - permitted)
|
||||
changed: Final = changed_team_fields(data, existing)
|
||||
blocked: Final = sorted(changed - permitted)
|
||||
if blocked:
|
||||
return TeamAdminFieldNotPermitted(field=blocked[0])
|
||||
return TeamAdminEditAllowed()
|
||||
return TeamAdminEditAllowed(request=_only_changes(data, changed))
|
||||
|
||||
|
||||
def raise_for_team_admin_edit_verdict(verdict: TeamAdminEditVerdict) -> None:
|
||||
def team_admin_request_or_raise(verdict: TeamAdminEditVerdict) -> UpdateTeamRequest:
|
||||
match verdict:
|
||||
case TeamAdminEditAllowed():
|
||||
return
|
||||
case TeamAdminEditAllowed(request=request):
|
||||
return request
|
||||
case TeamAdminEditingDisabled():
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
|
|
|
|||
|
|
@ -140,9 +140,9 @@ from litellm.proxy.management_endpoints.tag_management_endpoints import (
|
|||
)
|
||||
from litellm.proxy.management_endpoints.team_admin_field_permissions import (
|
||||
SUPPORTED_TEAM_ADMIN_EDITABLE_TEAM_FIELDS,
|
||||
raise_for_team_admin_edit_verdict,
|
||||
resolve_team_admin_editable_fields,
|
||||
team_admin_edit_verdict,
|
||||
team_admin_request_or_raise,
|
||||
)
|
||||
from litellm.proxy.management_helpers.access_group_team_sync import (
|
||||
TEAM_ADVISORY_LOCK_SQL,
|
||||
|
|
@ -2218,7 +2218,7 @@ async def update_team(
|
|||
if access_role is None:
|
||||
_raise_team_access_denied()
|
||||
if access_role == "team_admin":
|
||||
raise_for_team_admin_edit_verdict(
|
||||
data = team_admin_request_or_raise( # rebind-ok: resent values must not reach the derived writes below
|
||||
team_admin_edit_verdict(
|
||||
data=data,
|
||||
existing=existing_team,
|
||||
|
|
|
|||
|
|
@ -7,9 +7,9 @@ from litellm.proxy.management_endpoints.team_admin_field_permissions import (
|
|||
TeamAdminEditingDisabled,
|
||||
TeamAdminFieldNotPermitted,
|
||||
changed_team_fields,
|
||||
raise_for_team_admin_edit_verdict,
|
||||
resolve_team_admin_editable_fields,
|
||||
team_admin_edit_verdict,
|
||||
team_admin_request_or_raise,
|
||||
)
|
||||
|
||||
_SUPPORTED = frozenset({"tpm_limit", "rpm_limit", "team_alias"})
|
||||
|
|
@ -96,10 +96,22 @@ class TestTeamAdminEditVerdict:
|
|||
verdict = team_admin_edit_verdict(UpdateTeamRequest(team_id="team-1"), _team(), frozenset())
|
||||
assert verdict == TeamAdminEditingDisabled()
|
||||
|
||||
def test_changes_within_permitted_fields_are_allowed(self):
|
||||
data = UpdateTeamRequest(team_id="team-1", tpm_limit=6, team_alias="alpha")
|
||||
verdict = team_admin_edit_verdict(data, _team(team_alias="alpha"), frozenset({"tpm_limit"}))
|
||||
assert verdict == TeamAdminEditAllowed()
|
||||
def test_allowed_request_keeps_only_the_changed_fields(self):
|
||||
data = UpdateTeamRequest(team_id="team-1", tpm_limit=6, team_alias="alpha", budget_duration="30d")
|
||||
existing = _team(team_alias="alpha", budget_duration="30d")
|
||||
verdict = team_admin_edit_verdict(data, existing, frozenset({"tpm_limit"}))
|
||||
assert isinstance(verdict, TeamAdminEditAllowed)
|
||||
assert verdict.request.model_dump(exclude_unset=True) == {"team_id": "team-1", "tpm_limit": 6}
|
||||
|
||||
def test_permitted_field_changed_inside_metadata_keeps_the_metadata(self):
|
||||
data = UpdateTeamRequest(team_id="team-1", metadata={"guardrails": ["b"]}, team_alias="alpha")
|
||||
existing = _team(team_alias="alpha", metadata={"guardrails": ["a"]})
|
||||
verdict = team_admin_edit_verdict(data, existing, frozenset({"guardrails"}))
|
||||
assert isinstance(verdict, TeamAdminEditAllowed)
|
||||
assert verdict.request.model_dump(exclude_unset=True) == {
|
||||
"team_id": "team-1",
|
||||
"metadata": {"guardrails": ["b"]},
|
||||
}
|
||||
|
||||
def test_first_blocked_field_in_sorted_order_is_reported(self):
|
||||
data = UpdateTeamRequest(team_id="team-1", tpm_limit=6, rpm_limit=6, blocked=True)
|
||||
|
|
@ -107,19 +119,20 @@ class TestTeamAdminEditVerdict:
|
|||
assert verdict == TeamAdminFieldNotPermitted(field="blocked")
|
||||
|
||||
|
||||
class TestRaiseForTeamAdminEditVerdict:
|
||||
def test_allowed_does_not_raise(self):
|
||||
assert raise_for_team_admin_edit_verdict(TeamAdminEditAllowed()) is None
|
||||
class TestTeamAdminRequestOrRaise:
|
||||
def test_allowed_hands_back_its_request(self):
|
||||
request = UpdateTeamRequest(team_id="team-1", tpm_limit=6)
|
||||
assert team_admin_request_or_raise(TeamAdminEditAllowed(request=request)) is request
|
||||
|
||||
def test_disabled_is_a_403_pointing_at_the_proxy_admin(self):
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
raise_for_team_admin_edit_verdict(TeamAdminEditingDisabled())
|
||||
team_admin_request_or_raise(TeamAdminEditingDisabled())
|
||||
assert exc.value.status_code == 403
|
||||
assert "cannot edit team settings" in exc.value.detail
|
||||
assert "Settings > UI > Team admin editable fields" in exc.value.detail
|
||||
|
||||
def test_field_not_permitted_is_a_403_naming_the_field(self):
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
raise_for_team_admin_edit_verdict(TeamAdminFieldNotPermitted(field="blocked"))
|
||||
team_admin_request_or_raise(TeamAdminFieldNotPermitted(field="blocked"))
|
||||
assert exc.value.status_code == 403
|
||||
assert "'blocked'" in exc.value.detail
|
||||
|
|
|
|||
|
|
@ -15078,6 +15078,42 @@ async def test_update_team_team_admin_changes_tpm_limit_once_a_proxy_admin_enabl
|
|||
assert "'rpm_limit'" in str(refused.value.message)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_team_team_admin_resending_budget_settings_does_not_push_back_budget_resets(
|
||||
disable_audit_logging_for_mocked_team,
|
||||
):
|
||||
"""A resent budget_duration or budget_limits would otherwise recompute the reset timestamps from now."""
|
||||
import contextlib
|
||||
|
||||
stored_windows = [{"budget_duration": "7d", "max_budget": 5.0, "reset_at": "2026-09-20T00:00:00Z"}]
|
||||
budgeted_team = MagicMock()
|
||||
budgeted_team.metadata = {}
|
||||
budgeted_team.model_dump.return_value = {
|
||||
"team_id": "test_team_id",
|
||||
"team_alias": "test_team",
|
||||
"metadata": {},
|
||||
"budget_duration": "30d",
|
||||
"budget_limits": stored_windows,
|
||||
"members_with_roles": [{"user_id": "team-admin", "role": "admin"}],
|
||||
}
|
||||
|
||||
with contextlib.ExitStack() as stack:
|
||||
prisma = _wire_update_team(stack, {})
|
||||
prisma.db.litellm_teamtable.find_unique = AsyncMock(return_value=budgeted_team)
|
||||
stack.enter_context(_team_admin_may_edit("tpm_limit"))
|
||||
await update_team(
|
||||
data=UpdateTeamRequest(
|
||||
team_id="test_team_id", tpm_limit=5000, budget_duration="30d", budget_limits=stored_windows
|
||||
),
|
||||
http_request=_update_request_stub(),
|
||||
user_api_key_dict=_TEAM_ADMIN_CALLER,
|
||||
)
|
||||
|
||||
written = prisma.db.litellm_teamtable.update.call_args.kwargs["data"]
|
||||
assert written["tpm_limit"] == 5000
|
||||
assert not {"budget_duration", "budget_reset_at", "budget_limits"} & written.keys()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_team_holds_a_team_admin_to_the_org_tpm_limit(disable_audit_logging_for_mocked_team):
|
||||
"""The org ceiling lives on the org's budget row, so /team/update must load it to enforce the cap."""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue