diff --git a/CloudronManifest.json b/CloudronManifest.json index 61f8c86..ec2c29b 100644 --- a/CloudronManifest.json +++ b/CloudronManifest.json @@ -4,7 +4,7 @@ "title": "Community Rule", "author": "MEDLab", "description": "Community governance and rule-building app", - "version": "0.1.11", + "version": "0.1.12", "httpPort": 3000, "healthCheckPath": "/api/health", "memoryLimit": 805306368, diff --git a/docs/guides/backend-roadmap.md b/docs/guides/backend-roadmap.md index dfc5004..0b8e528 100644 --- a/docs/guides/backend-roadmap.md +++ b/docs/guides/backend-roadmap.md @@ -227,7 +227,7 @@ npm run dev 1. TLS certificates and hostnames. _On Cloudron: handled by the platform per chosen subdomain._ 2. PostgreSQL backups and restore drill. _On Cloudron: daily snapshots; configure retention in admin UI._ -3. SMTP DNS (SPF, DKIM). _On Cloudron: handled for the platform-managed domain._ +3. SMTP DNS (SPF, DKIM). _TLS for the app hostname is Cloudron/Let's Encrypt. Mail is SES-relayed: publish SES DKIM (and SPF `include:amazonses.com`) via Cloudron Domains → Namecheap. Cloudron skips SPF/DKIM checks when a relay is configured. See [`ops-runbook.md`](ops-runbook.md) §8.1._ 4. Health check URL for reverse proxy (`/api/health`). _On Cloudron: set `healthCheckPath` in `CloudronManifest.json`._ 5. Log retention and alerts for 5xx errors. _On Cloudron: app log viewer; export off-platform if longer retention is needed._ diff --git a/docs/guides/ops-backend-deploy.md b/docs/guides/ops-backend-deploy.md index 5d01268..6074ed9 100644 --- a/docs/guides/ops-backend-deploy.md +++ b/docs/guides/ops-backend-deploy.md @@ -101,8 +101,16 @@ per-app in the manifest and provisioned at install time. - Backups: Cloudron's automatic backups are already on for the host (legacy app shows weekly snapshots ~451 MB each). Same default applies to new apps. -- TLS / DNS / SPF / DKIM: handled by Cloudron for any subdomain of - `communityrule.info`. +- TLS for Cloudron app hostnames: handled by Cloudron (Let's Encrypt). +- **Mail DNS (SPF/DKIM):** *not* automatic for this domain. Cloudron's + DNS provider for `communityrule.info` is Namecheap, but outbound mail + is **Amazon SES relay**. Cloudron's own mail-status check **skips** + SPF and DKIM and says to configure them on the relay. Add the SES + identity's **DKIM CNAME** records (and optionally + `include:amazonses.com` on SPF) in Cloudron → *Domains* → + `communityrule.info` → DNS so they publish to Namecheap. See + [`ops-runbook.md`](ops-runbook.md) §8.1. DMARC on the domain is + currently `p=reject`. ## 5. Cutover plan (side-by-side, never in-place) @@ -476,7 +484,7 @@ steps below are still required. | ------- | ------------ | ----- | | Image pull error on install | Repo still private, or wrong tag in manifest | §6.3; `docker pull --platform linux/amd64 …` from laptop | | Health `503` / `database: disconnected` | Postgres addon not provisioned or URL missing | Cloudron app → Environment; expect `CLOUDRON_POSTGRESQL_URL` | -| Magic link not sent | Mail addon or `SMTP_FROM` | Cloudron mail logs; `CLOUDRON_MAIL_SMTP_*` vars | +| Magic link not sent | Mail addon, `SMTP_FROM`, or SES DNS | Cloudron mail logs; `CLOUDRON_MAIL_SMTP_*`; [ops-runbook §8.1](ops-runbook.md#81-mail-dns-when-ses-is-the-relay) | | Upload `server_misconfigured` | `UPLOAD_ROOT` unset | Set to `/app/data/uploads` (§3) | | Container crash on start | Migration failure | App logs around `prisma migrate deploy` | | No "Recommended" on method cards | `MethodFacet` not seeded | §10 step 6; API should return `matches.score > 0` for some methods when `facet.*` set | diff --git a/docs/guides/ops-runbook.md b/docs/guides/ops-runbook.md index 5a66797..48962f6 100644 --- a/docs/guides/ops-runbook.md +++ b/docs/guides/ops-runbook.md @@ -256,13 +256,29 @@ Full detail: [`ops-backend-deploy.md` §3](ops-backend-deploy.md#3-environment-v | Image pull error on update | Private repo, wrong tag, or amd64 manifest missing | Confirm repo is public; verify pull with `--platform linux/amd64` (§3.1) | | Health `503` / `database: disconnected` | Postgres addon or `CLOUDRON_POSTGRESQL_URL` missing | Cloudron app → Environment | | Container crash on start | Migration failure | App logs around `prisma migrate deploy`; fix forward with new migration | -| Magic link not sent | Mail addon or `SMTP_FROM` | Cloudron mail logs; `CLOUDRON_MAIL_SMTP_*` vars | +| Magic link not sent | Mail addon, `SMTP_FROM`, or SES DNS | Cloudron mail logs (`CLOUDRON_MAIL_SMTP_*`); inbox/spam; SPF/DKIM for SES (§8.1) | | Upload `server_misconfigured` | `UPLOAD_ROOT` unset | `cloudron env set --app UPLOAD_ROOT=/app/data/uploads` | | No “Recommended” on method cards | Seed not run | §3.4 — `node prisma/seed.bundle.cjs` | | Rate limit too aggressive after deploy | Expected per §6.1 | Single instance only; limits reset on container restart | App logs: Cloudron dashboard → *Logs* tab, or `cloudron logs --app -f`. +### 8.1 Mail DNS when SES is the relay + +`communityrule.info` outbound mail is **Amazon SES SMTP** (`email-smtp.us-east-2.amazonaws.com:587`), not Cloudron's own MTA. Cloudron Mail → domain status therefore **skips SPF and DKIM** ("configure the relay provider") and only checks MX, DMARC (`v=DMARC1; p=reject; pct=100`), and that the SES connection works. + +That is expected. Recipients still authenticate the visible `From:` (`staging.app@communityrule.info` on staging) against **SES DKIM/SPF**, not `a:my.medlab.host`. + +**Operator steps (AWS + Cloudron DNS, not app code):** + +1. In **AWS SES** (us-east-2), open the verified identity for `communityrule.info` (create one if missing). Copy the **DKIM CNAME** records SES shows (three `*._domainkey.communityrule.info` names). +2. In **Cloudron** → *Domains* → `communityrule.info` → DNS, add those CNAMEs. Cloudron's Namecheap provider publishes them to the registrar. Confirm with `dig +short CNAME ._domainkey.communityrule.info`. +3. Optional but recommended for SPF alignment: add `include:amazonses.com` to the existing TXT SPF, e.g. `v=spf1 include:amazonses.com a:my.medlab.host ~all`. Do not remove `a:my.medlab.host` until you know nothing still sends directly from the box. +4. Leave DMARC at `p=reject` once DKIM verifies in SES; if a provider still quarantines after DKIM is live, inspect that provider's headers before relaxing DMARC. +5. Retest: request a magic link to Gmail **and** a non-Gmail inbox (May First / university). Check spam. Staging From is `Community Rule `. + +`SMTP_FROM` should stay the Cloudron mailbox (`staging.app@communityrule.info` on staging, `hello@communityrule.info` on the apex app). The app falls back to `CLOUDRON_MAIL_FROM` if `SMTP_FROM` is unset. + ## 9. Related docs - [`ops-backend-deploy.md`](ops-backend-deploy.md) — first install, cutover diff --git a/lib/create/api.ts b/lib/create/api.ts index 97fe039..5815226 100644 --- a/lib/create/api.ts +++ b/lib/create/api.ts @@ -27,6 +27,26 @@ function readApiErrorMessage(data: unknown): string { return "Request failed"; } +function retryAfterFromResponse( + res: Response, + data: unknown, +): number | undefined { + if (res.status !== 429) return undefined; + if (data && typeof data === "object" && "details" in data) { + const d = (data as { details?: unknown }).details; + if (d && typeof d === "object" && "retryAfterMs" in d) { + const ms = (d as { retryAfterMs?: unknown }).retryAfterMs; + if (typeof ms === "number" && ms > 0) return ms; + } + } + const h = res.headers.get("retry-after"); + if (h) { + const sec = Number.parseInt(h, 10); + if (!Number.isNaN(sec)) return sec * 1000; + } + return undefined; +} + export async function fetchAuthSession(): Promise<{ user: { id: string; email: string } | null; }> { @@ -54,13 +74,12 @@ export async function requestMagicLink( ...(draft && Object.keys(draft).length > 0 ? { draft } : {}), }), }); - const data = await parseJson<{ error?: string; retryAfterMs?: number }>(res); + const data: unknown = await parseJson(res); if (!res.ok) { return { ok: false, error: readApiErrorMessage(data), - retryAfterMs: - typeof data.retryAfterMs === "number" ? data.retryAfterMs : undefined, + retryAfterMs: retryAfterFromResponse(res, data), }; } return { ok: true }; @@ -85,22 +104,10 @@ export async function requestEmailChange( }); const data: unknown = await res.json().catch(() => ({})); if (!res.ok) { - let retryAfterMs: number | undefined; - if ( - res.status === 429 && - data && - typeof data === "object" && - "details" in data - ) { - const d = (data as { details?: { retryAfterMs?: unknown } }).details; - if (d && typeof d.retryAfterMs === "number") { - retryAfterMs = d.retryAfterMs; - } - } return { ok: false, error: readApiErrorMessage(data), - retryAfterMs, + retryAfterMs: retryAfterFromResponse(res, data), }; } return { ok: true }; @@ -438,26 +445,6 @@ export type RuleStakeholderMutationResult = | { ok: true } | { ok: false; error: string; status: number; retryAfterMs?: number }; -function retryAfterFromResponse( - res: Response, - data: unknown, -): number | undefined { - if (res.status !== 429) return undefined; - if (data && typeof data === "object" && "details" in data) { - const d = (data as { details?: unknown }).details; - if (d && typeof d === "object" && "retryAfterMs" in d) { - const ms = (d as { retryAfterMs?: unknown }).retryAfterMs; - if (typeof ms === "number" && ms > 0) return ms; - } - } - const h = res.headers.get("retry-after"); - if (h) { - const sec = Number.parseInt(h, 10); - if (!Number.isNaN(sec)) return sec * 1000; - } - return undefined; -} - export async function addRuleStakeholder( ruleId: string, email: string, diff --git a/lib/server/mail.ts b/lib/server/mail.ts index 29e29fe..1b904cf 100644 --- a/lib/server/mail.ts +++ b/lib/server/mail.ts @@ -2,62 +2,108 @@ import nodemailer from "nodemailer"; import { logger } from "../logger"; import { getSmtpUrl } from "./env"; -export async function sendMagicLinkEmail( - to: string, - verifyUrl: string, -): Promise { - const url = getSmtpUrl(); +function escapeHtml(value: string): string { + return value + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """); +} - if (!url) { +export function resolveMailFrom(): string { + return ( + process.env.SMTP_FROM?.trim() || + process.env.CLOUDRON_MAIL_FROM?.trim() || + "noreply@localhost" + ); +} + +/** Plaintext + HTML for one-time verify URLs. HTML `href` survives quoted-printable wrapping. */ +export function buildVerifyLinkParts( + verifyUrl: string, + intro: string, + outro: string, + linkLabel: string, +): { text: string; html: string } { + const text = `${intro}\n\n${verifyUrl}\n\n${outro}`; + const html = + `

${escapeHtml(intro).replace(/\n/g, "
")}

` + + `

${escapeHtml(linkLabel)}

` + + `

${escapeHtml(outro)}

`; + return { text, html }; +} + +async function sendHtmlMail(opts: { + to: string; + subject: string; + text: string; + html: string; + from?: string; + replyTo?: string; + devLog: string; +}): Promise { + const smtpUrl = getSmtpUrl(); + + if (!smtpUrl) { if (process.env.NODE_ENV === "development") { - logger.info(`[dev] Magic link for ${to}: ${verifyUrl}`); + logger.info(opts.devLog); return; } throw new Error("CLOUDRON_MAIL_SMTP_* is not configured"); } - const transporter = nodemailer.createTransport(url); - const from = process.env.SMTP_FROM ?? "noreply@localhost"; - + const transporter = nodemailer.createTransport(smtpUrl); await transporter.sendMail({ - from, - to, - subject: "Sign in to Community Rule", - text: `Open this link to sign in (it expires in 15 minutes):\n\n${verifyUrl}\n\nIf you did not request this, you can ignore this email.`, + from: opts.from ?? resolveMailFrom(), + to: opts.to, + subject: opts.subject, + text: opts.text, + html: opts.html, + replyTo: opts.replyTo, + }); +} + +export async function sendMagicLinkEmail( + to: string, + verifyUrl: string, +): Promise { + const { text, html } = buildVerifyLinkParts( + verifyUrl, + "Open this link to sign in (it expires in 15 minutes):", + "If you did not request this, you can ignore this email.", + "Sign in", + ); + await sendHtmlMail({ + to, + subject: "Sign in to Community Rule", + text, + html, + devLog: `[dev] Magic link for ${to}: ${verifyUrl}`, }); } -/** CR-103: confirm control of the new inbox before `User.email` is updated. */ /** Stakeholder invite after rule publish (one-time link, same dev/Mailhog pattern as magic link). */ export async function sendRuleStakeholderInviteEmail( to: string, verifyUrl: string, ruleTitle: string, ): Promise { - const url = getSmtpUrl(); - - if (!url) { - if (process.env.NODE_ENV === "development") { - logger.info( - `[dev] Rule stakeholder invite (${ruleTitle}) for ${to}: ${verifyUrl}`, - ); - return; - } - throw new Error("CLOUDRON_MAIL_SMTP_* is not configured"); - } - - const transporter = nodemailer.createTransport(url); - const from = process.env.SMTP_FROM ?? "noreply@localhost"; - - await transporter.sendMail({ - from, + const { text, html } = buildVerifyLinkParts( + verifyUrl, + `You've been invited to view "${ruleTitle}" on Community Rule.\n\nOpen this link to create your account (or sign in) and open the rule. The link expires in 15 minutes and works once:`, + "If you did not expect this, you can ignore this email.", + "Open the rule", + ); + await sendHtmlMail({ to, subject: `You're invited to view a Community Rule: ${ruleTitle}`, - text: `You've been invited to view "${ruleTitle}" on Community Rule.\n\nOpen this link to create your account (or sign in) and open the rule. The link expires in 15 minutes and works once:\n\n${verifyUrl}\n\nIf you did not expect this, you can ignore this email.`, + text, + html, + devLog: `[dev] Rule stakeholder invite (${ruleTitle}) for ${to}: ${verifyUrl}`, }); } -/** CR-107: notify support/organizers when a visitor submits the Ask an organizer form. */ +/** Notify support/organizers when a visitor submits the Ask an organizer form. */ export async function sendOrganizerInquiryNotification(params: { /** Destination inbox (e.g. from ORGANIZER_INQUIRY_TO). */ to: string; @@ -67,26 +113,18 @@ export async function sendOrganizerInquiryNotification(params: { requestId: string; }): Promise { const { to, fromEmail, visitorEmail, message, requestId } = params; - const url = getSmtpUrl(); - - if (!url) { - if (process.env.NODE_ENV === "development") { - logger.info( - `[dev] Organizer inquiry (request ${requestId}) from ${visitorEmail} to ${to}:\n${message}`, - ); - return; - } - throw new Error("CLOUDRON_MAIL_SMTP_* is not configured"); - } - - const transporter = nodemailer.createTransport(url); - - await transporter.sendMail({ - from: fromEmail, + const text = `Request ID: ${requestId}\nFrom: ${visitorEmail}\n\n${message}\n`; + const html = + `

Request ID: ${escapeHtml(requestId)}
From: ${escapeHtml(visitorEmail)}

` + + `
${escapeHtml(message)}
`; + await sendHtmlMail({ to, + from: fromEmail, replyTo: visitorEmail, subject: `Ask an organizer inquiry from ${visitorEmail}`, - text: `Request ID: ${requestId}\nFrom: ${visitorEmail}\n\n${message}\n`, + text, + html, + devLog: `[dev] Organizer inquiry (request ${requestId}) from ${visitorEmail} to ${to}:\n${message}`, }); } @@ -94,24 +132,17 @@ export async function sendEmailChangeEmail( to: string, verifyUrl: string, ): Promise { - const url = getSmtpUrl(); - - if (!url) { - if (process.env.NODE_ENV === "development") { - logger.info(`[dev] Email change verify for ${to}: ${verifyUrl}`); - return; - } - throw new Error("CLOUDRON_MAIL_SMTP_* is not configured"); - } - - const transporter = nodemailer.createTransport(url); - const from = process.env.SMTP_FROM ?? "noreply@localhost"; - - await transporter.sendMail({ - from, + const { text, html } = buildVerifyLinkParts( + verifyUrl, + "You asked to change the email on your Community Rule account.\n\nOpen this link to confirm the new address (it expires in 15 minutes):", + "If you did not request this change, you can ignore this email. Your current login is unchanged until you confirm.", + "Confirm email", + ); + await sendHtmlMail({ to, subject: "Confirm your new Community Rule email", - text: `You asked to change the email on your Community Rule account.\n\nOpen this link to confirm the new address (it expires in 15 minutes):\n\n${verifyUrl}\n\nIf you did not request this change, you can ignore this email. Your current login is unchanged until you confirm.`, + text, + html, + devLog: `[dev] Email change verify for ${to}: ${verifyUrl}`, }); } - diff --git a/messages/en/create/community/communitySave.json b/messages/en/create/community/communitySave.json index 2e14367..a80a72a 100644 --- a/messages/en/create/community/communitySave.json +++ b/messages/en/create/community/communitySave.json @@ -4,6 +4,6 @@ "placeholder": "email@domain.com", "characterCountTemplate": "{current}/{max}", "magicLinkSuccessTitle": "Check your email to log in", - "magicLinkSuccessDescription": "Your account has been created. A login link has been emailed to you.", + "magicLinkSuccessDescription": "We emailed a sign-in link. Open it on this device to continue — check spam or promotions if you don't see it.", "magicLinkErrorTitle": "Could not send link" } diff --git a/messages/en/pages/login.json b/messages/en/pages/login.json index d7819ad..7391ab9 100644 --- a/messages/en/pages/login.json +++ b/messages/en/pages/login.json @@ -7,7 +7,7 @@ "emailPlaceholder": "you@example.com", "sendMagicLink": "Send me a magic link", "successTitle": "Check your email", - "successBody": "We sent a sign-in link. Open it on this device to continue.", + "successBody": "We sent a sign-in link. Open it on this device to continue. If you don't see it, check spam or promotions.", "legalPrefix": "By continuing, you agree to our ", "legalAnd": " and ", "legalSuffix": ".", diff --git a/tests/components/LoginForm.test.tsx b/tests/components/LoginForm.test.tsx index 58e113e..aeeb6b6 100644 --- a/tests/components/LoginForm.test.tsx +++ b/tests/components/LoginForm.test.tsx @@ -125,6 +125,7 @@ describe("LoginForm", () => { await screen.findByRole("heading", { name: /check your email/i }), ).toBeInTheDocument(); expect(screen.getByText(/we sent a sign-in link/i)).toBeInTheDocument(); + expect(screen.getByText(/check spam or promotions/i)).toBeInTheDocument(); }); it("submits a long email without treating length as invalid", async () => { diff --git a/tests/unit/mail.test.ts b/tests/unit/mail.test.ts new file mode 100644 index 0000000..0d1dabb --- /dev/null +++ b/tests/unit/mail.test.ts @@ -0,0 +1,103 @@ +import { afterEach, describe, expect, it } from "vitest"; +import nodemailer from "nodemailer"; +import { + buildVerifyLinkParts, + resolveMailFrom, +} from "../../lib/server/mail"; + +const VERIFY_URL = + "https://staging.communityrule.info/api/auth/magic-link/verify?token=5IdE_BHowaw-QJj7Rwue7CbB8wDXvYITvnxRb1FGqxA"; + +function decodeQuotedPrintable(value: string): string { + return value + .replace(/=\r?\n/g, "") + .replace(/=([0-9A-Fa-f]{2})/g, (_, hex: string) => + String.fromCharCode(Number.parseInt(hex, 16)), + ); +} + +const MAIL_FROM_KEYS = ["SMTP_FROM", "CLOUDRON_MAIL_FROM"] as const; +const ORIGINAL_FROM = Object.fromEntries( + MAIL_FROM_KEYS.map((key) => [key, process.env[key]]), +) as Record<(typeof MAIL_FROM_KEYS)[number], string | undefined>; + +afterEach(() => { + for (const key of MAIL_FROM_KEYS) { + const original = ORIGINAL_FROM[key]; + if (original === undefined) delete process.env[key]; + else process.env[key] = original; + } +}); + +describe("buildVerifyLinkParts", () => { + it("puts the exact verify URL in both text and the HTML href", () => { + const { text, html } = buildVerifyLinkParts( + VERIFY_URL, + "Open this link to sign in (it expires in 15 minutes):", + "If you did not request this, you can ignore this email.", + "Sign in", + ); + expect(text).toContain(VERIFY_URL); + expect(html).toContain(`href="${VERIFY_URL}"`); + expect(html).toContain(">Sign in"); + }); + + it("escapes HTML in the intro and href", () => { + const { html } = buildVerifyLinkParts( + 'https://example.test/verify?token=a&b="c"', + 'View "Rule "', + "Ignore if unexpected.", + "Open", + ); + expect(html).toContain("View "Rule <beta>""); + expect(html).toContain( + 'href="https://example.test/verify?token=a&b="c""', + ); + }); +}); + +describe("MIME encoding of verify-link mail", () => { + it("keeps a clickable href after quoted-printable encoding", async () => { + const { text, html } = buildVerifyLinkParts( + VERIFY_URL, + "Open this link to sign in (it expires in 15 minutes):", + "If you did not request this, you can ignore this email.", + "Sign in", + ); + const transporter = nodemailer.createTransport({ + streamTransport: true, + buffer: true, + newline: "unix", + }); + const info = await transporter.sendMail({ + from: "Community Rule ", + to: "member@example.com", + subject: "Sign in to Community Rule", + text, + html, + }); + const raw = Buffer.isBuffer(info.message) + ? info.message.toString("utf8") + : String(info.message); + const decoded = decodeQuotedPrintable(raw); + expect(decoded).toContain(`href="${VERIFY_URL}"`); + expect(decoded).toContain(VERIFY_URL); + expect(decoded).not.toContain("token=3D"); + }); +}); + +describe("resolveMailFrom", () => { + it("prefers SMTP_FROM, then CLOUDRON_MAIL_FROM", () => { + delete process.env.SMTP_FROM; + delete process.env.CLOUDRON_MAIL_FROM; + expect(resolveMailFrom()).toBe("noreply@localhost"); + + process.env.CLOUDRON_MAIL_FROM = "staging.app@communityrule.info"; + expect(resolveMailFrom()).toBe("staging.app@communityrule.info"); + + process.env.SMTP_FROM = "Community Rule "; + expect(resolveMailFrom()).toBe( + "Community Rule ", + ); + }); +});