Signup forms: require a sender for double opt-in, error instead of a fake "Check your inbox"

apps/api/src/forms/forms.test.ts

@@ -10,6 +10,8 @@ const mocks = {
   fireEvent: mock(async () => undefined),
   consumeRateLimit: mock(async () => true),
   createTransactionalEmail: mock(async (_input: any) => ({})),
+  resolveDeliverySource: mock(async (..._args: any[]): Promise<any> => ({})),
+  captureError: mock((_input: any) => undefined),
 };
 mock.module("../automation/fire-event", () => ({ fireEvent: mocks.fireEvent }));
 mock.module("../services/rate-counter", () => ({
@@ -18,6 +20,16 @@ mock.module("../services/rate-counter", () => ({
 mock.module("../transactional/queries", () => ({
   createTransactionalEmail: mocks.createTransactionalEmail,
 }));
+const realDelivery = await import("../delivery/queries");
+mock.module("../delivery/queries", () => ({
+  ...realDelivery,
+  resolveDeliverySource: mocks.resolveDeliverySource,
+}));
+const realObservability = await import("../observability");
+mock.module("../observability", () => ({
+  ...realObservability,
+  captureError: mocks.captureError,
+}));
 
 const { eq } = await import("drizzle-orm");
 const { db } = await import("../db/client");
@@ -28,9 +40,15 @@ const { addOrStrengthenSuppression } = await import(
   "../delivery-feedback/suppression-queries"
 );
 const { findContactByEmail } = await import("../contacts/queries");
-const { confirmSignup, createForm, submitSignup, toPublicForm } = await import(
-  "./queries"
-);
+const {
+  confirmSignup,
+  createForm,
+  FormSenderError,
+  submitSignup,
+  toPublicForm,
+  toPublicForms,
+  updateForm,
+} = await import("./queries");
 const { default: publicRoutes } = await import("./public-routes");
 type TestDb = import("../test/db").TestDb;
 
@@ -54,6 +72,9 @@ const confirmationToken = () =>
 beforeEach(async () => {
   mocks.fireEvent.mockClear();
   mocks.createTransactionalEmail.mockClear();
+  mocks.createTransactionalEmail.mockImplementation(async () => ({}));
+  mocks.resolveDeliverySource.mockImplementation(async () => ({}));
+  mocks.captureError.mockClear();
   mocks.consumeRateLimit.mockImplementation(async () => true);
   await truncateAll(tdb);
 });
@@ -282,4 +303,98 @@ describe("public form routes", () => {
       (await requestApp(app(), "/f/confirm/nope", { method: "POST" })).status,
     ).toBe(404);
   });
+
+  describe("without a sender", () => {
+    const noSender = () =>
+      mocks.resolveDeliverySource.mockImplementation(async () => {
+        throw new Error("delivery_source_unavailable");
+      });
+
+    it("refuses to create or switch on double opt-in, but allows single opt-in", async () => {
+      const { team } = await seedTeamAndContact(tdb);
+      noSender();
+
+      const error = await createForm(team.id, { name: "Newsletter" }).catch((e) => e);
+      expect(error).toBeInstanceOf(FormSenderError);
+      expect(error.message).toBe("confirmation_sender_unavailable");
+      expect(error.issue).toContain("no active sending mailbox");
+
+      const single = await createForm(team.id, { name: "Single", doubleOptIn: false });
+      expect(
+        await updateForm(team.id, single.formId, { doubleOptIn: true }).catch((e) => e),
+      ).toBeInstanceOf(FormSenderError);
+      expect(
+        (await updateForm(team.id, single.formId, { name: "Renamed" }))?.name,
+      ).toBe("Renamed");
+    });
+
+    it("flags double opt-in forms that can no longer send", async () => {
+      const { team } = await seedTeamAndContact(tdb);
+      const double = await createForm(team.id, { name: "Double" });
+      const single = await createForm(team.id, { name: "Single", doubleOptIn: false });
+      // An already double opt-in form can still be edited once the sender goes.
+      noSender();
+      await updateForm(team.id, double.formId, { name: "Double!", doubleOptIn: true });
+
+      const [doubleView, singleView] = await toPublicForms(team.id, [double, single]);
+      expect(doubleView.sendingIssue).toContain("no active sending mailbox");
+      expect(singleView.sendingIssue).toBeNull();
+      mocks.resolveDeliverySource.mockImplementation(async () => ({}));
+      expect((await toPublicForms(team.id, [double]))[0].sendingIssue).toBeNull();
+    });
+
+    it("answers every submission with the same error and reports it", async () => {
+      const { team } = await seedTeamAndContact(tdb, { team: { name: "Acme" } });
+      const form = await createForm(team.id, { name: "Newsletter" });
+      await addOrStrengthenSuppression({
+        teamId: team.id,
+        recipientEmail: "gone@example.com",
+        reason: "hard_bounce",
+        actorType: "system",
+      });
+      noSender();
+      const post = (body: Record<string, string>) =>
+        requestApp(app(), `/f/${form.formId}`, {
+          method: "POST",
+          headers: { "content-type": "application/json" },
+          body: JSON.stringify(body),
+        });
+
+      for (const body of <Record<string, string>[]>[
+        { email: "new@example.com" },
+        { email: "gone@example.com" },
+        { email: "bot@example.com", website: "spam" },
+      ]) {
+        const response = await post(body);
+        expect(response.status).toBe(503);
+        expect(response.json()).toMatchObject({ error: "form_unavailable" });
+      }
+      const page = await formPost(`/f/${form.formId}`, { email: "html@example.com" });
+      expect(page.status).toBe(503);
+      expect(page.body).toContain("can&#39;t accept signups right now");
+      expect(mocks.createTransactionalEmail).not.toHaveBeenCalled();
+      expect(mocks.captureError).toHaveBeenCalledTimes(4);
+      expect(mocks.captureError.mock.calls[0][0]).toMatchObject({
+        source: "forms.submit",
+        teamId: team.id,
+        context: { form_id: form.formId },
+      });
+    });
+
+    it("reports a confirmation email that fails to send", async () => {
+      const { team } = await seedTeamAndContact(tdb);
+      const form = await createForm(team.id, { name: "Newsletter" });
+      mocks.createTransactionalEmail.mockImplementation(async () => {
+        throw new Error("esp_not_configured");
+      });
+
+      expect(
+        await submitSignup({ form, teamName: "Acme", email: "new@example.com" }),
+      ).toBe("unavailable");
+      expect(mocks.captureError.mock.calls[0][0]).toMatchObject({
+        source: "forms.confirmation_email",
+        teamId: team.id,
+      });
+    });
+  });
 });

apps/api/src/forms/public-routes.ts

@@ -3,7 +3,13 @@ import { captureError } from "../observability";
 import logger from "../services/log";
 import { page } from "../tracking/unsubscribe-page";
 import { escapeHtml } from "../utils/html";
-import { confirmSignup, getPublicForm, type SignupForm, submitSignup } from "./queries";
+import {
+  confirmationSenderIssue,
+  confirmSignup,
+  getPublicForm,
+  type SignupForm,
+  submitSignup,
+} from "./queries";
 
 const router = Router();
 
@@ -41,6 +47,8 @@ function safeRedirect(form: SignupForm): string | null {
 
 const wantsHtml = (req: Request) => !req.is("application/json");
 
+const CANT_SEND = "This form can't accept signups right now. Please try again later.";
+
 // The emailed link lands on a page with a button, so link scanners that
 // open it don't subscribe anyone.
 router.get("/f/confirm/:token", (_req: Request, res: Response) => {
@@ -101,9 +109,23 @@ router.post("/f/:formId", async (req: Request, res: Response) => {
         : res.status(404).json({ error: "form_not_found" });
     }
     const body = (req.body ?? {}) as Record<string, unknown>;
+    // Checked before looking at the address, so every visitor (and bot)
+    // gets the same answer and the form still can't reveal who's listed.
+    const senderIssue = found.form.doubleOptIn
+      ? await confirmationSenderIssue(found.form.teamId)
+      : null;
+    if (senderIssue) {
+      captureError({
+        error: new Error(`confirmation_sender_unavailable: ${senderIssue}`),
+        source: "forms.submit",
+        teamId: found.form.teamId,
+        context: { form_id: found.form.formId },
+      });
+    }
     // Bots fill the hidden field; humans can't see it. Pretend it worked.
-    const outcome =
-      typeof body.website === "string" && body.website
+    const outcome = senderIssue
+      ? "unavailable"
+      : typeof body.website === "string" && body.website
         ? found.form.doubleOptIn
           ? "confirmation_sent"
           : "subscribed"
@@ -119,6 +141,8 @@ router.post("/f/:formId", async (req: Request, res: Response) => {
         return res.status(400).json({ error: "invalid_email" });
       if (outcome === "rate_limited")
         return res.status(429).json({ error: "too_many_requests" });
+      if (outcome === "unavailable")
+        return res.status(503).json({ error: "form_unavailable", message: CANT_SEND });
       return res.status(200).json({ status: outcome });
     }
     if (outcome === "invalid") {
@@ -141,6 +165,12 @@ router.post("/f/:formId", async (req: Request, res: Response) => {
           ),
         );
     }
+    if (outcome === "unavailable") {
+      return res
+        .status(503)
+        .type("html")
+        .send(formPage(found.form.formId, found.teamName, CANT_SEND));
+    }
     const redirect = safeRedirect(found.form);
     if (redirect) return res.redirect(303, redirect);
     res.type("html").send(donePage(found.teamName, found.form.doubleOptIn));

apps/api/src/forms/queries.ts

@@ -4,8 +4,9 @@ import { emailDomain, isDisposableDomain } from "../abuse/email-domains";
 import { createContact, findContactByEmail, updateContact } from "../contacts/queries";
 import { db } from "../db/client";
 import { signupConfirmations, signupForms, teams } from "../db/schema";
+import { resolveDeliverySource } from "../delivery/queries";
 import { getActiveSuppressionReasons } from "../delivery-feedback/suppression-queries";
-import { captureEvent } from "../observability";
+import { captureError, captureEvent } from "../observability";
 import logger from "../services/log";
 import { consumeRateLimit } from "../services/rate-counter";
 import { createTransactionalEmail } from "../transactional/queries";
@@ -15,20 +16,71 @@ import { getSiteUrl } from "../utils/mail";
 
 export type SignupForm = typeof signupForms.$inferSelect;
 
-/** A form as the API returns it, with its hosted page URL. */
-export function toPublicForm(form: SignupForm) {
+/** A form as the API returns it, with its hosted page URL. `sendingIssue`
+ * is why a double opt-in form can't email confirmations right now. */
+export function toPublicForm(form: SignupForm, sendingIssue: string | null = null) {
   return {
     formId: form.formId,
     name: form.name,
     tags: form.tags,
     doubleOptIn: form.doubleOptIn,
     redirectUrl: form.redirectUrl,
     url: `${getSiteUrl()}/f/${form.formId}`,
+    sendingIssue: form.doubleOptIn ? sendingIssue : null,
     createdAt: form.createdAt.toISOString(),
     updatedAt: form.updatedAt.toISOString(),
   };
 }
 
+/** The team's forms as the API returns them, checking the sender once. */
+export async function toPublicForms(teamId: string, forms: SignupForm[]) {
+  const issue = forms.some((form) => form.doubleOptIn)
+    ? await confirmationSenderIssue(teamId)
+    : null;
+  return forms.map((form) => toPublicForm(form, issue));
+}
+
+function senderIssueMessage(code: string): string {
+  switch (code) {
+    case "delivery_source_unavailable":
+    case "organization_delivery_disabled":
+    case "team_esp_disabled":
+    case "esp_not_configured":
+      return "This team has no active sending mailbox to send confirmation emails from. Add one under Settings → Email service providers.";
+    case "delivery_source_required":
+      return "This team can send from more than one mailbox and has no default. Choose a default sending mailbox in Settings.";
+    case "team_sending_suspended":
+    case "organization_sending_suspended":
+    case "sending_paused":
+      return "Sending is paused for this team, so confirmation emails can't go out.";
+    default:
+      return "This team can't send email right now, so confirmation emails can't go out.";
+  }
+}
+
+/** Why the team can't email double opt-in confirmations right now, or null.
+ * Uses the same sender choice as the confirmation email itself. */
+export async function confirmationSenderIssue(teamId: string): Promise<string | null> {
+  try {
+    await resolveDeliverySource(teamId, undefined, "transactional");
+    return null;
+  } catch (error) {
+    return senderIssueMessage((error as Error).message);
+  }
+}
+
+/** A double opt-in form was turned on for a team that can't send. */
+export class FormSenderError extends Error {
+  constructor(readonly issue: string) {
+    super("confirmation_sender_unavailable");
+  }
+}
+
+async function assertCanSendConfirmations(teamId: string) {
+  const issue = await confirmationSenderIssue(teamId);
+  if (issue) throw new FormSenderError(issue);
+}
+
 const CONFIRMATION_TTL_MS = 3 * 24 * 60 * 60 * 1000;
 /** Submissions per client IP per hour, per form. */
 const IP_LIMIT_PER_HOUR = 10;
@@ -80,6 +132,7 @@ export async function createForm(
     redirectUrl?: string | null;
   },
 ): Promise<SignupForm> {
+  if (input.doubleOptIn ?? true) await assertCanSendConfirmations(teamId);
   const [row] = await db
     .insert(signupForms)
     .values({
@@ -103,6 +156,11 @@ export async function updateForm(
     redirectUrl?: string | null;
   },
 ): Promise<SignupForm | null> {
+  if (input.doubleOptIn) {
+    const current = await getForm(teamId, formId);
+    if (!current) return null;
+    if (!current.doubleOptIn) await assertCanSendConfirmations(teamId);
+  }
   const [row] = await db
     .update(signupForms)
     .set({
@@ -131,7 +189,8 @@ export type SignupOutcome =
   | "confirmation_sent"
   | "subscribed"
   | "rate_limited"
-  | "invalid";
+  | "invalid"
+  | "unavailable";
 
 /** Subscribes (or resubscribes) a contact through a form and adds its tags.
  * Creating the contact fires "subscriber added" sequences, e.g. a welcome
@@ -160,7 +219,8 @@ async function subscribe(form: SignupForm, email: string, name?: string | null)
  * addresses straight away but never resubscribes someone who unsubscribed.
  * Suppressed, disposable and rate-limited submissions are dropped quietly:
  * the caller always gets the same answer, so the form can't be used to find
- * out who is on the list.
+ * out who is on the list. Callers check `confirmationSenderIssue` first, so
+ * a team that can't send answers `unavailable` to every address alike.
  */
 export async function submitSignup(input: {
   form: SignupForm;
@@ -231,6 +291,13 @@ export async function submitSignup(input: {
       { form_id: input.form.formId, error: (error as Error).message },
       "signup confirmation email not sent",
     );
+    captureError({
+      error,
+      source: "forms.confirmation_email",
+      teamId: input.form.teamId,
+      context: { form_id: input.form.formId },
+    });
+    return "unavailable";
   }
   captureEvent({
     event: "signup_form_confirmation_sent",