From 45c82ec3693e9d588c41fa64a6a0bc7a0158da71 Mon Sep 17 00:00:00 2001 From: Wanjohi Date: Fri, 16 Oct 2026 00:53:54 +0300 Subject: [PATCH] feat(auth): limit how often one address may ask for a device code Starting a device grant takes no secret and costs the caller nothing, so one address could create grants as fast as it liked: rows nobody will ever answer, and a wider target for anyone guessing user codes. Guesses were already limited per address; asking was not. POST /device/authorize now allows 10 requests per address per 10 minutes by default (`deviceVerification.requestLimit` and `requestWindow`), counted the same way as wrong user codes. Past that it answers 429 with `Retry-After` and an OAuth error body using `slow_down`, the code a device client already treats as "wait". --- apps/auth/WIP-installer.md | 1 + packages/auth/src/issuer.ts | 48 ++++++++++++++++++++++--- packages/auth/test/device.test.ts | 58 ++++++++++++++++++++++++++++++- 3 files changed, 102 insertions(+), 5 deletions(-) diff --git a/apps/auth/WIP-installer.md b/apps/auth/WIP-installer.md index c8be319c..f26e5187 100644 --- a/apps/auth/WIP-installer.md +++ b/apps/auth/WIP-installer.md @@ -4,6 +4,7 @@ Branch: feat/installer-sign-in ## Done - 1. installer device client, 15 min, no refresh (engine `clientTTL`, apps/auth `clientTTL`) +- 2. per-address limit on POST /device/authorize (engine `deviceVerification.requestLimit`), 429 slow_down + Retry-After ## Left 1. issuer: `installer` device client, 15 min access, no refresh token diff --git a/packages/auth/src/issuer.ts b/packages/auth/src/issuer.ts index 89b7c78a..90821c68 100644 --- a/packages/auth/src/issuer.ts +++ b/packages/auth/src/issuer.ts @@ -461,7 +461,17 @@ export interface IssuerInput< /** Length of the window, in seconds. @default 600 */ guessWindow?: number; /** - * Which caller a guess is charged to. + * Device codes one caller may ask for per window. + * + * Asking costs nothing and takes no secret, so without a limit one + * caller can fill the store with grants nobody will ever answer, and + * hold the user code space open for guessing. @default 10 + */ + requestLimit?: number; + /** Length of that window, in seconds. @default 600 */ + requestWindow?: number; + /** + * Which caller a guess, or a request for a code, 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 @@ -603,6 +613,8 @@ export function issuer< const deviceStore = input.deviceStore ?? MemoryDeviceStore(); const deviceGuessLimit = input.deviceVerification?.guessLimit ?? 10; 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) => @@ -889,16 +901,25 @@ export function issuer< * makes the limit worth having rather than a way to lock one person out. */ async function chargeGuess(req: Request): Promise { + const next = await charge('oauth:device:guess', req, deviceGuessWindow); + return next.count <= deviceGuessLimit; + } + + /** + * One more event against this caller's counter under `name`, in a window + * of `window` seconds that starts with the first of them. + */ + async function charge(name: string, req: Request, window: number) { const who = deviceAddress(req) ?? 'unknown'; - const key = ['oauth:device:guess', who]; + const key = [name, 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 }; + : { count: 1, resetAt: now + window * 1000 }; await Storage.set(storage!, key, next, Math.max(1, Math.ceil((next.resetAt - now) / 1000))); - return next.count <= deviceGuessLimit; + return next; } async function guessesLeft(req: Request): Promise { @@ -1451,6 +1472,25 @@ export function issuer< if (input.allowDeviceClient && !(await input.allowDeviceClient(clientID, c.req.raw))) return c.json({ error: 'invalid_client', error_description: 'Unknown client_id' }, 400); + // Charged per address, like a wrong user code. The answer is + // `slow_down` because it is the one code in this grant's vocabulary + // that means "too often, wait", and a device client already knows + // what to do with it; 429 and `Retry-After` say the same thing to + // anything that only reads HTTP. + const requests = await charge('oauth:device:request', c.req.raw, deviceRequestWindow); + if (requests.count > deviceRequestLimit) { + const wait = Math.max(1, Math.ceil((requests.resetAt - Date.now()) / 1000)); + const minutes = Math.ceil(wait / 60); + c.header('Retry-After', String(wait)); + return c.json( + { + error: 'slow_down', + error_description: `Too many sign-in requests from this network. Try again in ${minutes} minute${minutes === 1 ? '' : 's'}.` + }, + 429 + ); + } + // Not `randomUUID`: a device code is the credential the tokens are // handed to, so it gets the same treatment as one — full-width // randomness, and only its hash is written down. diff --git a/packages/auth/test/device.test.ts b/packages/auth/test/device.test.ts index 40c887be..f84bd402 100644 --- a/packages/auth/test/device.test.ts +++ b/packages/auth/test/device.test.ts @@ -23,7 +23,10 @@ const auth = issuer({ allow: async () => true, allowDeviceClient: async (clientID) => clientID !== 'banned', clientTTL: (clientID) => (clientID === 'one-step' ? { access: 15 * 60, refresh: 0 } : undefined), - deviceVerification: { guessLimit: 3, guessWindow: 60 }, + // Every test here asks for codes from one address at one frozen instant, + // so the request limit is widened out of the way; it has its own issuer + // below, on the defaults. + deviceVerification: { guessLimit: 3, guessWindow: 60, requestLimit: 10_000 }, providers: { dummy: { type: 'dummy', @@ -542,3 +545,56 @@ describe('guessing at user codes', () => { expect((await auth.request(`${ORIGIN}/device?user_code=ZZZZZZZD`)).status).toBe(429); }); }); + +/** + * Asking for a code costs the caller nothing, so it is charged per address + * like a wrong guess is. On the defaults, because those are what a deployment + * that never thought about it gets. + */ +describe('asking for codes', () => { + const limited = issuer({ + storage: MemoryStorage(), + subjects, + allow: async () => true, + providers: {}, + success: async (ctx) => ctx.subject('user', { userID: '123' }) + }); + + function ask(address?: string) { + return limited.request(`${ORIGIN}/device/authorize`, { + method: 'POST', + headers: { + 'content-type': 'application/x-www-form-urlencoded', + ...(address ? { 'cf-connecting-ip': address } : {}) + }, + body: new URLSearchParams({ client_id: 'desktop' }) + }); + } + + test('a caller gets ten in ten minutes, then a 429 that says when to come back', async () => { + for (let i = 0; i < 10; i++) expect((await ask('198.51.100.20')).status).toBe(200); + + const refused = await ask('198.51.100.20'); + expect(refused.status).toBe(429); + expect(refused.headers.get('retry-after')).toBe('600'); + const body = (await refused.json()) as any; + expect(body.error).toBe('slow_down'); + expect(body.error_description).toBe( + 'Too many sign-in requests from this network. Try again in 10 minutes.' + ); + }); + + test('the window ends, and another caller was never affected', async () => { + for (let i = 0; i < 11; i++) await ask('198.51.100.21'); + expect((await ask('198.51.100.21')).status).toBe(429); + expect((await ask('198.51.100.22')).status).toBe(200); + + setSystemTime(new Date(Date.now() + 601 * 1000)); + expect((await ask('198.51.100.21')).status).toBe(200); + }); + + test('a caller with no address shares one budget', async () => { + for (let i = 0; i < 10; i++) expect((await ask()).status).toBe(200); + expect((await ask()).status).toBe(429); + }); +});