From da71c5fa160375ba9f9874ceb78f8793329f0ced Mon Sep 17 00:00:00 2001 From: Haji Akhundov Date: Tue, 2 Dec 2025 23:35:26 +0100 Subject: [PATCH 1/6] Fix SCIM GET /Users error when user_email is UUID Root cause fix: - Set user_email=None instead of user_id when creating users without email (scim_v2.py line 313) - Prevents UUIDs from being stored in user_email field in the first place Defensive fix: - Add validation in scim_transformations.py to check if user_email contains '@' before creating SCIMUserEmail - Handles existing users in database that may have UUIDs in user_email field - Prevents validation error when transforming users to SCIM format Fixes issue where GET /Users returns 500 error with message: 'value is not a valid email address: An email address must have an @-sign' --- .../management_endpoints/scim/scim_transformations.py | 8 +++++--- litellm/proxy/management_endpoints/scim/scim_v2.py | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/litellm/proxy/management_endpoints/scim/scim_transformations.py b/litellm/proxy/management_endpoints/scim/scim_transformations.py index 1bb59888405..a741ddd697a 100644 --- a/litellm/proxy/management_endpoints/scim/scim_transformations.py +++ b/litellm/proxy/management_endpoints/scim/scim_transformations.py @@ -40,7 +40,9 @@ class ScimTransformations: user_updated_at = user.updated_at.isoformat() if user.updated_at else None emails = [] - if user.user_email: + # Only add email if it's a valid email address (contains @) + # user_email can be a UUID when users are created without an email + if user.user_email and "@" in user.user_email: emails.append(SCIMUserEmail(value=user.user_email, primary=True)) return SCIMUser( @@ -126,7 +128,7 @@ class ScimTransformations: for member in team.members_with_roles or []: if isinstance(member, dict): member = Member(**member) - + scim_members.append( SCIMMember( value=ScimTransformations._get_scim_member_value(member), @@ -161,7 +163,7 @@ class ScimTransformations: elif hasattr(member, "user_id"): return member.user_id or ScimTransformations.DEFAULT_SCIM_MEMBER_VALUE return ScimTransformations.DEFAULT_SCIM_MEMBER_VALUE - + @staticmethod def _get_scim_member_display(member: Member) -> str: """ diff --git a/litellm/proxy/management_endpoints/scim/scim_v2.py b/litellm/proxy/management_endpoints/scim/scim_v2.py index b8f6b4a4460..5bce2807487 100644 --- a/litellm/proxy/management_endpoints/scim/scim_v2.py +++ b/litellm/proxy/management_endpoints/scim/scim_v2.py @@ -310,7 +310,7 @@ async def _create_user_if_not_exists( new_user_request = NewUserRequest( user_id=user_id, - user_email=user_id, # We don't have email from group membership + user_email=None, # We don't have email from group membership user_alias=None, teams=[], # Teams will be added separately metadata={"created_via": created_via}, From c01a4c6e57537b932a2e8a803862321f3e870d08 Mon Sep 17 00:00:00 2001 From: Haji Akhundov Date: Wed, 3 Dec 2025 14:41:10 +0100 Subject: [PATCH 2/6] Add tests for SCIM user_email UUID validation fix - Test user with UUID in user_email (defensive fix scenario) - Test user with None email (root cause fix scenario) - Verifies transformation doesn't fail and emails array is empty for invalid emails --- .../scim/test_scim_transformations.py | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py b/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py index bacdfb225fb..3217a1c4480 100644 --- a/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py +++ b/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py @@ -229,6 +229,76 @@ class TestScimTransformations: result = ScimTransformations._get_scim_member_value(member_without_email) assert result == member_without_email.user_id + @pytest.mark.asyncio + async def test_transform_user_with_uuid_as_email(self, mock_prisma_client): + """ + Test that users with UUID in user_email field (from old bug) + don't cause validation errors when transforming to SCIM format. + This tests the defensive fix that validates email contains '@' before creating SCIMUserEmail. + """ + mock_client, mock_find_unique = mock_prisma_client + + # Create a user with UUID in user_email (simulating the bug scenario) + user_with_uuid_email = LiteLLM_UserTable( + user_id="21df4e37-2f38-4f2e-a21b-c33cb939ff5b", + user_email="21df4e37-2f38-4f2e-a21b-c33cb939ff5b", # UUID as email (bug scenario) + user_alias=None, + teams=[], + created_at=None, + updated_at=None, + metadata={}, + ) + + mock_find_unique.return_value = None # No teams to look up + + with patch("litellm.proxy.proxy_server.prisma_client", mock_client): + # This should not raise a validation error + scim_user = await ScimTransformations.transform_litellm_user_to_scim_user( + user_with_uuid_email + ) + + # Verify the user was transformed successfully + assert scim_user.id == user_with_uuid_email.user_id + # Email should not be in emails array since it's a UUID (no '@' sign) + assert scim_user.emails is None or len(scim_user.emails) == 0 + # userName should fall back to default since email is invalid + assert scim_user.userName == ScimTransformations.DEFAULT_SCIM_DISPLAY_NAME + + @pytest.mark.asyncio + async def test_transform_user_with_none_email(self, mock_prisma_client): + """ + Test that users created via group membership (with user_email=None) + are transformed correctly without errors. + This tests the root cause fix. + """ + mock_client, mock_find_unique = mock_prisma_client + + # Create a user with None email (correct behavior after fix) + user_with_none_email = LiteLLM_UserTable( + user_id="user-from-group", + user_email=None, # None instead of UUID (after fix) + user_alias=None, + teams=[], + created_at=None, + updated_at=None, + metadata={"created_via": "scim_group_membership"}, + ) + + mock_find_unique.return_value = None # No teams to look up + + with patch("litellm.proxy.proxy_server.prisma_client", mock_client): + # This should not raise any errors + scim_user = await ScimTransformations.transform_litellm_user_to_scim_user( + user_with_none_email + ) + + # Verify the user was transformed successfully + assert scim_user.id == user_with_none_email.user_id + # Email should not be in emails array + assert scim_user.emails is None or len(scim_user.emails) == 0 + # userName should use default since no email + assert scim_user.userName == ScimTransformations.DEFAULT_SCIM_DISPLAY_NAME + class TestSCIMPatchOperations: """Test SCIM PATCH operation validation and case-insensitive handling""" From 8871a5670be867c24ced2d252c40e42215975274 Mon Sep 17 00:00:00 2001 From: Haji Akhundov Date: Wed, 3 Dec 2025 14:50:03 +0100 Subject: [PATCH 3/6] Simplify SCIM tests to focus on core issue --- .../scim/test_scim_transformations.py | 43 +++++++------------ 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py b/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py index 3217a1c4480..be9c79911d0 100644 --- a/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py +++ b/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py @@ -232,13 +232,11 @@ class TestScimTransformations: @pytest.mark.asyncio async def test_transform_user_with_uuid_as_email(self, mock_prisma_client): """ - Test that users with UUID in user_email field (from old bug) - don't cause validation errors when transforming to SCIM format. + Test that users with UUID in user_email don't cause validation errors. This tests the defensive fix that validates email contains '@' before creating SCIMUserEmail. """ mock_client, mock_find_unique = mock_prisma_client - - # Create a user with UUID in user_email (simulating the bug scenario) + user_with_uuid_email = LiteLLM_UserTable( user_id="21df4e37-2f38-4f2e-a21b-c33cb939ff5b", user_email="21df4e37-2f38-4f2e-a21b-c33cb939ff5b", # UUID as email (bug scenario) @@ -248,56 +246,45 @@ class TestScimTransformations: updated_at=None, metadata={}, ) - - mock_find_unique.return_value = None # No teams to look up - + + mock_find_unique.return_value = None + with patch("litellm.proxy.proxy_server.prisma_client", mock_client): - # This should not raise a validation error scim_user = await ScimTransformations.transform_litellm_user_to_scim_user( user_with_uuid_email ) - - # Verify the user was transformed successfully + assert scim_user.id == user_with_uuid_email.user_id - # Email should not be in emails array since it's a UUID (no '@' sign) assert scim_user.emails is None or len(scim_user.emails) == 0 - # userName should fall back to default since email is invalid - assert scim_user.userName == ScimTransformations.DEFAULT_SCIM_DISPLAY_NAME @pytest.mark.asyncio async def test_transform_user_with_none_email(self, mock_prisma_client): """ - Test that users created via group membership (with user_email=None) - are transformed correctly without errors. + Test that users with user_email=None are transformed correctly. This tests the root cause fix. """ mock_client, mock_find_unique = mock_prisma_client - - # Create a user with None email (correct behavior after fix) + user_with_none_email = LiteLLM_UserTable( user_id="user-from-group", - user_email=None, # None instead of UUID (after fix) + user_email=None, user_alias=None, teams=[], created_at=None, updated_at=None, - metadata={"created_via": "scim_group_membership"}, + metadata={}, ) - - mock_find_unique.return_value = None # No teams to look up - + + mock_find_unique.return_value = None + with patch("litellm.proxy.proxy_server.prisma_client", mock_client): - # This should not raise any errors scim_user = await ScimTransformations.transform_litellm_user_to_scim_user( user_with_none_email ) - - # Verify the user was transformed successfully + assert scim_user.id == user_with_none_email.user_id - # Email should not be in emails array assert scim_user.emails is None or len(scim_user.emails) == 0 - # userName should use default since no email - assert scim_user.userName == ScimTransformations.DEFAULT_SCIM_DISPLAY_NAME + class TestSCIMPatchOperations: From e5baff780a561d8ca24fc16c66e48733a049d114 Mon Sep 17 00:00:00 2001 From: Haji Akhundov Date: Wed, 3 Dec 2025 20:44:54 +0100 Subject: [PATCH 4/6] Enforce SCIM 2.0 compliance: reject non-existent users in groups - Reject group operations when users don't exist (security fix) - Prevents unauthorized user provisioning via group membership - Aligns with SCIM 2.0 protocol: users must exist before group membership - Add validation for empty user IDs - Update tests to verify rejection behavior This is a breaking change but necessary for security and SCIM compliance. Users must be created via POST /Users before being added to groups. --- .../management_endpoints/scim/scim_v2.py | 67 +++++-- .../scim/test_scim_v2_endpoints.py | 185 +++--------------- 2 files changed, 68 insertions(+), 184 deletions(-) diff --git a/litellm/proxy/management_endpoints/scim/scim_v2.py b/litellm/proxy/management_endpoints/scim/scim_v2.py index 5bce2807487..4ec13177e0b 100644 --- a/litellm/proxy/management_endpoints/scim/scim_v2.py +++ b/litellm/proxy/management_endpoints/scim/scim_v2.py @@ -206,20 +206,35 @@ def _build_scim_metadata( async def _extract_group_member_ids(group: SCIMGroup) -> GroupMemberExtractionResult: """ - Extract member IDs from SCIMGroup, creating users that don't exist. + Extract member IDs from SCIMGroup, validating that all users exist. + + Per SCIM 2.0 protocol, groups should only reference existing users. + Users must be created via POST /Users before being added to groups. Returns: - GroupMemberExtractionResult with existing members, created users, and all member IDs + GroupMemberExtractionResult with existing members and all member IDs + + Raises: + HTTPException: If any member user does not exist (400 Bad Request) """ prisma_client = await _get_prisma_client_or_raise_exception() existing_member_ids = [] - created_users = [] + created_users = [] # Always empty - users must exist before group membership all_member_ids = [] if group.members: for member in group.members: user_id = member.value + # Validate user_id is not empty + if not user_id or not user_id.strip(): + raise HTTPException( + status_code=400, + detail={ + "error": "Invalid member: user ID cannot be empty." + }, + ) + # Check if user exists user = await prisma_client.db.litellm_usertable.find_unique( where={"user_id": user_id} @@ -229,16 +244,17 @@ async def _extract_group_member_ids(group: SCIMGroup) -> GroupMemberExtractionRe existing_member_ids.append(user_id) all_member_ids.append(user_id) else: - # Create the user if they don't exist using our helper - created_user = await _create_user_if_not_exists( - user_id=user_id, created_via="scim_group_membership" + # User doesn't exist - reject per SCIM 2.0 protocol + # This prevents security issues where users not assigned to app + # get provisioned via group membership + raise HTTPException( + status_code=400, + detail={ + "error": f"User with ID '{user_id}' does not exist. " + "Please create the user first via POST /Users before adding to group." + }, ) - if created_user: - created_users.append(created_user) - all_member_ids.append(user_id) - # If creation failed, user is skipped (logged in helper) - return GroupMemberExtractionResult( existing_member_ids=existing_member_ids, created_users=created_users, @@ -1024,7 +1040,7 @@ async def create_group( detail={"error": f"Group already exists with ID: {team_id}"}, ) - # Extract and process group members (creating users that don't exist) + # Extract and validate group members (all users must exist) member_result = await _extract_group_member_ids(group) members_with_roles = [ Member(user_id=member_id, role="user") @@ -1072,7 +1088,7 @@ async def update_group( prisma_client = await _get_prisma_client_or_raise_exception() existing_team = await _check_team_exists(group_id) - # Extract and process group members (creating users that don't exist) + # Extract and validate group members (all users must exist) member_result = await _extract_group_member_ids(group) verbose_proxy_logger.debug( f"SCIM PUT GROUP all_member_ids: {member_result.all_member_ids}" @@ -1189,24 +1205,33 @@ async def _process_group_patch_operations( elif path.startswith("members"): # Handle member operations member_values = _extract_group_values(value) - # Create users that don't exist and get all valid member IDs + # Validate all users exist - per SCIM 2.0, users must exist before group membership valid_members = [] for member_id in member_values: + # Validate member_id is not empty + if not member_id or not member_id.strip(): + raise HTTPException( + status_code=400, + detail={ + "error": "Invalid member: user ID cannot be empty." + }, + ) + user = await prisma_client.db.litellm_usertable.find_unique( where={"user_id": member_id} ) if user: valid_members.append(member_id) else: - # Create the user if they don't exist using our helper - created_user = await _create_user_if_not_exists( - user_id=member_id, created_via="scim_group_patch" + # User doesn't exist - reject per SCIM 2.0 protocol + raise HTTPException( + status_code=400, + detail={ + "error": f"User with ID '{member_id}' does not exist. " + "Please create the user first via POST /Users before adding to group." + }, ) - if created_user: - valid_members.append(member_id) - # If creation failed, user is skipped (logged in helper) - if op_type == "replace": final_members = set(valid_members) elif op_type == "add": 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 230e251a5d0..15c4272fa90 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 @@ -915,10 +915,11 @@ async def test_update_group_e2e(mocker): @pytest.mark.asyncio -async def test_create_group_with_nonexistent_users_creates_users(mocker): +async def test_create_group_with_nonexistent_users_rejects(mocker): """ - Test that creating a group with non-existent users creates those users. - This tests the scenario: Group Push ['new user', existing users...] + Test that creating a group with non-existent users is rejected. + Per SCIM 2.0 protocol, users must exist before being added to groups. + This prevents security issues where users not assigned to app get provisioned via group membership. """ # Test data group_id = "test-group-123" @@ -934,7 +935,7 @@ async def test_create_group_with_nonexistent_users_creates_users(mocker): ) ######################################################### - # We expect new-user-1 and new-user-2 to be created + # We expect the request to be rejected with 400 error ######################################################### # Mock prisma client @@ -963,95 +964,21 @@ async def test_create_group_with_nonexistent_users_creates_users(mocker): AsyncMock(return_value=mock_prisma_client) ) - # Mock new_user function to track user creation - mock_new_user = mocker.patch( - "litellm.proxy.management_endpoints.internal_user_endpoints.new_user", - AsyncMock() - ) + # Execute the create_group function - should raise HTTPException + with pytest.raises(HTTPException) as exc_info: + await create_group(group=scim_group) - # Mock created users return values - def mock_new_user_side_effect(data): - from litellm.proxy._types import NewUserResponse - return NewUserResponse( - key="sk-test-key-" + data.user_id, # Required field from GenerateKeyResponse - user_id=data.user_id, - user_email=data.user_email, - metadata=data.metadata, - teams=data.teams, - user_role=data.user_role - ) - - mock_new_user.side_effect = mock_new_user_side_effect - - # Mock new_team function - mock_created_team = mocker.MagicMock() - mock_created_team.team_id = group_id - mock_created_team.team_alias = "Test Group" - - mock_new_team = mocker.patch( - "litellm.proxy.management_endpoints.scim.scim_v2.new_team", - AsyncMock(return_value=mock_created_team) - ) - - # Mock SCIM transformation - expected_scim_response = SCIMGroup( - schemas=["urn:ietf:params:scim:schemas:core:2.0:Group"], - id=group_id, - displayName="Test Group", - members=[ - SCIMMember(value="existing-user", display="existing-user"), - SCIMMember(value="new-user-1", display="new-user-1"), - SCIMMember(value="new-user-2", display="new-user-2") - ] - ) - mocker.patch( - "litellm.proxy.management_endpoints.scim.scim_v2.ScimTransformations.transform_litellm_team_to_scim_group", - AsyncMock(return_value=expected_scim_response) - ) - - # Execute the create_group function - result = await create_group(group=scim_group) - - ######################################################### - # Assert that new-user-1 and new-user-2 were created - ######################################################### - - # Verify that new_user was called exactly twice (for new-user-1 and new-user-2) - assert mock_new_user.call_count == 2 - - # Check the user creation calls - created_user_ids = set() - for call in mock_new_user.call_args_list: - user_request = call.kwargs["data"] - created_user_ids.add(user_request.user_id) - assert user_request.metadata["created_via"] == "scim_group_membership" - assert user_request.user_role == LitellmUserRoles.INTERNAL_USER_VIEW_ONLY - assert user_request.auto_create_key is False - assert user_request.teams == [] # Teams added separately - - assert created_user_ids == {"new-user-1", "new-user-2"} - - # Verify team creation was called with all members (existing + created) - mock_new_team.assert_called_once() - team_request = mock_new_team.call_args.kwargs["data"] - assert team_request.team_id == group_id - assert team_request.team_alias == "Test Group" - - # Verify all members are in the team (existing + newly created) - member_user_ids = {member.user_id for member in team_request.members_with_roles} - assert member_user_ids == {"existing-user", "new-user-1", "new-user-2"} - - # Verify response - assert result.id == group_id - assert result.displayName == "Test Group" - assert len(result.members) == 3 + # Verify it's a 400 Bad Request + assert exc_info.value.status_code == 400 + assert "does not exist" in str(exc_info.value.detail) + assert "new-user-1" in str(exc_info.value.detail) or "new-user-2" in str(exc_info.value.detail) @pytest.mark.asyncio -async def test_update_group_with_nonexistent_users_creates_users(mocker): +async def test_update_group_with_nonexistent_users_rejects(mocker): """ - Test that updating a group with non-existent users creates those users. - This tests the scenario where a group is updated with members that don't exist in user table. + Test that updating a group with non-existent users is rejected. + Per SCIM 2.0 protocol, users must exist before being added to groups. """ # Test data group_id = "existing-group-456" @@ -1114,79 +1041,11 @@ async def test_update_group_with_nonexistent_users_creates_users(mocker): AsyncMock(return_value=mock_existing_team) ) - # Mock new_user function to track user creation - mock_new_user = mocker.patch( - "litellm.proxy.management_endpoints.internal_user_endpoints.new_user", - AsyncMock() - ) + # Execute the update_group function - should raise HTTPException + with pytest.raises(HTTPException) as exc_info: + await update_group(group_id=group_id, group=scim_group_update) - # Mock created users return values - def mock_new_user_side_effect(data): - from litellm.proxy._types import NewUserResponse - return NewUserResponse( - key="sk-test-key-" + data.user_id, # Required field from GenerateKeyResponse - user_id=data.user_id, - user_email=data.user_email, - metadata=data.metadata, - teams=data.teams, - user_role=data.user_role - ) - - mock_new_user.side_effect = mock_new_user_side_effect - - # Mock group membership changes - mock_handle_group_membership_changes = mocker.patch( - "litellm.proxy.management_endpoints.scim.scim_v2._handle_group_membership_changes", - AsyncMock() - ) - - # Mock SCIM transformation - expected_scim_response = SCIMGroup( - schemas=["urn:ietf:params:scim:schemas:core:2.0:Group"], - id=group_id, - displayName="Updated Group Name", - members=[ - SCIMMember(value="existing-user", display="existing-user"), - SCIMMember(value="new-user-3", display="new-user-3"), - SCIMMember(value="new-user-4", display="new-user-4") - ] - ) - mocker.patch( - "litellm.proxy.management_endpoints.scim.scim_v2.ScimTransformations.transform_litellm_team_to_scim_group", - AsyncMock(return_value=expected_scim_response) - ) - - # Execute the update_group function - result = await update_group(group_id=group_id, group=scim_group_update) - - # Verify that new_user was called exactly twice (for new-user-3 and new-user-4) - assert mock_new_user.call_count == 2 - - # Check the user creation calls - created_user_ids = set() - for call in mock_new_user.call_args_list: - user_request = call.kwargs["data"] - created_user_ids.add(user_request.user_id) - assert user_request.metadata["created_via"] == "scim_group_membership" - assert user_request.user_role == LitellmUserRoles.INTERNAL_USER_VIEW_ONLY - assert user_request.auto_create_key is False - assert user_request.teams == [] # Teams added separately - - assert created_user_ids == {"new-user-3", "new-user-4"} - - # Verify team update was called - mock_prisma_client.db.litellm_teamtable.update.assert_called_once() - update_call = mock_prisma_client.db.litellm_teamtable.update.call_args - assert update_call[1]["where"]["team_id"] == group_id - assert update_call[1]["data"]["team_alias"] == "Updated Group Name" - - # Verify group membership changes were handled with all members (existing + created) - mock_handle_group_membership_changes.assert_called_once() - membership_call = mock_handle_group_membership_changes.call_args - assert membership_call[1]["group_id"] == group_id - assert membership_call[1]["final_members"] == {"existing-user", "new-user-3", "new-user-4"} - - # Verify response - assert result.id == group_id - assert result.displayName == "Updated Group Name" - assert len(result.members) == 3 \ No newline at end of file + # Verify it's a 400 Bad Request + assert exc_info.value.status_code == 400 + assert "does not exist" in str(exc_info.value.detail) + assert "new-user-3" in str(exc_info.value.detail) or "new-user-4" in str(exc_info.value.detail) \ No newline at end of file From 69b32dd8702aca128a68a0b607c0384e20dde59b Mon Sep 17 00:00:00 2001 From: Haji Akhundov Date: Wed, 3 Dec 2025 21:06:32 +0100 Subject: [PATCH 5/6] Fix ruff linting errors: remove unused imports and variables --- .../scim/test_scim_transformations.py | 9 --------- .../management_endpoints/scim/test_scim_v2_endpoints.py | 9 +++------ 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py b/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py index be9c79911d0..e5857a10967 100644 --- a/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py +++ b/tests/test_litellm/proxy/management_endpoints/scim/test_scim_transformations.py @@ -1,14 +1,8 @@ -import asyncio -import json import os import sys -from litellm._uuid import uuid -from typing import Optional, cast from unittest.mock import AsyncMock, MagicMock, patch import pytest -from fastapi import HTTPException -from fastapi.testclient import TestClient sys.path.insert( 0, os.path.abspath("../../../") @@ -19,10 +13,7 @@ from litellm.proxy.management_endpoints.scim.scim_transformations import ( ScimTransformations, ) from litellm.types.proxy.management_endpoints.scim_v2 import ( - SCIMGroup, - SCIMPatchOp, SCIMPatchOperation, - SCIMUser, ) 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 15c4272fa90..cf0b9c58760 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 @@ -15,7 +15,6 @@ from litellm.proxy.management_endpoints.scim.scim_v2 import ( update_user, ) from litellm.types.proxy.management_endpoints.scim_v2 import ( - SCIMFeature, SCIMGroup, SCIMMember, SCIMPatchOp, @@ -428,7 +427,7 @@ async def test_update_user_success(mocker): "litellm.proxy.management_endpoints.scim.scim_v2._handle_team_membership_changes", AsyncMock() ) - mock_transform = mocker.patch( + mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2.ScimTransformations.transform_litellm_user_to_scim_user", AsyncMock(return_value=response_scim_user) ) @@ -524,7 +523,7 @@ async def test_patch_user_success(mocker): "litellm.proxy.management_endpoints.scim.scim_v2._handle_team_membership_changes", AsyncMock() ) - mock_transform = mocker.patch( + mocker.patch( "litellm.proxy.management_endpoints.scim.scim_v2.ScimTransformations.transform_litellm_user_to_scim_user", AsyncMock(return_value=response_scim_user) ) @@ -660,7 +659,7 @@ async def test_update_group_metadata_serialization_issue(mocker): ) # Call the function that had the bug - result = await update_group(group_id=group_id, group=scim_group) + await update_group(group_id=group_id, group=scim_group) # Verify the team update was called mock_prisma_client.db.litellm_teamtable.update.assert_called_once() @@ -696,7 +695,6 @@ async def test_team_membership_management(mocker): from litellm.proxy.management_endpoints.scim.scim_v2 import ( _get_team_member_user_ids_from_team, _handle_group_membership_changes, - patch_team_membership, ) # Mock team with members_with_roles as source of truth @@ -772,7 +770,6 @@ async def test_update_group_e2e(mocker): from litellm.proxy.management_endpoints.scim.scim_transformations import ( ScimTransformations, ) - from litellm.proxy.utils import safe_dumps # Setup test data group_id = "test-team-123" From 6022d37eefe259e2ad4665105da73b601328d75e Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Fri, 9 Jan 2026 15:07:53 -0800 Subject: [PATCH 6/6] patching with feature flag --- .../management_endpoints/scim/scim_v2.py | 100 +++-- .../scim/test_scim_v2_endpoints.py | 397 +++++++++++++++++- 2 files changed, 459 insertions(+), 38 deletions(-) diff --git a/litellm/proxy/management_endpoints/scim/scim_v2.py b/litellm/proxy/management_endpoints/scim/scim_v2.py index ce56ce3be85..0965198bad9 100644 --- a/litellm/proxy/management_endpoints/scim/scim_v2.py +++ b/litellm/proxy/management_endpoints/scim/scim_v2.py @@ -206,23 +206,52 @@ def _build_scim_metadata( return metadata +async def _get_scim_upsert_user_setting() -> bool: + """ + Get the scim_upsert_user setting from litellm_settings. + + Returns: + True if scim_upsert_user is not set or is True (default behavior), + False if scim_upsert_user is explicitly set to False (SCIM 2.0 strict mode) + """ + try: + from litellm.proxy.proxy_server import proxy_config + + config = await proxy_config.get_config() + litellm_settings = config.get("litellm_settings", {}) or {} + scim_upsert_user = litellm_settings.get("scim_upsert_user", True) + + # Default to True if not set (backward compatibility) + return bool(scim_upsert_user) + except Exception as e: + verbose_proxy_logger.warning( + f"Error reading scim_upsert_user setting, defaulting to True: {e}" + ) + # Default to True for backward compatibility + return True + + async def _extract_group_member_ids(group: SCIMGroup) -> GroupMemberExtractionResult: """ Extract member IDs from SCIMGroup, validating that all users exist. - Per SCIM 2.0 protocol, groups should only reference existing users. - Users must be created via POST /Users before being added to groups. + Behavior depends on litellm_settings.scim_upsert_user: + - If True (default): Creates users that don't exist (backward compatible) + - If False: Rejects non-existent users per SCIM 2.0 protocol Returns: - GroupMemberExtractionResult with existing members and all member IDs + GroupMemberExtractionResult with existing members, created users, and all member IDs Raises: - HTTPException: If any member user does not exist (400 Bad Request) + HTTPException: If scim_upsert_user is False and any member user does not exist (400 Bad Request) """ prisma_client = await _get_prisma_client_or_raise_exception() existing_member_ids = [] - created_users = [] # Always empty - users must exist before group membership + created_users = [] all_member_ids = [] + + # Check the feature flag + scim_upsert_user = await _get_scim_upsert_user_setting() if group.members: for member in group.members: @@ -246,16 +275,26 @@ async def _extract_group_member_ids(group: SCIMGroup) -> GroupMemberExtractionRe existing_member_ids.append(user_id) all_member_ids.append(user_id) else: - # User doesn't exist - reject per SCIM 2.0 protocol - # This prevents security issues where users not assigned to app - # get provisioned via group membership - raise HTTPException( - status_code=400, - detail={ - "error": f"User with ID '{user_id}' does not exist. " - "Please create the user first via POST /Users before adding to group." - }, - ) + if scim_upsert_user: + # Create the user if they don't exist (backward compatible behavior) + created_user = await _create_user_if_not_exists( + user_id=user_id, created_via="scim_group_membership" + ) + if created_user: + created_users.append(created_user) + all_member_ids.append(user_id) + # If creation failed, user is skipped (logged in helper) + else: + # User doesn't exist - reject per SCIM 2.0 protocol + # This prevents security issues where users not assigned to app + # get provisioned via group membership + raise HTTPException( + status_code=400, + detail={ + "error": f"User with ID '{user_id}' does not exist. " + "Please create the user first via POST /Users before adding to group." + }, + ) return GroupMemberExtractionResult( existing_member_ids=existing_member_ids, @@ -328,7 +367,7 @@ async def _create_user_if_not_exists( new_user_request = NewUserRequest( user_id=user_id, - user_email=None, # We don't have email from group membership + user_email=user_id, # We don't have email from group membership user_alias=None, teams=[], # Teams will be added separately metadata={"created_via": created_via}, @@ -1220,7 +1259,9 @@ async def _process_group_patch_operations( elif path.startswith("members"): # Handle member operations member_values = _extract_group_values(value) - # Validate all users exist - per SCIM 2.0, users must exist before group membership + # Check the feature flag + scim_upsert_user = await _get_scim_upsert_user_setting() + # Validate all users exist or create them based on feature flag valid_members = [] for member_id in member_values: # Validate member_id is not empty @@ -1238,14 +1279,23 @@ async def _process_group_patch_operations( if user: valid_members.append(member_id) else: - # User doesn't exist - reject per SCIM 2.0 protocol - raise HTTPException( - status_code=400, - detail={ - "error": f"User with ID '{member_id}' does not exist. " - "Please create the user first via POST /Users before adding to group." - }, - ) + if scim_upsert_user: + # Create the user if they don't exist (backward compatible behavior) + created_user = await _create_user_if_not_exists( + user_id=member_id, created_via="scim_group_patch" + ) + if created_user: + valid_members.append(member_id) + # If creation failed, user is skipped (logged in helper) + else: + # User doesn't exist - reject per SCIM 2.0 protocol + raise HTTPException( + status_code=400, + detail={ + "error": f"User with ID '{member_id}' does not exist. " + "Please create the user first via POST /Users before adding to group." + }, + ) if op_type == "replace": final_members = set(valid_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 21dc8b399e8..f8affda25d6 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 @@ -3,10 +3,12 @@ from unittest.mock import AsyncMock import pytest from fastapi import HTTPException -from litellm.proxy._types import LitellmUserRoles, NewUserRequest, ProxyException +from litellm.proxy._types import LitellmUserRoles, NewUserRequest, NewUserResponse, ProxyException from litellm.proxy.management_endpoints.scim.scim_v2 import ( UserProvisionerHelpers, + _extract_group_member_ids, _handle_team_membership_changes, + _process_group_patch_operations, create_group, create_user, get_service_provider_config, @@ -913,12 +915,23 @@ async def test_update_group_e2e(mocker): @pytest.mark.asyncio -async def test_create_group_with_nonexistent_users_rejects(mocker): +async def test_create_group_with_nonexistent_users_rejects(mocker, monkeypatch): """ - Test that creating a group with non-existent users is rejected. + Test that creating a group with non-existent users is rejected when scim_upsert_user is False. Per SCIM 2.0 protocol, users must exist before being added to groups. This prevents security issues where users not assigned to app get provisioned via group membership. """ + # Mock the feature flag to False (SCIM 2.0 strict mode) + async def mock_get_config(): + return { + "litellm_settings": { + "scim_upsert_user": False + } + } + + from litellm.proxy.proxy_server import proxy_config + monkeypatch.setattr(proxy_config, "get_config", mock_get_config) + # Test data group_id = "test-group-123" scim_group = SCIMGroup( @@ -962,22 +975,33 @@ async def test_create_group_with_nonexistent_users_rejects(mocker): AsyncMock(return_value=mock_prisma_client) ) - # Execute the create_group function - should raise HTTPException - with pytest.raises(HTTPException) as exc_info: + # Execute the create_group function - should raise ProxyException + with pytest.raises(ProxyException) as exc_info: await create_group(group=scim_group) # Verify it's a 400 Bad Request - assert exc_info.value.status_code == 400 - assert "does not exist" in str(exc_info.value.detail) - assert "new-user-1" in str(exc_info.value.detail) or "new-user-2" in str(exc_info.value.detail) + assert int(exc_info.value.code) == 400 + assert "does not exist" in str(exc_info.value.message) + assert "new-user-1" in str(exc_info.value.message) or "new-user-2" in str(exc_info.value.message) @pytest.mark.asyncio -async def test_update_group_with_nonexistent_users_rejects(mocker): +async def test_update_group_with_nonexistent_users_rejects(mocker, monkeypatch): """ - Test that updating a group with non-existent users is rejected. + Test that updating a group with non-existent users is rejected when scim_upsert_user is False. Per SCIM 2.0 protocol, users must exist before being added to groups. """ + # Mock the feature flag to False (SCIM 2.0 strict mode) + async def mock_get_config(): + return { + "litellm_settings": { + "scim_upsert_user": False + } + } + + from litellm.proxy.proxy_server import proxy_config + monkeypatch.setattr(proxy_config, "get_config", mock_get_config) + # Test data group_id = "existing-group-456" @@ -1039,11 +1063,358 @@ async def test_update_group_with_nonexistent_users_rejects(mocker): AsyncMock(return_value=mock_existing_team) ) - # Execute the update_group function - should raise HTTPException - with pytest.raises(HTTPException) as exc_info: + # Execute the update_group function - should raise ProxyException + with pytest.raises(ProxyException) as exc_info: await update_group(group_id=group_id, group=scim_group_update) + # Verify it's a 400 Bad Request + assert int(exc_info.value.code) == 400 + assert "does not exist" in str(exc_info.value.message) + assert "new-user-3" in str(exc_info.value.message) or "new-user-4" in str(exc_info.value.message) + + +@pytest.mark.asyncio +async def test_create_group_with_nonexistent_users_creates_when_flag_true(mocker, monkeypatch): + """ + Test that creating a group with non-existent users creates them when scim_upsert_user is True. + This preserves backward compatible behavior. + """ + # Mock the feature flag to True (backward compatible mode) + async def mock_get_config(): + return { + "litellm_settings": { + "scim_upsert_user": True + } + } + + from litellm.proxy.proxy_server import proxy_config + monkeypatch.setattr(proxy_config, "get_config", mock_get_config) + + # Test data + group_id = "test-group-123" + scim_group = SCIMGroup( + schemas=["urn:ietf:params:scim:schemas:core:2.0:Group"], + id=group_id, + displayName="Test Group", + members=[ + SCIMMember(value="existing-user", display="Existing User"), # This user exists + SCIMMember(value="new-user-1", display="New User 1"), # This user doesn't exist - should be created + SCIMMember(value="new-user-2", display="New User 2"), # This user doesn't exist - should be created + ] + ) + + # Mock prisma client + mock_prisma_client = mocker.MagicMock() + mock_prisma_client.db = mocker.MagicMock() + mock_prisma_client.db.litellm_teamtable = mocker.MagicMock() + mock_prisma_client.db.litellm_usertable = mocker.MagicMock() + + # Mock team operations - team doesn't exist yet + mock_prisma_client.db.litellm_teamtable.find_unique = AsyncMock(return_value=None) + + # Mock user lookup - only existing-user exists initially + def mock_user_lookup(where): + user_id = where["user_id"] + if user_id == "existing-user": + mock_user = mocker.MagicMock() + mock_user.user_id = user_id + return mock_user + 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 user creation + created_user_1 = NewUserResponse(user_id="new-user-1", key="test-key-1") + created_user_2 = NewUserResponse(user_id="new-user-2", key="test-key-2") + mock_create_user = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(side_effect=[created_user_1, created_user_2]) + ) + + # Mock new_team + mock_team = mocker.MagicMock() + mock_team.team_id = group_id + mock_new_team = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2.new_team", + AsyncMock(return_value=mock_team) + ) + + # Mock transformation + mock_scim_group = SCIMGroup( + schemas=["urn:ietf:params:scim:schemas:core:2.0:Group"], + id=group_id, + displayName="Test Group", + members=[] + ) + mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2.ScimTransformations.transform_litellm_team_to_scim_group", + AsyncMock(return_value=mock_scim_group) + ) + + # Mock dependencies + mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", + AsyncMock(return_value=mock_prisma_client) + ) + + # Execute the create_group function - should succeed + result = await create_group(group=scim_group) + + # Verify users were created + assert mock_create_user.call_count == 2 + assert mock_create_user.call_args_list[0].kwargs['user_id'] == "new-user-1" + assert mock_create_user.call_args_list[1].kwargs['user_id'] == "new-user-2" + + # Verify team was created + mock_new_team.assert_called_once() + + +@pytest.mark.asyncio +async def test_extract_group_member_ids_with_flag_true_creates_users(mocker, monkeypatch): + """ + Test that _extract_group_member_ids creates users when scim_upsert_user is True. + """ + # Mock the feature flag to True (backward compatible mode) + async def mock_get_config(): + return { + "litellm_settings": { + "scim_upsert_user": True + } + } + + from litellm.proxy.proxy_server import proxy_config + monkeypatch.setattr(proxy_config, "get_config", mock_get_config) + + # Test data + scim_group = SCIMGroup( + schemas=["urn:ietf:params:scim:schemas:core:2.0:Group"], + id="test-group", + displayName="Test Group", + members=[ + SCIMMember(value="existing-user", display="Existing User"), # This user exists + SCIMMember(value="new-user-1", display="New User 1"), # This user doesn't exist - should be created + ] + ) + + # Mock prisma client + mock_prisma_client = mocker.MagicMock() + mock_prisma_client.db = mocker.MagicMock() + mock_prisma_client.db.litellm_usertable = mocker.MagicMock() + + # Mock user lookup - only existing-user exists initially + def mock_user_lookup(where): + user_id = where["user_id"] + if user_id == "existing-user": + mock_user = mocker.MagicMock() + mock_user.user_id = user_id + return mock_user + return None # new-user-1 doesn't exist + + mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(side_effect=mock_user_lookup) + + # Mock user creation + created_user = NewUserResponse(user_id="new-user-1", key="test-key-1") + mock_create_user = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(return_value=created_user) + ) + + # Mock dependencies + mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", + AsyncMock(return_value=mock_prisma_client) + ) + + # Execute the function + result = await _extract_group_member_ids(scim_group) + + # Verify result + assert "existing-user" in result.existing_member_ids + assert "existing-user" in result.all_member_ids + assert "new-user-1" in result.all_member_ids + assert len(result.created_users) == 1 + + # Verify user was created + mock_create_user.assert_called_once_with( + user_id="new-user-1", + created_via="scim_group_membership" + ) + + +@pytest.mark.asyncio +async def test_extract_group_member_ids_with_flag_false_rejects(mocker, monkeypatch): + """ + Test that _extract_group_member_ids rejects non-existent users when scim_upsert_user is False. + """ + # Mock the feature flag to False (SCIM 2.0 strict mode) + async def mock_get_config(): + return { + "litellm_settings": { + "scim_upsert_user": False + } + } + + from litellm.proxy.proxy_server import proxy_config + monkeypatch.setattr(proxy_config, "get_config", mock_get_config) + + # Test data + scim_group = SCIMGroup( + schemas=["urn:ietf:params:scim:schemas:core:2.0:Group"], + id="test-group", + displayName="Test Group", + members=[ + SCIMMember(value="existing-user", display="Existing User"), # This user exists + SCIMMember(value="new-user-1", display="New User 1"), # This user doesn't exist - should be rejected + ] + ) + + # Mock prisma client + mock_prisma_client = mocker.MagicMock() + mock_prisma_client.db = mocker.MagicMock() + mock_prisma_client.db.litellm_usertable = mocker.MagicMock() + + # Mock user lookup - only existing-user exists + def mock_user_lookup(where): + user_id = where["user_id"] + if user_id == "existing-user": + mock_user = mocker.MagicMock() + mock_user.user_id = user_id + return mock_user + return None # new-user-1 doesn't exist + + mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(side_effect=mock_user_lookup) + + # Mock dependencies + mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception", + AsyncMock(return_value=mock_prisma_client) + ) + + # Execute the function - should raise HTTPException + with pytest.raises(HTTPException) as exc_info: + await _extract_group_member_ids(scim_group) + # Verify it's a 400 Bad Request assert exc_info.value.status_code == 400 assert "does not exist" in str(exc_info.value.detail) - assert "new-user-3" in str(exc_info.value.detail) or "new-user-4" in str(exc_info.value.detail) + assert "new-user-1" in str(exc_info.value.detail) + + +@pytest.mark.asyncio +async def test_process_group_patch_operations_with_flag_true_creates_users(mocker, monkeypatch): + """ + Test that _process_group_patch_operations creates users when scim_upsert_user is True. + """ + # Mock the feature flag to True (backward compatible mode) + async def mock_get_config(): + return { + "litellm_settings": { + "scim_upsert_user": True + } + } + + from litellm.proxy.proxy_server import proxy_config + monkeypatch.setattr(proxy_config, "get_config", mock_get_config) + + # Test data + patch_ops = SCIMPatchOp( + schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations=[ + SCIMPatchOperation( + op="add", + path="members", + value=[{"value": "new-user-1"}] + ) + ] + ) + + # Mock existing team + mock_existing_team = mocker.MagicMock() + mock_existing_team.members = [] + mock_existing_team.metadata = {} + + # Mock prisma client + mock_prisma_client = mocker.MagicMock() + mock_prisma_client.db = mocker.MagicMock() + mock_prisma_client.db.litellm_usertable = mocker.MagicMock() + + # Mock user lookup - new-user-1 doesn't exist + mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + + # Mock user creation + created_user = NewUserResponse(user_id="new-user-1", key="test-key-1") + mock_create_user = mocker.patch( + "litellm.proxy.management_endpoints.scim.scim_v2._create_user_if_not_exists", + AsyncMock(return_value=created_user) + ) + + # Execute the function + update_data, final_members = await _process_group_patch_operations( + patch_ops=patch_ops, + existing_team=mock_existing_team, + prisma_client=mock_prisma_client + ) + + # Verify result + assert "new-user-1" in final_members + + # Verify user was created + mock_create_user.assert_called_once_with( + user_id="new-user-1", + created_via="scim_group_patch" + ) + + +@pytest.mark.asyncio +async def test_process_group_patch_operations_with_flag_false_rejects(mocker, monkeypatch): + """ + Test that _process_group_patch_operations rejects non-existent users when scim_upsert_user is False. + """ + # Mock the feature flag to False (SCIM 2.0 strict mode) + async def mock_get_config(): + return { + "litellm_settings": { + "scim_upsert_user": False + } + } + + from litellm.proxy.proxy_server import proxy_config + monkeypatch.setattr(proxy_config, "get_config", mock_get_config) + + # Test data + patch_ops = SCIMPatchOp( + schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations=[ + SCIMPatchOperation( + op="add", + path="members", + value=[{"value": "new-user-1"}] + ) + ] + ) + + # Mock existing team + mock_existing_team = mocker.MagicMock() + mock_existing_team.members = [] + mock_existing_team.metadata = {} + + # Mock prisma client + mock_prisma_client = mocker.MagicMock() + mock_prisma_client.db = mocker.MagicMock() + mock_prisma_client.db.litellm_usertable = mocker.MagicMock() + + # Mock user lookup - new-user-1 doesn't exist + mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=None) + + # Execute the function - should raise HTTPException + with pytest.raises(HTTPException) as exc_info: + await _process_group_patch_operations( + patch_ops=patch_ops, + existing_team=mock_existing_team, + prisma_client=mock_prisma_client + ) + + # Verify it's a 400 Bad Request + assert exc_info.value.status_code == 400 + assert "does not exist" in str(exc_info.value.detail) + assert "new-user-1" in str(exc_info.value.detail)