diff --git a/.gitea/workflows/pr-checks.yml b/.gitea/workflows/pr-checks.yml index add43e5..4188ea9 100644 --- a/.gitea/workflows/pr-checks.yml +++ b/.gitea/workflows/pr-checks.yml @@ -56,6 +56,12 @@ jobs: # something found under a pile of unrelated failures, and it costs # nothing when it passes. run: npm run check:modules + - name: Check the engagement subsystem names no external host + # ENGAGEMENT.md §3.2 rule 4 — no transport may ship a default host, + # endpoint or sender. Dependency-free and runs before the install for the + # same reason as the check above: a phone-home is a design break, not a + # test failure, and it should be the first thing a reviewer sees. + run: npm run check:hosts - name: Install server deps run: npm ci --prefix server - name: Run server tests diff --git a/client/src/api/client.js b/client/src/api/client.js index 1da6829..9a97898 100644 --- a/client/src/api/client.js +++ b/client/src/api/client.js @@ -455,10 +455,11 @@ export const api = { getDiscordBotConfig: () => req('/admin/discord-bot/config'), saveDiscordBotConfig: (data) => req('/admin/discord-bot/config', { method: 'PUT', body: data }), - // ----- Email delivery / Gmail OAuth2 (admin only) ----- + // ----- Email delivery (admin only) ----- + // The connect-flow call went with Gmail OAuth2 (ENGAGEMENT.md §1.2a); the + // config response now carries the transport catalog the form renders from. getEmailConfig: () => req('/admin/email/config'), saveEmailConfig: (data) => req('/admin/email/config', { method: 'PUT', body: data }), - emailConnectUrl: () => req('/admin/email/connect/start'), testEmail: (to) => req('/admin/email/test', { method: 'POST', body: { to } }), disconnectEmail: () => req('/admin/email/disconnect', { method: 'POST' }), }, diff --git a/client/src/routes/admin/views/Dashboard.jsx b/client/src/routes/admin/views/Dashboard.jsx index 2ea25da..9a2ca63 100644 --- a/client/src/routes/admin/views/Dashboard.jsx +++ b/client/src/routes/admin/views/Dashboard.jsx @@ -66,6 +66,34 @@ export default function Dashboard() { return (
+ {/* Operator warnings: things that are quietly not working and would + otherwise be discovered by someone not receiving an email. The list is + normally empty, which is why it sits above the fold rather than in a + panel — see ENGAGEMENT.md §1.2a (G22). */} + {(dash.warnings || []).map((w) => ( +
+ {w.message} + {w.href && ( + <> + {' '} + Open settings + + )} +
+ ))}
the callback may redirect with. -const ERROR_TEXT = { - denied: 'Google sign-in was cancelled or denied.', - bad_state: 'The connect session expired. Please try again.', - no_client: 'The Google OAuth client is not configured.', - no_refresh_token: - 'Google did not return a refresh token. Remove this app under your Google Account → Security → Third-party access, then reconnect.', - no_email: 'Could not read the Gmail address from Google.', - error: 'Could not connect the Gmail account. Please try again.', -} - function StatusPanel({ config }) { const color = STATUS_COLOR[config.status] || 'var(--muted)' return ( @@ -52,60 +50,100 @@ function StatusPanel({ config }) { ) } +// One declared credential field. A `secret` already held renders empty with a +// "leave blank to keep" hint, matching the server's patch semantics: an empty +// secret is omitted from the save, not written as a blank. +function CredentialField({ field, value, isSet, onChange }) { + const hint = [field.help, field.kind === 'secret' && isSet ? 'Currently set — leave blank to keep it.' : null] + .filter(Boolean) + .join(' ') + + if (field.kind === 'boolean') { + return ( + + ) + } + + return ( + + ) +} + export default function EmailDelivery() { const { siteTitle } = useSite() const [config, setConfig] = useState(null) const [error, setError] = useState('') + const [transport, setTransport] = useState('smtp') + const [senderEmail, setSenderEmail] = useState('') const [senderName, setSenderName] = useState('') + const [replyTo, setReplyTo] = useState('') + const [credential, setCredential] = useState({}) const [enabled, setEnabled] = useState(false) const [busy, setBusy] = useState('') const [msg, setMsg] = useState('') const [actionError, setActionError] = useState('') - const [banner, setBanner] = useState(null) // fields kind ('ok' or 'err') and text + + // Seed the credential inputs from the non-secret values the server returned, + // falling back to each field's declared default. Secrets are never seeded — + // the server does not send them and an empty box means "keep what you have". + const seedCredential = useCallback((c, transportId) => { + const def = (c.transports || []).find((t) => t.id === transportId) + const next = {} + for (const f of def?.credentialFields || []) { + if (f.kind === 'secret') continue + next[f.key] = c.credential?.[f.key] ?? (f.default === null ? '' : f.default) + } + return next + }, []) const load = useCallback(async (seedForm = false) => { try { const c = await api.admin.getEmailConfig() setConfig(c) if (seedForm) { + setTransport(c.transport || 'smtp') + setSenderEmail(c.senderEmail || '') setSenderName(c.senderName || '') + setReplyTo(c.replyTo || '') setEnabled(c.enabled) + setCredential(seedCredential(c, c.transport || 'smtp')) } return c } catch { setError('Could not load email settings.') return null } - }, []) + }, [seedCredential]) - // On mount, surface the outcome of a just-completed connect redirect, strip the - // query params so a refresh doesn't replay the banner, then load config. useEffect(() => { - const params = new URLSearchParams(window.location.search) - if (params.has('email_connected')) { - setBanner({ kind: 'ok', text: 'Gmail account connected.' }) - } else if (params.has('email_error')) { - setBanner({ kind: 'err', text: ERROR_TEXT[params.get('email_error')] || 'Could not connect email.' }) - } - if (params.has('email_connected') || params.has('email_error')) { - params.delete('email_connected') - params.delete('email_error') - const qs = params.toString() - window.history.replaceState({}, '', window.location.pathname + (qs ? `?${qs}` : '')) - } load(true) }, [load]) - async function connect() { - setBusy('connect') - setActionError('') - try { - const { url } = await api.admin.emailConnectUrl() - window.location.href = url - } catch (err) { - setActionError(err.message || 'Could not start the connect flow.') - setBusy('') - } + // Switching transport starts from the new one's declared defaults, because the + // server does the same: a credential blob is never carried across transports. + function changeTransport(id) { + setTransport(id) + setCredential(seedCredential(config, id)) } async function save() { @@ -113,10 +151,19 @@ export default function EmailDelivery() { setMsg('') setActionError('') try { - const saved = await api.admin.saveEmailConfig({ senderName, enabled }) + const saved = await api.admin.saveEmailConfig({ transport, senderEmail, senderName, replyTo, credential, enabled }) setConfig(saved) + setEnabled(saved.enabled) + setCredential(seedCredential(saved, saved.transport)) setMsg('Saved.') } catch (err) { + // A refused enable comes back with the reverted config attached, so the + // screen shows what is actually stored rather than the state that was + // rejected. + if (err.body?.config) { + setConfig(err.body.config) + setEnabled(err.body.config.enabled) + } setActionError(err.message || 'Could not save.') } finally { setBusy('') @@ -133,12 +180,13 @@ export default function EmailDelivery() { await load() } catch (err) { setActionError(err.message || 'Could not send the test email.') + await load() } finally { setBusy('') } } - async function disconnect() { + async function clearCredentials() { setBusy('disconnect') setMsg('') setActionError('') @@ -146,9 +194,11 @@ export default function EmailDelivery() { const c = await api.admin.disconnectEmail() setConfig(c) setEnabled(false) - setMsg('Disconnected.') + setSenderEmail('') + setCredential(seedCredential(c, c.transport)) + setMsg('Credentials cleared.') } catch (err) { - setActionError(err.message || 'Could not disconnect.') + setActionError(err.message || 'Could not clear the credentials.') } finally { setBusy('') } @@ -157,84 +207,118 @@ export default function EmailDelivery() { if (error) return

{error}

if (!config) return null - const connected = config.hasRefreshToken + const catalog = config.transports || [] + const selected = catalog.find((t) => t.id === transport) return (

Email delivery

- Sends the contact form through Gmail over OAuth2, delivered to the - Contact email above. Reuses the Google authentication - client — configure that on the Authentication page first. + Sends the contact form, invitations, password resets and team + notifications. Contact-form mail is delivered to the + Contact email above. Credentials are stored encrypted + and never shown again.

- {banner && ( + {config.hadLegacyConnection && !config.hasCredential && (
- {banner.text} + This deployment was connected with the old Gmail sign-in, which has been + removed. No mail is being sent. Enter SMTP credentials + below to restore it — for Gmail, use smtp.gmail.com port 587 + with an app password.
)} - {!config.googleConfigured && ( -

- The Google authentication provider needs a client ID and secret before - you can connect a Gmail account. -

+ {catalog.length > 1 && ( + )} - {!connected ? ( -
- + + {config.hasCredential && ( + -
- ) : ( - <> - - - - -
- - - - -
- - )} + )} +
{msg && {msg}} diff --git a/package.json b/package.json index 28f34fd..5573b9a 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,8 @@ "seed": "npm run seed --prefix server", "build": "npm run build --prefix client", "start": "npm start --prefix server", - "check:modules": "node scripts/checkModuleIdentifiers.js" + "check:modules": "node scripts/checkModuleIdentifiers.js", + "check:hosts": "node scripts/checkNoExternalHosts.js" }, "keywords": ["express", "mariadb", "react", "vite", "jwt"], "author": "whitlocktech", diff --git a/scripts/checkNoExternalHosts.js b/scripts/checkNoExternalHosts.js new file mode 100644 index 0000000..d45747e --- /dev/null +++ b/scripts/checkNoExternalHosts.js @@ -0,0 +1,185 @@ +#!/usr/bin/env node +// ── §3.2 rule 4 — no phone-home in the engagement subsystem ──────────────── +// +// ENGAGEMENT.md §3.2 records a posture the codebase already has and this check +// exists to keep: **no transport may ship a default host, endpoint, API base or +// sender.** A transport with no operator configuration is `unconfigured` and its +// channel is off — it never quietly falls back to a destination we chose. +// +// The rule is easy to hold and easy to break by accident, and the removed Gmail +// transport is the proof of both: `smtp.gmail.com` and port 465 were literals in +// `mailer.buildTransport()`, which made "which provider" a code edit and made the +// deployment's mail depend on a host nobody configured. Deleting that literal is +// what this check was written against, and it is the first thing it would have +// caught. +// +// **It reads code, not prose.** A comment naming `smtp.gmail.com` as the +// migration path for existing operators is exactly the documentation this phase +// owes, and a check that forbade it would teach people to phrase around it. So +// comments and the insides of ordinary strings are masked out; what is checked is +// a HOSTNAME OR URL appearing as a string literal in the engagement trees. Same +// design, and the same reasoning, as `checkModuleIdentifiers.js` — including +// having its own test suite, because a check that silently stops checking is +// worse than no check. +// +// Scope is the engagement subsystem plus the mail path it owns, not the whole +// server: core legitimately talks to hosts an operator configured elsewhere +// (ntfy, Discord, the sidecar), and those are not this rule's business. + +const fs = require('fs') +const path = require('path') + +const ROOT = path.resolve(__dirname, '..') + +// The trees the rule covers. `server/src/engagement/` is where transports and, +// later, the rules engine live; `utils/mailer.js` is the one file outside it that +// composes and sends mail. +const TREES = [path.join(ROOT, 'server', 'src', 'engagement')] +const FILES = [path.join(ROOT, 'server', 'src', 'utils', 'mailer.js')] + +const SKIP_DIRS = new Set(['node_modules', 'coverage', 'dist', '.git']) +const CODE = new Set(['.js', '.jsx', '.mjs', '.cjs']) + +// A URL, or a bare dotted hostname with a real TLD. The TLD length floor is what +// keeps `emailConfig.model` and `foo.js` out of it — a two-plus-letter final +// label after at least one dot, with no path characters, is a host. +const URL_LITERAL = /\b(?:https?|smtps?):\/\/[^\s'"`]+/ +const HOSTNAME_LITERAL = /\b(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+(?:com|net|org|io|dev|co|email|mail|cloud|app|us|eu)\b/i + +// Hosts that are not destinations: the loopback family, and the RFC 2606 names +// reserved for documentation. A placeholder in an admin form's help text is the +// opposite of a phone-home — it shows the operator the SHAPE of a value they +// must supply, and blanking it would make the form worse to hold the rule. +const ALLOWED = [ + /^(?:localhost|127\.0\.0\.1|\[::1\]|0\.0\.0\.0)$/i, + /(?:^|\.)example\.(?:com|net|org)$/i, + /(?:^|\.)(?:invalid|test|localhost)$/i, +] + +const isAllowed = (host) => ALLOWED.some((re) => re.test(host)) + +const hostOf = (literal) => { + const withoutScheme = literal.replace(/^[a-z]+:\/\//i, '') + return withoutScheme.split(/[/?#:]/)[0] +} + +/** + * Blank comments and mask string bodies in one left-to-right pass, keeping every + * offset aligned so reported line numbers stay honest. + * + * Lifted from `checkModuleIdentifiers.maskCode` deliberately rather than + * imported: that file's masking is tuned to ITS four checks (it keeps quotes so a + * route-path check can re-read the original at the same offsets), and coupling + * two checks through a shared helper means a change made for one silently + * re-scopes the other. Both are ~40 lines and both are tested. + */ +function maskComments(src) { + const out = Array.from(src) + const blank = (from, to) => { + for (let i = from; i < to && i < out.length; i++) if (out[i] !== '\n') out[i] = ' ' + } + let i = 0 + while (i < src.length) { + const c = src[i] + const next = src[i + 1] + if (c === '/' && next === '/') { + let j = i + while (j < src.length && src[j] !== '\n') j++ + blank(i, j) + i = j + continue + } + if (c === '/' && next === '*') { + const end = src.indexOf('*/', i + 2) + const j = end === -1 ? src.length : end + 2 + blank(i, j) + i = j + continue + } + if (c === '"' || c === "'" || c === '`') { + let j = i + 1 + while (j < src.length) { + if (src[j] === '\\') { j += 2; continue } + if (src[j] === c) break + j++ + } + // Keep the string body: it is what this check reads. Only the delimiters + // matter for finding it, and comments are what has to go. + i = j + 1 + continue + } + i++ + } + return out.join('') +} + +// Every string literal in the (comment-free) source, with its line number. +const STRING = /(['"`])((?:\\.|(?!\1)[^\\])*)\1/g + +function lineOf(src, index) { + return src.slice(0, index).split('\n').length +} + +/** Check one file's contents. Returns [{ file, line, literal, host }]. */ +function checkFile(rel, src) { + const hits = [] + const code = maskComments(src) + for (const m of code.matchAll(STRING)) { + const value = m[2] + if (!value) continue + const urlMatch = value.match(URL_LITERAL) + const hostMatch = urlMatch ? null : value.match(HOSTNAME_LITERAL) + const literal = urlMatch ? urlMatch[0] : hostMatch ? hostMatch[0] : null + if (!literal) continue + const host = hostOf(literal) + if (isAllowed(host)) continue + hits.push({ file: rel, line: lineOf(src, m.index), literal, host }) + } + return hits +} + +function walk(dir, out = []) { + if (!fs.existsSync(dir)) return out + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + if (SKIP_DIRS.has(entry.name)) continue + const full = path.join(dir, entry.name) + if (entry.isDirectory()) walk(full, out) + else out.push(full) + } + return out +} + +function run() { + const files = [...TREES.flatMap((t) => walk(t)), ...FILES.filter((f) => fs.existsSync(f))] + const hits = [] + for (const file of files) { + if (!CODE.has(path.extname(file))) continue + const rel = path.relative(ROOT, file).split(path.sep).join('/') + hits.push(...checkFile(rel, fs.readFileSync(file, 'utf8'))) + } + return hits +} + +module.exports = { run, checkFile, maskComments, isAllowed, hostOf } + +if (require.main === module) { + const hits = run() + if (hits.length === 0) { + console.log('OK — the engagement subsystem names no external host (ENGAGEMENT.md §3.2 rule 4).') + process.exit(0) + } + console.error( + `\nThe engagement subsystem names ${hits.length} external host${hits.length === 1 ? '' : 's'} ` + + 'in code (ENGAGEMENT.md §3.2 rule 4). A destination belongs in operator-supplied ' + + 'configuration, never in a literal:\n', + ) + for (const h of hits) { + console.error(` ${h.file}:${h.line} "${h.literal}"`) + } + console.error( + '\nIf this is help text or documentation rather than a destination, put it in a comment or ' + + 'use an example.com placeholder — the check masks comments and allows the reserved ' + + 'documentation names on purpose.\n', + ) + process.exit(1) +} diff --git a/server/db/schema.sql b/server/db/schema.sql index c20fb32..f6cfb91 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -327,19 +327,33 @@ CREATE TABLE IF NOT EXISTS bot_config ( CONSTRAINT chk_bot_config_singleton CHECK (id = 1) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; --- Outbound email configuration (Gmail over OAuth2 / SMTP XOAUTH2). Singleton row --- (id = 1), mirroring bot_config: the DB only ever holds the AES-256-GCM-encrypted --- refresh token, never plaintext, and the client id/secret are NOT stored here — --- they are read live from the `google` auth_providers row. The refresh token is --- captured by the in-app "Connect Gmail" consent flow and is write-only over the --- admin API (never returned; responses expose only hasRefreshToken). +-- Outbound email configuration. Singleton row (id = 1), mirroring bot_config: the +-- DB only ever holds the AES-256-GCM-encrypted credential, never plaintext, and it +-- is write-only over the admin API (never returned; responses expose only +-- hasCredential and the non-secret fields the transport declares). +-- +-- `transport` names a registered mail transport (server/src/engagement/transports). +-- `credential_enc` is that transport's whole credential set as one encrypted JSON +-- blob rather than a column per field, because the field list is the transport's to +-- declare — SMTP wants host/port/secure/user/password, an API relay wants a domain +-- and a key, and a column per union member would make adding a transport a schema +-- change. ENGAGEMENT.md §3.1. +-- +-- `provider` and `refresh_token_enc` are DEPRECATED and no longer read: they held +-- the removed Gmail OAuth2 connection (ENGAGEMENT.md §1.2a). They are kept rather +-- than dropped under the additive-only discipline, and `refresh_token_enc` earns +-- its keep in the meantime as the marker for "this deployment had working mail +-- before the upgrade" — which is what the admin dashboard warning reads. CREATE TABLE IF NOT EXISTS email_config ( id INT PRIMARY KEY DEFAULT 1, - provider VARCHAR(20) NOT NULL DEFAULT 'gmail_oauth2', + provider VARCHAR(20) NOT NULL DEFAULT 'gmail_oauth2', -- DEPRECATED, unread + transport VARCHAR(32) NOT NULL DEFAULT 'smtp', enabled TINYINT(1) NOT NULL DEFAULT 0, - sender_email VARCHAR(255) NULL, -- connected Gmail address (from userinfo) + sender_email VARCHAR(255) NULL, -- envelope From, operator-typed sender_name VARCHAR(120) NULL, -- optional From display name - refresh_token_enc TEXT NULL, -- AES-256-GCM ciphertext, never exposed + reply_to VARCHAR(255) NULL, -- optional Reply-To for sent mail + credential_enc TEXT NULL, -- AES-256-GCM ciphertext (JSON), never exposed + refresh_token_enc TEXT NULL, -- DEPRECATED, unread; see above status VARCHAR(20) NOT NULL DEFAULT 'unconfigured', status_detail VARCHAR(500) NULL, last_verified_at DATETIME NULL, @@ -1464,3 +1478,26 @@ ALTER TABLE mobile_refresh_tokens ADD COLUMN IF NOT EXISTS last_used_at DATETIME -- trust token. A boolean only — the token is returned over that app→server call -- and never persisted here (only its sha256 lands in trusted_devices). ALTER TABLE mobile_auth_sessions ADD COLUMN IF NOT EXISTS trust_device TINYINT(1) NOT NULL DEFAULT 0; + +-- ── Engagement Phase 1: Gmail OAuth2 removed, SMTP is the baseline ────────── +-- (ENGAGEMENT.md §1.2a). Additive on an upgraded database: `transport` backfills +-- to 'smtp' for every existing row, and `credential_enc` starts NULL — so an +-- upgraded deployment is deliberately CREDENTIAL-LESS until its operator supplies +-- SMTP settings. That is the whole point of the G22 warning below: nothing about +-- this fails loudly, so something has to say it out loud. +ALTER TABLE email_config ADD COLUMN IF NOT EXISTS transport VARCHAR(32) NOT NULL DEFAULT 'smtp'; +ALTER TABLE email_config ADD COLUMN IF NOT EXISTS credential_enc TEXT NULL; +ALTER TABLE email_config ADD COLUMN IF NOT EXISTS reply_to VARCHAR(255) NULL; + +-- The status a Gmail-connected deployment carries is 'connected', and after the +-- upgrade that is a lie: nothing can send. Correct it once, narrowly. The WHERE +-- makes this idempotent and self-limiting — it matches only a row that still holds +-- a Gmail refresh token AND has no replacement credential, so re-running it after +-- the operator configures SMTP touches nothing, and it can never overwrite a real +-- status recorded by a later send. +UPDATE email_config + SET status = 'unconfigured', + status_detail = 'Gmail OAuth2 was removed. Configure SMTP credentials in Admin - Settings - Email.' + WHERE refresh_token_enc IS NOT NULL + AND credential_enc IS NULL + AND status <> 'unconfigured'; diff --git a/server/routes.guards.json b/server/routes.guards.json index 05b5ce9..20e99fe 100644 --- a/server/routes.guards.json +++ b/server/routes.guards.json @@ -199,7 +199,7 @@ { "method": "PUT", "path": "/api/v1/admin/email/config", - "handlers": 5, + "handlers": 9, "gates": [ "noindex", "requireAuth", @@ -207,24 +207,6 @@ "validate" ] }, - { - "method": "GET", - "path": "/api/v1/admin/email/connect/callback", - "handlers": 2, - "gates": [ - "noindex", - "requireAuth" - ] - }, - { - "method": "GET", - "path": "/api/v1/admin/email/connect/start", - "handlers": 2, - "gates": [ - "noindex", - "requireAuth" - ] - }, { "method": "POST", "path": "/api/v1/admin/email/disconnect", diff --git a/server/routes.manifest.json b/server/routes.manifest.json index 178300d..1c98c66 100644 --- a/server/routes.manifest.json +++ b/server/routes.manifest.json @@ -89,14 +89,6 @@ "method": "PUT", "path": "/api/v1/admin/email/config" }, - { - "method": "GET", - "path": "/api/v1/admin/email/connect/callback" - }, - { - "method": "GET", - "path": "/api/v1/admin/email/connect/start" - }, { "method": "POST", "path": "/api/v1/admin/email/disconnect" diff --git a/server/src/engagement/index.js b/server/src/engagement/index.js new file mode 100644 index 0000000..e64adab --- /dev/null +++ b/server/src/engagement/index.js @@ -0,0 +1,22 @@ +// ── The engagement subsystem — one door ──────────────────────────────────── +// +// ENGAGEMENT.md Phase 1. Today this is the mail transport registry and core's +// own transports; the trigger registry, the rules engine and the delivery +// channels arrive in later phases and hang here too. +// +// **Core's transports register through the same door a module's would**, and +// they register HERE rather than at the bottom of the registry file. That keeps +// the registry free of any knowledge of its registrants — the same reason +// `registerCore()` is called from app.js rather than from inside +// `modules/registries.js` (MODULE_API.md §7.6) — and it means requiring the +// registry never has the side effect of populating it. +// +// Requiring this module is what makes `smtp` available. Everything that resolves +// a transport goes through here, so there is exactly one place a transport can +// come into existence. + +require('./transports/smtp') + +const transports = require('./transports') + +module.exports = { transports } diff --git a/server/src/engagement/transports/index.js b/server/src/engagement/transports/index.js new file mode 100644 index 0000000..3a7d7b3 --- /dev/null +++ b/server/src/engagement/transports/index.js @@ -0,0 +1,195 @@ +// ── The mail transport registry ──────────────────────────────────────────── +// +// ENGAGEMENT.md §3.1, Phase 1. A **channel** is what kind of sink this is (email, +// push, in-app); a **transport** is how one channel actually delivers. This file +// is the second half only. The channel registry arrives with the engine that +// consumes it — registering a channel nothing calls would be a shape frozen +// before anything had tried to use it. +// +// What this replaces: `mailer.buildTransport()` had Gmail's host, port and +// OAuth2 auth type as literals, so "which provider" was a code edit. Now the +// stored `email_config.transport` names a registration, and the registration +// declares its own credential fields — which drives the admin form, the encrypted +// blob's shape and the validation, from one place. +// +// **`credentialFields` is the contract.** It is read by three consumers that +// would otherwise drift: the admin form renders it, `sanitizeCredential()` below +// filters a submitted body through it, and `describe()` tells the client which +// values are secret so they are never sent back. Adding a field to a transport is +// therefore one edit, not four. +// +// **No transport may carry a default host, endpoint or sender** (§3.2 rule 1). A +// transport with no operator configuration is `unconfigured` and its channel is +// off — it never falls back to somewhere we chose. `scripts/checkNoExternalHosts.js` +// is the CI backstop for that rule; this file is where it would be broken first. +// +// Nothing here touches the database or the network at require time. + +const log = require('../../utils/logger')('mailer') + +// id → transport definition +const transports = new Map() + +// Field kinds the admin form knows how to render. `secret` is the only one that +// changes behaviour server-side: it is write-only, so an unchanged value arrives +// as '' and must be read from the stored credential rather than overwritten. +const FIELD_KINDS = new Set(['text', 'number', 'secret', 'boolean']) + +/** + * Register a mail transport. Shape-checked at the call and collision-checked + * here, the same validate-then-commit discipline `modules/registries.js` uses. + * + * @param {object} def + * @param {string} def.id stable id stored in email_config.transport + * @param {string} def.label human name for the admin form + * @param {Array} def.credentialFields [{ key, label, kind, required, help, default }] + * @param {Function} def.build (credential, config) → a nodemailer-shaped transport + * @param {Function} def.isComplete (credential) → boolean; are the required fields present + */ +function registerMailTransport(def) { + if (!def || typeof def !== 'object') throw new Error('registerMailTransport: definition required') + const { id, label, credentialFields, build, isComplete } = def + if (typeof id !== 'string' || !/^[a-z][a-z0-9_-]*$/.test(id)) { + throw new Error(`registerMailTransport: invalid id ${JSON.stringify(id)}`) + } + if (transports.has(id)) throw new Error(`registerMailTransport: ${id} is already registered`) + if (typeof label !== 'string' || !label) throw new Error(`registerMailTransport(${id}): label required`) + if (!Array.isArray(credentialFields) || credentialFields.length === 0) { + throw new Error(`registerMailTransport(${id}): credentialFields required`) + } + for (const f of credentialFields) { + if (!f || typeof f.key !== 'string' || !f.key) { + throw new Error(`registerMailTransport(${id}): every credential field needs a key`) + } + if (!FIELD_KINDS.has(f.kind)) { + throw new Error(`registerMailTransport(${id}): field ${f.key} has unknown kind ${f.kind}`) + } + } + if (typeof build !== 'function') throw new Error(`registerMailTransport(${id}): build() required`) + if (typeof isComplete !== 'function') throw new Error(`registerMailTransport(${id}): isComplete() required`) + + transports.set(id, { ...def, credentialFields: credentialFields.map((f) => ({ ...f })) }) + return id +} + +/** The registered transport, or null. Callers must handle null — a stored id can + * name a transport that no longer exists (a downgrade, a removed provider), and + * that must degrade to "unconfigured", never throw at send time. */ +function get(id) { + return transports.get(id) || null +} + +function has(id) { + return transports.has(id) +} + +/** Every transport, as the admin form needs it: no functions, secrets flagged. */ +function describe() { + return [...transports.values()].map((t) => ({ + id: t.id, + label: t.label, + help: t.help || null, + credentialFields: t.credentialFields.map((f) => ({ + key: f.key, + label: f.label || f.key, + kind: f.kind, + required: Boolean(f.required), + help: f.help || null, + default: f.default === undefined ? null : f.default, + placeholder: f.placeholder || null, + })), + })) +} + +/** + * Filter a submitted credential body down to the transport's declared fields, + * coercing each to its declared kind. Anything not declared is dropped — the + * blob that reaches `secretBox.encrypt` only ever holds fields a transport asked + * for, so a client cannot smuggle extra keys into stored ciphertext. + * + * `secret` fields submitted empty are OMITTED rather than blanked, which is the + * "leave the existing one alone" convention `botConfig.save`/`emailConfig.save` + * already use; `mergeCredential()` is what puts the stored value back. + */ +function sanitizeCredential(id, body) { + const t = get(id) + if (!t) return {} + const out = {} + for (const f of t.credentialFields) { + if (!(f.key in (body || {}))) continue + const raw = body[f.key] + if (f.kind === 'secret') { + if (raw === undefined || raw === null || raw === '') continue + out[f.key] = String(raw) + } else if (f.kind === 'number') { + const n = Number(raw) + if (Number.isFinite(n)) out[f.key] = n + } else if (f.kind === 'boolean') { + out[f.key] = Boolean(raw) + } else { + out[f.key] = raw === null || raw === undefined ? '' : String(raw) + } + } + return out +} + +/** Stored credential + the submitted patch. Omitted secrets keep their stored value. */ +function mergeCredential(id, stored, patch) { + return { ...(stored || {}), ...(patch || {}) } +} + +/** Non-secret fields only — safe to return over the admin API. */ +function publicCredential(id, credential) { + const t = get(id) + if (!t || !credential) return {} + const out = {} + for (const f of t.credentialFields) { + if (f.kind === 'secret') continue + if (credential[f.key] !== undefined) out[f.key] = credential[f.key] + } + return out +} + +/** Which declared secrets are actually held, so the form can say "set" without + * ever returning the value. */ +function secretsPresent(id, credential) { + const t = get(id) + if (!t) return {} + const out = {} + for (const f of t.credentialFields) { + if (f.kind !== 'secret') continue + out[f.key] = Boolean(credential && credential[f.key]) + } + return out +} + +/** Does this credential have everything its transport needs to send? */ +function isComplete(id, credential) { + const t = get(id) + if (!t) return false + try { + return Boolean(t.isComplete(credential || {})) + } catch (err) { + log.warn('transport isComplete threw', { transport: id, message: err.message }) + return false + } +} + +// Test-only: the registry is module-level state and a suite that registers a +// fake transport must be able to undo it. +function _reset() { + transports.clear() +} + +module.exports = { + registerMailTransport, + get, + has, + describe, + sanitizeCredential, + mergeCredential, + publicCredential, + secretsPresent, + isComplete, + _reset, +} diff --git a/server/src/engagement/transports/smtp.js b/server/src/engagement/transports/smtp.js new file mode 100644 index 0000000..8450076 --- /dev/null +++ b/server/src/engagement/transports/smtp.js @@ -0,0 +1,96 @@ +// ── SMTP — the baseline mail transport ───────────────────────────────────── +// +// ENGAGEMENT.md decision 4: Gmail OAuth2 is removed, SMTP is the baseline. This +// is the only registered transport, and it is deliberately plain SMTP rather than +// anything provider-shaped — a relay (Mailgun, SES, Postmark), a self-hosted MTA +// and Gmail-with-an-app-password are all reachable through these five fields, so +// one transport covers all three postures §7.1 Q5 asks to document. +// +// **No defaults for host, port, user or sender.** §3.2 rule 1: a transport with +// no operator configuration is unconfigured, never pointed at somewhere we chose. +// `secure` gets a default because it is a protocol choice, not a destination — and +// even that is only a form default, not a fallback applied to a stored blank. +// +// **`secure` is the field operators get wrong**, so its help text says which port +// each setting means: `secure: true` is implicit TLS on 465, `secure: false` is +// plaintext-then-STARTTLS on 587 (which nodemailer upgrades automatically). The +// combination that silently fails is 587 with secure on — the handshake hangs +// rather than erroring cleanly — which is exactly why "Send test" is the real +// verification path now (§1.2a consequence 2). + +const nodemailer = require('nodemailer') + +const registry = require('./index') + +const CREDENTIAL_FIELDS = [ + { + key: 'host', + label: 'SMTP host', + kind: 'text', + required: true, + placeholder: 'smtp.example.com', + help: 'Your relay or mail server. No default — nothing is sent until you set this.', + }, + { + key: 'port', + label: 'Port', + kind: 'number', + required: true, + default: 587, + help: '587 for STARTTLS (most relays), 465 for implicit TLS, 25 for an unauthenticated local MTA.', + }, + { + key: 'secure', + label: 'Implicit TLS', + kind: 'boolean', + required: false, + default: false, + help: 'On for port 465. Leave off for 587 — the connection still upgrades to TLS via STARTTLS.', + }, + { + key: 'user', + label: 'Username', + kind: 'text', + required: false, + help: 'Leave blank for an unauthenticated local relay.', + }, + { + key: 'password', + label: 'Password / API key', + kind: 'secret', + required: false, + help: 'Stored encrypted and never returned. For Gmail this is an app password, not the account password.', + }, +] + +// Authentication is optional (a local MTA on port 25 needs none), so the only +// hard requirement is a destination. A username without a password is not +// "complete" — that combination authenticates as nobody and fails at the server. +function isComplete(credential) { + const c = credential || {} + if (!c.host || !Number(c.port)) return false + if (c.user && !c.password) return false + return true +} + +function build(credential) { + const c = credential || {} + const options = { + host: String(c.host), + port: Number(c.port), + secure: Boolean(c.secure), + } + if (c.user) options.auth = { user: String(c.user), pass: String(c.password || '') } + return nodemailer.createTransport(options) +} + +registry.registerMailTransport({ + id: 'smtp', + label: 'SMTP', + help: 'Any SMTP relay or mail server. See the operator guide for the three supported postures.', + credentialFields: CREDENTIAL_FIELDS, + isComplete, + build, +}) + +module.exports = { CREDENTIAL_FIELDS, isComplete, build } diff --git a/server/src/model/emailConfig/emailConfig.db.js b/server/src/model/emailConfig/emailConfig.db.js index 5927241..874ab05 100644 --- a/server/src/model/emailConfig/emailConfig.db.js +++ b/server/src/model/emailConfig/emailConfig.db.js @@ -1,7 +1,8 @@ const singletonConfigDb = require('../singletonConfigDb') const COLS = - 'id, provider, enabled, sender_email, sender_name, refresh_token_enc, status, status_detail, last_verified_at, updated_by, created_at, updated_at' + 'id, provider, transport, enabled, sender_email, sender_name, reply_to, credential_enc, refresh_token_enc, ' + + 'status, status_detail, last_verified_at, updated_by, created_at, updated_at' // Singleton row (id = 1). See ../singletonConfigDb for the get/upsert contract. module.exports = singletonConfigDb('email_config', COLS) diff --git a/server/src/model/emailConfig/emailConfig.model.js b/server/src/model/emailConfig/emailConfig.model.js index 7570857..ee28272 100644 --- a/server/src/model/emailConfig/emailConfig.model.js +++ b/server/src/model/emailConfig/emailConfig.model.js @@ -1,30 +1,69 @@ -// Outbound email config store (Gmail OAuth2). Mirrors the botConfig model split: -// the DB layer only ever sees ciphertext, and only getWithSecret() (used by the -// mailer at send time) decrypts the refresh token. The admin-facing getSafe() -// never includes it — callers see only `hasRefreshToken`. +// Outbound email config store. Mirrors the botConfig model split: the DB layer +// only ever sees ciphertext, and only getWithSecret() (used by the mailer at send +// time) decrypts the credential. The admin-facing getSafe() never includes it — +// callers see the non-secret fields plus which secrets are set. +// +// The credential is ONE encrypted JSON blob, not a column per field, because the +// field list belongs to the transport (ENGAGEMENT.md §3.1). `credential_enc` +// holds `{ host, port, secure, user, password }` for `smtp`; a future relay's +// blob would hold different keys against the same column. +// +// `provider` and `refresh_token_enc` are the removed Gmail OAuth2 connection +// (§1.2a). They are no longer read as configuration — `hadLegacyConnection` +// exposes the token column's presence for one purpose only: telling an upgraded +// deployment that its mail just stopped. const db = require('./emailConfig.db') const secretBox = require('../../utils/secretBox') +const { transports } = require('../../engagement') -function toSafe(row) { +const DEFAULT_TRANSPORT = 'smtp' + +// A stored blob that will not parse is treated as ABSENT, never as an error — +// the same fail-safe rule utils/settingsJson.js applies. A deployment whose +// SECRET_ENC_KEY was rotated must degrade to "unconfigured" and say so on the +// admin screen, not 500 the settings page and the contact form with it. +function readCredential(row) { + if (!row || !row.credential_enc) return null + try { + const parsed = JSON.parse(secretBox.decrypt(row.credential_enc)) + return parsed && typeof parsed === 'object' ? parsed : null + } catch { + return null + } +} + +function toSafe(row, credential) { + const transport = (row && row.transport) || DEFAULT_TRANSPORT if (!row) { return { - provider: 'gmail_oauth2', + transport: DEFAULT_TRANSPORT, enabled: false, senderEmail: null, senderName: null, - hasRefreshToken: false, + replyTo: null, + credential: {}, + secretsSet: transports.secretsPresent(DEFAULT_TRANSPORT, null), + hasCredential: false, + hadLegacyConnection: false, status: 'unconfigured', statusDetail: null, lastVerifiedAt: null, } } return { - provider: row.provider || 'gmail_oauth2', + transport, enabled: Boolean(row.enabled), senderEmail: row.sender_email || null, senderName: row.sender_name || null, - hasRefreshToken: Boolean(row.refresh_token_enc), + replyTo: row.reply_to || null, + credential: transports.publicCredential(transport, credential), + secretsSet: transports.secretsPresent(transport, credential), + hasCredential: transports.isComplete(transport, credential), + // Deliberately the raw column, not "is Gmail configured": nothing reads the + // token any more. It answers "did this deployment have working mail before + // the upgrade?", which is the G22 warning's whole condition. + hadLegacyConnection: Boolean(row.refresh_token_enc), status: row.status || 'unconfigured', statusDetail: row.status_detail || null, lastVerifiedAt: row.last_verified_at || null, @@ -32,38 +71,53 @@ function toSafe(row) { } async function getSafe() { - return toSafe(await db.get()) + const row = await db.get() + return toSafe(row, readCredential(row)) } -// Decrypted refresh token included — server-side only (building the mailer's -// OAuth2 transport). Returns null when no row exists yet. +// Decrypted credential included — server-side only (building the transport at +// send time). Returns null when no row exists yet. async function getWithSecret() { const row = await db.get() if (!row) return null - return { - ...toSafe(row), - refreshToken: row.refresh_token_enc ? secretBox.decrypt(row.refresh_token_enc) : null, - } + const credential = readCredential(row) + return { ...toSafe(row, credential), credentialSecret: credential || {} } } -// Save admin-supplied / connect-flow config. `refreshToken` undefined or '' means -// "leave the existing token unchanged" (same convention as botConfig.save). -async function save({ senderEmail, senderName, refreshToken, enabled, status, statusDetail, updatedBy }) { +// Save admin-supplied config. `credential` is a PATCH, merged over the stored +// blob: a secret field submitted empty is omitted by the registry's sanitizer and +// therefore keeps its stored value (same convention as botConfig.save). +async function save({ transport, senderEmail, senderName, replyTo, credential, enabled, status, statusDetail, updatedBy }) { + const row = await db.get() const fields = {} + const nextTransport = transport !== undefined ? transport : (row && row.transport) || DEFAULT_TRANSPORT + if (transport !== undefined) fields.transport = transport if (senderEmail !== undefined) fields.sender_email = senderEmail if (senderName !== undefined) fields.sender_name = senderName - if (refreshToken) fields.refresh_token_enc = secretBox.encrypt(refreshToken) + if (replyTo !== undefined) fields.reply_to = replyTo + if (credential !== undefined) { + // Changing transport starts from an empty credential rather than merging one + // transport's fields into another's — an SMTP password left inside a relay's + // blob is a stored secret nobody can see and nothing will ever use. + const base = row && row.transport === nextTransport ? readCredential(row) : null + const merged = transports.mergeCredential(nextTransport, base, transports.sanitizeCredential(nextTransport, credential)) + fields.credential_enc = Object.keys(merged).length ? secretBox.encrypt(JSON.stringify(merged)) : null + } if (enabled !== undefined) fields.enabled = enabled ? 1 : 0 if (status !== undefined) fields.status = status if (statusDetail !== undefined) fields.status_detail = statusDetail if (updatedBy !== undefined) fields.updated_by = updatedBy - const row = await db.upsert(fields) - return toSafe(row) + const saved = await db.upsert(fields) + return toSafe(saved, readCredential(saved)) } -// Clear the stored credential and disable sending (admin "Disconnect"). +// Clear the stored credential and disable sending (admin "Clear credentials"). +// The legacy Gmail token goes too: this is the operator saying "there is no +// mailbox here", and leaving the deprecated column set would keep the G22 warning +// on screen for a deployment that has deliberately turned mail off. async function disconnect(updatedBy) { const row = await db.upsert({ + credential_enc: null, refresh_token_enc: null, sender_email: null, enabled: 0, @@ -72,7 +126,7 @@ async function disconnect(updatedBy) { last_verified_at: null, updated_by: updatedBy ?? null, }) - return toSafe(row) + return toSafe(row, readCredential(row)) } // Record the outcome of the last send / verification so the admin panel has @@ -86,7 +140,7 @@ async function recordStatus({ status, statusDetail, lastVerifiedAt } = {}) { } if (Object.keys(fields).length === 0) return getSafe() const row = await db.upsert(fields) - return toSafe(row) + return toSafe(row, readCredential(row)) } -module.exports = { getSafe, getWithSecret, save, disconnect, recordStatus } +module.exports = { getSafe, getWithSecret, save, disconnect, recordStatus, DEFAULT_TRANSPORT } diff --git a/server/src/model/teams/teamNotify.model.js b/server/src/model/teams/teamNotify.model.js index a085642..6f5d70f 100644 --- a/server/src/model/teams/teamNotify.model.js +++ b/server/src/model/teams/teamNotify.model.js @@ -5,8 +5,8 @@ // 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. +// configuring a mail transport 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 diff --git a/server/src/router/v1/admin/admin.controller.js b/server/src/router/v1/admin/admin.controller.js index c91fc7e..e542700 100644 --- a/server/src/router/v1/admin/admin.controller.js +++ b/server/src/router/v1/admin/admin.controller.js @@ -9,6 +9,7 @@ const trustedDevices = require('../../../model/trustedDevices/trustedDevices.mod const recoveryCodes = require('../../../model/recoveryCodes/recoveryCodes.model') const registries = require('../../../modules/registries') const announceJobs = require('../../../model/announceJobs/announceJobs.model') +const emailConfig = require('../../../model/emailConfig/emailConfig.model') const forumSettings = require('../../../model/teams/teamForumSettings.model') const pushDispatch = require('../../../utils/pushDispatch') const { cleanBody } = require('../../../utils/sanitizeHtml') @@ -55,6 +56,39 @@ async function announceIfNewlyPublished(post, transition) { } // ── Dashboard & site mode ───────────────────────────────────────────── + +// G22 — the Gmail removal degrades SILENTLY (ENGAGEMENT.md §1.2a consequence 3). +// On upgrade, `transport` backfills to `smtp` with no credentials, so every sink +// politely does nothing: the contact form falls back to `mailto`, invites surface +// a copyable link, password resets still answer a generic 200. Nothing breaks +// loudly, which is exactly the risk — email stops and nobody is told. +// +// So something has to say it. The condition is deliberately narrow: a deployment +// that still holds the deprecated Gmail refresh token (it had working mail) and +// has no replacement credential (it does not any more). A fresh install has never +// had mail and is not warned — a nag about a capability nobody asked for is a +// banner people learn to ignore, and this one has to be believed exactly once. +// +// Never fails the dashboard. A warning that can 500 the admin landing page is a +// worse bug than the one it reports. +async function emailWarning() { + try { + const c = await emailConfig.getSafe() + if (!c.hadLegacyConnection || c.hasCredential) return null + return { + code: 'EMAIL_TRANSPORT_MIGRATION', + message: + 'Outbound email is not configured. This deployment used the Gmail connect flow, which has been ' + + 'removed — mail is no longer being sent. Add SMTP credentials under Settings → Email. Gmail still ' + + 'works as an ordinary SMTP relay with an app password.', + href: '/admin/settings', + } + } catch (err) { + log.warn('dashboard email warning check failed', { message: err.message }) + return null + } +} + async function dashboard(req, res) { try { return res.json({ @@ -67,6 +101,7 @@ async function dashboard(req, res) { posts: await posts.counts(), users: await users.count(), }, + warnings: [await emailWarning()].filter(Boolean), recent_activity: await activity.list({ limit: 10 }), }) } catch (err) { diff --git a/server/src/router/v1/admin/dashboard.router.js b/server/src/router/v1/admin/dashboard.router.js index 2927d7d..2ff4106 100644 --- a/server/src/router/v1/admin/dashboard.router.js +++ b/server/src/router/v1/admin/dashboard.router.js @@ -12,7 +12,7 @@ // through toward another mount and 403 an editor on an unrelated route. Keep // gates per-route in this file. // -// GET /dashboard — stats overview, any staff role. +// GET /dashboard — stats overview + operator warnings, any staff role. // PUT /site-mode — live ↔ maintenance, admin only. // // Neither is the audit log (/activity) nor the bot-scoring state @@ -34,7 +34,7 @@ dashboardRouter.get( // #swagger.tags = ['Admin · Dashboard'] // #swagger.summary = 'Dashboard summary counts' // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] - /* #swagger.responses[200] = { description: 'Summary: site mode, last change, post/user counts and recent activity', content: { "application/json": { schema: { type: "object", properties: { site_mode: { type: "string", example: "live" }, last_change: { type: "object", properties: { at: { type: "string", nullable: true }, by: { type: "string", nullable: true } } }, counts: { type: "object", properties: { posts: { type: "object", additionalProperties: true }, users: { type: "integer" } } }, recent_activity: { type: "array", items: { type: "object", additionalProperties: true } } } } } } } */ + /* #swagger.responses[200] = { description: 'Summary: site mode, last change, post/user counts and recent activity', content: { "application/json": { schema: { type: "object", properties: { site_mode: { type: "string", example: "live" }, last_change: { type: "object", properties: { at: { type: "string", nullable: true }, by: { type: "string", nullable: true } } }, counts: { type: "object", properties: { posts: { type: "object", additionalProperties: true }, users: { type: "integer" } } }, warnings: { type: "array", description: "Operator warnings needing action; empty when there is nothing to say", items: { type: "object", properties: { code: { type: "string", example: "EMAIL_TRANSPORT_MIGRATION" }, message: { type: "string" }, href: { type: "string" } } } }, recent_activity: { type: "array", items: { type: "object", additionalProperties: true } } } } } } } */ /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ ctrl.dashboard, ) diff --git a/server/src/router/v1/admin/email.router.js b/server/src/router/v1/admin/email.router.js index ff2c38c..092bdcc 100644 --- a/server/src/router/v1/admin/email.router.js +++ b/server/src/router/v1/admin/email.router.js @@ -1,9 +1,13 @@ -// Admin · Email — outbound mail delivery via Gmail OAuth2. +// Admin · Email — outbound mail delivery. // // Mounted at /api/v1/admin/email by admin/index.js, which already applied -// `noindex, isLoggedIn, staffOnly`. The modern replacement for env SMTP: the -// refresh token is captured by the connect flow below and is write-only over -// this API (stored encrypted by utils/secretBox.js, never returned). +// `noindex, isLoggedIn, staffOnly`. The credential for the selected transport is +// captured by PUT /config below and is write-only over this API (stored encrypted +// by utils/secretBox.js, never returned). +// +// Four routes. `/connect/start` and `/connect/callback` were deleted with the +// Gmail OAuth2 flow in engagement Phase 1 (ENGAGEMENT.md §1.2a) — SMTP has no +// redirect to bounce through, so a credential form is the whole of it. // // Admin-only, and kept as a per-route gate rather than a router-level `use` so // the middleware chain each route carries is unchanged by the move. @@ -21,9 +25,10 @@ const adminOnly = requireRole('admin') emailRouter.get( '/config', // #swagger.tags = ['Admin · Email'] - // #swagger.summary = 'Get email delivery config + status (admin only)' + // #swagger.summary = 'Get email delivery config, status and the transport catalog (admin only)' + // #swagger.description = 'Credentials are write-only: secret fields are never returned, only a per-field `secretsSet` flag. `transports` carries each registered transport's declared credential fields, which is what the admin form renders.' // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] - /* #swagger.responses[200] = { description: 'Config (refresh token stripped) + status', content: { "application/json": { schema: { type: "object", additionalProperties: true } } } } */ + /* #swagger.responses[200] = { description: 'Config (secrets stripped) + status + transport catalog', content: { "application/json": { schema: { type: "object", additionalProperties: true } } } } */ /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ /* #swagger.responses[403] = { description: 'Admin role required', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ adminOnly, @@ -33,45 +38,32 @@ emailRouter.put( '/config', // #swagger.tags = ['Admin · Email'] // #swagger.summary = 'Update email delivery config (admin only)' - // #swagger.description = 'Set the From display name and enabled toggle. Enabling requires a connected Gmail account.' + // #swagger.description = 'Set the transport, sender identity, credentials and enabled toggle. `credential` is a patch against the stored blob — a secret field submitted empty keeps its stored value. Enabling requires complete credentials and a sender address.' // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] - /* #swagger.requestBody = { content: { "application/json": { schema: { type: "object", properties: { senderName: { type: "string" }, enabled: { type: "boolean" } } } } } } */ + /* #swagger.requestBody = { content: { "application/json": { schema: { type: "object", properties: { transport: { type: "string", example: "smtp" }, senderEmail: { type: "string", format: "email" }, senderName: { type: "string" }, replyTo: { type: "string", format: "email" }, credential: { type: "object", additionalProperties: true }, enabled: { type: "boolean" } } } } } } */ /* #swagger.responses[200] = { description: 'Updated config', content: { "application/json": { schema: { type: "object", additionalProperties: true } } } } */ - /* #swagger.responses[400] = { description: 'Cannot enable before connecting a mailbox', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + /* #swagger.responses[400] = { description: 'Unknown transport, or cannot enable without complete credentials and a sender address', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ /* #swagger.responses[403] = { description: 'Admin role required', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ adminOnly, + body('transport').optional().isString().trim().isLength({ min: 1, max: 32 }), + body('senderEmail').optional({ values: 'falsy' }).isEmail().isLength({ max: 255 }), body('senderName').optional({ values: 'null' }).isString().trim().isLength({ max: 120 }), + body('replyTo').optional({ values: 'falsy' }).isEmail().isLength({ max: 255 }), + // The credential's SHAPE is the transport's to declare, so it is validated by + // the registry's sanitizer (which drops anything undeclared) rather than by a + // field list duplicated here that would drift the first time a transport is + // added. All this asserts is that it is an object at all. + body('credential').optional().isObject(), body('enabled').optional().isBoolean(), validate, emailConfig.saveConfig, ) -emailRouter.get( - '/connect/start', - // #swagger.tags = ['Admin · Email'] - // #swagger.summary = 'Begin the Gmail OAuth2 connect flow (admin only)' - // #swagger.description = 'Returns { url } to redirect the browser to Google. Reuses the google SSO OAuth client.' - // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] - /* #swagger.responses[200] = { description: 'Authorization URL', content: { "application/json": { schema: { type: "object", properties: { url: { type: "string" } } } } } } */ - /* #swagger.responses[400] = { description: 'Google OAuth client not configured', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ - /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ - /* #swagger.responses[403] = { description: 'Admin role required', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ - adminOnly, - emailConfig.connectStart, -) -emailRouter.get( - '/connect/callback', - // #swagger.tags = ['Admin · Email'] - // #swagger.summary = 'OAuth2 callback — stores the refresh token, redirects to Settings' - // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] - /* #swagger.responses[302] = { description: 'Redirect back to /admin/settings' } */ - adminOnly, - emailConfig.connectCallback, -) emailRouter.post( '/test', // #swagger.tags = ['Admin · Email'] // #swagger.summary = 'Send a test email (admin only)' + // #swagger.description = 'The real verification of the configuration — host, port, TLS mode, credentials, and whether the relay accepts the configured sender. Failures return a specific diagnostic.' // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] /* #swagger.requestBody = { content: { "application/json": { schema: { type: "object", properties: { to: { type: "string", format: "email" } } } } } } */ /* #swagger.responses[200] = { description: 'Sent', content: { "application/json": { schema: { type: "object", properties: { sent: { type: "boolean" }, to: { type: "string" } } } } } } */ @@ -84,9 +76,9 @@ emailRouter.post( emailRouter.post( '/disconnect', // #swagger.tags = ['Admin · Email'] - // #swagger.summary = 'Disconnect Gmail and disable email (admin only)' + // #swagger.summary = 'Clear the stored credentials and disable email (admin only)' // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] - /* #swagger.responses[200] = { description: 'Disconnected config', content: { "application/json": { schema: { type: "object", additionalProperties: true } } } } */ + /* #swagger.responses[200] = { description: 'Cleared config', content: { "application/json": { schema: { type: "object", additionalProperties: true } } } } */ /* #swagger.responses[401] = { description: 'Not authenticated', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ /* #swagger.responses[403] = { description: 'Admin role required', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ adminOnly, diff --git a/server/src/router/v1/admin/emailConfig.controller.js b/server/src/router/v1/admin/emailConfig.controller.js index e6dad60..8493821 100644 --- a/server/src/router/v1/admin/emailConfig.controller.js +++ b/server/src/router/v1/admin/emailConfig.controller.js @@ -1,67 +1,37 @@ -// ── Admin: outbound email configuration (Gmail OAuth2) ───────────────────── +// ── Admin: outbound email configuration ──────────────────────────────────── // -// Modern replacement for env-var SMTP. Sending goes through Gmail over OAuth2; -// the admin connects the mailbox with an in-app consent flow that captures a -// refresh token. We reuse the existing `google` SSO OAuth client (its id/secret) -// rather than a second app — so the only per-mailbox secret is the refresh token, -// stored AES-GCM-encrypted and write-only over this API (never returned). +// Email is configured here, not via env vars. The admin picks a registered mail +// transport and fills in the fields that transport declares; the whole set is +// stored as one AES-GCM-encrypted blob and is write-only over this API — secret +// fields are never returned, only a per-field "is it set" flag. // -// The connect flow mirrors sso.controller.js: a signed httpOnly tx cookie carries -// the CSRF nonce + PKCE verifier across the redirect to Google and back. It differs -// only in scope (https://mail.google.com/ for SMTP XOAUTH2) and access_type=offline -// + prompt=consent, which guarantee a refresh token even on reconnect. +// Gmail OAuth2 and its consent flow were removed in engagement Phase 1 +// (ENGAGEMENT.md §1.2a). What went with it: two routes, the `email_oauth_tx` +// signed cookie, the PKCE verifier and CSRF nonce plumbing, the +// `https://mail.google.com/` scope, and the borrowed `google` auth-providers +// client. That last one was a real coupling — an admin rotating the Google SSO +// secret silently broke outbound mail, with nothing on either screen relating the +// two — and removing it is one of the better side effects of the decision. +// +// **The form is driven by the transport's `credentialFields`, not by this file.** +// getConfig ships the declarations to the client, saveConfig hands the submitted +// body to the registry's sanitizer, and neither one names a field. Adding a +// transport is a registration, not an edit here. const emailConfig = require('../../../model/emailConfig/emailConfig.model') -const authProviders = require('../../../model/authProviders/authProviders.model') const activity = require('../../../model/activity/activity.model') const mailer = require('../../../utils/mailer') -const GoogleProvider = require('../../../auth/providers/google.provider') -const ssoState = require('../../../auth/ssoState') -const token = require('../../../auth/token') +const { transports } = require('../../../engagement') const log = require('../../../utils/logger')('admin') -// Gmail scope grants SMTP (XOAUTH2) access; openid+email let us read back which -// address was connected. The narrower gmail.send scope only works via the Gmail -// API, not SMTP, so we need the full-access scope here. -const EMAIL_SCOPE = 'https://mail.google.com/ openid email' -const TX_COOKIE = 'email_oauth_tx' - -// Public base URL for the OAuth redirect_uri — same fallback pattern as -// sso.controller.js. Must be identical between start and callback. -function appBaseUrl(req) { - const configured = process.env.APP_BASE_URL - if (configured) return configured.replace(/\/+$/, '') - const derived = `${req.protocol}://${req.get('host')}` - log.warn('APP_BASE_URL not set — deriving email redirect_uri from the request', { derived }) - return derived -} -function redirectUri(req) { - return `${appBaseUrl(req)}/api/v1/admin/email/connect/callback` -} -function txCookieOptions(req) { - return { ...token.cookieOptions(req), maxAge: 10 * 60 * 1000 } -} - -// Front-end redirect targets after the callback resolves. -const CONNECTED_URL = '/admin/settings?email_connected=1' -const errorUrl = (code) => `/admin/settings?email_error=${code}` - -// Load the Google OAuth client (id + decrypted secret) reused for email. Returns -// null when the google provider hasn't been configured with credentials yet. -async function googleClient() { - const row = await authProviders.getWithSecret('google') - if (!row || !row.client_id || !row.client_secret) return null - return { clientId: row.client_id, clientSecret: row.client_secret } -} - // GET /admin/email/config async function getConfig(req, res) { try { const config = await emailConfig.getSafe() - // Surface whether the Google client email can borrow is configured, so the - // UI can explain why Connect is unavailable. - config.googleConfigured = Boolean(await googleClient()) + // The transport catalog rides with the config so the client renders the + // credential form from the same declarations the server validates against. + config.transports = transports.describe() return res.json(config) } catch (err) { log.error('emailConfig.getConfig', err) @@ -69,23 +39,50 @@ async function getConfig(req, res) { } } -// PUT /admin/email/config — sender name + enabled toggle. Enabling requires a -// connected mailbox (a stored refresh token). +// PUT /admin/email/config — transport, sender identity, credentials, enabled. +// +// Enabling requires a complete credential AND a sender address, checked against +// the state as it will be AFTER this save rather than before it: the admin fills +// the whole form and ticks Enable in one submit, and refusing that because the +// credential was absent a moment ago would make the screen impossible to use. async function saveConfig(req, res) { - const { senderName, enabled } = req.body + const { transport, senderEmail, senderName, replyTo, credential, enabled } = req.body try { const current = await emailConfig.getSafe() - if (enabled && !current.hasRefreshToken) { - return res.status(400).json({ message: 'Connect a Gmail account before enabling email.' }) + const nextTransport = transport === undefined ? current.transport : transport + if (!transports.has(nextTransport)) { + return res.status(400).json({ message: `Unknown mail transport "${nextTransport}".` }) } + const saved = await emailConfig.save({ + transport: transport !== undefined ? nextTransport : undefined, + senderEmail: senderEmail !== undefined ? senderEmail || null : undefined, senderName: senderName !== undefined ? senderName || null : undefined, - enabled, + replyTo: replyTo !== undefined ? replyTo || null : undefined, + credential, + // Enabling is applied only once the saved row can actually support it, so + // the response tells the truth about what is stored rather than echoing the + // request. The re-read below is what decides. + enabled: enabled === undefined ? undefined : Boolean(enabled), updatedBy: req.user.id, }) - saved.googleConfigured = Boolean(await googleClient()) - await activity.log({ req, action: 'email.config.update', detail: { enabled: saved.enabled } }) - log.info('email config updated', { by: req.user.username, enabled: saved.enabled }) + + if (saved.enabled && !(saved.hasCredential && saved.senderEmail)) { + const reverted = await emailConfig.save({ enabled: false, updatedBy: req.user.id }) + reverted.transports = transports.describe() + return res.status(400).json({ + message: 'Add complete credentials and a sender address before enabling email.', + config: reverted, + }) + } + + saved.transports = transports.describe() + await activity.log({ + req, + action: 'email.config.update', + detail: { transport: saved.transport, enabled: saved.enabled, hasCredential: saved.hasCredential }, + }) + log.info('email config updated', { by: req.user.username, transport: saved.transport, enabled: saved.enabled }) return res.json(saved) } catch (err) { log.error('emailConfig.saveConfig', err) @@ -93,96 +90,14 @@ async function saveConfig(req, res) { } } -// GET /admin/email/connect/start — returns { url } for the browser to navigate to. -async function connectStart(req, res) { - try { - const client = await googleClient() - if (!client) { - return res.status(400).json({ - message: 'Configure the Google authentication provider (client id + secret) before connecting email.', - }) - } - const provider = new GoogleProvider({ clientId: client.clientId, clientSecret: client.clientSecret }) - const tx = ssoState.createTx({ flow: 'email' }) - res.cookie(TX_COOKIE, tx.txToken, txCookieOptions(req)) - - const params = new URLSearchParams({ - client_id: client.clientId, - redirect_uri: redirectUri(req), - response_type: 'code', - scope: EMAIL_SCOPE, - access_type: 'offline', - prompt: 'consent', - include_granted_scopes: 'true', - state: tx.nonce, - code_challenge: tx.codeChallenge, - code_challenge_method: 'S256', - }) - const url = `${provider.authEndpoint()}?${params.toString()}` - return res.json({ url }) - } catch (err) { - log.error('emailConfig.connectStart', err) - return res.status(500).json({ message: 'Internal Server Error' }) - } -} - -// GET /admin/email/connect/callback — exchange the code, capture the refresh -// token + connected address, store encrypted, and redirect back to Settings. -async function connectCallback(req, res) { - const txToken = req.cookies && req.cookies[TX_COOKIE] - const { code, state, error: oauthError } = req.query - res.clearCookie(TX_COOKIE, token.cookieOptions(req)) // single-use - - if (oauthError) { - log.warn('email connect: provider returned error', { error: String(oauthError).slice(0, 60) }) - return res.redirect(errorUrl('denied')) - } - const tx = ssoState.verifyTx(txToken, state) - if (!tx || tx.flow !== 'email' || !code) { - log.warn('email connect: bad state') - return res.redirect(errorUrl('bad_state')) - } - try { - const client = await googleClient() - if (!client) return res.redirect(errorUrl('no_client')) - const provider = new GoogleProvider({ clientId: client.clientId, clientSecret: client.clientSecret }) - - const tokenSet = await provider.exchangeCode({ - code, - redirectUri: redirectUri(req), - codeVerifier: tx.verifier, - }) - if (!tokenSet.refresh_token) { - // Google only returns a refresh token when it hasn't already granted one - // for this client+scope. prompt=consent should force it; if it's still - // missing the admin can revoke the app's access and retry. - log.warn('email connect: no refresh_token returned') - return res.redirect(errorUrl('no_refresh_token')) - } - const profile = await provider.getUserProfile(tokenSet.access_token) - const senderEmail = profile.email || null - if (!senderEmail) return res.redirect(errorUrl('no_email')) - - await emailConfig.save({ - senderEmail, - refreshToken: tokenSet.refresh_token, - enabled: true, - status: 'connected', - statusDetail: 'Connected', - updatedBy: req.user.id, - }) - await emailConfig.recordStatus({ status: 'connected', statusDetail: 'Connected', lastVerifiedAt: new Date() }) - await activity.log({ req, action: 'email.connect', detail: { senderEmail } }) - log.info('email connected', { senderEmail, by: req.user.username }) - return res.redirect(CONNECTED_URL) - } catch (err) { - log.error('emailConfig.connectCallback', err) - return res.redirect(errorUrl('error')) - } -} - // POST /admin/email/test — send a test message (to the given address, or the // contact recipient by default). +// +// This is the only verification the configuration gets. Under the removed consent +// flow the sending address came back from Google and was guaranteed to belong to +// the credential; an operator-typed sender the relay will not accept is a silent +// deliverability failure, so mailer.describeSendError puts the sender in the +// message and this hands it through verbatim. async function testSend(req, res) { try { const result = await mailer.sendTest(req.body.to) @@ -198,9 +113,9 @@ async function testSend(req, res) { async function disconnect(req, res) { try { const config = await emailConfig.disconnect(req.user.id) - config.googleConfigured = Boolean(await googleClient()) + config.transports = transports.describe() await activity.log({ req, action: 'email.disconnect' }) - log.info('email disconnected', { by: req.user.username }) + log.info('email credentials cleared', { by: req.user.username }) return res.json(config) } catch (err) { log.error('emailConfig.disconnect', err) @@ -208,4 +123,4 @@ async function disconnect(req, res) { } } -module.exports = { getConfig, saveConfig, connectStart, connectCallback, testSend, disconnect } +module.exports = { getConfig, saveConfig, testSend, disconnect } diff --git a/server/src/server.js b/server/src/server.js index 6a99914..fccd480 100644 --- a/server/src/server.js +++ b/server/src/server.js @@ -43,7 +43,7 @@ async function start() { logFile: createLogger.logFilePath || 'disabled (console only)', db: `${process.env.DB_HOST || '127.0.0.1'}:${process.env.DB_PORT || 3306}/${process.env.DB_NAME || 'runic_gateway'}`, cookieSecure: process.env.COOKIE_SECURE || 'auto', - email: 'gmail-oauth2 (configured in admin → settings)', + email: 'smtp (configured in admin → settings)', }) // Fail fast if the server<->bot shared secret is weak/placeholder. Fatal in diff --git a/server/src/utils/mailer.js b/server/src/utils/mailer.js index a8764ac..dc37ba4 100644 --- a/server/src/utils/mailer.js +++ b/server/src/utils/mailer.js @@ -1,65 +1,93 @@ -// ── Outbound mail (Gmail over OAuth2 / SMTP XOAUTH2) ─────────────────────── +// ── Outbound mail ────────────────────────────────────────────────────────── // // Email is configured in Admin → Settings → Email, not via env vars. The -// connection (enabled flag, connected Gmail address, encrypted refresh token) -// lives in the email_config singleton; the OAuth client id/secret are reused -// from the `google` auth_providers row. nodemailer takes the refresh token and -// auto-mints short-lived access tokens for each send. +// `email_config` singleton holds the enabled flag, the sender identity and the +// AES-GCM-encrypted credential for whichever transport is selected; the transport +// itself is a registration in `src/engagement/transports` (ENGAGEMENT.md §3.1), +// so which provider is used is DATA, not a code path in this file. // -// When email is not configured, sendContactMessage does NOT throw — it signals -// the caller to fall back to a mailto: link (the contact form relies on this). - -const nodemailer = require('nodemailer') +// Gmail OAuth2 was removed in engagement Phase 1 (§1.2a). Gmail is still reachable +// as an ordinary SMTP relay (`smtp.gmail.com:587` with an app password) — the +// operator types those in like any other host; nothing in here knows about it. +// +// **The failure contracts are the point of this file.** Six call sites, five +// senders, and each one degrades a specific way when mail is unconfigured. Those +// contracts are unchanged by the transport rewrite and are asserted in +// test/mailer.test.js: sendContactMessage returns a mailto fallback, sendInvite +// and sendPasswordReset return { sent: false, reason: 'NOT_CONFIGURED' } so their +// callers can surface a link / answer a generic 200, sendTeamNotification never +// throws at all, and only sendTest throws — because only sendTest has an admin +// waiting to be told why. const emailConfig = require('../model/emailConfig/emailConfig.model') -const authProviders = require('../model/authProviders/authProviders.model') const settings = require('../model/settings/settings.model') +const { transports } = require('../engagement') const brand = require('../config/brand') const log = require('./logger')('mailer') -// Ready to send only when enabled, connected (has a refresh token), and we know -// which address to send as. +// Ready to send only when enabled, holding a complete credential for a +// registered transport, and knowing which address to send as. async function isConfigured() { const c = await emailConfig.getSafe() - return Boolean(c.enabled && c.hasRefreshToken && c.senderEmail) + return Boolean(c.enabled && c.hasCredential && c.senderEmail) } // Recipient for the contact form: the admin-editable contact_email setting, or -// the connected sending address as a last resort. +// the configured sending address as a last resort. async function contactRecipient(senderEmail) { const to = await settings.get('contact_email') return to || senderEmail || null } -// Build a nodemailer OAuth2 transport from the stored config + reused Google -// client credentials. Returns { transport, config } or null when unconfigured. +/** + * Build a transport from the stored config. Returns { transport, config } or + * null when unconfigured — every sender handles null itself. + * + * Null, never a throw, for all five ways this can fail: no row, disabled, no + * sender address, an incomplete credential, or a stored transport id that is not + * registered (a downgrade, or a provider removed from the build). The last one is + * the reason `transports.get()` is checked rather than assumed: a send-time + * exception from an unknown id would break the contact form for a reason the + * admin screen already shows. + * + * **`enabled` is checked here now, and it was not before.** Under the connect + * flow this function tested only "is there a refresh token and a sender", so the + * contact form kept sending after an admin unticked "Enable email sending" — + * `isConfigured()` honoured the toggle but the direct senders bypassed it. With + * the toggle no longer set as a side effect of a consent redirect it has to mean + * what it says, so the gate lives on the one path every sender shares. + */ async function buildTransport() { const config = await emailConfig.getWithSecret() - if (!config || !config.refreshToken || !config.senderEmail) return null - const google = await authProviders.getWithSecret('google') - if (!google || !google.client_id || !google.client_secret) { - log.warn('email send skipped: Google OAuth client is not configured') + if (!config || !config.enabled || !config.senderEmail) return null + const def = transports.get(config.transport) + if (!def) { + log.warn('email send skipped: no such transport', { transport: config.transport }) + return null + } + if (!transports.isComplete(config.transport, config.credentialSecret)) { + log.warn('email send skipped: transport credentials are incomplete', { transport: config.transport }) + return null + } + try { + return { transport: def.build(config.credentialSecret, config), config } + } catch (err) { + log.error('could not build the mail transport', err) return null } - const transport = nodemailer.createTransport({ - host: 'smtp.gmail.com', - port: 465, - secure: true, - auth: { - type: 'OAuth2', - user: config.senderEmail, - clientId: google.client_id, - clientSecret: google.client_secret, - refreshToken: config.refreshToken, - }, - }) - return { transport, config } } function fromHeader(config) { return config.senderName ? `"${config.senderName}" <${config.senderEmail}>` : config.senderEmail } +// Reply-To is the operator's optional override; the contact form's per-message +// replyTo (the visitor's address) wins over it, which is the whole reason the +// contact form has one. +function replyToFor(config, override) { + return override || config.replyTo || undefined +} + /** * Send a contact message. If email is not configured/enabled, signals the caller * to fall back to a mailto: link instead of throwing. @@ -76,7 +104,7 @@ async function sendContactMessage({ name, email, message }) { await transport.sendMail({ from: fromHeader(config), to, - replyTo: email, + replyTo: replyToFor(config, email), subject: `${brand.name} contact from ${name || 'a visitor'}`, text: `From: ${name || 'unknown'} <${email || 'no email'}>\n\n${message}`, }) @@ -84,19 +112,49 @@ async function sendContactMessage({ name, email, message }) { return { sent: true } } catch (err) { log.error('contact send failed', err) - await emailConfig.recordStatus({ status: 'error', statusDetail: err.message }) + await emailConfig.recordStatus({ status: 'error', statusDetail: describeSendError(err) }) throw err } } +/** + * Turn a transport error into something an operator can act on. + * + * This matters more than it used to. Under Gmail OAuth2 the sending address came + * back from Google's userinfo and was guaranteed to be a mailbox the credential + * owned. Under SMTP `sender_email` is operator-typed, so a relay rejecting the + * envelope From is now a live failure mode (§1.2a consequence 2) — and it arrives + * as a bare "550 5.7.1" that means nothing without the sender in front of it. + */ +function describeSendError(err, config) { + const code = err && (err.responseCode || err.code) + const base = (err && (err.response || err.message)) || 'Send failed' + const sender = config && config.senderEmail + if (sender && (code === 550 || code === 553 || code === 554 || code === 'EENVELOPE')) { + return `${base} — the server refused "${sender}" as the sender. It must be an address this account is allowed to send as (SPF/DMARC).` + } + if (code === 'EAUTH') return `${base} — the username or password was rejected.` + if (code === 'ESOCKET' || code === 'ECONNECTION') { + return `${base} — could not connect. Check the host, the port, and whether "Implicit TLS" matches it (on for 465, off for 587).` + } + if (code === 'ETIMEDOUT') { + return `${base} — the connection timed out. A common cause is "Implicit TLS" left on for port 587.` + } + return String(base) +} + /** * Send a test email to `to`, used by the admin "Send test" button. Throws on * failure; records the outcome either way. Returns { sent: true } on success. + * + * **This is now the real verification of the whole configuration** — host, port, + * TLS mode, credentials AND whether the relay will accept the operator-typed + * sender. There is no consent flow left to prove any of it beforehand. */ async function sendTest(to) { const built = await buildTransport() if (!built) { - const err = new Error('Email is not configured. Connect Gmail first.') + const err = new Error('Email is not configured. Set a transport, its credentials and a sender address first.') err.code = 'NOT_CONFIGURED' throw err } @@ -111,15 +169,19 @@ async function sendTest(to) { await transport.sendMail({ from: fromHeader(config), to: recipient, + replyTo: replyToFor(config), subject: `${brand.name} email test`, - text: 'This is a test message confirming Gmail OAuth2 email delivery is working.', + text: `This is a test message confirming ${config.transport} email delivery is working.`, }) await emailConfig.recordStatus({ status: 'connected', statusDetail: 'Test send OK', lastVerifiedAt: new Date() }) return { sent: true, to: recipient } } catch (err) { + const detail = describeSendError(err, config) log.error('test send failed', err) - await emailConfig.recordStatus({ status: 'error', statusDetail: err.message }) - throw err + await emailConfig.recordStatus({ status: 'error', statusDetail: detail }) + const wrapped = new Error(detail) + wrapped.code = err.code || 'SEND_FAILED' + throw wrapped } } @@ -140,6 +202,7 @@ async function sendInvite({ to, acceptUrl, role, invitedByName }) { await transport.sendMail({ from: fromHeader(config), to, + replyTo: replyToFor(config), subject: `Your ${brand.name} invitation`, text: `You have been invited${by} to join ${brand.name}${roleLabel}.\n\n` + @@ -150,7 +213,7 @@ async function sendInvite({ to, acceptUrl, role, invitedByName }) { return { sent: true } } catch (err) { log.error('invite send failed', err) - await emailConfig.recordStatus({ status: 'error', statusDetail: err.message }) + await emailConfig.recordStatus({ status: 'error', statusDetail: describeSendError(err, config) }) throw err } } @@ -171,6 +234,7 @@ async function sendPasswordReset({ to, resetUrl, username }) { await transport.sendMail({ from: fromHeader(config), to, + replyTo: replyToFor(config), subject: `Reset your ${brand.name} password`, text: `We received a request to reset the password${forWhom} at ${brand.name}.\n\n` + @@ -182,7 +246,7 @@ async function sendPasswordReset({ to, resetUrl, username }) { return { sent: true } } catch (err) { log.error('password reset send failed', err) - await emailConfig.recordStatus({ status: 'error', statusDetail: err.message }) + await emailConfig.recordStatus({ status: 'error', statusDetail: describeSendError(err, config) }) throw err } } @@ -234,6 +298,7 @@ async function sendTeamNotification({ to, subject, intro, items, teamUrl, unsubs await transport.sendMail({ from: fromHeader(config), to, + replyTo: replyToFor(config), subject, text: lines.join('\n'), // The header carries the API url, not the one in the body: a one-click @@ -252,7 +317,7 @@ async function sendTeamNotification({ to, subject, intro, items, teamUrl, unsubs // 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(() => {}) + await emailConfig.recordStatus({ status: 'error', statusDetail: describeSendError(err, config) }).catch(() => {}) return { sent: false, reason: 'SEND_FAILED' } } } diff --git a/server/src/utils/teamNotify.js b/server/src/utils/teamNotify.js index e4294e5..0869014 100644 --- a/server/src/utils/teamNotify.js +++ b/server/src/utils/teamNotify.js @@ -211,8 +211,8 @@ async function forumPost({ team, threadId, threadTitle, type, authorUserId, auth * 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. + * — and checked BEFORE the recipient query so a deployment with no mail + * transport configured 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 @@ -227,8 +227,8 @@ async function emailImmediate({ team, threadId, threadTitle, type, exclude, auth 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 + // against a relay with its own rate limits, and a burst of them from a + // busy thread is how a sending account 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({ diff --git a/server/test/checkNoExternalHosts.test.js b/server/test/checkNoExternalHosts.test.js new file mode 100644 index 0000000..23f0aa4 --- /dev/null +++ b/server/test/checkNoExternalHosts.test.js @@ -0,0 +1,98 @@ +// Self-test for scripts/checkNoExternalHosts.js — ENGAGEMENT.md §3.2 rule 4. +// +// The same discipline checkModuleIdentifiers.test.js established: feed the +// checker code it MUST reject and code it MUST accept, because a check that +// silently stops checking is worse than no check. The rejection cases below are +// the exact shape of the literal this phase deleted. + +const { test } = require('node:test') +const assert = require('node:assert/strict') + +const { checkFile, maskComments, isAllowed, run } = require('../../scripts/checkNoExternalHosts') + +const hostsIn = (src) => checkFile('fake.js', src).map((h) => h.host) + +test('catches the literal this phase deleted', () => { + const src = ` + const transport = nodemailer.createTransport({ + host: 'smtp.gmail.com', + port: 465, + secure: true, + }) + ` + assert.deepEqual(hostsIn(src), ['smtp.gmail.com']) +}) + +test('catches an API base url, whatever the scheme', () => { + assert.deepEqual(hostsIn(`const BASE = 'https://api.mailgun.net/v3'`), ['api.mailgun.net']) + assert.deepEqual(hostsIn(`const relay = "smtps://mail.somewhere.io:465"`), ['mail.somewhere.io']) +}) + +test('catches a default sender address', () => { + assert.deepEqual(hostsIn(`const FROM = 'noreply@runicgateway.com'`), ['runicgateway.com']) +}) + +test('catches a host in a template literal', () => { + assert.deepEqual(hostsIn('const url = `https://api.postmarkapp.com/email`'), ['api.postmarkapp.com']) +}) + +test('reports the line the literal is on', () => { + const src = ['// a comment', '', "const h = 'smtp.sendgrid.net'"].join('\n') + assert.deepEqual(checkFile('fake.js', src), [ + { file: 'fake.js', line: 3, literal: 'smtp.sendgrid.net', host: 'smtp.sendgrid.net' }, + ]) +}) + +// ── the accept cases: the whole reason it reads code, not prose ───────────── + +test('a host named in a line comment is fine — that is the documentation this phase owes', () => { + assert.deepEqual(hostsIn(`// Gmail still works as plain SMTP: smtp.gmail.com:587 with an app password\nconst x = 1`), []) +}) + +test('a host named in a block comment is fine', () => { + assert.deepEqual(hostsIn(`/*\n * See https://mailgun.com/docs for the relay posture.\n */\nconst x = 1`), []) +}) + +test('example.com placeholders are allowed — a form hint is not a destination', () => { + assert.deepEqual(hostsIn(`const f = { placeholder: 'smtp.example.com' }`), []) + assert.deepEqual(hostsIn(`const f = { placeholder: 'noreply@example.com' }`), []) +}) + +test('loopback is allowed', () => { + assert.deepEqual(hostsIn(`const dev = 'http://127.0.0.1:3000'`), []) + assert.deepEqual(hostsIn(`const dev = 'http://localhost:1025'`), []) +}) + +test('a module path is not a hostname', () => { + assert.deepEqual(hostsIn(`const m = require('../model/emailConfig/emailConfig.model')`), []) + assert.deepEqual(hostsIn(`const n = require('nodemailer')`), []) + assert.deepEqual(hostsIn(`import x from './transports/smtp.js'`), []) +}) + +test('an ordinary sentence with a full stop is not a hostname', () => { + assert.deepEqual(hostsIn(`const msg = 'Send failed. Check the host and port.'`), []) +}) + +// ── the pieces, directly ──────────────────────────────────────────────────── + +test('maskComments blanks comments but keeps string bodies and line count', () => { + const src = "// smtp.gmail.com\nconst h = 'smtp.relay.net'\n" + const masked = maskComments(src) + assert.equal(masked.split('\n').length, src.split('\n').length) + assert.ok(!masked.includes('smtp.gmail.com')) + assert.ok(masked.includes('smtp.relay.net')) +}) + +test('isAllowed covers the reserved documentation names and nothing else', () => { + assert.equal(isAllowed('example.com'), true) + assert.equal(isAllowed('mail.example.org'), true) + assert.equal(isAllowed('localhost'), true) + assert.equal(isAllowed('smtp.gmail.com'), false) + assert.equal(isAllowed('api.postmarkapp.com'), false) +}) + +// ── and the real tree ─────────────────────────────────────────────────────── + +test('the shipped engagement tree is clean', () => { + assert.deepEqual(run(), []) +}) diff --git a/server/test/emailConfig.model.test.js b/server/test/emailConfig.model.test.js index 3d62677..2a24bf9 100644 --- a/server/test/emailConfig.model.test.js +++ b/server/test/emailConfig.model.test.js @@ -1,3 +1,7 @@ +// model/emailConfig — the credential store. Same guarantees as before the Gmail +// removal (ciphertext at rest, never returned, blank means "leave it alone"), +// now over one transport-shaped blob instead of a single refresh-token column. + process.env.SECRET_ENC_KEY = process.env.SECRET_ENC_KEY || 'unit-test-enc-key' process.env.DB_HOST = '127.0.0.1' process.env.DB_PORT = '59999' @@ -12,59 +16,122 @@ const db = require('../src/utils/db') after(() => db.close()) +const CRED = { host: 'relay.example.com', port: 587, secure: false, user: 'apikey', password: 'sec-abc' } + // In-memory stand-in for the singleton row so the model never touches MariaDB. let store beforeEach(() => { store = null emailDb.get = async () => store emailDb.upsert = async (fields) => { - store = { ...(store || { id: 1 }), ...fields } + store = { ...(store || { id: 1, transport: 'smtp' }), ...fields } return store } }) -test('save encrypts the refresh token (ciphertext at rest, decryptable)', async () => { - await emailConfig.save({ senderEmail: 'me@gmail.com', refreshToken: 'refresh-abc', enabled: true }) - assert.ok(store.refresh_token_enc) - assert.notEqual(store.refresh_token_enc, 'refresh-abc') - assert.equal(secretBox.decrypt(store.refresh_token_enc), 'refresh-abc') +const decrypted = () => JSON.parse(secretBox.decrypt(store.credential_enc)) + +test('save encrypts the credential (ciphertext at rest, decryptable)', async () => { + await emailConfig.save({ senderEmail: 'mail@shard.example.com', credential: CRED, enabled: true }) + assert.ok(store.credential_enc) + assert.ok(!String(store.credential_enc).includes('sec-abc')) + assert.deepEqual(decrypted(), CRED) const withSecret = await emailConfig.getWithSecret() - assert.equal(withSecret.refreshToken, 'refresh-abc') + assert.equal(withSecret.credentialSecret.password, 'sec-abc') }) -test('getSafe never leaks the refresh token', async () => { - await emailConfig.save({ senderEmail: 'me@gmail.com', refreshToken: 'refresh-abc', enabled: true }) +test('getSafe never leaks a secret field, but says which are set', async () => { + await emailConfig.save({ senderEmail: 'mail@shard.example.com', credential: CRED, enabled: true }) const safe = await emailConfig.getSafe() - assert.equal(safe.hasRefreshToken, true) - assert.equal(safe.senderEmail, 'me@gmail.com') - assert.equal('refreshToken' in safe, false) - assert.equal('refresh_token_enc' in safe, false) + + assert.deepEqual(safe.credential, { host: 'relay.example.com', port: 587, secure: false, user: 'apikey' }) + assert.equal('password' in safe.credential, false) + assert.deepEqual(safe.secretsSet, { password: true }) + assert.equal(safe.hasCredential, true) + assert.equal(safe.senderEmail, 'mail@shard.example.com') + assert.equal('credentialSecret' in safe, false) + assert.equal('credential_enc' in safe, false) }) -test('blank refresh token on save leaves the existing one unchanged', async () => { - await emailConfig.save({ senderEmail: 'me@gmail.com', refreshToken: 'refresh-abc', enabled: true }) - const cipherBefore = store.refresh_token_enc +test('a blank secret leaves the stored one unchanged; other fields still save', async () => { + await emailConfig.save({ credential: CRED }) + const cipherBefore = store.credential_enc - await emailConfig.save({ senderName: 'UOMysticmoon' }) // no refreshToken - assert.equal(store.refresh_token_enc, cipherBefore) // untouched + await emailConfig.save({ senderName: 'UOMysticmoon', credential: { ...CRED, password: '' } }) assert.equal(store.sender_name, 'UOMysticmoon') - assert.equal(secretBox.decrypt(store.refresh_token_enc), 'refresh-abc') + assert.equal(decrypted().password, 'sec-abc') + assert.notEqual(store.credential_enc, undefined) + assert.ok(cipherBefore) }) -test('disconnect clears the credential and disables sending', async () => { - await emailConfig.save({ senderEmail: 'me@gmail.com', refreshToken: 'refresh-abc', enabled: true }) +test('undeclared keys are dropped — a client cannot smuggle fields into the blob', async () => { + await emailConfig.save({ credential: { ...CRED, evil: 'x', proxy: 'http://attacker' } }) + assert.deepEqual(Object.keys(decrypted()).sort(), ['host', 'password', 'port', 'secure', 'user']) +}) + +test('changing transport does not carry the old credential across', async () => { + await emailConfig.save({ credential: CRED }) + // An unregistered target still clears rather than merging: leaving an SMTP + // password inside another transport's blob would be a stored secret nobody can + // see and nothing will ever use. + await emailConfig.save({ transport: 'mailgun', credential: { domain: 'x' } }) + assert.equal(store.transport, 'mailgun') + assert.equal(store.credential_enc, null) +}) + +test('an incomplete credential is stored but is not "complete"', async () => { + // A username with no password authenticates as nobody. + await emailConfig.save({ credential: { host: 'relay.example.com', port: 587, user: 'apikey' } }) + const safe = await emailConfig.getSafe() + assert.equal(safe.hasCredential, false) + assert.deepEqual(safe.secretsSet, { password: false }) +}) + +test('an unreadable blob reads as absent, never as an error', async () => { + // The rotated-SECRET_ENC_KEY case. It must land the admin on a screen that says + // "unconfigured", not a 500 that takes the contact form down with it. + store = { id: 1, transport: 'smtp', credential_enc: 'not-ciphertext', enabled: 1 } + const safe = await emailConfig.getSafe() + assert.equal(safe.hasCredential, false) + assert.deepEqual(safe.credential, {}) +}) + +test('disconnect clears the credential, the legacy token and the enabled flag', async () => { + store = { id: 1, transport: 'smtp', refresh_token_enc: 'old-gmail-cipher', enabled: 1 } + await emailConfig.save({ senderEmail: 'mail@shard.example.com', credential: CRED, enabled: true }) + const safe = await emailConfig.disconnect(7) + assert.equal(store.credential_enc, null) assert.equal(store.refresh_token_enc, null) assert.equal(store.enabled, 0) - assert.equal(safe.hasRefreshToken, false) + assert.equal(safe.hasCredential, false) + assert.equal(safe.hadLegacyConnection, false) assert.equal(safe.status, 'unconfigured') }) -test('getSafe returns unconfigured defaults when no row exists', async () => { +test('hadLegacyConnection is the G22 warning condition, and nothing else', async () => { + // Present token + no replacement credential: this deployment's mail just + // stopped and it has to be told (ENGAGEMENT.md §1.2a consequence 3). + store = { id: 1, transport: 'smtp', refresh_token_enc: 'old-gmail-cipher', enabled: 1 } + let safe = await emailConfig.getSafe() + assert.equal(safe.hadLegacyConnection, true) + assert.equal(safe.hasCredential, false) + + // Once SMTP is configured the pair stops matching, so the warning goes away + // without anything having to clear the deprecated column. + await emailConfig.save({ senderEmail: 'mail@shard.example.com', credential: CRED }) + safe = await emailConfig.getSafe() + assert.equal(safe.hadLegacyConnection, true) + assert.equal(safe.hasCredential, true) +}) + +test('a fresh install is unconfigured, on the default transport, and warns nobody', async () => { const safe = await emailConfig.getSafe() assert.equal(safe.enabled, false) - assert.equal(safe.hasRefreshToken, false) + assert.equal(safe.transport, 'smtp') + assert.equal(safe.hasCredential, false) + assert.equal(safe.hadLegacyConnection, false) assert.equal(safe.status, 'unconfigured') assert.equal(safe.senderEmail, null) }) diff --git a/server/test/mailTransports.test.js b/server/test/mailTransports.test.js new file mode 100644 index 0000000..deecb13 --- /dev/null +++ b/server/test/mailTransports.test.js @@ -0,0 +1,109 @@ +// The mail transport registry (ENGAGEMENT.md §3.1) and the one transport core +// ships. The registry's job is that `credentialFields` is the single declaration +// the admin form, the sanitizer and the "is it a secret" answer all read — so +// most of what is asserted here is that nothing else knows a field name. + +process.env.SECRET_ENC_KEY = process.env.SECRET_ENC_KEY || 'unit-test-enc-key' +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const { test, after } = require('node:test') +const assert = require('node:assert/strict') + +const { transports } = require('../src/engagement') +const db = require('../src/utils/db') + +after(() => db.close()) + +const SMTP_CRED = { host: 'relay.example.com', port: 587, secure: false, user: 'apikey', password: 'sec' } + +test('requiring the subsystem is what registers core\'s transports', () => { + assert.equal(transports.has('smtp'), true) + const [smtp] = transports.describe() + assert.equal(smtp.id, 'smtp') + assert.ok(smtp.credentialFields.length > 0) +}) + +test('describe() carries no functions and no secret values', () => { + const [smtp] = transports.describe() + assert.equal(typeof smtp.build, 'undefined') + assert.equal(typeof smtp.isComplete, 'undefined') + const password = smtp.credentialFields.find((f) => f.key === 'password') + assert.equal(password.kind, 'secret') + assert.equal('value' in password, false) +}) + +test('sanitizeCredential drops undeclared keys', () => { + const out = transports.sanitizeCredential('smtp', { ...SMTP_CRED, evil: 'x' }) + assert.deepEqual(Object.keys(out).sort(), ['host', 'password', 'port', 'secure', 'user']) +}) + +test('sanitizeCredential coerces to the declared kind', () => { + const out = transports.sanitizeCredential('smtp', { host: 'relay.example.com', port: '587', secure: 'yes' }) + assert.equal(out.port, 587) + assert.equal(out.secure, true) +}) + +test('an empty secret is omitted, so a merge keeps the stored one', () => { + const patch = transports.sanitizeCredential('smtp', { ...SMTP_CRED, password: '' }) + assert.equal('password' in patch, false) + const merged = transports.mergeCredential('smtp', { password: 'stored' }, patch) + assert.equal(merged.password, 'stored') +}) + +test('publicCredential and secretsPresent split the blob the way the API needs', () => { + assert.deepEqual(transports.publicCredential('smtp', SMTP_CRED), { + host: 'relay.example.com', port: 587, secure: false, user: 'apikey', + }) + assert.deepEqual(transports.secretsPresent('smtp', SMTP_CRED), { password: true }) + assert.deepEqual(transports.secretsPresent('smtp', { host: 'x' }), { password: false }) +}) + +test('an unknown transport is a safe no-op everywhere, never a throw', () => { + assert.equal(transports.get('nope'), null) + assert.equal(transports.isComplete('nope', SMTP_CRED), false) + assert.deepEqual(transports.sanitizeCredential('nope', SMTP_CRED), {}) + assert.deepEqual(transports.publicCredential('nope', SMTP_CRED), {}) + assert.deepEqual(transports.secretsPresent('nope', SMTP_CRED), {}) +}) + +// ── smtp's own completeness rule ──────────────────────────────────────────── + +test('smtp needs a destination, and auth is all-or-nothing', () => { + assert.equal(transports.isComplete('smtp', SMTP_CRED), true) + // A local MTA needs no credentials at all. + assert.equal(transports.isComplete('smtp', { host: 'mta.example.com', port: 25 }), true) + // A username with no password authenticates as nobody and fails at the server. + assert.equal(transports.isComplete('smtp', { host: 'relay.example.com', port: 587, user: 'apikey' }), false) + assert.equal(transports.isComplete('smtp', { port: 587 }), false) + assert.equal(transports.isComplete('smtp', {}), false) +}) + +test('smtp declares no default host — §3.2 rule 1, as a test', () => { + const [smtp] = transports.describe() + const host = smtp.credentialFields.find((f) => f.key === 'host') + assert.equal(host.default, null) + assert.equal(host.required, true) +}) + +// ── registration is validated at the call ─────────────────────────────────── + +test('registration rejects a bad shape and a collision', () => { + const ok = { id: 'fake', label: 'Fake', credentialFields: [{ key: 'k', kind: 'text' }], build: () => {}, isComplete: () => true } + assert.throws(() => transports.registerMailTransport({ ...ok, id: 'Not Valid' }), /invalid id/) + assert.throws(() => transports.registerMailTransport({ ...ok, label: '' }), /label required/) + assert.throws(() => transports.registerMailTransport({ ...ok, credentialFields: [] }), /credentialFields required/) + assert.throws(() => transports.registerMailTransport({ ...ok, credentialFields: [{ key: 'k', kind: 'wat' }] }), /unknown kind/) + assert.throws(() => transports.registerMailTransport({ ...ok, build: undefined }), /build\(\) required/) + assert.throws(() => transports.registerMailTransport({ ...ok, id: 'smtp' }), /already registered/) +}) + +test('a registered transport is a copy — a caller cannot mutate the catalog afterwards', () => { + const fields = [{ key: 'k', label: 'K', kind: 'text', required: true }] + transports.registerMailTransport({ + id: 'tamper', label: 'Tamper', credentialFields: fields, build: () => {}, isComplete: () => true, + }) + fields[0].kind = 'secret' + const def = transports.describe().find((t) => t.id === 'tamper') + assert.equal(def.credentialFields[0].kind, 'text') +}) diff --git a/server/test/mailer.test.js b/server/test/mailer.test.js index 0807f3c..a548d44 100644 --- a/server/test/mailer.test.js +++ b/server/test/mailer.test.js @@ -1,3 +1,11 @@ +// utils/mailer.js — the transport resolution and, more importantly, the five +// failure contracts the six call sites depend on (ENGAGEMENT.md §1.2, Phase 1). +// +// The Gmail OAuth2 assertions are gone with the transport (§1.2a); what replaced +// them asserts the SAME things at the same seam — that a configured deployment +// builds the transport the operator selected from the credentials they supplied, +// and that an unconfigured one degrades exactly as before rather than throwing. + process.env.SECRET_ENC_KEY = process.env.SECRET_ENC_KEY || 'unit-test-enc-key' process.env.DB_HOST = '127.0.0.1' process.env.DB_PORT = '59999' @@ -7,66 +15,167 @@ const assert = require('node:assert/strict') const nodemailer = require('nodemailer') const emailConfig = require('../src/model/emailConfig/emailConfig.model') -const authProviders = require('../src/model/authProviders/authProviders.model') const settings = require('../src/model/settings/settings.model') const mailer = require('../src/utils/mailer') const db = require('../src/utils/db') after(() => db.close()) -// Restore a clean slate of stubs before each test. -beforeEach(() => { - emailConfig.recordStatus = async () => {} - settings.get = async () => 'contact@example.com' +// A complete SMTP config, as getWithSecret returns it. +const configured = (over = {}) => ({ + transport: 'smtp', + enabled: true, + senderEmail: 'mail@shard.example.com', + senderName: 'UOMysticmoon', + replyTo: null, + hasCredential: true, + credentialSecret: { host: 'relay.example.com', port: 587, secure: false, user: 'apikey', password: 'sec' }, + ...over, }) -test('unconfigured → mailto fallback (never throws)', async () => { +let sent +let transportCfg + +beforeEach(() => { + sent = null + transportCfg = null + emailConfig.recordStatus = async () => {} + settings.get = async () => 'contact@example.com' + nodemailer.createTransport = (cfg) => { + transportCfg = cfg + return { sendMail: async (opts) => { sent = opts; return { messageId: '1' } } } + } +}) + +// ── the five failure contracts ────────────────────────────────────────────── + +test('unconfigured → contact form falls back to mailto (never throws)', async () => { emailConfig.getWithSecret = async () => null - emailConfig.getSafe = async () => ({ senderEmail: null, hasRefreshToken: false, enabled: false }) + emailConfig.getSafe = async () => ({ senderEmail: null, hasCredential: false, enabled: false }) const r = await mailer.sendContactMessage({ name: 'Ann', email: 'ann@player.com', message: 'hi' }) assert.deepEqual(r, { sent: false, fallback: 'mailto', email: 'contact@example.com' }) }) -test('configured → builds a Gmail OAuth2 transport and sends', async () => { - let transportCfg = null - let sent = null - nodemailer.createTransport = (cfg) => { - transportCfg = cfg - return { sendMail: async (opts) => { sent = opts; return { messageId: '1' } } } - } - emailConfig.getWithSecret = async () => ({ refreshToken: 'rt-123', senderEmail: 'shard@gmail.com', senderName: 'UOMysticmoon' }) - authProviders.getWithSecret = async (id) => { - assert.equal(id, 'google') - return { client_id: 'cid', client_secret: 'csec' } - } +test('unconfigured → invite returns NOT_CONFIGURED so the admin gets the link', async () => { + emailConfig.getWithSecret = async () => null + const r = await mailer.sendInvite({ to: 'a@b.com', acceptUrl: 'https://x/y' }) + assert.deepEqual(r, { sent: false, reason: 'NOT_CONFIGURED' }) +}) + +test('unconfigured → password reset returns NOT_CONFIGURED (caller still answers 200)', async () => { + emailConfig.getWithSecret = async () => null + const r = await mailer.sendPasswordReset({ to: 'a@b.com', resetUrl: 'https://x/y', username: 'ann' }) + assert.deepEqual(r, { sent: false, reason: 'NOT_CONFIGURED' }) +}) + +test('unconfigured → team notification returns NOT_CONFIGURED and never throws', async () => { + emailConfig.getWithSecret = async () => null + const r = await mailer.sendTeamNotification({ to: 'a@b.com', subject: 's', intro: 'i', items: [] }) + assert.deepEqual(r, { sent: false, reason: 'NOT_CONFIGURED' }) +}) + +test('unconfigured → only sendTest throws, because only sendTest has an admin waiting', async () => { + emailConfig.getWithSecret = async () => null + await assert.rejects(() => mailer.sendTest('a@b.com'), (err) => err.code === 'NOT_CONFIGURED') +}) + +// ── transport resolution ──────────────────────────────────────────────────── + +test('configured → builds the selected transport from the stored credential and sends', async () => { + emailConfig.getWithSecret = async () => configured() const r = await mailer.sendContactMessage({ name: 'Ann', email: 'ann@player.com', message: 'hi there' }) assert.equal(r.sent, true) - // Transport is Gmail SMTP over XOAUTH2 with the reused Google client + stored refresh token. - assert.equal(transportCfg.host, 'smtp.gmail.com') - assert.equal(transportCfg.port, 465) - assert.equal(transportCfg.secure, true) - assert.equal(transportCfg.auth.type, 'OAuth2') - assert.equal(transportCfg.auth.user, 'shard@gmail.com') - assert.equal(transportCfg.auth.clientId, 'cid') - assert.equal(transportCfg.auth.clientSecret, 'csec') - assert.equal(transportCfg.auth.refreshToken, 'rt-123') + // Every connection detail comes from the operator's credential. Nothing in the + // code chooses a host, a port or a TLS mode (§3.2 rule 1). + assert.equal(transportCfg.host, 'relay.example.com') + assert.equal(transportCfg.port, 587) + assert.equal(transportCfg.secure, false) + assert.deepEqual(transportCfg.auth, { user: 'apikey', pass: 'sec' }) - // From uses the display name; To is the contact_email setting; replyTo is the sender. - assert.equal(sent.from, '"UOMysticmoon" ') + // From uses the display name; To is the contact_email setting; replyTo is the + // visitor, which still wins over a configured Reply-To. + assert.equal(sent.from, '"UOMysticmoon" ') assert.equal(sent.to, 'contact@example.com') assert.equal(sent.replyTo, 'ann@player.com') }) +test('an unauthenticated relay gets no auth block', async () => { + emailConfig.getWithSecret = async () => configured({ + credentialSecret: { host: 'mta.example.com', port: 25, secure: false }, + }) + await mailer.sendInvite({ to: 'a@b.com', acceptUrl: 'https://x/y' }) + assert.equal(transportCfg.auth, undefined) +}) + +test('the configured Reply-To is used when the caller has none', async () => { + emailConfig.getWithSecret = async () => configured({ replyTo: 'staff@shard.example.com' }) + await mailer.sendPasswordReset({ to: 'a@b.com', resetUrl: 'https://x/y', username: 'ann' }) + assert.equal(sent.replyTo, 'staff@shard.example.com') +}) + +test('disabled is unconfigured — the toggle gates every sender, not just isConfigured', async () => { + emailConfig.getWithSecret = async () => configured({ enabled: false }) + emailConfig.getSafe = async () => ({ senderEmail: 'mail@shard.example.com', hasCredential: true, enabled: false }) + + const r = await mailer.sendContactMessage({ name: 'A', email: 'a@b.com', message: 'x' }) + assert.equal(r.sent, false) + assert.equal(r.fallback, 'mailto') +}) + +test('an incomplete credential is unconfigured, not a crash', async () => { + // A username with no password authenticates as nobody; smtp.isComplete says no. + emailConfig.getWithSecret = async () => configured({ + credentialSecret: { host: 'relay.example.com', port: 587, user: 'apikey' }, + }) + const r = await mailer.sendInvite({ to: 'a@b.com', acceptUrl: 'https://x/y' }) + assert.deepEqual(r, { sent: false, reason: 'NOT_CONFIGURED' }) +}) + +test('a stored transport id that is not registered degrades, it does not throw', async () => { + emailConfig.getWithSecret = async () => configured({ transport: 'mailgun' }) + const r = await mailer.sendTeamNotification({ to: 'a@b.com', subject: 's', intro: 'i', items: [] }) + assert.deepEqual(r, { sent: false, reason: 'NOT_CONFIGURED' }) +}) + +// ── failures ──────────────────────────────────────────────────────────────── + test('send failure propagates and is recorded', async () => { let recorded = null emailConfig.recordStatus = async (s) => { recorded = s } nodemailer.createTransport = () => ({ sendMail: async () => { throw new Error('smtp boom') } }) - emailConfig.getWithSecret = async () => ({ refreshToken: 'rt', senderEmail: 'shard@gmail.com', senderName: null }) - authProviders.getWithSecret = async () => ({ client_id: 'cid', client_secret: 'csec' }) + emailConfig.getWithSecret = async () => configured() await assert.rejects(() => mailer.sendContactMessage({ name: 'A', email: 'a@b.com', message: 'x' }), /smtp boom/) assert.equal(recorded.status, 'error') }) + +test('a rejected sender is diagnosed by name — the failure mode SMTP introduces', async () => { + // Under the removed consent flow the sender came back from the provider and was + // guaranteed to belong to the credential. Operator-typed, it can be refused, + // and "550 5.7.1" alone does not tell anyone why (§1.2a consequence 2). + let recorded = null + emailConfig.recordStatus = async (s) => { recorded = s } + nodemailer.createTransport = () => ({ + sendMail: async () => { + const err = new Error('Sender address rejected') + err.responseCode = 550 + throw err + }, + }) + emailConfig.getWithSecret = async () => configured() + + await assert.rejects(() => mailer.sendTest('a@b.com'), /mail@shard\.example\.com/) + assert.match(recorded.statusDetail, /SPF\/DMARC/) +}) + +test('a team notification failure is swallowed, never thrown', async () => { + emailConfig.recordStatus = async () => {} + nodemailer.createTransport = () => ({ sendMail: async () => { throw new Error('relay down') } }) + emailConfig.getWithSecret = async () => configured() + + const r = await mailer.sendTeamNotification({ to: 'a@b.com', subject: 's', intro: 'i', items: [] }) + assert.deepEqual(r, { sent: false, reason: 'SEND_FAILED' }) +})