Merge pull request #424 from trails-cool/fix/journal-rate-limit-auth

fix(journal): rate-limit auth endpoints

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");
+  });
+});

apps/journal/app/lib/rate-limit.server.ts

@@ -0,0 +1,95 @@
+// 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<string, Bucket>();
+
+// 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<typeof setInterval> | 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 {
+  // 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}`;
+  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();
+}

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

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