mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-08 22:21:35 +00:00
fix(scim): preserve team memberships on PUT /Users when groups is omitted
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
31ca4ddf32
commit
faebe31469
2 changed files with 94 additions and 4 deletions
|
|
@ -1762,6 +1762,10 @@ async def update_user(
|
|||
client_set_active: Final = "active" in user.model_fields_set
|
||||
scim_active_for_metadata: Final = user_data["active"] if client_set_active else prev_active
|
||||
|
||||
client_set_groups: Final = "groups" in user.model_fields_set
|
||||
existing_teams: Final = list(existing_user.teams or [])
|
||||
new_teams: Final = user_data["teams"] if client_set_groups else existing_teams
|
||||
|
||||
metadata: Final = _build_scim_metadata(
|
||||
user_data["given_name"],
|
||||
user_data["family_name"],
|
||||
|
|
@ -1773,20 +1777,20 @@ async def update_user(
|
|||
|
||||
await _handle_team_membership_changes(
|
||||
user_id=user_id,
|
||||
existing_teams=existing_user.teams or [],
|
||||
new_teams=user_data["teams"],
|
||||
existing_teams=existing_teams,
|
||||
new_teams=new_teams,
|
||||
)
|
||||
|
||||
update_data: Final = {
|
||||
"user_email": user_data["user_email"],
|
||||
"user_alias": user_data["user_alias"],
|
||||
"sso_user_id": user_data["sso_user_id"],
|
||||
"teams": user_data["teams"],
|
||||
"teams": new_teams,
|
||||
"metadata": safe_dumps(metadata),
|
||||
}
|
||||
|
||||
admin_group: Final = await _get_scim_admin_group()
|
||||
if admin_group is not None:
|
||||
if admin_group is not None and client_set_groups:
|
||||
update_data["user_role"] = _resolve_scim_user_role(
|
||||
user.groups or [], admin_group, _default_scim_user_role()
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1176,6 +1176,92 @@ async def test_update_user_success(mocker):
|
|||
assert call_args[1]["data"]["teams"] == ["new-team"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_user_without_groups_preserves_memberships(mocker):
|
||||
"""PUT /Users with `groups` omitted must leave team memberships and role untouched.
|
||||
|
||||
Regression: IdPs that sync membership only through /Groups omit `groups` on a
|
||||
profile update, and the empty default was applied as "remove from every team".
|
||||
"""
|
||||
existing_user = mocker.MagicMock()
|
||||
existing_user.teams = ["team1", "team2"]
|
||||
existing_user.metadata = {}
|
||||
|
||||
scim_user = SCIMUser(
|
||||
schemas=["urn:ietf:params:scim:schemas:core:2.0:User"],
|
||||
userName="test-user",
|
||||
name=SCIMUserName(familyName="User", givenName="Renamed"),
|
||||
emails=[SCIMUserEmail(value="renamed@example.com")],
|
||||
)
|
||||
|
||||
mock_prisma_client = mocker.MagicMock()
|
||||
mock_prisma_client.db.litellm_usertable.update = AsyncMock(return_value={"user_id": "test-user"})
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception",
|
||||
AsyncMock(return_value=mock_prisma_client),
|
||||
)
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2._check_user_exists",
|
||||
AsyncMock(return_value=existing_user),
|
||||
)
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2._get_scim_admin_group",
|
||||
AsyncMock(return_value="admin-group"),
|
||||
)
|
||||
patch_membership = mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2.patch_team_membership", AsyncMock()
|
||||
)
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2.ScimTransformations.transform_litellm_user_to_scim_user",
|
||||
AsyncMock(return_value=scim_user),
|
||||
)
|
||||
|
||||
await update_user(user_id="test-user", user=scim_user)
|
||||
|
||||
patch_membership.assert_not_awaited()
|
||||
update_data = mock_prisma_client.db.litellm_usertable.update.call_args[1]["data"]
|
||||
assert update_data["teams"] == ["team1", "team2"]
|
||||
assert update_data["user_email"] == "renamed@example.com"
|
||||
assert "user_role" not in update_data
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_user_with_explicit_empty_groups_removes_memberships(mocker):
|
||||
"""PUT /Users with `groups: []` sent explicitly still clears every team."""
|
||||
existing_user = mocker.MagicMock()
|
||||
existing_user.teams = ["team1"]
|
||||
|
||||
scim_user = SCIMUser(
|
||||
schemas=["urn:ietf:params:scim:schemas:core:2.0:User"],
|
||||
userName="test-user",
|
||||
emails=[SCIMUserEmail(value="user@example.com")],
|
||||
groups=[],
|
||||
)
|
||||
|
||||
mock_prisma_client = mocker.MagicMock()
|
||||
mock_prisma_client.db.litellm_usertable.update = AsyncMock(return_value={"user_id": "test-user"})
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception",
|
||||
AsyncMock(return_value=mock_prisma_client),
|
||||
)
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2._check_user_exists",
|
||||
AsyncMock(return_value=existing_user),
|
||||
)
|
||||
patch_membership = mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2.patch_team_membership", AsyncMock()
|
||||
)
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2.ScimTransformations.transform_litellm_user_to_scim_user",
|
||||
AsyncMock(return_value=scim_user),
|
||||
)
|
||||
|
||||
await update_user(user_id="test-user", user=scim_user)
|
||||
|
||||
assert patch_membership.call_args[1]["teams_ids_to_remove_user_from"] == ["team1"]
|
||||
assert mock_prisma_client.db.litellm_usertable.update.call_args[1]["data"]["teams"] == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_user_not_found(mocker):
|
||||
"""Should raise 404 when user doesn't exist"""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue