mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(scim): attribute SCIM-driven key deletions to a service account
SCIM provisioning calls like group member removal and user deletion ran team_member_delete and new_user with a bare UserAPIKeyAuth carrying only a role, so keys deleted in the cascade were archived with deleted_by NULL and showed 'Deleted By: -' in Deleted Keys. The SCIM endpoints now act under a litellm_scim service-account UserAPIKeyAuth, and the deleted-token transform falls back to the proxy admin name when the caller carries no user_id, which resolves deleted_by attribution for every internally triggered deletion. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
e0af9917a1
commit
ffa835a1e2
9 changed files with 146 additions and 9 deletions
|
|
@ -1659,6 +1659,7 @@ except (ValueError, TypeError):
|
|||
LITTELM_INTERNAL_HEALTH_SERVICE_ACCOUNT_NAME: Final = "litellm-internal-health-check"
|
||||
LITTELM_CLI_SERVICE_ACCOUNT_NAME: Final = "litellm-cli"
|
||||
LITELLM_INTERNAL_JOBS_SERVICE_ACCOUNT_NAME: Final = "litellm_internal_jobs"
|
||||
LITELLM_SCIM_SERVICE_ACCOUNT_NAME: Final = "litellm_scim"
|
||||
# Stable identifier substituted in place of the master key on UserAPIKeyAuth
|
||||
# objects so the master key (or its hash) never propagates to spend logs,
|
||||
# Prometheus metrics, audit trails, or any other downstream consumer.
|
||||
|
|
|
|||
|
|
@ -3408,6 +3408,24 @@ class UserAPIKeyAuth(LiteLLM_VerificationTokenView): # the expected response ob
|
|||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def get_litellm_scim_user_api_key_auth(cls) -> "UserAPIKeyAuth":
|
||||
"""
|
||||
Returns a `UserAPIKeyAuth` object for the SCIM provisioning endpoints.
|
||||
|
||||
This is used to track actions performed by SCIM provisioning on behalf of the IdP.
|
||||
"""
|
||||
from litellm.constants import LITELLM_SCIM_SERVICE_ACCOUNT_NAME
|
||||
|
||||
return cls(
|
||||
api_key=LITELLM_SCIM_SERVICE_ACCOUNT_NAME,
|
||||
team_id="system",
|
||||
key_alias=LITELLM_SCIM_SERVICE_ACCOUNT_NAME,
|
||||
team_alias="system",
|
||||
user_id=LITELLM_SCIM_SERVICE_ACCOUNT_NAME,
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
)
|
||||
|
||||
@property
|
||||
def is_team_service_account(self) -> bool:
|
||||
return (
|
||||
|
|
|
|||
|
|
@ -5011,6 +5011,8 @@ def _transform_verification_tokens_to_deleted_records(
|
|||
if not keys:
|
||||
return []
|
||||
|
||||
from litellm.proxy.proxy_server import litellm_proxy_admin_name
|
||||
|
||||
deleted_at: Final = datetime.now(timezone.utc)
|
||||
records: Final = []
|
||||
for key in keys:
|
||||
|
|
@ -5019,7 +5021,7 @@ def _transform_verification_tokens_to_deleted_records(
|
|||
{
|
||||
**key_payload,
|
||||
"deleted_at": deleted_at,
|
||||
"deleted_by": user_api_key_dict.user_id,
|
||||
"deleted_by": user_api_key_dict.user_id or litellm_proxy_admin_name,
|
||||
"deleted_by_api_key": user_api_key_dict.api_key,
|
||||
"litellm_changed_by": litellm_changed_by,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1133,7 +1133,7 @@ async def _create_user_if_not_exists(user_id: str, created_via: str = "scim_grou
|
|||
|
||||
created_user: Final = await new_user(
|
||||
data=new_user_request,
|
||||
user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN),
|
||||
user_api_key_dict=UserAPIKeyAuth.get_litellm_scim_user_api_key_auth(),
|
||||
)
|
||||
verbose_proxy_logger.info("Created user %s via %s", user_id, created_via)
|
||||
return created_user
|
||||
|
|
@ -1723,7 +1723,7 @@ async def create_user(
|
|||
|
||||
created_user: Final = await new_user(
|
||||
data=new_user_request,
|
||||
user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN),
|
||||
user_api_key_dict=UserAPIKeyAuth.get_litellm_scim_user_api_key_auth(),
|
||||
)
|
||||
|
||||
scim_user: Final = await ScimTransformations.transform_litellm_user_to_scim_user(user=created_user)
|
||||
|
|
@ -1861,7 +1861,7 @@ async def delete_user(
|
|||
if any(member.user_id == user_id for member in team_row.members_with_roles or []):
|
||||
await team_member_delete(
|
||||
data=TeamMemberDeleteRequest(team_id=team_row.team_id, user_id=user_id),
|
||||
user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN),
|
||||
user_api_key_dict=UserAPIKeyAuth.get_litellm_scim_user_api_key_auth(),
|
||||
)
|
||||
|
||||
await _set_user_keys_blocked(user_id=user_id, blocked=True)
|
||||
|
|
@ -2266,7 +2266,7 @@ async def _add_user_to_team(user_id: str, team_id: str) -> None:
|
|||
team_id=team_id,
|
||||
member=Member(user_id=user_id, role="user"),
|
||||
),
|
||||
user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN),
|
||||
user_api_key_dict=UserAPIKeyAuth.get_litellm_scim_user_api_key_auth(),
|
||||
)
|
||||
except ProxyException as e:
|
||||
if e.type != ProxyErrorTypes.team_member_already_in_team:
|
||||
|
|
@ -2278,7 +2278,7 @@ async def _remove_user_from_team(user_id: str, team_id: str) -> None:
|
|||
try:
|
||||
await team_member_delete(
|
||||
data=TeamMemberDeleteRequest(team_id=team_id, user_id=user_id),
|
||||
user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN),
|
||||
user_api_key_dict=UserAPIKeyAuth.get_litellm_scim_user_api_key_auth(),
|
||||
)
|
||||
except HTTPException as e:
|
||||
if not _is_user_not_in_team_error(e):
|
||||
|
|
@ -2583,7 +2583,7 @@ async def create_group(
|
|||
members_with_roles=members_with_roles,
|
||||
),
|
||||
http_request=Request(scope={"type": "http", "path": "/scim/v2/Groups"}),
|
||||
user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN),
|
||||
user_api_key_dict=UserAPIKeyAuth.get_litellm_scim_user_api_key_auth(),
|
||||
)
|
||||
|
||||
await _recompute_scim_member_roles(prisma_client, member_result.all_member_ids)
|
||||
|
|
|
|||
|
|
@ -58,6 +58,10 @@ from models import (
|
|||
OrgNewBody,
|
||||
OrgNewResponse,
|
||||
OrgUpdateBody,
|
||||
ScimGroupMemberValue,
|
||||
ScimGroupPatchBody,
|
||||
ScimGroupResponse,
|
||||
ScimPatchOperationBody,
|
||||
TagDeleteBody,
|
||||
TagListEntry,
|
||||
TagListResponse,
|
||||
|
|
@ -421,6 +425,26 @@ class ManagementClient:
|
|||
)
|
||||
)
|
||||
|
||||
def scim_remove_group_member(self, team_id: str, user_id: str) -> ScimGroupResponse:
|
||||
"""PATCH /scim/v2/Groups/{team_id} with a `remove` members operation, the
|
||||
membership-removal call a SCIM IdP makes when it drops a user from a group."""
|
||||
return unwrap(
|
||||
self.proxy.transport.patch(
|
||||
f"/scim/v2/Groups/{team_id}",
|
||||
headers=self.proxy.management_headers(),
|
||||
json=ScimGroupPatchBody(
|
||||
Operations=[
|
||||
ScimPatchOperationBody(
|
||||
op="remove",
|
||||
path="members",
|
||||
value=[ScimGroupMemberValue(value=user_id)],
|
||||
)
|
||||
]
|
||||
),
|
||||
response_type=ScimGroupResponse,
|
||||
)
|
||||
)
|
||||
|
||||
def create_user(self, body: UserNewBody) -> str:
|
||||
return unwrap(
|
||||
self.proxy.transport.post(
|
||||
|
|
|
|||
|
|
@ -900,6 +900,28 @@ class TestKeyDeletionAuditLog:
|
|||
_assert_key_deleted(client, created.key)
|
||||
_assert_single_deleted_row(_await_deleted_audit_rows(client, token), token)
|
||||
|
||||
@pytest.mark.covers("mgmt.team.member_delete.audit_logs_keys")
|
||||
def test_scim_group_member_removal_attributes_deleted_member_keys(
|
||||
self, client: ManagementClient, resources: ResourceManager
|
||||
) -> None:
|
||||
team_id = _create_team(client, resources, f"e2e-audit-team-{unique_marker()}", [])
|
||||
user_id = _create_user(
|
||||
client,
|
||||
resources,
|
||||
UserNewBody(user_email=f"e2e-audit-{unique_marker()}@example.com", user_role="internal_user"),
|
||||
)
|
||||
client.add_team_member(team_id, user_id)
|
||||
created = _generate_response(client, resources, KeyGenerateBody(user_id=user_id, team_id=team_id))
|
||||
token = _token_of(created)
|
||||
|
||||
client.scim_remove_group_member(team_id, user_id)
|
||||
|
||||
_assert_key_deleted(client, created.key)
|
||||
info = unwrap(client.key_info_as(created.key)).info
|
||||
assert info.status == "deleted", info
|
||||
assert isinstance(info.deleted_by, str) and info.deleted_by, info
|
||||
_assert_single_deleted_row(_await_deleted_audit_rows(client, token), token)
|
||||
|
||||
@pytest.mark.covers("mgmt.team.delete.audit_logs_keys")
|
||||
def test_team_delete_writes_audit_row_for_team_keys(
|
||||
self, client: ManagementClient, resources: ResourceManager
|
||||
|
|
|
|||
|
|
@ -164,6 +164,7 @@ class LiteLLMBudgetTable(BaseModel):
|
|||
class KeyInfo(BaseModel):
|
||||
key_alias: str | None = None
|
||||
status: str | None = None
|
||||
deleted_by: str | None = None
|
||||
metadata: KeyMetadata | None = None
|
||||
models: list[str] = []
|
||||
tpm_limit: int | None = None
|
||||
|
|
@ -1344,6 +1345,28 @@ class CredentialCreateResponse(BaseModel):
|
|||
success: bool
|
||||
|
||||
|
||||
# ---------- scim ----------
|
||||
|
||||
|
||||
class ScimGroupMemberValue(BaseModel):
|
||||
value: str
|
||||
|
||||
|
||||
class ScimPatchOperationBody(BaseModel):
|
||||
op: str
|
||||
path: str | None = None
|
||||
value: list[ScimGroupMemberValue] | None = None
|
||||
|
||||
|
||||
class ScimGroupPatchBody(BaseModel):
|
||||
schemas: list[str] = ["urn:ietf:params:scim:api:messages:2.0:PatchOp"]
|
||||
Operations: list[ScimPatchOperationBody]
|
||||
|
||||
|
||||
class ScimGroupResponse(BaseModel):
|
||||
id: str
|
||||
|
||||
|
||||
# ---------- key / team / user / organization management ----------
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -348,7 +348,7 @@ async def test_create_user_without_groups_defers_to_default_team(mocker: MockerF
|
|||
await create_user(user=scim_user)
|
||||
|
||||
assert new_user_mock.call_args.kwargs["data"].teams is None
|
||||
assert new_user_mock.call_args.kwargs["user_api_key_dict"] == UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN)
|
||||
assert new_user_mock.call_args.kwargs["user_api_key_dict"] == UserAPIKeyAuth.get_litellm_scim_user_api_key_auth()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -387,7 +387,7 @@ async def test_create_user_if_not_exists_defers_to_default_team(mocker: MockerFi
|
|||
|
||||
assert created is not None
|
||||
assert new_user_mock.call_args.kwargs["data"].teams is None
|
||||
assert new_user_mock.call_args.kwargs["user_api_key_dict"] == UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN)
|
||||
assert new_user_mock.call_args.kwargs["user_api_key_dict"] == UserAPIKeyAuth.get_litellm_scim_user_api_key_auth()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -1232,6 +1232,30 @@ async def test_handle_team_membership_changes_add_and_remove(mocker):
|
|||
assert call_args[1]["teams_ids_to_remove_user_from"] == ["team1"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_patch_team_membership_remove_attributes_scim_service_account(mocker):
|
||||
"""Removing a member via SCIM must run team_member_delete under the SCIM
|
||||
service-account identity so the cascaded key deletions record a deleted_by."""
|
||||
from litellm.constants import LITELLM_SCIM_SERVICE_ACCOUNT_NAME
|
||||
|
||||
mock_team_member_delete = mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2.team_member_delete",
|
||||
AsyncMock(),
|
||||
)
|
||||
|
||||
await patch_team_membership(
|
||||
user_id="uid",
|
||||
teams_ids_to_add_user_to=[],
|
||||
teams_ids_to_remove_user_from=["team-1"],
|
||||
raise_on_error=True,
|
||||
)
|
||||
|
||||
mock_team_member_delete.assert_awaited_once()
|
||||
auth = mock_team_member_delete.await_args.kwargs["user_api_key_dict"]
|
||||
assert auth.user_id == LITELLM_SCIM_SERVICE_ACCOUNT_NAME
|
||||
assert auth.user_role == LitellmUserRoles.PROXY_ADMIN
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_user_success(mocker):
|
||||
"""Should successfully update user with PUT request"""
|
||||
|
|
|
|||
|
|
@ -5307,6 +5307,29 @@ def test_transform_verification_tokens_to_deleted_records_empty_list():
|
|||
assert records == []
|
||||
|
||||
|
||||
def test_transform_verification_tokens_to_deleted_records_deleted_by_fallback():
|
||||
"""An identity-less auth (internal callers like SCIM roster sync that only carry
|
||||
a role) must still leave a non-empty deleted_by; a user_id-bearing auth keeps its own."""
|
||||
live_row = MagicMock()
|
||||
live_row.model_dump.return_value = {
|
||||
"token": "hashed-token-fallback",
|
||||
"user_id": "member-1",
|
||||
"team_id": "team-1",
|
||||
}
|
||||
|
||||
records = _transform_verification_tokens_to_deleted_records(
|
||||
keys=[live_row],
|
||||
user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN),
|
||||
)
|
||||
assert isinstance(records[0]["deleted_by"], str) and records[0]["deleted_by"]
|
||||
|
||||
records = _transform_verification_tokens_to_deleted_records(
|
||||
keys=[live_row],
|
||||
user_api_key_dict=UserAPIKeyAuth(user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN),
|
||||
)
|
||||
assert records[0]["deleted_by"] == "admin-1"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_save_deleted_verification_token_records():
|
||||
mock_prisma_client = AsyncMock()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue