From d1ae8f943f09de254ee6cd7525563b9c2c7e7537 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Thu, 29 Oct 2026 18:32:19 -0500 Subject: [PATCH] feat(account): self-service account deletion, MODULE_API 1.20.0 ACCOUNT_DELETION.md phase 2 (core): a member deletes their own account on the website, and the admin route runs the same procedure. - DELETE /api/v1/auth/me/account: typed username, password always (AD7), and a 2FA or recovery code when 2FA is on. A wrong password or code is 400, not 401 (AD12), so the app's refresh-on-401 never replays it. 409 password_required and last_admin; the demo's shared logins are locked. - auth/accountDeletion.js: module hooks first, then one transaction that reserves a staff username (reserved_usernames, AD9) and deletes the row, then a Teams reconcile, the audit entry (no username) and a notice to the old address (new protected template auth.account-deleted). - MODULE_API 1.20.0: api.onUserDeleting({ userId, deletedBy }), before the delete, 10 s limit, cannot veto (AD10, AD11). - utils/displayAuthor.js: a deleted account's username snapshots read "[deleted player]" to members and the stored name, marked, to staff (AD5, AD6). Used by the forum, the Reports queue, the guest list and the Team digest. A deleted account drops off Forum guests (AD13). - Reserved names refuse at every username write, answered as "taken". - Web: Delete my account on the player and staff Account pages; staff views mark "(deleted account)". - OpenAPI regenerated; route manifest and engagement manifest regenerated. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY --- client/src/api/client.js | 4 + client/src/components/DemoToast.jsx | 4 + .../security/DeleteAccountPanel.jsx | 178 ++++++++ client/src/lib/accountDeletion.js | 69 ++++ client/src/lib/deletedAccount.js | 25 ++ client/src/lib/notice.js | 27 ++ client/src/lib/teamAdmin.js | 6 +- client/src/lib/teamForum.js | 4 +- client/src/modules/TeamForumPanel.jsx | 8 +- client/src/modules/version.js | 5 +- .../src/routes/admin/views/AccountAdmin.jsx | 5 + .../src/routes/admin/views/ContentReports.jsx | 11 +- client/src/routes/admin/views/TeamsAdmin.jsx | 3 +- client/src/routes/player/PlayerAccount.jsx | 2 + client/test/accountDeletion.test.js | 80 ++++ server/db/schema.sql | 18 +- server/engagement-triggers.json | 2 +- server/routes.guards.json | 12 + server/routes.manifest.json | 4 + server/src/auth/accountDeletion.js | 174 ++++++++ server/src/demo/locks.js | 4 + server/src/engagement/templateSeeds.js | 30 ++ server/src/model/reports/contentReports.db.js | 2 +- .../src/model/reports/contentReports.model.js | 45 +- server/src/model/teams/teamAccess.model.js | 10 +- server/src/model/teams/teamForum.model.js | 28 +- server/src/model/teams/teamGrants.model.js | 13 +- server/src/model/teams/teamNotify.db.js | 2 +- server/src/model/users/users.db.js | 29 +- server/src/model/users/users.model.js | 45 +- server/src/modules/loader.js | 36 +- server/src/modules/version.js | 15 +- .../src/router/v1/admin/admin.controller.js | 16 +- .../router/v1/admin/moderation.controller.js | 2 +- .../src/router/v1/auth/account.controller.js | 85 +++- server/src/router/v1/auth/me.routes.js | 27 ++ .../router/v1/player/teamForum.controller.js | 7 +- server/src/utils/db.js | 33 +- server/src/utils/displayAuthor.js | 52 +++ server/src/utils/mailer.js | 38 +- server/src/utils/teamDigestWorker.js | 6 +- server/swagger/swagger-output.json | 283 ++++++++++++- server/swagger/swagger.js | 26 +- server/test/accountDeletion.test.js | 389 ++++++++++++++++++ server/test/contentReports.test.js | 56 ++- server/test/demoLocks.test.js | 1 + server/test/displayAuthor.test.js | 159 +++++++ server/test/emailTemplates.test.js | 3 +- server/test/eventModuleContract.test.js | 2 +- server/test/userDeletingHook.test.js | 87 ++++ 50 files changed, 2103 insertions(+), 69 deletions(-) create mode 100644 client/src/components/security/DeleteAccountPanel.jsx create mode 100644 client/src/lib/accountDeletion.js create mode 100644 client/src/lib/deletedAccount.js create mode 100644 client/src/lib/notice.js create mode 100644 client/test/accountDeletion.test.js create mode 100644 server/src/auth/accountDeletion.js create mode 100644 server/src/utils/displayAuthor.js create mode 100644 server/test/accountDeletion.test.js create mode 100644 server/test/displayAuthor.test.js create mode 100644 server/test/userDeletingHook.test.js diff --git a/client/src/api/client.js b/client/src/api/client.js index bfa41b6..8f1d9aa 100644 --- a/client/src/api/client.js +++ b/client/src/api/client.js @@ -117,6 +117,10 @@ export const api = { // The change endpoints re-issue the session cookie server-side, so the caller // stays signed in. myAccount: () => req('/auth/me/account'), + // Delete the signed-in account (ACCOUNT_DELETION.md §4.1). The password always, + // and `code` or `recoveryCode` when two-factor is on; refusals carry a `code`. + deleteMyAccount: ({ username, password, code, recoveryCode }) => + req('/auth/me/account', { method: 'DELETE', body: { username, password, code, recoveryCode } }), changeUsername: (username) => req('/auth/me/account/username', { method: 'PATCH', body: { username } }), changePassword: (newPassword, currentPassword) => diff --git a/client/src/components/DemoToast.jsx b/client/src/components/DemoToast.jsx index 5ca6a5a..dba913d 100644 --- a/client/src/components/DemoToast.jsx +++ b/client/src/components/DemoToast.jsx @@ -1,5 +1,6 @@ import { useEffect, useState } from 'react' import { onDemoLocked } from '../lib/demoNotice.js' +import { onNotice } from '../lib/notice.js' // The one toast for a write the public demo refused (DEMO_MODE.md §3.1). Fed // by api/client.js through lib/demoNotice.js, so every screen — and every @@ -11,6 +12,9 @@ export default function DemoToast() { const [message, setMessage] = useState(null) useEffect(() => onDemoLocked((text) => setMessage(text)), []) + // Also the site's one passing notice (lib/notice.js), on any deployment: the + // account page's "Your account has been deleted." after it has navigated home. + useEffect(() => onNotice((text) => setMessage(text)), []) useEffect(() => { if (!message) return undefined diff --git a/client/src/components/security/DeleteAccountPanel.jsx b/client/src/components/security/DeleteAccountPanel.jsx new file mode 100644 index 0000000..fdaca2d --- /dev/null +++ b/client/src/components/security/DeleteAccountPanel.jsx @@ -0,0 +1,178 @@ +import { useState } from 'react' +import { useNavigate } from 'react-router-dom' +import { useAuth } from '../../contexts/AuthContext.jsx' +import { useSite } from '../../contexts/SiteContext.jsx' +import { api } from '../../api/client.js' +import { showNotice } from '../../lib/notice.js' +import { + WHAT_GOES, WHAT_STAYS, STAFF_NAME_NOTE, isStaffRole, usernameMatches, isSharedDemoLogin, refusalMessage, +} from '../../lib/accountDeletion.js' + +// "Delete my account" (docs/website/ACCOUNT_DELETION.md §4.2). Last on the +// Account page and set apart from the sections above it. +// +// The account goes at once — there is no grace period (AD4) — so the safeguard +// is the confirm step: the typed username, the password, and a two-factor code +// when two-factor is on. Delete stays off until the username matches. +// +// On success the server has already dropped every session. This screen sends +// the browser home, re-reads the session (which is now gone) and says so. + +const DANGER = '#d98b84' + +export default function DeleteAccountPanel({ account }) { + const { refresh } = useAuth() + const { settings } = useSite() + const navigate = useNavigate() + + const [open, setOpen] = useState(false) + const [typed, setTyped] = useState('') + const [password, setPassword] = useState('') + const [code, setCode] = useState('') + const [useRecovery, setUseRecovery] = useState(false) + const [busy, setBusy] = useState(false) + const [error, setError] = useState('') + + const demoLocked = isSharedDemoLogin(settings, account.username) + const noPassword = account.has_password === false + const needsCode = Boolean(account.totp_enabled) + const ready = usernameMatches(typed, account.username) && password.length > 0 && (!needsCode || code.trim().length > 0) + + function close() { + setOpen(false) + setTyped('') + setPassword('') + setCode('') + setUseRecovery(false) + setError('') + } + + async function confirm(e) { + e.preventDefault() + if (!ready || busy) return + setBusy(true) + setError('') + try { + await api.deleteMyAccount({ + username: typed.trim(), + password, + ...(needsCode ? (useRecovery ? { recoveryCode: code.trim() } : { code: code.trim() }) : {}), + }) + navigate('/', { replace: true }) + showNotice('Your account has been deleted.') + await refresh() + } catch (err) { + setError(refusalMessage(err)) + setBusy(false) + } + } + + return ( +
+

+ Delete my account +

+
+

+ Your account is deleted straight away and cannot be restored. These go with it: +

+
    + {WHAT_GOES.map((line) =>
  • {line}
    • )} +
+

What stays:

+
    + {WHAT_STAYS.map((line) =>
  • {line}
    • )} +
+ {isStaffRole(account.role) &&

{STAFF_NAME_NOTE}

} +
+ + {noPassword && ( +

+ This account has no password yet. Set one in the Password section above first; the password is how you + confirm a deletion. +

+ )} + + {!open && ( +
+ + {demoLocked && ( +

+ This is turned off on the demo site. +

+ )} +
+ )} + + {open && ( +
+ + + {needsCode && ( + + )} +
+ + +
+ {error &&

{error}

} +
+ )} +
+ ) +} diff --git a/client/src/lib/accountDeletion.js b/client/src/lib/accountDeletion.js new file mode 100644 index 0000000..e41dff7 --- /dev/null +++ b/client/src/lib/accountDeletion.js @@ -0,0 +1,69 @@ +// What the Account page's "Delete my account" section says and decides +// (docs/website/ACCOUNT_DELETION.md §4.1, §4.2). Plain JS with tests, so the +// words and the confirm rule can be checked without a browser. +// +// The server is the guard: it checks the username, the password and the code +// again whatever this file decides. What is here only keeps a mis-tap from +// reaching it and turns its refusal codes into sentences. + +/** What goes with the account. The same list as BACKEND_DESIGN.md's. */ +export const WHAT_GOES = [ + 'your sign-in, password, email address, two-factor settings and recovery codes', + 'every signed-in device, trusted device and push registration', + 'your linked sign-ins, such as Google, Discord or Steam', + 'your game-account links, and anything this site granted in the game through them', + 'your notification settings and inbox', +] + +/** What stays, and who sees what of it. */ +export const WHAT_STAYS = [ + 'What you wrote where others read it, such as forum posts, stays. Other members see it as written by “[deleted player]”.', + 'Reports you filed and appeals you made stay as moderation records. Staff can still see who it was, and the activity log keeps its IP addresses.', + 'Event results stay with the score and the in-game name, with no site account behind them.', + 'A game account stays on the game server. Only its link to this site goes.', +] + +/** Said only to staff: their name is never free again (AD9). */ +export const STAFF_NAME_NOTE = + 'You are staff, so your username can never be used again, by anyone, once this account is deleted.' + +/** True for every role but player. */ +export const isStaffRole = (role) => Boolean(role) && role !== 'player' + +/** The confirm button is live once the typed name matches, ignoring case. */ +export function usernameMatches(typed, username) { + return typeof typed === 'string' + && typeof username === 'string' + && typed.trim().length > 0 + && typed.trim().toLowerCase() === username.toLowerCase() +} + +/** + * True when the signed-in account is one of the demo's shared logins, which the + * server refuses to delete. The section still shows (Play's reviewer must find + * it); only its button is off. + */ +export function isSharedDemoLogin(settings, username) { + const logins = settings?.demo?.logins + if (!Array.isArray(logins) || typeof username !== 'string') return false + const name = username.toLowerCase() + return logins.some((l) => typeof l?.username === 'string' && l.username.toLowerCase() === name) +} + +const BY_CODE = { + username_mismatch: 'Type your username exactly as it is shown.', + wrong_password: 'Your password is incorrect.', + code_required: 'Enter a code from your authenticator app, or a recovery code.', + wrong_code: 'That code is not valid. Try again.', + password_required: 'This account has no password yet. Set one in the Password section above, then delete the account.', + last_admin: 'You are the only admin. Make someone else an admin before you delete your account.', + DEMO_LOCKED: 'This is turned off on the demo site.', +} + +/** A refusal from DELETE /auth/me/account, as one sentence. */ +export function refusalMessage(err) { + const code = err?.body?.code + if (code && BY_CODE[code]) return BY_CODE[code] + if (err?.status === 429) return 'Too many attempts. Wait a few minutes and try again.' + return err?.message || 'Could not delete your account. Try again.' +} diff --git a/client/src/lib/deletedAccount.js b/client/src/lib/deletedAccount.js new file mode 100644 index 0000000..01568a1 --- /dev/null +++ b/client/src/lib/deletedAccount.js @@ -0,0 +1,25 @@ +// How a deleted account's name is shown (ACCOUNT_DELETION.md AD5, AD6). +// +// The server decides WHAT name a viewer gets (server/src/utils/displayAuthor.js): +// a member is sent "[deleted player]", staff are sent the stored name and a +// `…Deleted` flag. All this file does is mark the staff case, so a moderator +// reading "Ravenna" knows that account no longer exists. + +export const DELETED_PLAYER = '[deleted player]' + +/** "Ravenna (deleted account)" for staff; the placeholder is left as it is. */ +export function markDeleted(name, deleted) { + if (!name) return name + return deleted && name !== DELETED_PLAYER ? `${name} (deleted account)` : name +} + +/** + * The same for an admin view that is handed raw ledger rows (`actor_user_id`, + * `actor_username`): an id that is NULL beside a snapshot that is not is a + * deleted account, and nothing else clears those ids. Only an explicit null + * counts — a row that does not carry the id column at all says nothing. + */ +export function rowName(userId, username, fallback = '—') { + if (!username) return fallback + return markDeleted(username, userId === null) +} diff --git a/client/src/lib/notice.js b/client/src/lib/notice.js new file mode 100644 index 0000000..738c89d --- /dev/null +++ b/client/src/lib/notice.js @@ -0,0 +1,27 @@ +// A one-line notice for the site-wide toast (components/DemoToast.jsx), for the +// rare moment a screen has to say something after it has navigated away — the +// account page cannot show "Your account has been deleted." on itself, because +// the account it would be shown on no longer exists. +// +// The same shape as lib/demoNotice.js and drawn by the same toast, so the site +// has one place a passing message appears. + +const listeners = new Set() + +/** Subscribe to notices. Returns the unsubscribe function. */ +export function onNotice(fn) { + listeners.add(fn) + return () => listeners.delete(fn) +} + +/** Show one notice. */ +export function showNotice(message) { + if (typeof message !== 'string' || !message) return + for (const fn of listeners) { + try { + fn(message) + } catch { + // A broken listener must not stop the caller. + } + } +} diff --git a/client/src/lib/teamAdmin.js b/client/src/lib/teamAdmin.js index fd7d6e7..a1f5ba3 100644 --- a/client/src/lib/teamAdmin.js +++ b/client/src/lib/teamAdmin.js @@ -7,6 +7,8 @@ // for two hours", and those two produce almost the same page. Getting that // wording right is logic, not markup. +import { markDeleted, rowName } from './deletedAccount.js' + /** Tones the screen uses. Names, not colours — the view maps them. */ export const TONE = { ok: 'ok', warn: 'warn', bad: 'bad', idle: 'idle' } @@ -91,7 +93,7 @@ export const GATED_NOTE = /** A one-line description of a queued request, for the approval queue. */ export function describeRequest(request = {}) { const payload = parsePayload(request.payload) - const who = request.requested_username || 'a deleted user' + const who = rowName(request.requested_by, request.requested_username, 'a deleted user') switch (request.action) { case 'unhide': return `${who} asks to publish “${request.team_name}”` @@ -133,7 +135,7 @@ export function leadershipOf(member = {}) { return { isLeader: granted, overridden: true, - note: `${granted ? 'Granted' : 'Denied'} by ${member.leaderOverride.by || 'a deleted user'}` + note: `${granted ? 'Granted' : 'Denied'} by ${markDeleted(member.leaderOverride.by, member.leaderOverride.byDeleted) || 'a deleted user'}` + `${member.leaderOverride.reason ? ` — ${member.leaderOverride.reason}` : ''}` + ` (the game says ${member.isLeaderSynced ? 'leader' : 'not a leader'})`, } diff --git a/client/src/lib/teamForum.js b/client/src/lib/teamForum.js index b52ae3a..0f11086 100644 --- a/client/src/lib/teamForum.js +++ b/client/src/lib/teamForum.js @@ -12,6 +12,8 @@ // `created_at` when the write arrives. A client that granted one would be // deciding a time-bounded permission against the clock of the party it bounds. +import { markDeleted } from './deletedAccount.js' + export const REPORT_REASONS = [ ['abuse', 'Abusive or harassing'], ['spam', 'Spam'], @@ -75,7 +77,7 @@ export function stripToText(html) { export function threadSummary(thread) { const parts = [] if (thread.type === 'announcement') parts.push('Announcement') - parts.push(thread.author) + parts.push(markDeleted(thread.author, thread.authorDeleted)) if (thread.type === 'discussion' && thread.postCount > 1) { const replies = thread.postCount - 1 parts.push(`${replies} ${replies === 1 ? 'reply' : 'replies'}`) diff --git a/client/src/modules/TeamForumPanel.jsx b/client/src/modules/TeamForumPanel.jsx index 5b2ea9f..f7291f2 100644 --- a/client/src/modules/TeamForumPanel.jsx +++ b/client/src/modules/TeamForumPanel.jsx @@ -5,6 +5,8 @@ import { api } from '../api/client.js' import { useAuth } from '../contexts/AuthContext.jsx' import { useSite } from '../contexts/SiteContext.jsx' import { REPORT_REASONS, editOfferOpen, stripToText, threadSummary } from '../lib/teamForum.js' +// A member is sent "[deleted player]"; staff the stored name, which this marks. +import { markDeleted } from '../lib/deletedAccount.js' // Core's Team forum, rendered into a second slot a MODULE declares // (TEAMS.md Part 5, and the phase 3 amendment to §3.4). @@ -332,8 +334,7 @@ function ThreadView({ slug, thread, canModerate, imageMode, onBack, onChanged, o

