mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(proxy): allow unchanged key permissions on update
This commit is contained in:
parent
668df9494a
commit
6afdb5f21d
2 changed files with 80 additions and 60 deletions
|
|
@ -535,6 +535,7 @@ def _check_allowed_routes_caller_permission(
|
|||
user_api_key_dict: UserAPIKeyAuth,
|
||||
*,
|
||||
allowed_routes_was_provided: bool = False,
|
||||
allowed_routes_changed: bool | None = None,
|
||||
allow_safe_presets: bool = False,
|
||||
) -> None:
|
||||
"""
|
||||
|
|
@ -552,6 +553,8 @@ def _check_allowed_routes_caller_permission(
|
|||
carve-out below accepts any list of tokens in
|
||||
`_NON_ADMIN_SAFE_ALLOWED_ROUTES_PRESETS`.
|
||||
"""
|
||||
if allowed_routes_changed is False:
|
||||
return
|
||||
if not allowed_routes_was_provided and not allowed_routes:
|
||||
return
|
||||
if user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value:
|
||||
|
|
@ -576,6 +579,7 @@ def _check_allowed_routes_caller_permission(
|
|||
def _check_permissions_caller_permission(
|
||||
data: GenerateRequestBase,
|
||||
user_api_key_dict: UserAPIKeyAuth,
|
||||
permissions_changed: bool | None = None,
|
||||
) -> None:
|
||||
"""
|
||||
Require PROXY_ADMIN when `permissions` is present in the request body.
|
||||
|
|
@ -584,6 +588,8 @@ def _check_permissions_caller_permission(
|
|||
omits the field (default flows through) is distinct from one that
|
||||
sends any explicit value.
|
||||
"""
|
||||
if permissions_changed is False:
|
||||
return
|
||||
permissions_in_request = "permissions" in data.model_fields_set
|
||||
if not permissions_in_request and not data.permissions:
|
||||
return
|
||||
|
|
@ -2260,6 +2266,9 @@ async def _validate_update_key_data(
|
|||
allowed_routes=data.allowed_routes,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
allowed_routes_was_provided="allowed_routes" in data.model_fields_set,
|
||||
allowed_routes_changed=(
|
||||
"allowed_routes" in data.model_fields_set and data.allowed_routes != existing_key_row.allowed_routes
|
||||
),
|
||||
)
|
||||
_check_passthrough_routes_caller_permission(
|
||||
data=data,
|
||||
|
|
@ -2268,6 +2277,9 @@ async def _validate_update_key_data(
|
|||
_check_permissions_caller_permission(
|
||||
data=data,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
permissions_changed=(
|
||||
"permissions" in data.model_fields_set and data.permissions != existing_key_row.permissions
|
||||
),
|
||||
)
|
||||
|
||||
_validate_caller_can_change_key_ownership(
|
||||
|
|
|
|||
|
|
@ -11436,46 +11436,6 @@ class TestAllowedRoutesCallerPermission:
|
|||
assert str(exc_info.value.code) == "403"
|
||||
assert "allowed_routes" in str(exc_info.value.message)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_admin_update_key_explicit_empty_allowed_routes_rejected(self):
|
||||
"""`update_key_fn` rejects a non-admin when `allowed_routes` is
|
||||
present as `[]` in the request body. The value matches the model
|
||||
default but `model_fields_set` distinguishes the two."""
|
||||
from litellm.proxy.management_endpoints.key_management_endpoints import (
|
||||
update_key_fn,
|
||||
)
|
||||
|
||||
data = UpdateKeyRequest(key="sk-test", allowed_routes=[])
|
||||
assert "allowed_routes" in data.model_fields_set
|
||||
user_api_key_dict = UserAPIKeyAuth(
|
||||
user_id="internal-user-123",
|
||||
user_role=LitellmUserRoles.INTERNAL_USER,
|
||||
)
|
||||
mock_prisma_client = AsyncMock()
|
||||
|
||||
with (
|
||||
patch("litellm.proxy.proxy_server.prisma_client", mock_prisma_client),
|
||||
patch("litellm.proxy.proxy_server.user_api_key_cache", MagicMock()),
|
||||
patch("litellm.proxy.proxy_server.user_custom_key_update", None),
|
||||
patch("litellm.proxy.proxy_server.llm_router", None),
|
||||
patch("litellm.proxy.proxy_server.premium_user", True),
|
||||
patch("litellm.proxy.proxy_server.proxy_logging_obj", MagicMock()),
|
||||
patch(
|
||||
"litellm.proxy.management_endpoints.key_management_endpoints._get_and_validate_existing_key",
|
||||
new_callable=AsyncMock,
|
||||
return_value=MagicMock(),
|
||||
),
|
||||
):
|
||||
with pytest.raises(ProxyException) as exc_info:
|
||||
await update_key_fn(
|
||||
request=MagicMock(),
|
||||
data=data,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
litellm_changed_by=None,
|
||||
)
|
||||
assert str(exc_info.value.code) == "403"
|
||||
assert "allowed_routes" in str(exc_info.value.message)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_admin_update_key_explicit_null_allowed_routes_rejected(self):
|
||||
"""`update_key_fn` rejects a non-admin when `allowed_routes` is
|
||||
|
|
@ -14332,6 +14292,8 @@ def _make_personal_key_row_for_alice():
|
|||
max_budget=None,
|
||||
organization_id=None,
|
||||
project_id=None,
|
||||
allowed_routes=[],
|
||||
permissions={},
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -14370,33 +14332,79 @@ async def test_update_key_non_admin_permissions_non_empty_rejected(monkeypatch):
|
|||
assert "permissions" in str(exc.value.detail)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("data", "existing_allowed_routes", "existing_permissions"),
|
||||
[
|
||||
(
|
||||
UpdateKeyRequest(
|
||||
key="sk-alice-personal",
|
||||
tpm_limit=42,
|
||||
allowed_routes=[],
|
||||
),
|
||||
[],
|
||||
{},
|
||||
),
|
||||
(
|
||||
UpdateKeyRequest(
|
||||
key="sk-alice-personal",
|
||||
tpm_limit=42,
|
||||
allowed_routes=["llm_api_routes"],
|
||||
),
|
||||
["llm_api_routes"],
|
||||
{},
|
||||
),
|
||||
(
|
||||
UpdateKeyRequest(
|
||||
key="sk-alice-personal",
|
||||
tpm_limit=42,
|
||||
permissions={},
|
||||
),
|
||||
[],
|
||||
{},
|
||||
),
|
||||
(
|
||||
UpdateKeyRequest(
|
||||
key="sk-alice-personal",
|
||||
tpm_limit=42,
|
||||
permissions={"get_spend_routes": False},
|
||||
),
|
||||
[],
|
||||
{"get_spend_routes": False},
|
||||
),
|
||||
],
|
||||
)
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_key_non_admin_permissions_explicit_empty_rejected(monkeypatch):
|
||||
"""`_validate_update_key_data` rejects a non-admin when `permissions`
|
||||
is present as `{}` in the request body. The value matches the model
|
||||
default but `model_fields_set` distinguishes the two."""
|
||||
async def test_update_key_non_admin_can_resubmit_unchanged_privileged_fields(
|
||||
data,
|
||||
existing_allowed_routes,
|
||||
existing_permissions,
|
||||
monkeypatch,
|
||||
):
|
||||
mock_prisma_client = AsyncMock()
|
||||
mock_prisma_client.jsonify_object = lambda data: data
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma_client)
|
||||
|
||||
data = UpdateKeyRequest(
|
||||
key="sk-alice-personal",
|
||||
permissions={},
|
||||
existing_key_row = MagicMock(
|
||||
token="hashed_alice_personal_key",
|
||||
user_id="alice",
|
||||
team_id=None,
|
||||
created_by="alice",
|
||||
max_budget=None,
|
||||
organization_id=None,
|
||||
project_id=None,
|
||||
allowed_routes=existing_allowed_routes,
|
||||
permissions=existing_permissions,
|
||||
)
|
||||
assert "permissions" in data.model_fields_set
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
await _validate_update_key_data(
|
||||
data=data,
|
||||
existing_key_row=_make_personal_key_row_for_alice(),
|
||||
user_api_key_dict=_make_alice_internal_user(),
|
||||
llm_router=None,
|
||||
premium_user=True,
|
||||
prisma_client=mock_prisma_client,
|
||||
user_api_key_cache=MagicMock(),
|
||||
)
|
||||
assert exc.value.status_code == 403
|
||||
assert "permissions" in str(exc.value.detail)
|
||||
await _validate_update_key_data(
|
||||
data=data,
|
||||
existing_key_row=existing_key_row,
|
||||
user_api_key_dict=_make_alice_internal_user(),
|
||||
llm_router=None,
|
||||
premium_user=True,
|
||||
prisma_client=mock_prisma_client,
|
||||
user_api_key_cache=MagicMock(),
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue