feat: let a customer resend their own verification email (#110) #130
@@ -1,4 +1,4 @@
|
||||
import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
|
||||
import rateLimit, { ipKeyGenerator, MemoryStore } from 'express-rate-limit';
|
||||
import { Request } from 'express';
|
||||
|
||||
// First rate limiting in the codebase. The password-reset endpoints need it
|
||||
@@ -75,3 +75,52 @@ export const clientErrorLimiter = rateLimit({
|
||||
legacyHeaders: false,
|
||||
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.'
|
||||
}
|
||||
});
|
||||
|
||||
@@ -9,12 +9,46 @@ import { loadStoredTemplate } from './adminEmailTemplates';
|
||||
import { MARKETING_CONSENT_TEXT, isValidEmail } from '../utils';
|
||||
import { FAVORITE_ALERTS_CONSENT_TEXT } from '../favoriteAlerts';
|
||||
import { asyncRoute } from '../asyncRoute';
|
||||
import { passwordResetRequestLimiter } from '../rateLimit';
|
||||
import { passwordResetRequestLimiter, verificationResendLimiter } from '../rateLimit';
|
||||
|
||||
const router = Router();
|
||||
|
||||
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) {
|
||||
res.cookie('rd_session', token, {
|
||||
httpOnly: true,
|
||||
@@ -99,18 +133,7 @@ router.post('/register', asyncRoute(async (req: Request, res: Response) => {
|
||||
);
|
||||
const customer = rows[0];
|
||||
|
||||
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, 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));
|
||||
await issueVerificationEmail(customer.id, customer.email, customer.first_name);
|
||||
|
||||
const sessionToken = await createSession(customer.id);
|
||||
setSessionCookie(res, sessionToken);
|
||||
@@ -129,6 +152,28 @@ router.post('/verify-email', asyncRoute(async (req: Request, res: Response) => {
|
||||
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;
|
||||
|
||||
// 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]
|
||||
);
|
||||
|
||||
// Supersede any outstanding link, so one already sitting in the old inbox
|
||||
// cannot be used to 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)]
|
||||
);
|
||||
|
||||
// Supersedes any outstanding link as part of issuing the new one, so a
|
||||
// message already sitting in the old inbox cannot verify the new address.
|
||||
//
|
||||
// Both sends happen after the row is written, never before — the same rule
|
||||
// favoriteAlerts follows, so a change that failed cannot produce mail saying
|
||||
// it succeeded.
|
||||
const verifyUrl = process.env.PUBLIC_URL + '/verify-email?token=' + verifyToken;
|
||||
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));
|
||||
await issueVerificationEmail(req.customerId as number, normalized, customer.first_name);
|
||||
|
||||
const notice = renderTemplate('emailChanged', await loadStoredTemplate('emailChanged'), {
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect } from 'react';
|
||||
import { useEffect, useState } from 'react';
|
||||
import Typography from 'antd/es/typography';
|
||||
import Switch from 'antd/es/switch';
|
||||
import Button from 'antd/es/button';
|
||||
@@ -7,7 +7,7 @@ import message from 'antd/es/message';
|
||||
import Space from 'antd/es/space';
|
||||
import Divider from 'antd/es/divider';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
import { updateConsent, exportMyData, deleteMyAccount } from './customerApi';
|
||||
import { updateConsent, exportMyData, deleteMyAccount, resendVerificationEmail } from './customerApi';
|
||||
import { setFavoriteAlerts } from './favoritesApi';
|
||||
import { useCustomerAuth } from './CustomerAuthContext';
|
||||
import AccountDetails from './AccountDetails';
|
||||
@@ -23,6 +23,7 @@ interface Props {
|
||||
export default function Account({ onClose }: Props) {
|
||||
const { customer, loading, refresh, logout } = useCustomerAuth();
|
||||
const navigate = useNavigate();
|
||||
const [resending, setResending] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
if (!loading && !customer) navigate('/login');
|
||||
@@ -30,6 +31,20 @@ export default function Account({ onClose }: Props) {
|
||||
|
||||
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) {
|
||||
try {
|
||||
await setFavoriteAlerts(checked);
|
||||
@@ -99,9 +114,17 @@ export default function Account({ onClose }: Props) {
|
||||
>
|
||||
<div>
|
||||
<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 && (
|
||||
<div style={{ marginTop: 8 }}>
|
||||
<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>
|
||||
)}
|
||||
|
||||
|
||||
@@ -142,3 +142,16 @@ export function changeMyEmail(currentPassword: string, email: string): Promise<C
|
||||
body: JSON.stringify({ currentPassword, email })
|
||||
}).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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user