From 8d8f85b4bf976249266a89f692408027216711b7 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Fri, 14 Aug 2026 22:41:02 +0200 Subject: feat(account): a user can delete their own account, an admin can delete one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both go through the same erasure, so there is one description of what happens rather than two that drift. Gone: credentials, email, node key, group memberships, notifications, refresh tokens, node registrations. The username is released. Kept, on purpose and stated in the UI: the row itself, emptied, and the IP log that points at it. Those logs exist for a year to answer legal requests, and a log that can no longer say whose connection it recorded keeps the data while losing the only thing it is for. So the account becomes a tombstone rather than a hole in the table. Out of reach, also stated: files uploaded to nodes, and the identity keys nodes pinned. Those are on machines the hub does not command, and only their operators can remove them — `member unpin` and a delete on their own disk. Saying so in the confirmation matters more than the button. Owning groups blocks deletion, with the list. Cascading would delete other people's groups out from under them; the account holder can hand them over or delete them first, deliberately. Self-deletion re-checks the passphrase. A live token may be a borrowed laptop or a tab left open, and it is not consent to something irreversible. Admin deletion requires admin rather than moderator: suspension is the reversible moderation tool and stays one click away. A deleted account's access token stops working at once — the status check already refuses anything but "active", which the tests now pin down, because refresh tokens being gone would otherwise leave up to an hour of usable session. Tests: 8 covering what survives and what does not, plus a db_session fixture for assertions that cannot honestly be made through the API. Co-Authored-By: Claude Opus 5 --- packages/meshbay-hub/src/meshbay_hub/api/admin.py | 33 +++- packages/meshbay-hub/src/meshbay_hub/api/users.py | 80 ++++++++- packages/meshbay-hub/src/meshbay_hub/static/app.js | 70 +++++++- .../meshbay-hub/src/meshbay_hub/static/i18n.js | 15 ++ .../meshbay-hub/src/meshbay_hub/static/style.css | 13 ++ packages/meshbay-hub/tests/conftest.py | 14 ++ .../meshbay-hub/tests/test_account_deletion.py | 182 +++++++++++++++++++++ 7 files changed, 402 insertions(+), 5 deletions(-) create mode 100644 packages/meshbay-hub/tests/test_account_deletion.py diff --git a/packages/meshbay-hub/src/meshbay_hub/api/admin.py b/packages/meshbay-hub/src/meshbay_hub/api/admin.py index 164885d..efebb75 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/admin.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/admin.py @@ -14,7 +14,7 @@ from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from meshbay_hub.auth import decrypt_email -from meshbay_hub.api.deps import require_moderator +from meshbay_hub.api.deps import require_admin, require_moderator from meshbay_hub.api.revocation import get_connected_node_count from meshbay_hub.db.engine import get_db from meshbay_hub.db.models import Group, GroupMember, IPLog, Node, User @@ -181,6 +181,37 @@ async def admin_patch_user( # ── Groups ─────────────────────────────────────────────────────────────────── +@router.delete("/users/{user_id}") +async def admin_delete_user( + user_id: str, + current_user: User = Depends(require_admin), + db: AsyncSession = Depends(get_db), +): + """ + Erase an account. Same erasure a user performs on themselves. + + Admin rather than moderator: suspension is reversible and is the moderation + tool; this is not. Refused for one's own account — an administrator locking + themselves out is a support incident, and there is `DELETE /v1/users/me` for + someone who means it. + """ + from meshbay_hub.api.users import erase_account + + user = await db.get(User, user_id) + if not user: + raise HTTPException(status_code=404, detail="User not found") + if user.id == current_user.id: + raise HTTPException( + status_code=400, + detail="Use your own account settings to delete your account") + if user.status == "deleted": + raise HTTPException(status_code=410, detail="Account already deleted") + + result = await erase_account(db, user) + log.info("Account %s erased by admin %s", result["username"], current_user.username) + return result + + @router.get("/groups") async def admin_list_groups( current_user: User = Depends(require_moderator), diff --git a/packages/meshbay-hub/src/meshbay_hub/api/users.py b/packages/meshbay-hub/src/meshbay_hub/api/users.py index af9141c..b7b402f 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/users.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/users.py @@ -1,12 +1,13 @@ """User endpoints — /v1/users/*""" import base64 +import logging import uuid from datetime import datetime, timezone, timedelta from fastapi import APIRouter, Depends, HTTPException, Request, status from pydantic import BaseModel, field_validator -from sqlalchemy import select +from sqlalchemy import delete, select from sqlalchemy.ext.asyncio import AsyncSession from meshbay_hub.auth import ( @@ -25,9 +26,13 @@ from meshbay_hub.api.middleware import limiter from meshbay_hub.api.netutil import client_ip from meshbay_hub.config import HubConfig from meshbay_hub.db.engine import get_db -from meshbay_hub.db.models import GroupMember, IPLog, RefreshToken, User +from meshbay_hub.db.models import ( + Group, GroupMember, IPLog, Node, Notification, RefreshToken, User, +) from meshbay_hub.api.deps import get_current_user, require_user_scope +log = logging.getLogger(__name__) + router = APIRouter(prefix="/v1/users", tags=["users"]) _cfg: HubConfig | None = None @@ -324,6 +329,77 @@ async def register_node_key( # call that silently changes what every node believes about someone. +# ── Account deletion ───────────────────────────────────────────────────────── + +async def erase_account(db: AsyncSession, user: User) -> dict: + """ + Erase an account, keeping only what the law asked us to keep. + + Gone: credentials, email, node key, group memberships, notifications, refresh + tokens, node registrations. The username is released. + + Kept: the row itself, emptied, and the IP log that points at it. Those logs + exist for one year to answer legal requests, and a log that cannot say whose + connection it recorded does not do that — detaching them would keep the data + and lose the only thing it is for. So the account becomes a tombstone rather + than a hole in the table. + + Not touched, because the hub cannot: files this person uploaded to nodes, and + the identity keys nodes pinned for them. Those live on machines the hub does + not command, and only their operators can remove them. + """ + owned = (await db.execute( + select(Group).where(Group.admin_id == user.id))).scalars().all() + if owned: + raise HTTPException( + status_code=409, + detail=("This account still owns groups: " + + ", ".join(g.name for g in owned) + + ". Delete them or hand them over first — deleting the " + "account would strand their members."), + ) + + await db.execute(delete(GroupMember).where(GroupMember.user_id == user.id)) + await db.execute(delete(Notification).where(Notification.user_id == user.id)) + await db.execute(delete(RefreshToken).where(RefreshToken.user_id == user.id)) + await db.execute(delete(Node).where(Node.user_id == user.id)) + + username = user.username + user.username = f"deleted-{user.id[:8]}" + user.email = "" + user.pw_hash = b"" + user.pw_salt = b"" + user.pk_node_ed25519 = None + user.status = "deleted" + user.role = "user" + await db.commit() + log.info("Account erased: %s (%s)", username, user.id[:8]) + return {"status": "deleted", "username": username} + + +class DeleteAccountRequest(BaseModel): + auth_key: str + + +@router.delete("/me") +async def delete_own_account( + body: DeleteAccountRequest, + current_user: User = Depends(require_user_scope), + db: AsyncSession = Depends(get_db), +): + """ + Erase your own account. The passphrase is re-checked here. + + A live access token is not enough for something irreversible: it may be a + borrowed laptop or a session left open. Same value as at sign-in, so the hub + still never sees the passphrase itself. + """ + if not verify_password(body.auth_key, current_user.pw_hash, current_user.pw_salt, + current_user.pw_version): + raise HTTPException(status_code=403, detail="Passphrase does not match") + return await erase_account(db, current_user) + + @router.get("/{username}/pubkeys") async def get_user_pubkeys( username: str, diff --git a/packages/meshbay-hub/src/meshbay_hub/static/app.js b/packages/meshbay-hub/src/meshbay_hub/static/app.js index d8b83ed..7b1b5fc 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/app.js @@ -2130,7 +2130,7 @@ function SearchPage() { const THEME_OPTIONS = ['light', 'dark', 'system']; -function SettingsPage({ user, theme, onThemeChange, groups }) { +function SettingsPage({ user, theme, onThemeChange, groups, onLogout }) { const [locale, setLoc] = useState(getLocale); const [muted, setMuted] = useState(() => { try { return JSON.parse(localStorage.getItem('mb_muted') || '{}'); } @@ -2142,6 +2142,29 @@ function SettingsPage({ user, theme, onThemeChange, groups }) { const [nodeKeyLoading, setNodeKeyLoading] = useState(false); const [pinCount, setPinCount] = useState( () => (window.MeshBayTransport?.pinnedNodeCount?.() ?? 0)); + const [delOpen, setDelOpen] = useState(false); + const [delPass, setDelPass] = useState(''); + const [delError, setDelError] = useState(''); + const [deleting, setDeleting] = useState(false); + + const deleteAccount = useCallback(async (e) => { + e.preventDefault(); + setDeleting(true); + setDelError(''); + try { + // The passphrase is re-checked by the hub, not merely by this form: an + // open session is not consent to something irreversible. + const authKey = await window.MeshBayKeys.deriveAuthKey(delPass, user.username); + await hubFetch('/v1/users/me', { + method: 'DELETE', token: user.token, body: { auth_key: authKey }, + }); + onLogout(); + } catch (err) { + setDelError(err.message); + } finally { + setDeleting(false); + } + }, [delPass, user]); // 11.5.8: node identity pins are refused strictly on change, so users need a // deliberate way to accept a legitimate rotation (operator reinstalled a node). @@ -2250,6 +2273,33 @@ function SettingsPage({ user, theme, onThemeChange, groups }) { +
+

${t('settings.danger')}

+

${t('settings.delete_hint')}

+ ${delError && html`

${delError}

`} + ${!delOpen + ? html`` + : html` +
+

${t('settings.delete_confirm')}

+
+ setDelPass(e.target.value)} required /> + + +
+
+ `} +
+

${t('settings.appearance')}

@@ -2368,6 +2418,18 @@ function AdminPage({ token }) { else if (tab === 'blocklist') loadBlocklist(); }, [tab]); + const deleteUser = useCallback(async (u) => { + // Suspension is the reversible tool and stays one click away; this one is + // not, so it names the account and says what it cannot reach. + if (!confirm(t('admin.delete_confirm', { user: u.username }))) return; + try { + await hubFetch(`/v1/admin/users/${u.id}`, { method: 'DELETE', token }); + loadUsers(userSearch); + } catch (err) { + alert(err.message); + } + }, [token, userSearch, loadUsers]); + const patchUser = useCallback(async (userId, patch) => { try { await hubFetch(`/v1/admin/users/${userId}`, { method: 'PATCH', body: patch, token }); @@ -2510,6 +2572,10 @@ function AdminPage({ token }) { ? html`` : null } + ${u.status !== 'deleted' && html` + + `} `)} @@ -2806,7 +2872,7 @@ function App() { : html`<${HomePage} groups=${groups} notifications=${notifications} onMarkRead=${markRead} />`; } else if (route === '/settings') { page = html`<${SettingsPage} user=${user} theme=${theme} - onThemeChange=${setTheme} groups=${groups} />`; + onThemeChange=${setTheme} groups=${groups} onLogout=${authCtx.logout} />`; } else { page = html`<${HomePage} groups=${groups} notifications=${notifications} onMarkRead=${markRead} />`; } diff --git a/packages/meshbay-hub/src/meshbay_hub/static/i18n.js b/packages/meshbay-hub/src/meshbay_hub/static/i18n.js index c19411b..1b03375 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/i18n.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/i18n.js @@ -132,6 +132,16 @@ const en = { 'settings.node_pins_hint': "Each node's identity key is remembered the first time you connect. If it changes, the connection is refused — that is expected only when an operator reinstalls a node. Verify with them before clearing.", 'settings.node_pins_count': '{n} pinned', 'settings.node_pins_clear': 'Clear pinned identities', + 'settings.danger': 'Delete account', + 'settings.delete_hint': 'Removes your account, your group memberships and your ' + + 'notifications from the hub, and frees your username. It cannot reach what ' + + 'lives on nodes: files you uploaded stay where their operator keeps them, ' + + 'and each node keeps the identity it pinned for you until its operator ' + + 'unpins it. Connection logs are kept for a year, as the law requires.', + 'settings.delete_account': 'Delete my account', + 'settings.delete_confirm': 'Enter your passphrase to confirm. This cannot be undone.', + 'settings.delete_confirm_btn': 'Delete for good', + 'settings.cancel': 'Cancel', 'settings.appearance': 'Appearance', 'settings.theme': 'Theme', 'settings.theme_light': 'Light', @@ -190,6 +200,11 @@ const en = { 'admin.btn_suspend': 'Suspend', 'admin.btn_unsuspend': 'Unsuspend', 'admin.btn_details': 'Details', + 'admin.btn_delete': 'Delete', + 'admin.delete_confirm': 'Delete the account "{user}"? This cannot be undone. ' + + 'Files they uploaded stay on the nodes that host them, and each node keeps ' + + 'the identity it pinned until its operator unpins it. Suspending is the ' + + 'reversible option.', 'admin.user_detail': 'User details', 'admin.btn_close': 'Close', 'admin.self_note': '(you)', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/style.css b/packages/meshbay-hub/src/meshbay_hub/static/style.css index 361ff7e..d948dc2 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/style.css +++ b/packages/meshbay-hub/src/meshbay_hub/static/style.css @@ -1420,3 +1420,16 @@ button:disabled { opacity: 0.5; cursor: not-allowed; } border-radius: 8px; background: var(--bg-surface); } + +/* Irreversible actions look like it. */ +.btn-danger { + background: var(--error); + color: #fff; + border: none; + padding: 8px 16px; + border-radius: 6px; + cursor: pointer; + font-size: 0.9em; +} +.btn-danger:hover { filter: brightness(1.1); } +.btn-danger:disabled { opacity: 0.5; cursor: not-allowed; } diff --git a/packages/meshbay-hub/tests/conftest.py b/packages/meshbay-hub/tests/conftest.py index ffd6c2e..4593f86 100644 --- a/packages/meshbay-hub/tests/conftest.py +++ b/packages/meshbay-hub/tests/conftest.py @@ -70,3 +70,17 @@ async def client(app): base_url="http://test", ) as c: yield c + + +@pytest_asyncio.fixture +async def db_session(app): + """ + A session on the same in-memory database the app is using. + + For assertions that cannot be made through the API — what a deletion left + behind, for instance, which is exactly the sort of thing worth checking + directly rather than inferring. + """ + from meshbay_hub.db.engine import get_session_factory + async with get_session_factory()() as session: + yield session diff --git a/packages/meshbay-hub/tests/test_account_deletion.py b/packages/meshbay-hub/tests/test_account_deletion.py new file mode 100644 index 0000000..0ae70f4 --- /dev/null +++ b/packages/meshbay-hub/tests/test_account_deletion.py @@ -0,0 +1,182 @@ +""" +Account deletion, by the owner and by an administrator. + +Deletion is the one action here that cannot be undone from the UI, so the tests +state what survives it as carefully as what does not. Two things survive on +purpose: the IP log, which exists for a year to answer legal requests and would +be useless if it could no longer say whose connection it recorded, and everything +on a node — files and pinned identities live on machines the hub does not +command. +""" + +import hashlib + +import pytest +from sqlalchemy import select + +from meshbay_hub.db.models import GroupMember, Notification, RefreshToken, User + + +def _auth_key(password: str, username: str) -> str: + import base64 + salt = hashlib.sha256(f"meshbay:auth:v1:{username}".encode()).digest() + return base64.b64encode( + hashlib.pbkdf2_hmac("sha512", password.encode(), salt, 600_000, 32)).decode() + + +async def _register(client, username, password="a-long-enough-passphrase"): + r = await client.post("/v1/users/register", json={ + "username": username, "email": f"{username}@example.com", + "auth_key": _auth_key(password, username), + }) + assert r.status_code in (200, 201), r.text + login = await client.post("/v1/users/login", json={ + "username": username, "auth_key": _auth_key(password, username)}) + return login.json()["access_token"], password + + +@pytest.mark.asyncio +async def test_owner_can_delete_their_account(client, db_session): + token, password = await _register(client, "leaver") + headers = {"Authorization": f"Bearer {token}"} + + r = await client.request("DELETE", "/v1/users/me", headers=headers, + json={"auth_key": _auth_key(password, "leaver")}) + assert r.status_code == 200, r.text + + user = (await db_session.execute( + select(User).where(User.status == "deleted"))).scalar_one() + assert user.username.startswith("deleted-") + assert user.email == "" + assert user.pw_hash == b"" + assert user.pk_node_ed25519 is None + + +@pytest.mark.asyncio +async def test_deleting_needs_the_passphrase_not_just_a_session(client): + """ + A live token may be a borrowed laptop or a tab left open. Something + irreversible asks again. + """ + token, _ = await _register(client, "careful") + r = await client.request("DELETE", "/v1/users/me", + headers={"Authorization": f"Bearer {token}"}, + json={"auth_key": _auth_key("wrong one", "careful")}) + assert r.status_code == 403 + + me = await client.get("/v1/users/me", + headers={"Authorization": f"Bearer {token}"}) + assert me.status_code == 200, "the account must survive a failed attempt" + + +@pytest.mark.asyncio +async def test_the_username_is_released(client): + token, password = await _register(client, "recycled") + await client.request("DELETE", "/v1/users/me", + headers={"Authorization": f"Bearer {token}"}, + json={"auth_key": _auth_key(password, "recycled")}) + + again = await client.post("/v1/users/register", json={ + "username": "recycled", "email": "new@example.com", + "auth_key": _auth_key("another passphrase entirely", "recycled"), + }) + assert again.status_code in (200, 201), "the name should be free again" + + +@pytest.mark.asyncio +async def test_owning_a_group_blocks_deletion(client): + """ + Deleting an account that owns groups would strand their members, so it is + refused with the list rather than cascading into other people's data. + """ + token, password = await _register(client, "owner") + headers = {"Authorization": f"Bearer {token}"} + r = await client.post("/v1/groups", json={"name": "orphans"}, headers=headers) + assert r.status_code in (200, 201), r.text + + r = await client.request("DELETE", "/v1/users/me", headers=headers, + json={"auth_key": _auth_key(password, "owner")}) + assert r.status_code == 409 + assert "orphans" in r.json()["detail"] + + +@pytest.mark.asyncio +async def test_deletion_clears_memberships_notifications_and_tokens( + client, db_session): + token, password = await _register(client, "member1") + owner_token, _ = await _register(client, "grouper") + g = await client.post("/v1/groups", json={"name": "shared"}, + headers={"Authorization": f"Bearer {owner_token}"}) + gid = g.json()["group_id"] + await client.post(f"/v1/groups/{gid}/members/member1", json={}, + headers={"Authorization": f"Bearer {owner_token}"}) + + uid = (await db_session.execute( + select(User.id).where(User.username == "member1"))).scalar_one() + + await client.request("DELETE", "/v1/users/me", + headers={"Authorization": f"Bearer {token}"}, + json={"auth_key": _auth_key(password, "member1")}) + + for model in (GroupMember, Notification, RefreshToken): + rows = (await db_session.execute( + select(model).where(model.user_id == uid))).scalars().all() + assert rows == [], f"{model.__name__} survived the deletion" + + +@pytest.mark.asyncio +async def test_the_ip_log_survives_and_stays_attributable(client, db_session): + """ + Kept on purpose. These rows exist for a year to answer legal requests, and + detaching them would keep the data while losing the only thing it is for. + """ + from meshbay_hub.db.models import IPLog + + token, password = await _register(client, "logged") + uid = (await db_session.execute( + select(User.id).where(User.username == "logged"))).scalar_one() + + before = (await db_session.execute( + select(IPLog).where(IPLog.user_id == uid))).scalars().all() + assert before, "registration should have been logged" + + await client.request("DELETE", "/v1/users/me", + headers={"Authorization": f"Bearer {token}"}, + json={"auth_key": _auth_key(password, "logged")}) + + after = (await db_session.execute( + select(IPLog).where(IPLog.user_id == uid))).scalars().all() + assert len(after) >= len(before), "the compliance log must survive deletion" + + +@pytest.mark.asyncio +async def test_a_deleted_account_cannot_keep_using_its_token(client): + """ + Refresh tokens are removed, but an access token lives up to an hour. The + status check refuses it straight away — a deleted account must not keep + reading groups until its token happens to expire. + """ + token, password = await _register(client, "gone") + headers = {"Authorization": f"Bearer {token}"} + r = await client.request("DELETE", "/v1/users/me", headers=headers, + json={"auth_key": _auth_key(password, "gone")}) + assert r.status_code == 200 + + after = await client.get("/v1/groups/mine", headers=headers) + assert after.status_code in (401, 403), "the session outlived the account" + + +@pytest.mark.asyncio +async def test_only_an_admin_may_delete_someone_else(client, db_session): + token, _ = await _register(client, "ordinary") + victim_token, _ = await _register(client, "victim") + victim_id = (await db_session.execute( + select(User.id).where(User.username == "victim"))).scalar_one() + + r = await client.delete(f"/v1/admin/users/{victim_id}", + headers={"Authorization": f"Bearer {token}"}) + assert r.status_code in (401, 403), "a plain user must not delete accounts" + + me = await client.get("/v1/users/me", + headers={"Authorization": f"Bearer {victim_token}"}) + assert me.status_code == 200 -- cgit v1.2.3