fix(auth): bound sign-in code sends per mailbox and per caller

The send budget was keyed by every submitted form field, so adding a field
with a new value each time made a fresh budget, and one mailbox could be sent
codes without limit. A claim is now built only from the claim fields (email,
phone by default); anything else is dropped before it is counted and before
it reaches sendCode.

A budget per mailbox does nothing against a caller holding a list of
addresses: each send goes to a different stranger, and the deployment's
whole sending allowance goes with them. Add a second budget per requesting
address, 10 codes per 24 hours by default. The window is a day rather than an
hour because a caller that paces itself stays under any hourly figure a
person would also fit under.

The address lookup the device flow used is moved into util so both read the
caller the same way.

packages/auth/src/issuer.ts

@@ -234,7 +234,7 @@ import { HtmlRenderer, type Renderer } from './ui/render.js';
 import { type DeviceConfirmWording, IssuerScreens } from './ui/issuer.js';
 import type { ChooseOption, Screen } from './ui/screen.js';
 import type { Theme } from './ui/theme.js';
-import { getRelativeUrl, isDomainMatch, lazy } from './util.js';
+import { getRelativeUrl, isDomainMatch, lazy, requestAddress } from './util.js';
 
 /** @internal */
 export const aws = awsHandle;
@@ -624,13 +624,7 @@ export function issuer<
 	const deviceGuessWindow = input.deviceVerification?.guessWindow ?? 600;
 	const deviceRequestLimit = input.deviceVerification?.requestLimit ?? 10;
 	const deviceRequestWindow = input.deviceVerification?.requestWindow ?? 600;
-	const deviceAddress =
-		input.deviceVerification?.address ??
-		((req: Request) =>
-			req.headers.get('cf-connecting-ip') ??
-			req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ??
-			req.headers.get('x-real-ip') ??
-			undefined);
+	const deviceAddress = input.deviceVerification?.address ?? requestAddress;
 	const renderer = input.renderer ?? HtmlRenderer({ theme: input.theme });
 
 	/** What a token for `clientID` lives for. See {@link IssuerInput.clientTTL}. */

packages/auth/src/provider/code.ts

@@ -56,6 +56,7 @@ import { Context } from 'hono';
 
 import { generateUnbiasedDigits, generateUnbiasedString, timingSafeCompare } from '../random.js';
 import { Storage } from '../storage/storage.js';
+import { requestAddress } from '../util.js';
 import { MARK_CODE } from '../ui/mark.js';
 import type { Screen } from '../ui/screen.js';
 import { Provider } from './provider.js';
@@ -121,6 +122,47 @@ export interface CodeProviderConfig<
 	 * @default 30
 	 */
 	resendInterval?: number;
+	/**
+	 * The form fields that make up a claim. Anything else submitted is dropped
+	 * before the claim is counted or sent to.
+	 *
+	 * The send budget is keyed by the claim, so a claim built from every field
+	 * in the form is a budget the caller can mint at will: one extra field
+	 * with a new value each time is a new key, and the same mailbox gets an
+	 * unbounded number of codes.
+	 *
+	 * @default ['email', 'phone']
+	 */
+	claimFields?: string[];
+	/**
+	 * How many codes one caller may have sent, to any claims, inside
+	 * {@link addressWindow}.
+	 *
+	 * The per-claim budget protects a mailbox. This one protects everything
+	 * else: without it a caller with a list of addresses can spend the whole
+	 * sending allowance of the deployment, at five codes each, on strangers.
+	 *
+	 * @default 10
+	 */
+	maxSendsPerAddress?: number;
+	/**
+	 * The window {@link maxSendsPerAddress} is counted over, in seconds.
+	 *
+	 * A day rather than the hour a mailbox gets, because the caller this is
+	 * for paces itself: a handful of addresses an hour stays under any hourly
+	 * budget a person would also fit under, and still adds up to hundreds of
+	 * strangers mailed by the end of the day.
+	 *
+	 * @default 86400
+	 */
+	addressWindow?: number;
+	/**
+	 * Who is asking, for {@link maxSendsPerAddress}. A request it returns
+	 * nothing for is not counted.
+	 *
+	 * @default the address the proxy in front reports
+	 */
+	address?: (req: Request) => string | undefined;
 	/**
 	 * What to ask for at each step of the flow.
 	 *
@@ -210,6 +252,10 @@ export function CodeProvider<Claims extends Record<string, string> = Record<stri
 	const maxSends = config.maxSends ?? 5;
 	const sendWindow = config.sendWindow ?? 60 * 60;
 	const resendInterval = config.resendInterval ?? 30;
+	const claimFields = config.claimFields ?? ['email', 'phone'];
+	const maxSendsPerAddress = config.maxSendsPerAddress ?? 10;
+	const addressWindow = config.addressWindow ?? 60 * 60 * 24;
+	const address = config.address ?? requestAddress;
 
 	function generate() {
 		return generateUnbiasedDigits(length);
@@ -230,13 +276,33 @@ export function CodeProvider<Claims extends Record<string, string> = Record<stri
 	 */
 	function claimKey(claims: Record<string, string>) {
 		const flattened = Object.entries(claims)
-			.filter(([key]) => key !== 'action')
 			.map(([key, value]) => `${key}=${String(value).trim().toLowerCase()}`)
 			.sort()
 			.join('&');
 		return ['oauth:code:claim', flattened];
 	}
 
