diff --git a/gatehouse_app/api/v1/users/admin.py b/gatehouse_app/api/v1/users/admin.py index d4db372..58de371 100644 --- a/gatehouse_app/api/v1/users/admin.py +++ b/gatehouse_app/api/v1/users/admin.py @@ -306,10 +306,10 @@ def admin_delete_user(user_id): from gatehouse_app.models.user.user import User as _User from gatehouse_app.models.ssh_ca.ssh_key import SSHKey from gatehouse_app.models.ssh_ca.ssh_certificate import SSHCertificate - from gatehouse_app.models.auth.authentication_method import OAuthState from gatehouse_app.extensions import db as _db from gatehouse_app.utils.constants import AuditAction, OrganizationRole from gatehouse_app.services.audit_service import AuditService + from gatehouse_app.services.user_service import UserService caller = g.current_user data = request.get_json() or {} @@ -372,20 +372,10 @@ def admin_delete_user(user_id): target_email = target.email target_id_str = str(target.id) - now = datetime.now(timezone.utc) try: - # Soft delete the user — set deleted_at timestamp. - target.deleted_at = now - - # Soft delete associated OAuthState records. - OAuthState.query.filter_by(user_id=target_id_str).filter(OAuthState.deleted_at == None).update( - {"deleted_at": now}, synchronize_session=False - ) - - _db.session.flush() + UserService.delete_user(target, soft=True) except Exception as exc: - _db.session.rollback() _logger.error(f"Soft delete failed for {target_id_str}: {exc}") return api_response(success=False, message="Failed to delete user account. Please try again.", status=500, error_type="SERVER_ERROR") diff --git a/gatehouse_app/models/organization/organization.py b/gatehouse_app/models/organization/organization.py index ce96741..736e497 100644 --- a/gatehouse_app/models/organization/organization.py +++ b/gatehouse_app/models/organization/organization.py @@ -68,11 +68,18 @@ class Organization(BaseModel): def is_member(self, user_id: str) -> bool: """Check if a user is a member of the organization.""" from gatehouse_app.models.organization.organization_member import OrganizationMember + from gatehouse_app.models.user.user import User return ( - OrganizationMember.query.filter_by( - user_id=user_id, organization_id=self.id, deleted_at=None - ).first() + db.session.query(OrganizationMember) + .join(User, OrganizationMember.user_id == User.id) + .filter( + OrganizationMember.user_id == user_id, + OrganizationMember.organization_id == self.id, + OrganizationMember.deleted_at.is_(None), + User.deleted_at.is_(None), + ) + .first() is not None ) def get_active_members(self): diff --git a/gatehouse_app/services/user_service.py b/gatehouse_app/services/user_service.py index d98a404..173dd02 100644 --- a/gatehouse_app/services/user_service.py +++ b/gatehouse_app/services/user_service.py @@ -105,6 +105,7 @@ class UserService: - Session (all active sessions killed) - OIDCAuthCode (pending auth codes invalidated) - OIDCRefreshToken (refresh tokens invalidated) + - OAuthState (OAuth flow states invalidated) - OIDCSession (OIDC sessions killed) - OIDCTokenMetadata (token metadata hidden) @@ -120,6 +121,7 @@ class UserService: """ from datetime import datetime, timezone from gatehouse_app.extensions import db as _db + from gatehouse_app.models.auth.authentication_method import OAuthState if soft: now = datetime.now(timezone.utc) @@ -169,6 +171,11 @@ class UserService: pass cert.deleted_at = now + # --- OAuth states ----------------------------------------------- + OAuthState.query.filter_by(user_id=user.id).filter( + OAuthState.deleted_at == None + ).update({"deleted_at": now}, synchronize_session=False) + # --- Sessions --------------------------------------------------- for session in user.sessions: if session.deleted_at is None: diff --git a/tests/integration/test_admin_ops.py b/tests/integration/test_admin_ops.py index 9f66b9a..7bf31cb 100644 --- a/tests/integration/test_admin_ops.py +++ b/tests/integration/test_admin_ops.py @@ -212,6 +212,48 @@ class TestAdminUserManagement: integration_client.auth.login(email=victim["email"], password="VictimPass123!") assert exc_info.value.status_code in (400, 401) + def test_admin_soft_delete_cascades_org_membership(self, integration_app, integration_client, create_test_user, create_test_org, create_test_membership): + """TEST: ADMIN-12 — Admin soft-delete cascades to org memberships. + + WHAT: Admin soft-deletes a user in an org. The membership row + and the user row both get soft-deleted. + WHY: Ghost memberships (membership active but user deleted) would + make Organization.is_member() return True and break lookups. + EXPECTED: 200 OK. OrganizationMember.deleted_at and User.deleted_at + are set. Organization.is_member() returns False. + """ + from gatehouse_app.models.organization.organization_member import OrganizationMember + from gatehouse_app.models.organization.organization import Organization + from gatehouse_app.models.user.user import User + from gatehouse_app.extensions import db + + admin = create_test_user(password="AdminPass123!") + victim = create_test_user(password="VictimPass123!") + org = create_test_org() + create_test_membership(admin["id"], org["id"], OrganizationRole.OWNER) + create_test_membership(victim["id"], org["id"], OrganizationRole.MEMBER) + + integration_client.auth.login(email=admin["email"], password="AdminPass123!") + result = integration_client.admin.hard_delete_user(victim["id"], confirm=True) + assert_success(result) + + with integration_app.app_context(): + # Membership row still exists but is soft-deleted + membership = OrganizationMember.query.filter_by( + user_id=victim["id"], organization_id=org["id"] + ).first() + assert membership is not None + assert membership.deleted_at is not None + + # User row is soft-deleted + user = User.query.get(victim["id"]) + assert user is not None + assert user.deleted_at is not None + + # Organization.is_member() returns False (defense in depth) + org_obj = Organization.query.get(org["id"]) + assert org_obj.is_member(victim["id"]) is False + class TestAdminSSHCertificates: """Test admin SSH certificate listing endpoints."""