feat: let a customer resend their own verification email (#110) #130

Merged
bermudalamb merged 2 commits from feature/110-resend-verification into main 2026-08-22 12:59:10 -05:00
6 changed files with 363 additions and 35 deletions
Showing only changes of commit d2e9b85dd8 - Show all commits
+50 -1
View File
@@ -1,4 +1,4 @@
import rateLimit, { ipKeyGenerator } from 'express-rate-limit'; import rateLimit, { ipKeyGenerator, MemoryStore } from 'express-rate-limit';
import { Request } from 'express'; import { Request } from 'express';
// First rate limiting in the codebase. The password-reset endpoints need it // First rate limiting in the codebase. The password-reset endpoints need it
@@ -75,3 +75,52 @@ export const clientErrorLimiter = rateLimit({
legacyHeaders: false, legacyHeaders: false,
message: { error: 'too many reports' } message: { error: 'too many reports' }
}); });
// Resending a verification email makes the server send mail on request, which
// is the same class of endpoint as password reset and needs the same treatment.
//
// Keyed on the customer id, which is tighter than either limiter above and
// sidesteps the IPv6 problem of #84 entirely: the caller is signed in, so there
// is an identity better than an address to count against, and no amount of
// moving within a delegated prefix changes it. It also means one customer
// cannot spend anyone else's allowance, which keying on IP would allow.
//
// It does NOT make the store's process-wide lifetime a non-issue for tests, as
// was assumed at first. resetDb truncates with RESTART IDENTITY, so every
// integration test's first customer is id 1 and they all share one bucket:
// three tests that each send once exhaust the allowance for the fourth. The
// store below is explicit and exported so a test can clear it, rather than
// tests being written around an allowance they cannot see.
//
// Must be mounted *after* requireCustomer. Before it, req.customerId is
// undefined and every anonymous caller would share a single bucket — the same
// collapse the passwordResetRequestLimiter comment warns about.
export function keyByCustomer(req: Request): string {
return `customer:${req.customerId ?? 'anonymous'}`;
}
// Three an hour is generous for someone who genuinely lost the mail, and
// useless to anybody hammering it. The window is longer than the 15 minutes
// used above because the failure it guards against is slower: a verification
// link lasts 24 hours, so there is no reason to want a fourth inside an hour.
const VERIFICATION_RESEND_WINDOW_MS = 60 * 60 * 1000;
const VERIFICATION_RESEND_MAX = 3;
// Exported only so the integration suite can clear it between tests. See the
// note above: recycled customer ids make the allowance leak across tests.
export const verificationResendStore = new MemoryStore();
export const verificationResendLimiter = rateLimit({
windowMs: VERIFICATION_RESEND_WINDOW_MS,
limit: VERIFICATION_RESEND_MAX,
keyGenerator: keyByCustomer,
store: verificationResendStore,
standardHeaders: 'draft-7',
legacyHeaders: false,
// Says what actually happened rather than only that a limit was hit. The mail
// almost certainly did send, so "check your spam folder" is both the more
// useful instruction and the more honest one.
message: {
error: 'we have already sent several verification emails recently. Check your spam folder, and try again later.'
}
});
+62 -32
View File
@@ -9,12 +9,46 @@ import { loadStoredTemplate } from './adminEmailTemplates';
import { MARKETING_CONSENT_TEXT, isValidEmail } from '../utils'; import { MARKETING_CONSENT_TEXT, isValidEmail } from '../utils';
import { FAVORITE_ALERTS_CONSENT_TEXT } from '../favoriteAlerts'; import { FAVORITE_ALERTS_CONSENT_TEXT } from '../favoriteAlerts';
import { asyncRoute } from '../asyncRoute'; import { asyncRoute } from '../asyncRoute';
import { passwordResetRequestLimiter } from '../rateLimit'; import { passwordResetRequestLimiter, verificationResendLimiter } from '../rateLimit';
const router = Router(); const router = Router();
const SESSION_DAYS = 30; const SESSION_DAYS = 30;
const VERIFY_TOKEN_TTL_MS = 24 * 60 * 60 * 1000;
// Registration, changing an address, and resending all need the same three
// steps: supersede any outstanding link, mint a new one, send it. Written out
// three times they would drift, and the step most likely to be forgotten is the
// first — which is the one that matters, since an un-superseded link means an
// older message in the inbox still verifies.
//
// Sending is fire-and-forget by the same rule the rest of this file follows:
// the token row is written first, so a send that fails cannot leave a customer
// believing a link exists that does not, only waiting for one that never came.
async function issueVerificationEmail(
customerId: number,
email: string,
firstName: string | null
): Promise<void> {
await pool.query(
`DELETE FROM customer_tokens WHERE customer_id = $1 AND kind = 'verify_email'`,
[customerId]
);
const token = crypto.randomBytes(24).toString('hex');
await pool.query(
`INSERT INTO customer_tokens (token, customer_id, kind, expires_at) VALUES ($1, $2, 'verify_email', $3)`,
[token, customerId, new Date(Date.now() + VERIFY_TOKEN_TTL_MS)]
);
const verifyUrl = `${process.env.PUBLIC_URL}/verify-email?token=${token}`;
const template = renderTemplate('verification', await loadStoredTemplate('verification'), {
greeting: greeting(firstName),
verifyUrl
});
sendMail(email, template.subject, template.html)
.catch(err => console.error('verify email send failed', err));
}
function setSessionCookie(res: Response, token: string) { function setSessionCookie(res: Response, token: string) {
res.cookie('rd_session', token, { res.cookie('rd_session', token, {
httpOnly: true, httpOnly: true,
@@ -99,18 +133,7 @@ router.post('/register', asyncRoute(async (req: Request, res: Response) => {
); );
const customer = rows[0]; const customer = rows[0];
const verifyToken = crypto.randomBytes(24).toString('hex'); await issueVerificationEmail(customer.id, customer.email, customer.first_name);
await pool.query(
`INSERT INTO customer_tokens (token, customer_id, kind, expires_at) VALUES ($1, $2, 'verify_email', $3)`,
[verifyToken, customer.id, new Date(Date.now() + 24 * 60 * 60 * 1000)]
);
const verifyUrl = `${process.env.PUBLIC_URL}/verify-email?token=${verifyToken}`;
const verifyTemplate = renderTemplate('verification', await loadStoredTemplate('verification'), {
greeting: greeting(customer.first_name),
verifyUrl
});
sendMail(customer.email, verifyTemplate.subject, verifyTemplate.html)
.catch(err => console.error('verify email send failed', err));
const sessionToken = await createSession(customer.id); const sessionToken = await createSession(customer.id);
setSessionCookie(res, sessionToken); setSessionCookie(res, sessionToken);
@@ -129,6 +152,28 @@ router.post('/verify-email', asyncRoute(async (req: Request, res: Response) => {
res.json({ status: 'verified' }); res.json({ status: 'verified' });
})); }));
// The limiter is mounted after requireCustomer, deliberately: it keys on
// req.customerId, which does not exist until requireCustomer has run. Mounted
// the other way round every anonymous caller would share one bucket.
router.post(
'/resend-verification',
requireCustomer,
verificationResendLimiter,
asyncRoute(async (req: Request, res: Response) => {
const { rows } = await pool.query(`SELECT * FROM customers WHERE id = $1`, [req.customerId]);
const customer = rows[0];
// Refused rather than quietly sending. A pointless email is worse than an
// answer, and the account page has no reason to offer the button here.
if (customer.email_verified) {
return res.status(400).json({ error: 'your email address is already verified' });
}
await issueVerificationEmail(customer.id, customer.email, customer.first_name);
res.status(204).end();
})
);
const RESET_TOKEN_TTL_MS = 60 * 60 * 1000; const RESET_TOKEN_TTL_MS = 60 * 60 * 1000;
// Always answers 200, whether or not the address has an account. A response // Always answers 200, whether or not the address has an account. A response
@@ -386,28 +431,13 @@ router.put('/me/email', requireCustomer, asyncRoute(async (req: Request, res: Re
[normalized, req.customerId] [normalized, req.customerId]
); );
// Supersede any outstanding link, so one already sitting in the old inbox // Supersedes any outstanding link as part of issuing the new one, so a
// cannot be used to verify the new address. // message already sitting in the old inbox cannot verify the new address.
await pool.query( //
`DELETE FROM customer_tokens WHERE customer_id = $1 AND kind = 'verify_email'`,
[req.customerId]
);
const verifyToken = crypto.randomBytes(24).toString('hex');
await pool.query(
`INSERT INTO customer_tokens (token, customer_id, kind, expires_at) VALUES ($1, $2, 'verify_email', $3)`,
[verifyToken, req.customerId, new Date(Date.now() + 24 * 60 * 60 * 1000)]
);
// Both sends happen after the row is written, never before — the same rule // Both sends happen after the row is written, never before — the same rule
// favoriteAlerts follows, so a change that failed cannot produce mail saying // favoriteAlerts follows, so a change that failed cannot produce mail saying
// it succeeded. // it succeeded.
const verifyUrl = process.env.PUBLIC_URL + '/verify-email?token=' + verifyToken; await issueVerificationEmail(req.customerId as number, normalized, customer.first_name);
const verify = renderTemplate('verification', await loadStoredTemplate('verification'), {
greeting: greeting(customer.first_name),
verifyUrl
});
sendMail(normalized, verify.subject, verify.html)
.catch(err => console.error('verify email send failed', err));
const notice = renderTemplate('emailChanged', await loadStoredTemplate('emailChanged'), { const notice = renderTemplate('emailChanged', await loadStoredTemplate('emailChanged'), {
greeting: greeting(customer.first_name), greeting: greeting(customer.first_name),
@@ -0,0 +1,150 @@
import request from 'supertest';
import app from '../../src/app';
import { pool } from '../../src/db';
import { resetDb, closeDb } from './setup/testDb';
import { verificationResendStore } from '../../src/rateLimit';
jest.mock('../../src/mailer', () => ({
sendMail: jest.fn().mockResolvedValue(undefined)
}));
import { sendMail } from '../../src/mailer';
const sentMail = sendMail as jest.MockedFunction<typeof sendMail>;
const PASSWORD = 'supersecret123';
beforeEach(async () => {
await resetDb();
sentMail.mockClear();
// resetDb truncates with RESTART IDENTITY, so every test's first customer is
// id 1 and the limiter — keyed on customer id, with a process-wide store —
// hands them all the same bucket. Without this, three tests that each send
// once leave the fourth starting at its limit.
await verificationResendStore.resetAll?.();
});
afterAll(async () => {
await pool.end();
await closeDb();
});
// Each test registers its own customer. That alone does NOT isolate the
// allowance, which is what the beforeEach above is for: RESTART IDENTITY hands
// every test the same customer id, so "a fresh customer" is a fresh row with a
// recycled identity. This is the same class of leakage #62 and #84 recorded,
// surviving a key that looked like it had solved it.
async function register(email: string) {
const agent = request.agent(app);
const res = await agent
.post('/api/customers/register')
.send({ email, password: PASSWORD, firstName: 'Thom', lastName: 'Lamb' });
expect(res.status).toBe(200);
sentMail.mockClear();
return agent;
}
const tokensFor = async (email: string) => {
const { rows } = await pool.query(
`SELECT t.token FROM customer_tokens t
JOIN customers c ON c.id = t.customer_id
WHERE c.email = $1 AND t.kind = 'verify_email'`,
[email]
);
return rows.map(r => r.token as string);
};
describe('resending your own verification email', () => {
it('refuses an unauthenticated caller', async () => {
const res = await request(app).post('/api/customers/resend-verification');
expect(res.status).toBe(401);
expect(sentMail).not.toHaveBeenCalled();
});
it('sends to the address on the account', async () => {
const email = 'resend1@example.com';
const agent = await register(email);
const res = await agent.post('/api/customers/resend-verification');
expect(res.status).toBe(204);
expect(sentMail).toHaveBeenCalledTimes(1);
expect(String(sentMail.mock.calls[0][0])).toBe(email);
});
// The point of the whole thing. An un-superseded link means a message still
// sitting in the inbox goes on working, which is the case the supersede in
// issueVerificationEmail exists to prevent.
it('mints a new token and invalidates the previous one', async () => {
const email = 'resend2@example.com';
const agent = await register(email);
const [before] = await tokensFor(email);
expect(before).toBeDefined();
await agent.post('/api/customers/resend-verification');
const after = await tokensFor(email);
expect(after).toHaveLength(1);
expect(after[0]).not.toBe(before);
// And the old link is genuinely dead, asserted through the endpoint that
// would honour it rather than by counting rows.
const stale = await request(app).post('/api/customers/verify-email').send({ token: before });
expect(stale.status).toBe(400);
});
it('the new link verifies the address', async () => {
const email = 'resend3@example.com';
const agent = await register(email);
await agent.post('/api/customers/resend-verification');
const [token] = await tokensFor(email);
const res = await request(app).post('/api/customers/verify-email').send({ token });
expect(res.status).toBe(200);
const me = await agent.get('/api/customers/me');
expect(me.body.email_verified).toBe(true);
});
it('refuses once the address is already verified', async () => {
const email = 'resend4@example.com';
const agent = await register(email);
const [token] = await tokensFor(email);
await request(app).post('/api/customers/verify-email').send({ token });
sentMail.mockClear();
const res = await agent.post('/api/customers/resend-verification');
expect(res.status).toBe(400);
expect(res.body.error).toBe('your email address is already verified');
expect(sentMail).not.toHaveBeenCalled();
});
// Three per hour. The fourth is refused, and the message says what actually
// happened rather than only that a limit exists.
it('stops after the allowance, with a message worth reading', async () => {
const agent = await register('resend5@example.com');
for (let i = 0; i < 3; i++) {
expect((await agent.post('/api/customers/resend-verification')).status).toBe(204);
}
const fourth = await agent.post('/api/customers/resend-verification');
expect(fourth.status).toBe(429);
expect(String(fourth.body.error)).toContain('spam folder');
// Refused rather than merely reported: the fourth send must not have gone.
expect(sentMail).toHaveBeenCalledTimes(3);
});
// The allowance is per customer, not per address or per caller. Keying it any
// more coarsely would let one customer spend everybody else's.
it('one customer exhausting the allowance does not affect another', async () => {
const first = await register('resend6@example.com');
for (let i = 0; i < 3; i++) await first.post('/api/customers/resend-verification');
expect((await first.post('/api/customers/resend-verification')).status).toBe(429);
const second = await register('resend7@example.com');
expect((await second.post('/api/customers/resend-verification')).status).toBe(204);
});
});
+25 -2
View File
@@ -1,4 +1,4 @@
import { useEffect } from 'react'; import { useEffect, useState } from 'react';
import Typography from 'antd/es/typography'; import Typography from 'antd/es/typography';
import Switch from 'antd/es/switch'; import Switch from 'antd/es/switch';
import Button from 'antd/es/button'; import Button from 'antd/es/button';
@@ -7,7 +7,7 @@ import message from 'antd/es/message';
import Space from 'antd/es/space'; import Space from 'antd/es/space';
import Divider from 'antd/es/divider'; import Divider from 'antd/es/divider';
import { useNavigate } from 'react-router-dom'; import { useNavigate } from 'react-router-dom';
import { updateConsent, exportMyData, deleteMyAccount } from './customerApi'; import { updateConsent, exportMyData, deleteMyAccount, resendVerificationEmail } from './customerApi';
import { setFavoriteAlerts } from './favoritesApi'; import { setFavoriteAlerts } from './favoritesApi';
import { useCustomerAuth } from './CustomerAuthContext'; import { useCustomerAuth } from './CustomerAuthContext';
import AccountDetails from './AccountDetails'; import AccountDetails from './AccountDetails';
@@ -23,6 +23,7 @@ interface Props {
export default function Account({ onClose }: Props) { export default function Account({ onClose }: Props) {
const { customer, loading, refresh, logout } = useCustomerAuth(); const { customer, loading, refresh, logout } = useCustomerAuth();
const navigate = useNavigate(); const navigate = useNavigate();
const [resending, setResending] = useState(false);
useEffect(() => { useEffect(() => {
if (!loading && !customer) navigate('/login'); if (!loading && !customer) navigate('/login');
@@ -30,6 +31,20 @@ export default function Account({ onClose }: Props) {
if (!customer) return null; if (!customer) return null;
async function handleResendVerification() {
setResending(true);
try {
await resendVerificationEmail();
message.success('Sent. Check your inbox, and your spam folder.');
} catch (err) {
// Shown as it arrives: the rate limit's message says the mail probably
// did send and where to look, which a generic failure would throw away.
message.error((err as Error).message);
} finally {
setResending(false);
}
}
async function handleFavoriteAlertsToggle(checked: boolean) { async function handleFavoriteAlertsToggle(checked: boolean) {
try { try {
await setFavoriteAlerts(checked); await setFavoriteAlerts(checked);
@@ -99,9 +114,17 @@ export default function Account({ onClose }: Props) {
> >
<div> <div>
<Text>{customer.email}</Text> <Text>{customer.email}</Text>
{/* The button only exists while there is something to verify. Offering
it on a verified account would be a control whose only outcome is a
refusal. */}
{!customer.email_verified && ( {!customer.email_verified && (
<div style={{ marginTop: 8 }}> <div style={{ marginTop: 8 }}>
<Text type="warning">Email not verified check your inbox for a verification link.</Text> <Text type="warning">Email not verified check your inbox for a verification link.</Text>
<div style={{ marginTop: 8 }}>
<Button size="small" loading={resending} onClick={handleResendVerification}>
Send it again
</Button>
</div>
</div> </div>
)} )}
+13
View File
@@ -142,3 +142,16 @@ export function changeMyEmail(currentPassword: string, email: string): Promise<C
body: JSON.stringify({ currentPassword, email }) body: JSON.stringify({ currentPassword, email })
}).then(res => handle<Customer>(res)); }).then(res => handle<Customer>(res));
} }
export function resendVerificationEmail(): Promise<void> {
// 204 on success, so handle() would throw parsing an empty body. The failure
// path must still reject: the server's message distinguishes "already
// verified" from the rate limit's "check your spam folder", and both are
// worth showing rather than replacing with something generic.
return fetch('/api/customers/resend-verification', { method: 'POST' }).then(async (res) => {
if (!res.ok) {
const data = await res.json().catch(() => ({}));
throw new Error(data.error || 'Request failed');
}
});
}
@@ -0,0 +1,63 @@
import { test, expect, Page } from './fixtures';
const PASSWORD = 'supersecret123';
const uniqueEmail = () => `resend-${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}@example.com`;
// The generous wait matches the other account specs: registration is a bcrypt
// round-trip rather than a render, and runs past Playwright's 5s default when
// the suite's workers all register at once.
async function registerCustomer(page: Page): Promise<string> {
const email = uniqueEmail();
await page.goto('/register');
await page.getByRole('textbox', { name: 'Email' }).fill(email);
await page.getByRole('textbox', { name: 'First name' }).fill('Test');
await page.getByRole('textbox', { name: 'Last name' }).fill('Customer');
await page.getByLabel('Password').fill(PASSWORD);
await page.getByRole('button', { name: 'Create account' }).click();
await expect(page.getByRole('button', { name: 'My Account' })).toBeVisible({ timeout: 20000 });
return email;
}
const accountModal = (page: Page) => page.getByRole('dialog', { name: 'My Account' });
test.describe('Resending your verification email', () => {
test('the account page offers it while the address is unverified', async ({ page }) => {
await registerCustomer(page);
await page.goto('/account');
const modal = accountModal(page);
await expect(modal.getByText('Email not verified')).toBeVisible();
await expect(modal.getByRole('button', { name: 'Send it again' })).toBeVisible();
});
test('confirms when it has sent', async ({ page }) => {
await registerCustomer(page);
await page.goto('/account');
await accountModal(page).getByRole('button', { name: 'Send it again' }).click();
await expect(page.getByText('Check your inbox, and your spam folder.')).toBeVisible();
});
// The message the customer gets on the fourth attempt is the point of the
// limiter's copy: it says the mail probably did send and where to look,
// rather than only that a limit exists.
test('says something useful once the allowance runs out', async ({ page }) => {
await registerCustomer(page);
await page.goto('/account');
const resend = accountModal(page).getByRole('button', { name: 'Send it again' });
for (let i = 0; i < 3; i++) {
await resend.click();
await expect(resend).toBeEnabled();
}
await resend.click();
// Matched on the phrase unique to the refusal. "spam folder" appears in the
// success message too, and the three stacked success toasts from the loop
// above are still on screen, so the looser match finds them instead.
await expect(page.getByText(/already sent several/)).toBeVisible();
});
});