From 26c23bd6033a4f2d9045885ea4fa3c7bcd107f64 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 14:34:24 -0500 Subject: [PATCH 1/6] =?UTF-8?q?feat(teams):=20the=20notification=20core=20?= =?UTF-8?q?=E2=80=94=20four=20streams,=20and=20a=20recipient=20set?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 6's foundation: the fan-out shape the existing pipeline could not express. `pushDispatch.publish` answers "everyone subscribed to a stream" and "this one owner". Team notifications need "these N users", because Team scoping cannot live in a stream id: the catalog is a static registration validated at boot against a namespaced pattern, so a stream per Team is unexpressible, and stream ids are stored in `notification_subscriptions` rows that would need collecting every time a Team archived. So there are FOUR fixed core streams and the Team lives entirely in the recipient set. `team_notification_prefs` is opt-out for push and opt-IN for email — the two sinks default opposite ways, and the asymmetry lives in the column defaults so no condition anywhere has to remember it. One recipient query serves all four streams, because §6.2's two populations are the same set written twice: "active members with a user_id plus active grants" IS "everyone with resolved forum access". Mutes are subtracted in SQL rather than by the caller — there is no function here that returns an unfiltered set. Co-Authored-By: Claude --- server/db/schema.sql | 39 +++ server/src/config/coreStreams.js | 58 ++++- .../src/model/pushDevices/pushDevices.db.js | 42 +++- .../model/pushDevices/pushDevices.model.js | 10 +- server/src/model/teams/teamNotify.db.js | 223 ++++++++++++++++++ server/src/model/teams/teamNotify.model.js | 128 ++++++++++ server/src/utils/pushDispatch.js | 38 ++- 7 files changed, 533 insertions(+), 5 deletions(-) create mode 100644 server/src/model/teams/teamNotify.db.js create mode 100644 server/src/model/teams/teamNotify.model.js diff --git a/server/db/schema.sql b/server/db/schema.sql index b28e013..a1ba538 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -1266,6 +1266,45 @@ CREATE TABLE IF NOT EXISTS team_activity ( INDEX idx_team_activity_feed (team_id, occurred_at) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +-- Per-Team notification preference (TEAMS.md §6.3/§6.4, phase 6). OPT-OUT, not +-- opt-in: a user in a single Team must never have to configure anything, so the +-- absence of a row is the default and every column here is a deviation from it. +-- +-- Team scoping lives HERE and in the recipient computation, never in a stream id. +-- The push catalog is a static registration validated at boot against a namespaced +-- pattern; it cannot express one stream per Team, and stream ids are stored in +-- notification_subscriptions rows that would then need garbage-collecting every +-- time a Team archived. Four fixed streams plus this table is the same feature +-- with nothing to collect. +-- +-- `last_digest_at` is the digest's ONLY state. There is no queue of pending items: +-- the worker asks what arrived after this timestamp and re-runs the access +-- resolver, so a deployment that was down for a day sends one correct digest +-- rather than replaying a backlog, and a user who lost forum access between the +-- post and the send is not emailed content they can no longer read. +CREATE TABLE IF NOT EXISTS team_notification_prefs ( + user_id INT NOT NULL, + team_id INT NOT NULL, + muted TINYINT(1) NOT NULL DEFAULT 0, + -- 'off', and NOT the design-of-record's 'digest'. Digest-by-default would mean + -- every member of every Team starts receiving daily mail the moment an operator + -- connects Gmail, which is a decision about other people's inboxes made on their + -- behalf. Email is therefore the one sink here that is opt-IN; the mute is still + -- opt-out, because a mute silences something the user already asked for. + -- + -- It also keeps this column honest as a deviation-from-default: a row written to + -- set `muted` alone leaves email exactly where it was. + email_mode ENUM('off','digest','immediate') NOT NULL DEFAULT 'off', + last_digest_at DATETIME NULL, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + PRIMARY KEY (user_id, team_id), + CONSTRAINT fk_tnp_user FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + CONSTRAINT fk_tnp_team FOREIGN KEY (team_id) REFERENCES teams(id) ON DELETE CASCADE, + -- The digest worker's driving query is "rows in digest mode, oldest send first", + -- which is a scan of this index rather than of every preference ever written. + INDEX idx_tnp_digest (email_mode, last_digest_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + -- Migrations for databases created before the wiki upgrade. Each statement uses -- IF NOT EXISTS so re-running on every boot is a harmless no-op. New installs get -- these columns from the CREATE TABLE above; existing installs get them here. diff --git a/server/src/config/coreStreams.js b/server/src/config/coreStreams.js index 5c839e1..20e3bc1 100644 --- a/server/src/config/coreStreams.js +++ b/server/src/config/coreStreams.js @@ -2,13 +2,18 @@ // // What is left of config/notificationStreams.js once the shard-derived catalog // moved to config/shardStreams.js (MODULE_SYSTEM.md §1.8: push INFRASTRUCTURE is -// core, the CATALOG is content). Exactly one stream is core's: `news.post` is -// produced by the website's own posts path, not by any game feed. +// core, the CATALOG is content). `news.post` is produced by the website's own +// posts path, not by any game feed, and the four `team.*` streams by core's own +// Team sync and forum. // // Registered through modules/registries.js like any module's, and read back // through it — nothing imports this file to get "the catalog", because the // catalog is core's plus every module's. // +// Phase 6 added the four Team streams below. They are core's for the same reason +// the Team tables are: a module supplies who is in a Team, but who may be told +// about it is the access resolver's answer, and that is core's (TEAMS.md Part 6). +// // The payload that ever leaves the server is a CONTENT-FREE tickle // ({ stream, ref }); the app wakes and PULLS the real, ownership-checked content // over the authenticated API (docs/android/PLAN.md §11). @@ -21,6 +26,55 @@ const STREAMS = [ personal: false, requiresLinkedAccount: false, }, + // ── Teams (TEAMS.md §6.2, phase 6) ─────────────────────────────────────── + // + // FOUR streams, and not one per Team. The catalog is a static registration + // validated at boot; it has no way to express an unbounded runtime-created set, + // and a stream id per Team would leave rows in notification_subscriptions to + // collect every time a Team archived. Which Team an event came from lives in + // the RECIPIENT SET (utils/teamNotify.js) and in the `ref`, never in the id. + // + // `requiresLinkedAccount: false` on all four is deliberate and reads oddly. + // These are game-sourced events, so the instinct is to demand a linked game + // account — but a forum-granted user with no game identity at all is exactly + // the population §2.5 path 3 exists for, and they are a legitimate recipient of + // `team.forum.post`. The flag would refuse them a toggle they have every right + // to. What enforces who gets what is the recipient computation, which asks the + // access resolver; the stream flag is not a second, weaker copy of that rule. + // + // `personal: false` for the same reason it is false on news.post: these are not + // owner-keyed events about one account's own property. `publishToUsers` is a + // third fan-out shape alongside "everyone subscribed" and "this one owner", and + // the catalog has no flag for it because the flag would say nothing a caller + // does not already know by choosing the function. + { + id: 'team.member.joined', + label: 'Team — new member', + description: 'Someone joined a Team you belong to.', + personal: false, + requiresLinkedAccount: false, + }, + { + id: 'team.leadership.changed', + label: 'Team — leadership change', + description: 'Leadership changed in a Team you belong to.', + personal: false, + requiresLinkedAccount: false, + }, + { + id: 'team.forum.post', + label: 'Team — new forum post', + description: 'A new thread or reply in a Team forum you can read.', + personal: false, + requiresLinkedAccount: false, + }, + { + id: 'team.announcement', + label: 'Team — announcements', + description: 'A leader posted an announcement in a Team you can read.', + personal: false, + requiresLinkedAccount: false, + }, ] module.exports = { STREAMS } diff --git a/server/src/model/pushDevices/pushDevices.db.js b/server/src/model/pushDevices/pushDevices.db.js index 04b8492..0b17e7f 100644 --- a/server/src/model/pushDevices/pushDevices.db.js +++ b/server/src/model/pushDevices/pushDevices.db.js @@ -48,4 +48,44 @@ const endpointsForUserStream = (userId, streamId) => [userId, streamId], ) -module.exports = { upsert, getByUserEndpoint, listByUser, remove, endpointsForStream, endpointsForUserStream } +// `Number.isInteger` alone is not enough: `Number(null)` is 0 and 0 is an +// integer, so a null slipping into a caller's list would become user id 0 and +// ride into an IN clause. No row has id 0, so it is harmless today — which is +// exactly why it would never be noticed. +const isUserId = (n) => Number.isInteger(n) && n > 0 + +// Endpoints of a COMPUTED SET of users' devices, each still gated on that user's +// own subscription (TEAMS.md §6.2's third fan-out shape). +// +// The set is the whole Team-scoping mechanism: the four `team.*` streams are +// global, and which Team an event belongs to is expressed by who is in `userIds` +// rather than by a stream id per Team. The caller has already resolved access and +// subtracted mutes; this function's only remaining job is to honour each +// recipient's own opt-in, which is why the JOIN is here and not left to the +// caller — a fan-out that skipped it would deliver to a user who had turned the +// stream off. +// +// Returns [] for an empty set rather than building `IN ()`, which is a syntax +// error in MariaDB. That case is common, not exceptional: most Team events have +// no subscribed recipients on a deployment with no app installed at all. +async function endpointsForUsersStream(userIds, streamId) { + const ids = [...new Set((userIds || []).map(Number).filter(isUserId))] + if (ids.length === 0) return [] + return query( + `SELECT d.endpoint, d.transport + FROM push_devices d + JOIN notification_subscriptions s ON s.user_id = d.user_id + WHERE s.stream_id = ? AND d.user_id IN (${ids.map(() => '?').join(',')})`, + [streamId, ...ids], + ) +} + +module.exports = { + upsert, + getByUserEndpoint, + listByUser, + remove, + endpointsForStream, + endpointsForUserStream, + endpointsForUsersStream, +} diff --git a/server/src/model/pushDevices/pushDevices.model.js b/server/src/model/pushDevices/pushDevices.model.js index f42125a..0e94dbe 100644 --- a/server/src/model/pushDevices/pushDevices.model.js +++ b/server/src/model/pushDevices/pushDevices.model.js @@ -28,5 +28,13 @@ const remove = async (id, userId) => (await db.remove(id, userId)) > 0 // Fan-out helpers: raw { endpoint, transport } rows (not toSafe-shaped). const endpointsForStream = (streamId) => db.endpointsForStream(streamId) const endpointsForUserStream = (userId, streamId) => db.endpointsForUserStream(userId, streamId) +const endpointsForUsersStream = (userIds, streamId) => db.endpointsForUsersStream(userIds, streamId) -module.exports = { register, listForUser, remove, endpointsForStream, endpointsForUserStream } +module.exports = { + register, + listForUser, + remove, + endpointsForStream, + endpointsForUserStream, + endpointsForUsersStream, +} diff --git a/server/src/model/teams/teamNotify.db.js b/server/src/model/teams/teamNotify.db.js new file mode 100644 index 0000000..7beb8ee --- /dev/null +++ b/server/src/model/teams/teamNotify.db.js @@ -0,0 +1,223 @@ +// SQL for Team notification recipients and per-Team preferences (TEAMS.md Part 6). +// +// **The recipient set is the whole of Team scoping.** The four `team.*` streams +// are global and carry no Team in their id; who an event reaches is decided here. +// That is §6.2's design and it is not an optimisation — the push catalog is a +// static registration validated at boot, so a stream per Team is unexpressible, +// and stream ids live in `notification_subscriptions` rows that a per-Team id +// would leave behind every time a Team archived. +// +// **One recipient query serves all four streams**, because the two populations in +// §6.2's table are the same set written twice: "active members with a user_id, +// plus active forum grants" IS "everyone with resolved forum access", by the +// definition of teamAccess.forumAccess() (membership OR grant). What differs +// between the streams is only who is subtracted — the author of the post that +// caused it — and that is a caller's argument, not a second query. +// +// **Mutes are subtracted in SQL, not in the caller.** A recipient list that came +// back complete and was filtered afterwards would be one refactor away from being +// used unfiltered; there is no function here that returns an unmuted set. + +const { query } = require('../../utils/db') + +// The union, as a derived table both recipient functions build on. Written once +// so that "who is in a Team for notification purposes" has exactly one definition. +// +// `status = 'active'` on the membership half and `revoked_at IS NULL` on the +// grant half are the same two conditions the access resolver uses; a departed +// member and a revoked guest are both people who could still be read a private +// forum by a query that forgot one. +const RECIPIENT_UNION = ` + SELECT user_id FROM team_members + WHERE team_id = ? AND status = 'active' AND user_id IS NOT NULL + UNION + SELECT user_id FROM team_forum_grants + WHERE team_id = ? AND revoked_at IS NULL` + +// `Number.isInteger` alone is not enough: `Number(null)` is 0 and 0 is an +// integer, so a null slipping into a caller's list would become user id 0 and +// ride into an IN clause. No row has id 0, so it is harmless today — which is +// exactly why it would never be noticed. +const isUserId = (n) => Number.isInteger(n) && n > 0 + +/** + * Every user id that may be notified about `teamId`, mutes already removed. + * + * `exclude` is the author of the thing that happened. Passed rather than removed + * afterwards for the reason in the header, and taken as a list because a caller + * with nobody to exclude should not have to invent a sentinel. + */ +async function recipientIds(teamId, { exclude = [] } = {}) { + const skip = [...new Set(exclude.map(Number).filter(isUserId))] + const notMe = skip.length ? `AND r.user_id NOT IN (${skip.map(() => '?').join(',')})` : '' + const rows = await query( + `SELECT DISTINCT r.user_id + FROM (${RECIPIENT_UNION}) r + LEFT JOIN team_notification_prefs p ON p.user_id = r.user_id AND p.team_id = ? + WHERE COALESCE(p.muted, 0) = 0 ${notMe}`, + [teamId, teamId, teamId, ...skip], + ) + return rows.map((r) => Number(r.user_id)) +} + +/** + * The same set, narrowed to those reachable by EMAIL and carrying each one's mode. + * + * A separate query rather than a join onto `recipientIds` because email has two + * conditions push does not: an address to send to, and an account still allowed to + * have one. A banned or disabled account keeps its forum grant in the ledger — + * revoking it is a separate staff decision — but must not keep receiving the + * Team's private discussion in its inbox. + * + * `email_mode` is COALESCEd to the column default rather than read as NULL — and + * that default is `'off'`, so this query returns the whole set with most of it + * marked as not wanting mail. Filtering to a mode is the CALLER's job, because + * `immediate` and `digest` are consumed by two different senders. + */ +async function emailRecipients(teamId, { exclude = [] } = {}) { + const skip = [...new Set(exclude.map(Number).filter(isUserId))] + const notMe = skip.length ? `AND u.id NOT IN (${skip.map(() => '?').join(',')})` : '' + return query( + `SELECT u.id AS user_id, u.username, u.email, + COALESCE(p.email_mode, 'off') AS email_mode, + p.last_digest_at + FROM (${RECIPIENT_UNION}) r + JOIN users u ON u.id = r.user_id + LEFT JOIN team_notification_prefs p ON p.user_id = u.id AND p.team_id = ? + WHERE COALESCE(p.muted, 0) = 0 + AND u.email IS NOT NULL AND u.email <> '' + AND u.status = 'active' ${notMe} + GROUP BY u.id, u.username, u.email, p.email_mode, p.last_digest_at`, + [teamId, teamId, teamId, ...skip], + ) +} + +// ── Preferences ──────────────────────────────────────────────────────────── + +/** + * One row per Team this user may be notified about, whether or not a preference + * has ever been written for it — the account screen has to offer a Team the user + * has never touched, and a list built from the prefs table alone would be empty + * for exactly the users who have configured nothing. + * + * Archived Teams appear only when a preference row exists for them, so a mute the + * user set does not vanish from the screen the moment a guild disbands, while a + * disbanded guild nobody configured does not linger on it forever. + */ +async function prefsForUser(userId) { + return query( + `SELECT t.id AS team_id, t.slug, t.name, t.display_name_override, t.status AS team_status, + COALESCE(p.muted, 0) AS muted, + COALESCE(p.email_mode, 'off') AS email_mode + FROM teams t + LEFT JOIN team_notification_prefs p ON p.team_id = t.id AND p.user_id = ? + WHERE ( + EXISTS (SELECT 1 FROM team_members m + WHERE m.team_id = t.id AND m.user_id = ? AND m.status = 'active') + OR EXISTS (SELECT 1 FROM team_forum_grants g + WHERE g.team_id = t.id AND g.user_id = ? AND g.revoked_at IS NULL) + OR p.user_id IS NOT NULL + ) + ORDER BY t.status, t.name`, + [userId, userId, userId], + ) +} + +/** One Team's preference for one user, or undefined. Read by the mute toggle. */ +async function prefFor(userId, teamId) { + const rows = await query( + `SELECT team_id, muted, email_mode, last_digest_at + FROM team_notification_prefs WHERE user_id = ? AND team_id = ?`, + [userId, teamId], + ) + return rows[0] +} + +/** + * Write one preference. + * + * An upsert that touches ONLY the columns it was given: the one-click unsubscribe + * writes `muted` and must not reset an `email_mode` the user chose, and the + * settings screen writes both. `last_digest_at` is never written here — it is the + * worker's column, and a preference change must not look like a delivery. + */ +async function setPref(userId, teamId, { muted, emailMode }) { + const sets = ['updated_at = CURRENT_TIMESTAMP'] + if (muted != null) sets.push('muted = VALUES(muted)') + if (emailMode != null) sets.push('email_mode = VALUES(email_mode)') + await query( + `INSERT INTO team_notification_prefs (user_id, team_id, muted, email_mode) + VALUES (?, ?, ?, ?) + ON DUPLICATE KEY UPDATE ${sets.join(', ')}`, + [userId, teamId, muted ? 1 : 0, emailMode || 'off'], + ) +} + +/** Stamp a digest as delivered. The worker's column, and its only writer. */ +async function stampDigest(userId, teamId, at) { + await query( + `INSERT INTO team_notification_prefs (user_id, team_id, last_digest_at) + VALUES (?, ?, ?) + ON DUPLICATE KEY UPDATE last_digest_at = VALUES(last_digest_at)`, + [userId, teamId, at], + ) +} + +/** + * Active Teams that have had forum activity since `since` — the digest worker's + * driving query. + * + * Driven from ACTIVITY rather than from the prefs table, which is what makes the + * worker's cost proportional to what was WRITTEN rather than to how many people + * once opened a settings screen. A Team nobody posted in costs one row of this + * query and no recipient computation at all. + */ +async function teamsWithForumActivitySince(since) { + return query( + `SELECT DISTINCT t.id, t.slug, t.name, t.display_name_override + FROM teams t + JOIN team_forum_threads th ON th.team_id = t.id + JOIN team_forum_posts po ON po.thread_id = th.id + WHERE t.status = 'active' + AND po.created_at > ? + AND po.status = 'visible' + AND th.status = 'visible'`, + [since], + ) +} + +/** + * The posts one digest covers: visible posts in visible threads, newer than the + * recipient's own `since`. + * + * Re-read at send time rather than accumulated at publish time. A queue of pending + * items would have to be garbage-collected, would replay a backlog after an outage, + * and — the reason that actually matters — could email a body a moderator hid in + * between. This query cannot: a hidden post is simply not in it. + */ +async function digestPostsSince(teamId, since, limit = 20) { + return query( + `SELECT po.id, po.thread_id, po.body_html, po.created_at, po.author_username, + th.title, th.type + FROM team_forum_posts po + JOIN team_forum_threads th ON th.id = po.thread_id + WHERE th.team_id = ? + AND po.created_at > ? + AND po.status = 'visible' + AND th.status = 'visible' + ORDER BY po.created_at + LIMIT ?`, + [teamId, since, Number(limit)], + ) +} + +module.exports = { + recipientIds, + emailRecipients, + prefsForUser, + prefFor, + setPref, + stampDigest, + teamsWithForumActivitySince, + digestPostsSince, +} diff --git a/server/src/model/teams/teamNotify.model.js b/server/src/model/teams/teamNotify.model.js new file mode 100644 index 0000000..168dd66 --- /dev/null +++ b/server/src/model/teams/teamNotify.model.js @@ -0,0 +1,128 @@ +// Per-Team notification preferences, and the recipient sets built from them +// (TEAMS.md §6.2–§6.4, phase 6). +// +// **The absence of a row is the default, and the two sinks default OPPOSITE ways.** +// Push is opt-out: a user in one Team must never have to configure anything to be +// tickled about it, and the per-Team mute is how they stop. Email is opt-IN +// (`email_mode` defaults to `'off'`, deviating from §6.4 on the org lead's call): +// turning on Gmail in the admin panel must not start sending daily mail to every +// member of every Team on the deployment. +// +// Both are read the same way — COALESCE to the column default, never treat a +// missing row as "unknown" — so the asymmetry lives in ONE place, the schema, and +// not in a condition anybody has to remember. +// +// **This file never decides who may READ a Team.** It asks the same two tables +// teamAccess.forumAccess() asks, in one query, because a fan-out cannot afford a +// round trip per recipient — but it asks them for the same answer. If the access +// rule ever changes, both must; the SQL in teamNotify.db.js says so at the union +// it builds on, and the test that matters is the one asserting a revoked guest +// receives nothing. + +const db = require('./teamNotify.db') + +// Stored as an ENUM, restated here because a value arriving from a request body +// must be checked against something in JavaScript before it reaches the column — +// a bad value would otherwise be a 500 from the driver rather than a 400 from us. +const EMAIL_MODES = ['off', 'digest', 'immediate'] + +const isEmailMode = (v) => EMAIL_MODES.includes(v) + +function publicPref(row) { + return { + teamId: Number(row.team_id), + slug: row.slug, + // The same `display_name_override || name` rule every other Team surface + // uses (§2.8.3). A notification screen showing the raw name would show a name + // staff have deliberately replaced everywhere else. + name: row.display_name_override || row.name, + archived: row.team_status === 'archived', + muted: Boolean(Number(row.muted)), + emailMode: row.email_mode, + } +} + +/** Every Team this user could be notified about, with its current preference. */ +async function listPrefs(userId) { + return (await db.prefsForUser(userId)).map(publicPref) +} + +/** One Team's preference for one user, defaults applied. Never null. */ +async function prefFor(userId, teamId) { + const row = await db.prefFor(userId, teamId) + return { + teamId: Number(teamId), + muted: Boolean(row && Number(row.muted)), + emailMode: (row && row.email_mode) || 'off', + } +} + +/** + * Replace this user's whole set of Team preferences. + * + * PUT-the-whole-set, matching the existing subscription endpoint, and the + * Android gotcha carried forward from `docs/android/PLAN.md` §11 applies to the + * ROUTE rather than to this function: the array is required even when empty. + * + * **A preference may only be written for a Team the caller is actually in.** The + * ids are checked against `listPrefs`, not trusted from the body — otherwise any + * authenticated user could write a row naming any Team, which is a (small) write + * primitive into a table keyed by someone else's private membership. Unknown ids + * are dropped rather than 400'd: a Team the user left between loading the screen + * and saving it is an ordinary race, not a client bug. + */ +async function replacePrefs(userId, entries) { + const allowed = new Map((await listPrefs(userId)).map((p) => [p.teamId, p])) + const written = [] + for (const entry of entries) { + const teamId = Number(entry && entry.teamId) + if (!allowed.has(teamId)) continue + const emailMode = isEmailMode(entry.emailMode) ? entry.emailMode : 'off' + // eslint-disable-next-line no-await-in-loop + await db.setPref(userId, teamId, { muted: Boolean(entry.muted), emailMode }) + written.push(teamId) + } + return { written, prefs: await listPrefs(userId) } +} + +/** + * Mute one Team for one user — the one-click unsubscribe's only effect. + * + * Deliberately narrow. The unsubscribe link is reached without a session, so what + * it can do is what an attacker holding a leaked link can do: silence one Team's + * notifications for one account, visibly and reversibly on the account screen. + * It writes no other column, and there is no "unsubscribe from everything". + */ +async function mute(userId, teamId) { + await db.setPref(userId, teamId, { muted: true }) +} + +/** Un-mute, for the toggle's other half. */ +async function unmute(userId, teamId) { + await db.setPref(userId, teamId, { muted: false }) +} + +module.exports = { + EMAIL_MODES, + isEmailMode, + listPrefs, + prefFor, + replacePrefs, + mute, + unmute, + // Recipient sets, passed through so callers depend on the model rather than on + // the SQL. The fan-out in utils/teamNotify.js and the digest worker are the only + // callers. + // + // Wrapped rather than re-exported (`recipientIds: db.recipientIds`), which is + // the obvious shorter form and is wrong: that captures the function OBJECT at + // require time, so the layer below can never be substituted afterwards — which + // makes the db layer untestable in isolation and, more to the point, means the + // model is not really the seam it claims to be. These resolve `db.x` at call + // time, so the boundary is real. + recipientIds: (teamId, opts) => db.recipientIds(teamId, opts), + emailRecipients: (teamId, opts) => db.emailRecipients(teamId, opts), + stampDigest: (userId, teamId, at) => db.stampDigest(userId, teamId, at), + teamsWithForumActivitySince: (since) => db.teamsWithForumActivitySince(since), + digestPostsSince: (teamId, since, limit) => db.digestPostsSince(teamId, since, limit), +} diff --git a/server/src/utils/pushDispatch.js b/server/src/utils/pushDispatch.js index 5a96ccc..20688b6 100644 --- a/server/src/utils/pushDispatch.js +++ b/server/src/utils/pushDispatch.js @@ -108,4 +108,40 @@ async function publish(streamId, { ref, ownerUserId } = {}, deps = {}) { await Promise.all(rows.map((r) => postTickle(r.endpoint, bodyStr, deps))) } -module.exports = { publish, isAllowedEndpoint } +// `Number.isInteger` alone is not enough: `Number(null)` is 0 and 0 is an +// integer, so a null slipping into a caller's list would become user id 0 and +// ride into an IN clause. No row has id 0, so it is harmless today — which is +// exactly why it would never be noticed. +const isUserId = (n) => Number.isInteger(n) && n > 0 + +/** + * Publish one content-free tickle to a COMPUTED SET of users (TEAMS.md §6.2). + * + * The third fan-out shape. `publish` answers "everyone subscribed" and "this one + * owner"; Team notifications need "these N users", because the four `team.*` + * streams are global and which Team an event belongs to is expressed by who is in + * the set. Nothing about the tickle changes — same `{ stream, ref }`, same + * untrusted-relay assumption, same SSRF gate on every endpoint. + * + * The set arrives already resolved: the caller has asked the access resolver who + * may read this Team and subtracted the per-Team mutes. What this function still + * enforces is each recipient's own stream subscription, in the query. Never + * throws — a notification failing must not fail the write that produced it. + */ +async function publishToUsers(streamId, { ref, userIds } = {}, deps = {}) { + const devices = deps.pushDevices || pushDevicesModel + const ids = [...new Set((userIds || []).map(Number).filter(isUserId))] + if (ids.length === 0) return + let rows + try { + rows = await devices.endpointsForUsersStream(ids, streamId) + } catch (err) { + log.warn('push endpoint lookup failed', { streamId, message: err.message }) + return + } + if (!rows || rows.length === 0) return + const bodyStr = JSON.stringify({ stream: streamId, ref: ref ?? null }) + await Promise.all(rows.map((r) => postTickle(r.endpoint, bodyStr, deps))) +} + +module.exports = { publish, publishToUsers, isAllowedEndpoint } -- 2.49.1 From 686a2149793dbe2898dc0cae1df738c8e31b0005 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 14:34:38 -0500 Subject: [PATCH 2/6] feat(teams): email as the third sink, with a digest that keeps no queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A web-only user on a deployment running neither the Android app nor Discord gets no notification that someone replied to their own thread — which is most users on most deployments, and a forum where replies are invisible is a forum nobody returns to. Email is a third consumer of the recipient set the previous commit builds, not a fourth pipeline. Unlike a push tickle, an email carries content: a mailbox is a destination the recipient chose, not an untrusted relay reached by an unguessable topic. It carries a title and an excerpt, never a full post. The digest COMPUTES AT SEND TIME and keeps no pending-items queue. The only state is `last_digest_at`. Three properties fall out, and the third is why it was chosen: a deployment down for two days sends one correct digest rather than replaying a backlog; a post a moderator hid after it was written is simply not in the query; and a user who lost forum access between the post and the send is no longer in the recipient set, so they are not emailed content they can no longer read. `last_digest_at` is stamped only on a SUCCESSFUL send — stamping first would quietly eat a day of somebody's notifications every time the mail provider had a bad minute. One-click unsubscribe is a stateless HMAC rather than a token table. Every property that makes a password-reset token a row is absent: the link sits in a mailbox for months so it has no useful expiry, and clicking it twice must mean what clicking it once meant. Its whole capability is setting `muted` for one (user, Team) pair. Co-Authored-By: Claude --- server/src/server.js | 3 + server/src/utils/mailer.js | 79 +++++++++- server/src/utils/teamDigestWorker.js | 158 +++++++++++++++++++ server/src/utils/teamNotify.js | 225 +++++++++++++++++++++++++++ server/src/utils/unsubscribeToken.js | 91 +++++++++++ 5 files changed, 555 insertions(+), 1 deletion(-) create mode 100644 server/src/utils/teamDigestWorker.js create mode 100644 server/src/utils/teamNotify.js create mode 100644 server/src/utils/unsubscribeToken.js diff --git a/server/src/server.js b/server/src/server.js index 9b4dbce..6a99914 100644 --- a/server/src/server.js +++ b/server/src/server.js @@ -11,6 +11,7 @@ const botScore = require('./middleware/botScore') const announceWorker = require('./utils/announceWorker') const teamActivityPrune = require('./utils/teamActivityPrune') const teamForumUploadSweep = require('./utils/teamForumUploadSweep') +const teamDigestWorker = require('./utils/teamDigestWorker') const { ensureSchema, close } = require('./utils/db') const { seedDefaults, createInitialAdminFromEnv } = require('../db/seed') const settings = require('./model/settings/settings.model') @@ -157,6 +158,7 @@ async function start() { // rather than after someone notices. No-op on a deployment with no Teams. teamActivityPrune.start() teamForumUploadSweep.start() + teamDigestWorker.start() setupShutdown(server, internalServer) } @@ -177,6 +179,7 @@ function setupShutdown(server, internalServer) { announceWorker.stop() // stop the news-announcement dispatcher poller teamActivityPrune.stop() // stop the Team activity retention timer teamForumUploadSweep.stop() // stop the forum upload sweep + teamDigestWorker.stop() // stop the Team forum digest timer server.close(() => log.info('http server closed')) if (internalServer) internalServer.close(() => log.info('internal http server closed')) try { diff --git a/server/src/utils/mailer.js b/server/src/utils/mailer.js index 781a227..a8764ac 100644 --- a/server/src/utils/mailer.js +++ b/server/src/utils/mailer.js @@ -187,4 +187,81 @@ async function sendPasswordReset({ to, resetUrl, username }) { } } -module.exports = { isConfigured, sendContactMessage, sendTest, sendInvite, sendPasswordReset } +/** + * Send a Team notification — one event (`immediate` mode) or a day's worth + * (`digest` mode). TEAMS.md §6.4. + * + * **This one carries CONTENT, and the push tickle beside it deliberately does + * not.** A tickle goes to ntfy, an untrusted relay reachable by an unguessable + * topic, so it carries `{ stream, ref }` and the app pulls the real thing over an + * access-checked API. A mailbox is a destination the recipient chose. Same + * reasoning as the Discord bridge (§7.2), and it is why this function takes + * excerpts rather than ids. + * + * **Excerpts, never full posts.** Partly courtesy, mostly so that the blast radius + * of a mis-addressed or forwarded mail is a sentence rather than a thread. The + * caller does the truncation, because it is the caller that knows the body was + * already stripped of markup. + * + * The `List-Unsubscribe` pair is what makes a mail client's own unsubscribe button + * appear, and both halves are needed: the `mailto:`-free URL form for clients that + * open the link, and `List-Unsubscribe-Post` for RFC 8058 one-click, which POSTs + * without ever showing the user a page. Both reach the same tokened endpoint that + * writes the same per-Team mute the site shows. + * + * Never throws. A notification failing must not fail the forum write that caused + * it, and there is nobody up the stack to catch it — the digest worker runs on a + * timer and the immediate send is fired from a request that has already replied. + */ +async function sendTeamNotification({ to, subject, intro, items, teamUrl, unsubscribeUrl, unsubscribeApiUrl }) { + const built = await buildTransport() + if (!built) return { sent: false, reason: 'NOT_CONFIGURED' } + const { transport, config } = built + + const lines = [intro, ''] + for (const item of items || []) { + lines.push(`${item.heading}`) + if (item.excerpt) lines.push(` ${item.excerpt}`) + if (item.url) lines.push(` ${item.url}`) + lines.push('') + } + if (teamUrl) lines.push(teamUrl, '') + if (unsubscribeUrl) { + lines.push('To stop these emails for this team, use this link:', unsubscribeUrl) + } + + try { + await transport.sendMail({ + from: fromHeader(config), + to, + subject, + text: lines.join('\n'), + // The header carries the API url, not the one in the body: a one-click + // client POSTs to whatever is here without rendering anything, so it has to + // be an endpoint. Falls back to the body's url when no API one was passed. + headers: (unsubscribeApiUrl || unsubscribeUrl) + ? { + 'List-Unsubscribe': `<${unsubscribeApiUrl || unsubscribeUrl}>`, + 'List-Unsubscribe-Post': 'List-Unsubscribe=One-Click', + } + : undefined, + }) + return { sent: true } + } catch (err) { + // Logged and swallowed, unlike every other sender in this file. Those are + // called by a request that can report the failure to whoever caused it; this + // one is not, and recordStatus already puts the error where an admin reads it. + log.warn('team notification send failed', { message: err.message }) + await emailConfig.recordStatus({ status: 'error', statusDetail: err.message }).catch(() => {}) + return { sent: false, reason: 'SEND_FAILED' } + } +} + +module.exports = { + isConfigured, + sendContactMessage, + sendTest, + sendInvite, + sendPasswordReset, + sendTeamNotification, +} diff --git a/server/src/utils/teamDigestWorker.js b/server/src/utils/teamDigestWorker.js new file mode 100644 index 0000000..50d5c74 --- /dev/null +++ b/server/src/utils/teamDigestWorker.js @@ -0,0 +1,158 @@ +// ── Team forum digest worker (TEAMS.md §6.4, phase 6) ────────────────────── +// +// Daily, per (user, Team): "here is what you missed". The same in-process shape as +// utils/teamActivityPrune and utils/announceWorker — setInterval + unref + stop(), +// wired into server.js start/shutdown. There is no cron in this stack. +// +// **It computes at send time and keeps no queue.** The only state is +// `team_notification_prefs.last_digest_at`; everything else is re-derived from the +// forum tables when the mail is about to go out. Three properties fall out of that, +// and they are why the design chose it over a pending-items table: +// +// 1. A deployment that was down for two days sends ONE correct digest, not two +// days of replay. +// 2. A post a moderator hid after it was written is not in the query, so it is +// not in the mail. A queue written at publish time would have to remember to +// go back and remove it. +// 3. A user who lost forum access between the post and the send is no longer in +// the recipient set, so they are not emailed content they can no longer read. +// This is the one that would have been a security bug. +// +// **The first run is delayed, for the same reason the prune's is**: a boot that is +// crash-looping must not send mail on every loop. + +const teamNotify = require('../model/teams/teamNotify.model') +const forumSettings = require('../model/teams/teamForumSettings.model') +const mailer = require('./mailer') +const notify = require('./teamNotify') +const brand = require('../config/brand') +const log = require('./logger')('team-digest') + +const INTERVAL_MS = Number(process.env.TEAM_DIGEST_INTERVAL_MS) || 24 * 60 * 60 * 1000 +const FIRST_RUN_MS = Number(process.env.TEAM_DIGEST_DELAY_MS) || 10 * 60 * 1000 + +// How far back a recipient with no `last_digest_at` reaches. A first digest must +// not be the entire history of the forum, and this is also the clamp that stops a +// long outage producing one enormous mail — `since` is never older than this, +// however long ago the last send was. +const MAX_LOOKBACK_MS = 7 * 24 * 60 * 60 * 1000 + +// Posts per digest. Beyond this the mail is a list, not a summary, and the link +// to the Team is the better answer. +const MAX_ITEMS = 20 + +let timer = null +let firstRun = null + +const clampSince = (last, now) => { + const floor = new Date(now.getTime() - MAX_LOOKBACK_MS) + // No previous send: reach back one interval, not to the floor. A brand-new + // subscriber's first digest should cover today, not the past week. + if (!last) return new Date(now.getTime() - INTERVAL_MS) + const at = new Date(last) + return at < floor ? floor : at +} + +/** + * One recipient's digest for one Team. Returns true if a mail went out. + * + * Stamps `last_digest_at` ONLY on a successful send. A failed SMTP call leaves the + * stamp alone so the next run tries the same window again — the alternative, + * stamping first, silently eats a day of somebody's notifications every time the + * mail provider has a bad minute. + */ +async function sendOne(team, recipient, now) { + const since = clampSince(recipient.last_digest_at, now) + const posts = await teamNotify.digestPostsSince(team.id, since, MAX_ITEMS) + // Nothing new for THIS recipient — which is not the same as nothing new for the + // Team, because each recipient has their own `since`. No mail, and no stamp: a + // stamp here would move the window past posts they have not been told about. + if (posts.length === 0) return false + + const label = notify.teamLabel(team) + const res = await mailer.sendTeamNotification({ + to: recipient.email, + subject: `[${brand.name}] ${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'}`, + excerpt: notify.excerpt(p.body_html), + url: notify.threadUrl(team, p.thread_id), + })), + teamUrl: notify.teamPageUrl(team), + unsubscribeUrl: notify.unsubscribeUrl(recipient.user_id, team.id), + unsubscribeApiUrl: notify.unsubscribeApiUrl(recipient.user_id, team.id), + }) + if (!res || !res.sent) return false + await teamNotify.stampDigest(recipient.user_id, team.id, now) + return true +} + +/** + * One sweep. Never throws — it runs on a timer with nobody to catch it. + * + * Returns a small summary so a test (and the log line) can tell "nothing to do" + * from "did nothing". + */ +async function tick(now = new Date()) { + const summary = { teams: 0, sent: 0, skipped: null } + try { + // Two cheap gates before any query that costs anything. Forums switched off + // means the content this digest summarises is not readable on the site + // either, and un-configured email means there is no sink at all (§6.4). + if (!(await forumSettings.forumsEnabled())) return { ...summary, skipped: 'forums-disabled' } + if (!(await mailer.isConfigured())) return { ...summary, skipped: 'email-unconfigured' } + + const floor = new Date(now.getTime() - MAX_LOOKBACK_MS) + const teams = await teamNotify.teamsWithForumActivitySince(floor) + summary.teams = teams.length + + for (const team of teams) { + // eslint-disable-next-line no-await-in-loop + const rows = await teamNotify.emailRecipients(team.id) + for (const r of rows.filter((x) => x.email_mode === 'digest')) { + try { + // Serial, like the immediate sender and for the same reason: one SMTP + // conversation at a time against a provider with its own rate limits. + // eslint-disable-next-line no-await-in-loop + if (await sendOne(team, r, now)) summary.sent += 1 + } catch (err) { + // One recipient's failure must not end the sweep for the rest. The + // unstamped preference means the next run retries this one. + log.warn('digest send failed', { teamId: team.id, userId: r.user_id, message: err.message }) + } + } + } + if (summary.sent > 0) log.info('team digests sent', summary) + return summary + } catch (err) { + log.error('team digest sweep failed', { message: err.message }) + return { ...summary, skipped: 'error' } + } +} + +function start() { + if (timer || firstRun) return timer + firstRun = setTimeout(() => { + firstRun = null + tick() + timer = setInterval(() => { tick() }, INTERVAL_MS) + if (timer.unref) timer.unref() + }, FIRST_RUN_MS) + if (firstRun.unref) firstRun.unref() + log.info('team forum digests started', { intervalMs: INTERVAL_MS, firstRunMs: FIRST_RUN_MS }) + return timer +} + +function stop() { + if (firstRun) { + clearTimeout(firstRun) + firstRun = null + } + if (timer) { + clearInterval(timer) + timer = null + } +} + +module.exports = { start, stop, tick, clampSince, MAX_LOOKBACK_MS, MAX_ITEMS } diff --git a/server/src/utils/teamNotify.js b/server/src/utils/teamNotify.js new file mode 100644 index 0000000..9176bc3 --- /dev/null +++ b/server/src/utils/teamNotify.js @@ -0,0 +1,225 @@ +// ── Team notification fan-out (TEAMS.md Part 6, phase 6) ─────────────────── +// +// One event in, up to two sinks out: a content-free push tickle and — for forum +// content only — an email. The expensive part of a notification is working out +// who should get it, and that is computed once here and handed to both. +// +// **Nothing in this file ever throws.** Every entry point is called from a path +// that has already done the real work: a forum reply is written and answered +// before this runs, and the roster sync's whole job is the roster. A notification +// is a courtesy, and a courtesy that can fail the transaction behind it is a +// defect. So every export catches, logs and returns. +// +// **Push and email do not carry the same thing, on purpose.** The tickle is +// `{ stream, ref }` and goes to ntfy, an untrusted relay reached by an unguessable +// topic; the app wakes and PULLS the real content over the authenticated, +// access-checked API. The email carries a title and an excerpt, because a mailbox +// is a destination the recipient chose rather than a relay (§6.4). The asymmetry +// is the security model, not an inconsistency to tidy up. +// +// **Roster events are push-only, and forum events are the only ones that email.** +// §6.4's argument for the email sink is the web-only user who never learns that +// someone replied to their own thread. "Someone joined the guild" is not that: it +// arrives from a sweep that runs every fifteen minutes, it is already on the +// activity feed, and mailing it is how a notification feature earns a spam +// complaint. The streams exist for all four events; the SINKS differ, and this is +// the file that says so. + +const pushDispatch = require('./pushDispatch') +const teamNotify = require('../model/teams/teamNotify.model') +const forumSettings = require('../model/teams/teamForumSettings.model') +const mailer = require('./mailer') +const registries = require('../modules/registries') +const unsubscribeToken = require('./unsubscribeToken') +const brand = require('../config/brand') +const log = require('./logger')('team-notify') + +const STREAMS = { + MEMBER_JOINED: 'team.member.joined', + LEADERSHIP_CHANGED: 'team.leadership.changed', + FORUM_POST: 'team.forum.post', + ANNOUNCEMENT: 'team.announcement', +} + +// How much of a post body an email carries. Long enough to tell whether the +// thread is worth opening, short enough that the mail is not a copy of the forum. +const EXCERPT_CHARS = 200 + +const baseUrl = () => (process.env.APP_BASE_URL || 'http://localhost:5173').replace(/\/+$/, '') + +/** + * Where this Team's page lives, or null. + * + * Core does not own a Team page — the module that owns the vocabulary owns the + * page (Part 3) — so the only way core can write a link to one is the optional + * `pageUrlTemplate` the provider registers. A deployment whose module omits it + * gets email that names the Team and cannot link to it, which is a worse email + * and not a broken one. + */ +function teamPageUrl(team) { + const provider = registries.registeredTeamProvider() + const template = provider && provider.pageUrlTemplate + if (!template || !team) return null + const path = template + .replace('{externalId}', encodeURIComponent(team.external_id ?? team.externalId ?? '')) + .replace('{slug}', encodeURIComponent(team.slug ?? '')) + return `${baseUrl()}${path}` +} + +const threadUrl = (team, threadId) => { + const page = teamPageUrl(team) + // The forum navigates by SEARCH PARAM rather than by a route, because core has + // no route on a page it does not own (TeamForumPanel.jsx). So a deep link to a + // thread is the module's page plus `?thread=`, and it works under whatever path + // the module chose. + return page ? `${page}?thread=${Number(threadId)}` : null +} + +// TWO urls from one token, and they are not interchangeable. +// +// `unsubscribeUrl` is the human one that goes in the mail body: the site's own +// page, which explains what is about to happen and POSTs once a person has read +// it. `unsubscribeApiUrl` is the machine one that goes in the `List-Unsubscribe` +// header, where RFC 8058 says a client may POST without showing anybody anything — +// so it has to be an endpoint, not a page. The API route answers GET on the same +// path with a redirect to the page, which covers the clients that render the +// header as an ordinary link. +const unsubscribeUrl = (userId, teamId) => + `${baseUrl()}/unsubscribe/${unsubscribeToken.sign(userId, teamId)}` + +const unsubscribeApiUrl = (userId, teamId) => + `${baseUrl()}/api/v1/public/teams/unsubscribe/${unsubscribeToken.sign(userId, teamId)}` + +const teamLabel = (team) => (team && (team.display_name_override || team.name)) || 'your team' + +/** Markup out, whitespace collapsed, truncated. The email is plain text. */ +function excerpt(html) { + const text = String(html || '') + .replace(/<[^>]*>/g, ' ') + .replace(/ /g, ' ') + .replace(/&/g, '&') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/\s+/g, ' ') + .trim() + return text.length > EXCERPT_CHARS ? `${text.slice(0, EXCERPT_CHARS - 1)}…` : text +} + +/** The push half. Resolves recipients, honours mutes and subscriptions, never throws. */ +async function tickle(streamId, team, { ref, exclude = [] } = {}) { + const userIds = await teamNotify.recipientIds(team.id, { exclude }) + if (userIds.length === 0) return 0 + await pushDispatch.publishToUsers(streamId, { ref, userIds }) + return userIds.length +} + +// ── Roster events (push only, see the header) ────────────────────────────── + +// No `memberName` argument, and that is the point: a tickle is content-free, so +// there is nothing about WHO joined for this function to carry. The name is on +// the activity feed the app pulls after waking. +async function memberJoined(team) { + try { + return await tickle(STREAMS.MEMBER_JOINED, team, { ref: `team:${team.id}` }) + } catch (err) { + log.warn('member-joined notification failed', { teamId: team && team.id, message: err.message }) + return 0 + } +} + +async function leadershipChanged(team) { + try { + return await tickle(STREAMS.LEADERSHIP_CHANGED, team, { ref: `team:${team.id}` }) + } catch (err) { + log.warn('leadership notification failed', { teamId: team && team.id, message: err.message }) + return 0 + } +} + +// ── Forum events (push + immediate email) ────────────────────────────────── + +/** + * A new thread or reply. + * + * `type` picks the stream: an announcement is its own stream so a user can take + * the thing a leader wants everyone to read and mute the day-to-day chatter, + * which is the split §6.2 drew and the reason there are four streams and not two. + * + * The author is excluded from both sinks. Not as a nicety — a forum that emails + * you your own post is the first thing anyone turns off, and turning it off costs + * the deployment every other notification too. + */ +async function forumPost({ team, threadId, threadTitle, type, authorUserId, authorName, bodyHtml }) { + try { + // Belt and braces with the routes, which already 404 when forums are off. The + // digest worker has no route in front of it, so the check has to live here as + // well as there — and a switch flipped between a write and its notification + // must silence the notification. + if (!(await forumSettings.forumsEnabled())) return { push: 0, emails: 0 } + + const stream = type === 'announcement' ? STREAMS.ANNOUNCEMENT : STREAMS.FORUM_POST + const exclude = authorUserId ? [authorUserId] : [] + const push = await tickle(stream, team, { ref: `team:${team.id}:thread:${threadId}`, exclude }) + const emails = await emailImmediate({ team, threadId, threadTitle, type, exclude, authorName, bodyHtml }) + return { push, emails } + } catch (err) { + log.warn('forum notification failed', { teamId: team && team.id, message: err.message }) + return { push: 0, emails: 0 } + } +} + +/** + * The `immediate` email mode: one mail per event, to the people who asked for + * exactly that. + * + * Skipped entirely when no email is configured — §6.4's "off unless configured" + * — and checked BEFORE the recipient query so a deployment with no Gmail + * connected pays nothing for the sink it does not have. + */ +async function emailImmediate({ team, threadId, threadTitle, type, exclude, authorName, bodyHtml }) { + if (!(await mailer.isConfigured())) return 0 + const rows = await teamNotify.emailRecipients(team.id, { exclude }) + const recipients = rows.filter((r) => r.email_mode === 'immediate') + if (recipients.length === 0) return 0 + + const label = teamLabel(team) + const kind = type === 'announcement' ? 'announcement' : 'post' + const url = threadUrl(team, threadId) + let sent = 0 + + for (const r of recipients) { + // Serial rather than Promise.all: this is an SMTP conversation per recipient + // against a provider with its own rate limits, and a burst of them from a + // busy thread is how a Gmail sender gets throttled. The loop is also why the + // send below is fire-and-report rather than fire-and-throw. + // eslint-disable-next-line no-await-in-loop + const res = await mailer.sendTeamNotification({ + to: r.email, + subject: `[${brand.name}] ${label}: ${threadTitle}`, + intro: `${authorName || 'Someone'} posted a new ${kind} in ${label}.`, + items: [{ heading: threadTitle, excerpt: excerpt(bodyHtml), url }], + teamUrl: teamPageUrl(team), + unsubscribeUrl: unsubscribeUrl(r.user_id, team.id), + unsubscribeApiUrl: unsubscribeApiUrl(r.user_id, team.id), + }) + if (res && res.sent) sent += 1 + } + return sent +} + +module.exports = { + STREAMS, + memberJoined, + leadershipChanged, + forumPost, + // Exported for the digest worker and for the tests, which is the whole reason + // they are not inlined: a URL that only ever appears inside a mail body is a + // URL nothing can assert on. + teamPageUrl, + threadUrl, + unsubscribeUrl, + unsubscribeApiUrl, + excerpt, + teamLabel, +} diff --git a/server/src/utils/unsubscribeToken.js b/server/src/utils/unsubscribeToken.js new file mode 100644 index 0000000..fb4dc8c --- /dev/null +++ b/server/src/utils/unsubscribeToken.js @@ -0,0 +1,91 @@ +// ── One-click unsubscribe tokens (TEAMS.md §6.4) ─────────────────────────── +// +// A stateless HMAC over (userId, teamId, version), not a row in a table. +// +// **Why stateless.** The alternative is a `password_resets`-shaped token table, +// and it is the wrong shape for this: an unsubscribe link sits in a mailbox for +// months and must still work, so it has no useful expiry; it is not single-use, +// because clicking it twice must mean the same thing as clicking it once; and a +// table would need pruning for a capability that never expires. Every property +// that makes a reset token a row is absent here. +// +// **What the capability actually is.** Holding a token lets the holder set +// `muted = 1` for ONE (user, Team) pair. It cannot read anything, cannot unmute, +// cannot touch email mode, and names no other Team. So the honest threat model is: +// someone who intercepts the mail can silence one Team's notifications for that +// account, visibly and reversibly on the account screen. That is a smaller +// capability than the mail itself already carries (it contains the content). +// +// **`v` is the version prefix, and it is what makes rotation possible at all.** A +// stateless token cannot be revoked individually; bumping VERSION invalidates +// every outstanding link at once, which is the only revocation a design with no +// server-side state can offer, and it needs to exist before it is needed. +// +// The key is SECRET_ENC_KEY, derived through the same dev fallback as +// utils/secretBox — a separate label so an unsubscribe token can never be +// confused with, or replayed as, anything else keyed by the same secret. + +const crypto = require('crypto') +require('dotenv').config() + +const log = require('./logger')('unsub-token') + +const VERSION = 1 + +function resolveKey() { + const explicit = process.env.SECRET_ENC_KEY + if (explicit) return crypto.createHash('sha256').update(`unsubscribe:${explicit}`).digest() + if (process.env.NODE_ENV === 'production') { + throw new Error('SECRET_ENC_KEY must be set in production') + } + const jwt = process.env.JWT_SECRET || 'dev-insecure-jwt-secret-do-not-use-in-production' + log.warn('SECRET_ENC_KEY is not set — deriving an insecure unsubscribe key from JWT_SECRET for development.') + return crypto.createHash('sha256').update(`unsubscribe:${jwt}`).digest() +} + +let cachedKey = null +const key = () => { + // Lazily, not at require time. utils/secretBox resolves its key on import and + // that is fine for a module every boot loads anyway; this one is reached from a + // mail template, and a test that never sends mail should not have to set an env + // var to require the module that sends it. + if (!cachedKey) cachedKey = resolveKey() + return cachedKey +} + +// base64url so the token survives being a path segment, a query value and a mail +// client's own re-wrapping of a long URL without any of the three escaping it. +const b64u = (buf) => buf.toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') + +function sign(userId, teamId) { + const body = `${VERSION}.${Number(userId)}.${Number(teamId)}` + const mac = crypto.createHmac('sha256', key()).update(body).digest() + // Truncated to 16 bytes (128 bits). Full-length would double the URL for no + // reachable gain: forging this buys one mute, and 128 bits is far past the + // point where that is worth anyone's compute. + return `${body}.${b64u(mac.subarray(0, 16))}` +} + +/** + * Verify a token. Returns { userId, teamId } or null — null for every failure + * mode, deliberately, so a caller cannot accidentally report which part was wrong. + */ +function verify(token) { + const parts = String(token || '').split('.') + if (parts.length !== 4) return null + const [v, uid, tid] = parts + if (Number(v) !== VERSION) return null + const userId = Number(uid) + const teamId = Number(tid) + if (!Number.isInteger(userId) || !Number.isInteger(teamId)) return null + + const expected = sign(userId, teamId) + const a = Buffer.from(expected) + const b = Buffer.from(String(token)) + // Length-check first: timingSafeEqual throws on a length mismatch, and the + // length of a token is not a secret. + if (a.length !== b.length || !crypto.timingSafeEqual(a, b)) return null + return { userId, teamId } +} + +module.exports = { sign, verify, VERSION } -- 2.49.1 From 2a56cbf22afe8dd8575715c05451fd0314ced7cd Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 14:34:54 -0500 Subject: [PATCH 3/6] feat(teams): fire the four events, and the routes that configure them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The roster sync tickles at most ONCE per stream per run, not once per member: a tickle is content-free, so five people joining in one sweep is five identical notifications and one piece of information. Suppressed on a Team's FIRST roster, the same condition the activity feed uses and the half where it matters more — importing a 155-member guild would otherwise wake every one of their phones. Forum notifications fire from the CONTROLLER, not from the forum model. That file takes an already-resolved access decision and reads no membership table by design; the fan-out reads both to compute its recipients, so calling it from inside would make the forum model transitively depend on exactly what its header says it must not touch. The model returns a `notify` key the controller destructures out before the response, so the API's answer to "did my post save" is unchanged. `pageUrlTemplate` joins the team provider — the one thing phase 6 found that the design of record had not anticipated. Phase 3 left core with no Team page and therefore no way to LINK to one, so a notification email could name a Team and not take you to it. It is data rather than a callback: a function would put a module hook on the mail path to produce a string that never varies. Relative paths only, and protocol-relative is refused with absolute. The unsubscribe endpoint is the only write in the public tier and the only route with no `siteMode` — the reader is in their mail client, and the mail went out before the site went into maintenance. POST always answers 200, valid token or forged: distinguishing them would be an oracle for which (user, Team) pairs exist. GET redirects and acts on nothing, so a mail client's link scanner cannot mute Teams nobody asked to leave. Co-Authored-By: Claude --- server/routes.guards.json | 32 ++ server/routes.manifest.json | 16 + server/src/model/teams/teamForum.model.js | 18 +- server/src/model/teams/teamSync.model.js | 33 +- server/src/modules/registries.js | 38 +++ .../v1/auth/notifications.controller.js | 36 +++ .../router/v1/auth/notifications.routes.js | 36 +++ .../router/v1/player/teamForum.controller.js | 38 ++- .../src/router/v1/public/teams.controller.js | 52 ++- server/src/router/v1/public/teams.router.js | 34 ++ server/swagger/swagger-output.json | 306 ++++++++++++++++++ server/swagger/swagger.js | 19 ++ 12 files changed, 651 insertions(+), 7 deletions(-) diff --git a/server/routes.guards.json b/server/routes.guards.json index 2d8b6dc..343ac05 100644 --- a/server/routes.guards.json +++ b/server/routes.guards.json @@ -1387,6 +1387,26 @@ "validate" ] }, + { + "method": "GET", + "path": "/api/v1/auth/me/notifications/teams", + "handlers": 1, + "gates": [ + "noindex", + "requireAuth" + ] + }, + { + "method": "PUT", + "path": "/api/v1/auth/me/notifications/teams", + "handlers": 6, + "gates": [ + "noindex", + "requireAuth", + "middleware", + "validate" + ] + }, { "method": "GET", "path": "/api/v1/auth/me/sessions", @@ -1944,6 +1964,18 @@ "siteMode" ] }, + { + "method": "GET", + "path": "/api/v1/public/teams/unsubscribe/:token", + "handlers": 1, + "gates": [] + }, + { + "method": "POST", + "path": "/api/v1/public/teams/unsubscribe/:token", + "handlers": 1, + "gates": [] + }, { "method": "GET", "path": "/api/v1/public/version", diff --git a/server/routes.manifest.json b/server/routes.manifest.json index b6be23f..30bf058 100644 --- a/server/routes.manifest.json +++ b/server/routes.manifest.json @@ -549,6 +549,14 @@ "method": "PUT", "path": "/api/v1/auth/me/notifications/subscriptions" }, + { + "method": "GET", + "path": "/api/v1/auth/me/notifications/teams" + }, + { + "method": "PUT", + "path": "/api/v1/auth/me/notifications/teams" + }, { "method": "GET", "path": "/api/v1/auth/me/sessions" @@ -789,6 +797,14 @@ "method": "GET", "path": "/api/v1/public/teams/by-external/:moduleId/:externalId" }, + { + "method": "GET", + "path": "/api/v1/public/teams/unsubscribe/:token" + }, + { + "method": "POST", + "path": "/api/v1/public/teams/unsubscribe/:token" + }, { "method": "GET", "path": "/api/v1/public/version" diff --git a/server/src/model/teams/teamForum.model.js b/server/src/model/teams/teamForum.model.js index a5d8506..daa9692 100644 --- a/server/src/model/teams/teamForum.model.js +++ b/server/src/model/teams/teamForum.model.js @@ -217,7 +217,17 @@ async function createThread({ team, actor, type, title, body }) { authorUsername: actor.username, bodyHtml: cleaned, }) - return { ok: true, threadId, postId } + // `notify` is what the CONTROLLER needs to fan a notification out, and it is a + // separate key rather than more fields on the result because the controller + // spreads the result straight into the response body — a notification's excerpt + // is not part of the API's answer to "did my post save". + // + // The notification itself is fired from the controller and not from here, on + // this file's own rule (see the header): everything in it takes an + // already-resolved access decision and reads no membership table. The fan-out + // reads both, so importing it here would make the forum model transitively + // depend on exactly what it exists not to touch. + return { ok: true, threadId, postId, notify: { threadId, title, type, bodyHtml: cleaned } } } /** @@ -256,7 +266,11 @@ async function createPost({ team, threadId, actor, body }) { authorUsername: actor.username, bodyHtml: cleaned, }) - return { ok: true, threadId, postId } + // The thread's OWN title and type, not the reply's — a reply has neither, and + // what a recipient needs to know is which conversation moved. `type` is always + // 'discussion' here (an announcement takes no replies) and is carried anyway so + // the controller has one shape to hand the fan-out from both routes. + return { ok: true, threadId, postId, notify: { threadId, title: thread.title, type: thread.type, bodyHtml: cleaned } } } /** diff --git a/server/src/model/teams/teamSync.model.js b/server/src/model/teams/teamSync.model.js index 067f41f..395dbaa 100644 --- a/server/src/model/teams/teamSync.model.js +++ b/server/src/model/teams/teamSync.model.js @@ -33,6 +33,7 @@ const teamsDb = require('./teams.db') const teamProvider = require('./teamProvider') const moderation = require('./teamModeration.model') const activity = require('./teamActivity.model') +const teamNotify = require('../../utils/teamNotify') const { slugify, uniqueSlug } = require('./teamSlug') const settings = require('../settings/settings.model') const log = require('../../utils/logger')('teams') @@ -191,6 +192,34 @@ async function logRosterActivity(team, { joined, left, promoted, demoted }) { } } +/** + * The push half of the same roster run (TEAMS.md §6.2, phase 6). + * + * **At most one tickle per stream per run, not one per member.** A tickle is + * content-free — it says "something happened in this Team" and the app pulls the + * rest — so five people joining in one sweep is five identical notifications and + * one piece of information. The feed above is per-member because it is a record; + * this is per-run because it is a nudge. + * + * **Suppressed on a Team's FIRST roster, exactly as the feed is**, and this is the + * half where it matters more: importing a 155-member guild would otherwise wake + * every one of their phones. `roster_synced_at IS NULL` is the same condition, read + * from the same row before the same stamp moves. + * + * Never throws — the fan-out swallows its own failures, and this adds the guard + * for anything the surrounding read could raise. A roster sync is the source of + * truth; a notification about it is not. + */ +async function notifyRoster(team, { joined, promoted, demoted }) { + if (!team.roster_synced_at) return + try { + if (joined.length > 0) await teamNotify.memberJoined(team) + if (promoted.length > 0 || demoted.length > 0) await teamNotify.leadershipChanged(team) + } catch (err) { + log.warn('roster notification not sent', { teamId: team.id, message: err.message }) + } +} + /** * Sync one Team's roster and leadership. Gates 3 and 4 live here. * @@ -285,8 +314,10 @@ async function syncRoster(team) { } await teamsDb.recount(team.id) - // Read before `markRosterSynced` moves the stamp this decision turns on. + // Both read before `markRosterSynced` moves the stamp their first-roster + // suppression turns on. await logRosterActivity(team, { joined, left: left.filter(Boolean), promoted, demoted }) + await notifyRoster(team, { joined, promoted, demoted }) await teamsDb.markRosterSynced(team.id) return true } diff --git a/server/src/modules/registries.js b/server/src/modules/registries.js index e0fa24c..21a77a7 100644 --- a/server/src/modules/registries.js +++ b/server/src/modules/registries.js @@ -255,6 +255,10 @@ function checkLegShape(entry) { // implementing it means core fails CLOSED when the call cannot be made, so this // is a member to add deliberately rather than by habit. // +// `pageUrlTemplate` is the fifth, also OPTIONAL, and is data rather than a method +// — see its own comment below. A module that omits it costs its deployment +// clickable links in Team notification email and nothing else. +// // The copy is explicit rather than a spread: this object is what core calls, so // anything not named here is not part of the contract and must not survive // registration. A method that silently rode along would look implemented from the @@ -274,9 +278,43 @@ function checkTeamProviderShape(entry) { } out.projectRoster = provider.projectRoster } + if (provider.pageUrlTemplate !== undefined) { + out.pageUrlTemplate = checkPageUrlTemplate(provider.pageUrlTemplate) + } return out } +// `pageUrlTemplate` is the fifth member and OPTIONAL (TEAMS.md §6.4, phase 6). +// +// **Why a module has to supply this at all.** Teams are a contract primitive with +// no core surface: core owns the tables and the access rules, and the MODULE owns +// the page, because core does not own the word for a Team. That is settled and +// right — but it leaves core unable to write a link to one, and a notification +// email that cannot link to the thread it is about is most of the way to useless. +// So the module that owns the page says where it is. +// +// **A template, not a callback.** Core substitutes `{externalId}` and `{slug}` +// into a relative path and does nothing else with it. A function would be a +// module hook on the mail path — one more thing that can hang or throw between a +// forum reply and the mail about it — to produce a string that never varies. +// +// Validated hard, because the output goes into an email as a link. Relative only: +// a template naming its own host would let a module redirect the site's outbound +// mail somewhere else, and there is no reason for one to. +// One leading slash, and the second character may not be another. `//evil.test/x` +// passes an "is it rooted" check and is a PROTOCOL-RELATIVE url — core prefixing +// its own base makes it harmless today, but a template is a string that ends up +// in an href sooner or later, and this is a character class rather than a +// judgement call about who concatenates it. +const PAGE_URL_TEMPLATE = /^\/(?!\/)[A-Za-z0-9\-._~/{}]*$/ + +function checkPageUrlTemplate(value) { + if (typeof value !== 'string' || !PAGE_URL_TEMPLATE.test(value)) { + throw new Error(`registerTeamProvider: pageUrlTemplate must be a relative path, got "${value}"`) + } + return value +} + /** * `registerPostHook({ onSaved, onDeleted })` — both optional, at least one * required. A registration with neither is a subscription that can never fire, diff --git a/server/src/router/v1/auth/notifications.controller.js b/server/src/router/v1/auth/notifications.controller.js index a05e19d..f2b4889 100644 --- a/server/src/router/v1/auth/notifications.controller.js +++ b/server/src/router/v1/auth/notifications.controller.js @@ -6,6 +6,7 @@ const pushDevices = require('../../../model/pushDevices/pushDevices.model') const notificationSubs = require('../../../model/notificationSubs/notificationSubs.model') const registries = require('../../../modules/registries') +const teamPrefs = require('../../../model/teams/teamNotify.model') const { isAllowedEndpoint } = require('../../../utils/pushDispatch') const log = require('../../../utils/logger')('notifications') @@ -78,6 +79,39 @@ async function putSubscriptions(req, res) { } } +// GET /auth/me/notifications/teams — this user's per-Team preferences, one row +// per Team they could be notified about whether or not they have ever set one. +// +// Not gated on `teams_forums_enabled`: two of the four streams (member joined, +// leadership changed) have nothing to do with the forum, so a deployment with +// forums switched off still has preferences worth showing. +async function getTeamPrefs(req, res) { + try { + return res.json({ teams: await teamPrefs.listPrefs(req.user.id) }) + } catch (err) { + log.error('getTeamPrefs', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + +// PUT /auth/me/notifications/teams — replace the caller's whole preference set. +// +// PUT-the-whole-set, matching the subscriptions endpoint beside it, and the +// `teams` array is REQUIRED even when empty — the Android gotcha in +// docs/android/PLAN.md §11: a DTO field with a default is dropped by kotlinx when +// it equals that default, so clearing the last entry would arrive as a body with +// no array at all and 400. Entries naming a Team the caller is not in are dropped +// by the model rather than refused here (an ordinary race, not a client bug). +async function putTeamPrefs(req, res) { + try { + const { prefs } = await teamPrefs.replacePrefs(req.user.id, req.body.teams) + return res.json({ teams: prefs }) + } catch (err) { + log.error('putTeamPrefs', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + module.exports = { registerDevice, listDevices, @@ -85,4 +119,6 @@ module.exports = { getStreams, getSubscriptions, putSubscriptions, + getTeamPrefs, + putTeamPrefs, } diff --git a/server/src/router/v1/auth/notifications.routes.js b/server/src/router/v1/auth/notifications.routes.js index 579777f..d3b6731 100644 --- a/server/src/router/v1/auth/notifications.routes.js +++ b/server/src/router/v1/auth/notifications.routes.js @@ -13,6 +13,7 @@ const notif = require('./notifications.controller') const { requireAuth } = require('../../../auth/session.middleware') const noindex = require('../../../middleware/noindex') const validate = require('../../../middleware/validate') +const { EMAIL_MODES } = require('../../../model/teams/teamNotify.model') const notifRouter = express.Router() @@ -97,4 +98,39 @@ notifRouter.put( notif.putSubscriptions, ) +// ── Per-Team preferences (TEAMS.md §6.3, phase 6) ────────────────────────── +// +// The granularity per-stream opt-in cannot express: "I am in five Teams and want +// notifications from one". Opt-OUT for push (no row means notified) and opt-IN +// for email, so a user who never opens this screen is in the state the schema +// documents rather than in one this router has to describe. +notifRouter.get( + '/notifications/teams', + // #swagger.tags = ['Auth · Me'] + // #swagger.summary = 'Get the current user’s per-Team notification preferences' + // #swagger.description = 'One entry per Team the caller could be notified about — active membership or an active forum grant — plus any Team they have a stored preference for. Defaults are applied server-side: `muted` false, `emailMode` "off".' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Per-Team preferences', content: { "application/json": { schema: { $ref: "#/components/schemas/TeamNotificationPrefs" } } } } */ + /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + notif.getTeamPrefs, +) + +notifRouter.put( + '/notifications/teams', + // #swagger.tags = ['Auth · Me'] + // #swagger.summary = 'Replace the current user’s per-Team notification preferences' + // #swagger.description = 'Replaces the whole set. The `teams` array is required even when empty. Entries naming a Team the caller has no access to are ignored; the stored set is echoed back.' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.requestBody = { required: true, content: { "application/json": { schema: { $ref: "#/components/schemas/TeamNotificationPrefs" } } } } */ + /* #swagger.responses[200] = { description: 'Updated preferences', content: { "application/json": { schema: { $ref: "#/components/schemas/TeamNotificationPrefs" } } } } */ + /* #swagger.responses[400] = { description: 'Validation error', content: { "application/json": { schema: { $ref: "#/components/schemas/ValidationError" } } } } */ + /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + body('teams').isArray(), + body('teams.*.teamId').isInt({ min: 1 }), + body('teams.*.muted').optional().isBoolean(), + body('teams.*.emailMode').optional().isIn(EMAIL_MODES), + validate, + notif.putTeamPrefs, +) + module.exports = notifRouter diff --git a/server/src/router/v1/player/teamForum.controller.js b/server/src/router/v1/player/teamForum.controller.js index 204c248..ac3250a 100644 --- a/server/src/router/v1/player/teamForum.controller.js +++ b/server/src/router/v1/player/teamForum.controller.js @@ -26,6 +26,7 @@ const forumSettings = require('../../../model/teams/teamForumSettings.model') const uploads = require('../../../model/teams/teamForumUploads.model') const reports = require('../../../model/reports/contentReports.model') const activity = require('../../../model/activity/activity.model') +const teamNotify = require('../../../utils/teamNotify') const log = require('../../../utils/logger')('teams') @@ -86,6 +87,34 @@ async function viewerFor(ctx, user) { } } +/** + * Fan a new thread or reply out to the Team (TEAMS.md Part 6, phase 6). + * + * **Here rather than in the forum model**, because the model takes an + * already-resolved access decision and reads no membership table by design, and + * the fan-out reads both to compute its recipients. A notification call inside the + * model would make it transitively depend on what its own header says it must not. + * + * **Awaited, and it still cannot fail the request.** `teamNotify.forumPost` catches + * everything and returns; awaiting it costs the response the time of one recipient + * query plus, in `immediate` mode, the SMTP calls — which is why the alternative + * (fire-and-forget) is tempting and wrong here: an un-awaited rejection in an + * Express handler is an unhandled rejection, and the tests would have no moment at + * which to assert the fan-out happened. + */ +async function announce(ctx, actor, notify) { + if (!notify) return + await teamNotify.forumPost({ + team: ctx.team, + threadId: notify.threadId, + threadTitle: notify.title, + type: notify.type, + authorUserId: actor.id, + authorName: actor.username, + bodyHtml: notify.bodyHtml, + }) +} + // ── threads ──────────────────────────────────────────────────────────────── async function listThreads(req, res) { @@ -151,13 +180,14 @@ async function createThread(req, res) { return res.status(403).json({ message: 'Only Team leaders may post announcements' }) } - const result = await forum.createThread({ + const { notify, ...result } = await forum.createThread({ team: ctx.team, actor: req.user, type, title: req.body.title, body: req.body.body, }) + if (result.ok) await announce(ctx, req.user, notify) return send(res, result) } catch (err) { return fail(res, err, 'create thread') @@ -170,12 +200,14 @@ async function createPost(req, res) { const ctx = await resolveForum(req) if (!ctx) return res.status(404).json({ message: 'Not found' }) - return send(res, await forum.createPost({ + const { notify, ...result } = await forum.createPost({ team: ctx.team, threadId: Number(req.params.id), actor: req.user, body: req.body.body, - })) + }) + if (result.ok) await announce(ctx, req.user, notify) + return send(res, result) } catch (err) { return fail(res, err, 'create post') } diff --git a/server/src/router/v1/public/teams.controller.js b/server/src/router/v1/public/teams.controller.js index d2be486..b41825f 100644 --- a/server/src/router/v1/public/teams.controller.js +++ b/server/src/router/v1/public/teams.controller.js @@ -6,6 +6,8 @@ const teams = require('../../../model/teams/teams.model') const teamActivity = require('../../../model/teams/teamActivity.model') +const teamPrefs = require('../../../model/teams/teamNotify.model') +const unsubscribeToken = require('../../../utils/unsubscribeToken') const log = require('../../../utils/logger')('teams') @@ -98,4 +100,52 @@ async function getActivity(req, res) { } } -module.exports = { listTeams, getTeam, getTeamByExternalId, getRoster, getActivity } +/** + * POST /public/teams/unsubscribe/:token — one-click unsubscribe (TEAMS.md §6.4). + * + * **The one write in this tier, and it is unauthenticated on purpose.** A person + * reading their mail is not logged into the site, and an unsubscribe that first + * demands a login is an unsubscribe most people do not complete. The token is what + * stands in for the session, and the capability it carries is deliberately the + * narrowest one that does the job: set `muted` for ONE (user, Team) pair. It reads + * nothing, cannot un-mute, and names no other Team. + * + * **Always 200, whatever the token was.** A response that distinguished a valid + * token from a forged one would turn this into an oracle for which (user, Team) + * pairs exist, on an endpoint with no session behind it. The page says "you will + * not receive further emails about this team" either way, which is true either way. + * + * Reached two ways with the same effect: a mail client's RFC 8058 one-click POST + * (the `List-Unsubscribe-Post` header), and the site's own /unsubscribe page, + * which POSTs here after a human clicks the link in the body. + */ +async function unsubscribe(req, res) { + const claim = unsubscribeToken.verify(req.params.token) + if (claim) { + try { + await teamPrefs.mute(claim.userId, claim.teamId) + } catch (err) { + // Logged, not surfaced. A failed write here is worth an operator's + // attention and is not worth telling an anonymous caller about — and a 500 + // would make a mail client retry a request it should not repeat. + log.error('unsubscribe', err) + } + } + return res.json({ ok: true }) +} + +/** + * GET on the same path — for a mail client that shows the `List-Unsubscribe` URL + * as a link and has no one-click support. + * + * Redirects to the site's own page rather than acting, because a GET must not + * mutate: a link prefetcher or a mail client's link scanner would otherwise + * silently mute Teams nobody asked to leave. The page it lands on does the POST + * once a human is looking at it. + */ +function unsubscribeLanding(req, res) { + const base = (process.env.APP_BASE_URL || 'http://localhost:5173').replace(/\/+$/, '') + return res.redirect(302, `${base}/unsubscribe/${encodeURIComponent(req.params.token)}`) +} + +module.exports = { listTeams, getTeam, getTeamByExternalId, getRoster, getActivity, unsubscribe, unsubscribeLanding } diff --git a/server/src/router/v1/public/teams.router.js b/server/src/router/v1/public/teams.router.js index 5723ad2..118ae81 100644 --- a/server/src/router/v1/public/teams.router.js +++ b/server/src/router/v1/public/teams.router.js @@ -90,4 +90,38 @@ teamsRouter.get( ctrl.getActivity, ) +// ── One-click unsubscribe (TEAMS.md §6.4) ────────────────────────────────── +// +// Declared last, and the shadowing question is worth answering rather than +// assuming: these are two segments, so the one-segment '/:slug' cannot take them, +// and the two-segment '/:slug/members' and '/:slug/activity' both pin a LITERAL +// second segment. Only a token spelled exactly "members" or "activity" could +// collide, and a token is `...`. +// +// No `siteMode`, unlike every other route in this file. An unsubscribe has to work +// while the site is in maintenance: the mail that carried the link went out before +// the site went down, and "we are doing maintenance" is not an answer to "stop +// emailing me". +teamsRouter.post( + '/unsubscribe/:token', + // #swagger.tags = ['Public · Teams'] + // #swagger.summary = 'Unsubscribe from one Team’s notification emails' + // #swagger.description = 'Honours the tokened link in a Team notification email, including RFC 8058 one-click. Sets the same per-Team mute the account screen shows. Always answers 200 — a response that distinguished a valid token from a forged one would be an oracle for which (user, Team) pairs exist.' + // #swagger.parameters['token'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The signed token from the email link.' } + // #swagger.security = [{}] + /* #swagger.responses[200] = { description: 'Acknowledged', content: { "application/json": { schema: { $ref: "#/components/schemas/OkFlag" } } } } */ + ctrl.unsubscribe, +) + +teamsRouter.get( + '/unsubscribe/:token', + // #swagger.tags = ['Public · Teams'] + // #swagger.summary = 'Land a human on the unsubscribe page' + // #swagger.description = 'For mail clients that render the List-Unsubscribe URL as an ordinary link. Redirects to the site’s own confirmation page and changes nothing — a GET must not mutate, or a link scanner would mute Teams nobody asked to leave.' + // #swagger.parameters['token'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The signed token from the email link.' } + // #swagger.security = [{}] + /* #swagger.responses[302] = { description: 'Redirect to the site’s unsubscribe page' } */ + ctrl.unsubscribeLanding, +) + module.exports = teamsRouter diff --git a/server/swagger/swagger-output.json b/server/swagger/swagger-output.json index 8c0e56c..5cdc2dd 100644 --- a/server/swagger/swagger-output.json +++ b/server/swagger/swagger-output.json @@ -8364,6 +8364,114 @@ } } }, + "/api/v1/auth/me/notifications/teams": { + "get": { + "tags": [ + "Auth · Me" + ], + "summary": "Get the current user’s per-Team notification preferences", + "description": "One entry per Team the caller could be notified about — active membership or an active forum grant — plus any Team they have a stored preference for. Defaults are applied server-side: `muted` false, `emailMode` \"off\".", + "responses": { + "200": { + "description": "Per-Team preferences", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/TeamNotificationPrefs" + } + } + } + }, + "401": { + "description": "Not authenticated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Forbidden" + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + }, + "put": { + "tags": [ + "Auth · Me" + ], + "summary": "Replace the current user’s per-Team notification preferences", + "description": "Replaces the whole set. The `teams` array is required even when empty. Entries naming a Team the caller has no access to are ignored; the stored set is echoed back.", + "responses": { + "200": { + "description": "Updated preferences", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/TeamNotificationPrefs" + } + } + } + }, + "400": { + "description": "Validation error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ValidationError" + } + } + } + }, + "401": { + "description": "Not authenticated", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Forbidden" + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/TeamNotificationPrefs" + } + } + } + } + } + }, "/api/v1/auth/me/sessions": { "get": { "tags": [ @@ -12070,6 +12178,67 @@ } } }, + "/api/v1/public/teams/unsubscribe/{token}": { + "post": { + "tags": [ + "Public · Teams" + ], + "summary": "Unsubscribe from one Team’s notification emails", + "description": "Honours the tokened link in a Team notification email, including RFC 8058 one-click. Sets the same per-Team mute the account screen shows. Always answers 200 — a response that distinguished a valid token from a forged one would be an oracle for which (user, Team) pairs exist.", + "parameters": [ + { + "name": "token", + "in": "path", + "required": true, + "schema": { + "type": "string" + }, + "description": "The signed token from the email link." + } + ], + "responses": { + "200": { + "description": "Acknowledged", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/OkFlag" + } + } + } + } + }, + "security": [ + {} + ] + }, + "get": { + "tags": [ + "Public · Teams" + ], + "summary": "Land a human on the unsubscribe page", + "description": "For mail clients that render the List-Unsubscribe URL as an ordinary link. Redirects to the site’s own confirmation page and changes nothing — a GET must not mutate, or a link scanner would mute Teams nobody asked to leave.", + "parameters": [ + { + "name": "token", + "in": "path", + "required": true, + "schema": { + "type": "string" + }, + "description": "The signed token from the email link." + } + ], + "responses": { + "302": { + "description": "Redirect to the site’s unsubscribe page" + } + }, + "security": [ + {} + ] + } + }, "/api/v1/public/teams/{slug}": { "get": { "tags": [ @@ -16081,6 +16250,143 @@ } } }, + "TeamNotificationPref": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "object" + }, + "description": { + "type": "string", + "example": "One Team's notification preference for the current user. Absent fields take the stored defaults: push is opt-OUT (not muted) and email is opt-IN (`off`)." + }, + "properties": { + "type": "object", + "properties": { + "teamId": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "integer" + }, + "example": { + "type": "number", + "example": 3 + } + } + }, + "slug": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "example": { + "type": "string", + "example": "the-silver-hand" + } + } + }, + "name": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "example": { + "type": "string", + "example": "The Silver Hand" + } + } + }, + "archived": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "boolean" + }, + "example": { + "type": "boolean", + "example": false + } + } + }, + "muted": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "boolean" + }, + "example": { + "type": "boolean", + "example": false + } + } + }, + "emailMode": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "string" + }, + "enum": { + "type": "array", + "example": [ + "off", + "digest", + "immediate" + ], + "items": { + "type": "string" + } + }, + "example": { + "type": "string", + "example": "off" + } + } + } + } + } + } + }, + "TeamNotificationPrefs": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "object" + }, + "description": { + "type": "string", + "example": "Per-Team notification preferences (used for both GET and PUT). The `teams` array is required on PUT even when empty." + }, + "properties": { + "type": "object", + "properties": { + "teams": { + "type": "object", + "properties": { + "type": { + "type": "string", + "example": "array" + }, + "items": { + "$ref": "#/components/schemas/TeamNotificationPref" + } + } + } + } + } + } + }, "Appeal": { "type": "object", "properties": { diff --git a/server/swagger/swagger.js b/server/swagger/swagger.js index 6ec430f..62dbc12 100644 --- a/server/swagger/swagger.js +++ b/server/swagger/swagger.js @@ -573,6 +573,25 @@ const doc = { }, }, }, + TeamNotificationPref: { + type: 'object', + description: "One Team's notification preference for the current user. Absent fields take the stored defaults: push is opt-OUT (not muted) and email is opt-IN (`off`).", + properties: { + teamId: { type: 'integer', example: 3 }, + slug: { type: 'string', example: 'the-silver-hand' }, + name: { type: 'string', example: 'The Silver Hand' }, + archived: { type: 'boolean', example: false }, + muted: { type: 'boolean', example: false }, + emailMode: { type: 'string', enum: ['off', 'digest', 'immediate'], example: 'off' }, + }, + }, + TeamNotificationPrefs: { + type: 'object', + description: 'Per-Team notification preferences (used for both GET and PUT). The `teams` array is required on PUT even when empty.', + properties: { + teams: { type: 'array', items: { $ref: '#/components/schemas/TeamNotificationPref' } }, + }, + }, // ── Moderation appeals (Phase 6c/6d) ──────────────────────────────────── Appeal: { type: 'object', -- 2.49.1 From b458c1f46fb9d32b274dbdfc881a5444b54d6542 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 14:35:08 -0500 Subject: [PATCH 4/6] =?UTF-8?q?feat(teams):=20the=20web=20surface=20?= =?UTF-8?q?=E2=80=94=20a=20notifications=20screen=20that=20did=20not=20exi?= =?UTF-8?q?st?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is phase 6's first finding, and it changed the phase's shape. TEAMS.md §6.3 says the per-Team mute list is surfaced "under the existing notification settings screen". There was no such screen. `/auth/me/notifications/*` was built for the Android app in M7 and had ZERO web consumers — a browser could not see the stream catalog or its own subscriptions at all. That is tolerable while push is the only sink, because push needs the app anyway. It is not tolerable for email, whose entire argument is the web-only user who runs neither the app nor Discord, so the sink and the screen to configure it had to ship together. `/account/notifications` carries all three: what to be told about, which Teams, and whether any of it reaches a mailbox — in the order a user actually reasons about them. The mute toggle goes in a THIRD module-declared slot, above the roster, because muting is an action ON the guild page while the feed and forum are content IN it. It renders nothing for a viewer with no preference available, which is a privacy property rather than a tidiness one: whether a preference EXISTS for a Team answers "is this person in it", and the guild page is public. `/unsubscribe/:token` is public and POSTs on mount — the link the user clicked was a GET, and a GET that mutated would be triggered by every mail-client link scanner. Co-Authored-By: Claude --- client/src/App.jsx | 7 + client/src/api/client.js | 20 ++ client/src/main.jsx | 7 + client/src/modules/TeamNotifyToggle.jsx | 102 +++++++ .../src/routes/player/PlayerNotifications.jsx | 268 ++++++++++++++++++ .../src/routes/player/PlayerPortalLayout.jsx | 3 + client/src/routes/player/Unsubscribe.jsx | 69 +++++ client/test/teamNotify.test.js | 87 ++++++ 8 files changed, 563 insertions(+) create mode 100644 client/src/modules/TeamNotifyToggle.jsx create mode 100644 client/src/routes/player/PlayerNotifications.jsx create mode 100644 client/src/routes/player/Unsubscribe.jsx create mode 100644 client/test/teamNotify.test.js diff --git a/client/src/App.jsx b/client/src/App.jsx index a63c557..88ff3f7 100644 --- a/client/src/App.jsx +++ b/client/src/App.jsx @@ -57,6 +57,8 @@ import ResetPassword from './routes/player/ResetPassword.jsx' import AcceptInvite from './routes/player/AcceptInvite.jsx' import PlayerPortalLayout, { PlayerIndex } from './routes/player/PlayerPortalLayout.jsx' import PlayerAccount from './routes/player/PlayerAccount.jsx' +import PlayerNotifications from './routes/player/PlayerNotifications.jsx' +import Unsubscribe from './routes/player/Unsubscribe.jsx' import PlayerAppeals from './routes/player/PlayerAppeals.jsx' export default function App() { @@ -204,6 +206,10 @@ export default function App() { } /> } /> } /> + {/* PUBLIC, and grouped with the other tokened landings above rather + than with the portal below: the person following an unsubscribe + link is reading their mail, not signed in (TEAMS.md §6.4). */} + } /> @@ -218,6 +224,7 @@ export default function App() { } /> } /> } /> + } /> {/* Installed modules' player-portal pages, at /player//…. This group's own routes are absolute (its layout route has no path), so the prefix is written here rather than inherited — the one diff --git a/client/src/api/client.js b/client/src/api/client.js index a8ae529..010d9c7 100644 --- a/client/src/api/client.js +++ b/client/src/api/client.js @@ -187,6 +187,26 @@ export const api = { req(`/player/teams/${encodeURIComponent(slug)}/grants`, { method: 'POST', body }), teamGrantRevoke: (slug, userId) => req(`/player/teams/${encodeURIComponent(slug)}/grants/${userId}`, { method: 'DELETE' }), + + // ----- notifications (TEAMS.md Part 6) ----- + // + // Under /auth/me rather than /player: these are role-agnostic self-service, the + // same rule that put the forum under /player rather than behind a staff gate. + // The streams catalog and the per-stream subscriptions were built for the app + // and had no web consumer at all until phase 6 gave them one. + notificationStreams: () => req('/auth/me/notifications/streams'), + notificationSubscriptions: () => req('/auth/me/notifications/subscriptions'), + // `streams` is always sent, empty array included — the endpoint requires the + // field, so clearing the last subscription must not become an absent key. + setNotificationSubscriptions: (streams) => + req('/auth/me/notifications/subscriptions', { method: 'PUT', body: { streams } }), + teamNotificationPrefs: () => req('/auth/me/notifications/teams'), + setTeamNotificationPrefs: (teams) => + req('/auth/me/notifications/teams', { method: 'PUT', body: { teams } }), + // Unauthenticated, and the one write in the public tier: the caller is reading + // their mail, not signed in. Always resolves 200 whatever the token was. + unsubscribeTeam: (token) => + req(`/public/teams/unsubscribe/${encodeURIComponent(token)}`, { method: 'POST' }), wikiTags: () => req('/public/wiki/tags'), wikiPage: (slug) => req(`/public/wiki/${slug}`), // CMS pages (block-based). Published-only for the public; a draft-preview link diff --git a/client/src/main.jsx b/client/src/main.jsx index 0d26693..4b6cb27 100644 --- a/client/src/main.jsx +++ b/client/src/main.jsx @@ -6,6 +6,7 @@ import { publishSharedDependencies } from './modules/shared.js' import { declareSlot, applyCoreFills, fillModuleSlot } from './modules/registry.js' import TeamActivityFeed from './modules/TeamActivityFeed.jsx' import TeamForumPanel from './modules/TeamForumPanel.jsx' +import TeamNotifyToggle from './modules/TeamNotifyToggle.jsx' import './styles/theme.css' // Publish window.__rg BEFORE rendering and before any module chunk evaluates. @@ -84,6 +85,12 @@ fillModuleSlot('uo.guild.detail', TeamActivityFeed) // exactly as before. fillModuleSlot('uo.guild.forum', TeamForumPanel) +// And the notification control, in a third place the module declares ABOVE its +// roster. A third slot rather than a corner of the feed for the same reason there +// were two: this is an action on the page and the other two are content in it, +// and only the module can say where each belongs on a page it owns. +fillModuleSlot('uo.guild.header', TeamNotifyToggle) + // Render on DOMContentLoaded rather than immediately, and that is the one line // of core's boot the module system changes. // diff --git a/client/src/modules/TeamNotifyToggle.jsx b/client/src/modules/TeamNotifyToggle.jsx new file mode 100644 index 0000000..e48c8e6 --- /dev/null +++ b/client/src/modules/TeamNotifyToggle.jsx @@ -0,0 +1,102 @@ +import { useCallback, useEffect, useState } from 'react' +import { Link } from 'react-router-dom' +import { api } from '../api/client.js' +import { useAuth } from '../contexts/AuthContext.jsx' + +// Core's per-Team notification control, rendered into a THIRD slot a module +// declares (TEAMS.md §6.3, phase 6). +// +// **Why this is a slot at all, and why it is the third one.** Teams have no core +// page — the module that owns the vocabulary owns the page — so a control that +// acts on one Team has nowhere of core's to live. The feed and the forum go below +// the module's roster; this goes above it, because muting a guild is an action ON +// the page rather than more content in it, and that is exactly the placement +// decision a module cannot make if core stacks everything into one fill. +// +// **It renders nothing for a viewer who is not in the Team**, including anonymous +// ones, and that is a privacy property rather than a tidiness one: whether a +// notification preference EXISTS for a Team answers "is this person in it", and +// the guild page is public. The server decides — the preference list only contains +// Teams the caller may be notified about — and this file never infers membership +// from anything it can see on the page. +// +// **Muting is per-Team and covers all four streams.** The per-stream on/off lives +// on the account screen, where the catalog does; the thing that could not be +// expressed before phase 6 is "I am in five Teams and want notifications from +// one", and that is the only question this control asks. + +export default function TeamNotifyToggle({ externalId, moduleId }) { + const { user } = useAuth() + const [state, setState] = useState({ loading: true, team: null, pref: null }) + const [busy, setBusy] = useState(false) + + const load = useCallback(async () => { + // Anonymous viewers never fetch. The endpoint would 401 harmlessly, but a + // guild page rendering a public roster should not put an authenticated + // request on the wire for every visitor. + if (!user) return setState({ loading: false, team: null, pref: null }) + try { + const team = await api.teamByExternalId(moduleId, externalId) + const { teams } = await api.teamNotificationPrefs() + const pref = (teams || []).find((t) => t.teamId === team.id) || null + setState({ loading: false, team, pref }) + } catch { + // Same rule as the feed and the forum: this is core's content on a page + // core does not own, so a failure renders nothing rather than putting an + // error box on somebody else's surface. + setState({ loading: false, team: null, pref: null }) + } + }, [externalId, moduleId, user]) + + useEffect(() => { load() }, [load]) + + const { loading, pref } = state + if (loading || !pref) return null + + async function toggle() { + setBusy(true) + // Optimistic, and reconciled from the server's echo rather than assumed: a + // PUT that silently dropped the entry (a Team left in another tab) must not + // leave the control claiming a state the server does not hold. + const next = { ...pref, muted: !pref.muted } + setState((s) => ({ ...s, pref: next })) + try { + const { teams } = await api.setTeamNotificationPrefs([ + { teamId: pref.teamId, muted: next.muted, emailMode: pref.emailMode }, + ]) + const echoed = (teams || []).find((t) => t.teamId === pref.teamId) + if (echoed) setState((s) => ({ ...s, pref: echoed })) + } catch { + setState((s) => ({ ...s, pref })) + } finally { + setBusy(false) + } + } + + return ( +
+ + + {pref.muted + ? 'You get no notifications about this team.' + : 'You get notifications about this team.'} + + {/* The one link off this control, because "mute" is a blunt answer to a + question the account screen asks properly — which streams, and whether + email is on at all. */} + All notification settings +
+ ) +} diff --git a/client/src/routes/player/PlayerNotifications.jsx b/client/src/routes/player/PlayerNotifications.jsx new file mode 100644 index 0000000..ad9ac9b --- /dev/null +++ b/client/src/routes/player/PlayerNotifications.jsx @@ -0,0 +1,268 @@ +import { useCallback, useEffect, useState } from 'react' +import { Loading, ErrorState } from '../../components/PageState.jsx' +import { api } from '../../api/client.js' + +// The account's notification settings (TEAMS.md §6.3/§6.4, phase 6). +// +// **This screen did not exist before phase 6, and that was the phase's first +// finding.** §6.3 says the per-Team mute list is "surfaced under the existing +// notification settings screen" — there was no such screen on the web. The stream +// catalog and the per-stream subscriptions have been built and shipped since M7, +// with the Android app as their only consumer; a browser could not see them at +// all. That is tolerable for push, which needs the app anyway. It is not tolerable +// for email, whose whole reason for existing (§6.4) is the web-only user who runs +// neither the app nor Discord — so the sink and the screen to configure it had to +// arrive together. +// +// Three blocks, in the order a user actually reasons about them: what kinds of +// thing to be told about, then which Teams, then whether any of it should reach a +// mailbox. + +const EMAIL_MODES = [ + { value: 'off', label: 'No email' }, + { value: 'digest', label: 'Daily digest' }, + { value: 'immediate', label: 'Every post' }, +] + +// Streams whose scoping lives in this page's second block rather than in the +// first. Shown as a group so a user does not toggle `team.forum.post` off site- +// wide when what they meant was "not this one guild". +const isTeamStream = (id) => String(id).startsWith('team.') + +function Section({ title, hint, children }) { + return ( +
+

