fix(scim): keep human email ownership consistent with local writes

litellm/proxy/management_endpoints/scim/human_provisioning.py

@@ -17,13 +17,12 @@ from prisma.types import (
 )
 from pydantic import TypeAdapter
 
-from litellm.proxy._types import LiteLLM_UserTable as UserPolicy
 from litellm.proxy._types import LitellmUserRoles
 from litellm.proxy.management_endpoints.internal_user_endpoints import check_user_license_capacity
 from litellm.proxy.utils import PrismaClient
 from litellm.repositories.base_repository import is_unique_violation
 from litellm.types.proxy.management_endpoints.scim_agent_provisioning import canonical_directory_id
-from litellm.types.proxy.management_endpoints.scim_v2 import SCIMPatchOp, SCIMPatchOperation, SCIMUser
+from litellm.types.proxy.management_endpoints.scim_v2 import SCIMPatchOp, SCIMPatchOperation, SCIMUser, SCIMUserEmail
 
 
 def human_email(user: SCIMUser) -> str:
@@ -43,7 +42,16 @@ def changes_readonly_attribute(operation: SCIMPatchOperation) -> bool:
     return any(attribute.startswith(("groups", "externalid")) for attribute in attributes)
 
 
+def changes_email_attribute(operation: SCIMPatchOperation) -> bool:
+    value: Final = operation.value
+    fields: Final = TypeAdapter(dict[str, object]).validate_python(value) if isinstance(value, dict) else {}
+    attributes: Final = (operation.path,) if operation.path else tuple(fields)
+    return any(attribute.casefold().split("[", 1)[0].split(".", 1)[0] == "emails" for attribute in attributes)
+
+
 def validate_human_patch(patch: SCIMPatchOp) -> None:
+    if any(changes_email_attribute(operation) for operation in patch.Operations):
+        raise HTTPException(400, "Use PUT to replace a provisioned human's email")
     if any(changes_readonly_attribute(operation) for operation in patch.Operations):
         raise HTTPException(400, "externalId is immutable; update group membership through this source's Groups")
 
@@ -165,19 +173,15 @@ class SourceHumanProvisioner:
             raise HTTPException(409, "The human local identity was removed; automatic recreation is not permitted")
         if isinstance(change, SCIMUser):
             await self.claim_email(row, human_email(change))
-        if isinstance(change, SCIMPatchOp):
-            async with self.client.tx() as tx:
-                current: Final = await tx.litellm_usertable.find_unique(where=local_filter)
-            if current is None:
-                raise HTTPException(409, "The human local identity was removed during provisioning")
-            preview, _ = scim_v2.apply_scim_user_patch(UserPolicy.model_validate(current.model_dump()), change)
-            email: Final = TypeAdapter[str | None](str | None).validate_python(preview.get("user_email"))
-            if email:
-                await self.claim_email(row, email.casefold())
         result: Final = (
             await scim_v2.patch_user(user_id=row.local_id, patch_ops=change)
             if isinstance(change, SCIMPatchOp)
-            else await scim_v2.update_user(user_id=row.local_id, user=change.model_copy(update={"groups": None}))
+            else await scim_v2.update_user(
+                user_id=row.local_id,
+                user=change.model_copy(
+                    update={"groups": None, "emails": [SCIMUserEmail(value=human_email(change), primary=True)]}
+                ),
+            )
         )
         document: Final = result.model_copy(update={"id": row.id, "externalId": row.external_id, "userName": username})
         resource_filter: Final[LiteLLM_SCIMResourceWhereUniqueInput] = {"id": row.id}

litellm/proxy/management_endpoints/scim/scim_v2.py

