Adding e2e specs for the submission page found a defect in the page they were written for, which is what they were for. One limiter counted page loads and submissions against the same twenty-per-quarter-hour allowance, so a sender working through a box of stock ran out after ten items — the exact person the feature exists for, and the exact case the limiter's own comment said must not be refused. The comment said refusing them costs a consignment while the number quietly did it. Worse, the page could not tell a 429 from a 404. `fetchIntakeLink` treated any non-OK response as "no link", so a throttled sender was told "This link is not active" and sent to ask for a replacement — which could not have helped, because the problem was their address and a minute of patience. Two conditions needing opposite reactions were sharing a message. Now two limiters, because the two requests cost different things. Reading a link hits one indexed row and writes nothing, so that allowance is generous at 120: someone re-reading the form or losing their signal should never be told to wait. Submitting writes up to six files, so that is the one worth bounding, at 30 — more than anyone photographing items can manage and far less than a script would want. The page gains a third state. Unknown, revoked and used-up still collapse into one "not active" card, because whether a link exists is not something a stranger needs to learn. Throttled is deliberately kept apart from them, since "wait a moment" and "go and ask for another link" are opposite instructions. Measured rather than assumed, on a freshly started process both times: before, 25 page loads produced 14 rejections; after, 40 produce none. The first attempt at that measurement was wrong and worth recording — the restart had failed with EADDRINUSE, so it read 30 of 30 against the old process's already-exhausted store. The two specs now pass in a full parallel run alongside everything else. They are scoped the way #241 asks: unique run ids, assertions naming only this run's rows, nothing asserted about the table as a whole. Backend: 284 integration, 309 unit. Frontend: build clean, lint unchanged at 2 pre-existing warnings. Ref #222, #241
187 lines
8.9 KiB
TypeScript
187 lines
8.9 KiB
TypeScript
import rateLimit, { ipKeyGenerator, MemoryStore } from 'express-rate-limit';
|
|
import { Request } from 'express';
|
|
|
|
// First rate limiting in the codebase. The password-reset endpoints need it
|
|
// most: without one, anyone can make the server send unlimited mail to any
|
|
// address. Login and registration are the obvious next candidates.
|
|
//
|
|
// The default in-memory store suits a single-instance deployment, which this
|
|
// is. Running more than one app container would need a shared store, or each
|
|
// instance would enforce its own separate allowance.
|
|
|
|
const WINDOW_MS = 15 * 60 * 1000;
|
|
const MAX_REQUESTS = 5;
|
|
|
|
// Keyed on caller *and* address rather than caller alone. Keying on IP only
|
|
// would let one person's reset attempts lock out everyone behind the same
|
|
// NAT or reverse proxy — and everything here arrives via Nginx Proxy Manager,
|
|
// so a great many customers share an apparent address.
|
|
//
|
|
// This key only makes sense on a request that carries an email. Applying the
|
|
// same limiter to an endpoint without one collapses every caller into a single
|
|
// `ip:` bucket, which is a shared allowance rather than a per-caller one.
|
|
//
|
|
// The caller half goes through ipKeyGenerator rather than using req.ip raw.
|
|
// A raw IPv6 address is the full 128 bits, but a residential IPv6 customer is
|
|
// delegated an entire prefix and can source every request from a different
|
|
// address inside it for free — so keyed on the exact address this limiter
|
|
// counted each request as a new caller and never bound at all. That is not a
|
|
// small miss: this limiter is the only thing stopping anyone making the server
|
|
// send unlimited mail to any address they choose. express-rate-limit reported
|
|
// it as ERR_ERL_KEY_GEN_IPV6 on every boot; see #84.
|
|
//
|
|
// The helper's default groups IPv6 by /56 rather than /64. That is the
|
|
// deliberate choice: /56 covers a whole delegated site, so an attacker cannot
|
|
// escape the bucket by moving within their own allocation. It does mean
|
|
// several households behind one delegation share an allowance — acceptable
|
|
// here only because the key also contains the email address, so they collide
|
|
// just when targeting the same account. IPv4 is returned unchanged.
|
|
//
|
|
// Exported for the unit test. The limiter's own allowance is not worth
|
|
// asserting in a test — its store is process-wide, so exhausting it leaks into
|
|
// every later test from the same address — but the key function is pure and
|
|
// is where the bug actually was.
|
|
export function keyByCallerAndEmail(req: Request): string {
|
|
const email = typeof req.body?.email === 'string' ? req.body.email.toLowerCase().trim() : '';
|
|
return `${ipKeyGenerator(req.ip ?? '')}:${email}`;
|
|
}
|
|
|
|
export const passwordResetRequestLimiter = rateLimit({
|
|
windowMs: WINDOW_MS,
|
|
limit: MAX_REQUESTS,
|
|
keyGenerator: keyByCallerAndEmail,
|
|
standardHeaders: 'draft-7',
|
|
legacyHeaders: false,
|
|
message: { error: 'too many attempts, please try again later' }
|
|
});
|
|
|
|
// Client error reports carry no email, so this one is keyed on the caller
|
|
// alone — deliberately not reusing passwordResetRequestLimiter, whose comment
|
|
// above explains why its key is wrong for an endpoint without an email.
|
|
//
|
|
// `trust proxy` is set in app.ts, so `req.ip` is the real client address from
|
|
// X-Forwarded-For rather than Nginx Proxy Manager's, making this a per-customer
|
|
// allowance rather than one shared by everybody behind the proxy.
|
|
//
|
|
// Generous, because hitting the limit is harmless: the reporter ignores the
|
|
// response either way. It exists so a render loop cannot fill the log.
|
|
const CLIENT_ERROR_WINDOW_MS = 15 * 60 * 1000;
|
|
const CLIENT_ERROR_MAX_REQUESTS = 30;
|
|
|
|
export const clientErrorLimiter = rateLimit({
|
|
windowMs: CLIENT_ERROR_WINDOW_MS,
|
|
limit: CLIENT_ERROR_MAX_REQUESTS,
|
|
standardHeaders: 'draft-7',
|
|
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.'
|
|
}
|
|
});
|
|
|
|
/**
|
|
* Keyed on the caller alone, because an intake submission carries no email.
|
|
*
|
|
* The comment on `keyByCallerAndEmail` warns that a bare `ip:` bucket is a
|
|
* shared allowance rather than a per-caller one, and that trade is accepted
|
|
* here deliberately: the *link* is the per-caller identity, and its
|
|
* `submission_count` against `max_submissions` is the per-caller cap. This
|
|
* limiter exists for a different job — bounding what one address can throw at
|
|
* an unauthenticated endpoint that writes files to disk.
|
|
*
|
|
* ipKeyGenerator rather than `req.ip` raw, for the reason #84 records: a
|
|
* residential IPv6 customer is delegated a whole prefix and can source every
|
|
* request from a different address inside it for free, so keying on the exact
|
|
* address counts each one as a new caller and never bounds anything.
|
|
*/
|
|
export function keyByCaller(req: Request): string {
|
|
return ipKeyGenerator(req.ip ?? '');
|
|
}
|
|
|
|
/**
|
|
* Two limiters rather than one, because the two requests cost different things.
|
|
*
|
|
* Reading a link is a page load: it hits one indexed row and writes nothing.
|
|
* Submitting writes up to six files to the uploads volume. Counting them
|
|
* against a single allowance meant reloading the page consumed the budget for
|
|
* sending items, and at twenty apiece that allowance ran out after ten items —
|
|
* for exactly the person this feature is for, somebody working through a box
|
|
* of stock. The comment here used to say refusing them costs a consignment,
|
|
* while the number quietly did it.
|
|
*
|
|
* Both still key on the caller alone, since a submission carries no email. The
|
|
* `keyByCallerAndEmail` comment warns that a bare `ip:` bucket is a shared
|
|
* allowance rather than a per-caller one, and that trade is accepted here: the
|
|
* link is the per-caller identity and its `max_submissions` is the per-caller
|
|
* cap, while these bound what one address can throw at an unauthenticated
|
|
* endpoint.
|
|
*/
|
|
export const intakeViewLimiter = rateLimit({
|
|
windowMs: 15 * 60 * 1000,
|
|
// Generous, because it is a page load. Someone re-reading the form, losing
|
|
// their signal, or coming back to it should never be told to wait.
|
|
limit: 120,
|
|
keyGenerator: keyByCaller,
|
|
standardHeaders: 'draft-7',
|
|
legacyHeaders: false,
|
|
message: { error: 'too many requests — please try again shortly' }
|
|
});
|
|
|
|
export const intakeSubmitLimiter = rateLimit({
|
|
windowMs: 15 * 60 * 1000,
|
|
// Each of these writes files, so this is the one worth bounding. Thirty in a
|
|
// quarter of an hour is more than anyone photographing items can manage and
|
|
// far less than a script would want.
|
|
limit: 30,
|
|
keyGenerator: keyByCaller,
|
|
standardHeaders: 'draft-7',
|
|
legacyHeaders: false,
|
|
message: { error: 'too many submissions — please try again later' }
|
|
});
|