From 1a050c9f8601c69d2bb626338fe8b985c5a46cf1 Mon Sep 17 00:00:00 2001 From: Dream <42954461+eureka928@users.noreply.github.com> Date: Mon, 9 Feb 2026 21:17:27 -0500 Subject: [PATCH] fix(api): clean up orphaned pending accounts on member removal (#32151) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- api/services/account_service.py | 33 ++++- .../services/test_account_service.py | 126 ++++++++++++++++++ 2 files changed, 156 insertions(+), 3 deletions(-) diff --git a/api/services/account_service.py b/api/services/account_service.py index d3893c1207..b4b25a1194 100644 --- a/api/services/account_service.py +++ b/api/services/account_service.py @@ -1225,7 +1225,12 @@ class TenantService: @staticmethod def remove_member_from_tenant(tenant: Tenant, account: Account, operator: Account): - """Remove member from tenant""" + """Remove member from tenant. + + If the removed member has ``AccountStatus.PENDING`` (invited but never + activated) and no remaining workspace memberships, the orphaned account + record is deleted as well. + """ if operator.id == account.id: raise CannotOperateSelfError("Cannot operate self.") @@ -1235,9 +1240,31 @@ class TenantService: if not ta: raise MemberNotInTenantError("Member not in tenant.") + # Capture identifiers before any deletions; attribute access on the ORM + # object may fail after commit() expires the instance. + account_id = account.id + account_email = account.email + db.session.delete(ta) + + # Clean up orphaned pending accounts (invited but never activated) + should_delete_account = False + if account.status == AccountStatus.PENDING: + # autoflush flushes ta deletion before this query, so 0 means no remaining joins + remaining_joins = db.session.query(TenantAccountJoin).filter_by(account_id=account_id).count() + if remaining_joins == 0: + db.session.delete(account) + should_delete_account = True + db.session.commit() + if should_delete_account: + logger.info( + "Deleted orphaned pending account: account_id=%s, email=%s", + account_id, + account_email, + ) + if dify_config.BILLING_ENABLED: BillingService.clean_billing_info_cache(tenant.id) @@ -1245,13 +1272,13 @@ class TenantService: from services.enterprise.account_deletion_sync import sync_workspace_member_removal sync_success = sync_workspace_member_removal( - workspace_id=tenant.id, member_id=account.id, source="workspace_member_removed" + workspace_id=tenant.id, member_id=account_id, source="workspace_member_removed" ) if not sync_success: logger.warning( "Enterprise workspace member removal sync failed: workspace_id=%s, member_id=%s", tenant.id, - account.id, + account_id, ) @staticmethod diff --git a/api/tests/unit_tests/services/test_account_service.py b/api/tests/unit_tests/services/test_account_service.py index 8ae20f35d8..1fc45d1c35 100644 --- a/api/tests/unit_tests/services/test_account_service.py +++ b/api/tests/unit_tests/services/test_account_service.py @@ -698,6 +698,132 @@ class TestTenantService: self._assert_database_operations_called(mock_db_dependencies["db"]) + # ==================== Member Removal Tests ==================== + + def test_remove_pending_member_deletes_orphaned_account(self): + """Test that removing a pending member with no other workspaces deletes the account.""" + # Arrange + mock_tenant = MagicMock() + mock_tenant.id = "tenant-456" + mock_operator = TestAccountAssociatedDataFactory.create_account_mock(account_id="operator-123", role="owner") + mock_pending_member = TestAccountAssociatedDataFactory.create_account_mock( + account_id="pending-user-789", email="pending@example.com", status=AccountStatus.PENDING + ) + + mock_ta = TestAccountAssociatedDataFactory.create_tenant_join_mock( + tenant_id="tenant-456", account_id="pending-user-789", role="normal" + ) + + with patch("services.account_service.db") as mock_db: + mock_operator_join = TestAccountAssociatedDataFactory.create_tenant_join_mock( + tenant_id="tenant-456", account_id="operator-123", role="owner" + ) + + query_mock_permission = MagicMock() + query_mock_permission.filter_by.return_value.first.return_value = mock_operator_join + + query_mock_ta = MagicMock() + query_mock_ta.filter_by.return_value.first.return_value = mock_ta + + query_mock_count = MagicMock() + query_mock_count.filter_by.return_value.count.return_value = 0 + + mock_db.session.query.side_effect = [query_mock_permission, query_mock_ta, query_mock_count] + + with patch("services.enterprise.account_deletion_sync.sync_workspace_member_removal") as mock_sync: + mock_sync.return_value = True + + # Act + TenantService.remove_member_from_tenant(mock_tenant, mock_pending_member, mock_operator) + + # Assert: enterprise sync still receives the correct member ID + mock_sync.assert_called_once_with( + workspace_id="tenant-456", + member_id="pending-user-789", + source="workspace_member_removed", + ) + + # Assert: both join record and account should be deleted + mock_db.session.delete.assert_any_call(mock_ta) + mock_db.session.delete.assert_any_call(mock_pending_member) + assert mock_db.session.delete.call_count == 2 + + def test_remove_pending_member_keeps_account_with_other_workspaces(self): + """Test that removing a pending member who belongs to other workspaces preserves the account.""" + # Arrange + mock_tenant = MagicMock() + mock_tenant.id = "tenant-456" + mock_operator = TestAccountAssociatedDataFactory.create_account_mock(account_id="operator-123", role="owner") + mock_pending_member = TestAccountAssociatedDataFactory.create_account_mock( + account_id="pending-user-789", email="pending@example.com", status=AccountStatus.PENDING + ) + + mock_ta = TestAccountAssociatedDataFactory.create_tenant_join_mock( + tenant_id="tenant-456", account_id="pending-user-789", role="normal" + ) + + with patch("services.account_service.db") as mock_db: + mock_operator_join = TestAccountAssociatedDataFactory.create_tenant_join_mock( + tenant_id="tenant-456", account_id="operator-123", role="owner" + ) + + query_mock_permission = MagicMock() + query_mock_permission.filter_by.return_value.first.return_value = mock_operator_join + + query_mock_ta = MagicMock() + query_mock_ta.filter_by.return_value.first.return_value = mock_ta + + # Remaining join count = 1 (still in another workspace) + query_mock_count = MagicMock() + query_mock_count.filter_by.return_value.count.return_value = 1 + + mock_db.session.query.side_effect = [query_mock_permission, query_mock_ta, query_mock_count] + + with patch("services.enterprise.account_deletion_sync.sync_workspace_member_removal") as mock_sync: + mock_sync.return_value = True + + # Act + TenantService.remove_member_from_tenant(mock_tenant, mock_pending_member, mock_operator) + + # Assert: only the join record should be deleted, not the account + mock_db.session.delete.assert_called_once_with(mock_ta) + + def test_remove_active_member_preserves_account(self): + """Test that removing an active member never deletes the account, even with no other workspaces.""" + # Arrange + mock_tenant = MagicMock() + mock_tenant.id = "tenant-456" + mock_operator = TestAccountAssociatedDataFactory.create_account_mock(account_id="operator-123", role="owner") + mock_active_member = TestAccountAssociatedDataFactory.create_account_mock( + account_id="active-user-789", email="active@example.com", status=AccountStatus.ACTIVE + ) + + mock_ta = TestAccountAssociatedDataFactory.create_tenant_join_mock( + tenant_id="tenant-456", account_id="active-user-789", role="normal" + ) + + with patch("services.account_service.db") as mock_db: + mock_operator_join = TestAccountAssociatedDataFactory.create_tenant_join_mock( + tenant_id="tenant-456", account_id="operator-123", role="owner" + ) + + query_mock_permission = MagicMock() + query_mock_permission.filter_by.return_value.first.return_value = mock_operator_join + + query_mock_ta = MagicMock() + query_mock_ta.filter_by.return_value.first.return_value = mock_ta + + mock_db.session.query.side_effect = [query_mock_permission, query_mock_ta] + + with patch("services.enterprise.account_deletion_sync.sync_workspace_member_removal") as mock_sync: + mock_sync.return_value = True + + # Act + TenantService.remove_member_from_tenant(mock_tenant, mock_active_member, mock_operator) + + # Assert: only the join record should be deleted + mock_db.session.delete.assert_called_once_with(mock_ta) + # ==================== Tenant Switching Tests ==================== def test_switch_tenant_success(self):