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 }; }