@@ -2133,7 +2133,7 @@ def _handle_generic_metadata(path: str, op_type: str, value: object, metadata: d
         metadata[path] = value
 
 
-def apply_scim_user_patch(
+def _apply_patch_ops(
     existing_user: LiteLLM_UserTable,
     patch_ops: SCIMPatchOp,
 ) -> tuple[dict[str, object], set[str]]:
@@ -2347,7 +2347,7 @@ async def patch_user(
 
         prev_active: Final = _user_scim_active(existing_user)
 
-        update_data, final_team_set = apply_scim_user_patch(
+        update_data, final_team_set = _apply_patch_ops(
             existing_user=existing_user,
             patch_ops=patch_ops,
         )

tests/test_litellm/proxy/management_endpoints/scim/test_human_provisioning.py

@@ -72,6 +72,81 @@ def test_human_ownership_email_uses_primary_and_normalizes_case(emails: object,
     assert human_email(user) == expected
 
 
+def test_missing_human_ownership_identifier_is_rejected() -> None:
+    with pytest.raises(HTTPException) as failure:
+        human_email(SCIMUser(schemas=[]))
+    assert failure.value.status_code == 400
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("missing", ["externalId", "userName"])
+async def test_direct_reservation_requires_complete_directory_identity(missing: str) -> None:
+    service, tx, _, user = human_fixture()
+    with pytest.raises(HTTPException) as failure:
+        await service.reserve(user.model_copy(update={missing: None}))
+    assert failure.value.status_code == 400
+    tx.litellm_scimresource.create.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_human_create_does_not_hide_storage_failure() -> None:
+    service, tx, _, user = human_fixture()
+    tx.litellm_scimresource.find_unique.side_effect = ConnectionError("unavailable")
+    with pytest.raises(ConnectionError, match="unavailable"):
+        await service.create(user)
+    tx.litellm_usertable.create.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_incomplete_human_record_cannot_be_updated() -> None:
+    service, tx, row, user = human_fixture()
+    with pytest.raises(HTTPException) as failure:
+        await service.update(row.model_copy(update={"local_id": None}), user)
+    assert failure.value.status_code == 409
+    tx.litellm_usertable.find_unique.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "path,value",
+    [
+        ("emails", [{"value": "NEW@example.com"}]),
+        ('emails[type eq "work"].value', "NEW@example.com"),
+        (None, {"emails": [{"value": "NEW@example.com"}]}),
+    ],
+)
+async def test_scoped_email_patch_requires_put_before_mutation(path: str | None, value: object) -> None:
+    service, tx, row, _ = human_fixture()
+    change: Final = SCIMPatchOp(Operations=[{"op": "replace", "path": path, "value": value}])
+    with pytest.raises(HTTPException, match="PUT") as failure:
+        await service.update(row, change)
+    assert failure.value.status_code == 400
+    tx.litellm_usertable.find_unique.assert_not_awaited()
+    tx.litellm_scimresource.update.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "emails", [None, [{"value": "FIRST@example.com"}, {"value": "PRIMARY@example.com", "primary": True}]]
+)
+async def test_human_put_claims_the_same_email_it_writes(emails: object, monkeypatch: pytest.MonkeyPatch) -> None:
+    service, tx, row, user = human_fixture()
+    tx.litellm_usertable.find_unique.return_value = SimpleNamespace(user_id=row.local_id)
+    change: Final = SCIMUser.model_validate({**user.model_dump(), "emails": emails})
+    expected: Final = human_email(change)
+
+    async def apply_update(*, user_id: str, user: SCIMUser) -> SCIMUser:
+        assert user_id == row.local_id
+        assert user.emails and user.emails[0].value == expected
+        tx.litellm_scimresource.update.assert_awaited_once_with(where={"id": row.id}, data={"human_email": expected})
+        return user
+
+    monkeypatch.setattr(scim_v2, "update_user", apply_update)
+    result: Final = await service.update(row, change)
+    assert result.id == row.id
+    assert result.emails and result.emails[0].value == expected
+
+
 @pytest.mark.asyncio
 async def test_reservation_replay_preserves_identity_before_creating_a_local_user() -> None:
     service, tx, row, user = human_fixture()
@@ -90,10 +165,14 @@ async def test_reservation_claims_email_subject_and_local_identity() -> None:
     assert data["human_email"] == user.userName
     assert data["human_subject_key"] == f"{TENANT}:{SUBJECT}"
     assert data["id"] == data["document"].data["id"]
-    tx.litellm_usertable.create.assert_awaited_once_with(data={
-        "user_id": user.userName, "user_email": user.userName,
-        "user_role": "internal_user_viewer", "teams": [],
-    })
+    tx.litellm_usertable.create.assert_awaited_once_with(
+        data={
+            "user_id": user.userName,
+            "user_email": user.userName,
+            "user_role": "internal_user_viewer",
+            "teams": [],
+        }
+    )
 
 
 @pytest.mark.asyncio
@@ -194,7 +273,7 @@ async def test_ownership_collision_is_a_conflict_before_legacy_user_mutation(
 
 
 @pytest.mark.asyncio
-async def test_human_patch_preserves_scim_id_and_claims_the_changed_email(monkeypatch: pytest.MonkeyPatch) -> None:
+async def test_human_patch_preserves_scim_id_and_updates_activity(monkeypatch: pytest.MonkeyPatch) -> None:
     from litellm.proxy._types import LiteLLM_UserTable
 
     service, tx, row, user = human_fixture()
@@ -291,7 +370,9 @@ async def test_email_update_cannot_claim_an_unrelated_local_human(monkeypatch: p
     update: Final = AsyncMock()
     monkeypatch.setattr(scim_v2, "update_user", update)
     with pytest.raises(HTTPException) as failure:
-        await service.update(row, SCIMUser.model_validate({**user.model_dump(), "emails": [{"value": "ADMIN@example.com"}]}))
+        await service.update(
+            row, SCIMUser.model_validate({**user.model_dump(), "emails": [{"value": "ADMIN@example.com"}]})
+        )
     assert failure.value.status_code == 409
     update.assert_not_awaited()
     tx.litellm_scimresource.update.assert_not_awaited()
@@ -322,3 +403,24 @@ async def test_replayed_human_does_not_consume_another_license_seat(monkeypatch:
     assert await service.reserve(user) == row
     license_check.is_over_limit.assert_not_called()
     tx.litellm_usertable.create.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_existing_native_agent_cannot_be_reclassified_as_human() -> None:
+    from litellm.types.proxy.management_endpoints.scim_v2 import SCIM_AGENT_USER_SCHEMA
+
+    service, tx, row, user = human_fixture()
+    native: Final = SCIMUser.model_validate(
+        {
+            **user.model_dump(),
+            SCIM_AGENT_USER_SCHEMA: {"identityParentId": TENANT},
+        }
+    )
+    tx.litellm_scimresource.find_unique.return_value = row.model_copy(
+        update={"document": native.model_dump(by_alias=True, mode="json")}
+    )
+    with pytest.raises(HTTPException) as failure:
+        await service.create(user)
+    assert failure.value.status_code == 409
+    tx.litellm_usertable.find_unique.assert_not_awaited()
+    tx.litellm_scimresource.update.assert_not_awaited()

tests/test_litellm/proxy/management_endpoints/scim/test_scim_patch_user.py

@@ -4,7 +4,7 @@ import pytest
 from fastapi import HTTPException
 
 from litellm.proxy._types import LiteLLM_UserTable
-from litellm.proxy.management_endpoints.scim.scim_v2 import apply_scim_user_patch, patch_user
+from litellm.proxy.management_endpoints.scim.scim_v2 import _apply_patch_ops, patch_user
 from litellm.types.proxy.management_endpoints.scim_v2 import (
     SCIMPatchOp,
     SCIMPatchOperation,
@@ -355,7 +355,7 @@ def test_apply_patch_ops_replace_entitlements_writes_canonical_key():
         ]
     )
 
-    update_data, _ = apply_scim_user_patch(
+    update_data, _ = _apply_patch_ops(
         existing_user=_user_with_metadata({}), patch_ops=patch_ops
     )
 
@@ -373,7 +373,7 @@ def test_apply_patch_ops_add_roles_appends_to_existing():
         ]
     )
 
-    update_data, _ = apply_scim_user_patch(
+    update_data, _ = _apply_patch_ops(
         existing_user=_user_with_metadata({"scim_roles": [{"value": "viewer"}]}),
         patch_ops=patch_ops,
     )
@@ -389,7 +389,7 @@ def test_apply_patch_ops_remove_entitlements_clears_canonical_key():
         Operations=[SCIMPatchOperation(op="remove", path="entitlements")]
     )
 
-    update_data, _ = apply_scim_user_patch(
+    update_data, _ = _apply_patch_ops(
         existing_user=_user_with_metadata(
             {"scim_entitlements": [{"value": "jira-software"}]}
         ),
@@ -409,7 +409,7 @@ def test_apply_patch_ops_pathless_value_dict_handles_roles():
         ]
     )
 
-    update_data, _ = apply_scim_user_patch(
+    update_data, _ = _apply_patch_ops(
         existing_user=_user_with_metadata({}), patch_ops=patch_ops
     )
 
@@ -428,7 +428,7 @@ def test_apply_patch_ops_invalid_entitlements_value_raises_400():
     )
 
     with pytest.raises(HTTPException) as exc_info:
-        apply_scim_user_patch(existing_user=_user_with_metadata({}), patch_ops=patch_ops)
+        _apply_patch_ops(existing_user=_user_with_metadata({}), patch_ops=patch_ops)
 
     assert exc_info.value.status_code == 400
 
@@ -442,7 +442,7 @@ def test_apply_patch_ops_replace_entitlements_without_value_member_is_stored_as_
         ]
     )
 
-    update_data, _ = apply_scim_user_patch(
+    update_data, _ = _apply_patch_ops(
         existing_user=_user_with_metadata({}), patch_ops=patch_ops
     )
 
@@ -455,7 +455,7 @@ def test_apply_patch_ops_add_without_value_raises_400_naming_value_member():
     )
 
     with pytest.raises(HTTPException) as exc_info:
-        apply_scim_user_patch(existing_user=_user_with_metadata({}), patch_ops=patch_ops)
+        _apply_patch_ops(existing_user=_user_with_metadata({}), patch_ops=patch_ops)
 
     assert exc_info.value.status_code == 400
     assert "value" in str(exc_info.value.detail)
@@ -473,7 +473,7 @@ def test_apply_patch_ops_filtered_path_raises_400_instead_of_junk_metadata():
     )
 
     with pytest.raises(HTTPException) as exc_info:
-        apply_scim_user_patch(
+        _apply_patch_ops(
             existing_user=_user_with_metadata(
                 {"scim_roles": [{"value": "engineering-admin"}]}
             ),
@@ -496,7 +496,7 @@ def test_apply_patch_ops_remove_group_filtered_path_without_value():
         Operations=[SCIMPatchOperation(op="remove", path='groups[value eq "team-1"]')]
     )
 
-    _, final_team_set = apply_scim_user_patch(existing_user=user, patch_ops=patch_ops)
+    _, final_team_set = _apply_patch_ops(existing_user=user, patch_ops=patch_ops)
 
     assert final_team_set == {"team-2"}
 
@@ -513,7 +513,7 @@ def test_apply_patch_ops_add_group_filtered_path_without_value():
         Operations=[SCIMPatchOperation(op="add", path="groups[value eq 'team-3']")]
     )
 
-    _, final_team_set = apply_scim_user_patch(existing_user=user, patch_ops=patch_ops)
+    _, final_team_set = _apply_patch_ops(existing_user=user, patch_ops=patch_ops)
 
     assert final_team_set == {"team-1", "team-3"}
 
@@ -533,6 +533,6 @@ def test_apply_patch_ops_replace_groups_empty_value_does_not_use_path_filter():
         ]
     )
 
-    _, final_team_set = apply_scim_user_patch(existing_user=user, patch_ops=patch_ops)
+    _, final_team_set = _apply_patch_ops(existing_user=user, patch_ops=patch_ops)
 
     assert final_team_set == set()