From fd27f886fa2dbc303e60ec0cb33c60074dcdddff Mon Sep 17 00:00:00 2001 From: Jesus Velarde Date: Mon, 9 Nov 2026 17:52:43 -0800 Subject: [PATCH] fix(scim): repair a member's teams list when a SCIM user write names a team they are already on the roster of --- .../management_endpoints/scim/scim_v2.py | 3 +- litellm/proxy/management_helpers/utils.py | 6 +-- .../management/test_scim_user_team_race.py | 39 +++++++++++++++++++ .../scim/test_scim_v2_endpoints.py | 14 +++++-- 4 files changed, 55 insertions(+), 7 deletions(-) diff --git a/litellm/proxy/management_endpoints/scim/scim_v2.py b/litellm/proxy/management_endpoints/scim/scim_v2.py index f5899c3bba4..19e6856e9f3 100644 --- a/litellm/proxy/management_endpoints/scim/scim_v2.py +++ b/litellm/proxy/management_endpoints/scim/scim_v2.py @@ -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: diff --git a/litellm/proxy/management_helpers/utils.py b/litellm/proxy/management_helpers/utils.py index 1c4eada67fb..72246a570c9 100644 --- a/litellm/proxy/management_helpers/utils.py +++ b/litellm/proxy/management_helpers/utils.py @@ -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( diff --git a/tests/integration/management/test_scim_user_team_race.py b/tests/integration/management/test_scim_user_team_race.py index 058d07b80d7..b12882407cd 100644 --- a/tests/integration/management/test_scim_user_team_race.py +++ b/tests/integration/management/test_scim_user_team_race.py @@ -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}) diff --git a/tests/unit/proxy/management_endpoints/scim/test_scim_v2_endpoints.py b/tests/unit/proxy/management_endpoints/scim/test_scim_v2_endpoints.py index 86197a1ac32..11fa0d2bf51 100644 --- a/tests/unit/proxy/management_endpoints/scim/test_scim_v2_endpoints.py +++ b/tests/unit/proxy/management_endpoints/scim/test_scim_v2_endpoints.py @@ -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