+	/** Where the sends one caller has asked for are counted. */
+	function addressKey(value: string) {
+		return ['oauth:code:address', value];
+	}
+
+	type SendRecord = { at: number; count: number; since: number };
+
+	/** The record after one more send, opening a new window if the last one closed. */
+	function counted(record: SendRecord | null, now: number, window: number): SendRecord {
+		const open = record && now - record.since < window * 1000;
+		return {
+			at: now,
+			count: open ? record.count + 1 : 1,
+			since: open ? record.since : now
+		};
+	}
+
+	function exhausted(record: SendRecord | null, now: number, limit: number, window: number) {
+		return !!record && now - record.since < window * 1000 && record.count >= limit;
+	}
+
 	return {
 		type: 'code',
 		display: { name: 'Email', icon: MARK_CODE },
@@ -267,29 +333,35 @@ export function CodeProvider<Claims extends Record<string, string> = Record<stri
 				const action = fd.get('action')?.toString();
 
 				if (action === 'request' || action === 'resend') {
-					const claims = Object.fromEntries(fd) as Claims;
-					delete claims.action;
+					const claims = {} as Record<string, string>;
+					for (const field of claimFields) {
+						const value = fd.get(field);
+						if (typeof value === 'string') claims[field] = value;
+					}
 
-					// Asked for too soon, or too many times for this mailbox.
-					// Both answers are the same on purpose: saying which would
-					// tell a caller whether the address they typed has had a
-					// code sent to it lately, which is a fact about somebody
-					// else's mailbox.
+					// Asked for too soon, or too many times for this mailbox,
+					// or too many times from this caller. All three answers are
+					// the same on purpose: saying which would tell a caller
+					// whether the address they typed has had a code sent to it
+					// lately, which is a fact about somebody else's mailbox.
 					const now = Date.now();
-					const sent = await Storage.get<{ at: number; count: number; since: number }>(
-						ctx.storage,
-						claimKey(claims)
-					);
-					const open = sent && now - sent.since < sendWindow * 1000;
+					const sent = await Storage.get<SendRecord>(ctx.storage, claimKey(claims));
 					if (sent && now - sent.at < resendInterval * 1000) {
 						return transition(c, state ?? { type: 'start' }, fd, { type: 'rate_limit' });
 					}
-					if (open && sent.count >= maxSends) {
+					if (exhausted(sent, now, maxSends, sendWindow)) {
+						return transition(c, state ?? { type: 'start' }, fd, { type: 'rate_limit' });
+					}
+					const caller = address(c.req.raw);
+					const fromCaller = caller
+						? await Storage.get<SendRecord>(ctx.storage, addressKey(caller))
+						: null;
+					if (exhausted(fromCaller, now, maxSendsPerAddress, addressWindow)) {
 						return transition(c, state ?? { type: 'start' }, fd, { type: 'rate_limit' });
 					}
 
 					const code = generate();
-					const err = await config.sendCode(claims, code);
+					const err = await config.sendCode(claims as Claims, code);
 					if (err) return transition(c, { type: 'start' }, fd, err);
 
 					// The code that was live until a moment ago stops being
@@ -310,13 +382,17 @@ export function CodeProvider<Claims extends Record<string, string> = Record<stri
 					await Storage.set(
 						ctx.storage,
 						claimKey(claims),
-						{
-							at: now,
-							count: open ? sent.count + 1 : 1,
-							since: open ? sent.since : now
-						},
+						counted(sent, now, sendWindow),
 						sendWindow
 					);
+					if (caller) {
+						await Storage.set(
+							ctx.storage,
+							addressKey(caller),
+							counted(fromCaller, now, addressWindow),
+							addressWindow
+						);
+					}
 
 					return transition(
 						c,

packages/auth/src/util.ts

@@ -45,6 +45,23 @@ export function isDomainMatch(a: string, b: string): boolean {
 	return tailA === tailB;
 }
 
+/**
+ * The address a request came from, as the proxy in front of this server
+ * reports it.
+ *
+ * Only as trustworthy as that proxy: these headers are whatever the caller
+ * sent unless something in front overwrites them, so a deployment reachable
+ * directly should pass its own function instead.
+ */
+export function requestAddress(req: Request): string | undefined {
+	return (
+		req.headers.get('cf-connecting-ip') ??
+		req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ??
+		req.headers.get('x-real-ip') ??
+		undefined
+	);
+}
+
 export function lazy<T>(fn: () => T): () => T {
 	let value: T | undefined;
 	return () => {

packages/auth/test/code.test.ts

@@ -23,6 +23,7 @@ const auth = issuer({
 		code: CodeProvider({
 			maxAttempts: 3,
 			maxSends: 2,
+			maxSendsPerAddress: 3,
 			sendWindow: 3600,
 			resendInterval: 0,
 			request: async (_req, _state, _form, error) => ({
@@ -60,10 +61,18 @@ function jar() {
 	};
 }
 
-async function post(cookies: ReturnType<typeof jar>, body: Record<string, string>) {
+async function post(
+	cookies: ReturnType<typeof jar>,
+	body: Record<string, string>,
+	headers: Record<string, string> = {}
+) {
 	const response = await auth.request(`${ORIGIN}/code/authorize`, {
 		method: 'POST',
-		headers: { cookie: cookies.header(), 'content-type': 'application/x-www-form-urlencoded' },
+		headers: {
+			...headers,
+			cookie: cookies.header(),
+			'content-type': 'application/x-www-form-urlencoded'
+		},
 		body: new URLSearchParams(body)
 	});
 	cookies.absorb(response);
@@ -261,6 +270,45 @@ describe('asking for codes', () => {
 		expect(await errorOf(withOldCode)).toBe('rate_limit');
 	});
 
+	// The budget is keyed by the claim. When the claim was every field in the
+	// form, a field with a new value each time was a new key, and so a new
+	// budget for the same mailbox.
+	test('an extra form field does not buy more codes', async () => {
+		const email = 'padded@example.com';
+		for (let i = 0; i < 4; i++) {
+			const cookies = await begin();
+			await post(cookies, { action: 'request', email, pad: String(i) });
+		}
+		expect(sent).toHaveLength(2);
+	});
+
+	test('an extra form field does not reach the claims', async () => {
+		const cookies = await begin();
+		await post(cookies, { action: 'request', email: 'clean@example.com', role: 'admin' });
+		const response = await post(cookies, { action: 'verify', code: sent.at(-1)! });
+		expect(response.status).toBe(302);
+	});
+
+	// A list of addresses walks around a budget held per mailbox. This is the
+	// one that bounds what a single caller can spend.
+	test('one caller cannot mail a list of strangers', async () => {
+		const from = { 'cf-connecting-ip': '198.51.100.20' };
+		for (let i = 0; i < 5; i++) {
+			const cookies = await begin();
+			await post(cookies, { action: 'request', email: `stranger-${i}@example.com` }, from);
+		}
+		expect(sent).toHaveLength(3);
+
+		// Somebody else is not caught by it.
+		const other = await begin();
+		await post(
+			other,
+			{ action: 'request', email: 'neighbour@example.com' },
+			{ 'cf-connecting-ip': '198.51.100.21' }
+		);
+		expect(sent).toHaveLength(4);
+	});
+
 	test('a bad address still gets told it is a bad address', async () => {
 		const cookies = await begin();
 		const response = await post(cookies, { action: 'request', email: 'not-an-address' });