diff --git a/packages/auth/src/issuer.ts b/packages/auth/src/issuer.ts index 553031d1..1eba9057 100644 --- a/packages/auth/src/issuer.ts +++ b/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}. */ diff --git a/packages/auth/src/provider/code.ts b/packages/auth/src/provider/code.ts index a55bd1df..def05d79 100644 --- a/packages/auth/src/provider/code.ts +++ b/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 = Record = Record) { 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 = Record; + 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(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(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 = Record(fn: () => T): () => T { let value: T | undefined; return () => { diff --git a/packages/auth/test/code.test.ts b/packages/auth/test/code.test.ts index df13e40a..1aa60cd3 100644 --- a/packages/auth/test/code.test.ts +++ b/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, body: Record) { +async function post( + cookies: ReturnType, + body: Record, + headers: Record = {} +) { 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' });