{title}

+ {hint &&

{hint}

} + {children} +
+ ) +} + +function Note({ msg, error }) { + if (!msg && !error) return null + return ( +

+ {error || msg} +

+ ) +} + +// ── What to be told about ────────────────────────────────────────────────── + +function Streams({ streams, subscribed, onSave, busy, msg, error }) { + const [set, setSet] = useState(() => new Set(subscribed)) + useEffect(() => { setSet(new Set(subscribed)) }, [subscribed]) + + const toggle = (id) => { + const next = new Set(set) + if (next.has(id)) next.delete(id) + else next.add(id) + setSet(next) + } + + const team = streams.filter((s) => isTeamStream(s.id)) + const rest = streams.filter((s) => !isTeamStream(s.id)) + + const row = (s) => ( + + ) + + return ( +
+
{rest.map(row)}
+ {team.length > 0 && ( + <> +

+ Teams +

+
{team.map(row)}
+ + )} +
+ +
+ +
+ ) +} + +// ── Which Teams, and whether by email ────────────────────────────────────── + +function Teams({ teams, onSave, busy, msg, error }) { + const [rows, setRows] = useState(teams) + useEffect(() => { setRows(teams) }, [teams]) + + const patch = (teamId, change) => + setRows((rs) => rs.map((r) => (r.teamId === teamId ? { ...r, ...change } : r))) + + if (rows.length === 0) { + return ( +
+

