refactor: implement user deletion functionality and enhance settings management
This commit is contained in:
@@ -3,23 +3,191 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
from uuid import UUID
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, status
|
||||
from sqlmodel import col, select
|
||||
|
||||
from app.core.auth import AuthContext, get_auth_context
|
||||
from app.core.auth import AuthContext, delete_clerk_user, get_auth_context
|
||||
from app.db import crud
|
||||
from app.db.session import get_session
|
||||
from app.models.activity_events import ActivityEvent
|
||||
from app.models.agents import Agent
|
||||
from app.models.approvals import Approval
|
||||
from app.models.board_group_memory import BoardGroupMemory
|
||||
from app.models.board_groups import BoardGroup
|
||||
from app.models.board_memory import BoardMemory
|
||||
from app.models.board_onboarding import BoardOnboardingSession
|
||||
from app.models.boards import Board
|
||||
from app.models.gateways import Gateway
|
||||
from app.models.organization_board_access import OrganizationBoardAccess
|
||||
from app.models.organization_invite_board_access import OrganizationInviteBoardAccess
|
||||
from app.models.organization_invites import OrganizationInvite
|
||||
from app.models.organization_members import OrganizationMember
|
||||
from app.models.organizations import Organization
|
||||
from app.models.task_dependencies import TaskDependency
|
||||
from app.models.task_fingerprints import TaskFingerprint
|
||||
from app.models.tasks import Task
|
||||
from app.models.users import User
|
||||
from app.schemas.common import OkResponse
|
||||
from app.schemas.users import UserRead, UserUpdate
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from sqlmodel.ext.asyncio.session import AsyncSession
|
||||
|
||||
from app.models.users import User
|
||||
|
||||
router = APIRouter(prefix="/users", tags=["users"])
|
||||
AUTH_CONTEXT_DEP = Depends(get_auth_context)
|
||||
SESSION_DEP = Depends(get_session)
|
||||
|
||||
|
||||
async def _delete_organization_tree(
|
||||
session: AsyncSession,
|
||||
*,
|
||||
organization_id: UUID,
|
||||
) -> None:
|
||||
"""Delete an organization and dependent rows without committing."""
|
||||
board_ids = select(Board.id).where(col(Board.organization_id) == organization_id)
|
||||
task_ids = select(Task.id).where(col(Task.board_id).in_(board_ids))
|
||||
agent_ids = select(Agent.id).where(col(Agent.board_id).in_(board_ids))
|
||||
member_ids = select(OrganizationMember.id).where(
|
||||
col(OrganizationMember.organization_id) == organization_id,
|
||||
)
|
||||
invite_ids = select(OrganizationInvite.id).where(
|
||||
col(OrganizationInvite.organization_id) == organization_id,
|
||||
)
|
||||
group_ids = select(BoardGroup.id).where(
|
||||
col(BoardGroup.organization_id) == organization_id,
|
||||
)
|
||||
|
||||
await crud.delete_where(
|
||||
session,
|
||||
ActivityEvent,
|
||||
col(ActivityEvent.task_id).in_(task_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
ActivityEvent,
|
||||
col(ActivityEvent.agent_id).in_(agent_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
TaskDependency,
|
||||
col(TaskDependency.board_id).in_(board_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
TaskFingerprint,
|
||||
col(TaskFingerprint.board_id).in_(board_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
Approval,
|
||||
col(Approval.board_id).in_(board_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
BoardMemory,
|
||||
col(BoardMemory.board_id).in_(board_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
BoardOnboardingSession,
|
||||
col(BoardOnboardingSession.board_id).in_(board_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
OrganizationBoardAccess,
|
||||
col(OrganizationBoardAccess.board_id).in_(board_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
OrganizationInviteBoardAccess,
|
||||
col(OrganizationInviteBoardAccess.board_id).in_(board_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
OrganizationBoardAccess,
|
||||
col(OrganizationBoardAccess.organization_member_id).in_(member_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
OrganizationInviteBoardAccess,
|
||||
col(OrganizationInviteBoardAccess.organization_invite_id).in_(invite_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
Task,
|
||||
col(Task.board_id).in_(board_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
Agent,
|
||||
col(Agent.board_id).in_(board_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
Board,
|
||||
col(Board.organization_id) == organization_id,
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
BoardGroupMemory,
|
||||
col(BoardGroupMemory.board_group_id).in_(group_ids),
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
BoardGroup,
|
||||
col(BoardGroup.organization_id) == organization_id,
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
Gateway,
|
||||
col(Gateway.organization_id) == organization_id,
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
OrganizationInvite,
|
||||
col(OrganizationInvite.organization_id) == organization_id,
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
OrganizationMember,
|
||||
col(OrganizationMember.organization_id) == organization_id,
|
||||
commit=False,
|
||||
)
|
||||
await crud.update_where(
|
||||
session,
|
||||
User,
|
||||
col(User.active_organization_id) == organization_id,
|
||||
active_organization_id=None,
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
Organization,
|
||||
col(Organization.id) == organization_id,
|
||||
commit=False,
|
||||
)
|
||||
|
||||
|
||||
@router.get("/me", response_model=UserRead)
|
||||
async def get_me(auth: AuthContext = AUTH_CONTEXT_DEP) -> UserRead:
|
||||
"""Return the authenticated user's current profile payload."""
|
||||
@@ -45,3 +213,71 @@ async def update_me(
|
||||
await session.commit()
|
||||
await session.refresh(user)
|
||||
return UserRead.model_validate(user)
|
||||
|
||||
|
||||
@router.delete("/me", response_model=OkResponse)
|
||||
async def delete_me(
|
||||
session: AsyncSession = SESSION_DEP,
|
||||
auth: AuthContext = AUTH_CONTEXT_DEP,
|
||||
) -> OkResponse:
|
||||
"""Delete the authenticated account and any personal-only organizations."""
|
||||
if auth.actor_type != "user" or auth.user is None:
|
||||
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED)
|
||||
|
||||
user: User = auth.user
|
||||
await delete_clerk_user(user.clerk_user_id)
|
||||
memberships = await OrganizationMember.objects.filter_by(user_id=user.id).all(session)
|
||||
|
||||
await crud.update_where(
|
||||
session,
|
||||
OrganizationInvite,
|
||||
col(OrganizationInvite.created_by_user_id) == user.id,
|
||||
created_by_user_id=None,
|
||||
commit=False,
|
||||
)
|
||||
await crud.update_where(
|
||||
session,
|
||||
OrganizationInvite,
|
||||
col(OrganizationInvite.accepted_by_user_id) == user.id,
|
||||
accepted_by_user_id=None,
|
||||
commit=False,
|
||||
)
|
||||
await crud.update_where(
|
||||
session,
|
||||
Task,
|
||||
col(Task.created_by_user_id) == user.id,
|
||||
created_by_user_id=None,
|
||||
commit=False,
|
||||
)
|
||||
|
||||
for member in memberships:
|
||||
org_members = await OrganizationMember.objects.filter_by(
|
||||
organization_id=member.organization_id,
|
||||
).all(session)
|
||||
if len(org_members) <= 1:
|
||||
await _delete_organization_tree(
|
||||
session,
|
||||
organization_id=member.organization_id,
|
||||
)
|
||||
continue
|
||||
await crud.delete_where(
|
||||
session,
|
||||
OrganizationBoardAccess,
|
||||
col(OrganizationBoardAccess.organization_member_id) == member.id,
|
||||
commit=False,
|
||||
)
|
||||
await crud.delete_where(
|
||||
session,
|
||||
OrganizationMember,
|
||||
col(OrganizationMember.id) == member.id,
|
||||
commit=False,
|
||||
)
|
||||
|
||||
await crud.delete_where(
|
||||
session,
|
||||
User,
|
||||
col(User.id) == user.id,
|
||||
commit=False,
|
||||
)
|
||||
await session.commit()
|
||||
return OkResponse()
|
||||
|
||||
@@ -332,6 +332,67 @@ async def _fetch_clerk_profile(clerk_user_id: str) -> tuple[str | None, str | No
|
||||
return None, None
|
||||
|
||||
|
||||
async def delete_clerk_user(clerk_user_id: str) -> None:
|
||||
"""Delete a Clerk user via the official Clerk SDK."""
|
||||
secret = settings.clerk_secret_key.strip()
|
||||
secret_kind = secret.split("_", maxsplit=1)[0] if "_" in secret else "unknown"
|
||||
server_url = _normalize_clerk_server_url(settings.clerk_api_url or "")
|
||||
|
||||
try:
|
||||
async with Clerk(
|
||||
bearer_auth=secret,
|
||||
server_url=server_url,
|
||||
timeout_ms=5000,
|
||||
) as clerk:
|
||||
await clerk.users.delete_async(user_id=clerk_user_id)
|
||||
logger.info("auth.clerk.user.delete clerk_user_id=%s", clerk_user_id)
|
||||
except ClerkErrors as exc:
|
||||
errors_payload = str(exc)
|
||||
if len(errors_payload) > 300:
|
||||
errors_payload = f"{errors_payload[:300]}..."
|
||||
logger.warning(
|
||||
"auth.clerk.user.delete_failed clerk_user_id=%s reason=clerk_errors "
|
||||
"secret_kind=%s body=%s",
|
||||
clerk_user_id,
|
||||
secret_kind,
|
||||
errors_payload,
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_502_BAD_GATEWAY,
|
||||
detail="Failed to delete account from Clerk",
|
||||
) from exc
|
||||
except SDKError as exc:
|
||||
if exc.status_code == 404:
|
||||
logger.info("auth.clerk.user.delete_missing clerk_user_id=%s", clerk_user_id)
|
||||
return
|
||||
response_body = exc.body.strip() or None
|
||||
if response_body and len(response_body) > 300:
|
||||
response_body = f"{response_body[:300]}..."
|
||||
logger.warning(
|
||||
"auth.clerk.user.delete_failed clerk_user_id=%s status=%s reason=sdk_error "
|
||||
"server_url=%s secret_kind=%s body=%s",
|
||||
clerk_user_id,
|
||||
exc.status_code,
|
||||
server_url,
|
||||
secret_kind,
|
||||
response_body,
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_502_BAD_GATEWAY,
|
||||
detail="Failed to delete account from Clerk",
|
||||
) from exc
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"auth.clerk.user.delete_failed clerk_user_id=%s reason=sdk_exception",
|
||||
clerk_user_id,
|
||||
exc_info=True,
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_502_BAD_GATEWAY,
|
||||
detail="Failed to delete account from Clerk",
|
||||
) from exc
|
||||
|
||||
|
||||
async def _get_or_sync_user(
|
||||
session: AsyncSession,
|
||||
*,
|
||||
|
||||
98
backend/tests/test_users_delete_api.py
Normal file
98
backend/tests/test_users_delete_api.py
Normal file
@@ -0,0 +1,98 @@
|
||||
# ruff: noqa: S101
|
||||
"""Tests for user self-delete API behavior."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass
|
||||
from typing import Any
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException, status
|
||||
|
||||
from app.api import users
|
||||
from app.core.auth import AuthContext
|
||||
from app.models.users import User
|
||||
|
||||
|
||||
@dataclass
|
||||
class _FakeSession:
|
||||
committed: int = 0
|
||||
|
||||
async def commit(self) -> None:
|
||||
self.committed += 1
|
||||
|
||||
|
||||
class _EmptyMembershipQuery:
|
||||
async def all(self, _session: Any) -> list[Any]:
|
||||
return []
|
||||
|
||||
|
||||
class _FakeOrganizationMemberModel:
|
||||
class objects:
|
||||
@staticmethod
|
||||
def filter_by(**_kwargs: Any) -> _EmptyMembershipQuery:
|
||||
return _EmptyMembershipQuery()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_me_aborts_when_clerk_delete_fails(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Local deletion should not run if Clerk account deletion fails."""
|
||||
session = _FakeSession()
|
||||
user = User(id=uuid4(), clerk_user_id="user_123")
|
||||
auth = AuthContext(actor_type="user", user=user)
|
||||
|
||||
async def _fail_delete(_clerk_user_id: str) -> None:
|
||||
raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="clerk failure")
|
||||
|
||||
async def _unexpected_update(*_args: Any, **_kwargs: Any) -> int:
|
||||
raise AssertionError("crud.update_where should not be called on Clerk failure")
|
||||
|
||||
async def _unexpected_delete(*_args: Any, **_kwargs: Any) -> int:
|
||||
raise AssertionError("crud.delete_where should not be called on Clerk failure")
|
||||
|
||||
monkeypatch.setattr(users, "delete_clerk_user", _fail_delete)
|
||||
monkeypatch.setattr(users.crud, "update_where", _unexpected_update)
|
||||
monkeypatch.setattr(users.crud, "delete_where", _unexpected_delete)
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await users.delete_me(session=session, auth=auth)
|
||||
|
||||
assert exc_info.value.status_code == status.HTTP_502_BAD_GATEWAY
|
||||
assert session.committed == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_me_deletes_local_user_after_clerk_success(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""User delete should invoke Clerk deletion, then remove local account."""
|
||||
session = _FakeSession()
|
||||
user = User(id=uuid4(), clerk_user_id="user_456")
|
||||
auth = AuthContext(actor_type="user", user=user)
|
||||
calls: dict[str, int] = {"clerk": 0, "update": 0, "delete": 0}
|
||||
|
||||
async def _delete_from_clerk(clerk_user_id: str) -> None:
|
||||
assert clerk_user_id == "user_456"
|
||||
calls["clerk"] += 1
|
||||
|
||||
async def _update_where(*_args: Any, **_kwargs: Any) -> int:
|
||||
calls["update"] += 1
|
||||
return 0
|
||||
|
||||
async def _delete_where(*_args: Any, **_kwargs: Any) -> int:
|
||||
calls["delete"] += 1
|
||||
return 1
|
||||
|
||||
monkeypatch.setattr(users, "delete_clerk_user", _delete_from_clerk)
|
||||
monkeypatch.setattr(users, "OrganizationMember", _FakeOrganizationMemberModel)
|
||||
monkeypatch.setattr(users.crud, "update_where", _update_where)
|
||||
monkeypatch.setattr(users.crud, "delete_where", _delete_where)
|
||||
|
||||
response = await users.delete_me(session=session, auth=auth)
|
||||
|
||||
assert response.ok is True
|
||||
assert calls["clerk"] == 1
|
||||
assert calls["update"] == 3
|
||||
assert calls["delete"] == 1
|
||||
assert session.committed == 1
|
||||
Reference in New Issue
Block a user