Fix: Deletion Deadlocks (Owner, User)
This commit is contained in:
@@ -220,48 +220,73 @@ def update_organization(org_id):
|
|||||||
|
|
||||||
@api_v1_bp.route("/organizations/<org_id>", methods=["DELETE"])
|
@api_v1_bp.route("/organizations/<org_id>", methods=["DELETE"])
|
||||||
@login_required
|
@login_required
|
||||||
@require_owner
|
|
||||||
@full_access_required
|
@full_access_required
|
||||||
def delete_organization(org_id):
|
def delete_organization(org_id):
|
||||||
"""
|
"""
|
||||||
Delete organization (soft delete).
|
Delete organization (soft delete).
|
||||||
|
|
||||||
The owner may only delete the organization if they are the *sole* remaining
|
Only the OWNER of the organization may call this endpoint.
|
||||||
member. If other active members exist they must first transfer ownership
|
|
||||||
(or remove all other members) before deleting the organization.
|
When the organization has other active members the caller must explicitly
|
||||||
|
confirm the deletion by sending ``{"confirm": true}`` in the request body.
|
||||||
|
All members (and their memberships) are soft-deleted together with the org
|
||||||
|
in a single atomic transaction so no orphaned data is left behind.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
org_id: Organization ID
|
org_id: Organization ID
|
||||||
|
|
||||||
|
Request body (JSON, optional):
|
||||||
|
confirm (bool): Required when the org has other active members.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
200: Organization deleted successfully
|
200: Organization deleted successfully
|
||||||
|
400: Organization has other members but confirm was not true
|
||||||
401: Not authenticated
|
401: Not authenticated
|
||||||
403: Not the owner
|
403: Not the owner
|
||||||
404: Organization not found
|
404: Organization not found
|
||||||
409: Organization still has other members — transfer ownership first
|
|
||||||
"""
|
"""
|
||||||
|
from gatehouse_app.models.organization.organization_member import OrganizationMember as _OrgMember
|
||||||
|
from gatehouse_app.utils.constants import OrganizationRole as _OrgRole
|
||||||
|
|
||||||
|
caller = g.current_user
|
||||||
|
|
||||||
org = OrganizationService.get_organization_by_id(org_id)
|
org = OrganizationService.get_organization_by_id(org_id)
|
||||||
|
|
||||||
# Guard: block deletion while non-owner members still exist so ownership
|
# Only the owner may delete the organization.
|
||||||
# can be transferred rather than silently orphaning them.
|
caller_membership = _OrgMember.query.filter_by(
|
||||||
active_member_count = org.get_member_count()
|
user_id=caller.id,
|
||||||
if active_member_count > 1:
|
organization_id=org.id,
|
||||||
|
deleted_at=None,
|
||||||
|
).first()
|
||||||
|
|
||||||
|
if not caller_membership or caller_membership.role != _OrgRole.OWNER:
|
||||||
return api_response(
|
return api_response(
|
||||||
success=False,
|
success=False,
|
||||||
message=(
|
message="Only the organization owner can delete the organization.",
|
||||||
"This organization still has other members. "
|
status=403,
|
||||||
"Please transfer ownership to another member or remove all "
|
error_type="AUTHORIZATION_ERROR",
|
||||||
"other members before deleting the organization."
|
|
||||||
),
|
|
||||||
status=409,
|
|
||||||
error_type="ORG_HAS_MEMBERS",
|
|
||||||
error_details={"member_count": active_member_count},
|
|
||||||
)
|
)
|
||||||
|
|
||||||
OrganizationService.delete_organization(
|
# If other members exist, require explicit confirmation to avoid accidents.
|
||||||
|
active_member_count = org.get_member_count()
|
||||||
|
if active_member_count > 1:
|
||||||
|
data = request.get_json(silent=True) or {}
|
||||||
|
if not data.get("confirm"):
|
||||||
|
return api_response(
|
||||||
|
success=False,
|
||||||
|
message=(
|
||||||
|
f"This organization has {active_member_count} active members. "
|
||||||
|
"Deleting it will remove all members and their data. "
|
||||||
|
'Send {"confirm": true} to confirm.'
|
||||||
|
),
|
||||||
|
status=400,
|
||||||
|
error_type="CONFIRMATION_REQUIRED",
|
||||||
|
error_details={"member_count": active_member_count},
|
||||||
|
)
|
||||||
|
|
||||||
|
OrganizationService.force_delete_organization(
|
||||||
org=org,
|
org=org,
|
||||||
user_id=g.current_user.id,
|
user_id=caller.id,
|
||||||
soft=True,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
return api_response(
|
return api_response(
|
||||||
|
|||||||
@@ -73,50 +73,62 @@ def delete_me():
|
|||||||
"""
|
"""
|
||||||
Delete current user account (soft delete).
|
Delete current user account (soft delete).
|
||||||
|
|
||||||
Blocked if the user is the sole owner of any organization that has other
|
Behaviour for owned organizations:
|
||||||
active members — they must transfer ownership or dissolve those organizations
|
- If the org has other active members → blocked; user must transfer ownership first.
|
||||||
first.
|
- If they are the sole member → org is automatically cascade-deleted (no orphan risk).
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
200: Account deleted successfully
|
200: Account deleted successfully (sole-member orgs auto-deleted)
|
||||||
401: Not authenticated
|
401: Not authenticated
|
||||||
409: User is sole owner of one or more organizations with other members
|
409: USER_IS_SOLE_OWNER — user owns orgs that still have other members
|
||||||
"""
|
"""
|
||||||
from gatehouse_app.models.organization.organization_member import OrganizationMember
|
from gatehouse_app.models.organization.organization_member import OrganizationMember
|
||||||
from gatehouse_app.utils.constants import OrganizationRole
|
from gatehouse_app.utils.constants import OrganizationRole
|
||||||
|
from gatehouse_app.services.organization_service import OrganizationService
|
||||||
|
|
||||||
user = g.current_user
|
user = g.current_user
|
||||||
|
|
||||||
# Find orgs where this user is the sole owner AND other members exist.
|
# Find all orgs where this user is the owner.
|
||||||
owned_memberships = OrganizationMember.query.filter_by(
|
owned_memberships = OrganizationMember.query.filter_by(
|
||||||
user_id=user.id,
|
user_id=user.id,
|
||||||
role=OrganizationRole.OWNER,
|
role=OrganizationRole.OWNER,
|
||||||
deleted_at=None,
|
deleted_at=None,
|
||||||
).all()
|
).all()
|
||||||
|
|
||||||
blocked_orgs = []
|
# Separate into two buckets depending on whether other members exist.
|
||||||
|
transfer_needed = [] # org has other members → must transfer ownership first
|
||||||
|
auto_delete = [] # user is sole member → safe to cascade-delete automatically
|
||||||
|
|
||||||
for membership in owned_memberships:
|
for membership in owned_memberships:
|
||||||
org = membership.organization
|
org = membership.organization
|
||||||
if org.deleted_at is not None:
|
if org.deleted_at is not None:
|
||||||
continue
|
continue
|
||||||
member_count = org.get_member_count()
|
member_count = org.get_member_count()
|
||||||
if member_count > 1:
|
if member_count > 1:
|
||||||
blocked_orgs.append(org.name)
|
transfer_needed.append(org.name)
|
||||||
|
else:
|
||||||
|
auto_delete.append(org)
|
||||||
|
|
||||||
if blocked_orgs:
|
# Hard block: user owns orgs with other members — must transfer first.
|
||||||
names = ", ".join(f'"{n}"' for n in blocked_orgs)
|
if transfer_needed:
|
||||||
|
names = ", ".join(f'"{n}"' for n in transfer_needed)
|
||||||
return api_response(
|
return api_response(
|
||||||
success=False,
|
success=False,
|
||||||
message=(
|
message=(
|
||||||
f"You are the sole owner of {len(blocked_orgs)} organization"
|
f"You are the owner of {len(transfer_needed)} organization"
|
||||||
f"{'s' if len(blocked_orgs) > 1 else ''}: {names}. "
|
f"{'s' if len(transfer_needed) > 1 else ''} that still "
|
||||||
"Transfer ownership or delete those organizations before deleting your account."
|
f"{'have' if len(transfer_needed) > 1 else 'has'} other members "
|
||||||
|
f"({names}). Transfer ownership to another member first."
|
||||||
),
|
),
|
||||||
status=409,
|
status=409,
|
||||||
error_type="USER_IS_SOLE_OWNER",
|
error_type="USER_IS_SOLE_OWNER",
|
||||||
error_details={"organizations": blocked_orgs},
|
error_details={"transfer_ownership": transfer_needed},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Auto-delete any sole-member orgs so no orphaned org rows can ever be left behind.
|
||||||
|
for org in auto_delete:
|
||||||
|
OrganizationService.force_delete_organization(org, user_id=user.id)
|
||||||
|
|
||||||
UserService.delete_user(user, soft=True)
|
UserService.delete_user(user, soft=True)
|
||||||
|
|
||||||
return api_response(
|
return api_response(
|
||||||
|
|||||||
@@ -171,6 +171,52 @@ class OrganizationService:
|
|||||||
|
|
||||||
return org
|
return org
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def force_delete_organization(org, user_id):
|
||||||
|
"""
|
||||||
|
Force-delete an organization and all its members in a single atomic operation.
|
||||||
|
|
||||||
|
All active memberships are soft-deleted before the organization itself
|
||||||
|
is soft-deleted, preventing orphaned membership rows and avoiding any
|
||||||
|
cascade deadlocks.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
org: Organization instance
|
||||||
|
user_id: ID of the owner performing the delete
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
Deleted Organization instance
|
||||||
|
"""
|
||||||
|
from datetime import datetime, timezone
|
||||||
|
|
||||||
|
now = datetime.now(timezone.utc)
|
||||||
|
member_count = 0
|
||||||
|
|
||||||
|
# Soft-delete all active memberships first.
|
||||||
|
for member in org.members:
|
||||||
|
if member.deleted_at is None:
|
||||||
|
member.deleted_at = now
|
||||||
|
member_count += 1
|
||||||
|
|
||||||
|
# Now soft-delete the organization itself.
|
||||||
|
org.delete(soft=True)
|
||||||
|
|
||||||
|
# Log with member count for audit trail.
|
||||||
|
AuditService.log_action(
|
||||||
|
action=AuditAction.ORG_DELETE,
|
||||||
|
user_id=user_id,
|
||||||
|
organization_id=org.id,
|
||||||
|
resource_type="organization",
|
||||||
|
resource_id=org.id,
|
||||||
|
metadata={"members_removed": member_count},
|
||||||
|
description=(
|
||||||
|
f"Organization deleted by owner; "
|
||||||
|
f"{member_count} membership(s) removed."
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
return org
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def add_member(org, user_id, role, inviter_id):
|
def add_member(org, user_id, role, inviter_id):
|
||||||
"""
|
"""
|
||||||
|
|||||||
@@ -91,6 +91,26 @@ class UserService:
|
|||||||
"""
|
"""
|
||||||
Delete user account.
|
Delete user account.
|
||||||
|
|
||||||
|
For a soft delete this method also soft-deletes every related row that
|
||||||
|
has its own ``deleted_at`` column so that those records stop appearing
|
||||||
|
in active queries immediately:
|
||||||
|
- OrganizationMember (user no longer shows in member lists)
|
||||||
|
- DepartmentMembership
|
||||||
|
- PrincipalMembership
|
||||||
|
- CAPermission (CA access revoked)
|
||||||
|
- MfaPolicyCompliance (compliance record hidden)
|
||||||
|
- AuthenticationMethod (login methods hidden)
|
||||||
|
- SSHKey (keys are hidden)
|
||||||
|
- SSHCertificate (certs are revoked + hidden)
|
||||||
|
- Session (all active sessions killed)
|
||||||
|
- OIDCAuthCode (pending auth codes invalidated)
|
||||||
|
- OIDCRefreshToken (refresh tokens invalidated)
|
||||||
|
- OIDCSession (OIDC sessions killed)
|
||||||
|
- OIDCTokenMetadata (token metadata hidden)
|
||||||
|
|
||||||
|
All changes are committed in a single transaction after the user row
|
||||||
|
itself is marked deleted, preventing any partial-delete state.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
user: User instance
|
user: User instance
|
||||||
soft: If True, performs soft delete
|
soft: If True, performs soft delete
|
||||||
@@ -98,7 +118,84 @@ class UserService:
|
|||||||
Returns:
|
Returns:
|
||||||
Deleted User instance
|
Deleted User instance
|
||||||
"""
|
"""
|
||||||
user.delete(soft=soft)
|
from datetime import datetime, timezone
|
||||||
|
from gatehouse_app.extensions import db as _db
|
||||||
|
|
||||||
|
if soft:
|
||||||
|
now = datetime.now(timezone.utc)
|
||||||
|
|
||||||
|
# --- Org memberships -------------------------------------------
|
||||||
|
for m in user.organization_memberships:
|
||||||
|
if m.deleted_at is None:
|
||||||
|
m.deleted_at = now
|
||||||
|
|
||||||
|
# --- Department memberships -------------------------------------
|
||||||
|
for m in user.department_memberships:
|
||||||
|
if m.deleted_at is None:
|
||||||
|
m.deleted_at = now
|
||||||
|
|
||||||
|
# --- Principal memberships --------------------------------------
|
||||||
|
for m in user.principal_memberships:
|
||||||
|
if m.deleted_at is None:
|
||||||
|
m.deleted_at = now
|
||||||
|
|
||||||
|
# --- CA permissions --------------------------------------------
|
||||||
|
for p in user.ca_permissions:
|
||||||
|
if p.deleted_at is None:
|
||||||
|
p.deleted_at = now
|
||||||
|
|
||||||
|
# --- MFA compliance records ------------------------------------
|
||||||
|
for c in user.mfa_compliance:
|
||||||
|
if c.deleted_at is None:
|
||||||
|
c.deleted_at = now
|
||||||
|
|
||||||
|
# --- Authentication methods ------------------------------------
|
||||||
|
for m in user.authentication_methods:
|
||||||
|
if m.deleted_at is None:
|
||||||
|
m.deleted_at = now
|
||||||
|
|
||||||
|
# --- SSH keys ---------------------------------------------------
|
||||||
|
for key in user.ssh_keys:
|
||||||
|
if key.deleted_at is None:
|
||||||
|
key.deleted_at = now
|
||||||
|
|
||||||
|
# --- SSH certificates: revoke then soft-delete ------------------
|
||||||
|
for cert in user.ssh_certificates:
|
||||||
|
if cert.deleted_at is None:
|
||||||
|
try:
|
||||||
|
if not getattr(cert, "revoked", False):
|
||||||
|
cert.revoke("account_deleted")
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
cert.deleted_at = now
|
||||||
|
|
||||||
|
# --- Sessions ---------------------------------------------------
|
||||||
|
for session in user.sessions:
|
||||||
|
if session.deleted_at is None:
|
||||||
|
session.deleted_at = now
|
||||||
|
|
||||||
|
# --- OIDC tokens / sessions ------------------------------------
|
||||||
|
for code in user.oidc_auth_codes:
|
||||||
|
if code.deleted_at is None:
|
||||||
|
code.deleted_at = now
|
||||||
|
|
||||||
|
for token in user.oidc_refresh_tokens:
|
||||||
|
if token.deleted_at is None:
|
||||||
|
token.deleted_at = now
|
||||||
|
|
||||||
|
for oidc_session in user.oidc_sessions:
|
||||||
|
if oidc_session.deleted_at is None:
|
||||||
|
oidc_session.deleted_at = now
|
||||||
|
|
||||||
|
for meta in user.oidc_token_metadata:
|
||||||
|
if meta.deleted_at is None:
|
||||||
|
meta.deleted_at = now
|
||||||
|
|
||||||
|
# --- Mark the user row itself -----------------------------------
|
||||||
|
user.deleted_at = now
|
||||||
|
_db.session.commit()
|
||||||
|
else:
|
||||||
|
user.delete(soft=False)
|
||||||
|
|
||||||
# Log user deletion
|
# Log user deletion
|
||||||
AuditService.log_action(
|
AuditService.log_action(
|
||||||
|
|||||||
Reference in New Issue
Block a user