diff --git a/packages/auth/src/issuer.ts b/packages/auth/src/issuer.ts index 05e5bee3..48def0e5 100644 --- a/packages/auth/src/issuer.ts +++ b/packages/auth/src/issuer.ts @@ -397,6 +397,31 @@ export interface IssuerInput< * store can make each transition a single operation. */ deviceStore?: DeviceStore; + /** + * How hard a caller may guess at user codes before `/device` stops + * answering them. + * + * A user code is short so that a person can read it off one screen and type + * it into another, and short means guessable given enough tries. RFC 8628 + * §5.2 asks for a limit on the verification endpoint for exactly this + * reason. Counted per caller address over a rolling window; a caller who + * gets one right is not charged for it. + */ + deviceVerification?: { + /** Wrong codes allowed per window. @default 10 */ + guessLimit?: number; + /** Length of the window, in seconds. @default 600 */ + guessWindow?: number; + /** + * Which caller a guess is charged to. + * + * Defaults to the usual forwarded-address headers. Returning undefined + * puts the request in one shared bucket, which is the right answer for + * a caller whose address cannot be established: it means stripping the + * headers buys a smaller budget rather than an unlimited one. + */ + address?(req: Request): string | undefined; + }; /** * Whether a client may start a device authorization grant. * @@ -529,6 +554,15 @@ export function issuer< const ttlDevice = input.ttl?.device ?? 60 * 10; const deviceInterval = input.ttl?.deviceInterval ?? 5; const deviceStore = input.deviceStore ?? MemoryDeviceStore(); + const deviceGuessLimit = input.deviceVerification?.guessLimit ?? 10; + const deviceGuessWindow = input.deviceVerification?.guessWindow ?? 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); if (input.theme) { setTheme(input.theme); } @@ -769,6 +803,44 @@ export function issuer< subject: DeviceGrantSubject; } + /** + * How many user codes this caller has got wrong lately. + * + * Kept in the general-purpose store rather than with the grants, because it + * is a counter and not a grant, and because being approximate is fine here: + * the number that matters is whether somebody is working through the code + * space, and a handful either way does not change the answer. A caller + * spread across several addresses gets a budget per address, which is what + * makes the limit worth having rather than a way to lock one person out. + */ + async function chargeGuess(req: Request): Promise { + const who = deviceAddress(req) ?? 'unknown'; + const key = ['oauth:device:guess', who]; + const now = Date.now(); + const bucket = await Storage.get<{ count: number; resetAt: number }>(storage!, key); + const next = + bucket && bucket.resetAt > now + ? { count: bucket.count + 1, resetAt: bucket.resetAt } + : { count: 1, resetAt: now + deviceGuessWindow * 1000 }; + await Storage.set( + storage!, + key, + next, + Math.max(1, Math.ceil((next.resetAt - now) / 1000)) + ); + return next.count <= deviceGuessLimit; + } + + async function guessesLeft(req: Request): Promise { + const who = deviceAddress(req) ?? 'unknown'; + const bucket = await Storage.get<{ count: number; resetAt: number }>(storage!, [ + 'oauth:device:guess', + who + ]); + if (!bucket || bucket.resetAt <= Date.now()) return true; + return bucket.count < deviceGuessLimit; + } + /** * The code as stored, from the code as a person typed it. * @@ -1390,8 +1462,16 @@ export function issuer< ); } + if (!(await guessesLeft(c.req.raw))) { + return c.text('Too many codes tried. Wait a while and start again from the app.', 429); + } + const found = await deviceStore.byUserCode(canonicalUserCode(raw)); if (!found || found.status !== 'pending' || found.expires <= Date.now()) { + // Charged only when the code was wrong. Getting one right costs + // nothing, so a person mistyping once and then succeeding is not + // walking towards a lockout. + await chargeGuess(c.req.raw); return c.text('That code is not valid any more. Ask the app for a new one.', 400); } diff --git a/packages/auth/test/device.test.ts b/packages/auth/test/device.test.ts index 4be67308..b559672f 100644 --- a/packages/auth/test/device.test.ts +++ b/packages/auth/test/device.test.ts @@ -21,6 +21,7 @@ const auth = issuer({ subjects, allow: async () => true, allowDeviceClient: async (clientID) => clientID !== 'banned', + deviceVerification: { guessLimit: 3, guessWindow: 60 }, providers: { dummy: { type: 'dummy', @@ -266,7 +267,11 @@ describe('approval', () => { }); test('an unknown user code does not start a provider flow', async () => { - const response = await auth.request(`${ORIGIN}/device?user_code=ZZZZZZZZ`); + // Its own address, so the budget it spends is its own — the shared + // bucket for callers with no address is asserted on further down. + const response = await auth.request(`${ORIGIN}/device?user_code=ZZZZZZZZ`, { + headers: { 'cf-connecting-ip': '198.51.100.9' } + }); expect(response.status).toBe(400); }); @@ -411,3 +416,73 @@ describe('when both halves move at once', () => { expect([a, b].filter(Boolean)).toHaveLength(1); }); }); + +/** + * Working through the code space, and what stops it. + * + * A user code is eight characters from an alphabet of twenty-five, so guessing + * one is not cheap — but it is a fixed cost, and the endpoint that checks them + * had no opinion about how often you asked. RFC 8628 §5.2 asks for one. + */ +describe('guessing at user codes', () => { + /** A caller with an address of its own, so budgets do not run together. */ + function from(address: string) { + return (userCode: string) => + auth.request(`${ORIGIN}/device?user_code=${encodeURIComponent(userCode)}`, { + headers: { 'cf-connecting-ip': address } + }); + } + + test('a caller runs out of tries', async () => { + const tries = from('198.51.100.1'); + + expect((await tries('ZZZZZZZZ')).status).toBe(400); + expect((await tries('ZZZZZZZY')).status).toBe(400); + expect((await tries('ZZZZZZZX')).status).toBe(400); + expect((await tries('ZZZZZZZW')).status).toBe(429); + }); + + test('one caller running out does not lock out another', async () => { + const noisy = from('198.51.100.2'); + for (let i = 0; i < 4; i++) await noisy(`ZZZZZZZ${'ABCD'[i]}`); + expect((await noisy('ZZZZZZZZ')).status).toBe(429); + + const grant = await started(); + const quiet = await auth.request( + `${ORIGIN}/device?user_code=${encodeURIComponent(grant.user_code)}`, + { headers: { 'cf-connecting-ip': '198.51.100.3' } } + ); + expect(quiet.status).toBe(302); + }); + + test('getting one right is not charged for', async () => { + const address = '198.51.100.4'; + const tries = from(address); + expect((await tries('ZZZZZZZZ')).status).toBe(400); + expect((await tries('ZZZZZZZY')).status).toBe(400); + + // Two wrong out of a budget of three. A correct code in between must + // not be what tips the next wrong one over. + const grant = await started(); + const right = await auth.request( + `${ORIGIN}/device?user_code=${encodeURIComponent(grant.user_code)}`, + { headers: { 'cf-connecting-ip': address } } + ); + expect(right.status).toBe(302); + + expect((await tries('ZZZZZZZX')).status).toBe(400); + expect((await tries('ZZZZZZZW')).status).toBe(429); + }); + + // A caller who strips the headers that say where they are lands in one + // shared bucket. That is deliberate: it makes hiding cost a smaller budget + // rather than buying an unlimited one. + test('a caller with no address still has a budget', async () => { + for (let i = 0; i < 3; i++) { + expect((await auth.request(`${ORIGIN}/device?user_code=ZZZZZZZ${'ABC'[i]}`)).status).toBe( + 400 + ); + } + expect((await auth.request(`${ORIGIN}/device?user_code=ZZZZZZZD`)).status).toBe(429); + }); +});