+ You are not in a team, and nobody has given you access to a team forum. There is nothing to + configure here yet. +

+
+ ) + } + + return ( +
+
+ + + + + + + + + + {rows.map((t) => ( + + + + + + ))} + +
TeamNotificationsEmail
+ {t.name} + {/* An archived Team is still listed when a preference exists for + it, so a mute does not silently vanish when a guild disbands + and reappear if it re-forms under the same name. */} + {t.archived && · archived} + + + + +
+
+
+ +
+ +
+ ) +} + +// ── Page ─────────────────────────────────────────────────────────────────── + +export default function PlayerNotifications() { + const [loading, setLoading] = useState(true) + const [error, setError] = useState('') + const [streams, setStreams] = useState([]) + const [subscribed, setSubscribed] = useState([]) + const [teams, setTeams] = useState([]) + const [saving, setSaving] = useState({ streams: false, teams: false }) + const [notes, setNotes] = useState({ streams: '', teams: '', streamsError: '', teamsError: '' }) + + const load = useCallback(async () => { + setLoading(true) + try { + // Three reads in parallel: the catalog is boot-fixed, the subscriptions and + // the Team list are this user's. None depends on another. + const [cat, subs, prefs] = await Promise.all([ + api.notificationStreams(), + api.notificationSubscriptions(), + api.teamNotificationPrefs(), + ]) + setStreams(cat.streams || []) + setSubscribed(subs.streams || []) + setTeams(prefs.teams || []) + setError('') + } catch { + setError('Could not load your notification settings.') + } finally { + setLoading(false) + } + }, []) + + useEffect(() => { load() }, [load]) + + const saveStreams = useCallback(async (ids) => { + setSaving((s) => ({ ...s, streams: true })) + setNotes((n) => ({ ...n, streams: '', streamsError: '' })) + try { + const { streams: stored } = await api.setNotificationSubscriptions(ids) + setSubscribed(stored || []) + setNotes((n) => ({ ...n, streams: 'Saved.' })) + } catch { + setNotes((n) => ({ ...n, streamsError: 'Could not save that.' })) + } finally { + setSaving((s) => ({ ...s, streams: false })) + } + }, []) + + const saveTeams = useCallback(async (rows) => { + setSaving((s) => ({ ...s, teams: true })) + setNotes((n) => ({ ...n, teams: '', teamsError: '' })) + try { + // The whole set, every time, and the array is sent even when empty — the + // endpoint requires the field (docs/android/PLAN.md §11). + const { teams: stored } = await api.setTeamNotificationPrefs( + rows.map((t) => ({ teamId: t.teamId, muted: t.muted, emailMode: t.emailMode })), + ) + setTeams(stored || []) + setNotes((n) => ({ ...n, teams: 'Saved.' })) + } catch { + setNotes((n) => ({ ...n, teamsError: 'Could not save that.' })) + } finally { + setSaving((s) => ({ ...s, teams: false })) + } + }, []) + + if (loading) return + if (error) return + + return ( +
+

