-
-
Notifications
You must be signed in to change notification settings - Fork 49
Feature/delete account endpoint #265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1a5dea6
2f60ac4
3ff845f
a771b69
9662c1f
be4980e
6449a2e
ec59247
e8fd89f
aa4ed9c
11e3c99
d175de1
171c9b3
f0999e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| from http import HTTPStatus | ||
| from typing import Annotated, Any | ||
|
|
||
| from fastapi import APIRouter, Depends, HTTPException | ||
| from sqlalchemy import Connection | ||
|
|
||
| from core.errors import UserError | ||
| from database.users import User, UserGroup, delete_user, get_user_resource_count | ||
| from routers.dependencies import expdb_connection, fetch_user, userdb_connection | ||
|
|
||
| router = APIRouter(prefix="/users", tags=["users"]) | ||
|
|
||
|
|
||
| @router.delete( | ||
| "/{user_id}", | ||
| summary="Delete a user account", | ||
| description=( | ||
| "Deletes the account of the specified user. " | ||
| "Only the account owner or an admin may perform this action. " | ||
| "Deletion is blocked if the user has uploaded any owned resources." | ||
| ), | ||
| ) | ||
| def delete_account( | ||
| user_id: int, | ||
| caller: Annotated[User | None, Depends(fetch_user)] = None, | ||
| user_db: Annotated[Connection, Depends(userdb_connection)] = None, | ||
| expdb: Annotated[Connection, Depends(expdb_connection)] = None, | ||
| ) -> dict[str, Any]: | ||
| if caller is None: | ||
| raise HTTPException( | ||
| status_code=HTTPStatus.UNAUTHORIZED, | ||
| detail={"code": str(int(UserError.NO_ACCESS)), "message": "Authentication required"}, | ||
| ) | ||
|
|
||
| is_admin = UserGroup.ADMIN in caller.groups | ||
| is_self = caller.user_id == user_id | ||
|
|
||
| if not is_admin and not is_self: | ||
| raise HTTPException( | ||
| status_code=HTTPStatus.FORBIDDEN, | ||
| detail={"code": str(int(UserError.NO_ACCESS)), "message": "No access granted"}, | ||
| ) | ||
|
|
||
| import uuid | ||
|
|
||
| from sqlalchemy import text # noqa: PLC0415 | ||
|
|
||
| original = user_db.execute( | ||
| text("SELECT session_hash FROM users WHERE id = :id FOR UPDATE"), | ||
| parameters={"id": user_id}, | ||
| ).fetchone() | ||
|
|
||
| if original is None: | ||
| raise HTTPException( | ||
| status_code=HTTPStatus.NOT_FOUND, | ||
| detail={"code": str(int(UserError.NOT_FOUND)), "message": "User not found"}, | ||
| ) | ||
|
|
||
| # Invalidate session immediately to prevent concurrent resource creation | ||
| original_session_hash = original[0] | ||
| temp_lock_hash = uuid.uuid4().hex | ||
| user_db.execute( | ||
| text("UPDATE users SET session_hash = :lock_hash WHERE id = :id"), | ||
| parameters={"lock_hash": temp_lock_hash, "id": user_id}, | ||
| ) | ||
| user_db.commit() | ||
|
Comment on lines
+48
to
+66
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Concurrent delete attempts can restore the wrong Lines 48-66 snapshot whatever value is currently in Also applies to: 87-94 🤖 Prompt for AI Agents |
||
|
|
||
| deletion_successful = False | ||
| try: | ||
| resource_count = get_user_resource_count(user_id=user_id, expdb=expdb) | ||
| if resource_count > 0: | ||
| raise HTTPException( | ||
| status_code=HTTPStatus.CONFLICT, | ||
| detail={ | ||
| "code": str(int(UserError.HAS_RESOURCES)), | ||
| "message": ( | ||
| f"User has {resource_count} resource(s). " | ||
| "Remove or transfer resources before deleting the account." | ||
| ), | ||
| }, | ||
| ) | ||
|
|
||
| delete_user(user_id=user_id, connection=user_db) | ||
| user_db.commit() | ||
| deletion_successful = True | ||
| return {"user_id": user_id, "deleted": True} | ||
| finally: | ||
| if not deletion_successful: | ||
| # Restore session hash if deletion did not complete successfully | ||
| user_db.execute( | ||
| text("UPDATE users SET session_hash = :hash WHERE id = :id"), | ||
| parameters={"hash": original_session_hash, "id": user_id}, | ||
| ) | ||
| user_db.commit() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DELETION_PENDINGbecomes a working credential here.src/database/users.py:29-40still authenticates withWHERE session_hash = :api_key, so after Line 61 any request that sendsapi_key=DELETION_PENDINGcan impersonate the target user until this flow restores or deletes the row. Use an unguessable tombstone value here, or better, a dedicated pending flag that the auth path rejects.Possible minimal fix
from http import HTTPStatus +import secrets from typing import Annotated, Any🤖 Prompt for AI Agents