mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
fix(scim): prune deleted user from teams' members_with_roles (#34180)
SCIM delete_user removed the user from the legacy team.members column and deleted
their team membership rows, but never pruned members_with_roles, which is the
source of truth ScimTransformations reads for GET /Groups/{id}. A deleted user
therefore lingered as a dangling member reference on every team they belonged to
Prune each of the user's teams directly via team_member_delete before deleting
the user row, and only for teams whose members_with_roles actually contain the
user, so a real DB failure surfaces (the endpoint fails loudly and the user is
kept; SCIM DELETE is idempotent, so the IdP retries) while a user who was never
in a team's members_with_roles stays a no-op. patch_team_membership is left
unchanged for its other callers
This commit is contained in:
parent
d514497979
commit
482f05190c
2 changed files with 131 additions and 0 deletions
|
|
@ -1317,6 +1317,13 @@ async def delete_user(
|
|||
where={"team_id": team.team_id}, data={"members": new_members}
|
||||
)
|
||||
|
||||
team_row = LiteLLM_TeamTable(**team.model_dump())
|
||||
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),
|
||||
)
|
||||
|
||||
await _set_user_keys_blocked(user_id=user_id, blocked=True)
|
||||
|
||||
await _delete_rows_referencing_user(prisma_client, user_id=user_id)
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ from litellm.proxy.management_endpoints.scim.scim_v2 import (
|
|||
create_group,
|
||||
create_user,
|
||||
delete_group,
|
||||
delete_user,
|
||||
get_groups,
|
||||
get_users,
|
||||
get_service_provider_config,
|
||||
|
|
@ -3062,3 +3063,126 @@ async def test_apply_group_patch_updates_does_not_write_legacy_members(mocker):
|
|||
written = mock_prisma_client.db.litellm_teamtable.update.call_args.kwargs["data"]
|
||||
assert "members" not in written
|
||||
assert written["team_alias"] == "Renamed"
|
||||
|
||||
|
||||
def _mock_prisma_for_delete_user(mocker, team):
|
||||
mock_prisma_client = mocker.MagicMock()
|
||||
mock_prisma_client.db = mocker.MagicMock()
|
||||
mock_prisma_client.db.litellm_teamtable = mocker.MagicMock()
|
||||
mock_prisma_client.db.litellm_teamtable.find_unique = AsyncMock(return_value=team)
|
||||
mock_prisma_client.db.litellm_teamtable.update = AsyncMock()
|
||||
mock_prisma_client.db.litellm_usertable = mocker.MagicMock()
|
||||
mock_prisma_client.db.litellm_usertable.delete = AsyncMock()
|
||||
return mock_prisma_client
|
||||
|
||||
|
||||
def _patch_delete_user_dependencies(mocker, mock_prisma_client, existing_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._set_user_keys_blocked",
|
||||
AsyncMock(),
|
||||
)
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2._delete_rows_referencing_user",
|
||||
AsyncMock(),
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_user_prunes_members_with_roles(mocker):
|
||||
"""Deleting a SCIM user must remove them from every team they belong to via
|
||||
team_member_delete, which prunes members_with_roles (the source of truth for
|
||||
SCIM group membership) so GET /Groups no longer returns a dangling reference
|
||||
to the now-deleted user."""
|
||||
user_id = "scim-del-user"
|
||||
|
||||
existing_user = mocker.MagicMock()
|
||||
existing_user.teams = ["team-1"]
|
||||
|
||||
team = LiteLLM_TeamTable(
|
||||
team_id="team-1",
|
||||
members=[user_id, "other-user"],
|
||||
members_with_roles=[Member(user_id=user_id, role="user"), Member(user_id="other-user", role="admin")],
|
||||
)
|
||||
|
||||
mock_prisma_client = _mock_prisma_for_delete_user(mocker, team)
|
||||
_patch_delete_user_dependencies(mocker, mock_prisma_client, existing_user)
|
||||
team_member_delete_mock = mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2.team_member_delete",
|
||||
AsyncMock(),
|
||||
)
|
||||
|
||||
await delete_user(user_id=user_id)
|
||||
|
||||
team_member_delete_mock.assert_awaited_once()
|
||||
call = team_member_delete_mock.call_args
|
||||
assert call.kwargs["data"].team_id == "team-1"
|
||||
assert call.kwargs["data"].user_id == user_id
|
||||
assert call.kwargs["user_api_key_dict"].user_role == LitellmUserRoles.PROXY_ADMIN
|
||||
mock_prisma_client.db.litellm_usertable.delete.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_user_surfaces_prune_failure_and_keeps_user(mocker):
|
||||
"""A genuine failure while pruning members_with_roles must surface: the
|
||||
endpoint fails loudly and the user row is NOT deleted, so we never report a
|
||||
successful delete while leaving a dangling member (SCIM DELETE is idempotent,
|
||||
so the IdP retries)."""
|
||||
user_id = "scim-del-user"
|
||||
|
||||
existing_user = mocker.MagicMock()
|
||||
existing_user.teams = ["team-1"]
|
||||
|
||||
team = LiteLLM_TeamTable(
|
||||
team_id="team-1",
|
||||
members=[user_id],
|
||||
members_with_roles=[Member(user_id=user_id, role="user")],
|
||||
)
|
||||
|
||||
mock_prisma_client = _mock_prisma_for_delete_user(mocker, team)
|
||||
_patch_delete_user_dependencies(mocker, mock_prisma_client, existing_user)
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2.team_member_delete",
|
||||
AsyncMock(side_effect=Exception("database connection lost")),
|
||||
)
|
||||
|
||||
with pytest.raises(Exception):
|
||||
await delete_user(user_id=user_id)
|
||||
|
||||
mock_prisma_client.db.litellm_usertable.delete.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_user_skips_teams_where_not_a_member(mocker):
|
||||
"""If the user is not in a team's members_with_roles, deletion must treat that
|
||||
team as a no-op (no team_member_delete call, no error) and still delete the
|
||||
user, so a stale legacy membership can't block the delete."""
|
||||
user_id = "scim-del-user"
|
||||
|
||||
existing_user = mocker.MagicMock()
|
||||
existing_user.teams = ["team-1"]
|
||||
|
||||
team = LiteLLM_TeamTable(
|
||||
team_id="team-1",
|
||||
members=[user_id],
|
||||
members_with_roles=[Member(user_id="someone-else", role="admin")],
|
||||
)
|
||||
|
||||
mock_prisma_client = _mock_prisma_for_delete_user(mocker, team)
|
||||
_patch_delete_user_dependencies(mocker, mock_prisma_client, existing_user)
|
||||
team_member_delete_mock = mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2.team_member_delete",
|
||||
AsyncMock(),
|
||||
)
|
||||
|
||||
await delete_user(user_id=user_id)
|
||||
|
||||
team_member_delete_mock.assert_not_awaited()
|
||||
mock_prisma_client.db.litellm_usertable.delete.assert_awaited_once()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue