From 6fbfe71955fb9b9bd84e51bd3c8947cf5fc7189f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 25 Oct 2026 12:32:04 +0100 Subject: [PATCH] fix(auth): stop exempting a previously used address from the sign-in throttle The per-account password throttle skipped every attempt that came from an address one of the account's sessions or device logins had used in the last thirty days. An address is the easiest thing about a request to share or claim: in a household or an office the guesser has the owner's address, and a direct caller could name it in a forwarding header. Either way the throttle switched off and guessing was bounded only by the hash cost. A remembered browser (the trusted-device cookie) and a phone presenting the device id of one of the account's logins still pass; everything else gets the growing wait. --- src/app/api/auth/login/route.ts | 4 +- src/lib/auth/__tests__/login-throttle.test.ts | 33 ++++++--------- src/lib/auth/login-throttle.ts | 40 ++++++------------- 3 files changed, 28 insertions(+), 49 deletions(-) diff --git a/src/app/api/auth/login/route.ts b/src/app/api/auth/login/route.ts index 538cbee2f..4c8927f91 100644 --- a/src/app/api/auth/login/route.ts +++ b/src/app/api/auth/login/route.ts @@ -113,7 +113,8 @@ export const POST = apiHandler(async (request: NextRequest) => { } // The per-account half of the throttle (`@/lib/auth/login-throttle`): a - // place this account has signed in from is neither counted nor held back, + // remembered browser or a phone this account signed in on is neither + // counted nor held back, // anywhere else waits longer after each failure past the first few, up to // fifteen minutes. Decided before the Argon2id verification, so a waiting // attempt costs no hashing. @@ -121,7 +122,6 @@ export const POST = apiHandler(async (request: NextRequest) => { user, identifier, request, - ip: rl.ip, }); annotate({ meta: { login_known_source: attempt.source ?? "none" } }); if (attempt.waiting) { diff --git a/src/lib/auth/__tests__/login-throttle.test.ts b/src/lib/auth/__tests__/login-throttle.test.ts index 5c09dd16b..99aa4e102 100644 --- a/src/lib/auth/__tests__/login-throttle.test.ts +++ b/src/lib/auth/__tests__/login-throttle.test.ts @@ -4,7 +4,7 @@ * The case it exists for: somebody who is not the owner types wrong passwords * for the account from two addresses. That used to lock a password-only * account (the phone app's password sign-in with it) for as long as they kept - * it up. Now the owner's own device or network is not held back at all, and + * it up. Now the owner's own browser or phone is not held back at all, and * anywhere else waits a bounded, growing time. */ import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -97,8 +97,7 @@ async function guess( const attempt = await beginAccountLoginAttempt({ user: OWNER, identifier: "owner", - request: req(opts.headers), - ip, + request: req({ "x-forwarded-for": ip, ...opts.headers }), }); if (attempt.waiting) return "waiting"; if (opts.right) await attempt.succeeded(); @@ -172,24 +171,19 @@ describe("somebody guessing from two addresses", () => { expect(await guess("203.0.113.51", { right: true })).toBe("checked"); }); - it("the owner's home network, seen on a recent session, still gets through", async () => { - await tenGuesses(); - vi.mocked(prisma.session.findFirst).mockImplementation((async (args: { - where: { ipAddress?: string }; - }) => - args.where.ipAddress === "203.0.113.9" ? { id: "s-1" } : null) as never); - expect(await guess("203.0.113.9", { right: true })).toBe("checked"); - }); - - it("a place the account has signed in from never counts toward the wait", async () => { + it("an address the account signed in from is no exemption: a guesser can claim it", async () => { + // A session of the owner's from this address. The guesser shares the + // network, or names the address in a forwarding header; either way the + // address says nothing about who is typing. vi.mocked(prisma.session.findFirst).mockResolvedValue({ id: "s-1", } as never); - for (let i = 0; i < 20; i++) { - expect(await guess("203.0.113.9")).toBe("checked"); - } - vi.mocked(prisma.session.findFirst).mockResolvedValue(null); - expect(await guess("198.51.100.1")).toBe("checked"); + vi.mocked(prisma.refreshToken.findFirst).mockResolvedValue({ + id: "rt-1", + } as never); + const answers: string[] = []; + for (let i = 0; i < 10; i++) answers.push(await guess("203.0.113.9")); + expect(answers.slice(FREE_FAILURES)).toContain("waiting"); }); it("an unknown place waits at most fifteen minutes, however long the guessing went on", async () => { @@ -215,8 +209,7 @@ describe("an identifier that names no account", () => { const attempt = await beginAccountLoginAttempt({ user: null, identifier: "Nobody", - request: req(), - ip: "198.51.100.1", + request: req({ "x-forwarded-for": "198.51.100.1" }), }); await attempt.failed(); } diff --git a/src/lib/auth/login-throttle.ts b/src/lib/auth/login-throttle.ts index 1420e95ef..781157f4c 100644 --- a/src/lib/auth/login-throttle.ts +++ b/src/lib/auth/login-throttle.ts @@ -14,11 +14,16 @@ * * A place the account has signed in from before is not counted and not * blocked. "Before" means a remembered browser (the trusted-device cookie, - * checked without being spent), a phone that presents the device id a login - * of this account was issued to, or an address a session or device login of - * this account was created from in the last thirty days. The guesser is by - * definition somewhere the account has not signed in from; the owner mostly - * is not. The per-IP limit still applies to every caller. + * checked without being spent) or a phone that presents the device id a + * login of this account was issued to. The guesser is by definition + * somewhere the account has not signed in from; the owner mostly is not. The + * per-IP limit still applies to every caller. + * + * An address the account signed in from used to count as such a place. An + * address is the one thing about a request a guesser can most easily claim: + * in a household or an office it is shared with the owner, and a caller that + * reached the server directly could name it in a forwarding header. Claiming + * it switched this whole throttle off. * * Everywhere else gets a growing wait instead of a wall. Five failures a day * cost nothing; after that each failure makes the next attempt wait twice as @@ -46,15 +51,13 @@ const FAILURE_WINDOW_MS = 24 * 60 * 60 * 1000; export const BASE_WAIT_MS = 30 * 1000; /** No wait is ever longer than this. */ export const MAX_WAIT_MS = 15 * 60 * 1000; -/** How far back a sign-in from an address still makes it a known place. */ -const KNOWN_ADDRESS_WINDOW_MS = 30 * 24 * 60 * 60 * 1000; /** * The failure counter is read, not enforced, so its ceiling only has to be * out of reach; `limit - remaining` is the count. */ const COUNTER_CEILING = 1_000_000; -export type KnownSource = "trusted_device" | "device_id" | "address"; +export type KnownSource = "trusted_device" | "device_id"; /** * Has this account signed in from where this request comes from? Read-only: @@ -63,7 +66,6 @@ export type KnownSource = "trusted_device" | "device_id" | "address"; export async function knownSignInSource( userId: string, request: Request, - ip: string | null, ): Promise { const now = new Date(); @@ -92,21 +94,6 @@ export async function knownSignInSource( if (login) return "device_id"; } - if (ip) { - const since = new Date(now.getTime() - KNOWN_ADDRESS_WINDOW_MS); - const [session, login] = await Promise.all([ - prisma.session.findFirst({ - where: { userId, ipAddress: ip, createdAt: { gte: since } }, - select: { id: true }, - }), - prisma.refreshToken.findFirst({ - where: { userId, ipAddress: ip, createdAt: { gte: since } }, - select: { id: true }, - }), - ]); - if (session || login) return "address"; - } - return null; } @@ -138,12 +125,11 @@ export async function beginAccountLoginAttempt(args: { user: Pick | null; identifier: string; request: Request; - ip: string | null; }): Promise { - const { user, identifier, request, ip } = args; + const { user, identifier, request } = args; if (user) { - const source = await knownSignInSource(user.id, request, ip); + const source = await knownSignInSource(user.id, request); if (source) { return { waiting: null, source, failed: noop, succeeded: noop }; }