diff --git a/apps/auth/test/worker.test.ts b/apps/auth/test/worker.test.ts index bcc47647..2707745f 100644 --- a/apps/auth/test/worker.test.ts +++ b/apps/auth/test/worker.test.ts @@ -95,8 +95,14 @@ function jar() { }; } -/** Ask for a code, redeem it, and come back holding tokens. */ -async function signIn() { +/** + * Ask for a code, redeem it, and come back holding tokens. + * + * The address is a parameter because codes to one mailbox are rate limited, and + * two sign-ins in the same second are exactly what that limit is for. Each + * caller uses its own. + */ +async function signIn(email: string) { const client = createClient({ issuer: 'https://auth.internal', clientID: 'api', @@ -115,7 +121,7 @@ async function signIn() { const requested = await auth.request('https://auth.internal/code/authorize', { method: 'POST', headers: { cookie: cookies.header(), 'content-type': 'application/x-www-form-urlencoded' }, - body: new URLSearchParams({ action: 'request', email: 'ada@example.com' }) + body: new URLSearchParams({ action: 'request', email }) }); cookies.absorb(requested); expect(lastCode).not.toBe(''); @@ -142,7 +148,7 @@ async function signIn() { describe('signing in with an email address', () => { test('a redeemed code becomes tokens that verify', async () => { - const { client, tokens } = await signIn(); + const { client, tokens } = await signIn('ada@example.com'); expect(tokens.access).toBeString(); expect(tokens.refresh).toBeString(); @@ -161,7 +167,7 @@ describe('signing in with an email address', () => { describe('User info', () => { test('returns subject properties for valid access token', async () => { - const { tokens } = await signIn(); + const { tokens } = await signIn('grace@example.com'); const infoRes = await auth.request('https://auth.internal/userinfo', { headers: { Authorization: `Bearer ${tokens.access}` } diff --git a/packages/auth/src/provider/code.ts b/packages/auth/src/provider/code.ts index 9464dc3f..e643fb06 100644 --- a/packages/auth/src/provider/code.ts +++ b/packages/auth/src/provider/code.ts @@ -54,7 +54,8 @@ */ import { Context } from 'hono'; -import { generateUnbiasedDigits, timingSafeCompare } from '../random.js'; +import { generateUnbiasedDigits, generateUnbiasedString, timingSafeCompare } from '../random.js'; +import { Storage } from '../storage/storage.js'; import { Provider } from './provider.js'; export interface CodeProviderConfig< @@ -66,6 +67,46 @@ export interface CodeProviderConfig< * @default 6 */ length?: number; + /** + * How long a code stays usable, in seconds. + * + * A pin is six digits, which is a small space, and the only thing keeping + * it small enough to type is that it does not have to last. A code that is + * still good tomorrow is a password with a million possible values. + * + * @default 600 + */ + ttl?: number; + /** + * How many wrong guesses a code survives. + * + * Counted where the person asking cannot reach it, which is the whole + * point: the code itself travels in an encrypted cookie the caller holds, + * so a counter kept alongside it would be a counter they could reset by + * replaying an older copy. Starting over is allowed and costs them a fresh + * code — sent to the mailbox they are trying to break into, where somebody + * notices. + * + * @default 5 + */ + maxAttempts?: number; + /** + * How many codes one attempt at signing in may ask for. + * + * @default 3 + */ + maxSends?: number; + /** + * Seconds between one code and the next for the same claim. + * + * Without this, `resend` is an open relay pointed at anybody's mailbox: the + * address is not the caller's own and nothing asks them to prove otherwise, + * so the send button is a way to mail a stranger as fast as requests go + * out. + * + * @default 30 + */ + resendInterval?: number; /** * The request handler to generate the UI for the code flow. * @@ -116,6 +157,14 @@ export type CodeProviderState = resend?: boolean; code: string; claims: Record; + /** + * Names the server-side record holding this code's remaining + * guesses. Regenerated with every code, so a caller who rolls back + * to an older cookie rolls back to a code that is no longer live. + */ + flow: string; + /** When the code stops being accepted, in ms. */ + expires: number; }; /** @@ -134,16 +183,50 @@ export type CodeProviderError = type: 'invalid_claim'; key: string; value: string; + } + /** Too many guesses, or codes asked for too quickly. */ + | { + type: 'rate_limit'; }; +/** Nothing a person reads, so the whole alphabet is available. */ +const FLOW_ALPHABET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; + export function CodeProvider = Record>( config: CodeProviderConfig ): Provider<{ claims: Claims }> { const length = config.length || 6; + const ttl = config.ttl ?? 60 * 10; + const maxAttempts = config.maxAttempts ?? 5; + const maxSends = config.maxSends ?? 3; + const resendInterval = config.resendInterval ?? 30; + function generate() { return generateUnbiasedDigits(length); } + /** Where a flow's remaining guesses live, on the server. */ + function attemptKey(flow: string) { + return ['oauth:code:flow', flow]; + } + + /** + * Where the last send to one claim is remembered. + * + * Keyed by the claim and not by the caller, because the mailbox is what is + * being protected and the caller is whoever is pointing at it. Two people + * asking for a code for one address in the same minute is the case this is + * for, and it is the same case whether they are the same person or not. + */ + function claimKey(claims: 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]; + } + return { type: 'code', init(routes, ctx) { @@ -153,10 +236,14 @@ export function CodeProvider = Record(c, 'provider', 60 * 60 * 24, next); + // The cookie lives exactly as long as the code inside it. + // Twenty-four hours, which is what this was, made a six-digit + // pin usable for a day. + await ctx.set(c, 'provider', ttl, next); const resp = ctx.forward(c, await config.request(c.req.raw, next, fd, err)); return resp; } + routes.get('/authorize', async (c) => { const resp = await transition(c, { type: 'start' @@ -165,7 +252,6 @@ export function CodeProvider = Record { - const code = generate(); const fd = await c.req.formData(); const state = await ctx.get(c, 'provider'); const action = fd.get('action')?.toString(); @@ -173,22 +259,83 @@ export function CodeProvider = Record(ctx.storage, claimKey(claims)); + if (sentAt && Date.now() - sentAt.at < resendInterval * 1000) { + return transition(c, state ?? { type: 'start' }, fd, { type: 'rate_limit' }); + } + if (action === 'resend' && state?.type === 'code') { + const record = await Storage.get<{ attempts: number; sends: number }>( + ctx.storage, + attemptKey(state.flow) + ); + if ((record?.sends ?? 1) >= maxSends) { + return transition(c, state, fd, { type: 'rate_limit' }); + } + } + + const code = generate(); const err = await config.sendCode(claims, code); if (err) return transition(c, { type: 'start' }, fd, err); + + // A new code means a new flow, which means a fresh budget + // of guesses — and, more to the point, that the budget + // attached to the previous code is now unreachable rather + // than reset. + const flow = generateUnbiasedString(FLOW_ALPHABET, 32); + const sends = + action === 'resend' && state?.type === 'code' + ? (( + await Storage.get<{ sends: number }>(ctx.storage, attemptKey(state.flow)) + )?.sends ?? 1) + 1 + : 1; + await Storage.set(ctx.storage, attemptKey(flow), { attempts: 0, sends }, ttl); + await Storage.set(ctx.storage, claimKey(claims), { at: Date.now() }, resendInterval); + return transition( c, { type: 'code', resend: action === 'resend', claims, - code + code, + flow, + expires: Date.now() + ttl * 1000 }, fd ); } - if (fd.get('action')?.toString() === 'verify' && state.type === 'code') { - const fd = await c.req.formData(); + if (action === 'verify' && state?.type === 'code') { + if (state.expires <= Date.now()) { + await ctx.unset(c, 'provider'); + return transition(c, { type: 'start' }, fd, { type: 'invalid_code' }); + } + + // Counted before the comparison, so a guess costs whether or + // not it is right. Counted on the server, so the caller + // holding the cookie cannot wind it back. + const record = await Storage.get<{ attempts: number; sends: number }>( + ctx.storage, + attemptKey(state.flow) + ); + if (!record || record.attempts >= maxAttempts) { + await ctx.unset(c, 'provider'); + await Storage.remove(ctx.storage, attemptKey(state.flow)); + return transition(c, { type: 'start' }, fd, { type: 'rate_limit' }); + } + await Storage.set( + ctx.storage, + attemptKey(state.flow), + { ...record, attempts: record.attempts + 1 }, + Math.max(1, Math.ceil((state.expires - Date.now()) / 1000)) + ); + const compare = fd.get('code')?.toString(); if (!state.code || !compare || !timingSafeCompare(state.code, compare)) { return transition( @@ -201,12 +348,18 @@ export function CodeProvider = Record
{error?.type === 'invalid_claim' && } + {error?.type === 'rate_limit' && } {error?.type === 'invalid_code' && } + {error?.type === 'rate_limit' && } {state.type === 'code' && ( true, + providers: { + code: CodeProvider({ + maxAttempts: 3, + maxSends: 2, + resendInterval: 0, + request: async (_req, _state, _form, error) => + new Response(JSON.stringify({ error: error?.type ?? null }), { + status: 200, + headers: { 'content-type': 'application/json' } + }), + sendCode: async (claims, code) => { + if (!claims.email?.includes('@')) { + return { type: 'invalid_claim', key: 'email', value: claims.email ?? '' }; + } + sent.push(code); + } + }) + }, + success: async (ctx, value) => ctx.subject('user', { email: (value as any).claims.email }) +}); + +const ORIGIN = 'https://auth.example.com'; + +function jar() { + const cookies = new Map(); + return { + absorb(response: Response) { + for (const raw of response.headers.getSetCookie()) { + const [pair] = raw.split(';'); + const index = pair!.indexOf('='); + cookies.set(pair!.slice(0, index), pair!.slice(index + 1)); + } + }, + header() { + return [...cookies].map(([name, value]) => `${name}=${value}`).join('; '); + } + }; +} + +async function post(cookies: ReturnType, body: Record) { + const response = await auth.request(`${ORIGIN}/code/authorize`, { + method: 'POST', + headers: { cookie: cookies.header(), 'content-type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams(body) + }); + cookies.absorb(response); + return response; +} + +/** + * Begin an authorization the way a client does, so success has somewhere to go. + * + * Without this there is no authorization state and a correct code produces + * tokens rather than the redirect a browser flow ends in — which would make + * "did this sign in?" a different question in the test than in the product. + */ +async function begin() { + const cookies = jar(); + const url = new URL(`${ORIGIN}/authorize`); + url.searchParams.set('client_id', 'test'); + url.searchParams.set('redirect_uri', 'https://client.example.com/callback'); + url.searchParams.set('response_type', 'code'); + url.searchParams.set('provider', 'code'); + cookies.absorb(await auth.request(url.toString())); + return cookies; +} + +/** Start a sign-in and ask for a code, coming back with the cookies and the code. */ +async function ask(email = 'ada@example.com') { + const cookies = await begin(); + await post(cookies, { action: 'request', email }); + return { cookies, code: sent.at(-1)! }; +} + +/** What the stub UI reported, so a test can name the error rather than a status. */ +async function errorOf(response: Response) { + return ((await response.clone().json()) as { error: string | null }).error; +} + +beforeEach(() => { + sent = []; +}); + +describe('signing in with a code', () => { + test('the right code signs you in', async () => { + const { cookies, code } = await ask(); + const response = await post(cookies, { action: 'verify', code }); + expect(response.status).toBe(302); + }); + + test('a wrong code is refused and says so', async () => { + const { cookies, code } = await ask(); + const response = await post(cookies, { action: 'verify', code: code === '000000' ? '111111' : '000000' }); + expect(response.status).toBe(200); + expect(await errorOf(response)).toBe('invalid_code'); + }); +}); + +/** + * The attack a six-digit pin invites, and what stops it. + * + * The code travels in an encrypted cookie the caller holds, and the caller is + * not necessarily the person the code was mailed to — anybody can type somebody + * else's address into the first screen. So the only thing between an attacker + * and an account is how many times they may guess, and that number has to be + * kept somewhere they cannot reach. + */ +describe('guessing the code', () => { + test('runs out of guesses long before it runs out of codes', async () => { + const { cookies, code } = await ask(); + const wrong = code === '000000' ? '111111' : '000000'; + + expect(await errorOf(await post(cookies, { action: 'verify', code: wrong }))).toBe( + 'invalid_code' + ); + expect(await errorOf(await post(cookies, { action: 'verify', code: wrong }))).toBe( + 'invalid_code' + ); + expect(await errorOf(await post(cookies, { action: 'verify', code: wrong }))).toBe( + 'invalid_code' + ); + + // Out of budget. The next guess is refused whether or not it is right. + expect(await errorOf(await post(cookies, { action: 'verify', code: wrong }))).toBe( + 'rate_limit' + ); + }); + + test('the real code stops working once the guesses are spent', async () => { + const { cookies, code } = await ask(); + const wrong = code === '000000' ? '111111' : '000000'; + for (let i = 0; i < 3; i++) await post(cookies, { action: 'verify', code: wrong }); + + const response = await post(cookies, { action: 'verify', code }); + expect(response.status).toBe(200); + expect(await errorOf(response)).toBe('rate_limit'); + }); + + // The counter would be worthless if it lived where the guesser does. This + // replays the cookie from before any guess was made, which is the cheapest + // way to wind back anything held in one. + test('replaying an earlier cookie does not hand back the spent guesses', async () => { + const { cookies, code } = await ask(); + const untouched = cookies.header(); + const wrong = code === '000000' ? '111111' : '000000'; + for (let i = 0; i < 3; i++) await post(cookies, { action: 'verify', code: wrong }); + + const replayed = await auth.request(`${ORIGIN}/code/authorize`, { + method: 'POST', + headers: { cookie: untouched, 'content-type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ action: 'verify', code: wrong }) + }); + expect(await errorOf(replayed)).toBe('rate_limit'); + }); + + test('a code is spent when it is used, so its guesses do not carry over', async () => { + const { cookies, code } = await ask(); + expect((await post(cookies, { action: 'verify', code })).status).toBe(302); + + const again = await post(cookies, { action: 'verify', code }); + expect(again.status).toBe(200); + }); +}); + +describe('asking for codes', () => { + test('a fresh code comes with a fresh budget of guesses', async () => { + const first = await ask(); + const wrong = '000000' === first.code ? '111111' : '000000'; + for (let i = 0; i < 3; i++) await post(first.cookies, { action: 'verify', code: wrong }); + expect(await errorOf(await post(first.cookies, { action: 'verify', code: wrong }))).toBe( + 'rate_limit' + ); + + // Starting over is allowed. It costs a code sent to the mailbox being + // aimed at, which is where somebody would notice. + const second = await ask(); + expect(second.code).not.toBe(first.code); + expect((await post(second.cookies, { action: 'verify', code: second.code })).status).toBe(302); + }); + + test('one sign-in cannot ask for codes forever', async () => { + const { cookies } = await ask(); + expect(await errorOf(await post(cookies, { action: 'resend', email: 'ada@example.com' }))).toBe( + null + ); + expect(await errorOf(await post(cookies, { action: 'resend', email: 'ada@example.com' }))).toBe( + 'rate_limit' + ); + expect(sent).toHaveLength(2); + }); + + 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' }); + expect(await errorOf(response)).toBe('invalid_claim'); + expect(sent).toHaveLength(0); + }); +});