mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-08 22:21:35 +00:00
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.
This commit is contained in:
parent
8871a5670b
commit
e5baff780a
2 changed files with 68 additions and 184 deletions
|
|
@ -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":
|
||||
|
|
|
|||
|
|
@ -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
|
||||
# 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)
|
||||
Loading…
Add table
Reference in a new issue