From bf579b93e64559082ef3d02c53629c8a28a467bc Mon Sep 17 00:00:00 2001
From: Yves Gugger <guggeryves@hotmail.com>
Date: Sun, 21 Dec 2025 17:18:31 +0100
Subject: [PATCH] fix: Prevent admin user-delete 500 via soft-delete fallback

---
 backend/app/api/admin.py   | 56 ++++++++++++++++++++++++++++++++++----
 backend/app/models/user.py |  5 +++-
 2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/backend/app/api/admin.py b/backend/app/api/admin.py
index b27d9fa..cff63b1 100644
--- a/backend/app/api/admin.py
+++ b/backend/app/api/admin.py
@@ -662,15 +662,29 @@ async def delete_user(
     db: Database,
     admin: User = Depends(require_admin),
 ):
-    """Delete a user and all their data."""
+    """
+    Delete a user and all their data.
+
+    Production-hardening:
+    - Prefer hard-delete (keeps DB tidy).
+    - If hard-delete is blocked by FK constraints, fall back to a safe deactivation
+      (soft-delete) so the admin UI never hits a 500.
+    """
     from app.models.blog import BlogPost
     from app.models.admin_log import AdminActivityLog
+    from app.services.auth import AuthService
+    from sqlalchemy.exc import IntegrityError
+    import secrets
     
     result = await db.execute(select(User).where(User.id == user_id))
     user = result.scalar_one_or_none()
     
     if not user:
         raise HTTPException(status_code=404, detail="User not found")
+
+    # Safety rails
+    if user.id == admin.id:
+        raise HTTPException(status_code=400, detail="Cannot delete your own admin account")
     
     if user.is_admin:
         raise HTTPException(status_code=400, detail="Cannot delete admin user")
@@ -687,17 +701,47 @@ async def delete_user(
         AdminActivityLog.__table__.delete().where(AdminActivityLog.admin_id == user_id)
     )
     
-    # Now delete the user (cascades to domains, subscriptions, portfolio, price_alerts)
-    await db.delete(user)
-    await db.commit()
+    # Now delete the user (cascades to domains, subscriptions, portfolio, listings, alerts, etc.)
+    # If FK constraints block the delete (e.g., some rows reference users.id without cascade),
+    # we fall back to a safe soft-delete.
+    try:
+        await db.delete(user)
+        await db.commit()
+        deleted_mode = "hard"
+    except IntegrityError:
+        await db.rollback()
+
+        # Soft delete: disable account + remove auth factors so the user can never log in again.
+        # (We keep the row to satisfy FK constraints elsewhere.)
+        user.is_active = False
+        user.is_verified = False
+        user.hashed_password = AuthService.hash_password(secrets.token_urlsafe(32))
+        user.stripe_customer_id = None
+        user.password_reset_token = None
+        user.password_reset_expires = None
+        user.email_verification_token = None
+        user.email_verification_expires = None
+        user.oauth_provider = None
+        user.oauth_id = None
+        user.oauth_avatar = None
+        user.last_login = None
+
+        await db.commit()
+        deleted_mode = "soft"
     
     # Log this action
     await log_admin_activity(
         db, admin.id, "user_delete",
-        f"Deleted user {user_email} and all their data"
+        f"Deleted user {user_email} (mode={deleted_mode})"
     )
     
-    return {"message": f"User {user_email} and all their data have been deleted"}
+    if deleted_mode == "hard":
+        return {"message": f"User {user_email} and all their data have been deleted"}
+
+    return {
+        "message": f"User {user_email} has been deactivated (soft delete) due to existing references",
+        "mode": "soft",
+    }
 
 
 @router.post("/users/{user_id}/upgrade")
diff --git a/backend/app/models/user.py b/backend/app/models/user.py
index ce0e6c4..553f5bf 100644
--- a/backend/app/models/user.py
+++ b/backend/app/models/user.py
@@ -64,7 +64,10 @@ class User(Base):
         "PortfolioDomain", back_populates="user", cascade="all, delete-orphan"
     )
     price_alerts: Mapped[List["PriceAlert"]] = relationship(
-        "PriceAlert", cascade="all, delete-orphan", passive_deletes=True
+        # NOTE:
+        # We do not rely on DB-level ON DELETE CASCADE for this FK (it is not declared in the model),
+        # so we must not set passive_deletes=True. Otherwise deleting a user can fail with FK violations.
+        "PriceAlert", cascade="all, delete-orphan"
     )
     # For Sale Marketplace
     listings: Mapped[List["DomainListing"]] = relationship(