diff --git a/litellm/proxy/management_endpoints/scim/scim_v2.py b/litellm/proxy/management_endpoints/scim/scim_v2.py index 8d255859571..7183e6cb402 100644 --- a/litellm/proxy/management_endpoints/scim/scim_v2.py +++ b/litellm/proxy/management_endpoints/scim/scim_v2.py @@ -487,13 +487,18 @@ class _UnknownMember(NamedTuple): value: str -_ClassifiedGroupMember = Union[_ResolvedUserMember, _SkippedGroupMember, _UnknownMember] +class _AmbiguousMember(NamedTuple): + value: str + + +_ClassifiedGroupMember = Union[_ResolvedUserMember, _SkippedGroupMember, _UnknownMember, _AmbiguousMember] class _PartitionedMembers(NamedTuple): resolved_ids: tuple[str, ...] skipped: tuple[_SkippedGroupMember, ...] unknown_ids: tuple[str, ...] + ambiguous_values: tuple[str, ...] def _member_value(member: SCIMMember) -> str: @@ -536,6 +541,44 @@ def _team_metadata_has_scim_provenance(team_metadata: object) -> bool: return bool(fields.get(SCIM_MANAGED_TEAM_METADATA_KEY)) or fields.get(SCIM_TEAM_DATA_METADATA_KEY) is not None +class _CaseInsensitiveMatch(TypedDict): + equals: ReadOnly[str] + mode: ReadOnly[str] + + +async def _users_named_by_member_value( + value: str, prisma_client: PrismaClient, *, take: int | None = 2 +) -> tuple[str, ...]: + """Every user id this member value names, by SSO identity or by email. + + Both fields are searched in one pass, because searching either first would hide a + value that names one account by its SSO identity and another by its email, and + hand the group to whichever field was searched first. + + They are not compared alike. An email is matched the way ``new_user`` matches one + before it accepts a new account, case-insensitively: matching more strictly than + the layer that would reject the placeholder is what turned a member id whose + casing differed from the stored email into a 500 on the whole push. An SSO + identity is matched exactly, because OIDC defines ``sub`` as case-sensitive and + nothing folds its case on the way in, so treating two subjects that differ in case + as one would hand the group to an account the provider never named. + + ``take`` bounds the read for a caller that only needs to know whether the value + names one account or several; ``user_email`` carries no index, so letting the scan + stop early is worth the two rows. A caller that has to know *which* accounts, as a + removal does, passes None. That set is the accounts sharing one identity, which is + a handful at worst. + """ + subject: Final = value.strip() + email: Final[_CaseInsensitiveMatch] = {"equals": subject, "mode": "insensitive"} + rows: Final = await _table(UserRepository(prisma_client)).find_many( + # mutable-ok: the Prisma serializer requires concrete dicts and a concrete list + where={"OR": [{"sso_user_id": subject}, {"user_email": email}]}, + take=take, + ) + return tuple(dict.fromkeys(row.user_id for row in rows)) + + async def _classify_group_member(member: SCIMMember, prisma_client: PrismaClient) -> _ClassifiedGroupMember: """ Decide what a single SCIM group member refers to. @@ -557,6 +600,20 @@ async def _classify_group_member(member: SCIMMember, prisma_client: PrismaClient one the identity provider writes. An id the IdP called a User is a user even if some team happens to share the id, and a team created here rather than through SCIM is not evidence of anything about the member. + + When those checks miss on an otherwise user-shaped member, its value is looked + up as an SSO identity or an email, and a match resolves to that user's + ``user_id``. A value that names more than one account is ambiguous rather than + unknown: it names a real person we cannot identify, so it is neither guessed at + nor provisioned. + + An exact ``user_id`` hit is checked the same way rather than trusted outright. A + value can be one account's id and another's SSO identity or email, and taking the + id on sight would hand the group to whichever account happened to be keyed by it. + The placeholders this bug provisioned are that shape exactly, since they are keyed + by the very id the provider keeps pushing, so on a tenant that already has them + the membership is refused and named rather than silently landing on the + placeholder again. """ value: Final = _member_value(member) member_type: Final = _normalized_member_type(member) @@ -566,6 +623,18 @@ async def _classify_group_member(member: SCIMMember, prisma_client: PrismaClient user: Final = await _table(UserRepository(prisma_client)).find_unique(where={"user_id": value}) if user is not None: + shared_with: Final = tuple( + other for other in await _users_named_by_member_value(value, prisma_client) if other != value + ) + if shared_with: + verbose_proxy_logger.warning( + "SCIM: group member '%s' is one account's user id and is also account '%s' by SSO identity or email, " + "so the membership cannot be attributed. A placeholder an earlier release provisioned under this id " + "looks exactly like this and should be deleted so the real account can be matched", + value, + shared_with[0], + ) + return _AmbiguousMember(value=value) return _ResolvedUserMember(user_id=value) if member_type is not None and member_type != "user": @@ -576,6 +645,22 @@ async def _classify_group_member(member: SCIMMember, prisma_client: PrismaClient if team is not None and _team_metadata_has_scim_provenance(team.metadata): return _SkippedGroupMember(value=value, reason="existing_team") + named: Final = await _users_named_by_member_value(value, prisma_client) + if len(named) == 1: + verbose_proxy_logger.info( + "SCIM: group member '%s' matched user_id '%s' by SSO identity or email", + value, + named[0], + ) + return _ResolvedUserMember(user_id=named[0]) + if len(named) > 1: + verbose_proxy_logger.warning( + "SCIM: group member '%s' names more than one account by SSO identity or email and cannot be resolved " + "unambiguously", + value, + ) + return _AmbiguousMember(value=value) + return _UnknownMember(value=value) @@ -583,11 +668,13 @@ def _bucketed_member(entry: _ClassifiedGroupMember) -> _PartitionedMembers: """The single-member partition one classified entry contributes.""" match entry: case _ResolvedUserMember(user_id=user_id): - return _PartitionedMembers(resolved_ids=(user_id,), skipped=(), unknown_ids=()) + return _PartitionedMembers(resolved_ids=(user_id,), skipped=(), unknown_ids=(), ambiguous_values=()) case _SkippedGroupMember(): - return _PartitionedMembers(resolved_ids=(), skipped=(entry,), unknown_ids=()) + return _PartitionedMembers(resolved_ids=(), skipped=(entry,), unknown_ids=(), ambiguous_values=()) case _UnknownMember(value=value): - return _PartitionedMembers(resolved_ids=(), skipped=(), unknown_ids=(value,)) + return _PartitionedMembers(resolved_ids=(), skipped=(), unknown_ids=(value,), ambiguous_values=()) + case _AmbiguousMember(value=value): + return _PartitionedMembers(resolved_ids=(), skipped=(), unknown_ids=(), ambiguous_values=(value,)) case _: assert_never(entry) @@ -599,6 +686,7 @@ def _partition_classified_members(classified: Iterable[_ClassifiedGroupMember]) resolved_ids=tuple(chain.from_iterable(bucket.resolved_ids for bucket in bucketed)), skipped=tuple(chain.from_iterable(bucket.skipped for bucket in bucketed)), unknown_ids=tuple(chain.from_iterable(bucket.unknown_ids for bucket in bucketed)), + ambiguous_values=tuple(chain.from_iterable(bucket.ambiguous_values for bucket in bucketed)), ) @@ -608,7 +696,7 @@ def _admitted_member_id(entry: _ClassifiedGroupMember, created_ids: frozenset[st return user_id case _UnknownMember(value=value): return value if value in created_ids else None - case _SkippedGroupMember(): + case _SkippedGroupMember() | _AmbiguousMember(): return None case _: assert_never(entry) @@ -662,6 +750,70 @@ async def _ensure_group_member_user( raise HTTPException(status_code=500, detail=detail) +def _roster_entries_named_by(value: str, roster: frozenset[str], resolved: tuple[str, ...]) -> tuple[str, ...]: + """The members of this group a removal value names. + + Both ways of naming one count together. The id as written counts when the roster + holds it verbatim, which is how an earlier release recorded a member it could not + match, and the accounts it resolves to count when they are on the roster. Counting + only the resolved ones would let a value that is one member's canonical id and + another member's email revoke both, since each looks singular on its own. + """ + return tuple( + dict.fromkeys( + chain( + (value,) if value in roster else (), + (user_id for user_id in resolved if user_id in roster), + ) + ) + ) + + +async def _member_ids_to_drop( + members: Sequence[SCIMMember], roster: frozenset[str], prisma_client: PrismaClient +) -> frozenset[str]: + """The members a ``remove`` clears, one per id the request names. + + The roster holds canonical user ids, so a directory that added someone by their + email or SSO identity has to be able to remove them by that same value, and a + member an earlier release recorded under the raw id has to stay removable by it. + + Ambiguity is a property of the table as it stands, not of the value, so a value + that named one person when they were admitted can name two later. Resolving a + removal against the whole table would then drop nobody while answering 200, and + the person the directory just took out of the group would keep the team. So a + removal keeps only the accounts already on the roster: one is unambiguous however + many strangers share the address, none means there is nothing to revoke, and only + a value naming two of this group's own members is genuinely undecidable. That last + case fails rather than reporting a removal it did not perform, or revoking both. + + Raises: + HTTPException: 400 when a member id names more than one current member. + """ + written: Final = frozenset(_member_value(member) for member in members) + matched: Final = tuple( + [ + ( + value, + _roster_entries_named_by( + value, roster, await _users_named_by_member_value(value, prisma_client, take=None) + ), + ) + for value in sorted(written) + ] + ) + undecidable: Final = tuple(value for value, entries in matched if len(entries) > 1) + if undecidable: + raise HTTPException( + status_code=400, + detail={ + "error": f"Member ID '{undecidable[0]}' names more than one member of this group, so the removal " + "cannot be attributed. Send the LiteLLM user ID as the member value, or resolve the duplicate." + }, + ) + return frozenset(chain.from_iterable(entries for _, entries in matched)) + + async def _resolve_group_member_ids( members: Sequence[SCIMMember], created_via: str, @@ -670,17 +822,18 @@ async def _resolve_group_member_ids( """ Resolve SCIM group members to LiteLLM user ids, dropping members that are not users. - Only the operations that put ids onto a roster resolve their members: an id - that resolves to nothing is created when litellm_settings.scim_upsert_user is - True (default) and rejected per SCIM 2.0 otherwise. Removals do not come - through here; dropping an id is idempotent, so it needs neither a lookup nor a - user to drop. + Member ids are matched by ``user_id`` first, then by SSO identity or email. An + id that resolves to nothing is created when litellm_settings.scim_upsert_user is + True (default) and rejected per SCIM 2.0 otherwise. Removals do not come through + here: they resolve through ``_member_ids_to_drop`` instead, which neither creates + a user nor fails on an id it cannot place. Raises: - HTTPException: 400 when a member id is empty, or when scim_upsert_user is - False and a member id is neither an existing user, an existing team, nor a - member declared to be something other than a user. 500 when a member's - user row can neither be created nor found. + HTTPException: 400 when a member id is empty, when a member id names more + than one user, or when scim_upsert_user is False and a member id is neither + an existing user, an existing team, nor a member declared to be something + other than a user. 500 when a member's user row can neither be created nor + found. """ classified: Final = tuple([await _classify_group_member(member, prisma_client) for member in members]) partition: Final = _partition_classified_members(classified) @@ -692,6 +845,16 @@ async def _resolve_group_member_ids( skipped.reason, ) + if partition.ambiguous_values: + raise HTTPException( + status_code=400, + detail={ + "error": f"Member ID '{partition.ambiguous_values[0]}' names more than one LiteLLM user, so the " + "group membership cannot be attributed. Resolve the duplicate, which for an id that also matches a " + "SCIM-provisioned placeholder means deleting that placeholder." + }, + ) + if partition.unknown_ids and not await _get_scim_upsert_user_setting(): raise HTTPException( status_code=400, @@ -702,6 +865,13 @@ async def _resolve_group_member_ids( ) unique_unknown_ids: Final = tuple(dict.fromkeys(partition.unknown_ids)) + for user_id in unique_unknown_ids: + verbose_proxy_logger.warning( + "SCIM: creating placeholder user for group member '%s'; matched no user by user_id, sso_user_id or " + "user_email. An SSO-provisioned user's real account stays teamless if this is a mismatch", + user_id, + ) + creations: Final = tuple( [ ( @@ -2428,7 +2598,9 @@ async def _process_group_patch_operations( ) if op_type == "remove": - final_members = final_members - {_member_value(member) for member in patched_members} + final_members = final_members - await _member_ids_to_drop( + patched_members, frozenset(final_members), prisma_client + ) else: member_result = await _resolve_group_member_ids( members=patched_members, diff --git a/tests/test_litellm/proxy/management_endpoints/scim/test_scim_v2_endpoints.py b/tests/test_litellm/proxy/management_endpoints/scim/test_scim_v2_endpoints.py index f933cf6655e..0a9efd40b48 100644 --- a/tests/test_litellm/proxy/management_endpoints/scim/test_scim_v2_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/scim/test_scim_v2_endpoints.py @@ -1,8 +1,13 @@ +import logging import time -from unittest.mock import AsyncMock +from collections.abc import Mapping +from itertools import chain +from typing import Final +from unittest.mock import AsyncMock, MagicMock, call import pytest from fastapi import HTTPException +from pytest_mock import MockerFixture from litellm.proxy._types import ( LiteLLM_TeamTable, @@ -72,6 +77,7 @@ async def test_create_user_existing_user_conflict(mocker): mock_prisma_client.db = mocker.MagicMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value={"user_id": "existing-user"}) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) # Mock the _get_prisma_client_or_raise_exception to return our mock mocker.patch( @@ -108,6 +114,7 @@ async def test_create_user_defaults_to_viewer(mocker, monkeypatch): mock_prisma_client.db = mocker.MagicMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) monkeypatch.setattr("litellm.default_internal_user_params", None, raising=False) @@ -158,6 +165,7 @@ async def test_create_user_ingests_enterprise_extension(mocker, monkeypatch): mock_prisma_client.db = mocker.MagicMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) monkeypatch.setattr("litellm.default_internal_user_params", None, raising=False) @@ -214,6 +222,7 @@ async def test_create_user_ingests_entitlements_and_roles(mocker, monkeypatch): mock_prisma_client.db = mocker.MagicMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) monkeypatch.setattr("litellm.default_internal_user_params", None, raising=False) @@ -263,6 +272,7 @@ async def test_create_user_uses_default_internal_user_params_role(mocker, monkey mock_prisma_client.db = mocker.MagicMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) # Set default_internal_user_params with a specific role @@ -362,6 +372,7 @@ async def test_scim_create_user_respects_default_role_set_via_ui(mocker, monkeyp mock_prisma_client.db = mocker.MagicMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) mocker.patch( @@ -1282,6 +1293,7 @@ async def test_update_group_metadata_serialization_issue(mocker): mock_user.user_email = "user1@example.com" # Add proper string value for user_email mock_user.teams = [group_id] mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mock_user) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mock_prisma_client.db.litellm_usertable.update = AsyncMock(return_value=mock_user) # Mock the _get_prisma_client_or_raise_exception to return our mock @@ -1484,6 +1496,7 @@ async def test_update_group_e2e(mocker): mock_user = mocker.MagicMock() mock_user.user_id = "test-user" mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mock_user) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) # Mock dependencies mocker.patch( @@ -1618,6 +1631,8 @@ async def test_create_group_with_nonexistent_users_rejects(mocker, monkeypatch): return None # new-user-1 and new-user-2 don't exist mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(side_effect=mock_user_lookup) + mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=[]) # Mock dependencies mocker.patch( @@ -1702,6 +1717,8 @@ async def test_update_group_with_nonexistent_users_rejects(mocker, monkeypatch): return None # new-user-3 and new-user-4 don't exist mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(side_effect=mock_user_lookup) + mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=[]) # Mock dependencies mocker.patch( @@ -1771,6 +1788,8 @@ async def test_create_group_with_nonexistent_users_creates_when_flag_true(mocker return None # new-user-1 and new-user-2 don't exist mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(side_effect=mock_user_lookup) + mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=[]) # Mock user creation created_user_1 = NewUserResponse(user_id="new-user-1", key="test-key-1") @@ -1859,6 +1878,8 @@ async def test_extract_group_member_ids_with_flag_true_creates_users(mocker, mon return None # new-user-1 doesn't exist mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(side_effect=mock_user_lookup) + mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=[]) # Mock user creation created_user = NewUserResponse(user_id="new-user-1", key="test-key-1") @@ -1927,6 +1948,8 @@ async def test_extract_group_member_ids_with_flag_false_rejects(mocker, monkeypa return None # new-user-1 doesn't exist mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(side_effect=mock_user_lookup) + mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=[]) # Mock dependencies mocker.patch( @@ -1976,6 +1999,8 @@ async def test_process_group_patch_operations_with_flag_true_creates_users(mocke # Mock user lookup - new-user-1 doesn't exist mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=[]) mock_prisma_client.db.litellm_teamtable.find_unique = AsyncMock(return_value=None) # Mock user creation @@ -2031,6 +2056,8 @@ async def test_process_group_patch_operations_with_flag_false_rejects(mocker, mo # Mock user lookup - new-user-1 doesn't exist mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=[]) mock_prisma_client.db.litellm_teamtable.find_unique = AsyncMock(return_value=None) # Execute the function - should raise HTTPException @@ -2070,6 +2097,7 @@ async def test_create_user_grants_admin_when_in_scim_admin_group(mocker, monkeyp mock_prisma_client.db = mocker.MagicMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) mocker.patch( @@ -2114,6 +2142,7 @@ async def test_create_user_keeps_default_when_not_in_scim_admin_group(mocker, mo mock_prisma_client.db = mocker.MagicMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) mocker.patch( @@ -2463,6 +2492,7 @@ def _scim_admin_prisma(mocker, *, user_teams): prisma.db = mocker.MagicMock() prisma.db.litellm_usertable = mocker.MagicMock() prisma.db.litellm_usertable.find_unique = AsyncMock(return_value=user) + prisma.db.litellm_usertable.find_many = AsyncMock(return_value=()) prisma.db.litellm_usertable.update = AsyncMock(return_value=user) prisma.db.litellm_teamtable = mocker.MagicMock() prisma.db.litellm_teamtable.find_unique = AsyncMock(side_effect=_team_find_unique) @@ -2561,6 +2591,7 @@ async def test_update_group_recomputes_roles_for_changed_members(mocker): mock_prisma_client.db.litellm_teamtable.update = AsyncMock(return_value=existing_team) mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock()) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", @@ -2618,6 +2649,7 @@ async def test_patch_group_recomputes_roles_for_changed_members(mocker): mock_prisma_client.db.litellm_teamtable.update = AsyncMock(return_value=existing_team) mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock()) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", @@ -2671,6 +2703,7 @@ async def test_delete_group_recomputes_roles_for_members(mocker): mock_prisma_client.db.litellm_teamtable.delete = AsyncMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=member) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mock_prisma_client.db.litellm_usertable.update = AsyncMock() mocker.patch( @@ -2794,6 +2827,7 @@ async def test_create_user_existing_email_upsert_demotes_when_admin_group_set(mo mock_prisma_client.db = mocker.MagicMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mock_prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=existing_user) mock_prisma_client.db.litellm_usertable.update = AsyncMock(return_value={"user_id": "returning-user"}) @@ -2843,6 +2877,7 @@ async def test_create_group_recomputes_roles_for_members(mocker): mock_prisma_client.db.litellm_teamtable.find_unique = AsyncMock(return_value=None) mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock()) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", @@ -2900,6 +2935,7 @@ async def test_update_group_rename_recomputes_retained_members(mocker): mock_prisma_client.db.litellm_teamtable.update = AsyncMock(return_value=existing_team) mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock()) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", @@ -2954,6 +2990,7 @@ async def test_patch_group_rename_recomputes_retained_members(mocker): mock_prisma_client.db.litellm_teamtable.update = AsyncMock(return_value=existing_team) mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock()) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", @@ -3019,6 +3056,7 @@ async def test_process_group_patch_operations_add_retains_existing_members(mocke mock_prisma_client.db.litellm_usertable = mocker.MagicMock() # new-user already exists in the DB mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock(user_id="new-user")) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) _, final_members, _ = await _process_group_patch_operations( patch_ops=patch_ops, @@ -3059,6 +3097,7 @@ async def test_process_group_patch_operations_remove_uses_members_with_roles(moc mock_prisma_client.db = mocker.MagicMock() mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock(user_id="drop-user")) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) _, final_members, _ = await _process_group_patch_operations( patch_ops=patch_ops, @@ -3090,6 +3129,7 @@ async def test_get_groups_reports_members_from_members_with_roles(mocker): mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock( return_value=mocker.MagicMock(user_id="member-1", user_email="member-1@example.com") ) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", @@ -3310,6 +3350,7 @@ async def test_patch_group_add_applies_delta_and_keeps_concurrent_add(mocker): mock_prisma_client.db.litellm_teamtable.update = AsyncMock(return_value=final_team) mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock()) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", @@ -3403,6 +3444,7 @@ async def test_patch_group_replace_stays_absolute_against_concurrent_roster(mock mock_prisma_client.db.litellm_teamtable.update = AsyncMock(return_value=final_team) mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock()) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", @@ -3503,6 +3545,7 @@ async def test_process_group_patch_remove_filtered_path_without_value(mocker): prisma_client.db = mocker.MagicMock() prisma_client.db.litellm_usertable = mocker.MagicMock() prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=LiteLLM_UserTable(user_id="user-1")) + prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) _, final_members, _ = await _process_group_patch_operations( patch_ops=patch_ops, @@ -3532,6 +3575,7 @@ async def test_process_group_patch_add_filtered_path_without_value(mocker): prisma_client.db = mocker.MagicMock() prisma_client.db.litellm_usertable = mocker.MagicMock() prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=LiteLLM_UserTable(user_id="user-3")) + prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) _, final_members, _ = await _process_group_patch_operations( patch_ops=patch_ops, @@ -3565,6 +3609,7 @@ async def test_process_group_patch_replace_empty_value_does_not_use_path_filter( prisma_client.db = mocker.MagicMock() prisma_client.db.litellm_usertable = mocker.MagicMock() prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=LiteLLM_UserTable(user_id="user-1")) + prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) _, final_members, _ = await _process_group_patch_operations( patch_ops=patch_ops, @@ -3575,7 +3620,16 @@ async def test_process_group_patch_replace_empty_value_does_not_use_path_filter( assert final_members == set() -def _member_resolution_prisma(mocker, *, users: set, teams: set, unmanaged_teams: frozenset = frozenset()): +def _member_resolution_prisma( + mocker: MockerFixture, + *, + users: set[str], + teams: set[str], + unmanaged_teams: frozenset[str] = frozenset(), + email_to_user_id: Mapping[str, str] | None = None, + email_to_user_ids: Mapping[str, tuple[str, ...]] | None = None, + sso_user_id_to_user_id: Mapping[str, str] | None = None, +) -> MagicMock: """Prisma mock where only the given ids resolve to a user row / team row. ``teams`` are teams a SCIM group write created, so they carry provenance; @@ -3589,14 +3643,78 @@ def _member_resolution_prisma(mocker, *, users: set, teams: set, unmanaged_teams return LiteLLM_TeamTable(team_id=team_id, metadata={}) return None + def user_row(where: Mapping[str, str]) -> LiteLLM_UserTable | None: + user_id: Final = where["user_id"] + if user_id in users: + return LiteLLM_UserTable(user_id=user_id) + return None + prisma_client = mocker.MagicMock() prisma_client.db = mocker.MagicMock() prisma_client.db.litellm_usertable = mocker.MagicMock() - prisma_client.db.litellm_usertable.find_unique = AsyncMock( - side_effect=lambda where: LiteLLM_UserTable(user_id=where["user_id"]) if where["user_id"] in users else None + prisma_client.db.litellm_usertable.find_unique = AsyncMock(side_effect=user_row) + + emails_to_ids: Final[Mapping[str, tuple[str, ...]]] = ( + dict(email_to_user_ids) + if email_to_user_ids is not None + else ({email: (user_id,) for email, user_id in email_to_user_id.items()} if email_to_user_id else {}) ) + ssos_to_ids: Final[Mapping[str, str]] = dict(sso_user_id_to_user_id) if sso_user_id_to_user_id else {} + + def identity_rows(where: Mapping[str, object], take: int | None = None) -> tuple[LiteLLM_UserTable, ...]: + """Stand-in for the cross-field lookup, honouring the comparison mode + production actually asks for per field, so a field that stops folding case, or + starts folding it, fails here instead of passing. + + A caller that must know which accounts match rather than merely how many + passes take=None, so an unbounded read returns every match. + """ + clauses: Final = where["OR"] + assert isinstance(clauses, list) + fields: Final = tuple(next(iter(clause)) for clause in clauses) + assert fields == ("sso_user_id", "user_email"), fields + + def comparison(clause: Mapping[str, object]) -> tuple[str, bool]: + """The needle and whether production asked for a case-insensitive compare, + read per field so a field that stops folding case fails here.""" + criterion = next(iter(clause.values())) + if isinstance(criterion, str): + return criterion, False + assert isinstance(criterion, dict), criterion + return criterion["equals"], criterion.get("mode") == "insensitive" + + sso_needle, sso_insensitive = comparison(clauses[0]) + email_needle, email_insensitive = comparison(clauses[1]) + + def same(stored: str, needle: str, insensitive: bool) -> bool: + return stored.casefold() == needle.casefold() if insensitive else stored == needle + + matched: Final = tuple( + chain( + ( + user_id + for sso_user_id, user_id in ssos_to_ids.items() + if same(sso_user_id, sso_needle, sso_insensitive) + ), + ( + user_id + for email, user_ids in emails_to_ids.items() + if same(email, email_needle, email_insensitive) + for user_id in user_ids + ), + ) + ) + found: Final = tuple(dict.fromkeys(matched)) + return tuple(LiteLLM_UserTable(user_id=user_id) for user_id in (found[:take] if take else found)) + + def team_lookup(where: Mapping[str, str]) -> LiteLLM_TeamTable | None: + team_id: Final = where["team_id"] + return team_row(team_id) + + prisma_client.db.litellm_usertable.find_first = AsyncMock(return_value=None) + prisma_client.db.litellm_usertable.find_many = AsyncMock(side_effect=identity_rows) prisma_client.db.litellm_teamtable = mocker.MagicMock() - prisma_client.db.litellm_teamtable.find_unique = AsyncMock(side_effect=lambda where: team_row(where["team_id"])) + prisma_client.db.litellm_teamtable.find_unique = AsyncMock(side_effect=team_lookup) return prisma_client @@ -4363,6 +4481,581 @@ async def test_resolve_group_member_ids_dedupes_repeated_member(mocker, scim_ups assert result.all_member_ids == ["dup-user"] +def _identity_lookup(value: str) -> object: + """The single cross-field lookup the classifier is expected to issue.""" + return call( + where={"OR": [{"sso_user_id": value}, {"user_email": {"equals": value, "mode": "insensitive"}}]}, + take=2, + ) + + +@pytest.mark.asyncio +async def test_resolve_group_member_ids_matches_sso_user_id(mocker, scim_upsert_user_enabled): + """An OIDC subject in a group payload must resolve to the existing user's + internal id instead of provisioning a placeholder.""" + prisma_client = _member_resolution_prisma( + mocker, + users=set(), + teams=set(), + sso_user_id_to_user_id={"member-sub": "sso-user"}, + ) + create_user_mock = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(), + ) + + result = await _resolve_group_member_ids( + members=[SCIMMember(value="member-sub")], + created_via="scim_group_membership", + prisma_client=prisma_client, + ) + + create_user_mock.assert_not_called() + assert result.existing_member_ids == ["sso-user"] + assert result.created_users == [] + assert result.all_member_ids == ["sso-user"] + assert prisma_client.db.litellm_usertable.find_many.await_args_list == [_identity_lookup("member-sub")] + + +@pytest.mark.asyncio +async def test_resolve_group_member_ids_matches_user_email(mocker, scim_upsert_user_enabled): + """A group member email must resolve to the existing user's internal id + when the identity provider sends email rather than the user id.""" + prisma_client = _member_resolution_prisma( + mocker, + users=set(), + teams=set(), + email_to_user_id={"member@example.com": "email-user"}, + ) + create_user_mock = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(), + ) + + result = await _resolve_group_member_ids( + members=[SCIMMember(value="member@example.com")], + created_via="scim_group_membership", + prisma_client=prisma_client, + ) + + create_user_mock.assert_not_called() + assert result.existing_member_ids == ["email-user"] + assert result.created_users == [] + assert result.all_member_ids == ["email-user"] + assert prisma_client.db.litellm_usertable.find_many.await_args_list == [_identity_lookup("member@example.com")] + + +@pytest.mark.parametrize( + "pushed", + ["MEMBER@EXAMPLE.COM", "Member@Example.com", " member@example.com "], + ids=["upper", "mixed", "padded"], +) +@pytest.mark.asyncio +async def test_resolve_group_member_ids_matches_user_email_as_the_write_path_would( + mocker, scim_upsert_user_enabled, pushed +): + """The member value must be compared the way the layer that would reject a + placeholder compares it. + + ``new_user`` refuses a duplicate email case-insensitively and after stripping, so + a lookup that is stricter than that resolves nothing, creates a placeholder, and + is refused by that same layer, which surfaces as a 500 on the whole group push. + """ + prisma_client = _member_resolution_prisma( + mocker, + users=set(), + teams=set(), + email_to_user_id={"member@example.com": "email-user"}, + ) + create_user_mock = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(return_value=None), + ) + + result = await _resolve_group_member_ids( + members=[SCIMMember(value=pushed)], + created_via="scim_group_membership", + prisma_client=prisma_client, + ) + + create_user_mock.assert_not_called() + assert result.all_member_ids == ["email-user"] + + +@pytest.mark.parametrize( + "population", + [ + {"email_to_user_ids": {"duplicate@example.com": ("email-user-a", "email-user-b")}}, + {"email_to_user_ids": {"duplicate@example.com": ("email-user-a",), "DUPLICATE@EXAMPLE.COM": ("email-user-b",)}}, + { + "sso_user_id_to_user_id": {"duplicate@example.com": "sso-user"}, + "email_to_user_id": {"duplicate@example.com": "email-user"}, + }, + ], + ids=["same-email-twice", "emails-differing-only-in-case", "one-account-by-sso-another-by-email"], +) +@pytest.mark.asyncio +async def test_resolve_group_member_ids_rejects_a_value_naming_two_accounts( + mocker, scim_upsert_user_enabled, caplog, population +): + """A value that names two accounts names a real person we cannot identify, so + the write is refused rather than attributed to one of them. + + Every shape of collision is refused, not just two rows holding the same email + verbatim: rows whose emails differ only in case are one row to the layer that + rejects duplicates, and a value that is one account's SSO identity and another's + email would otherwise be handed to whichever field happened to be searched first. + + It must not fall through to placeholder creation. That path can only fail: the + placeholder carries ``user_email`` set to the member value, which the duplicate + email check rejects, and the recovery lookup that follows searches by ``user_id`` + and so misses the very rows that caused the collision. The operator's data problem + then surfaces as an HTTP 500 the identity provider retries forever. + """ + prisma_client = _member_resolution_prisma(mocker, users=set(), teams=set(), **population) + create_user_mock = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(return_value=None), + ) + + with caplog.at_level(logging.WARNING, logger="LiteLLM Proxy"): + with pytest.raises(HTTPException) as exc_info: + await _resolve_group_member_ids( + members=[SCIMMember(value="duplicate@example.com")], + created_via="scim_group_membership", + prisma_client=prisma_client, + ) + + assert exc_info.value.status_code == 400 + assert "duplicate@example.com" in str(exc_info.value.detail) + assert "more than one" in str(exc_info.value.detail) + create_user_mock.assert_not_called() + assert any( + record.levelno >= logging.WARNING + and "duplicate@example.com" in record.getMessage() + and "more than one account" in record.getMessage() + for record in caplog.records + ) + + +@pytest.mark.asyncio +async def test_resolve_group_member_ids_does_not_fold_case_on_the_sso_identity(mocker, scim_upsert_user_enabled): + """An email and an SSO identity are not comparable the same way. + + OIDC defines ``sub`` as case-sensitive and nothing folds its case on the way in, + so two subjects differing only in case are two people. Folding it would hand the + group to an account the provider never named, which is the mis-grant the email + comparison is deliberately loose enough to avoid and this one is not. + """ + prisma_client = _member_resolution_prisma( + mocker, + users=set(), + teams=set(), + sso_user_id_to_user_id={"AbC-subject": "other-user"}, + ) + create_user_mock = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(return_value=NewUserResponse(user_id="abc-subject", key="placeholder-key")), + ) + + result = await _resolve_group_member_ids( + members=[SCIMMember(value="abc-subject")], + created_via="scim_group_membership", + prisma_client=prisma_client, + ) + + assert result.existing_member_ids == [] + assert result.all_member_ids == ["abc-subject"] + create_user_mock.assert_awaited_once_with(user_id="abc-subject", created_via="scim_group_membership") + + +@pytest.mark.asyncio +async def test_resolve_group_member_ids_ambiguous_email_outranks_upsert_rejection(mocker, scim_upsert_user_disabled): + """Ambiguity does not depend on scim_upsert_user, so the operator gets the + actionable message on either setting rather than being told to create a user that + already exists twice.""" + prisma_client = _member_resolution_prisma( + mocker, + users=set(), + teams=set(), + email_to_user_ids={"duplicate@example.com": ("email-user-a", "email-user-b")}, + ) + + with pytest.raises(HTTPException) as exc_info: + await _resolve_group_member_ids( + members=[SCIMMember(value="duplicate@example.com")], + created_via="scim_group_membership", + prisma_client=prisma_client, + ) + + assert exc_info.value.status_code == 400 + assert "more than one" in str(exc_info.value.detail) + assert "does not exist" not in str(exc_info.value.detail) + + +@pytest.mark.asyncio +async def test_create_group_rejects_ambiguous_member_email(mocker, scim_upsert_user_enabled): + """The refusal reaches the endpoint, so the identity provider sees a 400 on the + group write rather than a 500 it will retry.""" + scim_group = SCIMGroup( + schemas=["urn:ietf:params:scim:schemas:core:2.0:Group"], + id="ambiguous-group", + displayName="Ambiguous Group", + members=[SCIMMember(value="duplicate@example.com")], + ) + mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", + AsyncMock( + return_value=_member_resolution_prisma( + mocker, + users=set(), + teams=set(), + email_to_user_ids={"duplicate@example.com": ("email-user-a", "email-user-b")}, + ) + ), + ) + create_user_mock = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(return_value=None), + ) + + with pytest.raises(ProxyException) as exc_info: + await create_group(group=scim_group) + + assert int(exc_info.value.code) == 400 + assert "duplicate@example.com" in str(exc_info.value.message) + create_user_mock.assert_not_called() + + +@pytest.mark.parametrize( + "removed_by", + ["member@example.com", "member-sub"], + ids=["by-email", "by-sso-subject"], +) +@pytest.mark.asyncio +async def test_process_group_patch_remove_by_the_id_the_directory_added_with( + mocker, scim_upsert_user_enabled, removed_by +): + """A directory removes people by the same id it added them with. + + Resolving on add and not on remove would let someone keep a team after the + directory took them out of the group: the roster holds the canonical user id, so + subtracting the email or the subject the request names would match nothing. + """ + patch_ops = SCIMPatchOp( + schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations=[SCIMPatchOperation(op="remove", path="members", value=[{"value": removed_by}])], + ) + existing_team = LiteLLM_TeamTable( + team_id="parent-group", + team_alias="Parent Group", + members=[], + members_with_roles=[Member(user_id="real-user", role="user"), Member(user_id="keep-user", role="user")], + ) + create_user_mock = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(return_value=NewUserResponse(user_id="phantom-user", key="phantom-key")), + ) + + _, final_members, _ = await _process_group_patch_operations( + patch_ops=patch_ops, + existing_team=existing_team, + prisma_client=_member_resolution_prisma( + mocker, + users={"real-user", "keep-user"}, + teams=set(), + email_to_user_id={"member@example.com": "real-user"}, + sso_user_id_to_user_id={"member-sub": "real-user"}, + ), + ) + + create_user_mock.assert_not_called() + assert final_members == {"keep-user"} + + +@pytest.mark.asyncio +async def test_process_group_patch_remove_still_drops_a_placeholder_by_its_literal_id( + mocker, scim_upsert_user_enabled +): + """An earlier release put unmatched ids on the roster verbatim, so a remove has to + keep clearing the id as written even once it also resolves.""" + patch_ops = SCIMPatchOp( + schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations=[SCIMPatchOperation(op="remove", path="members", value=[{"value": "legacy@example.com"}])], + ) + existing_team = LiteLLM_TeamTable( + team_id="parent-group", + team_alias="Parent Group", + members=[], + members_with_roles=[Member(user_id="legacy@example.com", role="user"), Member(user_id="keep-user", role="user")], + ) + + _, final_members, _ = await _process_group_patch_operations( + patch_ops=patch_ops, + existing_team=existing_team, + prisma_client=_member_resolution_prisma(mocker, users={"keep-user"}, teams=set()), + ) + + assert final_members == {"keep-user"} + + +@pytest.mark.asyncio +async def test_process_group_patch_remove_when_the_id_turned_ambiguous_after_admission( + mocker, scim_upsert_user_enabled +): + """Ambiguity is a property of the table as it stands, not of the value. + + Someone admitted while their email was theirs alone must stay removable after a + second account takes that email. Resolving the removal against the whole table + would find two accounts, decline to pick, drop nobody, and still answer 200, + leaving the person the directory just removed holding the team. + """ + patch_ops = SCIMPatchOp( + schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations=[SCIMPatchOperation(op="remove", path="members", value=[{"value": "shared@example.com"}])], + ) + existing_team = LiteLLM_TeamTable( + team_id="parent-group", + team_alias="Parent Group", + members=[], + members_with_roles=[Member(user_id="admitted-user", role="user"), Member(user_id="keep-user", role="user")], + ) + + _, final_members, _ = await _process_group_patch_operations( + patch_ops=patch_ops, + existing_team=existing_team, + # the newcomer took the address but never joined the group + prisma_client=_member_resolution_prisma( + mocker, + users={"admitted-user", "keep-user"}, + teams=set(), + email_to_user_ids={"shared@example.com": ("admitted-user", "newcomer")}, + ), + ) + + assert final_members == {"keep-user"} + + +@pytest.mark.asyncio +async def test_process_group_patch_remove_refuses_a_value_naming_one_member_by_id_and_another_by_email( + mocker, scim_upsert_user_enabled +): + """One value must never revoke two people. + + A SCIM-provisioned account is keyed by its userName, so a canonical user id that + looks like an email is ordinary rather than exotic, and a second account can hold + that address as its email. Counting the id as written and the resolved accounts + separately makes each look singular, and the removal then takes both. + """ + patch_ops = SCIMPatchOp( + schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations=[SCIMPatchOperation(op="remove", path="members", value=[{"value": "shared@example.com"}])], + ) + existing_team = LiteLLM_TeamTable( + team_id="parent-group", + team_alias="Parent Group", + members=[], + members_with_roles=[ + Member(user_id="shared@example.com", role="user"), + Member(user_id="other-account", role="user"), + ], + ) + + with pytest.raises(HTTPException) as exc_info: + await _process_group_patch_operations( + patch_ops=patch_ops, + existing_team=existing_team, + prisma_client=_member_resolution_prisma( + mocker, + users={"shared@example.com", "other-account"}, + teams=set(), + email_to_user_id={"shared@example.com": "other-account"}, + ), + ) + + assert exc_info.value.status_code == 400 + assert "shared@example.com" in str(exc_info.value.detail) + assert "more than one member of this group" in str(exc_info.value.detail) + + +@pytest.mark.parametrize("position", [0, 1, 2], ids=["first", "middle", "last"]) +@pytest.mark.asyncio +async def test_process_group_patch_remove_finds_the_member_past_the_bounded_read( + mocker, scim_upsert_user_enabled, position +): + """A removal has to know *which* accounts a value names, not merely whether it + names several, so it reads them all. + + An add stops after two matches, which is all it needs to decide the value is + ambiguous. Reusing that bounded read here would silently drop the member whenever + the one on the roster sorted past the cap, which no fixture smaller than the cap + can show. The member is placed at each position so the test cannot pass by luck + of ordering. + """ + strangers = ["stranger-one", "stranger-two"] + sharers = tuple(strangers[:position] + ["admitted-user"] + strangers[position:]) + patch_ops = SCIMPatchOp( + schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations=[SCIMPatchOperation(op="remove", path="members", value=[{"value": "shared@example.com"}])], + ) + existing_team = LiteLLM_TeamTable( + team_id="parent-group", + team_alias="Parent Group", + members=[], + members_with_roles=[Member(user_id="admitted-user", role="user"), Member(user_id="keep-user", role="user")], + ) + + _, final_members, _ = await _process_group_patch_operations( + patch_ops=patch_ops, + existing_team=existing_team, + prisma_client=_member_resolution_prisma( + mocker, + users={"admitted-user", "keep-user"}, + teams=set(), + email_to_user_ids={"shared@example.com": sharers}, + ), + ) + + assert final_members == {"keep-user"} + + +@pytest.mark.asyncio +async def test_process_group_patch_remove_refuses_when_two_members_share_the_id(mocker, scim_upsert_user_enabled): + """When both accounts a value names are on the roster the removal is genuinely + undecidable, so it fails rather than reporting a removal it did not perform or + revoking a membership the directory did not name.""" + patch_ops = SCIMPatchOp( + schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations=[SCIMPatchOperation(op="remove", path="members", value=[{"value": "shared@example.com"}])], + ) + existing_team = LiteLLM_TeamTable( + team_id="parent-group", + team_alias="Parent Group", + members=[], + members_with_roles=[Member(user_id="member-a", role="user"), Member(user_id="member-b", role="user")], + ) + + with pytest.raises(HTTPException) as exc_info: + await _process_group_patch_operations( + patch_ops=patch_ops, + existing_team=existing_team, + prisma_client=_member_resolution_prisma( + mocker, + users={"member-a", "member-b"}, + teams=set(), + email_to_user_ids={"shared@example.com": ("member-a", "member-b")}, + ), + ) + + assert exc_info.value.status_code == 400 + assert "shared@example.com" in str(exc_info.value.detail) + assert "more than one member of this group" in str(exc_info.value.detail) + + + +@pytest.mark.asyncio +async def test_resolve_group_member_ids_exact_user_id_wins_when_it_names_nobody_else( + mocker, scim_upsert_user_enabled +): + """The canonical user id stays authoritative, including when the same account also + holds that value as its email, which is how a SCIM-provisioned account is keyed.""" + prisma_client = _member_resolution_prisma( + mocker, + users={"member-id"}, + teams=set(), + email_to_user_id={"member-id": "member-id"}, + ) + create_user_mock = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(), + ) + + result = await _resolve_group_member_ids( + members=[SCIMMember(value="member-id")], + created_via="scim_group_membership", + prisma_client=prisma_client, + ) + + create_user_mock.assert_not_called() + assert result.existing_member_ids == ["member-id"] + assert result.all_member_ids == ["member-id"] + + +@pytest.mark.parametrize( + "population", + [ + {"sso_user_id_to_user_id": {"member-id": "someone-else"}}, + {"email_to_user_id": {"member-id": "someone-else"}}, + ], + ids=["another-account-by-sso", "another-account-by-email"], +) +@pytest.mark.asyncio +async def test_resolve_group_member_ids_refuses_a_user_id_that_names_another_account( + mocker, scim_upsert_user_enabled, caplog, population +): + """An exact user id is checked for collisions like every other match. + + Taking it on sight would hand the group to whichever account happened to be keyed + by the value. The placeholders this bug provisioned are exactly that shape, since + they are keyed by the very id the provider keeps pushing, so on a tenant that + already has them the real account can never win. Refusing names the problem + instead of silently landing on the placeholder again. + """ + prisma_client = _member_resolution_prisma(mocker, users={"member-id"}, teams=set(), **population) + create_user_mock = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(return_value=None), + ) + + with caplog.at_level(logging.WARNING, logger="LiteLLM Proxy"): + with pytest.raises(HTTPException) as exc_info: + await _resolve_group_member_ids( + members=[SCIMMember(value="member-id")], + created_via="scim_group_membership", + prisma_client=prisma_client, + ) + + assert exc_info.value.status_code == 400 + assert "member-id" in str(exc_info.value.detail) + create_user_mock.assert_not_called() + assert any( + record.levelno >= logging.WARNING and "someone-else" in record.getMessage() for record in caplog.records + ) + + + +@pytest.mark.asyncio +async def test_resolve_group_member_ids_warns_before_creating_unmatched_placeholder( + mocker, scim_upsert_user_enabled, caplog +): + """An unmatched member still follows upsert behavior, but operators receive + a warning before the placeholder can leave an SSO user teamless.""" + prisma_client = _member_resolution_prisma(mocker, users=set(), teams=set()) + create_user_mock = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(return_value=NewUserResponse(user_id="placeholder", key="placeholder-key")), + ) + + with caplog.at_level(logging.WARNING, logger="LiteLLM Proxy"): + result = await _resolve_group_member_ids( + members=[SCIMMember(value="unmatched-id")], + created_via="scim_group_membership", + prisma_client=prisma_client, + ) + + create_user_mock.assert_awaited_once_with(user_id="unmatched-id", created_via="scim_group_membership") + assert result.existing_member_ids == [] + assert result.created_users == [NewUserResponse(user_id="placeholder", key="placeholder-key")] + assert result.all_member_ids == ["unmatched-id"] + assert any( + record.levelno >= logging.WARNING + and "unmatched-id" in record.getMessage() + and "matched no user by user_id, sso_user_id or user_email" in record.getMessage() + and "real account stays teamless" in record.getMessage() + for record in caplog.records + ) + + @pytest.mark.parametrize( "operation", [ @@ -4429,6 +5122,7 @@ async def test_get_groups_members_are_typed_as_users(mocker): mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock( return_value=mocker.MagicMock(user_id="member-1", user_email="member-1@example.com") ) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", @@ -4663,6 +5357,7 @@ async def test_update_group_roster_failure_propagates(mocker): mock_prisma_client.db.litellm_teamtable.update = AsyncMock(return_value=existing_team) mock_prisma_client.db.litellm_usertable = mocker.MagicMock() mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock()) + mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", @@ -4715,6 +5410,7 @@ async def test_resolve_group_member_ids_admits_member_created_concurrently(mocke prisma_client.db.litellm_usertable.find_unique = AsyncMock( side_effect=[None, LiteLLM_UserTable(user_id="raced-user")] ) + prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=()) mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", AsyncMock(return_value=None),