mirror of
https://github.com/tiennm99/litellm.git
synced 2026-07-29 04:20:42 +00:00
[Bug Fix] SCIM v2 - ensure group PUSH and PUT ops allow creating non-existent members (#14581)
* fix: scim handle non existent members * test - scim v2 * test fix * fix: NewUserResponse
This commit is contained in:
Ishaan Jaff
@@ -18,6 +18,7 @@ from fastapi import (
|
||||
Response,
|
||||
)
|
||||
from typing_extensions import TypedDict
|
||||
from pydantic import BaseModel
|
||||
|
||||
import litellm
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
@@ -29,6 +30,7 @@ from litellm.proxy._types import (
|
||||
Member,
|
||||
NewTeamRequest,
|
||||
NewUserRequest,
|
||||
NewUserResponse,
|
||||
TeamMemberAddRequest,
|
||||
TeamMemberDeleteRequest,
|
||||
UserAPIKeyAuth,
|
||||
@@ -101,6 +103,13 @@ class ScimUserData(TypedDict):
|
||||
active: Optional[bool]
|
||||
|
||||
|
||||
class GroupMemberExtractionResult(BaseModel):
|
||||
"""Result of extracting and processing group members."""
|
||||
existing_member_ids: List[str]
|
||||
created_users: List[NewUserResponse]
|
||||
all_member_ids: List[str] # existing + newly created
|
||||
|
||||
|
||||
scim_router = APIRouter(
|
||||
prefix="/scim/v2",
|
||||
tags=["✨ SCIM v2 (Enterprise Only)"],
|
||||
@@ -190,21 +199,47 @@ def _build_scim_metadata(given_name: Optional[str], family_name: Optional[str],
|
||||
return metadata
|
||||
|
||||
|
||||
async def _extract_group_member_ids(group: SCIMGroup) -> List[str]:
|
||||
"""Extract valid member IDs from SCIMGroup, verifying users exist."""
|
||||
async def _extract_group_member_ids(group: SCIMGroup) -> GroupMemberExtractionResult:
|
||||
"""
|
||||
Extract member IDs from SCIMGroup, creating users that don't exist.
|
||||
|
||||
Returns:
|
||||
GroupMemberExtractionResult with existing members, created users, and all member IDs
|
||||
"""
|
||||
prisma_client = await _get_prisma_client_or_raise_exception()
|
||||
member_ids = []
|
||||
existing_member_ids = []
|
||||
created_users = []
|
||||
all_member_ids = []
|
||||
|
||||
if group.members:
|
||||
for member in group.members:
|
||||
user_id = member.value
|
||||
|
||||
# Check if user exists
|
||||
user = await prisma_client.db.litellm_usertable.find_unique(
|
||||
where={"user_id": member.value}
|
||||
where={"user_id": user_id}
|
||||
)
|
||||
|
||||
if user:
|
||||
member_ids.append(member.value)
|
||||
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"
|
||||
)
|
||||
|
||||
if created_user:
|
||||
created_users.append(created_user)
|
||||
all_member_ids.append(user_id)
|
||||
# If creation failed, user is skipped (logged in helper)
|
||||
|
||||
return member_ids
|
||||
return GroupMemberExtractionResult(
|
||||
existing_member_ids=existing_member_ids,
|
||||
created_users=created_users,
|
||||
all_member_ids=all_member_ids
|
||||
)
|
||||
|
||||
|
||||
async def _get_team_members_display(member_ids: List[str]) -> List[SCIMMember]:
|
||||
@@ -239,6 +274,51 @@ async def _handle_team_membership_changes(user_id: str, existing_teams: List[str
|
||||
)
|
||||
|
||||
|
||||
async def _create_user_if_not_exists(user_id: str, created_via: str = "scim_group") -> Optional[NewUserResponse]:
|
||||
"""
|
||||
Helper function to create a user if they don't exist.
|
||||
|
||||
Args:
|
||||
user_id: The user ID to create
|
||||
created_via: Context for where the user was created from
|
||||
|
||||
Returns:
|
||||
LiteLLM_UserTable if user was created, None if creation failed
|
||||
"""
|
||||
from litellm.proxy.management_endpoints.internal_user_endpoints import new_user
|
||||
|
||||
try:
|
||||
# Get default role for new internal users
|
||||
default_role: Optional[
|
||||
Literal[
|
||||
LitellmUserRoles.PROXY_ADMIN,
|
||||
LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY,
|
||||
LitellmUserRoles.INTERNAL_USER,
|
||||
LitellmUserRoles.INTERNAL_USER_VIEW_ONLY,
|
||||
]
|
||||
] = LitellmUserRoles.INTERNAL_USER_VIEW_ONLY
|
||||
if litellm.default_internal_user_params:
|
||||
default_role = litellm.default_internal_user_params.get("user_role")
|
||||
|
||||
new_user_request = NewUserRequest(
|
||||
user_id=user_id,
|
||||
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},
|
||||
auto_create_key=False,
|
||||
user_role=default_role,
|
||||
)
|
||||
|
||||
created_user = await new_user(data=new_user_request)
|
||||
verbose_proxy_logger.info(f"Created user {user_id} via {created_via}")
|
||||
return created_user
|
||||
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.exception(f"Failed to create user {user_id}: {e}")
|
||||
return None
|
||||
|
||||
|
||||
async def _get_team_member_user_ids_from_team(team: LiteLLM_TeamTable) -> List[str]:
|
||||
"""
|
||||
Get the IDs of the members from a team.
|
||||
@@ -256,6 +336,8 @@ async def _get_team_member_user_ids_from_team(team: LiteLLM_TeamTable) -> List[s
|
||||
member_user_ids.append(user_id)
|
||||
return member_user_ids
|
||||
|
||||
|
||||
|
||||
# Dependency to set the correct SCIM Content-Type
|
||||
async def set_scim_content_type(response: Response):
|
||||
"""Sets the Content-Type header to application/scim+json"""
|
||||
@@ -914,9 +996,9 @@ async def create_group(
|
||||
detail={"error": f"Group already exists with ID: {team_id}"},
|
||||
)
|
||||
|
||||
# Extract valid member IDs
|
||||
member_ids = await _extract_group_member_ids(group)
|
||||
members_with_roles = [Member(user_id=member_id, role="user") for member_id in member_ids]
|
||||
# Extract and process group members (creating users that don't exist)
|
||||
member_result = await _extract_group_member_ids(group)
|
||||
members_with_roles = [Member(user_id=member_id, role="user") for member_id in member_result.all_member_ids]
|
||||
|
||||
# Create team in database
|
||||
created_team = await new_team(
|
||||
@@ -959,9 +1041,10 @@ async def update_group(
|
||||
prisma_client = await _get_prisma_client_or_raise_exception()
|
||||
existing_team = await _check_team_exists(group_id)
|
||||
|
||||
# Extract valid member IDs
|
||||
member_ids = await _extract_group_member_ids(group)
|
||||
verbose_proxy_logger.debug(f"SCIM PUT GROUP member_ids: {member_ids}")
|
||||
# Extract and process group members (creating users that don't 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}")
|
||||
verbose_proxy_logger.debug(f"SCIM PUT GROUP created_users: {len(member_result.created_users)}")
|
||||
|
||||
# Prepare update data
|
||||
existing_metadata = existing_team.metadata if existing_team.metadata else {}
|
||||
@@ -978,10 +1061,10 @@ async def update_group(
|
||||
data=update_data,
|
||||
)
|
||||
|
||||
# Handle user-team relationship changes using the same approach as patch_group
|
||||
# Handle user-team relationship changes
|
||||
current_members = set(await _get_team_member_user_ids_from_team(existing_team))
|
||||
verbose_proxy_logger.debug(f"SCIM PUT GROUP current_members: {current_members}")
|
||||
final_members = set(member_ids)
|
||||
final_members = set(member_result.all_member_ids)
|
||||
verbose_proxy_logger.debug(f"SCIM PUT GROUP final_members: {final_members}")
|
||||
|
||||
await _handle_group_membership_changes(
|
||||
@@ -1075,7 +1158,7 @@ async def _process_group_patch_operations(
|
||||
elif path.startswith("members"):
|
||||
# Handle member operations
|
||||
member_values = _extract_group_values(value)
|
||||
# Validate that users exist
|
||||
# Create users that don't exist and get all valid member IDs
|
||||
valid_members = []
|
||||
for member_id in member_values:
|
||||
user = await prisma_client.db.litellm_usertable.find_unique(
|
||||
@@ -1083,6 +1166,16 @@ async def _process_group_patch_operations(
|
||||
)
|
||||
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"
|
||||
)
|
||||
|
||||
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)
|
||||
|
||||
@@ -7,6 +7,7 @@ from litellm.proxy._types import LitellmUserRoles, NewUserRequest, ProxyExceptio
|
||||
from litellm.proxy.management_endpoints.scim.scim_v2 import (
|
||||
UserProvisionerHelpers,
|
||||
_handle_team_membership_changes,
|
||||
create_group,
|
||||
create_user,
|
||||
get_service_provider_config,
|
||||
patch_user,
|
||||
@@ -910,4 +911,280 @@ async def test_update_group_e2e(mocker):
|
||||
assert len(result.members) == 3
|
||||
|
||||
# Verify SCIM transformation was called with updated team
|
||||
ScimTransformations.transform_litellm_team_to_scim_group.assert_called_once_with(updated_team)
|
||||
ScimTransformations.transform_litellm_team_to_scim_group.assert_called_once_with(updated_team)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_group_with_nonexistent_users_creates_users(mocker):
|
||||
"""
|
||||
Test that creating a group with non-existent users creates those users.
|
||||
This tests the scenario: Group Push ['new user', existing users...]
|
||||
"""
|
||||
# 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
|
||||
SCIMMember(value="new-user-2", display="New User 2"), # This user doesn't exist
|
||||
]
|
||||
)
|
||||
|
||||
#########################################################
|
||||
# We expect new-user-1 and new-user-2 to 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
|
||||
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 dependencies
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2._get_prisma_client_or_raise_exception",
|
||||
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()
|
||||
)
|
||||
|
||||
# Mock created users return values
|
||||
def mock_new_user_side_effect(data):
|
||||
from litellm.proxy._types import LiteLLM_UserTable
|
||||
return LiteLLM_UserTable(
|
||||
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
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_group_with_nonexistent_users_creates_users(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 data
|
||||
group_id = "existing-group-456"
|
||||
|
||||
# Mock existing team
|
||||
mock_existing_team = mocker.MagicMock()
|
||||
mock_existing_team.team_id = group_id
|
||||
mock_existing_team.team_alias = "Old Group Name"
|
||||
mock_existing_team.members = ["old-user"]
|
||||
mock_existing_team.members_with_roles = [{"user_id": "old-user", "role": "user"}]
|
||||
mock_existing_team.metadata = {"existing": "data"}
|
||||
|
||||
# SCIM group update request
|
||||
scim_group_update = 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"), # This user exists
|
||||
SCIMMember(value="new-user-3", display="New User 3"), # This user doesn't exist
|
||||
SCIMMember(value="new-user-4", display="New User 4"), # This user doesn't exist
|
||||
]
|
||||
)
|
||||
|
||||
# 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
|
||||
mock_prisma_client.db.litellm_teamtable.find_unique = AsyncMock(return_value=mock_existing_team)
|
||||
|
||||
# Mock updated team response
|
||||
mock_updated_team = mocker.MagicMock()
|
||||
mock_updated_team.team_id = group_id
|
||||
mock_updated_team.team_alias = "Updated Group Name"
|
||||
mock_updated_team.members = ["existing-user", "new-user-3", "new-user-4"]
|
||||
mock_prisma_client.db.litellm_teamtable.update = AsyncMock(return_value=mock_updated_team)
|
||||
|
||||
# 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-3 and new-user-4 don'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)
|
||||
)
|
||||
|
||||
mocker.patch(
|
||||
"litellm.proxy.management_endpoints.scim.scim_v2._check_team_exists",
|
||||
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()
|
||||
)
|
||||
|
||||
# Mock created users return values
|
||||
def mock_new_user_side_effect(data):
|
||||
from litellm.proxy._types import LiteLLM_UserTable
|
||||
return LiteLLM_UserTable(
|
||||
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
|
||||
Reference in New Issue
Block a user