+ Choose what you are told about, and how. Nothing here is on by default except team + notifications to the app, which you can mute per team below. +

+ + +
+ ) +} diff --git a/client/src/routes/player/PlayerPortalLayout.jsx b/client/src/routes/player/PlayerPortalLayout.jsx index 12e9153..35c3312 100644 --- a/client/src/routes/player/PlayerPortalLayout.jsx +++ b/client/src/routes/player/PlayerPortalLayout.jsx @@ -35,6 +35,7 @@ function Icon({ children, size = 16 }) { } const IconGear = () => const IconShield = () => +const IconBell = () => // Exported because Admin -> Navigation edits this list. It stays declared here; // the editor may only relabel, reorder and hide what it finds (§7). No CORE row @@ -47,6 +48,7 @@ const IconShield = () => + + {state === 'working' &&

One moment…

} + {state === 'done' && ( + <> +

+ You will not receive further notification emails about this team. +

+

+ This muted the team rather than switching off your account’s email, so your other + teams are unaffected. You can turn it back on any time under{' '} + notification settings. +

+ + )} + {state === 'failed' && ( +

+ We could not reach the site to record that. Please try the link again, or change the + setting yourself under notification settings. +

+ )} + + ) +} diff --git a/client/test/teamNotify.test.js b/client/test/teamNotify.test.js new file mode 100644 index 0000000..fb39a0a --- /dev/null +++ b/client/test/teamNotify.test.js @@ -0,0 +1,87 @@ +import { test, beforeEach, afterEach } from 'node:test' +import assert from 'node:assert/strict' +import { api } from '../src/api/client.js' + +// The client half of Team notifications (docs/website/TEAMS.md Part 6, phase 6). +// +// There is no DOM in this runner, so what is asserted here is the WIRE — which is +// where this feature's client-side mistakes actually live. Two of them have +// already been made once in this repo and are recorded rather than re-derived: +// +// 1. **A PUT-the-whole-set body must always carry its array**, empty included. +// `docs/android/PLAN.md` §11: a DTO field with a default is dropped by +// kotlinx when it equals that default, so "clear the last entry" arrives as a +// body with no array at all and 400s. The web client has no such +// serialisation quirk, but it shares the endpoint's contract, and a test that +// pins the shape here is what keeps the two clients honest about the same +// rule. +// 2. **The unsubscribe call is a POST**, not the GET the link in the mail was. +// A GET that mutated would be triggered by every mail-client link scanner. + +let calls +const realFetch = global.fetch + +function reply(body = {}) { + return { + ok: true, + status: 200, + statusText: 'OK', + text: async () => JSON.stringify(body), + } +} + +beforeEach(() => { + calls = [] + global.fetch = async (url, opts = {}) => { + calls.push({ url, opts }) + return reply({ teams: [], streams: [], ok: true }) + } +}) + +afterEach(() => { global.fetch = realFetch }) + +const body = (i = 0) => JSON.parse(calls[i].opts.body) + +test('the per-Team preference endpoints sit under /auth/me, not /player', async () => { + await api.teamNotificationPrefs() + // Role-agnostic self-service, the same rule that put the Team forum under + // /player rather than behind a staff gate: staff are a superset of players and + // manage their own notifications like anyone else. + assert.match(calls[0].url, /\/auth\/me\/notifications\/teams$/) + assert.equal(calls[0].opts.method ?? 'GET', 'GET') +}) + +test('saving preferences PUTs the whole set under a `teams` key', async () => { + await api.setTeamNotificationPrefs([{ teamId: 3, muted: true, emailMode: 'digest' }]) + assert.equal(calls[0].opts.method, 'PUT') + assert.deepEqual(body(), { teams: [{ teamId: 3, muted: true, emailMode: 'digest' }] }) +}) + +test('clearing every preference still sends the array, never an absent key', async () => { + await api.setTeamNotificationPrefs([]) + assert.deepEqual(body(), { teams: [] }) + assert.equal('teams' in body(), true) +}) + +test('the same rule holds for the stream subscriptions beside them', async () => { + await api.setNotificationSubscriptions([]) + assert.deepEqual(body(), { streams: [] }) +}) + +test('unsubscribe is a POST to the public tier, with the token encoded into the path', async () => { + await api.unsubscribeTeam('1.7.3.abcDEF') + assert.equal(calls[0].opts.method, 'POST') + assert.match(calls[0].url, /\/public\/teams\/unsubscribe\/1\.7\.3\.abcDEF$/) +}) + +test('a token with url-unsafe characters is encoded rather than pasted in', async () => { + await api.unsubscribeTeam('a/b c') + assert.match(calls[0].url, /unsubscribe\/a%2Fb%20c$/) +}) + +test('the streams catalog and subscriptions are separate reads', async () => { + await api.notificationStreams() + await api.notificationSubscriptions() + assert.match(calls[0].url, /\/notifications\/streams$/) + assert.match(calls[1].url, /\/notifications\/subscriptions$/) +}) -- 2.49.1 From 5fa88baa0a072f026cc8f142de5c5709ec18bb37 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 14:35:23 -0500 Subject: [PATCH 5/6] test(teams): the refusals, which is most of what a notification feature is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A notification feature is mostly things that correctly do NOT happen, and each of these is invisible until it goes wrong in production: a departed member and a revoked guest are not recipients; a mute subtracts per Team and leaves the user's other Teams alone; the author of a post never receives the notification about it; forums switched off silences the forum streams including the digest; a Team's first roster wakes nobody; a failed send does not stamp `last_digest_at`. Two real defects came out of writing them. `Number(null)` is 0 and 0 is an integer, so a null in a caller's id list survived `filter(Number.isInteger)` and rode into an IN clause as user id 0. No row has id 0, so it was harmless — which is exactly why it would never have been noticed. Fixed in all three places that filter ids. `recipientIds: db.recipientIds` in the model captured the function OBJECT at require time, so the layer below could never be substituted. That is not only untestable; it means the model was not really the seam it claimed to be. Wrapped so `db.x` resolves at call time. The registries catalog assertion is now an exact five-element list, so a shard-content stream creeping back into core's registration fails here rather than shipping. Co-Authored-By: Claude --- server/test/moduleRegistries.test.js | 15 +- server/test/notificationsRoutes.test.js | 42 ++++ server/test/pushDispatch.test.js | 50 +++++ server/test/teamNotify.test.js | 259 ++++++++++++++++++++++ server/test/teamNotifyDispatch.test.js | 281 ++++++++++++++++++++++++ server/test/teamProvider.test.js | 33 +++ server/test/teamSync.test.js | 41 ++++ 7 files changed, 719 insertions(+), 2 deletions(-) create mode 100644 server/test/teamNotify.test.js create mode 100644 server/test/teamNotifyDispatch.test.js diff --git a/server/test/moduleRegistries.test.js b/server/test/moduleRegistries.test.js index bade2b5..e659321 100644 --- a/server/test/moduleRegistries.test.js +++ b/server/test/moduleRegistries.test.js @@ -44,11 +44,22 @@ function tryApply(owner, build) { test('registerCore registers exactly what core owns, and nothing else', () => { registries.registerCore() - // One stream, one leg, no filled slot. Before Phase 3 this was eight streams, + // Five streams, one leg, no filled slot. Before Phase 3 this was eight streams, // two legs and a core-filled `admin.users.detail` — core was holding shard // CONTENT so the seam would be exercised on every boot before a module first // used it. module-uo registers all of it now, through the same door. - assert.deepEqual(registries.allStreams().map((s) => s.id), ['news.post']) + // + // The four `team.*` streams arrived with Teams phase 6 and ARE core's: a module + // supplies who is in a Team, but who may be told about it is the access + // resolver's answer. Asserted as an exact list so a shard-content stream + // creeping back into core's registration fails here rather than shipping. + assert.deepEqual(registries.allStreams().map((s) => s.id), [ + 'news.post', + 'team.member.joined', + 'team.leadership.changed', + 'team.forum.post', + 'team.announcement', + ]) assert.deepEqual(registries.announceLegIds(), ['discord']) assert.equal(registries.slotFilledBy('admin.users.detail'), null) assert.equal(registries.isCoreRegistered(), true) diff --git a/server/test/notificationsRoutes.test.js b/server/test/notificationsRoutes.test.js index adff96e..042a5c9 100644 --- a/server/test/notificationsRoutes.test.js +++ b/server/test/notificationsRoutes.test.js @@ -25,6 +25,11 @@ test('/auth/me push routes reject unauthenticated callers with 401', async () => ['GET', '/api/v1/auth/me/notifications/streams'], ['GET', '/api/v1/auth/me/notifications/subscriptions'], ['PUT', '/api/v1/auth/me/notifications/subscriptions', { streams: ['news.post'] }], + // Phase 6's two. Per-Team preferences are as much a private fact as the + // subscriptions beside them — the LIST of Teams a user may be notified + // about answers "which guilds is this person in". + ['GET', '/api/v1/auth/me/notifications/teams'], + ['PUT', '/api/v1/auth/me/notifications/teams', { teams: [] }], ] for (const [method, path, body] of calls) { const res = await fetch(app.url + path, { @@ -38,3 +43,40 @@ test('/auth/me push routes reject unauthenticated callers with 401', async () => await app.close() } }) + +// ── The unsubscribe endpoint (TEAMS.md §6.4) ─────────────────────────────── +// +// The mirror image of every test above: this one is reached WITHOUT a session, on +// purpose, because the person following it is reading their mail. So the things +// worth asserting are that it is mounted unauthenticated, that it says the same +// thing whatever the token was, and that the GET twin does not write. + +const publicRouter = require('../src/router/v1/public') + +test('unsubscribe answers 200 unauthenticated, and says nothing about the token', async () => { + const app = await startApp((a) => a.use('/api/v1/public', publicRouter)) + try { + // A forged token and a well-formed one must be indistinguishable from the + // outside — otherwise this is an oracle for which (user, Team) pairs exist. + for (const token of ['1.1.1.AAAAAAAAAAAAAAAAAAAAAA', 'total-nonsense']) { + const res = await fetch(`${app.url}/api/v1/public/teams/unsubscribe/${token}`, { method: 'POST' }) + assert.equal(res.status, 200) + assert.deepEqual(await res.json(), { ok: true }) + } + } finally { + await app.close() + } +}) + +test('a GET on the same path redirects and does not act', async () => { + const app = await startApp((a) => a.use('/api/v1/public', publicRouter)) + try { + const res = await fetch(`${app.url}/api/v1/public/teams/unsubscribe/1.1.1.AAAAAAAAAAAAAAAAAAAAAA`, { + redirect: 'manual', + }) + assert.equal(res.status, 302) + assert.match(res.headers.get('location') || '', /\/unsubscribe\//) + } finally { + await app.close() + } +}) diff --git a/server/test/pushDispatch.test.js b/server/test/pushDispatch.test.js index 1b4542c..29b4896 100644 --- a/server/test/pushDispatch.test.js +++ b/server/test/pushDispatch.test.js @@ -126,3 +126,53 @@ test('publish skips endpoints that fail the SSRF guard', async () => { assert.equal(calls[0].url, 'https://relay.test/ok') }) }) + +// ── publishToUsers — the third fan-out shape (TEAMS.md §6.2) ─────────────── +// +// `publish` answers "everyone subscribed" and "this one owner". Team +// notifications need "these N users", because the four `team.*` streams are +// global and which Team an event belongs to lives in the SET, not the stream id. + +test('publishToUsers tickles the given set, and asks for exactly that set', async () => { + await withEnv({ NTFY_BASE_URL: undefined, NTFY_ALLOWED_ORIGINS: undefined }, async () => { + const { calls, fetchImpl } = captureFetch() + const asked = [] + const pushDevices = { + endpointsForUsersStream: async (ids, stream) => { + asked.push({ ids, stream }) + return [{ endpoint: 'https://relay.test/a' }, { endpoint: 'https://relay.test/b' }] + }, + } + await pushDispatch.publishToUsers('team.forum.post', { ref: 'team:1:thread:7', userIds: [4, 9] }, { pushDevices, fetchImpl }) + assert.deepEqual(asked, [{ ids: [4, 9], stream: 'team.forum.post' }]) + assert.equal(calls.length, 2) + assert.deepEqual(JSON.parse(calls[0].opts.body), { stream: 'team.forum.post', ref: 'team:1:thread:7' }) + }) +}) + +test('publishToUsers with an empty set never touches the database', async () => { + const { calls, fetchImpl } = captureFetch() + let looked = false + const pushDevices = { endpointsForUsersStream: async () => { looked = true; return [] } } + await pushDispatch.publishToUsers('team.forum.post', { ref: 'x', userIds: [] }, { pushDevices, fetchImpl }) + assert.equal(looked, false, 'an empty IN () is a syntax error, so the query must not be made at all') + assert.equal(calls.length, 0) +}) + +test('publishToUsers de-duplicates and drops non-numeric ids', async () => { + await withEnv({ NTFY_BASE_URL: undefined, NTFY_ALLOWED_ORIGINS: undefined }, async () => { + const { fetchImpl } = captureFetch() + const asked = [] + const pushDevices = { + endpointsForUsersStream: async (ids) => { asked.push(ids); return [] }, + } + await pushDispatch.publishToUsers('team.forum.post', { ref: 'x', userIds: [4, 4, null, 'nope', 9] }, { pushDevices, fetchImpl }) + assert.deepEqual(asked, [[4, 9]]) + }) +}) + +test('publishToUsers never throws when the lookup fails', async () => { + const { fetchImpl } = captureFetch() + const pushDevices = { endpointsForUsersStream: async () => { throw new Error('down') } } + await pushDispatch.publishToUsers('team.forum.post', { ref: 'x', userIds: [1] }, { pushDevices, fetchImpl }) +}) diff --git a/server/test/teamNotify.test.js b/server/test/teamNotify.test.js new file mode 100644 index 0000000..303d6aa --- /dev/null +++ b/server/test/teamNotify.test.js @@ -0,0 +1,259 @@ +// Team notifications (docs/website/TEAMS.md Part 6, phase 6). +// +// The db layer is stubbed and in-memory tables stand in for `team_members`, +// `team_forum_grants` and `team_notification_prefs`, so these are assertions +// about the RULES rather than about SQL. Five are worth protecting because each +// one is a leak or a nuisance if it is "simplified" away: +// +// 1. a revoked guest and a departed member are NOT recipients — the fan-out +// asks the same two conditions the access resolver asks, and a fan-out that +// forgot one would mail a private forum to somebody who was removed from it; +// 2. a mute subtracts from the recipient set, per Team, and does not touch the +// user's other Teams; +// 3. the author of a post is never a recipient of the notification about it; +// 4. email is opt-IN (`off` unless chosen) while push is opt-OUT, which is the +// one asymmetry in the whole feature and the one most likely to be +// "tidied up" into a single default; +// 5. an unsubscribe token is honoured for exactly the pair it was signed for, +// and forging one is not distinguishable from failing. +const { test, beforeEach, afterEach } = require('node:test') +const assert = require('node:assert/strict') + +const notifyDb = require('../src/model/teams/teamNotify.db') +const notifyModel = require('../src/model/teams/teamNotify.model') +const unsubscribeToken = require('../src/utils/unsubscribeToken') + +const saved = new Map() + +function patch(mod, name, fn) { + if (!saved.has(mod)) saved.set(mod, new Map()) + if (!saved.get(mod).has(name)) saved.get(mod).set(name, mod[name]) + mod[name] = fn +} + +function restore() { + for (const [mod, names] of saved) for (const [name, fn] of names) mod[name] = fn + saved.clear() +} + +// ── The in-memory stand-in ───────────────────────────────────────────────── +// +// Modelled on the three real tables rather than on the queries, so a rule the SQL +// gets wrong is a rule this gets wrong too. `members` and `grants` carry their +// status columns for exactly that reason: the interesting cases are the rows that +// exist and do not count. +let store + +function stub() { + store = { + teams: [ + { id: 1, slug: 'silver-hand', name: 'The Silver Hand', display_name_override: null, status: 'active' }, + { id: 2, slug: 'iron-few', name: 'The Iron Few', display_name_override: null, status: 'active' }, + ], + members: [ + { team_id: 1, user_id: 10, status: 'active' }, + { team_id: 1, user_id: 11, status: 'active' }, + { team_id: 1, user_id: 12, status: 'departed' }, // left the guild + { team_id: 1, user_id: null, status: 'active' }, // unlinked character + { team_id: 2, user_id: 10, status: 'active' }, + ], + grants: [ + { team_id: 1, user_id: 20, revoked_at: null }, // a forum guest + { team_id: 1, user_id: 21, revoked_at: '2026-08-01' }, // revoked + ], + users: [ + { id: 10, username: 'ten', email: 'ten@example.test', status: 'active' }, + { id: 11, username: 'eleven', email: null, status: 'active' }, + { id: 20, username: 'twenty', email: 'twenty@example.test', status: 'active' }, + { id: 30, username: 'banned', email: 'banned@example.test', status: 'banned' }, + ], + prefs: [], // { user_id, team_id, muted, email_mode, last_digest_at } + } + + const prefFor = (userId, teamId) => + store.prefs.find((p) => p.user_id === userId && p.team_id === teamId) + + // The union the real RECIPIENT_UNION builds, with the same two conditions. + const union = (teamId) => { + const ids = new Set() + for (const m of store.members) { + if (m.team_id === teamId && m.status === 'active' && m.user_id != null) ids.add(m.user_id) + } + for (const g of store.grants) { + if (g.team_id === teamId && g.revoked_at == null) ids.add(g.user_id) + } + return [...ids] + } + + patch(notifyDb, 'recipientIds', async (teamId, { exclude = [] } = {}) => + union(teamId) + .filter((id) => !exclude.includes(id)) + .filter((id) => !(prefFor(id, teamId)?.muted))) + + patch(notifyDb, 'emailRecipients', async (teamId, { exclude = [] } = {}) => + union(teamId) + .filter((id) => !exclude.includes(id)) + .filter((id) => !(prefFor(id, teamId)?.muted)) + .map((id) => ({ id, user: store.users.find((u) => u.id === id) })) + .filter(({ user }) => user && user.email && user.status === 'active') + .map(({ id, user }) => ({ + user_id: id, + username: user.username, + email: user.email, + email_mode: prefFor(id, teamId)?.email_mode || 'off', + last_digest_at: prefFor(id, teamId)?.last_digest_at || null, + }))) + + patch(notifyDb, 'prefsForUser', async (userId) => + store.teams + .filter((t) => union(t.id).includes(userId) || prefFor(userId, t.id)) + .map((t) => ({ + team_id: t.id, + slug: t.slug, + name: t.name, + display_name_override: t.display_name_override, + team_status: t.status, + muted: prefFor(userId, t.id)?.muted ? 1 : 0, + email_mode: prefFor(userId, t.id)?.email_mode || 'off', + }))) + + patch(notifyDb, 'prefFor', async (userId, teamId) => prefFor(userId, teamId)) + + patch(notifyDb, 'setPref', async (userId, teamId, { muted, emailMode }) => { + let row = prefFor(userId, teamId) + if (!row) { + row = { user_id: userId, team_id: teamId, muted: 0, email_mode: 'off', last_digest_at: null } + store.prefs.push(row) + } + // Only the columns the caller named, exactly as the ON DUPLICATE KEY clause + // does — this is the property the unsubscribe path depends on. + if (muted != null) row.muted = muted ? 1 : 0 + if (emailMode != null) row.email_mode = emailMode + }) +} + +beforeEach(stub) +afterEach(restore) + +// ── 1. Access, and the rows that exist but do not count ──────────────────── + +test('recipients are active linked members plus active grants, and nobody else', async () => { + const ids = await notifyModel.recipientIds(1) + assert.deepEqual(ids.sort((a, b) => a - b), [10, 11, 20]) +}) + +test('a departed member and a revoked guest are not recipients', async () => { + const ids = await notifyModel.recipientIds(1) + assert.equal(ids.includes(12), false, 'a departed member is still in the table and must not be notified') + assert.equal(ids.includes(21), false, 'a revoked grant is still in the ledger and must not be notified') +}) + +test('an unlinked member contributes no recipient rather than a null one', async () => { + const ids = await notifyModel.recipientIds(1) + assert.equal(ids.includes(null), false) + assert.equal(ids.every((id) => Number.isInteger(id)), true) +}) + +// ── 2. Mutes ─────────────────────────────────────────────────────────────── + +test('a mute removes that user from that Team only', async () => { + await notifyModel.mute(10, 1) + assert.deepEqual((await notifyModel.recipientIds(1)).sort((a, b) => a - b), [11, 20]) + assert.deepEqual(await notifyModel.recipientIds(2), [10], 'the same user is untouched in another Team') +}) + +test('unmuting puts them back', async () => { + await notifyModel.mute(10, 1) + await notifyModel.unmute(10, 1) + assert.equal((await notifyModel.recipientIds(1)).includes(10), true) +}) + +test('a mute does not disturb an email mode the user chose', async () => { + await notifyModel.replacePrefs(10, [{ teamId: 1, muted: false, emailMode: 'immediate' }]) + await notifyModel.mute(10, 1) + const pref = await notifyModel.prefFor(10, 1) + assert.equal(pref.muted, true) + assert.equal(pref.emailMode, 'immediate', 'unsubscribing from one email must not silently rewrite the mode') +}) + +// ── 3. The author ────────────────────────────────────────────────────────── + +test('the author of a post is excluded from the notification about it', async () => { + const ids = await notifyModel.recipientIds(1, { exclude: [10] }) + assert.equal(ids.includes(10), false) + assert.deepEqual(ids.sort((a, b) => a - b), [11, 20]) +}) + +// ── 4. The two defaults, which point opposite ways ───────────────────────── + +test('push is opt-OUT: a user who has never configured anything is a recipient', async () => { + assert.equal(store.prefs.length, 0) + assert.equal((await notifyModel.recipientIds(1)).includes(10), true) +}) + +test('email is opt-IN: the same user is in no email mode until they pick one', async () => { + const rows = await notifyModel.emailRecipients(1) + assert.equal(rows.every((r) => r.email_mode === 'off'), true) + assert.equal(rows.filter((r) => r.email_mode === 'digest' || r.email_mode === 'immediate').length, 0) +}) + +test('a user with no email address is a push recipient and not an email one', async () => { + assert.equal((await notifyModel.recipientIds(1)).includes(11), true) + assert.equal((await notifyModel.emailRecipients(1)).some((r) => r.user_id === 11), false) +}) + +// ── 5. Preferences a caller may write ────────────────────────────────────── + +test('replacePrefs ignores a Team the caller is not in', async () => { + // User 20 holds a grant on Team 1 and has nothing at all on Team 2. + const { written } = await notifyModel.replacePrefs(20, [ + { teamId: 1, muted: true, emailMode: 'digest' }, + { teamId: 2, muted: true, emailMode: 'digest' }, + ]) + assert.deepEqual(written, [1]) + assert.equal(store.prefs.some((p) => p.user_id === 20 && p.team_id === 2), false) +}) + +test('an unknown email mode falls back to off rather than reaching the column', async () => { + await notifyModel.replacePrefs(10, [{ teamId: 1, muted: false, emailMode: 'hourly' }]) + assert.equal((await notifyModel.prefFor(10, 1)).emailMode, 'off') +}) + +test('listPrefs offers a Team the user has never configured', async () => { + const prefs = await notifyModel.listPrefs(10) + assert.deepEqual(prefs.map((p) => p.teamId).sort(), [1, 2]) + assert.equal(prefs.every((p) => p.muted === false && p.emailMode === 'off'), true) +}) + +test('listPrefs names a Team by its display-name override when staff set one', async () => { + store.teams[0].display_name_override = 'A Renamed Guild' + const prefs = await notifyModel.listPrefs(10) + assert.equal(prefs.find((p) => p.teamId === 1).name, 'A Renamed Guild') +}) + +// ── 6. The unsubscribe token ─────────────────────────────────────────────── + +test('a token verifies for exactly the pair it was signed for', () => { + const token = unsubscribeToken.sign(10, 1) + assert.deepEqual(unsubscribeToken.verify(token), { userId: 10, teamId: 1 }) +}) + +test('editing the ids in a token invalidates it — the mac covers them', () => { + const token = unsubscribeToken.sign(10, 1) + const [v, uid, tid, mac] = token.split('.') + assert.equal(unsubscribeToken.verify(`${v}.99.${tid}.${mac}`), null) + assert.equal(unsubscribeToken.verify(`${v}.${uid}.99.${mac}`), null) +}) + +test('a garbage token and a well-formed forgery both verify as null', () => { + assert.equal(unsubscribeToken.verify('nonsense'), null) + assert.equal(unsubscribeToken.verify(''), null) + assert.equal(unsubscribeToken.verify(null), null) + assert.equal(unsubscribeToken.verify('1.10.1.AAAAAAAAAAAAAAAAAAAAAA'), null) +}) + +test('a version bump is what invalidates every outstanding link at once', () => { + const token = unsubscribeToken.sign(10, 1) + const [, uid, tid, mac] = token.split('.') + assert.equal(unsubscribeToken.verify(`${unsubscribeToken.VERSION + 1}.${uid}.${tid}.${mac}`), null) +}) diff --git a/server/test/teamNotifyDispatch.test.js b/server/test/teamNotifyDispatch.test.js new file mode 100644 index 0000000..11ae236 --- /dev/null +++ b/server/test/teamNotifyDispatch.test.js @@ -0,0 +1,281 @@ +// The Team notification fan-out and the digest worker (TEAMS.md §6.2/§6.4). +// +// The layer above teamNotify.test.js: that one asserts WHO a recipient set +// contains, this one asserts what actually happens to them — which stream fires, +// what a mail carries, and the four ways a notification is correctly suppressed. +// +// The suppressions are the point. A notification feature is mostly refusals, and +// each of these is one that would be invisible until it went wrong in production: +// +// • forums switched off silences forum notifications, including the digest; +// • no email configured means the sink is absent, not broken; +// • a Team's FIRST roster does not wake 155 phones; +// • a failed send does not stamp `last_digest_at`, so the window is retried +// rather than silently skipped. +const { test, beforeEach, afterEach } = require('node:test') +const assert = require('node:assert/strict') + +const notify = require('../src/utils/teamNotify') +const digest = require('../src/utils/teamDigestWorker') +const pushDispatch = require('../src/utils/pushDispatch') +const mailer = require('../src/utils/mailer') +const forumSettings = require('../src/model/teams/teamForumSettings.model') +const notifyModel = require('../src/model/teams/teamNotify.model') +const registries = require('../src/modules/registries') + +const saved = new Map() + +function patch(mod, name, fn) { + if (!saved.has(mod)) saved.set(mod, new Map()) + if (!saved.get(mod).has(name)) saved.get(mod).set(name, mod[name]) + mod[name] = fn +} + +function restore() { + for (const [mod, names] of saved) for (const [name, fn] of names) mod[name] = fn + saved.clear() +} + +const TEAM = { id: 1, slug: 'silver-hand', name: 'The Silver Hand', external_id: 'g1', display_name_override: null } + +let sent // tickles +let mails // emails +let world + +function stub({ forumsEnabled = true, emailConfigured = true, recipients = [10, 11], emailRows = [] } = {}) { + sent = [] + mails = [] + world = { stamped: [] } + + patch(forumSettings, 'forumsEnabled', async () => forumsEnabled) + patch(mailer, 'isConfigured', async () => emailConfigured) + patch(mailer, 'sendTeamNotification', async (msg) => { + mails.push(msg) + return { sent: true } + }) + patch(pushDispatch, 'publishToUsers', async (streamId, payload) => { sent.push({ streamId, ...payload }) }) + patch(notifyModel, 'recipientIds', async (teamId, { exclude = [] } = {}) => + recipients.filter((id) => !exclude.includes(id))) + patch(notifyModel, 'emailRecipients', async (teamId, { exclude = [] } = {}) => + emailRows.filter((r) => !exclude.includes(r.user_id))) + patch(notifyModel, 'stampDigest', async (userId, teamId, at) => { world.stamped.push({ userId, teamId, at }) }) + // No module registered: the default in most tests, so the link-building ones + // have to opt in and the absence is exercised rather than assumed. + patch(registries, 'registeredTeamProvider', () => null) +} + +beforeEach(() => stub()) +afterEach(restore) + +// ── Which stream fires ───────────────────────────────────────────────────── + +test('a discussion reply fires team.forum.post; an announcement fires its own stream', async () => { + await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + await notify.forumPost({ team: TEAM, threadId: 8, threadTitle: 'Notice', type: 'announcement', authorUserId: 10 }) + assert.deepEqual(sent.map((s) => s.streamId), ['team.forum.post', 'team.announcement']) +}) + +test('the tickle is content-free and refs the thread, never the body', async () => { + await notify.forumPost({ + team: TEAM, threadId: 7, threadTitle: 'Secret plans', type: 'discussion', authorUserId: 99, + bodyHtml: '

