fix(scim): repair a member's teams list when a SCIM user write names a team they are already on the roster of

This commit is contained in:
Jesus Velarde 2026-11-09 17:52:43 -08:00
parent 2666739c51
commit fd27f886fa
4 changed files with 55 additions and 7 deletions

View file

@ -94,6 +94,7 @@ from litellm.proxy.management_helpers.team_roster_sync import (
team_tx_db,
user_tx_db,
)
from litellm.proxy.management_helpers.utils import append_team_id_if_absent
from litellm.proxy.utils import ( # noqa: F401 # legacy module exports
PrismaClient,
_premium_user_check, # pyright: ignore[reportPrivateUsage,reportUnusedImport] # backwards-compatible package export
@ -2698,7 +2699,7 @@ async def _add_user_to_team(user_id: str, team_id: str) -> None:
except ProxyException as e:
if e.type != ProxyErrorTypes.team_member_already_in_team:
raise
verbose_proxy_logger.debug("User %s is already in team %s, skipping add", user_id, team_id)
await append_team_id_if_absent(await _get_prisma_client_or_raise_exception(), user_id, team_id)
async def _remove_user_from_team(user_id: str, team_id: str) -> None:

View file

@ -400,7 +400,7 @@ async def _resolve_member_budget_id(
return response.budget_id
async def _append_team_id_if_absent(
async def append_team_id_if_absent(
prisma_client: PrismaClient, user_id: str, team_id: str, tx: MemberWriteTx | None = None
) -> None:
"""Append team_id to a user's teams array, only if it is not already present.
@ -453,7 +453,7 @@ async def add_new_member(
new_member.user_id,
{"teams": [team_id], **new_user_defaults},
)
await _append_team_id_if_absent(prisma_client, new_member.user_id, team_id, tx)
await append_team_id_if_absent(prisma_client, new_member.user_id, team_id, tx)
if _returned_user is not None:
returned_user = LiteLLM_UserTable.model_validate(_returned_user.model_dump())
elif new_member.user_email is not None:
@ -471,7 +471,7 @@ async def add_new_member(
returned_user = LiteLLM_UserTable.model_validate(_returned_user.model_dump())
elif len(existing_user_row) == 1:
user_info: Final = existing_user_row[0]
await _append_team_id_if_absent(prisma_client, user_info.user_id, team_id, tx)
await append_team_id_if_absent(prisma_client, user_info.user_id, team_id, tx)
returned_user = LiteLLM_UserTable.model_validate(user_info.model_dump())
elif len(existing_user_row) > 1:
raise HTTPException(

View file

@ -26,6 +26,7 @@ SCIM_CORE_USER_SCHEMA: Final = "urn:ietf:params:scim:schemas:core:2.0:User"
SET_EMAIL_SQL: Final = 'UPDATE "LiteLLM_UserTable" SET user_email = %s WHERE user_id = %s'
PROFILE_NAME: Final = "Race"
USER_ALIAS_SQL: Final = 'SELECT user_alias FROM "LiteLLM_UserTable" WHERE user_id = %s'
DROP_TEAM_SQL: Final = 'UPDATE "LiteLLM_UserTable" SET teams = array_remove(teams, %s) WHERE user_id = %s'
APPEND_TEAM_SQL: Final = 'UPDATE "LiteLLM_UserTable" SET teams = array_append(teams, %s) WHERE user_id = %s'
@ -97,3 +98,41 @@ def test_a_scim_user_write_keeps_a_team_a_group_push_added_after_the_write_read_
assert response.status_code == status, response.text
assert users_referencing(team) == frozenset({user})
assert read_rows(USER_ALIAS_SQL, (user,)) == [{"user_alias": PROFILE_NAME}]
def _put_with_group(candidate: Gateway, user: str, team: str) -> httpx.Response:
return candidate.request(
"PUT",
f"/scim/v2/Users/{user}",
{
"schemas": [SCIM_CORE_USER_SCHEMA],
"userName": user,
"emails": [{"value": f"{user}@example.com", "primary": True}],
"groups": [{"value": team}],
"active": True,
},
)
def _patch_add_group(candidate: Gateway, user: str, team: str) -> httpx.Response:
return candidate.request(
"PATCH",
f"/scim/v2/Users/{user}",
{"schemas": [PATCH_OP_SCHEMA], "Operations": [{"op": "add", "path": "groups", "value": [{"value": team}]}]},
)
@pytest.mark.parametrize("write", [_put_with_group, _patch_add_group], ids=["put", "patch"])
def test_a_scim_user_write_naming_a_team_repairs_a_member_whose_teams_lost_it(
gateway: Gateway, write: Callable[[Gateway, str, str], httpx.Response]
) -> None:
with gateway.scenario() as scenario:
(user,) = seed_users(scenario, 1)
team: Final = created_team(scenario, create_group(gateway, (user,)))
write_rows(DROP_TEAM_SQL, (team, user))
assert users_referencing(team) == frozenset()
response: Final = write(gateway, user, team)
assert response.status_code == 200, response.text
assert users_referencing(team) == frozenset({user})

View file

@ -1107,9 +1107,8 @@ async def test_handle_existing_user_by_email_roster_add_failure_blocks_teams_wri
@pytest.mark.asyncio
async def test_handle_existing_user_by_email_roster_add_already_member_is_noop(mocker):
"""Being already in the team is benign even under the strict path: the upsert
succeeds and the deduped teams array is still persisted."""
async def test_handle_existing_user_by_email_roster_add_already_member_repairs_user_teams(mocker):
"""A user already on the roster whose teams list lacks the team gets it appended, not skipped"""
existing_user = mocker.MagicMock()
existing_user.user_id = "uid"
existing_user.user_email = "member@example.com"
@ -1122,6 +1121,11 @@ async def test_handle_existing_user_by_email_roster_add_already_member_is_noop(m
mock_prisma_client.db.litellm_usertable = mocker.MagicMock()
mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=existing_user)
mock_prisma_client.db.litellm_usertable.update = AsyncMock(return_value={})
mock_prisma_client.db.litellm_usertable.update_many = AsyncMock(return_value=1)
mocker.patch( # test-quality-ok: the roster helpers resolve the proxy prisma client at module level
"litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception",
AsyncMock(return_value=mock_prisma_client),
)
mock_team_member_add = mocker.patch(
"litellm.proxy.management_endpoints.scim.scim_v2.team_member_add",
@ -1153,6 +1157,10 @@ async def test_handle_existing_user_by_email_roster_add_already_member_is_noop(m
)
mock_team_member_add.assert_awaited_once()
mock_prisma_client.db.litellm_usertable.update_many.assert_awaited_once_with(
where={"user_id": "uid", "NOT": {"teams": {"has": "team-x"}}},
data={"teams": {"push": ["team-x"]}},
)
update_calls = mock_prisma_client.db.litellm_usertable.update.call_args_list
assert len(update_calls) == 1