mirror of https://github.com/langgenius/dify.git
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>
This commit is contained in:
parent
7fb6e0cdfe
commit
1a050c9f86
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Reference in New Issue