the actual private text

', + }) + assert.deepEqual(Object.keys(sent[0]).sort(), ['ref', 'streamId', 'userIds']) + assert.equal(sent[0].ref, 'team:1:thread:7') + assert.equal(JSON.stringify(sent[0]).includes('private text'), false) +}) + +test('the author is not among the tickled', async () => { + await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + assert.deepEqual(sent[0].userIds, [11]) +}) + +test('roster events fire one tickle for the run, not one per member', async () => { + await notify.memberJoined(TEAM) + await notify.leadershipChanged(TEAM) + assert.deepEqual(sent.map((s) => s.streamId), ['team.member.joined', 'team.leadership.changed']) + assert.equal(sent.every((s) => s.ref === 'team:1'), true) +}) + +// ── Suppression ──────────────────────────────────────────────────────────── + +test('forums switched off silences a forum notification entirely', async () => { + stub({ forumsEnabled: false }) + const res = await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + assert.deepEqual(res, { push: 0, emails: 0 }) + assert.equal(sent.length, 0) +}) + +test('a roster tickle survives forums being off — it is not forum content', async () => { + stub({ forumsEnabled: false }) + await notify.memberJoined(TEAM) + assert.equal(sent.length, 1) +}) + +test('no recipients means no publish call at all', async () => { + stub({ recipients: [] }) + await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + assert.equal(sent.length, 0) +}) + +test('a fan-out never throws, whatever the layer below does', async () => { + patch(notifyModel, 'recipientIds', async () => { throw new Error('database is on fire') }) + const res = await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + assert.deepEqual(res, { push: 0, emails: 0 }) + assert.equal(await notify.memberJoined(TEAM), 0) +}) + +// ── Email, the immediate mode ────────────────────────────────────────────── + +const IMMEDIATE = [{ user_id: 11, username: 'eleven', email: 'e@example.test', email_mode: 'immediate' }] + +test('only the immediate-mode recipients are emailed per event', async () => { + stub({ + emailRows: [ + ...IMMEDIATE, + { user_id: 12, username: 'twelve', email: 'd@example.test', email_mode: 'digest' }, + { user_id: 13, username: 'thirteen', email: 'o@example.test', email_mode: 'off' }, + ], + }) + await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10, bodyHtml: '