{thread.type === 'announcement' ? 'Announcement · ' : ''} - {thread.author} - {thread.authorDeleted && ' (account removed)'} + {markDeleted(thread.author, thread.authorDeleted)} {thread.locked && ' · locked'}

@@ -448,8 +449,7 @@ function PostView({ slug, post, canModerate, now, onChanged }) { return (

- {post.author} - {post.authorDeleted && ' (account removed)'} + {markDeleted(post.author, post.authorDeleted)} {post.editedAt && ' · edited'} {post.status === 'hidden' && ' · hidden'}

diff --git a/client/src/modules/version.js b/client/src/modules/version.js index 138f1b4..f9deb23 100644 --- a/client/src/modules/version.js +++ b/client/src/modules/version.js @@ -11,6 +11,9 @@ // that the two files can drift, so a test asserts they agree // (client/test/moduleRegistry.test.js) rather than trusting a bump to remember // both. +// 1.20.0 — `api.onUserDeleting` (ACCOUNT_DELETION.md §4.1, AD10): a module +// unlinks a person's game links before their account is deleted. Server-side +// only. This file bumps for the reason at the top. // 1.19.0 — an event action may declare `demoRefuses(params)` (DEMO_MODE.md §3.4, // DM18): the demo refuses a step at save. Server-side only; the refusal reaches // the browser as an ordinary DEMO_LOCKED answer. This file bumps for the reason at the top. @@ -105,4 +108,4 @@ // but the two halves state ONE version: a module declares a single coreApi range // and is served one chunk, so a client that claimed 1.0.0 while the server // answered 1.1.0 would be two answers to one question. -export const MODULE_API_VERSION = '1.19.0' +export const MODULE_API_VERSION = '1.20.0' diff --git a/client/src/routes/admin/views/AccountAdmin.jsx b/client/src/routes/admin/views/AccountAdmin.jsx index 0745ce7..181ea8b 100644 --- a/client/src/routes/admin/views/AccountAdmin.jsx +++ b/client/src/routes/admin/views/AccountAdmin.jsx @@ -5,6 +5,7 @@ import RecoveryCodesDisplay from '../../../components/security/RecoveryCodesDisp import TrustedDevicesPanel from '../../../components/security/TrustedDevicesPanel.jsx' import RecoveryCodesPanel from '../../../components/security/RecoveryCodesPanel.jsx' import EmailAddressPanel from '../../../components/security/EmailAddressPanel.jsx' +import DeleteAccountPanel from '../../../components/security/DeleteAccountPanel.jsx' import { IdentitySummary, linkBanner, linkableProviders, unlinkPrompt } from '../../../components/security/linkedIdentities.jsx' import { api } from '../../../api/client.js' @@ -315,6 +316,10 @@ export default function AccountAdmin() { {account && } + + {/* The same section the player portal shows: every member can delete + themself, staff included (ACCOUNT_DELETION.md §1). */} + {account && } ) } diff --git a/client/src/routes/admin/views/ContentReports.jsx b/client/src/routes/admin/views/ContentReports.jsx index 9f780a6..d0d8fbc 100644 --- a/client/src/routes/admin/views/ContentReports.jsx +++ b/client/src/routes/admin/views/ContentReports.jsx @@ -4,6 +4,7 @@ import { Loading, ErrorState } from '../../../components/PageState.jsx' import { useAsync } from '../../../lib/useAsync.js' import { ago, dateTime } from '../../../lib/format.js' import { api } from '../../../api/client.js' +import { markDeleted } from '../../../lib/deletedAccount.js' // The member-raised content-report queue (TEAMS.md §5.6). // @@ -84,7 +85,7 @@ function TargetCell({ report }) { {t.filename} - {t.uploader || 'unknown'} · {t.mimetype} · {bytes(t.byteSize)} + {markDeleted(t.uploader, t.uploaderDeleted) || 'unknown'} · {t.mimetype} · {bytes(t.byteSize)} {t.deleted && ' · removed'} @@ -95,7 +96,7 @@ function TargetCell({ report }) { {t.title} - {t.type} by {t.author || 'unknown'} + {t.type} by {markDeleted(t.author, t.authorDeleted) || 'unknown'} {t.status !== 'visible' && ` · ${t.status}`} @@ -105,7 +106,7 @@ function TargetCell({ report }) { {t.excerpt || (no text)} - {t.author || 'unknown'} in “{t.threadTitle}” + {markDeleted(t.author, t.authorDeleted) || 'unknown'} in “{t.threadTitle}” {t.status !== 'visible' && ` · ${t.status}`} @@ -192,13 +193,13 @@ export default function ContentReports() { {REASON_LABEL[r.reason] || r.reason} {r.detail || '—'} - {r.reporter} + {markDeleted(r.reporter, r.reporterDeleted)} {ago(r.createdAt)} {STATUS_LABEL[r.status] || r.status} {r.handledBy && ( - {r.handledBy} + {markDeleted(r.handledBy, r.handledByDeleted)} {r.handledNote ? ` — ${r.handledNote}` : ''} )} diff --git a/client/src/routes/admin/views/TeamsAdmin.jsx b/client/src/routes/admin/views/TeamsAdmin.jsx index 00f9d8b..2282312 100644 --- a/client/src/routes/admin/views/TeamsAdmin.jsx +++ b/client/src/routes/admin/views/TeamsAdmin.jsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useState } from 'react' import { Loading, ErrorState } from '../../../components/PageState.jsx' import { dateTime } from '../../../lib/format.js' +import { rowName } from '../../../lib/deletedAccount.js' import { freshnessOf, statusOf, gateLabelFor, describeRequest, leadershipOf, GATED_NOTE, } from '../../../lib/teamAdmin.js' @@ -331,7 +332,7 @@ function ForumLedger({ team, onClose }) { {dateTime(r.created_at)} {r.action} {r.target_type} #{r.target_id} - {r.actor_username || '—'} + {rowName(r.actor_user_id, r.actor_username)} {/* The distinction the whole ledger exists to preserve. */} {r.actor_role} diff --git a/client/src/routes/player/PlayerAccount.jsx b/client/src/routes/player/PlayerAccount.jsx index 8817414..d8de1a2 100644 --- a/client/src/routes/player/PlayerAccount.jsx +++ b/client/src/routes/player/PlayerAccount.jsx @@ -5,6 +5,7 @@ import RecoveryCodesDisplay from '../../components/security/RecoveryCodesDisplay import TrustedDevicesPanel from '../../components/security/TrustedDevicesPanel.jsx' import RecoveryCodesPanel from '../../components/security/RecoveryCodesPanel.jsx' import EmailAddressPanel from '../../components/security/EmailAddressPanel.jsx' +import DeleteAccountPanel from '../../components/security/DeleteAccountPanel.jsx' import { IdentitySummary, linkBanner, linkableProviders, unlinkPrompt } from '../../components/security/linkedIdentities.jsx' import { useAuth } from '../../contexts/AuthContext.jsx' import { api } from '../../api/client.js' @@ -422,6 +423,7 @@ export default function PlayerAccount() { )} + )} diff --git a/client/test/accountDeletion.test.js b/client/test/accountDeletion.test.js new file mode 100644 index 0000000..d9e2f1c --- /dev/null +++ b/client/test/accountDeletion.test.js @@ -0,0 +1,80 @@ +// The Account page's "Delete my account" section (client/src/lib/accountDeletion.js) +// and how a deleted account's name is marked for staff (lib/deletedAccount.js). +// docs/website/ACCOUNT_DELETION.md §4.1, §4.2, AD5, AD6. +import { test } from 'node:test' +import assert from 'node:assert/strict' + +import { + usernameMatches, isSharedDemoLogin, refusalMessage, isStaffRole, WHAT_GOES, WHAT_STAYS, +} from '../src/lib/accountDeletion.js' +import { DELETED_PLAYER, markDeleted, rowName } from '../src/lib/deletedAccount.js' +import { describeRequest, leadershipOf } from '../src/lib/teamAdmin.js' +import { threadSummary } from '../src/lib/teamForum.js' + +// ── the confirm step ─────────────────────────────────────────────────────── + +test('Delete is live only once the typed username matches, ignoring case and spaces', () => { + assert.equal(usernameMatches('Thorne', 'Thorne'), true) + assert.equal(usernameMatches(' thorne ', 'Thorne'), true) + assert.equal(usernameMatches('Thorn', 'Thorne'), false) + assert.equal(usernameMatches('', 'Thorne'), false) + assert.equal(usernameMatches(' ', 'Thorne'), false) + assert.equal(usernameMatches(undefined, 'Thorne'), false) +}) + +test('the demo’s shared logins are recognised from the published list, and nothing else is', () => { + const settings = { demo: { logins: [{ username: 'demo-player' }, { username: 'play-review' }] } } + assert.equal(isSharedDemoLogin(settings, 'Demo-Player'), true) + assert.equal(isSharedDemoLogin(settings, 'alice'), false) + // Off the demo there is no `demo` block, so nobody is a shared login. + assert.equal(isSharedDemoLogin({}, 'demo-player'), false) + assert.equal(isSharedDemoLogin(undefined, 'demo-player'), false) +}) + +test('every refusal code the route answers has its own sentence', () => { + for (const code of ['username_mismatch', 'wrong_password', 'code_required', 'wrong_code', 'password_required', 'last_admin', 'DEMO_LOCKED']) { + const msg = refusalMessage({ status: 400, body: { code }, message: 'server words' }) + assert.notEqual(msg, 'server words', code) + assert.ok(msg.length > 10, code) + } + assert.match(refusalMessage({ status: 429, body: null }), /Too many attempts/) + assert.equal(refusalMessage({ status: 500, message: 'Internal Server Error' }), 'Internal Server Error') + assert.match(refusalMessage(null), /Could not delete/) +}) + +test('the screen says what goes and what stays, and that staff names are kept forever', () => { + assert.ok(WHAT_GOES.some((l) => /game-account links/.test(l))) + assert.ok(WHAT_STAYS.some((l) => l.includes('[deleted player]'))) + assert.equal(isStaffRole('moderator'), true) + assert.equal(isStaffRole('player'), false) +}) + +// ── staff see a deleted account's name, marked ───────────────────────────── + +test('a deleted account’s stored name is marked; the placeholder is left alone', () => { + assert.equal(markDeleted('Ravenna', true), 'Ravenna (deleted account)') + assert.equal(markDeleted('Ravenna', false), 'Ravenna') + assert.equal(markDeleted(DELETED_PLAYER, true), DELETED_PLAYER) + assert.equal(markDeleted(null, true), null) +}) + +test('a raw ledger row is a deleted account only when its id is an explicit null', () => { + assert.equal(rowName(null, 'OldMod'), 'OldMod (deleted account)') + assert.equal(rowName(4, 'OldMod'), 'OldMod') + assert.equal(rowName(undefined, 'OldMod'), 'OldMod') + assert.equal(rowName(null, null), '—') +}) + +test('Admin → Teams marks a deleted requester and a deleted override author', () => { + assert.match( + describeRequest({ action: 'unhide', requested_by: null, requested_username: 'mod1', team_name: 'X' }), + /^mod1 \(deleted account\) asks/, + ) + const l = leadershipOf({ isLeaderSynced: true, leaderOverride: { effect: 'deny', by: 'mod1', byDeleted: true } }) + assert.match(l.note, /Denied by mod1 \(deleted account\)/) +}) + +test('a forum thread by a deleted account is marked in the list for staff', () => { + assert.match(threadSummary({ type: 'discussion', author: 'Ravenna', authorDeleted: true, postCount: 1 }), /^Ravenna \(deleted account\)/) + assert.equal(threadSummary({ type: 'discussion', author: DELETED_PLAYER, authorDeleted: true, postCount: 1 }), DELETED_PLAYER) +}) diff --git a/server/db/schema.sql b/server/db/schema.sql index 7dd569e..3ca41fc 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -65,6 +65,22 @@ CREATE TABLE IF NOT EXISTS users ( UNIQUE KEY uq_users_email_norm (email_norm) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +-- Usernames that can never be taken again (ACCOUNT_DELETION.md AD9). A deleted +-- STAFF account's name lands here in the same transaction that deletes the row, +-- so nobody can register "Ravenna" the day after the moderator Ravenna left and +-- speak with her name. A player's name is never added and is free at once. +-- +-- There is no release path, by decision ("completely unavailable"). The column +-- carries the users.username collation so "reserved" means exactly what "taken" +-- means: the same case folding the UNIQUE index above applies. Registration, an +-- invite's acceptance, an admin creating or renaming an account and a username +-- change all refuse a reserved name with the answer they give a taken one. +CREATE TABLE IF NOT EXISTS reserved_usernames ( + username_norm VARCHAR(32) NOT NULL COLLATE utf8mb4_general_ci PRIMARY KEY, + reason ENUM('deleted_staff') NOT NULL, + reserved_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + CREATE TABLE IF NOT EXISTS posts ( id INT AUTO_INCREMENT PRIMARY KEY, category ENUM('news','five_on_friday','newsletter','screenshot') NOT NULL, @@ -1153,7 +1169,7 @@ CREATE TABLE IF NOT EXISTS team_forum_posts ( id BIGINT AUTO_INCREMENT PRIMARY KEY, thread_id INT NOT NULL, author_user_id INT NULL, - author_username VARCHAR(32) NULL, -- snapshot; renders as "[deleted account]" when both are gone + author_username VARCHAR(32) NULL, -- snapshot; with the id NULL it is a deleted account: "[deleted player]" to members, the name to staff (utils/displayAuthor.js) body_html MEDIUMTEXT NOT NULL, -- sanitised on write via utils/forumHtml.js created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, edited_at DATETIME NULL, diff --git a/server/engagement-triggers.json b/server/engagement-triggers.json index eba8631..e73b79b 100644 --- a/server/engagement-triggers.json +++ b/server/engagement-triggers.json @@ -1,6 +1,6 @@ { "_comment": "Generated event-trigger inventory - the authoritative freeze of CORE's engagement contract (docs/website/ENGAGEMENT.md 4.3). Regenerate with `npm run engagement:manifest` in website/server. A renamed variable, a changed type or a widened ceiling breaks stored templates and rules, so the diff here is the review signal. A module ships its own copy in its bundle; this file never contains one.", - "moduleApiVersion": "1.19.0", + "moduleApiVersion": "1.20.0", "triggers": [ { "id": "event.phase.changed", diff --git a/server/routes.guards.json b/server/routes.guards.json index 0c23f9e..ed1681d 100644 --- a/server/routes.guards.json +++ b/server/routes.guards.json @@ -1784,6 +1784,18 @@ "requireAuth" ] }, + { + "method": "DELETE", + "path": "/api/v1/auth/me/account", + "handlers": 9, + "gates": [ + "noindex", + "requireAuth", + "backoffGuard", + "middleware", + "validate" + ] + }, { "method": "GET", "path": "/api/v1/auth/me/account", diff --git a/server/routes.manifest.json b/server/routes.manifest.json index 9425200..465c840 100644 --- a/server/routes.manifest.json +++ b/server/routes.manifest.json @@ -733,6 +733,10 @@ "method": "GET", "path": "/api/v1/auth/me" }, + { + "method": "DELETE", + "path": "/api/v1/auth/me/account" + }, { "method": "GET", "path": "/api/v1/auth/me/account" diff --git a/server/src/auth/accountDeletion.js b/server/src/auth/accountDeletion.js new file mode 100644 index 0000000..e447955 --- /dev/null +++ b/server/src/auth/accountDeletion.js @@ -0,0 +1,174 @@ +// ── Deleting an account (docs/website/ACCOUNT_DELETION.md §4.1) ──────────── +// +// One procedure for both ways an account goes: a member deleting their own +// (`DELETE /auth/me/account`, by: 'self') and staff deleting it from the admin +// panel (`DELETE /admin/users/:id`, by: 'staff'). The two routes check different +// things before they get here — the member proves who they are, the admin is not +// allowed to delete themself — and from here on they do the same thing. +// +// In order: +// +// 1. **Before the delete.** Read what is needed afterwards (the address for the +// notice, the username, the role), then ask every started module's +// `onUserDeleting` hook in turn (MODULE_API 1.20.0). That is where game links +// are unlinked through each module's own unlink (AD10): a cascade is not an +// unlink, and for UO the link lives on the shard. +// 2. **One transaction.** A staff name is added to `reserved_usernames` (AD9) +// and the users row is deleted. The foreign keys do the rest: credentials, +// sessions and settings cascade, authored content is detached. Snapshot +// columns, the content and the activity log are NOT touched (AD5, AD6) — +// who sees which name is utils/displayAuthor's rule, applied when read. +// 3. **After commit**, fire-and-forget, each step on its own: a Teams +// reconcile, the activity-log entry, and the notice to the old address. +// +// **Nothing stops a deletion once it starts** (AD11). A hook that throws, or runs +// past HOOK_TIMEOUT_MS, is logged and the deletion goes ahead: Play requires the +// request to be honoured, and a game server that is down must not hold a +// member's request open. Whatever a module could not finish is its own pending +// work. The refusals below all come BEFORE step 1. + +const users = require('../model/users/users.model') +const activity = require('../model/activity/activity.model') +const teamSync = require('../model/teams/teamSync.model') +const loader = require('../modules/loader') +const mailer = require('../utils/mailer') +const { isDemo, isDemoUsername } = require('../demo/mode') +const log = require('../utils/logger')('account-deletion') + +const HOOK_TIMEOUT_MS = 10_000 +const BY = ['self', 'staff'] + +function withTimeout(fn, ms) { + let timer + const deadline = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`no answer within ${ms} ms`)), ms) + }) + return Promise.race([Promise.resolve().then(fn), deadline]).finally(() => clearTimeout(timer)) +} + +/** + * Ask every started module's `onUserDeleting`, one at a time. + * + * In turn rather than at once so a module's log lines read in the order they ran, + * and because a site runs one module: there is nothing to parallelise. Never + * throws; returns what each module did, for the log. + * + * @returns {Promise<{module: string, ok: boolean, error?: string}[]>} + */ +async function runHooks(payload, timeoutMs) { + const frozen = Object.freeze({ userId: payload.userId, deletedBy: payload.deletedBy }) + const outcomes = [] + for (const { id, hook } of loader.userDeletingHooks()) { + try { + // eslint-disable-next-line no-await-in-loop + await withTimeout(() => hook(frozen), timeoutMs) + outcomes.push({ module: id, ok: true }) + } catch (err) { + log.error('a module could not finish before an account was deleted; deleting anyway', { + module: id, + userId: frozen.userId, + message: err.message, + }) + outcomes.push({ module: id, ok: false, error: err.message }) + } + } + return outcomes +} + +// "29 October 2026", in UTC so the date in the mail does not depend on the +// host's zone. +function formatDate(when) { + return when.toLocaleDateString('en-GB', { day: 'numeric', month: 'long', year: 'numeric', timeZone: 'UTC' }) +} + +/** + * Step 3. Each step in its own try, and the whole thing never rejects, so the + * caller may leave it un-awaited. Returned anyway, so a test has a moment at + * which the after-commit work has happened. + */ +async function afterCommit({ userId, by, req, email, username, reserved, when }) { + try { + teamSync.request({ reason: 'account deleted' }) + } catch (err) { + log.warn('teams reconcile request failed after a deletion', { message: err.message }) + } + + // Never the username: the entry outlives the account, and the activity log is + // a record of what was done, not a second place the name is kept. A member + // deleting themself is no longer a user the row could name, so the entry + // carries no user id, only the IP the request came from (AD6). + try { + const action = by === 'self' ? 'user.self_delete' : 'user.delete' + const detail = { id: userId, ...(reserved ? { usernameReserved: true } : {}) } + if (by === 'self') { + await activity.log({ req: { ip: req ? req.ip : null }, action, detail }) + } else { + await activity.log({ req, action, detail }) + } + } catch (err) { + log.warn('activity entry failed after a deletion', { message: err.message }) + } + + if (email) { + try { + const sent = await mailer.sendAccountDeleted({ to: email, username, deletedOn: formatDate(when), by }) + if (!sent.sent) log.info('deletion notice not sent (mail not configured)', { userId }) + } catch (err) { + log.warn('deletion notice failed', { userId, message: err.message }) + } + } +} + +/** + * Delete one account. + * + * @param {number} userId + * @param {{by: 'self'|'staff', req?: object, hookTimeoutMs?: number}} opts + * @returns {Promise< + * | {ok: true, id: number, reserved: boolean, modules: object[], settled: Promise} + * | {ok: false, reason: 'not_found' | 'demo' | 'last_admin'} + * >} + */ +async function deleteAccount(userId, { by, req = null, hookTimeoutMs = HOOK_TIMEOUT_MS } = {}) { + if (!BY.includes(by)) throw new Error(`deleteAccount: by must be one of ${BY.join(', ')}`) + + const target = await users.getById(userId) + if (!target) return { ok: false, reason: 'not_found' } + // The demo's shared logins. The demo lock refuses both routes before they get + // here (demo/locks.js); this is the line behind it, so no future caller of the + // service can delete the account every visitor signs in as. + if (isDemo() && isDemoUsername(target.username)) return { ok: false, reason: 'demo' } + // A deployment with no admin left has nobody who can run it. + if (target.role === 'admin' && (await users.countAdmins()) <= 1) return { ok: false, reason: 'last_admin' } + + // 1. Before the delete. + const modules = await runHooks({ userId, deletedBy: by }, hookTimeoutMs) + + // 2. One transaction: reserve a staff name, delete the row. + const reserveName = target.role !== 'player' ? target.username : null + const removed = await users.removeReserving(userId, reserveName) + // Gone between the read above and now: somebody else's deletion won. Nothing + // of ours happened, so nothing after commit runs either. + if (!removed) return { ok: false, reason: 'not_found' } + + log.info('account deleted', { + userId, + by, + reserved: Boolean(reserveName), + modules: modules.map((m) => `${m.module}:${m.ok ? 'ok' : 'failed'}`), + }) + + // 3. After commit. + const settled = afterCommit({ + userId, + by, + req, + email: target.email || null, + username: target.username, + reserved: Boolean(reserveName), + when: new Date(), + }) + return { ok: true, id: userId, reserved: Boolean(reserveName), modules, settled } +} + +module.exports = { deleteAccount, HOOK_TIMEOUT_MS } diff --git a/server/src/demo/locks.js b/server/src/demo/locks.js index 470c7e6..4d381d6 100644 --- a/server/src/demo/locks.js +++ b/server/src/demo/locks.js @@ -109,6 +109,10 @@ const CORE_LOCKS = [ { methods: ['POST'], path: '/auth/me/account/totp/disable', when: callerIsDemo }, { methods: ['POST'], path: '/auth/me/account/recovery-codes/generate', when: callerIsDemo }, { methods: ['DELETE'], path: '/auth/me/account/identities/:provider', when: callerIsDemo }, + // ACCOUNT_DELETION.md §4.3: the screen stays visible on the demo, because + // Play's reviewer must be able to find it, and the shared logins cannot be + // deleted. An account an admin created on the demo can be. + { methods: ['DELETE'], path: '/auth/me/account', when: callerIsDemo }, // DM17: the whole route, because a mobile access token carries no refresh-row // id and "a session that is not the caller's" cannot be decided. Signing out // (POST /auth/mobile/logout) revokes the caller's own row and stays open. diff --git a/server/src/engagement/templateSeeds.js b/server/src/engagement/templateSeeds.js index 510e8f7..1bbaab7 100644 --- a/server/src/engagement/templateSeeds.js +++ b/server/src/engagement/templateSeeds.js @@ -157,6 +157,36 @@ const SEEDS = [ ), ], }, + { + // ACCOUNT_DELETION.md §4.1 step 3: the one notice a person gets if somebody + // else used their unlocked phone. It goes to the address the account HAD, + // read before the row went, and there is no link in it because there is no + // account left for a link to reach. + key: 'auth.account-deleted', + name: 'Account deleted', + channel: 'email', + protected: true, + seedVersion: 1, + subject: 'Your {{siteName}} account was deleted', + variables: [ + { name: 'username', type: 'string', required: true, example: 'Darrow' }, + { name: 'deletedOn', type: 'string', required: true, example: '29 October 2026' }, + { name: 'byWhom', type: 'string', required: false, example: ' at your request' }, + ], + blocks: [ + text('p1', 'The {{siteName}} account “{{username}}” was deleted on {{deletedOn}}{{byWhom}}.'), + text( + 'p2', + 'Its sign-in, email address, two-factor settings, linked sign-ins and game links are gone. ' + + 'What it wrote where others read it stays, and other members see it as written by “[deleted player]”.', + ), + text( + 'p3', + 'If you did not ask for this, contact the {{siteName}} staff at {{siteUrl}}. ' + + 'A deleted account cannot be restored, but they can find out what happened.', + ), + ], + }, { key: 'admin.contact-message', name: 'Contact form message', diff --git a/server/src/model/reports/contentReports.db.js b/server/src/model/reports/contentReports.db.js index 8194714..c233f46 100644 --- a/server/src/model/reports/contentReports.db.js +++ b/server/src/model/reports/contentReports.db.js @@ -114,7 +114,7 @@ async function handle(id, { status, handledBy, handledUsername, note }) { async function threadsByIds(ids) { if (!ids.length) return [] return query( - `SELECT id, team_id, title, type, status, created_username FROM team_forum_threads + `SELECT id, team_id, title, type, status, created_by, created_username FROM team_forum_threads WHERE id IN (${ids.map(() => '?').join(',')})`, ids, ) diff --git a/server/src/model/reports/contentReports.model.js b/server/src/model/reports/contentReports.model.js index 49fd435..3b120fa 100644 --- a/server/src/model/reports/contentReports.model.js +++ b/server/src/model/reports/contentReports.model.js @@ -26,6 +26,7 @@ const reportsDb = require('./contentReports.db') const forumDb = require('../teams/teamForum.db') +const { displayAuthor, hasActor } = require('../../utils/displayAuthor') const TARGET_TYPES = ['team_forum_thread', 'team_forum_post', 'team_forum_upload'] const REASONS = ['spam', 'abuse', 'sexual', 'illegal', 'impersonation', 'other'] @@ -116,7 +117,7 @@ async function file({ team, actor, targetType, targetId, reason, detail }) { * hide the pattern of a member deleting their own content the moment it is * reported. */ -async function queue({ status, teamId, limit, offset } = {}) { +async function queue({ status, teamId, limit, offset, viewer = null } = {}) { const rows = await reportsDb.list({ status, teamId, limit, offset }) if (!rows.length) return [] @@ -132,9 +133,18 @@ async function queue({ status, teamId, limit, offset } = {}) { const postMap = byId(posts) const uploadMap = byId(uploads) - return rows.map((r) => ({ ...publicReport(r), target: describeTarget(r, { threadMap, postMap, uploadMap }) })) + return rows.map((r) => ({ + ...publicReport(r, viewer), + target: describeTarget(r, { threadMap, postMap, uploadMap }, viewer), + })) } +// A snapshot pair through displayAuthor's rule (ACCOUNT_DELETION.md AD5, AD6). +// The queue is staff-only by its route, so a staff viewer sees the stored name +// and the `*Deleted` flag marks it; the rule is still asked rather than assumed, +// so this file cannot become the place a member's view leaks a deleted name. +const shown = (userId, username, viewer) => displayAuthor({ userId, username }, viewer) + /** * The reported content, resolved. * @@ -144,29 +154,33 @@ async function queue({ status, teamId, limit, offset } = {}) { * than as an explicit null, and the queue's own contract says nullable. A client * distinguishing "gone" from "not resolved yet" would get it wrong. */ -function describeTarget(report, { threadMap, postMap, uploadMap }) { +function describeTarget(report, { threadMap, postMap, uploadMap }, viewer = null) { const id = Number(report.target_id) if (report.target_type === 'team_forum_thread') { const t = threadMap.get(id) if (!t) return null + const author = shown(t.created_by, t.created_username, viewer) return { kind: 'thread', threadId: t.id, title: t.title, type: t.type, status: t.status, - author: t.created_username, + author: author.name, + authorDeleted: author.deleted, } } if (report.target_type === 'team_forum_post') { const p = postMap.get(id) if (!p) return null + const author = shown(p.author_user_id, p.author_username, viewer) return { kind: 'post', postId: p.id, threadId: p.thread_id, threadTitle: p.thread_title, - author: p.author_username, + author: author.name, + authorDeleted: author.deleted, status: p.status, excerpt: excerpt(p.body_html), createdAt: p.created_at, @@ -178,11 +192,13 @@ function describeTarget(report, { threadMap, postMap, uploadMap }) { // §5.6's fourth rule: uploader, size and the SNIFFED type, without hunting. // This is the payoff for §5.5.4's attribution table being load-bearing rather // than bookkeeping. + const uploader = shown(u.uploader_user_id, u.uploader_username, viewer) return { kind: 'upload', uploadId: u.id, postId: u.post_id, - uploader: u.uploader_username, + uploader: uploader.name, + uploaderDeleted: uploader.deleted, filename: u.filename, url: `/uploads/${u.filename}`, mimetype: u.mimetype, @@ -194,18 +210,25 @@ function describeTarget(report, { threadMap, postMap, uploadMap }) { return null } -function publicReport(row) { +function publicReport(row, viewer = null) { + const reporter = shown(row.reporter_user_id, row.reporter_username, viewer) + // Nobody has handled an open report, which is not the same as a deleted + // handler: only a pair that names someone goes through the rule. + const handler = hasActor(row.handled_by, row.handled_username) + ? shown(row.handled_by, row.handled_username, viewer) + : null return { id: row.id, targetType: row.target_type, targetId: Number(row.target_id), teamId: row.team_id, - reporter: row.reporter_username || '[deleted account]', - reporterDeleted: row.reporter_user_id == null, + reporter: reporter.name, + reporterDeleted: reporter.deleted, reason: row.reason, detail: row.detail, status: row.status, - handledBy: row.handled_username, + handledBy: handler ? handler.name : null, + handledByDeleted: handler ? handler.deleted : false, handledNote: row.handled_note, handledAt: row.handled_at, createdAt: row.created_at, @@ -226,7 +249,7 @@ async function handle({ id, actor, status, note }) { handledUsername: actor.username, note, }) - return { ok: true, report: publicReport(await reportsDb.byId(id)) } + return { ok: true, report: publicReport(await reportsDb.byId(id), actor) } } module.exports = { diff --git a/server/src/model/teams/teamAccess.model.js b/server/src/model/teams/teamAccess.model.js index 83cecfe..5fd8454 100644 --- a/server/src/model/teams/teamAccess.model.js +++ b/server/src/model/teams/teamAccess.model.js @@ -112,7 +112,15 @@ async function rosterWithOverrides(teamId, { includeDeparted = false } = {}) { is_leader_synced: Boolean(m.is_leader), is_leader: override ? override.effect === 'grant' : Boolean(m.is_leader), leader_override: override - ? { effect: override.effect, reason: override.reason, by: override.actor_username, at: override.created_at } + ? { + effect: override.effect, + reason: override.reason, + by: override.actor_username, + // Staff-only (adminMember); a deleted actor keeps the stored name here, + // marked (ACCOUNT_DELETION.md AD6). The public roster never carries it. + byDeleted: override.actor_user_id == null && Boolean(override.actor_username), + at: override.created_at, + } : null, } }) diff --git a/server/src/model/teams/teamForum.model.js b/server/src/model/teams/teamForum.model.js index daa9692..f2e29be 100644 --- a/server/src/model/teams/teamForum.model.js +++ b/server/src/model/teams/teamForum.model.js @@ -21,6 +21,7 @@ const forumDb = require('./teamForum.db') const forumSettings = require('./teamForumSettings.model') const { cleanForumBody, renderForumBody } = require('../../utils/forumHtml') +const { displayAuthor } = require('../../utils/displayAuthor') // Announcements are leader-authored and take no replies; discussion threads are // member-authored and do. Both have been in the enum since 5a — what phase 5 @@ -42,8 +43,6 @@ const CREATABLE_TYPES_5A = ['announcement'] // look like it would open replies on an announcement. const REPLYABLE_TYPES = ['discussion'] -const DELETED_AUTHOR = '[deleted account]' - /** * Moderation actions, and what each one does to the row. * @@ -74,13 +73,21 @@ const POST_ACTIONS = { restore: { status: 'visible' }, } -function publicThread(row) { +/** + * One thread's list shape, for one viewer. + * + * `author` follows displayAuthor's rule: a deleted account's snapshot reads + * "[deleted player]" to a member and its stored name to staff, and + * `authorDeleted` says which case it is so the staff view can mark it. + */ +function publicThread(row, viewer = null) { + const author = displayAuthor({ userId: row.created_by, username: row.created_username }, viewer) return { id: row.id, type: row.type, title: row.title, - author: row.created_username || DELETED_AUTHOR, - authorDeleted: row.created_by == null, + author: author.name, + authorDeleted: author.deleted, createdAt: row.created_at, lastPostAt: row.last_post_at, postCount: row.post_count, @@ -129,10 +136,11 @@ function editability(row, { userId = null, isStaff = false, windowMinutes = 0, n * said who is reading. */ function renderPost(row, mode, viewer) { + const author = displayAuthor({ userId: row.author_user_id, username: row.author_username }, viewer) return { id: row.id, - author: row.author_username || DELETED_AUTHOR, - authorDeleted: row.author_user_id == null, + author: author.name, + authorDeleted: author.deleted, body: renderForumBody(row.body_html, mode), createdAt: row.created_at, editedAt: row.edited_at, @@ -150,9 +158,9 @@ function renderPost(row, mode, viewer) { * same call answers both audiences without a second endpoint that could disagree * with this one. */ -async function listThreads(teamId, { canModerate = false, limit = 50, offset = 0 } = {}) { +async function listThreads(teamId, { canModerate = false, viewer = null, limit = 50, offset = 0 } = {}) { const rows = await forumDb.threadsByTeam(teamId, { includeHidden: canModerate, limit, offset }) - return rows.map(publicThread) + return rows.map((row) => publicThread(row, viewer)) } /** @@ -176,7 +184,7 @@ async function getThread(teamId, threadId, { canModerate = false, viewer } = {}) const mode = await forumSettings.imageMode() const posts = await forumDb.postsByThread(threadId, { includeHidden: canModerate }) return { - ...publicThread(thread), + ...publicThread(thread, viewer), // A reply control is offered when the TYPE takes replies and the thread is // open. Both halves are reported separately (`type`, `locked`) so the UI can // say which one is why, but the decision itself is made here — a client that diff --git a/server/src/model/teams/teamGrants.model.js b/server/src/model/teams/teamGrants.model.js index 8d2f446..f39b820 100644 --- a/server/src/model/teams/teamGrants.model.js +++ b/server/src/model/teams/teamGrants.model.js @@ -26,6 +26,7 @@ const teamsDb = require('./teams.db') const access = require('./teamAccess.model') const usersDb = require('../users/users.db') const settingsDb = require('../settings/settings.db') +const { displayAuthor } = require('../../utils/displayAuthor') // The per-Team ceiling on ACTIVE leader-issued grants. A leader admitting // unlimited arbitrary accounts to a private space on the operator's host is a @@ -136,19 +137,25 @@ async function revoke({ team, actor, userId, reason }) { * The subtraction is the §3.2 "Forum guests" list: someone who is both a member * and a grantee is a member, listed on the roster, and appears here not at all. * Both facts stay true in the ledger; only the presentation picks one. + * + * **A grant whose account was deleted is not listed** (ACCOUNT_DELETION.md AD13). + * Deleting an account clears `user_id` and leaves the row, so the grant would + * otherwise sit here as a guest nobody can be: a grant is checked by user id, and + * there is no id left to match it. The staff grant ledger still shows it, with the + * name. Who GRANTED it follows displayAuthor's rule for `viewer`. */ -async function forumGuests(teamId) { +async function forumGuests(teamId, viewer = null) { const [grants, members] = await Promise.all([ accessDb.activeGrants(teamId), teamsDb.membersByTeam(teamId, { includeDeparted: false }), ]) const memberUserIds = new Set(members.map((m) => m.user_id).filter((id) => id != null)) return grants - .filter((g) => g.user_id == null || !memberUserIds.has(g.user_id)) + .filter((g) => g.user_id != null && !memberUserIds.has(g.user_id)) .map((g) => ({ userId: g.user_id, username: g.username, - grantedBy: g.granted_username, + grantedBy: displayAuthor({ userId: g.granted_by, username: g.granted_username }, viewer).name, grantedAt: g.granted_at, reason: g.reason, })) diff --git a/server/src/model/teams/teamNotify.db.js b/server/src/model/teams/teamNotify.db.js index 32a8f2d..77755f9 100644 --- a/server/src/model/teams/teamNotify.db.js +++ b/server/src/model/teams/teamNotify.db.js @@ -197,7 +197,7 @@ async function teamsWithForumActivitySince(since) { */ async function digestPostsSince(teamId, since, limit = 20) { return query( - `SELECT po.id, po.thread_id, po.body_html, po.created_at, po.author_username, + `SELECT po.id, po.thread_id, po.body_html, po.created_at, po.author_user_id, po.author_username, th.title, th.type FROM team_forum_posts po JOIN team_forum_threads th ON th.id = po.thread_id diff --git a/server/src/model/users/users.db.js b/server/src/model/users/users.db.js index 495c697..40c9b9b 100644 --- a/server/src/model/users/users.db.js +++ b/server/src/model/users/users.db.js @@ -1,4 +1,4 @@ -const { query } = require('../../utils/db') +const { query, transaction } = require('../../utils/db') const PUBLIC_COLS = 'id, username, role, status, email, email_verified, email_pending, totp_enabled, created_at, last_login_at' @@ -92,6 +92,31 @@ async function deleteUser(id) { return query('DELETE FROM users WHERE id = ?', [id]) } +// Delete the account and, when `reserveName` is given, reserve it, as ONE +// transaction (ACCOUNT_DELETION.md §4.1 step 2): a staff name is never free for +// a moment between the two. `INSERT IGNORE` because a name already reserved +// (a second staff account renamed onto it cannot exist, but a hand-inserted row +// can) is the state wanted, not an error. Returns the rows deleted, 0 or 1. +async function deleteUserReserving(id, reserveName = null) { + return transaction(async (conn) => { + if (reserveName) { + await conn.query( + "INSERT IGNORE INTO reserved_usernames (username_norm, reason) VALUES (LOWER(?), 'deleted_staff')", + [reserveName], + ) + } + const result = await conn.query('DELETE FROM users WHERE id = ?', [id]) + return Number(result.affectedRows || 0) + }) +} + +// True when the name can never be taken again (AD9). The column's collation is +// users.username's, so this folds case exactly as the UNIQUE index does. +async function isUsernameReserved(username) { + const rows = await query('SELECT 1 FROM reserved_usernames WHERE username_norm = LOWER(?) LIMIT 1', [username]) + return rows.length > 0 +} + async function countUsers() { const rows = await query('SELECT COUNT(*) AS c FROM users') return Number(rows[0].c) @@ -148,6 +173,8 @@ module.exports = { listUsers, updateUser, deleteUser, + deleteUserReserving, + isUsernameReserved, countUsers, countAdmins, touchLastLogin, diff --git a/server/src/model/users/users.model.js b/server/src/model/users/users.model.js index 01adef8..2cf818b 100644 --- a/server/src/model/users/users.model.js +++ b/server/src/model/users/users.model.js @@ -13,6 +13,7 @@ function sanitize(user) { // password may be omitted/null — an SSO-provisioned player has no password until // they set one (a null hash makes password login impossible, see validatePassword). async function createUser({ username, password, role = 'admin', email = null, status = 'active', emailVerified = false }) { + await refuseReserved(username) const passwordHash = password ? await bcrypt.hash(password, SALT_ROUNDS) : null const id = await usersDb.insertUser({ username, passwordHash, role, email, status, emailVerified }) return sanitize(await usersDb.findById(id)) @@ -63,9 +64,36 @@ function isDuplicateEmail(err) { // and nothing newly falls through to a 500. Only the case we can positively // identify — email — is carved out. function isDuplicateUsername(err) { + if (err && err.code === USERNAME_RESERVED) return true return isDuplicateKeyError(err) && !isDuplicateEmail(err) } +// ── Reserved usernames (ACCOUNT_DELETION.md AD9) ─────────────────────────── +// +// A deleted staff account's name can never be taken again. The refusal is +// raised HERE, in the two writes every username passes through, rather than in +// each of the six routes that create or rename an account: registration, an +// invite's acceptance, SSO provisioning, an admin creating or renaming a user, +// and a self-service change. Each of them already answers a duplicate username, +// and `isDuplicateUsername` above reports a reserved one as exactly that, so a +// reserved name reads as a taken one everywhere — the same 409, the same words, +// and SSO provisioning moves on to its next candidate. +// +// A check-then-write, so not atomic against a deletion committing in between. +// That window is one request wide and needs a staff account deleted at the same +// instant someone registers its name; the UNIQUE index still holds until the +// deletion commits. +const USERNAME_RESERVED = 'USERNAME_RESERVED' + +async function refuseReserved(username) { + if (typeof username !== 'string' || !username) return + if (await usersDb.isUsernameReserved(username.trim())) { + const err = new Error('username is reserved') + err.code = USERNAME_RESERVED + throw err + } +} + // Returns the raw row (incl. hash) — used by login only. async function getRawByUsername(username) { return usersDb.findByUsername(username) @@ -112,7 +140,10 @@ async function list() { async function update(id, { username, password, role, email, status, emailVerified }) { const fields = {} - if (username !== undefined) fields.username = username + if (username !== undefined) { + await refuseReserved(username) + fields.username = username + } if (role !== undefined) fields.role = role if (email !== undefined) fields.email = email if (status !== undefined) fields.status = status @@ -155,6 +186,15 @@ async function remove(id) { return usersDb.deleteUser(id) } +// The deletion's own write: the row goes, and a staff name is reserved in the +// same transaction. Only auth/accountDeletion.js calls this; `remove` above is +// for undoing an account this request itself just created (a lost invite race). +async function removeReserving(id, reserveName = null) { + return usersDb.deleteUserReserving(id, reserveName) +} + +const isUsernameReserved = (username) => usersDb.isUsernameReserved(username) + async function count() { return usersDb.countUsers() } @@ -186,6 +226,9 @@ module.exports = { invalidateSessions, setSessionCutoff, remove, + removeReserving, + isUsernameReserved, + USERNAME_RESERVED, count, countAdmins, recordLogin, diff --git a/server/src/modules/loader.js b/server/src/modules/loader.js index 31a37ad..d41579d 100644 --- a/server/src/modules/loader.js +++ b/server/src/modules/loader.js @@ -523,6 +523,15 @@ function buildApi(record) { // never writes a module's tables. Called from the seed script only, never // by the running server, and only with DEMO_MODE on. onDemoSeed: hook('onDemoSeed'), + // API 1.20.0 (ACCOUNT_DELETION.md §4.1, AD10, AD11). Core telling the module + // an account is about to be deleted, dispatched from auth/accountDeletion.js + // BEFORE the row goes, so the module can still read the person's links and + // undo them through its own unlink — a cascade is not an unlink, and for UO + // the link lives on the shard. It cannot veto: a throw or a hook past core's + // time limit is logged and the deletion goes ahead, because a member's + // deletion request must be honoured. Whatever the module could not finish is + // its own pending work. + onUserDeleting: hook('onUserDeleting'), } } @@ -898,7 +907,12 @@ function load(tierRouters) { tables: new Set(), called: new Set(), hooks: { - onBoot: null, onShutdown: null, onIdentityLinking: null, onIdentityLinked: null, onDemoSeed: null, + onBoot: null, + onShutdown: null, + onIdentityLinking: null, + onIdentityLinked: null, + onDemoSeed: null, + onUserDeleting: null, }, client: null, ctx: null, @@ -1007,7 +1021,7 @@ function refuseSecond(id, dir, holder) { staged: null, tables: new Set(), called: new Set(), - hooks: { onBoot: null, onShutdown: null, onIdentityLinking: null, onIdentityLinked: null }, + hooks: { onBoot: null, onShutdown: null, onIdentityLinking: null, onIdentityLinked: null, onUserDeleting: null }, client: null, ctx: null, state: 'startup_failed', @@ -1186,6 +1200,23 @@ function identityHooks(name) { .map((r) => ({ id: r.id, hook: r.hooks[name] })) } +/** + * The `onUserDeleting` hooks, in registration order (API 1.20.0, + * ACCOUNT_DELETION.md AD10). + * + * `started` only, like the identity hooks above: a disabled module, or one whose + * `onBoot` threw, is not serving and holds no link it could undo now. Empty in a + * process that never loaded modules, so a deletion there is core's alone. + * + * @returns {{id: string, hook: Function}[]} + */ +function userDeletingHooks() { + if (!loaded) return [] + return [...modules.values()] + .filter((r) => r.state === 'started' && r.hooks.onUserDeleting) + .map((r) => ({ id: r.id, hook: r.hooks.onUserDeleting })) +} + /** * Every installed module's `demoLocks`, compiled, for the demo lock in * demo/locks.js (API 1.18.0). @@ -1361,6 +1392,7 @@ module.exports = { bootable, shutdownHooks, identityHooks, + userDeletingHooks, demoLocks, demoSeedHooks, stopHook, diff --git a/server/src/modules/version.js b/server/src/modules/version.js index 6c69e52..479b222 100644 --- a/server/src/modules/version.js +++ b/server/src/modules/version.js @@ -9,6 +9,19 @@ // Deliberately separate from PROTOCOL_VERSION (which versions the shard wire and // has nothing to say about a website module) and from any module's own version. +// 1.20.0 — a module hears that an account is about to be deleted +// (docs/website/ACCOUNT_DELETION.md §4.1, AD10, AD11). One addition, optional: +// • `api.onUserDeleting(async ({ userId, deletedBy }) => {})`, where +// `deletedBy` is 'self' | 'staff'. Called BEFORE the users row goes, while +// the module can still read the person's links, so it unlinks each game link +// through its own unlink and cleans up anything it keeps without a foreign +// key. It cannot veto: a throw, or running past core's ten seconds, is logged +// and the deletion goes ahead. A module that could not finish keeps its own +// record of what is left and retries. +// Additions only, so minor. Module-uo declares `^1.10.0` and registers none +// until its own PR: with no hook, a deletion is core's cascade alone, exactly as +// the admin route's delete was. USER_SAFETY.md's `reportTargets` moves to 1.21.0. +// // 1.19.0 — the public demo refuses a step at SAVE (docs/website/DEMO_MODE.md // §3.4, DM18). An event action may declare `demoRefuses(params)`, returning a // sentence or null. With DEMO_MODE on, core asks it for every step when a @@ -254,6 +267,6 @@ // an admin action a module performs belongs in core's one audit log, the // extension slot needs the user its prefix names, and §2.7 forbids a module // reading core's `APP_BASE_URL` for itself. Additions only, so minor. -const MODULE_API_VERSION = '1.19.0' +const MODULE_API_VERSION = '1.20.0' module.exports = { MODULE_API_VERSION } diff --git a/server/src/router/v1/admin/admin.controller.js b/server/src/router/v1/admin/admin.controller.js index 04bed7e..e509177 100644 --- a/server/src/router/v1/admin/admin.controller.js +++ b/server/src/router/v1/admin/admin.controller.js @@ -7,6 +7,8 @@ const users = require('../../../model/users/users.model') const activity = require('../../../model/activity/activity.model') const trustedDevices = require('../../../model/trustedDevices/trustedDevices.model') const recoveryCodes = require('../../../model/recoveryCodes/recoveryCodes.model') +const accountDeletion = require('../../../auth/accountDeletion') +const { refuse: demoRefuse } = require('../../../demo/locks') const registries = require('../../../modules/registries') const announceJobs = require('../../../model/announceJobs/announceJobs.model') const emailConfig = require('../../../model/emailConfig/emailConfig.model') @@ -1028,19 +1030,21 @@ async function updateUser(req, res) { } } +// The same procedure a member's own deletion runs (auth/accountDeletion.js): +// modules unlink their game links first, a staff name is reserved, and the +// account's owner is mailed. The refusals keep this route's 400s. async function deleteUser(req, res) { const id = Number(req.params.id) try { if (id === req.user.id) { return res.status(400).json({ message: 'You cannot delete your own account' }) } - const target = await users.getById(id) - if (!target) return res.status(404).json({ message: 'Not found' }) - if (target.role === 'admin' && (await users.countAdmins()) <= 1) { - return res.status(400).json({ message: 'Cannot delete the last admin' }) + const result = await accountDeletion.deleteAccount(id, { by: 'staff', req }) + if (!result.ok) { + if (result.reason === 'not_found') return res.status(404).json({ message: 'Not found' }) + if (result.reason === 'last_admin') return res.status(400).json({ message: 'Cannot delete the last admin' }) + if (result.reason === 'demo') return demoRefuse(res) } - await users.remove(id) - await activity.log({ req, action: 'user.delete', detail: { id } }) return res.json({ id }) } catch (err) { log.error('deleteUser', err) diff --git a/server/src/router/v1/admin/moderation.controller.js b/server/src/router/v1/admin/moderation.controller.js index 711556a..b79d9b1 100644 --- a/server/src/router/v1/admin/moderation.controller.js +++ b/server/src/router/v1/admin/moderation.controller.js @@ -318,7 +318,7 @@ async function getContentReports(req, res) { } const teamId = Number(req.query.teamId) || undefined return res.json({ - reports: await contentReports.queue({ status, teamId, limit, offset }), + reports: await contentReports.queue({ status, teamId, limit, offset, viewer: req.user }), openCount: await contentReports.openCount(), }) } catch (err) { diff --git a/server/src/router/v1/auth/account.controller.js b/server/src/router/v1/auth/account.controller.js index ec55202..57716f4 100644 --- a/server/src/router/v1/auth/account.controller.js +++ b/server/src/router/v1/auth/account.controller.js @@ -18,7 +18,9 @@ const trustedDevices = require('../../../model/trustedDevices/trustedDevices.mod const recoveryCodes = require('../../../model/recoveryCodes/recoveryCodes.model') const sessionService = require('../../../auth/session.service') const { establishTrust } = require('./trustDevice.helper') -const { setAuthCookie, setTrustCookie, clearTrustCookie } = require('../../../auth/token') +const { setAuthCookie, setTrustCookie, clearTrustCookie, clearAuthCookie } = require('../../../auth/token') +const accountDeletion = require('../../../auth/accountDeletion') +const { refuse: demoRefuse } = require('../../../demo/locks') const usernamePolicy = require('../../../auth/usernamePolicy') const loginProtection = require('../../../middleware/loginProtection') const botScore = require('../../../middleware/botScore') @@ -538,7 +540,88 @@ async function generateRecoveryCodes(req, res) { } } +// ── Deleting your own account (ACCOUNT_DELETION.md §4.1) ─────────────────── +// +// DELETE /account. The proof is the password, always (AD7): signing in with +// Google, Discord or Steam is a convenience on an account that already has its +// own password. A two-factor code or a recovery code is asked for too when 2FA is +// on, and the typed username guards against a mis-tap on a phone. +// +// **A wrong password or code is 400, not 401** (AD12). Every self-service +// password check here answers 400, and the app's TokenAuthenticator reads a 401 +// on a signed-in call as an expired session: it would spend the refresh token and +// replay the DELETE, making two password attempts out of one typo. +// +// Each refusal carries a `code` the screens switch on; the words are for a +// client that does not know the code. +async function deleteAccount(req, res) { + try { + const raw = await users.getRawById(req.user.id) + if (!raw) return res.status(401).json({ message: 'Unauthorized' }) + + const typed = String(req.body.username || '').trim() + if (typed.toLowerCase() !== String(raw.username).toLowerCase()) { + return res.status(400).json({ code: 'username_mismatch', message: 'Type your username exactly as it is shown.' }) + } + if (!raw.password_hash) { + return res.status(409).json({ + code: 'password_required', + message: 'This account has no password yet. Set one above, then delete the account.', + }) + } + + const passwordOk = await users.validatePassword(raw, req.body.password || '') + if (!passwordOk) { + // Credential-guessing on an open session, like a wrong current password. + loginProtection.recordFailure(req.ip) + botScore.recordLoginFailure(req.ip) + log.warn('deleteAccount wrong password', { id: raw.id, ip: req.ip }) + return res.status(400).json({ code: 'wrong_password', message: 'Your password is incorrect.' }) + } + + if (raw.totp_enabled) { + const code = String(req.body.code || '').trim() + const recovery = String(req.body.recoveryCode || '').trim() + if (!code && !recovery) { + return res.status(400).json({ code: 'code_required', message: 'Enter a code from your authenticator app, or a recovery code.' }) + } + // A recovery code is spent here even though the account is about to go: + // the deletion can still be refused below, and a code that worked once must + // not work twice. + const passed = code ? totp.verifyCode(raw.totp_secret, code) : await recoveryCodes.consumeForUser(raw.id, recovery) + if (!passed) { + loginProtection.recordFailure(req.ip) + botScore.recordLoginFailure(req.ip) + log.warn('deleteAccount wrong second factor', { id: raw.id, ip: req.ip, recovery: Boolean(!code) }) + return res.status(400).json({ code: 'wrong_code', message: 'That code is not valid. Try again.' }) + } + } + + const result = await accountDeletion.deleteAccount(raw.id, { by: 'self', req }) + if (!result.ok) { + if (result.reason === 'last_admin') { + return res.status(409).json({ + code: 'last_admin', + message: 'You are the only admin. Make someone else an admin before you delete your account.', + }) + } + if (result.reason === 'demo') return demoRefuse(res) + return res.status(401).json({ message: 'Unauthorized' }) + } + + // The server has already dropped every session, trusted device and push + // registration with the row. These two cookies are this browser's copies. + clearAuthCookie(req, res) + clearTrustCookie(req, res) + return res.json({ deleted: true }) + } catch (err) { + log.error('deleteAccount', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + module.exports = { + deleteAccount, changeEmail, resendEmailVerification, cancelEmailChange, diff --git a/server/src/router/v1/auth/me.routes.js b/server/src/router/v1/auth/me.routes.js index ed4de8e..1c41fc4 100644 --- a/server/src/router/v1/auth/me.routes.js +++ b/server/src/router/v1/auth/me.routes.js @@ -79,6 +79,33 @@ meRouter.patch( account.changePassword, ) +// Deleting your own account (ACCOUNT_DELETION.md §4.1). Behind the login backoff +// and bot guards as well as the change limiter, because a wrong password here is +// credential-guessing exactly as it is on the recovery-codes route. +meRouter.delete( + '/account', + // #swagger.tags = ['Auth · Me'] + // #swagger.summary = 'Delete the current account (self, any role)' + // #swagger.description = 'Deletes the account at once (no grace period). Requires the typed username (case-insensitive), the account password, and — when two-factor sign-in is on — an authenticator code or a recovery code. Before the row goes, every installed game module unlinks the account’s game links through its own unlink (MODULE_API 1.20.0 onUserDeleting); a module that cannot finish never stops the deletion. Every session, trusted device, push registration, linked sign-in and game link goes with the account. What the member wrote stays and other members see it as "[deleted player]"; staff keep the name and the activity log for moderation. A deleted staff account’s username can never be registered again. A wrong password or code is a 400 (not 401), so a native client’s refresh-on-401 never replays it. Refused with 403 DEMO_LOCKED for the demo’s shared logins.' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.requestBody = { required: true, content: { "application/json": { schema: { $ref: "#/components/schemas/DeleteAccountRequest" } } } } */ + /* #swagger.responses[200] = { description: 'Deleted; this browser’s session and trust cookies are cleared', content: { "application/json": { schema: { type: "object", properties: { deleted: { type: "boolean", example: true } } } } } } */ + /* #swagger.responses[400] = { description: 'username_mismatch, wrong_password, code_required or wrong_code', content: { "application/json": { schema: { $ref: "#/components/schemas/DeleteAccountRefusal" } } } } */ + /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + /* #swagger.responses[403] = { description: 'Demo login (DEMO_LOCKED)', content: { "application/json": { schema: { $ref: "#/components/schemas/DemoLocked" } } } } */ + /* #swagger.responses[409] = { description: 'password_required (the account has no password yet) or last_admin', content: { "application/json": { schema: { $ref: "#/components/schemas/DeleteAccountRefusal" } } } } */ + /* #swagger.responses[429] = { description: 'Too many attempts (rate limited)', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + backoffGuard, + slowLogin, + accountChangeLimiter, + body('username').isString().trim().isLength({ min: 1, max: 64 }), + body('password').optional({ values: 'falsy' }).isString().isLength({ max: 128 }), + body('code').optional({ values: 'falsy' }).isString().trim().isLength({ max: 16 }), + body('recoveryCode').optional({ values: 'falsy' }).isString().trim().isLength({ max: 64 }), + validate, + account.deleteAccount, +) + // Email address (engagement Phase 1b). The change is STAGED and only a tokened // link installs it, so these three routes never alter the address that is // currently receiving mail. The confirm half is public and lives at diff --git a/server/src/router/v1/player/teamForum.controller.js b/server/src/router/v1/player/teamForum.controller.js index ac3250a..955f0e6 100644 --- a/server/src/router/v1/player/teamForum.controller.js +++ b/server/src/router/v1/player/teamForum.controller.js @@ -82,6 +82,9 @@ async function resolveForum(req) { async function viewerFor(ctx, user) { return { userId: user.id, + // The account role, for whose name a deleted author shows as (displayAuthor): + // an editor is not forum staff and still sees it (AD6). + role: user.role, isStaff: ctx.staff, windowMinutes: await forumSettings.editWindowMinutes(), } @@ -122,7 +125,7 @@ async function listThreads(req, res) { const ctx = await resolveForum(req) if (!ctx) return res.status(404).json({ message: 'Not found' }) return res.json({ - threads: await forum.listThreads(ctx.team.id, { canModerate: ctx.canModerate }), + threads: await forum.listThreads(ctx.team.id, { canModerate: ctx.canModerate, viewer: { role: req.user.role } }), // Two capabilities, not one. Phase 4 had a single `canPost` because there // was a single kind of thread to post; phase 5 opened discussion to every // participant while announcements stayed with the leaders, so a client that @@ -334,7 +337,7 @@ async function listGrants(req, res) { const authority = await grants.authorityFor(team.id, req.user) if (!authority.may) return res.status(403).json({ message: 'Not a leader of this Team' }) return res.json({ - guests: await grants.forumGuests(team.id), + guests: await grants.forumGuests(team.id, req.user), cap: await grants.grantCap(), as: authority.as, }) diff --git a/server/src/utils/db.js b/server/src/utils/db.js index 755aba3..a53fb2f 100644 --- a/server/src/utils/db.js +++ b/server/src/utils/db.js @@ -40,6 +40,37 @@ async function query(sql, params) { } } +/** + * Run `fn(conn)` inside one transaction on one connection: committed when it + * resolves, rolled back when it throws (and the error rethrown). + * + * Every statement that must stand or fall together goes through the `conn` it is + * handed, never through `query()` above, which takes a different connection from + * the pool and so sits outside the transaction. + * + * @template T + * @param {(conn: import('mariadb').PoolConnection) => Promise} fn + * @returns {Promise} + */ +async function transaction(fn) { + const conn = await pool.getConnection() + try { + await conn.beginTransaction() + const out = await fn(conn) + await conn.commit() + return out + } catch (err) { + try { + await conn.rollback() + } catch (rollbackErr) { + log.error('rollback failed', { message: rollbackErr.message }) + } + throw err + } finally { + conn.release() + } +} + const SCHEMA_PATH = path.join(__dirname, '..', '..', 'db', 'schema.sql') /** @@ -106,4 +137,4 @@ async function close() { await pool.end() } -module.exports = { pool, query, ensureSchema, close } +module.exports = { pool, query, transaction, ensureSchema, close } diff --git a/server/src/utils/displayAuthor.js b/server/src/utils/displayAuthor.js new file mode 100644 index 0000000..fcb40e7 --- /dev/null +++ b/server/src/utils/displayAuthor.js @@ -0,0 +1,52 @@ +// ── Who a username snapshot shows as, to whom (ACCOUNT_DELETION.md AD5, AD6) ── +// +// Several tables keep a username next to a user id, on purpose, so a record still +// names someone after the account is gone (§3 gap 2): forum threads, posts and +// uploads, content reports, the forum's moderation ledger, forum grants, leader +// overrides and Team moderation requests. Deleting an account clears the id +// (ON DELETE SET NULL) and leaves the snapshot alone, and nothing else clears +// those ids. So **"id NULL, snapshot present" means a deleted account.** +// +// What a viewer sees then is a rule about the VIEWER, not a change to the row: +// +// • members, Team leaders included, and signed-out visitors see +// "[deleted player]" for every snapshot of a deleted account (AD5); +// • staff — admin, editor, moderator — see the stored name, marked as a +// deleted account, for moderation (AD6). +// +// One function, so the rule cannot drift between the eight places that render a +// snapshot. A caller with a pair that is legitimately empty (a report nobody has +// handled yet) asks `hasActor` first: an absent actor is not a deleted one. + +const DELETED_PLAYER = '[deleted player]' + +// Every staff role, which is wider than the forum's own "staff" (admin and +// moderator, who may moderate there). An editor cannot moderate a forum and is +// still staff for the question this file answers: may they see who it was. +const STAFF_ROLES = Object.freeze(['admin', 'editor', 'moderator']) + +/** True when this viewer sees the stored name of a deleted account. */ +function seesDeletedNames(viewer) { + return Boolean(viewer && STAFF_ROLES.includes(viewer.role)) +} + +/** True when the pair names anybody at all, deleted or not. */ +function hasActor(userId, username) { + return userId != null || (username != null && username !== '') +} + +/** + * One snapshot, shown to one viewer. + * + * @param {{userId: number|null|undefined, username: string|null|undefined}} who + * @param {{role?: string}|null|undefined} viewer the reader; null for a visitor + * @returns {{name: string, deleted: boolean}} + */ +function displayAuthor({ userId, username }, viewer) { + const deleted = userId == null + if (!deleted) return { name: username || DELETED_PLAYER, deleted: false } + if (username && seesDeletedNames(viewer)) return { name: username, deleted: true } + return { name: DELETED_PLAYER, deleted: true } +} + +module.exports = { DELETED_PLAYER, STAFF_ROLES, seesDeletedNames, hasActor, displayAuthor } diff --git a/server/src/utils/mailer.js b/server/src/utils/mailer.js index 523ec5c..ac1f871 100644 --- a/server/src/utils/mailer.js +++ b/server/src/utils/mailer.js @@ -15,7 +15,7 @@ // it says is an `engagement_templates` row rendered by `engagement/templates.js` // (§4.6.1), which an operator can edit and which falls back to the shipped seed. // -// **The failure contracts are the point of this file.** Six call sites, five +// **The failure contracts are the point of this file.** Seven call sites, six // senders, and each one degrades a specific way when mail is unconfigured. Those // contracts are unchanged by the transport rewrite and are asserted in // test/mailer.test.js: sendContactMessage returns a mailto fallback, sendInvite @@ -344,6 +344,41 @@ async function sendEmailVerification({ to, verifyUrl, username }) { } } +/** + * Tell an address its account was deleted (ACCOUNT_DELETION.md §4.1 step 3). + * + * Sent after the row is gone, to the address read before it went, so nothing + * here can reach the account. `by` decides one phrase: a member deleting their + * own account reads "at your request", a staff deletion "by the site's staff". + * + * Returns { sent: false, reason: 'NOT_CONFIGURED' } when mail is unconfigured + * and throws on a send failure, like the senders above; the deletion's caller + * has already finished and only logs either. + */ +async function sendAccountDeleted({ to, username, deletedOn, by }) { + const built = await buildTransport() + if (!built) return { sent: false, reason: 'NOT_CONFIGURED' } + const { transport, config } = built + const byWhom = by === 'staff' ? ' by the site’s staff' : ' at your request' + const { subject, html, text } = await body('auth.account-deleted', { username, deletedOn, byWhom }) + try { + await transport.sendMail({ + from: fromHeader(config), + to, + replyTo: replyToFor(config), + subject, + text, + html, + }) + await emailConfig.recordStatus({ status: 'connected', statusDetail: 'Account deletion notice send OK', lastVerifiedAt: new Date() }) + return { sent: true } + } catch (err) { + log.error('account deletion notice send failed', err) + await emailConfig.recordStatus({ status: 'error', statusDetail: describeSendError(err, config) }) + throw err + } +} + /** * Send an ALREADY-RENDERED body to one address — the template editor's test send * (§4.6.2, Phase 5b). @@ -481,5 +516,6 @@ module.exports = { sendInvite, sendPasswordReset, sendEmailVerification, + sendAccountDeleted, PERMANENT_CODES, } diff --git a/server/src/utils/teamDigestWorker.js b/server/src/utils/teamDigestWorker.js index 28cf176..43f88b5 100644 --- a/server/src/utils/teamDigestWorker.js +++ b/server/src/utils/teamDigestWorker.js @@ -42,6 +42,7 @@ const templates = require('../engagement/templates') const emailChannel = require('../engagement/emailChannel') const mailer = require('./mailer') const notify = require('./teamNotify') +const { DELETED_PLAYER } = require('./displayAuthor') const log = require('./logger')('team-digest') const INTERVAL_MS = Number(process.env.TEAM_DIGEST_INTERVAL_MS) || 24 * 60 * 60 * 1000 @@ -127,7 +128,10 @@ async function sendOne(team, recipient, rule, now) { periodLabel: `${label}: ${posts.length} new post${posts.length === 1 ? '' : 's'}`, intro: `Since your last digest, ${posts.length} new post${posts.length === 1 ? '' : 's'} in ${label}:`, items: posts.map((p) => ({ - heading: `${p.title} — ${p.author_username || 'someone'}`, + // A digest goes to members, who see a deleted author as "[deleted player]" + // (ACCOUNT_DELETION.md AD5). The post outlives its author on purpose; the + // name in it does not reach a member's inbox. + heading: `${p.title} — ${p.author_user_id === null ? DELETED_PLAYER : p.author_username || 'someone'}`, excerpt: notify.excerpt(p.body_html), url: notify.threadPath(team, p.thread_id), })), diff --git a/server/swagger/swagger-output.json b/server/swagger/swagger-output.json index 88c14eb..778d7f6 100644 --- a/server/swagger/swagger-output.json +++ b/server/swagger/swagger-output.json @@ -12861,6 +12861,102 @@ "bearerAuth": [] } ] + }, + "delete": { + "tags": [ + "Auth · Me" + ], + "summary": "Delete the current account (self, any role)", + "description": "Deletes the account at once (no grace period). Requires the typed username (case-insensitive), the account password, and — when two-factor sign-in is on — an authenticator code or a recovery code. Before the row goes, every installed game module unlinks the account’s game links through its own unlink (MODULE_API 1.20.0 onUserDeleting); a module that cannot finish never stops the deletion. Every session, trusted device, push registration, linked sign-in and game link goes with the account. What the member wrote stays and other members see it as \"[deleted player]\"; staff keep the name and the activity log for moderation. A deleted staff account’s username can never be registered again. A wrong password or code is a 400 (not 401), so a native client’s refresh-on-401 never replays it. Refused with 403 DEMO_LOCKED for the demo’s shared logins.", + "responses": { + "200": { + "description": "Deleted; this browser’s session and trust cookies are cleared", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "deleted": { + "type": "boolean", + "example": true + } + } + } + } + } + }, + "400": { + "description": "username_mismatch, wrong_password, code_required or wrong_code", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DeleteAccountRefusal" + } + } + } + }, + "401": { + "description": "Not authenticated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Demo login (DEMO_LOCKED)", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DemoLocked" + } + } + } + }, + "409": { + "description": "password_required (the account has no password yet) or last_admin", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DeleteAccountRefusal" + } + } + } + }, + "429": { + "description": "Too many attempts (rate limited)", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DeleteAccountRequest" + } + } + } + } } }, "/api/v1/auth/me/account/email": { @@ -21590,6 +21686,148 @@ } } }, + "DeleteAccountRequest": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "object" + }, + "required": { + "type": "array", + "example": [ + "username", + "password" + ], + "items": { + "type": "string" + } + }, + "description": { + "type": "string", + "example": "The confirm step of deleting your own account (ACCOUNT_DELETION.md §4.1). The password is always required (AD7); `code` or `recoveryCode` only when two-factor sign-in is on." + }, + "properties": { + "type": "object", + "properties": { + "username": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "example": { + "type": "string", + "example": "wanderer" + }, + "description": { + "type": "string", + "example": "The account's username, typed to confirm; compared ignoring case." + } + } + }, + "password": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "format": { + "type": "string", + "example": "password" + } + } + }, + "code": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "example": { + "type": "string", + "example": "123456" + }, + "description": { + "type": "string", + "example": "A current authenticator code." + } + } + }, + "recoveryCode": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "example": { + "type": "string", + "example": "ABCD-EFGH" + }, + "description": { + "type": "string", + "example": "A single-use recovery code, instead of `code`." + } + } + } + } + } + } + }, + "DeleteAccountRefusal": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "object" + }, + "properties": { + "type": "object", + "properties": { + "code": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "enum": { + "type": "array", + "example": [ + "username_mismatch", + "wrong_password", + "code_required", + "wrong_code", + "password_required", + "last_admin" + ], + "items": { + "type": "string" + } + } + } + }, + "message": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "example": { + "type": "string", + "example": "Your password is incorrect." + } + } + } + } + } + } + }, "ChangePasswordRequest": { "type": "object", "properties": { @@ -23811,7 +24049,7 @@ }, "description": { "type": "string", - "example": "Username snapshot; \"[deleted account]\" once the account is gone." + "example": "Username snapshot. Staff see it even after the account is deleted, with `reporterDeleted` true; anyone else would see \"[deleted player]\" (ACCOUNT_DELETION.md AD5, AD6)." } } }, @@ -23918,6 +24156,19 @@ } } }, + "handledByDeleted": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "boolean" + }, + "example": { + "type": "boolean", + "example": false + } + } + }, "handledNote": { "type": "object", "properties": { @@ -24124,6 +24375,23 @@ } } }, + "authorDeleted": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "boolean" + }, + "nullable": { + "type": "boolean", + "example": true + }, + "description": { + "type": "string", + "example": "The author's account was deleted; `author` is the stored name, shown to staff (ACCOUNT_DELETION.md AD6)." + } + } + }, "uploader": { "type": "object", "properties": { @@ -24137,6 +24405,19 @@ } } }, + "uploaderDeleted": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "boolean" + }, + "nullable": { + "type": "boolean", + "example": true + } + } + }, "excerpt": { "type": "object", "properties": { diff --git a/server/swagger/swagger.js b/server/swagger/swagger.js index c96d9bc..0191281 100644 --- a/server/swagger/swagger.js +++ b/server/swagger/swagger.js @@ -493,6 +493,27 @@ const doc = { username: { type: 'string', minLength: 3, maxLength: 32, example: 'newname' }, }, }, + DeleteAccountRequest: { + type: 'object', + required: ['username', 'password'], + description: 'The confirm step of deleting your own account (ACCOUNT_DELETION.md §4.1). The password is always required (AD7); `code` or `recoveryCode` only when two-factor sign-in is on.', + properties: { + username: { type: 'string', example: 'wanderer', description: 'The account\'s username, typed to confirm; compared ignoring case.' }, + password: { type: 'string', format: 'password' }, + code: { type: 'string', example: '123456', description: 'A current authenticator code.' }, + recoveryCode: { type: 'string', example: 'ABCD-EFGH', description: 'A single-use recovery code, instead of `code`.' }, + }, + }, + DeleteAccountRefusal: { + type: 'object', + properties: { + code: { + type: 'string', + enum: ['username_mismatch', 'wrong_password', 'code_required', 'wrong_code', 'password_required', 'last_admin'], + }, + message: { type: 'string', example: 'Your password is incorrect.' }, + }, + }, ChangePasswordRequest: { type: 'object', required: ['newPassword'], @@ -844,12 +865,13 @@ const doc = { targetType: { type: 'string', example: 'team_forum_post', description: 'team_forum_thread | team_forum_post | team_forum_upload' }, targetId: { type: 'integer', example: 812 }, teamId: { type: 'integer', nullable: true, example: 7, description: 'Denormalised so the queue can filter by Team.' }, - reporter: { type: 'string', example: 'wanderer', description: 'Username snapshot; "[deleted account]" once the account is gone.' }, + reporter: { type: 'string', example: 'wanderer', description: 'Username snapshot. Staff see it even after the account is deleted, with `reporterDeleted` true; anyone else would see "[deleted player]" (ACCOUNT_DELETION.md AD5, AD6).' }, reporterDeleted: { type: 'boolean', example: false }, reason: { type: 'string', enum: ['spam', 'abuse', 'sexual', 'illegal', 'impersonation', 'other'], example: 'abuse' }, detail: { type: 'string', nullable: true, maxLength: 500, example: 'Personal attacks in the third paragraph.' }, status: { type: 'string', enum: ['open', 'reviewing', 'actioned', 'dismissed'], example: 'open' }, handledBy: { type: 'string', nullable: true, example: 'moderator1' }, + handledByDeleted: { type: 'boolean', example: false }, handledNote: { type: 'string', nullable: true, example: 'Post hidden, author warned.' }, handledAt: { type: 'string', format: 'date-time', nullable: true }, createdAt: { type: 'string', format: 'date-time' }, @@ -869,7 +891,9 @@ const doc = { title: { type: 'string', nullable: true }, type: { type: 'string', nullable: true, enum: ['announcement', 'discussion'] }, author: { type: 'string', nullable: true, example: 'someone' }, + authorDeleted: { type: 'boolean', nullable: true, description: 'The author\'s account was deleted; `author` is the stored name, shown to staff (ACCOUNT_DELETION.md AD6).' }, uploader: { type: 'string', nullable: true }, + uploaderDeleted: { type: 'boolean', nullable: true }, excerpt: { type: 'string', nullable: true, description: 'Plain-text excerpt of the post body, capped at 300 characters.' }, status: { type: 'string', nullable: true, enum: ['visible', 'hidden', 'deleted'] }, filename: { type: 'string', nullable: true }, diff --git a/server/test/accountDeletion.test.js b/server/test/accountDeletion.test.js new file mode 100644 index 0000000..c73babc --- /dev/null +++ b/server/test/accountDeletion.test.js @@ -0,0 +1,389 @@ +// ── Deleting an account (docs/website/ACCOUNT_DELETION.md §4.1) ──────────── +// +// The service's procedure, the member's route and its refusals, the admin +// route going through the same procedure, and reserved staff usernames. Every +// model call is stubbed; the DB pool points at a dead port (test/_setup.js). +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const { test, beforeEach, afterEach, after } = require('node:test') +const assert = require('node:assert/strict') + +const users = require('../src/model/users/users.model') +const usersDb = require('../src/model/users/users.db') +const activity = require('../src/model/activity/activity.model') +const teamSync = require('../src/model/teams/teamSync.model') +const recoveryCodes = require('../src/model/recoveryCodes/recoveryCodes.model') +const loader = require('../src/modules/loader') +const mailer = require('../src/utils/mailer') +const totp = require('../src/utils/totp') +const loginProtection = require('../src/middleware/loginProtection') +const botScore = require('../src/middleware/botScore') +const accountDeletion = require('../src/auth/accountDeletion') +const account = require('../src/router/v1/auth/account.controller') +const admin = require('../src/router/v1/admin/admin.controller') +const db = require('../src/utils/db') + +after(() => db.close()) + +// ── stubbing ─────────────────────────────────────────────────────────────── + +const saved = [] +function patch(obj, key, value) { + saved.push([obj, key, obj[key]]) + obj[key] = value +} +afterEach(() => { + while (saved.length) { + const [obj, key, value] = saved.pop() + obj[key] = value + } + delete process.env.DEMO_MODE +}) + +// What the procedure did, in order. +let events +let logged +let mailed +const ACCOUNTS = { + 7: { id: 7, username: 'Thorne', role: 'player', email: 'thorne@example.com' }, + 8: { id: 8, username: 'Ravenna', role: 'moderator', email: null }, + 9: { id: 9, username: 'boss', role: 'admin', email: 'boss@example.com' }, + 3: { id: 3, username: 'demo-player', role: 'player', email: null }, +} + +beforeEach(() => { + events = [] + logged = [] + mailed = [] + patch(users, 'getById', async (id) => (ACCOUNTS[id] ? { ...ACCOUNTS[id] } : null)) + patch(users, 'countAdmins', async () => 2) + patch(users, 'removeReserving', async (id, name) => { + events.push(['delete', id, name]) + return ACCOUNTS[id] ? 1 : 0 + }) + patch(loader, 'userDeletingHooks', () => []) + patch(teamSync, 'request', (opts) => events.push(['teams', opts.reason])) + patch(activity, 'log', async (entry) => { logged.push(entry) }) + patch(mailer, 'sendAccountDeleted', async (args) => { + mailed.push(args) + return { sent: true } + }) +}) + +function mockRes() { + return { + statusCode: 200, + body: null, + cleared: [], + status(c) { this.statusCode = c; return this }, + json(b) { this.body = b; return this }, + clearCookie(name) { this.cleared.push(name); return this }, + set() { return this }, + } +} + +// ── the service ──────────────────────────────────────────────────────────── + +test('modules are asked BEFORE the row goes, with who deleted it (AD10)', async () => { + patch(loader, 'userDeletingHooks', () => [ + { id: 'rust', hook: async (payload) => { events.push(['hook', payload]) } }, + ]) + const result = await accountDeletion.deleteAccount(7, { by: 'self', req: { ip: '10.0.0.1' } }) + await result.settled + assert.equal(result.ok, true) + assert.deepEqual(events.slice(0, 2), [ + ['hook', { userId: 7, deletedBy: 'self' }], + ['delete', 7, null], + ]) +}) + +test('the hook payload is frozen, so a module cannot rewrite what the next one hears', async () => { + let seen + patch(loader, 'userDeletingHooks', () => [{ id: 'rust', hook: async (p) => { seen = p } }]) + await (await accountDeletion.deleteAccount(7, { by: 'staff' })).settled + assert.equal(Object.isFrozen(seen), true) + assert.deepEqual(seen, { userId: 7, deletedBy: 'staff' }) +}) + +test('a module hook that throws never stops the deletion (AD11)', async () => { + patch(loader, 'userDeletingHooks', () => [ + { id: 'uo', hook: async () => { throw new Error('sidecar down') } }, + ]) + const result = await accountDeletion.deleteAccount(7, { by: 'self' }) + await result.settled + assert.equal(result.ok, true) + assert.deepEqual(result.modules, [{ module: 'uo', ok: false, error: 'sidecar down' }]) + assert.deepEqual(events.find((e) => e[0] === 'delete'), ['delete', 7, null]) +}) + +test('a module hook that hangs is cut off at the time limit and the deletion goes ahead', async () => { + patch(loader, 'userDeletingHooks', () => [{ id: 'uo', hook: () => new Promise(() => {}) }]) + const result = await accountDeletion.deleteAccount(7, { by: 'self', hookTimeoutMs: 20 }) + await result.settled + assert.equal(result.ok, true) + assert.equal(result.modules[0].ok, false) + assert.match(result.modules[0].error, /no answer within 20 ms/) + assert.equal(accountDeletion.HOOK_TIMEOUT_MS, 10_000) +}) + +test('a staff account’s username is reserved in the deletion’s transaction; a player’s is not (AD9)', async () => { + const staff = await accountDeletion.deleteAccount(8, { by: 'staff', req: { user: { id: 9 } } }) + await staff.settled + assert.equal(staff.reserved, true) + const player = await accountDeletion.deleteAccount(7, { by: 'self' }) + await player.settled + assert.equal(player.reserved, false) + assert.deepEqual(events.filter((e) => e[0] === 'delete'), [['delete', 8, 'Ravenna'], ['delete', 7, null]]) +}) + +test('the last admin is refused before any module is asked', async () => { + let asked = false + patch(users, 'countAdmins', async () => 1) + patch(loader, 'userDeletingHooks', () => [{ id: 'rust', hook: async () => { asked = true } }]) + const result = await accountDeletion.deleteAccount(9, { by: 'self' }) + assert.deepEqual(result, { ok: false, reason: 'last_admin' }) + assert.equal(asked, false) + assert.equal(events.length, 0) +}) + +test('the demo’s shared logins are refused by the service too, behind the demo lock', async () => { + process.env.DEMO_MODE = 'true' + assert.deepEqual(await accountDeletion.deleteAccount(3, { by: 'staff' }), { ok: false, reason: 'demo' }) + assert.equal(events.length, 0) + // Off the demo the same username is just a name. + delete process.env.DEMO_MODE + assert.equal((await accountDeletion.deleteAccount(3, { by: 'staff' })).ok, true) +}) + +test('an account that is already gone is not_found and nothing runs after it', async () => { + assert.deepEqual(await accountDeletion.deleteAccount(404, { by: 'self' }), { ok: false, reason: 'not_found' }) + // Gone between the read and the delete: somebody else's deletion won. + patch(users, 'removeReserving', async () => 0) + assert.deepEqual(await accountDeletion.deleteAccount(7, { by: 'self' }), { ok: false, reason: 'not_found' }) + assert.equal(logged.length, 0) + assert.equal(mailed.length, 0) +}) + +test('after commit: a Teams reconcile, an activity entry with no username, and the notice', async () => { + const result = await accountDeletion.deleteAccount(7, { by: 'self', req: { ip: '10.0.0.1', user: { id: 7 } } }) + await result.settled + assert.deepEqual(events.find((e) => e[0] === 'teams'), ['teams', 'account deleted']) + + // The member is gone, so the entry carries no user id — only the IP (AD6). + assert.equal(logged.length, 1) + assert.equal(logged[0].action, 'user.self_delete') + assert.deepEqual(logged[0].detail, { id: 7 }) + assert.equal(logged[0].req.user, undefined) + assert.equal(logged[0].req.ip, '10.0.0.1') + assert.ok(!JSON.stringify(logged[0]).includes('Thorne'), 'the username is never logged') + + assert.equal(mailed.length, 1) + assert.equal(mailed[0].to, 'thorne@example.com') + assert.equal(mailed[0].username, 'Thorne') + assert.equal(mailed[0].by, 'self') + assert.match(mailed[0].deletedOn, /^\d{1,2} [A-Z][a-z]+ \d{4}$/) +}) + +test('a staff deletion is logged as user.delete against the acting admin', async () => { + const req = { ip: '10.0.0.2', user: { id: 9 } } + await (await accountDeletion.deleteAccount(8, { by: 'staff', req })).settled + assert.equal(logged[0].action, 'user.delete') + assert.equal(logged[0].req, req) + assert.deepEqual(logged[0].detail, { id: 8, usernameReserved: true }) + // No address on file, so no notice. + assert.equal(mailed.length, 0) +}) + +test('a failing mail or audit step after commit never rejects', async () => { + patch(mailer, 'sendAccountDeleted', async () => { throw new Error('smtp down') }) + patch(activity, 'log', async () => { throw new Error('db gone') }) + patch(teamSync, 'request', () => { throw new Error('sync broken') }) + const result = await accountDeletion.deleteAccount(7, { by: 'self' }) + await result.settled // resolves + assert.equal(result.ok, true) +}) + +test('by must say who deleted it', async () => { + await assert.rejects(() => accountDeletion.deleteAccount(7, {}), /by must be one of/) +}) + +// ── DELETE /auth/me/account ──────────────────────────────────────────────── + +const RAW = { + id: 7, username: 'Thorne', role: 'player', password_hash: '$2a$hash', totp_enabled: 0, totp_secret: null, +} +let failures +let deleted +function stubSelf(raw = RAW, { passwordOk = true } = {}) { + failures = 0 + deleted = null + patch(users, 'getRawById', async () => (raw ? { ...raw } : null)) + patch(users, 'validatePassword', async () => passwordOk) + patch(loginProtection, 'recordFailure', () => { failures += 1 }) + patch(botScore, 'recordLoginFailure', () => {}) + patch(accountDeletion, 'deleteAccount', async (id, opts) => { + deleted = { id, by: opts.by } + return { ok: true, id, settled: Promise.resolve() } + }) +} +const selfReq = (body) => ({ body, user: { id: 7, username: 'Thorne' }, ip: '10.0.0.1', headers: {}, cookies: {} }) + +test('the typed username must match, ignoring case, before the password is even checked', async () => { + let checked = false + stubSelf() + patch(users, 'validatePassword', async () => { checked = true; return true }) + const res = mockRes() + await account.deleteAccount(selfReq({ username: 'Thorn', password: 'pw' }), res) + assert.equal(res.statusCode, 400) + assert.equal(res.body.code, 'username_mismatch') + assert.equal(checked, false) + assert.equal(deleted, null) + + const ok = mockRes() + await account.deleteAccount(selfReq({ username: ' thorne ', password: 'pw' }), ok) + assert.equal(ok.statusCode, 200) + assert.deepEqual(ok.body, { deleted: true }) + assert.deepEqual(deleted, { id: 7, by: 'self' }) +}) + +test('an account with no password is told to set one first (AD7, 409 password_required)', async () => { + stubSelf({ ...RAW, password_hash: null }) + const res = mockRes() + await account.deleteAccount(selfReq({ username: 'Thorne' }), res) + assert.equal(res.statusCode, 409) + assert.equal(res.body.code, 'password_required') + assert.equal(deleted, null) +}) + +test('a wrong password is a 400, never a 401, and counts as a failed login (AD12)', async () => { + stubSelf(RAW, { passwordOk: false }) + const res = mockRes() + await account.deleteAccount(selfReq({ username: 'Thorne', password: 'nope' }), res) + assert.equal(res.statusCode, 400) + assert.equal(res.body.code, 'wrong_password') + assert.equal(failures, 1) + assert.equal(deleted, null) +}) + +test('with two-factor on, a code is required, and a wrong one is a 400', async () => { + stubSelf({ ...RAW, totp_enabled: 1, totp_secret: 'SECRET' }) + patch(totp, 'verifyCode', (secret, code) => secret === 'SECRET' && code === '123456') + + const none = mockRes() + await account.deleteAccount(selfReq({ username: 'Thorne', password: 'pw' }), none) + assert.equal(none.statusCode, 400) + assert.equal(none.body.code, 'code_required') + + const wrong = mockRes() + await account.deleteAccount(selfReq({ username: 'Thorne', password: 'pw', code: '000000' }), wrong) + assert.equal(wrong.statusCode, 400) + assert.equal(wrong.body.code, 'wrong_code') + assert.equal(failures, 1) + assert.equal(deleted, null) + + const right = mockRes() + await account.deleteAccount(selfReq({ username: 'Thorne', password: 'pw', code: '123456' }), right) + assert.equal(right.statusCode, 200) + assert.deepEqual(deleted, { id: 7, by: 'self' }) +}) + +test('a recovery code stands in for the authenticator, and is spent', async () => { + stubSelf({ ...RAW, totp_enabled: 1, totp_secret: 'SECRET' }) + const spent = [] + patch(recoveryCodes, 'consumeForUser', async (id, code) => { spent.push([id, code]); return code === 'ABCD-EFGH' }) + const res = mockRes() + await account.deleteAccount(selfReq({ username: 'Thorne', password: 'pw', recoveryCode: 'ABCD-EFGH' }), res) + assert.equal(res.statusCode, 200) + assert.deepEqual(spent, [[7, 'ABCD-EFGH']]) +}) + +test('success clears this browser’s session and trust cookies', async () => { + stubSelf() + const res = mockRes() + await account.deleteAccount(selfReq({ username: 'Thorne', password: 'pw' }), res) + const { COOKIE_NAME, TRUST_COOKIE_NAME } = require('../src/auth/token') + assert.deepEqual(res.cleared.sort(), [COOKIE_NAME, TRUST_COOKIE_NAME].sort()) +}) + +test('the last admin deleting themself is a 409 with a code the screen can explain', async () => { + stubSelf({ ...RAW, role: 'admin' }) + patch(accountDeletion, 'deleteAccount', async () => ({ ok: false, reason: 'last_admin' })) + const res = mockRes() + await account.deleteAccount(selfReq({ username: 'Thorne', password: 'pw' }), res) + assert.equal(res.statusCode, 409) + assert.equal(res.body.code, 'last_admin') +}) + +test('a demo login reaching the handler gets the demo’s own refusal', async () => { + stubSelf() + patch(accountDeletion, 'deleteAccount', async () => ({ ok: false, reason: 'demo' })) + const res = mockRes() + await account.deleteAccount(selfReq({ username: 'Thorne', password: 'pw' }), res) + assert.equal(res.statusCode, 403) + assert.equal(res.body.code, 'DEMO_LOCKED') +}) + +// ── DELETE /admin/users/:id ──────────────────────────────────────────────── + +test('the admin route runs the same procedure, by staff, and keeps its 400s', async () => { + let calledWith = null + patch(accountDeletion, 'deleteAccount', async (id, opts) => { + calledWith = { id, by: opts.by } + return { ok: true, id, settled: Promise.resolve() } + }) + const res = mockRes() + await admin.deleteUser({ params: { id: '8' }, user: { id: 9 }, ip: '10.0.0.2' }, res) + assert.equal(res.statusCode, 200) + assert.deepEqual(res.body, { id: 8 }) + assert.deepEqual(calledWith, { id: 8, by: 'staff' }) + + const self = mockRes() + await admin.deleteUser({ params: { id: '9' }, user: { id: 9 } }, self) + assert.equal(self.statusCode, 400) + + patch(accountDeletion, 'deleteAccount', async () => ({ ok: false, reason: 'last_admin' })) + const last = mockRes() + await admin.deleteUser({ params: { id: '8' }, user: { id: 9 } }, last) + assert.equal(last.statusCode, 400) + assert.match(last.body.message, /last admin/) + + patch(accountDeletion, 'deleteAccount', async () => ({ ok: false, reason: 'not_found' })) + const missing = mockRes() + await admin.deleteUser({ params: { id: '8' }, user: { id: 9 } }, missing) + assert.equal(missing.statusCode, 404) +}) + +// ── reserved usernames (AD9) ─────────────────────────────────────────────── + +test('a reserved name reads as a taken one wherever a username is written', async () => { + patch(usersDb, 'isUsernameReserved', async (name) => name.toLowerCase() === 'ravenna') + patch(usersDb, 'insertUser', async () => { throw new Error('must not insert') }) + patch(usersDb, 'updateUser', async () => { throw new Error('must not update') }) + + const created = await users.createUser({ username: 'RAVENNA', password: 'pw12345678', role: 'player' }).catch((e) => e) + assert.equal(created.code, users.USERNAME_RESERVED) + assert.equal(users.isDuplicateUsername(created), true) + assert.equal(users.isDuplicateEmail(created), false) + + const renamed = await users.update(7, { username: 'Ravenna' }).catch((e) => e) + assert.equal(users.isDuplicateUsername(renamed), true) +}) + +test('a self-service rename onto a reserved name answers the taken-name 409', async () => { + patch(usersDb, 'isUsernameReserved', async () => true) + const res = mockRes() + await account.changeUsername({ body: { username: 'Ravenna' }, user: { id: 7, username: 'Thorne' }, headers: {} }, res) + assert.equal(res.statusCode, 409) + assert.equal(res.body.message, 'That username is already taken.') +}) + +test('a name that is not reserved goes through untouched', async () => { + patch(usersDb, 'isUsernameReserved', async () => false) + let inserted = null + patch(usersDb, 'insertUser', async (row) => { inserted = row; return 70 }) + patch(usersDb, 'findById', async () => ({ id: 70, username: 'Newcomer', role: 'player' })) + const user = await users.createUser({ username: 'Newcomer', role: 'player' }) + assert.equal(user.id, 70) + assert.equal(inserted.username, 'Newcomer') +}) diff --git a/server/test/contentReports.test.js b/server/test/contentReports.test.js index f905398..a7edb23 100644 --- a/server/test/contentReports.test.js +++ b/server/test/contentReports.test.js @@ -192,7 +192,7 @@ test('an upload report carries uploader, size and the SNIFFED type', async () => patch(reportsDb, 'threadsByIds', async () => []) patch(reportsDb, 'postsByIds', async () => []) patch(reportsDb, 'uploadsByIds', async () => [{ - id: 3, team_id: 1, post_id: 80, uploader_username: 'someone', filename: 'a1b2.png', + id: 3, team_id: 1, post_id: 80, uploader_user_id: 12, uploader_username: 'someone', filename: 'a1b2.png', mimetype: 'image/png', byte_size: 184320, created_at: new Date(), deleted_at: null, }]) @@ -201,6 +201,7 @@ test('an upload report carries uploader, size and the SNIFFED type', async () => // load-bearing rather than bookkeeping. assert.equal(item.target.kind, 'upload') assert.equal(item.target.uploader, 'someone') + assert.equal(item.target.uploaderDeleted, false) assert.equal(item.target.byteSize, 184320) assert.equal(item.target.mimetype, 'image/png') assert.equal(item.target.url, '/uploads/a1b2.png') @@ -234,17 +235,64 @@ test('a report whose target is already gone still lists, with a null target', as assert.equal(item.target, null) }) -test('a deleted reporter still shows as somebody, and is marked deleted', async () => { +test('a deleted reporter with no snapshot still shows as somebody, and is marked deleted', async () => { patch(reportsDb, 'list', async () => [row({ reporter_user_id: null, reporter_username: null })]) patch(reportsDb, 'threadsByIds', async () => []) patch(reportsDb, 'postsByIds', async () => []) patch(reportsDb, 'uploadsByIds', async () => []) - const [item] = await reports.queue({}) - assert.equal(item.reporter, '[deleted account]') + const [item] = await reports.queue({ viewer: { role: 'moderator' } }) + assert.equal(item.reporter, '[deleted player]') assert.equal(item.reporterDeleted, true) }) +// ACCOUNT_DELETION.md AD6: staff keep the stored name of a deleted account, for +// moderation, and the flag says which it is. Every snapshot in a report row goes +// through the one rule — reporter, handler, and the target's author. +test('staff see a deleted account’s stored names in the queue, each marked deleted', async () => { + patch(reportsDb, 'list', async () => [row({ + reporter_user_id: null, reporter_username: 'Ravenna', + status: 'actioned', handled_by: null, handled_username: 'OldMod', handled_at: new Date(), + })]) + patch(reportsDb, 'threadsByIds', async () => []) + patch(reportsDb, 'uploadsByIds', async () => []) + patch(reportsDb, 'postsByIds', async () => [{ + id: 80, thread_id: 5, author_user_id: null, author_username: 'Thorne', status: 'hidden', + body_html: '

x

', created_at: new Date(), team_id: 1, thread_title: 'Raid night', + }]) + + for (const role of ['admin', 'editor', 'moderator']) { + // eslint-disable-next-line no-await-in-loop + const [item] = await reports.queue({ viewer: { role } }) + assert.equal(item.reporter, 'Ravenna', role) + assert.equal(item.reporterDeleted, true) + assert.equal(item.handledBy, 'OldMod') + assert.equal(item.handledByDeleted, true) + assert.equal(item.target.author, 'Thorne') + assert.equal(item.target.authorDeleted, true) + } + + // Nothing but a staff viewer sees those names, so a caller that forgot to say + // who is reading cannot leak them. + const [blind] = await reports.queue({}) + assert.equal(blind.reporter, '[deleted player]') + assert.equal(blind.handledBy, '[deleted player]') + assert.equal(blind.target.author, '[deleted player]') +}) + +test('an open report has no handler, which is not a deleted handler', async () => { + patch(reportsDb, 'list', async () => [row()]) + patch(reportsDb, 'threadsByIds', async () => []) + patch(reportsDb, 'postsByIds', async () => []) + patch(reportsDb, 'uploadsByIds', async () => []) + + const [item] = await reports.queue({ viewer: { role: 'admin' } }) + assert.equal(item.handledBy, null) + assert.equal(item.handledByDeleted, false) + assert.equal(item.reporter, 'wanderer') + assert.equal(item.reporterDeleted, false) +}) + // ── handling ─────────────────────────────────────────────────────────────── test('handling records who decided, when, and why', async () => { diff --git a/server/test/demoLocks.test.js b/server/test/demoLocks.test.js index e0111fa..1a14fa6 100644 --- a/server/test/demoLocks.test.js +++ b/server/test/demoLocks.test.js @@ -234,6 +234,7 @@ const SELF_ROWS = [ ['POST', '/auth/me/account/recovery-codes/generate'], ['DELETE', '/auth/me/account/identities/steam'], ['DELETE', '/auth/me/sessions/12'], // DM17 + ['DELETE', '/auth/me/account'], // ACCOUNT_DELETION.md §4.3 ] test('signed in as a shared login, the account cannot be changed or other sessions revoked', async () => { diff --git a/server/test/displayAuthor.test.js b/server/test/displayAuthor.test.js new file mode 100644 index 0000000..d566a73 --- /dev/null +++ b/server/test/displayAuthor.test.js @@ -0,0 +1,159 @@ +// ── Who a deleted account's snapshots show as (ACCOUNT_DELETION.md AD5, AD6) ── +// +// The rule itself, then each place a member can read a snapshot: forum threads +// and posts, the leader's guest list, and the Team digest mail. The Reports +// queue (staff-only) is covered in contentReports.test.js. +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const { test, afterEach, after } = require('node:test') +const assert = require('node:assert/strict') + +const { displayAuthor, DELETED_PLAYER, seesDeletedNames, hasActor } = require('../src/utils/displayAuthor') +const forum = require('../src/model/teams/teamForum.model') +const grants = require('../src/model/teams/teamGrants.model') +const accessDb = require('../src/model/teams/teamAccess.db') +const teamsDb = require('../src/model/teams/teams.db') +const db = require('../src/utils/db') + +after(() => db.close()) + +const saved = [] +function patch(obj, key, value) { + saved.push([obj, key, obj[key]]) + obj[key] = value +} +afterEach(() => { + while (saved.length) { + const [obj, key, value] = saved.pop() + obj[key] = value + } +}) + +const MEMBER = { role: 'player' } +const STAFF = ['admin', 'editor', 'moderator'].map((role) => ({ role })) + +// ── the rule ─────────────────────────────────────────────────────────────── + +test('a live account shows its name to everyone', () => { + for (const viewer of [null, MEMBER, ...STAFF]) { + assert.deepEqual(displayAuthor({ userId: 4, username: 'Thorne' }, viewer), { name: 'Thorne', deleted: false }) + } +}) + +test('a deleted account reads "[deleted player]" to members and visitors (AD5)', () => { + assert.equal(DELETED_PLAYER, '[deleted player]') + for (const viewer of [null, undefined, MEMBER, { role: 'leader' }, {}]) { + assert.deepEqual(displayAuthor({ userId: null, username: 'Ravenna' }, viewer), { name: DELETED_PLAYER, deleted: true }) + } +}) + +test('staff — admin, editor and moderator — see the stored name, marked deleted (AD6)', () => { + for (const viewer of STAFF) { + assert.equal(seesDeletedNames(viewer), true) + assert.deepEqual(displayAuthor({ userId: null, username: 'Ravenna' }, viewer), { name: 'Ravenna', deleted: true }) + } + // With no snapshot at all there is no name to show anybody. + assert.deepEqual(displayAuthor({ userId: null, username: null }, STAFF[0]), { name: DELETED_PLAYER, deleted: true }) +}) + +test('an absent actor is not a deleted one', () => { + assert.equal(hasActor(null, null), false) + assert.equal(hasActor(null, ''), false) + assert.equal(hasActor(null, 'OldMod'), true) + assert.equal(hasActor(5, null), true) +}) + +// ── forum threads and posts ──────────────────────────────────────────────── + +const threadRow = (over = {}) => ({ + id: 5, type: 'discussion', title: 'Raid night', created_by: null, created_username: 'Ravenna', + created_at: new Date(), last_post_at: new Date(), post_count: 2, pinned: 0, locked: 0, status: 'visible', ...over, +}) +const postRow = (over = {}) => ({ + id: 80, thread_id: 5, author_user_id: null, author_username: 'Ravenna', body_html: '

Guide

', + created_at: new Date(), edited_at: null, status: 'visible', ...over, +}) + +test('a forum thread by a deleted account: "[deleted player]" to a member, the name to staff', () => { + assert.equal(forum.publicThread(threadRow(), MEMBER).author, DELETED_PLAYER) + assert.equal(forum.publicThread(threadRow()).author, DELETED_PLAYER) // no viewer said + assert.equal(forum.publicThread(threadRow(), MEMBER).authorDeleted, true) + for (const viewer of STAFF) { + const t = forum.publicThread(threadRow(), viewer) + assert.equal(t.author, 'Ravenna') + assert.equal(t.authorDeleted, true) + } + assert.equal(forum.publicThread(threadRow({ created_by: 4 }), MEMBER).author, 'Ravenna') + assert.equal(forum.publicThread(threadRow({ created_by: 4 }), MEMBER).authorDeleted, false) +}) + +test('a forum post by a deleted account follows the same rule, and is nobody’s to edit', () => { + const asMember = forum.renderPost(postRow(), 'disabled', { userId: 9, role: 'player', isStaff: false }) + assert.equal(asMember.author, DELETED_PLAYER) + assert.equal(asMember.authorDeleted, true) + assert.equal(asMember.mine, false) + assert.equal(asMember.canEdit, false) + + const asEditor = forum.renderPost(postRow(), 'disabled', { userId: 2, role: 'editor', isStaff: false }) + assert.equal(asEditor.author, 'Ravenna') + assert.equal(asEditor.authorDeleted, true) +}) + +test('the thread list and a thread pass the viewer down to every row', async () => { + const forumDb = require('../src/model/teams/teamForum.db') + const forumSettings = require('../src/model/teams/teamForumSettings.model') + patch(forumDb, 'threadsByTeam', async () => [threadRow()]) + patch(forumDb, 'threadById', async () => ({ ...threadRow(), team_id: 1 })) + patch(forumDb, 'postsByThread', async () => [postRow()]) + patch(forumSettings, 'imageMode', async () => 'disabled') + + const [listedForMember] = await forum.listThreads(1, { viewer: MEMBER }) + assert.equal(listedForMember.author, DELETED_PLAYER) + const [listedForStaff] = await forum.listThreads(1, { viewer: { role: 'moderator' } }) + assert.equal(listedForStaff.author, 'Ravenna') + + const thread = await forum.getThread(1, 5, { viewer: { userId: 9, role: 'player' } }) + assert.equal(thread.author, DELETED_PLAYER) + assert.equal(thread.posts[0].author, DELETED_PLAYER) + const staffThread = await forum.getThread(1, 5, { canModerate: true, viewer: { userId: 2, role: 'admin', isStaff: true } }) + assert.equal(staffThread.author, 'Ravenna') + assert.equal(staffThread.posts[0].author, 'Ravenna') +}) + +// ── the leader's guest list ──────────────────────────────────────────────── + +test('a deleted account is not a forum guest, and a deleted granter reads "[deleted player]" (AD13)', async () => { + patch(accessDb, 'activeGrants', async () => [ + { user_id: 21, username: 'Guest', granted_by: null, granted_username: 'OldLeader', granted_at: new Date(), reason: null }, + { user_id: null, username: 'Ravenna', granted_by: 4, granted_username: 'Leader', granted_at: new Date(), reason: null }, + { user_id: 22, username: 'Member', granted_by: 4, granted_username: 'Leader', granted_at: new Date(), reason: null }, + ]) + patch(teamsDb, 'membersByTeam', async () => [{ user_id: 22 }]) + + const guests = await grants.forumGuests(1, MEMBER) + assert.deepEqual(guests.map((g) => g.username), ['Guest']) + assert.equal(guests[0].grantedBy, DELETED_PLAYER) + + const forStaff = await grants.forumGuests(1, { role: 'moderator' }) + assert.equal(forStaff[0].grantedBy, 'OldLeader') +}) + +// ── the Team digest ──────────────────────────────────────────────────────── + +test('the digest mail names a deleted author "[deleted player]"', async () => { + const worker = require('../src/utils/teamDigestWorker') + const teamNotify = require('../src/model/teams/teamNotify.model') + const templates = require('../src/engagement/templates') + let items = null + patch(teamNotify, 'digestPostsSince', async () => [ + { id: 1, thread_id: 5, body_html: '

a

', created_at: new Date(), author_user_id: null, author_username: 'Ravenna', title: 'Raid night', type: 'discussion' }, + { id: 2, thread_id: 5, body_html: '

b

', created_at: new Date(), author_user_id: 4, author_username: 'Thorne', title: 'Raid night', type: 'discussion' }, + ]) + patch(templates, 'renderByKey', async (key, values) => { + items = values.items + return null // stop before anything is sent + }) + await worker.sendOne({ id: 1, slug: 'wolves', name: 'Iron Wolves' }, { user_id: 9, last_digest_at: null }, null, new Date()) + assert.deepEqual(items.map((i) => i.heading), ['Raid night — [deleted player]', 'Raid night — Thorne']) +}) diff --git a/server/test/emailTemplates.test.js b/server/test/emailTemplates.test.js index 1c4cfe8..7105ffa 100644 --- a/server/test/emailTemplates.test.js +++ b/server/test/emailTemplates.test.js @@ -299,12 +299,13 @@ test('every shipped template validates against the email registry', () => { assert.equal(typeof seed.seedVersion, 'number') assert.equal(['email', 'inapp'].includes(seed.channel), true) } - // The five transactional bodies the system itself depends on are protected. + // The six transactional bodies the system itself depends on are protected. const guarded = SEEDS.filter((s) => s.protected).map((s) => s.key) assert.deepEqual(guarded, [ 'auth.password-reset', 'auth.invite', 'auth.email-verify', + 'auth.account-deleted', 'admin.contact-message', 'admin.test', ]) diff --git a/server/test/eventModuleContract.test.js b/server/test/eventModuleContract.test.js index fe37060..5892182 100644 --- a/server/test/eventModuleContract.test.js +++ b/server/test/eventModuleContract.test.js @@ -112,7 +112,7 @@ test('the version a module declares against satisfies ^1.10.0', () => { // 1.10.0 loading — Module-uo declares exactly `^1.10.0` — and this is the // assertion that says it does. The number itself is pinned in version.js. assert.ok(semver.satisfies(MODULE_API_VERSION, '^1.10.0'), MODULE_API_VERSION) - assert.equal(MODULE_API_VERSION, '1.19.0') + assert.equal(MODULE_API_VERSION, '1.20.0') }) test('a module registers actions, budgets, leases and option sources', () => { diff --git a/server/test/userDeletingHook.test.js b/server/test/userDeletingHook.test.js new file mode 100644 index 0000000..0f7729b --- /dev/null +++ b/server/test/userDeletingHook.test.js @@ -0,0 +1,87 @@ +// ── api.onUserDeleting (MODULE_API 1.20.0, ACCOUNT_DELETION.md AD10) ─────── +// +// Registration and collection only; what the deletion does with the hook is in +// accountDeletion.test.js. Each test builds a throwaway modules directory and +// re-requires the loader, as moduleLoader.test.js does. +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const fs = require('fs') +const os = require('os') +const path = require('path') + +const { test, after } = require('node:test') +const assert = require('node:assert/strict') +const express = require('express') + +const db = require('../src/utils/db') +const registries = require('../src/modules/registries') + +after(() => db.close()) + +const emptyTiers = () => ({ public: express.Router(), admin: express.Router(), player: express.Router() }) + +function loadOne(server) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-deleting-')) + const dir = path.join(root, 'rust') + fs.mkdirSync(dir) + fs.writeFileSync(path.join(dir, 'module.json'), JSON.stringify({ + id: 'rust', + name: 'Rust', + version: '1.0.0', + coreApi: '^1.20.0', + server: 'index.js', + mounts: { admin: ['/rust'] }, + })) + fs.writeFileSync(path.join(dir, 'index.js'), server) + process.env.MODULES_DIR = root + registries._reset() + delete require.cache[require.resolve('../src/modules/loader')] + // eslint-disable-next-line global-require + const loader = require('../src/modules/loader') + loader.load(emptyTiers()) + return loader +} +const stateOf = (loader) => loader.list().find((m) => m.id === 'rust') + +test('onUserDeleting is collected once the module is started, and not before', async () => { + const loader = loadOne(`module.exports = (ctx, api) => { + api.registerRoutes({ admin: { '/rust': ctx.express.Router() } }) + api.onUserDeleting(async (p) => { global.__deleting = p }) + }`) + assert.equal(stateOf(loader).state, 'registered') + // A registered module has not booted: it holds no link it could undo yet. + assert.deepEqual(loader.userDeletingHooks(), []) + + loader.setState('rust', 'started') + const hooks = loader.userDeletingHooks() + assert.deepEqual(hooks.map((h) => h.id), ['rust']) + await hooks[0].hook({ userId: 7, deletedBy: 'self' }) + assert.deepEqual(global.__deleting, { userId: 7, deletedBy: 'self' }) + delete global.__deleting + + loader.setState('rust', 'disabled') + assert.deepEqual(loader.userDeletingHooks(), []) +}) + +test('a module that registers no hook adds nothing', () => { + const loader = loadOne(`module.exports = (ctx, api) => { + api.registerRoutes({ admin: { '/rust': ctx.express.Router() } }) + }`) + loader.setState('rust', 'started') + assert.deepEqual(loader.userDeletingHooks(), []) +}) + +test('onUserDeleting twice, or with no function, is a registration error', () => { + for (const [body, expected] of [ + ['api.onUserDeleting(() => {}); api.onUserDeleting(() => {})', /onUserDeleting\(\) called twice/], + ['api.onUserDeleting("later")', /onUserDeleting: expected a function/], + ]) { + const loader = loadOne(`module.exports = (ctx, api) => { + api.registerRoutes({ admin: { '/rust': ctx.express.Router() } }) + ${body} + }`) + assert.equal(stateOf(loader).state, 'startup_failed') + assert.match(stateOf(loader).reason, expected) + } +}) -- 2.49.1