Skip to content

Commit f8571f2

Browse files
PttCodingManclaude
andcommitted
fix: user deletion was blocked by FK constraints.
Soft-delete now tombstones the username so the slot frees up immediately; pages.created_by and other FK references keep resolving because the row stays put. Auth filters deleted_at on login and cookie-based session revalidation so deleted users can't re-enter via existing JWTs. Adds POST /api/users/{id}/restore with a body-supplied username fallback for namespace collisions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent fd6013e commit f8571f2

5 files changed

Lines changed: 408 additions & 28 deletions

File tree

backend/app/auth.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ async def get_current_user(request: Request):
5858

5959
db = await get_db()
6060
row = await db.execute_fetchall(
61-
"SELECT id, username, role, display_name, email FROM users WHERE id = ?",
61+
"SELECT id, username, role, display_name, email FROM users WHERE id = ? AND deleted_at IS NULL",
6262
(user_id,),
6363
)
6464
if not row:

backend/app/database.py

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@
1616

1717
SCHEMA_SQL = """
1818
CREATE TABLE IF NOT EXISTS users (
19-
id INTEGER PRIMARY KEY AUTOINCREMENT,
20-
username TEXT UNIQUE NOT NULL,
21-
password_hash TEXT NOT NULL,
22-
role TEXT NOT NULL DEFAULT 'editor',
23-
display_name TEXT DEFAULT '',
24-
email TEXT DEFAULT '',
25-
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
19+
id INTEGER PRIMARY KEY AUTOINCREMENT,
20+
username TEXT UNIQUE NOT NULL,
21+
password_hash TEXT NOT NULL,
22+
role TEXT NOT NULL DEFAULT 'editor',
23+
display_name TEXT DEFAULT '',
24+
email TEXT DEFAULT '',
25+
original_username TEXT,
26+
deleted_at TIMESTAMP,
27+
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
2628
);
2729
2830
CREATE TABLE IF NOT EXISTS api_tokens (
@@ -1094,6 +1096,11 @@ async def init_db():
10941096
await db.execute("ALTER TABLE users ADD COLUMN display_name TEXT DEFAULT ''")
10951097
if "email" not in col_names:
10961098
await db.execute("ALTER TABLE users ADD COLUMN email TEXT DEFAULT ''")
1099+
if "deleted_at" not in col_names:
1100+
await db.execute("ALTER TABLE users ADD COLUMN deleted_at TIMESTAMP")
1101+
if "original_username" not in col_names:
1102+
await db.execute("ALTER TABLE users ADD COLUMN original_username TEXT")
1103+
await db.execute("CREATE INDEX IF NOT EXISTS idx_users_deleted ON users(deleted_at)")
10971104
await db.commit()
10981105

10991106
# Migrate: add new page columns if missing

backend/app/routers/auth_router.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ async def login(body: LoginRequest, request: Request, response: Response):
3636

3737
db = await get_db()
3838
rows = await db.execute_fetchall(
39-
"SELECT id, username, password_hash, role, display_name, email FROM users WHERE username = ?",
39+
"SELECT id, username, password_hash, role, display_name, email FROM users WHERE username = ? AND deleted_at IS NULL",
4040
(body.username,),
4141
)
4242
if not rows or not verify_password(body.password, rows[0]["password_hash"]):

backend/app/routers/users.py

Lines changed: 102 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import sqlite3
2+
13
from fastapi import APIRouter, Depends, HTTPException, Query
24
from pydantic import BaseModel
35
from typing import Optional
@@ -7,6 +9,15 @@
79

810
router = APIRouter(prefix="/api/users", tags=["users"])
911

12+
# Reserved prefix applied to a soft-deleted user's `username` so the original
13+
# name is immediately free for reuse while the row stays put to preserve FK
14+
# references (pages.created_by, page_versions.edited_by, comments.user_id, ...).
15+
TOMBSTONE_PREFIX = "__deleted_"
16+
17+
18+
def _is_reserved(name: str) -> bool:
19+
return name.startswith(TOMBSTONE_PREFIX)
20+
1021

1122
class UserCreate(BaseModel):
1223
username: str
@@ -20,6 +31,10 @@ class UserUpdate(BaseModel):
2031
is_active: Optional[bool] = None
2132

2233

34+
class UserRestore(BaseModel):
35+
username: Optional[str] = None
36+
37+
2338
@router.get("/search")
2439
async def search_users(
2540
q: str = Query("", description="Substring match on username or display_name"),
@@ -39,7 +54,7 @@ async def search_users(
3954
rows = await db.execute_fetchall(
4055
"""SELECT id, username, display_name, role
4156
FROM users
42-
WHERE username LIKE ? OR display_name LIKE ?
57+
WHERE deleted_at IS NULL AND (username LIKE ? OR display_name LIKE ?)
4358
ORDER BY username
4459
LIMIT ?""",
4560
(pattern, pattern, limit),
@@ -51,14 +66,18 @@ async def search_users(
5166
async def list_users(
5267
page: int = Query(1, ge=1),
5368
per_page: int = Query(50, ge=1, le=200),
69+
include_deleted: bool = Query(False, description="Include soft-deleted users"),
5470
user=Depends(require_admin),
5571
):
5672
db = await get_db()
5773
offset = (page - 1) * per_page
58-
count_rows = await db.execute_fetchall("SELECT COUNT(*) as cnt FROM users")
74+
where = "" if include_deleted else "WHERE deleted_at IS NULL"
75+
count_rows = await db.execute_fetchall(f"SELECT COUNT(*) as cnt FROM users {where}")
5976
total = count_rows[0]["cnt"]
6077
rows = await db.execute_fetchall(
61-
"SELECT id, username, role, created_at FROM users ORDER BY id LIMIT ? OFFSET ?",
78+
f"""SELECT id, username, original_username, role, deleted_at, created_at
79+
FROM users {where}
80+
ORDER BY id LIMIT ? OFFSET ?""",
6281
(per_page, offset),
6382
)
6483
return {
@@ -69,14 +88,31 @@ async def list_users(
6988
}
7089

7190

91+
@router.get("/deleted")
92+
async def list_deleted_users(user=Depends(require_admin)):
93+
"""Trash list: soft-deleted users with their original username preserved."""
94+
db = await get_db()
95+
rows = await db.execute_fetchall(
96+
"""SELECT id, original_username, display_name, email, role, deleted_at, created_at
97+
FROM users
98+
WHERE deleted_at IS NOT NULL
99+
ORDER BY deleted_at DESC"""
100+
)
101+
return [dict(r) for r in rows]
102+
103+
72104
ALLOWED_ROLES = ("admin", "editor", "viewer")
73105

74106

75107
@router.post("", status_code=201)
76108
async def create_user(body: UserCreate, user=Depends(require_admin)):
77109
if body.role not in ALLOWED_ROLES:
78110
raise HTTPException(status_code=400, detail="Role must be admin, editor, or viewer")
111+
if _is_reserved(body.username):
112+
raise HTTPException(status_code=400, detail="Username prefix is reserved")
79113
db = await get_db()
114+
# Deleted users have tombstone usernames so they won't match here; the
115+
# uniqueness check naturally only considers the active namespace.
80116
existing = await db.execute_fetchall(
81117
"SELECT id FROM users WHERE username = ?", (body.username,)
82118
)
@@ -99,7 +135,8 @@ async def create_user(body: UserCreate, user=Depends(require_admin)):
99135
async def update_user(user_id: int, body: UserUpdate, user=Depends(require_admin)):
100136
db = await get_db()
101137
rows = await db.execute_fetchall(
102-
"SELECT id, username, role FROM users WHERE id = ?", (user_id,)
138+
"SELECT id, username, role FROM users WHERE id = ? AND deleted_at IS NULL",
139+
(user_id,),
103140
)
104141
if not rows:
105142
raise HTTPException(status_code=404, detail="User not found")
@@ -112,7 +149,7 @@ async def update_user(user_id: int, body: UserUpdate, user=Depends(require_admin
112149
# Prevent last admin from demoting themselves
113150
if user_id == user["id"] and body.role != "admin":
114151
admin_count = await db.execute_fetchall(
115-
"SELECT COUNT(*) as cnt FROM users WHERE role = 'admin'"
152+
"SELECT COUNT(*) as cnt FROM users WHERE role = 'admin' AND deleted_at IS NULL"
116153
)
117154
if admin_count[0]["cnt"] <= 1:
118155
raise HTTPException(
@@ -143,8 +180,66 @@ async def delete_user(user_id: int, user=Depends(require_admin)):
143180
if user_id == user["id"]:
144181
raise HTTPException(status_code=400, detail="Cannot delete yourself")
145182
db = await get_db()
146-
rows = await db.execute_fetchall("SELECT id FROM users WHERE id = ?", (user_id,))
183+
rows = await db.execute_fetchall(
184+
"SELECT id FROM users WHERE id = ? AND deleted_at IS NULL",
185+
(user_id,),
186+
)
147187
if not rows:
148188
raise HTTPException(status_code=404, detail="User not found")
149-
await db.execute("DELETE FROM users WHERE id = ?", (user_id,))
189+
# No "last admin" guard here: `require_admin` means the caller is an
190+
# admin, the self-check above ensures they differ from the target, so at
191+
# least two admins exist whenever this point is reached.
192+
193+
# Soft-delete: keep the row (FKs still resolve) but rename `username` to a
194+
# tombstone that is guaranteed unique. The epoch suffix covers the
195+
# delete → restore → delete loop for the same user id.
196+
await db.execute(
197+
"""UPDATE users
198+
SET deleted_at = CURRENT_TIMESTAMP,
199+
original_username = username,
200+
username = '__deleted_' || id || '_' || strftime('%s','now')
201+
WHERE id = ?""",
202+
(user_id,),
203+
)
150204
await db.commit()
205+
206+
207+
@router.post("/{user_id}/restore")
208+
async def restore_user(user_id: int, body: UserRestore, user=Depends(require_admin)):
209+
db = await get_db()
210+
rows = await db.execute_fetchall(
211+
"SELECT id, original_username FROM users WHERE id = ? AND deleted_at IS NOT NULL",
212+
(user_id,),
213+
)
214+
if not rows:
215+
raise HTTPException(status_code=404, detail="Deleted user not found")
216+
217+
target = (body.username or rows[0]["original_username"] or "").strip()
218+
if not target:
219+
raise HTTPException(status_code=400, detail="Username required for restore")
220+
if _is_reserved(target):
221+
raise HTTPException(status_code=400, detail="Username prefix is reserved")
222+
223+
# UPDATE relies on the UNIQUE(username) constraint to reject collisions —
224+
# catching IntegrityError closes the TOCTOU window that a SELECT-then-UPDATE
225+
# check would open if two admins restored into the same slot concurrently.
226+
try:
227+
await db.execute(
228+
"""UPDATE users
229+
SET deleted_at = NULL,
230+
original_username = NULL,
231+
username = ?
232+
WHERE id = ?""",
233+
(target, user_id),
234+
)
235+
await db.commit()
236+
except sqlite3.IntegrityError:
237+
await db.rollback()
238+
raise HTTPException(
239+
status_code=409,
240+
detail=f"Username '{target}' is taken; choose a different one",
241+
)
242+
row = await db.execute_fetchall(
243+
"SELECT id, username, role, created_at FROM users WHERE id = ?", (user_id,)
244+
)
245+
return dict(row[0])

0 commit comments

Comments
 (0)