hello

' }) + assert.deepEqual(mails.map((m) => m.to), ['e@example.test']) +}) + +test('an email carries an excerpt and never the whole post', async () => { + stub({ emailRows: IMMEDIATE }) + const long = `

${'x'.repeat(500)}

` + await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10, bodyHtml: long }) + const body = mails[0].items[0].excerpt + assert.equal(body.length < 250, true) + assert.equal(body.endsWith('…'), true) +}) + +test('no email configured means no send and no recipient query', async () => { + stub({ emailConfigured: false, emailRows: IMMEDIATE }) + const res = await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + assert.equal(res.emails, 0) + assert.equal(mails.length, 0) +}) + +test('every email carries an unsubscribe url for that recipient and that Team', async () => { + stub({ emailRows: IMMEDIATE }) + await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + assert.equal(typeof mails[0].unsubscribeUrl, 'string') + // The header one is an API endpoint (a one-click client POSTs to it without + // rendering anything); the body one is the site's page. They must differ. + assert.notEqual(mails[0].unsubscribeUrl, mails[0].unsubscribeApiUrl) + assert.match(mails[0].unsubscribeApiUrl, /\/api\/v1\/public\/teams\/unsubscribe\//) +}) + +// ── Links, and the module that supplies them ─────────────────────────────── + +test('with no module-supplied template there is no Team link, and nothing breaks', async () => { + assert.equal(notify.teamPageUrl(TEAM), null) + assert.equal(notify.threadUrl(TEAM, 7), null) +}) + +test('a registered template becomes an absolute link, and a thread deep-links by search param', () => { + patch(registries, 'registeredTeamProvider', () => ({ pageUrlTemplate: '/uo/guilds/{externalId}' })) + assert.match(notify.teamPageUrl(TEAM), /\/uo\/guilds\/g1$/) + assert.match(notify.threadUrl(TEAM, 7), /\/uo\/guilds\/g1\?thread=7$/) +}) + +test('a Team is labelled by its display-name override where staff set one', () => { + assert.equal(notify.teamLabel(TEAM), 'The Silver Hand') + assert.equal(notify.teamLabel({ ...TEAM, display_name_override: 'Renamed' }), 'Renamed') +}) + +// ── The digest worker ────────────────────────────────────────────────────── + +const DIGEST_ROW = { user_id: 11, username: 'eleven', email: 'e@example.test', email_mode: 'digest', last_digest_at: null } + +function stubDigest({ posts = [], teams = [TEAM], rows = [DIGEST_ROW], sendOk = true } = {}) { + patch(notifyModel, 'teamsWithForumActivitySince', async () => teams) + patch(notifyModel, 'emailRecipients', async () => rows) + patch(notifyModel, 'digestPostsSince', async () => posts) + patch(mailer, 'sendTeamNotification', async (msg) => { + mails.push(msg) + return { sent: sendOk } + }) +} + +test('a digest gathers the Team’s new posts into one mail', async () => { + stubDigest({ + posts: [ + { id: 1, thread_id: 7, title: 'Raid', body_html: '

tonight

', author_username: 'ten' }, + { id: 2, thread_id: 7, title: 'Raid', body_html: '

bring rope

', author_username: 'eleven' }, + ], + }) + const res = await digest.tick(new Date()) + assert.equal(res.sent, 1) + assert.equal(mails.length, 1) + assert.equal(mails[0].items.length, 2) +}) + +test('a recipient with nothing new gets no mail and no stamp', async () => { + stubDigest({ posts: [] }) + const res = await digest.tick(new Date()) + assert.equal(res.sent, 0) + assert.equal(mails.length, 0) + assert.equal(world.stamped.length, 0, 'stamping here would move the window past unsent posts') +}) + +test('a failed send leaves last_digest_at alone so the window is retried', async () => { + stubDigest({ posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }], sendOk: false }) + const res = await digest.tick(new Date()) + assert.equal(res.sent, 0) + assert.equal(world.stamped.length, 0) +}) + +test('a successful send stamps exactly that (user, Team)', async () => { + stubDigest({ posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }] }) + const now = new Date() + await digest.tick(now) + assert.deepEqual(world.stamped, [{ userId: 11, teamId: 1, at: now }]) +}) + +test('only digest-mode recipients are swept', async () => { + stubDigest({ + posts: [{ id: 1, thread_id: 7, title: 'Raid', body_html: '

x

', author_username: 'ten' }], + rows: [ + DIGEST_ROW, + { user_id: 12, username: 'twelve', email: 'i@example.test', email_mode: 'immediate', last_digest_at: null }, + { user_id: 13, username: 'thirteen', email: 'o@example.test', email_mode: 'off', last_digest_at: null }, + ], + }) + await digest.tick(new Date()) + assert.deepEqual(mails.map((m) => m.to), ['e@example.test']) +}) + +test('the sweep is skipped whole when forums are off or email is unconfigured', async () => { + stub({ forumsEnabled: false }) + assert.equal((await digest.tick(new Date())).skipped, 'forums-disabled') + stub({ emailConfigured: false }) + assert.equal((await digest.tick(new Date())).skipped, 'email-unconfigured') +}) + +test('the sweep never throws, and says so in its summary', async () => { + patch(notifyModel, 'teamsWithForumActivitySince', async () => { throw new Error('nope') }) + assert.equal((await digest.tick(new Date())).skipped, 'error') +}) + +// ── The digest window ────────────────────────────────────────────────────── + +test('a first digest reaches back one interval, not to the lookback floor', () => { + const now = new Date('2026-08-18T12:00:00Z') + const since = digest.clampSince(null, now) + assert.equal(now - since <= 24 * 60 * 60 * 1000, true) +}) + +test('a long outage is clamped: one digest, not a week of replay', () => { + const now = new Date('2026-08-18T12:00:00Z') + const ancient = new Date('2026-01-01T00:00:00Z') + const since = digest.clampSince(ancient, now) + assert.equal(now - since, digest.MAX_LOOKBACK_MS) +}) + +test('an ordinary last-send is used as-is', () => { + const now = new Date('2026-08-18T12:00:00Z') + const yesterday = new Date('2026-08-17T12:00:00Z') + assert.equal(digest.clampSince(yesterday, now).getTime(), yesterday.getTime()) +}) diff --git a/server/test/teamProvider.test.js b/server/test/teamProvider.test.js index f160d6b..a121276 100644 --- a/server/test/teamProvider.test.js +++ b/server/test/teamProvider.test.js @@ -309,6 +309,39 @@ test('a non-function projectRoster is rejected at registration, not at call time ) }) +// ── pageUrlTemplate: the optional fifth member (§6.4) ────────────────────── +// +// Data, not a method, and the only thing core can use to link to a Team page — +// Teams have no core surface, so the module that owns the page has to say where +// it is. Validated hard because the output goes into an email as a link. + +test('pageUrlTemplate is optional: a provider without it registers fine', () => { + const api = registries.stage('uo') + assert.doesNotThrow(() => api.registerTeamProvider(ok())) +}) + +test('a relative template is kept exactly as given', () => { + register('uo', { ...ok(), pageUrlTemplate: '/uo/guilds/{externalId}' }) + assert.equal(registries.registeredTeamProvider().pageUrlTemplate, '/uo/guilds/{externalId}') +}) + +test('an absolute template is refused — a module may not redirect the site’s mail', () => { + const api = registries.stage('uo') + for (const bad of [ + 'https://evil.test/{externalId}', + '//evil.test/x', + 'uo/guilds/{externalId}', // not rooted + '/uo/guilds/{externalId}?x=