From f20c956196ee906b7ce888f1b842674e4ea8c581 Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Tue, 10 Feb 2026 20:01:24 +0700 Subject: [PATCH] feat(backend): implement org member patch api (#12800) --- enterprise/server/constants.py | 5 + enterprise/server/routes/org_models.py | 46 +- enterprise/server/routes/orgs.py | 86 +++ .../server/services/org_member_service.py | 177 ++++++- .../tests/unit/server/routes/test_orgs.py | 195 +++++++ .../services/test_org_member_service.py | 496 ++++++++++++++++-- 6 files changed, 955 insertions(+), 50 deletions(-) diff --git a/enterprise/server/constants.py b/enterprise/server/constants.py index ae06918665..f75dde750d 100644 --- a/enterprise/server/constants.py +++ b/enterprise/server/constants.py @@ -15,6 +15,11 @@ IS_FEATURE_ENV = ( ) # Does not include the staging deployment IS_LOCAL_ENV = bool(HOST == 'localhost') +# Role name constants +ROLE_OWNER = 'owner' +ROLE_ADMIN = 'admin' +ROLE_USER = 'user' + # Deprecated - billing margins are now handled internally in litellm DEFAULT_BILLING_MARGIN = float(os.environ.get('DEFAULT_BILLING_MARGIN', '1.0')) diff --git a/enterprise/server/routes/org_models.py b/enterprise/server/routes/org_models.py index 07efec52be..89362c3d46 100644 --- a/enterprise/server/routes/org_models.py +++ b/enterprise/server/routes/org_models.py @@ -59,7 +59,7 @@ class OrgMemberNotFoundError(Exception): def __init__(self, org_id: str, user_id: str): self.org_id = org_id self.user_id = user_id - super().__init__(f'Member not found in organization "{org_id}"') + super().__init__(f'Member "{user_id}" not found in organization "{org_id}"') class RoleNotFoundError(Exception): @@ -70,6 +70,44 @@ class RoleNotFoundError(Exception): super().__init__(f'Role with id "{role_id}" not found') +class InvalidRoleError(Exception): + """Raised when an invalid role name is specified.""" + + def __init__(self, role_name: str): + self.role_name = role_name + super().__init__(f'Invalid role: "{role_name}"') + + +class InsufficientPermissionError(Exception): + """Raised when user lacks permission to perform an operation.""" + + def __init__(self, message: str = 'Insufficient permission'): + super().__init__(message) + + +class CannotModifySelfError(Exception): + """Raised when user attempts to modify their own membership.""" + + def __init__(self, action: str = 'modify'): + self.action = action + super().__init__(f'Cannot {action} your own membership') + + +class LastOwnerError(Exception): + """Raised when attempting to remove or demote the last owner.""" + + def __init__(self, action: str = 'remove'): + self.action = action + super().__init__(f'Cannot {action} the last owner of an organization') + + +class MemberUpdateError(Exception): + """Raised when member update operation fails.""" + + def __init__(self, message: str = 'Failed to update member'): + super().__init__(message) + + class OrgCreate(BaseModel): """Request model for creating a new organization.""" @@ -217,6 +255,12 @@ class OrgMemberPage(BaseModel): next_page_id: str | None = None +class OrgMemberUpdate(BaseModel): + """Request model for updating an organization member.""" + + role: str | None = None # Role name: 'owner', 'admin', or 'user' + + class MeResponse(BaseModel): """Response model for the current user's membership in an organization.""" diff --git a/enterprise/server/routes/orgs.py b/enterprise/server/routes/orgs.py index 44b2fdc513..dd2e2af68c 100644 --- a/enterprise/server/routes/orgs.py +++ b/enterprise/server/routes/orgs.py @@ -4,13 +4,20 @@ from uuid import UUID from fastapi import APIRouter, Depends, HTTPException, Query, status from server.email_validation import get_admin_user_id from server.routes.org_models import ( + CannotModifySelfError, + InsufficientPermissionError, + InvalidRoleError, + LastOwnerError, LiteLLMIntegrationError, + MemberUpdateError, MeResponse, OrgAuthorizationError, OrgCreate, OrgDatabaseError, OrgMemberNotFoundError, OrgMemberPage, + OrgMemberResponse, + OrgMemberUpdate, OrgNameExistsError, OrgNotFoundError, OrgPage, @@ -678,3 +685,82 @@ async def switch_org( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail='An unexpected error occurred', ) + + +@org_router.patch('/{org_id}/members/{user_id}', response_model=OrgMemberResponse) +async def update_org_member( + org_id: str, + user_id: str, + update_data: OrgMemberUpdate, + current_user_id: str = Depends(get_user_id), +) -> OrgMemberResponse: + """Update a member's role in an organization. + + Permission rules: + - Admins can change roles of regular users to Admin or User + - Admins cannot modify other Admins or Owners + - Owners can change roles of Admins and Users to any role (Owner, Admin, User) + - Owners cannot modify other Owners + + Users cannot modify their own role. The last owner cannot be demoted. + """ + try: + return await OrgMemberService.update_org_member( + org_id=UUID(org_id), + target_user_id=UUID(user_id), + current_user_id=UUID(current_user_id), + update_data=update_data, + ) + except OrgMemberNotFoundError as e: + # Distinguish between requester not being a member vs target not found + if str(current_user_id) in str(e): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail='You are not a member of this organization', + ) + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail='Member not found in this organization', + ) + except CannotModifySelfError: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail='Cannot modify your own role', + ) + except RoleNotFoundError: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail='Role configuration error', + ) + except InvalidRoleError: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail='Invalid role specified', + ) + except InsufficientPermissionError: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail='You do not have permission to modify this member', + ) + except LastOwnerError: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail='Cannot demote the last owner of an organization', + ) + except MemberUpdateError: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail='Failed to update member', + ) + except ValueError: + logger.exception('Invalid UUID format') + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail='Invalid organization or user ID format', + ) + except Exception: + logger.exception('Error updating organization member') + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail='Failed to update member', + ) diff --git a/enterprise/server/services/org_member_service.py b/enterprise/server/services/org_member_service.py index b33e80c167..7e281ad1e0 100644 --- a/enterprise/server/services/org_member_service.py +++ b/enterprise/server/services/org_member_service.py @@ -2,11 +2,18 @@ from uuid import UUID +from server.constants import ROLE_ADMIN, ROLE_OWNER, ROLE_USER from server.routes.org_models import ( + CannotModifySelfError, + InsufficientPermissionError, + InvalidRoleError, + LastOwnerError, + MemberUpdateError, MeResponse, OrgMemberNotFoundError, OrgMemberPage, OrgMemberResponse, + OrgMemberUpdate, RoleNotFoundError, ) from storage.org_member_store import OrgMemberStore @@ -15,10 +22,6 @@ from storage.user_store import UserStore from openhands.utils.async_utils import call_sync_from_async -# Role rank constants -OWNER_RANK = 10 -ADMIN_RANK = 20 - class OrgMemberService: """Service for organization member operations.""" @@ -149,14 +152,14 @@ class OrgMemberService: if not requester_role or not target_role: return False, 'role_not_found' - # Check permission based on role ranks + # Check permission based on roles if not OrgMemberService._can_remove_member( - requester_role.rank, target_role.rank + requester_role.name, target_role.name ): return False, 'insufficient_permission' # Check if removing the last owner - if target_role.rank == OWNER_RANK: + if target_role.name == ROLE_OWNER: if OrgMemberService._is_last_owner(org_id, target_user_id): return False, 'cannot_remove_last_owner' @@ -170,12 +173,160 @@ class OrgMemberService: return await call_sync_from_async(_remove_member) @staticmethod - def _can_remove_member(requester_rank: int, target_rank: int) -> bool: - """Check if requester can remove target based on role ranks.""" - if requester_rank == OWNER_RANK: + async def update_org_member( + org_id: UUID, + target_user_id: UUID, + current_user_id: UUID, + update_data: OrgMemberUpdate, + ) -> OrgMemberResponse: + """Update a member's role in an organization. + + Permission rules: + - Admins can change roles of users (rank > ADMIN_RANK) to Admin or User + - Admins cannot modify other Admins or Owners + - Owners can change roles of non-owners (rank > OWNER_RANK) to any role + - Owners cannot modify other Owners + + Args: + org_id: Organization ID + target_user_id: User ID of the member to update + current_user_id: User ID of the requester + update_data: Update data containing fields to modify + + Returns: + OrgMemberResponse: The updated member data + + Raises: + OrgMemberNotFoundError: If requester or target is not a member + CannotModifySelfError: If trying to modify self + RoleNotFoundError: If role configuration is invalid + InvalidRoleError: If new_role_name is not a valid role + InsufficientPermissionError: If requester lacks permission + LastOwnerError: If trying to demote the last owner + MemberUpdateError: If update operation fails + """ + new_role_name = update_data.role + + def _update_member(): + # Get current user's membership in the org + requester_membership = OrgMemberStore.get_org_member( + org_id, current_user_id + ) + if not requester_membership: + raise OrgMemberNotFoundError(str(org_id), str(current_user_id)) + + # Check if trying to modify self + if str(current_user_id) == str(target_user_id): + raise CannotModifySelfError('modify') + + # Get target user's membership + target_membership = OrgMemberStore.get_org_member(org_id, target_user_id) + if not target_membership: + raise OrgMemberNotFoundError(str(org_id), str(target_user_id)) + + # Get roles + requester_role = RoleStore.get_role_by_id(requester_membership.role_id) + target_role = RoleStore.get_role_by_id(target_membership.role_id) + + if not requester_role: + raise RoleNotFoundError(requester_membership.role_id) + if not target_role: + raise RoleNotFoundError(target_membership.role_id) + + # If no role change requested, return current state + if new_role_name is None: + user = UserStore.get_user_by_id(str(target_user_id)) + return OrgMemberResponse( + user_id=str(target_membership.user_id), + email=user.email if user else None, + role_id=target_membership.role_id, + role_name=target_role.name, + role_rank=target_role.rank, + status=target_membership.status, + ) + + # Validate new role exists + new_role = RoleStore.get_role_by_name(new_role_name.lower()) + if not new_role: + raise InvalidRoleError(new_role_name) + + # Check permission to modify target + if not OrgMemberService._can_update_member_role( + requester_role.name, target_role.name, new_role.name + ): + raise InsufficientPermissionError( + 'You do not have permission to modify this member' + ) + + # Check if demoting the last owner + if ( + target_role.name == ROLE_OWNER + and new_role.name != ROLE_OWNER + and OrgMemberService._is_last_owner(org_id, target_user_id) + ): + raise LastOwnerError('demote') + + # Perform the update + updated_member = OrgMemberStore.update_user_role_in_org( + org_id, target_user_id, new_role.id + ) + if not updated_member: + raise MemberUpdateError('Failed to update member') + + # Get user email for response + user = UserStore.get_user_by_id(str(target_user_id)) + + return OrgMemberResponse( + user_id=str(updated_member.user_id), + email=user.email if user else None, + role_id=updated_member.role_id, + role_name=new_role.name, + role_rank=new_role.rank, + status=updated_member.status, + ) + + return await call_sync_from_async(_update_member) + + @staticmethod + def _can_update_member_role( + requester_role_name: str, target_role_name: str, new_role_name: str + ) -> bool: + """Check if requester can change target's role to new_role. + + Permission rules: + - Owners can modify admins and users, can set any role + - Owners cannot modify other owners + - Admins can only modify users + - Admins can only set admin or user roles (not owner) + """ + is_requester_owner = requester_role_name == ROLE_OWNER + is_requester_admin = requester_role_name == ROLE_ADMIN + is_target_owner = target_role_name == ROLE_OWNER + is_target_admin = target_role_name == ROLE_ADMIN + is_new_role_owner = new_role_name == ROLE_OWNER + + if is_requester_owner: + # Owners cannot modify other owners + if is_target_owner: + return False + # Owners can set any role (owner, admin, user) return True - elif requester_rank == ADMIN_RANK: - return target_rank > ADMIN_RANK + elif is_requester_admin: + # Admins cannot modify owners or other admins + if is_target_owner or is_target_admin: + return False + # Admins can only set admin or user roles (not owner) + return not is_new_role_owner + return False + + @staticmethod + def _can_remove_member(requester_role_name: str, target_role_name: str) -> bool: + """Check if requester can remove target based on roles.""" + if requester_role_name == ROLE_OWNER: + return True + elif requester_role_name == ROLE_ADMIN: + # Admins can only remove users (not owners or other admins) + return target_role_name == ROLE_USER return False @staticmethod @@ -186,6 +337,6 @@ class OrgMemberService: for m in members: # Use role_id (column) instead of role (relationship) to avoid DetachedInstanceError role = RoleStore.get_role_by_id(m.role_id) - if role and role.rank == OWNER_RANK: + if role and role.name == ROLE_OWNER: owners.append(m) return len(owners) == 1 and str(owners[0].user_id) == str(user_id) diff --git a/enterprise/tests/unit/server/routes/test_orgs.py b/enterprise/tests/unit/server/routes/test_orgs.py index 4364737109..ccb6e67496 100644 --- a/enterprise/tests/unit/server/routes/test_orgs.py +++ b/enterprise/tests/unit/server/routes/test_orgs.py @@ -18,6 +18,10 @@ with patch('storage.database.engine', create=True), patch( ): from server.email_validation import get_admin_user_id from server.routes.org_models import ( + CannotModifySelfError, + InsufficientPermissionError, + InvalidRoleError, + LastOwnerError, LiteLLMIntegrationError, MeResponse, OrgAuthorizationError, @@ -25,6 +29,7 @@ with patch('storage.database.engine', create=True), patch( OrgMemberNotFoundError, OrgMemberPage, OrgMemberResponse, + OrgMemberUpdate, OrgNameExistsError, OrgNotFoundError, RoleNotFoundError, @@ -34,6 +39,7 @@ with patch('storage.database.engine', create=True), patch( get_org_members, org_router, remove_org_member, + update_org_member, ) from storage.org import Org @@ -2299,6 +2305,195 @@ class TestRemoveOrgMemberEndpoint: assert exc_info.value.detail == 'Service temporarily unavailable' +class TestUpdateOrgMemberEndpoint: + """Test cases for PATCH /api/organizations/{org_id}/members/{user_id} endpoint.""" + + @pytest.mark.asyncio + async def test_update_member_role_succeeds_returns_member_response( + self, org_id, current_user_id, target_user_id + ): + """GIVEN valid role update request WHEN PATCH is called THEN returns 200 with updated OrgMemberResponse.""" + # Arrange + updated = OrgMemberResponse( + user_id=target_user_id, + email='user@example.com', + role_id=2, + role_name='admin', + role_rank=20, + status='active', + ) + with patch( + 'server.routes.orgs.OrgMemberService.update_org_member' + ) as mock_update: + mock_update.return_value = updated + + # Act + result = await update_org_member( + org_id=org_id, + user_id=target_user_id, + update_data=OrgMemberUpdate(role='admin'), + current_user_id=current_user_id, + ) + + # Assert + assert result == updated + mock_update.assert_called_once_with( + org_id=uuid.UUID(org_id), + target_user_id=uuid.UUID(target_user_id), + current_user_id=uuid.UUID(current_user_id), + update_data=OrgMemberUpdate(role='admin'), + ) + + @pytest.mark.asyncio + async def test_not_a_member_returns_403( + self, org_id, current_user_id, target_user_id + ): + """GIVEN requester is not a member WHEN PATCH is called THEN returns 403.""" + # Arrange + with patch( + 'server.routes.orgs.OrgMemberService.update_org_member' + ) as mock_update: + mock_update.side_effect = OrgMemberNotFoundError(org_id, current_user_id) + + # Act & Assert + with pytest.raises(HTTPException) as exc_info: + await update_org_member( + org_id=org_id, + user_id=target_user_id, + update_data=OrgMemberUpdate(role='user'), + current_user_id=current_user_id, + ) + assert exc_info.value.status_code == status.HTTP_403_FORBIDDEN + assert 'not a member' in exc_info.value.detail + + @pytest.mark.asyncio + async def test_cannot_modify_self_returns_403(self, org_id, current_user_id): + """GIVEN target user is self WHEN PATCH is called THEN returns 403.""" + # Arrange + with patch( + 'server.routes.orgs.OrgMemberService.update_org_member' + ) as mock_update: + mock_update.side_effect = CannotModifySelfError('modify') + + # Act & Assert + with pytest.raises(HTTPException) as exc_info: + await update_org_member( + org_id=org_id, + user_id=current_user_id, + update_data=OrgMemberUpdate(role='admin'), + current_user_id=current_user_id, + ) + assert exc_info.value.status_code == status.HTTP_403_FORBIDDEN + assert 'Cannot modify your own role' in exc_info.value.detail + + @pytest.mark.asyncio + async def test_member_not_found_returns_404( + self, org_id, current_user_id, target_user_id + ): + """GIVEN target member does not exist WHEN PATCH is called THEN returns 404.""" + # Arrange + with patch( + 'server.routes.orgs.OrgMemberService.update_org_member' + ) as mock_update: + mock_update.side_effect = OrgMemberNotFoundError(org_id, target_user_id) + + # Act & Assert + with pytest.raises(HTTPException) as exc_info: + await update_org_member( + org_id=org_id, + user_id=target_user_id, + update_data=OrgMemberUpdate(role='user'), + current_user_id=current_user_id, + ) + assert exc_info.value.status_code == status.HTTP_404_NOT_FOUND + assert 'Member not found' in exc_info.value.detail + + @pytest.mark.asyncio + async def test_invalid_role_returns_400( + self, org_id, current_user_id, target_user_id + ): + """GIVEN invalid role name WHEN PATCH is called THEN returns 400.""" + # Arrange + with patch( + 'server.routes.orgs.OrgMemberService.update_org_member' + ) as mock_update: + mock_update.side_effect = InvalidRoleError('superuser') + + # Act & Assert + with pytest.raises(HTTPException) as exc_info: + await update_org_member( + org_id=org_id, + user_id=target_user_id, + update_data=OrgMemberUpdate(role='superuser'), + current_user_id=current_user_id, + ) + assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST + assert 'Invalid role' in exc_info.value.detail + + @pytest.mark.asyncio + async def test_insufficient_permission_returns_403( + self, org_id, current_user_id, target_user_id + ): + """GIVEN requester lacks permission to change target WHEN PATCH is called THEN returns 403.""" + # Arrange + with patch( + 'server.routes.orgs.OrgMemberService.update_org_member' + ) as mock_update: + mock_update.side_effect = InsufficientPermissionError( + 'You do not have permission to modify this member' + ) + + # Act & Assert + with pytest.raises(HTTPException) as exc_info: + await update_org_member( + org_id=org_id, + user_id=target_user_id, + update_data=OrgMemberUpdate(role='admin'), + current_user_id=current_user_id, + ) + assert exc_info.value.status_code == status.HTTP_403_FORBIDDEN + assert 'do not have permission' in exc_info.value.detail + + @pytest.mark.asyncio + async def test_cannot_demote_last_owner_returns_400( + self, org_id, current_user_id, target_user_id + ): + """GIVEN demoting last owner WHEN PATCH is called THEN returns 400.""" + # Arrange + with patch( + 'server.routes.orgs.OrgMemberService.update_org_member' + ) as mock_update: + mock_update.side_effect = LastOwnerError('demote') + + # Act & Assert + with pytest.raises(HTTPException) as exc_info: + await update_org_member( + org_id=org_id, + user_id=target_user_id, + update_data=OrgMemberUpdate(role='admin'), + current_user_id=current_user_id, + ) + assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST + assert 'Cannot demote the last owner' in exc_info.value.detail + + @pytest.mark.asyncio + async def test_invalid_org_id_returns_400(self, current_user_id, target_user_id): + """GIVEN invalid org_id UUID WHEN PATCH is called THEN returns 400.""" + # Arrange + invalid_org_id = 'not-a-uuid' + + # Act & Assert + with pytest.raises(HTTPException) as exc_info: + await update_org_member( + org_id=invalid_org_id, + user_id=target_user_id, + update_data=OrgMemberUpdate(role='user'), + current_user_id=current_user_id, + ) + assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST + assert 'Invalid organization or user ID format' in exc_info.value.detail + + class TestGetMeEndpoint: """Tests for GET /api/organizations/{org_id}/me endpoint. diff --git a/enterprise/tests/unit/server/services/test_org_member_service.py b/enterprise/tests/unit/server/services/test_org_member_service.py index 7bf99d72dd..a17c312a4f 100644 --- a/enterprise/tests/unit/server/services/test_org_member_service.py +++ b/enterprise/tests/unit/server/services/test_org_member_service.py @@ -6,8 +6,14 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest from pydantic import SecretStr from server.routes.org_models import ( + CannotModifySelfError, + InsufficientPermissionError, + InvalidRoleError, + LastOwnerError, MeResponse, OrgMemberNotFoundError, + OrgMemberResponse, + OrgMemberUpdate, RoleNotFoundError, ) from server.services.org_member_service import OrgMemberService @@ -989,78 +995,496 @@ class TestOrgMemberServiceCanRemoveMember: """Test cases for OrgMemberService._can_remove_member.""" def test_owner_can_remove_admin(self): - """Test that owner (rank 10) can remove admin (rank 20).""" - # Arrange - requester_rank = 10 - target_rank = 20 - + """Test that owner can remove admin.""" # Act - result = OrgMemberService._can_remove_member(requester_rank, target_rank) + result = OrgMemberService._can_remove_member('owner', 'admin') # Assert assert result is True def test_owner_can_remove_user(self): - """Test that owner (rank 10) can remove user (rank 1000).""" - # Arrange - requester_rank = 10 - target_rank = 1000 - + """Test that owner can remove user.""" # Act - result = OrgMemberService._can_remove_member(requester_rank, target_rank) + result = OrgMemberService._can_remove_member('owner', 'user') # Assert assert result is True def test_admin_can_remove_user(self): - """Test that admin (rank 20) can remove user (rank 1000).""" - # Arrange - requester_rank = 20 - target_rank = 1000 - + """Test that admin can remove user.""" # Act - result = OrgMemberService._can_remove_member(requester_rank, target_rank) + result = OrgMemberService._can_remove_member('admin', 'user') # Assert assert result is True def test_admin_cannot_remove_admin(self): - """Test that admin (rank 20) cannot remove another admin (rank 20).""" - # Arrange - requester_rank = 20 - target_rank = 20 - + """Test that admin cannot remove another admin.""" # Act - result = OrgMemberService._can_remove_member(requester_rank, target_rank) + result = OrgMemberService._can_remove_member('admin', 'admin') # Assert assert result is False def test_admin_cannot_remove_owner(self): - """Test that admin (rank 20) cannot remove owner (rank 10).""" - # Arrange - requester_rank = 20 - target_rank = 10 - + """Test that admin cannot remove owner.""" # Act - result = OrgMemberService._can_remove_member(requester_rank, target_rank) + result = OrgMemberService._can_remove_member('admin', 'owner') # Assert assert result is False def test_user_cannot_remove_anyone(self): - """Test that user (rank 1000) cannot remove anyone.""" - # Arrange - requester_rank = 1000 - target_rank = 1000 - + """Test that user cannot remove anyone.""" # Act - result = OrgMemberService._can_remove_member(requester_rank, target_rank) + result = OrgMemberService._can_remove_member('user', 'user') # Assert assert result is False +class TestOrgMemberServiceUpdateOrgMember: + """Test cases for OrgMemberService.update_org_member.""" + + @pytest.mark.asyncio + async def test_owner_updates_user_to_admin_succeeds( + self, + org_id, + current_user_id, + target_user_id, + requester_membership_owner, + target_membership_user, + owner_role, + user_role, + admin_role, + ): + """GIVEN owner and target user WHEN owner sets target role to admin THEN update succeeds and returns OrgMemberResponse.""" + # Arrange + updated_member = MagicMock(spec=OrgMember) + updated_member.user_id = target_user_id + updated_member.role_id = admin_role.id + updated_member.status = 'active' + mock_user = MagicMock() + mock_user.email = 'target@example.com' + with ( + patch( + 'server.services.org_member_service.OrgMemberStore.get_org_member' + ) as mock_get_member, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_id' + ) as mock_get_role, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_name' + ) as mock_get_role_by_name, + patch( + 'server.services.org_member_service.OrgMemberStore.update_user_role_in_org' + ) as mock_update, + patch( + 'server.services.org_member_service.UserStore.get_user_by_id' + ) as mock_get_user, + ): + mock_get_member.side_effect = [ + requester_membership_owner, + target_membership_user, + ] + mock_get_role.side_effect = [owner_role, user_role] + mock_get_role_by_name.return_value = admin_role + mock_update.return_value = updated_member + mock_get_user.return_value = mock_user + + # Act + data = await OrgMemberService.update_org_member( + org_id, target_user_id, current_user_id, OrgMemberUpdate(role='admin') + ) + + # Assert + assert isinstance(data, OrgMemberResponse) + assert data.role_name == 'admin' + assert data.role_rank == 20 + mock_update.assert_called_once_with(org_id, target_user_id, admin_role.id) + + @pytest.mark.asyncio + async def test_admin_updates_user_to_admin_succeeds( + self, + org_id, + current_user_id, + target_user_id, + requester_membership_admin, + target_membership_user, + admin_role, + user_role, + ): + """GIVEN admin and target user WHEN admin sets target role to admin THEN update succeeds.""" + # Arrange + updated_member = MagicMock(spec=OrgMember) + updated_member.user_id = target_user_id + updated_member.role_id = admin_role.id + updated_member.status = 'active' + mock_user = MagicMock() + mock_user.email = 'target@example.com' + with ( + patch( + 'server.services.org_member_service.OrgMemberStore.get_org_member' + ) as mock_get_member, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_id' + ) as mock_get_role, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_name' + ) as mock_get_role_by_name, + patch( + 'server.services.org_member_service.OrgMemberStore.update_user_role_in_org' + ) as mock_update, + patch( + 'server.services.org_member_service.UserStore.get_user_by_id' + ) as mock_get_user, + ): + mock_get_member.side_effect = [ + requester_membership_admin, + target_membership_user, + ] + mock_get_role.side_effect = [admin_role, user_role] + mock_get_role_by_name.return_value = admin_role + mock_update.return_value = updated_member + mock_get_user.return_value = mock_user + + # Act + data = await OrgMemberService.update_org_member( + org_id, target_user_id, current_user_id, OrgMemberUpdate(role='admin') + ) + + # Assert + assert data is not None + mock_update.assert_called_once_with(org_id, target_user_id, admin_role.id) + + @pytest.mark.asyncio + async def test_admin_cannot_update_admin_raises_insufficient_permission( + self, + org_id, + current_user_id, + target_user_id, + requester_membership_admin, + target_membership_admin, + admin_role, + user_role, + ): + """GIVEN admin and target admin WHEN admin tries to change target role THEN raises InsufficientPermissionError.""" + # Arrange + with ( + patch( + 'server.services.org_member_service.OrgMemberStore.get_org_member' + ) as mock_get_member, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_id' + ) as mock_get_role, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_name' + ) as mock_get_role_by_name, + ): + mock_get_member.side_effect = [ + requester_membership_admin, + target_membership_admin, + ] + mock_get_role.side_effect = [admin_role, admin_role] + mock_get_role_by_name.return_value = user_role + + # Act & Assert + with pytest.raises(InsufficientPermissionError): + await OrgMemberService.update_org_member( + org_id, + target_user_id, + current_user_id, + OrgMemberUpdate(role='user'), + ) + + @pytest.mark.asyncio + async def test_owner_cannot_update_owner_raises_insufficient_permission( + self, + org_id, + current_user_id, + target_user_id, + requester_membership_owner, + target_membership_owner, + owner_role, + admin_role, + ): + """GIVEN owner and target owner WHEN owner tries to change target role THEN raises InsufficientPermissionError.""" + # Arrange + with ( + patch( + 'server.services.org_member_service.OrgMemberStore.get_org_member' + ) as mock_get_member, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_id' + ) as mock_get_role, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_name' + ) as mock_get_role_by_name, + ): + mock_get_member.side_effect = [ + requester_membership_owner, + target_membership_owner, + ] + mock_get_role.side_effect = [owner_role, owner_role] + mock_get_role_by_name.return_value = admin_role + + # Act & Assert + with pytest.raises(InsufficientPermissionError): + await OrgMemberService.update_org_member( + org_id, + target_user_id, + current_user_id, + OrgMemberUpdate(role='admin'), + ) + + @pytest.mark.asyncio + async def test_requester_not_a_member_raises_error( + self, org_id, current_user_id, target_user_id + ): + """GIVEN requester not in org WHEN update_org_member THEN raises OrgMemberNotFoundError.""" + # Arrange + with patch( + 'server.services.org_member_service.OrgMemberStore.get_org_member' + ) as mock_get_member: + mock_get_member.return_value = None + + # Act & Assert + with pytest.raises(OrgMemberNotFoundError): + await OrgMemberService.update_org_member( + org_id, + target_user_id, + current_user_id, + OrgMemberUpdate(role='user'), + ) + + @pytest.mark.asyncio + async def test_cannot_modify_self_raises_error( + self, org_id, current_user_id, requester_membership_owner, owner_role + ): + """GIVEN requester updates self WHEN update_org_member THEN raises CannotModifySelfError.""" + # Arrange + with patch( + 'server.services.org_member_service.OrgMemberStore.get_org_member' + ) as mock_get_member: + mock_get_member.return_value = requester_membership_owner + + # Act & Assert + with pytest.raises(CannotModifySelfError): + await OrgMemberService.update_org_member( + org_id, + current_user_id, + current_user_id, + OrgMemberUpdate(role='user'), + ) + + @pytest.mark.asyncio + async def test_target_member_not_found_raises_error( + self, + org_id, + current_user_id, + target_user_id, + requester_membership_owner, + owner_role, + ): + """GIVEN target not in org WHEN update_org_member THEN raises OrgMemberNotFoundError.""" + # Arrange + with patch( + 'server.services.org_member_service.OrgMemberStore.get_org_member' + ) as mock_get_member: + mock_get_member.side_effect = [requester_membership_owner, None] + + # Act & Assert + with pytest.raises(OrgMemberNotFoundError): + await OrgMemberService.update_org_member( + org_id, + target_user_id, + current_user_id, + OrgMemberUpdate(role='user'), + ) + + @pytest.mark.asyncio + async def test_invalid_role_name_raises_error( + self, + org_id, + current_user_id, + target_user_id, + requester_membership_owner, + target_membership_user, + owner_role, + user_role, + ): + """GIVEN unknown role name WHEN update_org_member THEN raises InvalidRoleError.""" + # Arrange + with ( + patch( + 'server.services.org_member_service.OrgMemberStore.get_org_member' + ) as mock_get_member, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_id' + ) as mock_get_role, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_name' + ) as mock_get_role_by_name, + ): + mock_get_member.side_effect = [ + requester_membership_owner, + target_membership_user, + ] + mock_get_role.side_effect = [owner_role, user_role] + mock_get_role_by_name.return_value = None + + # Act & Assert + with pytest.raises(InvalidRoleError): + await OrgMemberService.update_org_member( + org_id, + target_user_id, + current_user_id, + OrgMemberUpdate(role='superuser'), + ) + + @pytest.mark.asyncio + async def test_cannot_demote_last_owner_raises_error( + self, + org_id, + current_user_id, + target_user_id, + requester_membership_owner, + target_membership_owner, + owner_role, + admin_role, + ): + """GIVEN last owner would be demoted WHEN update_org_member THEN raises LastOwnerError.""" + # Arrange: patch _can_update_member_role so we reach the last-owner check (owner cannot normally modify owner) + with ( + patch( + 'server.services.org_member_service.OrgMemberStore.get_org_member' + ) as mock_get_member, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_id' + ) as mock_get_role, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_name' + ) as mock_get_role_by_name, + patch( + 'server.services.org_member_service.OrgMemberService._can_update_member_role' + ) as mock_can_update, + patch( + 'server.services.org_member_service.OrgMemberService._is_last_owner' + ) as mock_is_last_owner, + ): + mock_get_member.side_effect = [ + requester_membership_owner, + target_membership_owner, + ] + mock_get_role.side_effect = [owner_role, owner_role] + mock_get_role_by_name.return_value = admin_role + mock_can_update.return_value = True + mock_is_last_owner.return_value = True + + # Act & Assert + with pytest.raises(LastOwnerError): + await OrgMemberService.update_org_member( + org_id, + target_user_id, + current_user_id, + OrgMemberUpdate(role='admin'), + ) + + @pytest.mark.asyncio + async def test_no_role_in_body_returns_current_member_state( + self, + org_id, + current_user_id, + target_user_id, + requester_membership_owner, + target_membership_user, + owner_role, + user_role, + ): + """GIVEN update with no role WHEN update_org_member THEN returns current member without changing role.""" + # Arrange + mock_user = MagicMock() + mock_user.email = 'target@example.com' + target_membership_user.status = 'active' + with ( + patch( + 'server.services.org_member_service.OrgMemberStore.get_org_member' + ) as mock_get_member, + patch( + 'server.services.org_member_service.RoleStore.get_role_by_id' + ) as mock_get_role, + patch( + 'server.services.org_member_service.UserStore.get_user_by_id' + ) as mock_get_user, + ): + mock_get_member.side_effect = [ + requester_membership_owner, + target_membership_user, + ] + mock_get_role.side_effect = [owner_role, user_role] + mock_get_user.return_value = mock_user + + # Act + data = await OrgMemberService.update_org_member( + org_id, target_user_id, current_user_id, OrgMemberUpdate(role=None) + ) + + # Assert + assert data is not None + assert data.role_name == 'user' + assert data.role_rank == 1000 + + +class TestOrgMemberServiceCanUpdateMemberRole: + """Test cases for OrgMemberService._can_update_member_role.""" + + def test_owner_can_set_any_role_for_non_owner(self): + """Owner can change admin/user target to any role.""" + assert ( + OrgMemberService._can_update_member_role('owner', 'admin', 'owner') is True + ) + assert ( + OrgMemberService._can_update_member_role('owner', 'admin', 'admin') is True + ) + assert ( + OrgMemberService._can_update_member_role('owner', 'user', 'owner') is True + ) + + def test_owner_cannot_modify_owner(self): + """Owner cannot change another owner's role.""" + assert ( + OrgMemberService._can_update_member_role('owner', 'owner', 'admin') is False + ) + + def test_admin_can_set_admin_or_user_for_user(self): + """Admin can set admin or user role for a user target.""" + assert ( + OrgMemberService._can_update_member_role('admin', 'user', 'admin') is True + ) + assert OrgMemberService._can_update_member_role('admin', 'user', 'user') is True + + def test_admin_cannot_modify_admin_or_owner(self): + """Admin cannot modify admin or owner targets.""" + assert ( + OrgMemberService._can_update_member_role('admin', 'admin', 'user') is False + ) + assert ( + OrgMemberService._can_update_member_role('admin', 'owner', 'admin') is False + ) + + def test_admin_cannot_set_owner_role(self): + """Admin cannot set role to owner.""" + assert ( + OrgMemberService._can_update_member_role('admin', 'user', 'owner') is False + ) + + def test_user_cannot_update_anyone(self): + """User cannot update any member's role.""" + assert ( + OrgMemberService._can_update_member_role('user', 'user', 'admin') is False + ) + + class TestOrgMemberServiceIsLastOwner: """Test cases for OrgMemberService._is_last_owner."""