mirror of
https://github.com/BerriAI/litellm.git
synced 2025-12-06 11:33:26 +08:00
fix(team): use organization.members instead of deprecated organization.users (#17557)
Fixes #17552 - Change Prisma include from 'users' to 'members' - Use LiteLLM_OrganizationTableWithMembers type for membership validation - Access organization.members instead of organization.users - Add tests for membership validation
This commit is contained in:
@@ -31,6 +31,7 @@ from litellm.proxy._types import (
|
||||
LiteLLM_ManagementEndpoint_MetadataFields_Premium,
|
||||
LiteLLM_ModelTable,
|
||||
LiteLLM_OrganizationTable,
|
||||
LiteLLM_OrganizationTableWithMembers,
|
||||
LiteLLM_TeamMembership,
|
||||
LiteLLM_TeamTable,
|
||||
LiteLLM_TeamTableCachedObj,
|
||||
@@ -1051,7 +1052,7 @@ async def fetch_and_validate_organization(
|
||||
|
||||
organization_row = await prisma_client.db.litellm_organizationtable.find_unique(
|
||||
where={"organization_id": organization_id},
|
||||
include={"litellm_budget_table": True, "users": True},
|
||||
include={"litellm_budget_table": True, "members": True},
|
||||
)
|
||||
|
||||
if organization_row is None:
|
||||
@@ -1064,7 +1065,7 @@ async def fetch_and_validate_organization(
|
||||
|
||||
validate_team_org_change(
|
||||
team=LiteLLM_TeamTable(**existing_team_row.model_dump()),
|
||||
organization=LiteLLM_OrganizationTable(**organization_row.model_dump()),
|
||||
organization=LiteLLM_OrganizationTableWithMembers(**organization_row.model_dump()),
|
||||
llm_router=llm_router,
|
||||
)
|
||||
|
||||
@@ -1072,7 +1073,7 @@ async def fetch_and_validate_organization(
|
||||
|
||||
|
||||
def validate_team_org_change(
|
||||
team: LiteLLM_TeamTable, organization: LiteLLM_OrganizationTable, llm_router: Router
|
||||
team: LiteLLM_TeamTable, organization: LiteLLM_OrganizationTableWithMembers, llm_router: Router
|
||||
) -> bool:
|
||||
"""
|
||||
Validate that a team can be moved to an organization.
|
||||
@@ -1123,7 +1124,7 @@ def validate_team_org_change(
|
||||
|
||||
# Check if the team's user_id is a member of the org
|
||||
team_members = [m.user_id for m in team.members_with_roles]
|
||||
org_members = [m.user_id for m in organization.users] if organization.users else []
|
||||
org_members = [m.user_id for m in organization.members] if organization.members else []
|
||||
not_in_org = [
|
||||
m
|
||||
for m in team_members
|
||||
|
||||
@@ -16,7 +16,9 @@ sys.path.insert(
|
||||
) # Adds the parent directory to the system path
|
||||
from litellm.proxy._types import UserAPIKeyAuth # Import UserAPIKeyAuth
|
||||
from litellm.proxy._types import (
|
||||
LiteLLM_OrganizationMembershipTable,
|
||||
LiteLLM_OrganizationTable,
|
||||
LiteLLM_OrganizationTableWithMembers,
|
||||
LiteLLM_TeamTable,
|
||||
LitellmUserRoles,
|
||||
Member,
|
||||
@@ -95,7 +97,7 @@ async def test_validate_team_org_change_same_org_id():
|
||||
team.members_with_roles = []
|
||||
|
||||
# Mock organization
|
||||
organization = MagicMock(spec=LiteLLM_OrganizationTable)
|
||||
organization = MagicMock(spec=LiteLLM_OrganizationTableWithMembers)
|
||||
organization.organization_id = org_id
|
||||
organization.models = []
|
||||
organization.litellm_budget_table = MagicMock()
|
||||
@@ -108,7 +110,7 @@ async def test_validate_team_org_change_same_org_id():
|
||||
organization.litellm_budget_table.rpm_limit = (
|
||||
50 # This would normally fail validation
|
||||
)
|
||||
organization.users = []
|
||||
organization.members = []
|
||||
|
||||
# Mock Router
|
||||
mock_router = MagicMock(spec=Router)
|
||||
@@ -126,6 +128,114 @@ async def test_validate_team_org_change_same_org_id():
|
||||
mock_access_check.assert_not_called() # Ensure access check wasn't called
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validate_team_org_change_members_in_org():
|
||||
"""
|
||||
Test that validate_team_org_change passes when team members are in organization.members.
|
||||
|
||||
This tests the fix for issue #17552 where membership was incorrectly checked against
|
||||
organization.users (deprecated) instead of organization.members (correct).
|
||||
"""
|
||||
team_org_id = "team-org-123"
|
||||
new_org_id = "new-org-456"
|
||||
user_id_1 = "user-123"
|
||||
user_id_2 = "user-456"
|
||||
|
||||
# Mock team with members
|
||||
team = MagicMock(spec=LiteLLM_TeamTable)
|
||||
team.organization_id = team_org_id
|
||||
team.models = []
|
||||
team.max_budget = None
|
||||
team.tpm_limit = None
|
||||
team.rpm_limit = None
|
||||
|
||||
# Create mock team members
|
||||
team_member_1 = MagicMock()
|
||||
team_member_1.user_id = user_id_1
|
||||
team_member_2 = MagicMock()
|
||||
team_member_2.user_id = user_id_2
|
||||
team.members_with_roles = [team_member_1, team_member_2]
|
||||
|
||||
# Mock organization with members (using LiteLLM_OrganizationMembershipTable structure)
|
||||
organization = MagicMock(spec=LiteLLM_OrganizationTableWithMembers)
|
||||
organization.organization_id = new_org_id
|
||||
organization.models = []
|
||||
organization.litellm_budget_table = None
|
||||
|
||||
# Create mock organization members - these should match team members
|
||||
org_member_1 = MagicMock(spec=LiteLLM_OrganizationMembershipTable)
|
||||
org_member_1.user_id = user_id_1
|
||||
org_member_2 = MagicMock(spec=LiteLLM_OrganizationMembershipTable)
|
||||
org_member_2.user_id = user_id_2
|
||||
organization.members = [org_member_1, org_member_2]
|
||||
|
||||
# Mock Router
|
||||
mock_router = MagicMock(spec=Router)
|
||||
|
||||
# Test should pass - all team members are in org members
|
||||
result = validate_team_org_change(
|
||||
team=team, organization=organization, llm_router=mock_router
|
||||
)
|
||||
assert result is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validate_team_org_change_member_not_in_org():
|
||||
"""
|
||||
Test that validate_team_org_change raises HTTPException when team members
|
||||
are NOT in organization.members.
|
||||
|
||||
This tests the fix for issue #17552 where membership was incorrectly checked against
|
||||
organization.users (deprecated) instead of organization.members (correct).
|
||||
"""
|
||||
team_org_id = "team-org-123"
|
||||
new_org_id = "new-org-456"
|
||||
user_id_1 = "user-123"
|
||||
user_id_2 = "user-456"
|
||||
user_id_not_in_org = "user-not-in-org-789"
|
||||
|
||||
# Mock team with members (including one not in org)
|
||||
team = MagicMock(spec=LiteLLM_TeamTable)
|
||||
team.organization_id = team_org_id
|
||||
team.models = []
|
||||
team.max_budget = None
|
||||
team.tpm_limit = None
|
||||
team.rpm_limit = None
|
||||
|
||||
# Create mock team members - user_id_not_in_org is not in the org
|
||||
team_member_1 = MagicMock()
|
||||
team_member_1.user_id = user_id_1
|
||||
team_member_2 = MagicMock()
|
||||
team_member_2.user_id = user_id_not_in_org
|
||||
team.members_with_roles = [team_member_1, team_member_2]
|
||||
|
||||
# Mock organization with members (missing user_id_not_in_org)
|
||||
organization = MagicMock(spec=LiteLLM_OrganizationTableWithMembers)
|
||||
organization.organization_id = new_org_id
|
||||
organization.models = []
|
||||
organization.litellm_budget_table = None
|
||||
|
||||
# Create mock organization members - only user_id_1 and user_id_2 are members
|
||||
org_member_1 = MagicMock(spec=LiteLLM_OrganizationMembershipTable)
|
||||
org_member_1.user_id = user_id_1
|
||||
org_member_2 = MagicMock(spec=LiteLLM_OrganizationMembershipTable)
|
||||
org_member_2.user_id = user_id_2
|
||||
organization.members = [org_member_1, org_member_2]
|
||||
|
||||
# Mock Router
|
||||
mock_router = MagicMock(spec=Router)
|
||||
|
||||
# Test should fail - user_id_not_in_org is not in org members
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
validate_team_org_change(
|
||||
team=team, organization=organization, llm_router=mock_router
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 403
|
||||
assert "not a member of the organization" in str(exc_info.value.detail)
|
||||
assert user_id_not_in_org in str(exc_info.value.detail)
|
||||
|
||||
|
||||
# Test for /team/permissions_list endpoint (GET)
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_team_permissions_list_success(mock_db_client, mock_admin_auth):
|
||||
|
||||
Reference in New Issue
Block a user