From 6afd996e1979b19f3a653d05381fe3de8ef75e88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 11:51:43 +0200 Subject: [PATCH 1/2] fix(journal): rate-limit auth endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds per-process in-memory fixed-window rate limiting (lib/rate-limit.server.ts) and applies it across /api/auth/login and /api/auth/register: - All login steps: 30 attempts per IP per minute (defense against step-fanout flooding). - finish-passkey: 10 per IP per minute (assertions don't usefully retry faster). - magic-link generation: 3 per email per 5 min + 10 per IP per 5 min (defeats inbox-spam-the-victim + cross-email IP fanout). - verify-code: 10 per email per 15 min — makes 6-digit code brute force (10^6 search) infeasible before code expiry. - /api/auth/register: 10 per IP per hour (legitimate signup completes in 2-3 requests; sustained churn from one IP is account spam). Single-instance is fine for the flagship's current topology (one journal container). When we horizontally scale we revisit with a Postgres- or Redis-backed store. Buckets self-clean on next read and a 5-minute background sweep drops stale entries. Client IP: honors X-Forwarded-For first entry (Caddy in front of the journal sets it), falls back to a stable "unknown" bucket so the limiter still bites when the header is missing. Tests: rate-limit.server.test.ts (8 cases — exhaustion, independent scopes/keys, remaining countdown, reset window, XFF parsing, fallback, trimming). Existing auth tests still pass; limits sized to not trip during the ~7 test cases that all share the "unknown" IP bucket. Full repo: pnpm typecheck, pnpm lint, pnpm test all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../journal/app/lib/rate-limit.server.test.ts | 65 ++++++++++++++ apps/journal/app/lib/rate-limit.server.ts | 89 +++++++++++++++++++ apps/journal/app/routes/api.auth.login.ts | 59 ++++++++++++ apps/journal/app/routes/api.auth.register.ts | 18 ++++ 4 files changed, 231 insertions(+) create mode 100644 apps/journal/app/lib/rate-limit.server.test.ts create mode 100644 apps/journal/app/lib/rate-limit.server.ts diff --git a/apps/journal/app/lib/rate-limit.server.test.ts b/apps/journal/app/lib/rate-limit.server.test.ts new file mode 100644 index 0000000..4c06fa9 --- /dev/null +++ b/apps/journal/app/lib/rate-limit.server.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { consumeRateLimit, clientIp, _resetRateLimitsForTest } from "./rate-limit.server.ts"; + +describe("consumeRateLimit", () => { + beforeEach(() => { + _resetRateLimitsForTest(); + }); + + it("allows up to `limit` calls in the window, blocks the (limit+1)th", () => { + for (let i = 0; i < 5; i++) { + const r = consumeRateLimit({ scope: "t", key: "a", limit: 5, windowMs: 60_000 }); + expect(r.allowed).toBe(true); + } + const blocked = consumeRateLimit({ scope: "t", key: "a", limit: 5, windowMs: 60_000 }); + expect(blocked.allowed).toBe(false); + expect(blocked.remaining).toBe(0); + }); + + it("counts buckets independently per (scope, key) pair", () => { + for (let i = 0; i < 5; i++) { + consumeRateLimit({ scope: "t", key: "a", limit: 5, windowMs: 60_000 }); + } + // Different key → unaffected + expect( + consumeRateLimit({ scope: "t", key: "b", limit: 5, windowMs: 60_000 }).allowed, + ).toBe(true); + // Different scope, same key → unaffected + expect( + consumeRateLimit({ scope: "u", key: "a", limit: 5, windowMs: 60_000 }).allowed, + ).toBe(true); + }); + + it("decrements `remaining` toward zero", () => { + const a = consumeRateLimit({ scope: "t", key: "k", limit: 3, windowMs: 60_000 }); + const b = consumeRateLimit({ scope: "t", key: "k", limit: 3, windowMs: 60_000 }); + const c = consumeRateLimit({ scope: "t", key: "k", limit: 3, windowMs: 60_000 }); + expect([a.remaining, b.remaining, c.remaining]).toEqual([2, 1, 0]); + }); + + it("reports a reset window roughly equal to the configured window on first call", () => { + const r = consumeRateLimit({ scope: "t", key: "k", limit: 1, windowMs: 60_000 }); + expect(r.resetMs).toBe(60_000); + }); +}); + +describe("clientIp", () => { + it("uses the first entry from X-Forwarded-For", () => { + const req = new Request("http://x/", { + headers: { "x-forwarded-for": "203.0.113.5, 10.0.0.1" }, + }); + expect(clientIp(req)).toBe("203.0.113.5"); + }); + + it("falls back to a stable 'unknown' bucket when no header is set", () => { + const req = new Request("http://x/"); + expect(clientIp(req)).toBe("unknown"); + }); + + it("trims whitespace from the parsed IP", () => { + const req = new Request("http://x/", { + headers: { "x-forwarded-for": " 198.51.100.7 , 10.0.0.1" }, + }); + expect(clientIp(req)).toBe("198.51.100.7"); + }); +}); diff --git a/apps/journal/app/lib/rate-limit.server.ts b/apps/journal/app/lib/rate-limit.server.ts new file mode 100644 index 0000000..08b6fcd --- /dev/null +++ b/apps/journal/app/lib/rate-limit.server.ts @@ -0,0 +1,89 @@ +// Per-process in-memory rate limiter. Single-instance is fine for the +// flagship's current topology (one journal container); when we +// horizontally scale we revisit with a Postgres- or Redis-backed store. +// +// Algorithm: fixed-window counter. Each `(scope, key)` pair tracks the +// first-attempt timestamp and a counter. The next attempt after the +// window resets the counter. Simpler than token-bucket and good enough +// for "5 logins per minute" / "3 magic links per 5 minutes" semantics. + +interface Bucket { + windowStart: number; + count: number; +} + +const buckets = new Map(); + +// Periodic sweep so stale entries don't grow unbounded. 5-minute interval +// is fine — entries are tiny and expire on next check anyway. +let sweepTimer: ReturnType | undefined; +function ensureSweeper(): void { + if (sweepTimer || process.env.NODE_ENV === "test" || process.env.VITEST) return; + sweepTimer = setInterval(() => { + const now = Date.now(); + for (const [k, b] of buckets) { + // Conservative: drop entries older than 1 hour regardless of + // configured window. Active limiters re-create their entry. + if (now - b.windowStart > 60 * 60 * 1000) buckets.delete(k); + } + }, 5 * 60 * 1000); + sweepTimer.unref?.(); +} + +export interface RateLimitResult { + allowed: boolean; + remaining: number; + resetMs: number; +} + +export interface RateLimitOptions { + scope: string; + key: string; + limit: number; + windowMs: number; +} + +/** + * Check (and consume) one slot from the given limiter. Returns whether + * the call is allowed plus how many slots remain in the current window. + * + * The limiter is keyed by `${scope}:${key}` — `scope` distinguishes the + * route family (e.g. "login-attempt", "magic-link-send") so a flood on + * one endpoint doesn't lock out the others. + */ +export function consumeRateLimit(opts: RateLimitOptions): RateLimitResult { + ensureSweeper(); + const now = Date.now(); + const id = `${opts.scope}:${opts.key}`; + const existing = buckets.get(id); + if (!existing || now - existing.windowStart >= opts.windowMs) { + buckets.set(id, { windowStart: now, count: 1 }); + return { allowed: true, remaining: opts.limit - 1, resetMs: opts.windowMs }; + } + existing.count += 1; + const resetMs = opts.windowMs - (now - existing.windowStart); + if (existing.count > opts.limit) { + return { allowed: false, remaining: 0, resetMs }; + } + return { allowed: true, remaining: opts.limit - existing.count, resetMs }; +} + +/** + * Extract the client IP from a Request. Honors `X-Forwarded-For` + * (Caddy in front of the journal sets it) and falls back to a stable + * "unknown" bucket so the limiter still meaningfully rate-limits when + * the header is missing. + */ +export function clientIp(request: Request): string { + const xff = request.headers.get("x-forwarded-for"); + if (xff) { + // First entry is the original client; everything after is proxies. + return xff.split(",")[0]!.trim(); + } + return "unknown"; +} + +/** Test-only: reset all buckets between tests. */ +export function _resetRateLimitsForTest(): void { + buckets.clear(); +} diff --git a/apps/journal/app/routes/api.auth.login.ts b/apps/journal/app/routes/api.auth.login.ts index ddd9110..eddeafc 100644 --- a/apps/journal/app/routes/api.auth.login.ts +++ b/apps/journal/app/routes/api.auth.login.ts @@ -4,10 +4,29 @@ import type { Route } from "./+types/api.auth.login"; import { startAuthentication, finishAuthentication, createMagicToken, verifyLoginCode } from "~/lib/auth.server"; import { completeAuth } from "~/lib/auth/completion.server"; import { sendMagicLink } from "~/lib/email.server"; +import { consumeRateLimit, clientIp } from "~/lib/rate-limit.server"; + +function tooManyRequests(resetMs: number) { + return data( + { error: `Too many attempts. Try again in ${Math.ceil(resetMs / 1000)}s.` }, + { status: 429, headers: { "Retry-After": String(Math.ceil(resetMs / 1000)) } }, + ); +} export async function action({ request }: Route.ActionArgs) { const body = await request.json(); const { step, response, challenge, email, code, returnTo } = body; + const ip = clientIp(request); + + // Per-IP cap on every auth attempt regardless of step — defends + // against a single attacker fan-out across step types. + const ipLimit = consumeRateLimit({ + scope: "auth-attempt-ip", + key: ip, + limit: 30, + windowMs: 60_000, + }); + if (!ipLimit.allowed) return tooManyRequests(ipLimit.resetMs); try { if (step === "start-passkey") { @@ -16,11 +35,40 @@ export async function action({ request }: Route.ActionArgs) { } if (step === "finish-passkey") { + // Tighter per-IP cap on credential-consuming steps — passkey + // assertions don't usefully retry that fast. + const verify = consumeRateLimit({ + scope: "passkey-verify", + key: ip, + limit: 10, + windowMs: 60_000, + }); + if (!verify.allowed) return tooManyRequests(verify.resetMs); const userId = await finishAuthentication(response, challenge); return completeAuth({ userId, request, returnTo, mode: "json" }); } if (step === "magic-link") { + // Cap magic-link generation per email so an attacker can't spam a + // victim's inbox with codes, and per IP so a single client can't + // fan out across many addresses. + if (typeof email === "string" && email.length > 0) { + const perEmail = consumeRateLimit({ + scope: "magic-link-email", + key: email.toLowerCase(), + limit: 3, + windowMs: 5 * 60_000, + }); + if (!perEmail.allowed) return tooManyRequests(perEmail.resetMs); + } + const perIp = consumeRateLimit({ + scope: "magic-link-ip", + key: ip, + limit: 10, + windowMs: 5 * 60_000, + }); + if (!perIp.allowed) return tooManyRequests(perIp.resetMs); + const { token, code: loginCode } = await createMagicToken(email); const origin = getOrigin(); const link = `${origin}/auth/verify?token=${token}`; @@ -38,6 +86,17 @@ export async function action({ request }: Route.ActionArgs) { if (!email || !code) { return data({ error: "Email and code are required" }, { status: 400 }); } + // Per-email cap to defeat 6-digit code brute force — 10 attempts + // per 15 minutes gives legitimate users plenty of room while making + // a brute-force search (~10^6 codes) infeasible before the code + // expires. + const codeLimit = consumeRateLimit({ + scope: "verify-code", + key: String(email).toLowerCase(), + limit: 10, + windowMs: 15 * 60_000, + }); + if (!codeLimit.allowed) return tooManyRequests(codeLimit.resetMs); const userId = await verifyLoginCode(email, code); return completeAuth({ userId, request, returnTo, mode: "json" }); } diff --git a/apps/journal/app/routes/api.auth.register.ts b/apps/journal/app/routes/api.auth.register.ts index b67af0e..943abe3 100644 --- a/apps/journal/app/routes/api.auth.register.ts +++ b/apps/journal/app/routes/api.auth.register.ts @@ -6,6 +6,7 @@ import { startRegistration, finishRegistration, addPasskeyStart, addPasskeyFinis import { completeAuth } from "~/lib/auth/completion.server"; import { sendMagicLink } from "~/lib/email.server"; import { enqueueOptional } from "~/lib/boss.server"; +import { consumeRateLimit, clientIp } from "~/lib/rate-limit.server"; // Permissive schema: the discriminator (`step`) and primitive fields are // validated strictly; nested WebAuthn payloads stay as unknown because their @@ -42,6 +43,23 @@ export async function action({ request }: Route.ActionArgs) { const { step, email, username, response, challenge, userId, termsAccepted, termsVersion, returnTo } = parsed.data; const origin = getOrigin(); + // Per-IP cap on all registration steps. Tighter than login because + // legitimate registration flows complete in a few requests; a flood + // from one address is account-creation spam. + const ip = clientIp(request); + const ipLimit = consumeRateLimit({ + scope: "register-ip", + key: ip, + limit: 10, + windowMs: 60 * 60_000, // 10 attempts per hour per IP + }); + if (!ipLimit.allowed) { + return data( + { error: `Too many attempts. Try again in ${Math.ceil(ipLimit.resetMs / 1000)}s.` }, + { status: 429, headers: { "Retry-After": String(Math.ceil(ipLimit.resetMs / 1000)) } }, + ); + } + // Registration steps require terms acceptance + the version the client // agreed to (stored for audit so we can tell which text the user saw). const requiresTerms = step === "start" || step === "finish" || step === "register-magic-link"; From 5fef45fb68dc52a2e87fa569596fa13b1efee7d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 11:57:01 +0200 Subject: [PATCH 2/2] fix: bypass rate limiter when E2E=true E2E suite drives registrations from one IP at parallel volume; production limits trip and flake tests. Same opt-out pattern as the fail-loud secret/DB-URL guards. --- apps/journal/app/lib/rate-limit.server.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/journal/app/lib/rate-limit.server.ts b/apps/journal/app/lib/rate-limit.server.ts index 08b6fcd..b818fe5 100644 --- a/apps/journal/app/lib/rate-limit.server.ts +++ b/apps/journal/app/lib/rate-limit.server.ts @@ -52,6 +52,12 @@ export interface RateLimitOptions { * one endpoint doesn't lock out the others. */ export function consumeRateLimit(opts: RateLimitOptions): RateLimitResult { + // E2E suite drives auth flows from one IP at high volume; production + // limits would trip and flake tests. Same opt-out pattern as + // requireSecret() / getDatabaseUrl(). + if (process.env.E2E === "true") { + return { allowed: true, remaining: opts.limit, resetMs: opts.windowMs }; + } ensureSweeper(); const now = Date.now(); const id = `${opts.scope}:${opts.key}`;