Fix sign-in issues from the review of #38

api/.dev.vars.example

@@ -6,8 +6,11 @@ RESEND_API_KEY=re_...
 # At least 32 random characters, e.g. from `openssl rand -base64 32`.
 AUTH_SECRET=
 
-# The site's dev server, so it can call the local service.
-ALLOWED_ORIGINS=http://127.0.0.1:5173,http://localhost:5173
+# The site's dev server, so it can call the local service. Open it at
+# 127.0.0.1, not localhost: the session cookie is SameSite=Strict on the
+# service's host, 127.0.0.1, and a page on localhost is another site to it,
+# so the cookie would never come back and sign-in would seem to fail.
+ALLOWED_ORIGINS=http://127.0.0.1:5173
 
 # Who can approve the waitlist at /app/admin, comma-separated.
 ADMIN_EMAILS=you@example.com

api/README.md

@@ -127,6 +127,10 @@ echo VITE_AUTH_API_URL=http://127.0.0.1:8787 > site/.env.development.local
 npm --prefix site run dev        # http://127.0.0.1:5173/login
 ```
 
+Open the site at `127.0.0.1`, not `localhost`. The session cookie is
+`SameSite=Strict` on the service's host, `127.0.0.1`, and browsers count
+`localhost` as another site, so the cookie would never be sent back.
+
 The codes are real emails, so use an address you can read. Resend only
 sends from a domain that's verified in the account the key belongs to.
 

api/src/admin.ts

@@ -45,10 +45,13 @@ export async function listUsers(request: Request, env: Env, now: number) {
   )
     .bind(LIST_LIMIT)
     .all<AccountRow>();
+  const open = isOpen(env);
   const users = results.map((row) => ({
     email: row.email,
-    // An admin never waits, even one added to ADMIN_EMAILS after joining the waitlist.
-    status: admins.has(row.email) ? "approved" : row.status,
+    // The status the account has now: an admin never waits, even one added to
+    // ADMIN_EMAILS after joining the waitlist, and nobody waits while sign-up
+    // is open. A row stored as waiting waits again if the waitlist comes back.
+    status: open || admins.has(row.email) ? "approved" : row.status,
     admin: admins.has(row.email),
     createdAt: row.created_at,
     lastSignInAt: row.last_sign_in_at,
@@ -57,7 +60,7 @@ export async function listUsers(request: Request, env: Env, now: number) {
   }));
   // Sorting is stable, so each group stays newest first.
   users.sort((a, b) => Number(b.status === "waiting") - Number(a.status === "waiting"));
-  return json({ signup: isOpen(env) ? "open" : "waitlist", users });
+  return json({ signup: open ? "open" : "waitlist", users });
 }
 
 /** Approves a waiting account and tells its owner. `origin` is the site the admin is on. */
@@ -73,8 +76,9 @@ export async function approveUser(request: Request, env: Env, now: number, origi
   if (!approved) {
     const exists = await env.DB.prepare("SELECT 1 FROM users WHERE email = ?").bind(email).first();
     if (!exists) throw new HttpError(404, MESSAGES.noSuchUser);
-    // Already approved, by another admin or an earlier click: nothing to send.
-    return json({ approved: true, emailed: false });
+    // Already approved, by another admin or an earlier click: nothing to send,
+    // and nothing failed.
+    return json({ approved: true, emailed: false, alreadyApproved: true });
   }
   // The approval stands even if the email doesn't go: the account works the
   // next time its owner opens the site, and the admin is told.

api/src/auth.ts

@@ -10,7 +10,7 @@
 
 import type { Env } from "./env";
 import { HttpError, json, readJson, readSessionCookie, sessionCookie } from "./http";
-import { countRequest } from "./limits";
+import { countRequest, uncountRequests } from "./limits";
 import { sendCodeEmail } from "./mail";
 import {
   CODE_LENGTH,
@@ -52,9 +52,24 @@ export function normalizeEmail(value: unknown) {
   return email.length <= 254 && /^[^\s@\x00-\x1f]+@[^\s@\x00-\x1f]+\.[^\s@\x00-\x1f]+$/.test(email) ? email : null;
 }
 
-// Cloudflare sets this from the connection, so callers can't choose it.
+// Cloudflare sets this from the connection, so callers can't choose it. An
+// IPv6 host is usually given a whole /64, so the limits per network count an
+// IPv6 address by its /64: otherwise each new address would get fresh limits.
 function network(request: Request) {
-  return request.headers.get("CF-Connecting-IP") ?? "unknown";
+  const ip = request.headers.get("CF-Connecting-IP") ?? "unknown";
+  return ip.includes(":") ? ipv6Network(ip) : ip;
+}
+
+/** "2001:db8:1:2::/64" for any address in that /64; the address itself if it can't be read. */
+function ipv6Network(ip: string) {
+  // An IPv4 address written as IPv6 (::ffff:192.0.2.1) is its own network.
+  if (ip.includes(".")) return ip;
+  const halves = ip.split("::");
+  if (halves.length > 2) return ip;
+  const [head, tail] = halves.map((half) => (half ? half.split(":") : []));
+  const groups = tail ? [...head, ...Array<string>(8 - head.length - tail.length).fill("0"), ...tail] : head;
+  if (groups.length !== 8 || !groups.every((g) => /^[0-9a-f]{1,4}$/i.test(g))) return ip;
+  return `${groups.slice(0, 4).map((g) => parseInt(g, 16).toString(16)).join(":")}::/64`;
 }
 
 function cooldown(waitMs: number) {
@@ -82,6 +97,17 @@ export async function sendCode(request: Request, env: Env, now: number) {
   if (last && now < last.sent_at + RESEND_COOLDOWN_MS) throw cooldown(last.sent_at + RESEND_COOLDOWN_MS - now);
   await countRequest(db, `code:address:${email}`, LIMITS.codesPerAddressHour, now, MESSAGES.tooManyForAddress);
   await countRequest(db, `code:address-day:${email}`, LIMITS.codesPerAddressDay, now, MESSAGES.tooManyForAddress);
+  // The address's limits count codes sent, so a request that ends up sending
+  // none (another one won the minute, or the email failed) gives its count back.
+  const uncount = () =>
+    uncountRequests(
+      db,
+      [
+        [`code:address:${email}`, LIMITS.codesPerAddressHour],
+        [`code:address-day:${email}`, LIMITS.codesPerAddressDay],
+      ],
+      now,
+    );
 
   // The new code replaces the last one, unless another request sent one in
   // the last minute: the condition makes that check and the write one step.
@@ -97,13 +123,17 @@ export async function sendCode(request: Request, env: Env, now: number) {
     )
     .bind(email, hash, now, now + CODE_TTL_MS, now - RESEND_COOLDOWN_MS)
     .first();
-  if (!saved) throw cooldown(RESEND_COOLDOWN_MS);
+  if (!saved) {
+    await uncount();
+    throw cooldown(RESEND_COOLDOWN_MS);
+  }
 
   try {
     await sendCodeEmail(env, email, code);
   } catch (error) {
     // A code nobody received shouldn't hold the address to the minute's wait.
     await db.prepare("DELETE FROM sign_in_codes WHERE email = ? AND code_hash = ?").bind(email, hash).run();
+    await uncount();
     throw error;
   }
   return json({ sent: true });

api/src/http.ts

@@ -30,14 +30,35 @@ export function json(body: unknown, status = 200, headers: Record<string, string
 // An email address and a code fit many times over.
 const MAX_BODY_LENGTH = 4096;
 
+/**
+ * The body as text, refused with a 413 once it passes MAX_BODY_LENGTH bytes. It's read a chunk at
+ * a time, so a body sent without a Content-Length is stopped there rather than read in full.
+ */
+async function readBody(request: Request) {
+  if (Number(request.headers.get("Content-Length")) > MAX_BODY_LENGTH) throw new HttpError(413, MESSAGES.badRequest);
+  if (!request.body) return "";
+  const reader = request.body.getReader();
+  const bytes = new Uint8Array(MAX_BODY_LENGTH);
+  let length = 0;
+  for (;;) {
+    const { done, value } = await reader.read();
+    if (done) break;
+    if (length + value.byteLength > MAX_BODY_LENGTH) {
+      await reader.cancel();
+      throw new HttpError(413, MESSAGES.badRequest);
+    }
+    bytes.set(value, length);
+    length += value.byteLength;
+  }
+  return new TextDecoder().decode(bytes.subarray(0, length));
+}
+
 /** The request's JSON object. Anything else is refused. */
 export async function readJson(request: Request): Promise<Record<string, unknown>> {
   if (!request.headers.get("Content-Type")?.startsWith("application/json")) {
     throw new HttpError(415, MESSAGES.badRequest);
   }
-  if (Number(request.headers.get("Content-Length")) > MAX_BODY_LENGTH) throw new HttpError(413, MESSAGES.badRequest);
-  const text = await request.text();
-  if (text.length > MAX_BODY_LENGTH) throw new HttpError(413, MESSAGES.badRequest);
+  const text = await readBody(request);
   try {
     const body: unknown = JSON.parse(text);
     if (body && typeof body === "object" && !Array.isArray(body)) return body as Record<string, unknown>;

api/src/limits.ts

@@ -29,3 +29,14 @@ export async function countRequest(
     throw new HttpError(429, message(waitText(wait)), { "Retry-After": String(Math.ceil(wait / 1000)) });
   }
 }
+
+/** Takes back requests countRequest counted at `now`, one under each key, in its limit's window. */
+export async function uncountRequests(db: D1Database, keys: [string, Limit][], now: number) {
+  await db.batch(
+    keys.map(([key, { windowMs }]) =>
+      db
+        .prepare("UPDATE rate_limits SET count = count - 1 WHERE key = ? AND window_start = ? AND count > 0")
+        .bind(key, now - (now % windowMs)),
+    ),
+  );
+}

api/test/auth.test.mjs

@@ -1,9 +1,11 @@
 import assert from "node:assert/strict";
 import { after, before, beforeEach, describe, test } from "node:test";
-import { API, KEY, SECRET, SITE, start } from "./harness.mjs";
+import { API, KEY, SECRET, SITE, clearOfWindowEnd, start } from "./harness.mjs";
 
 const WRONG = "That code is wrong or has expired. Check the newest code, or ask for a new one.";
 const MINUTE = 60_000;
+const HOUR = 60 * MINUTE;
+const DAY = 24 * HOUR;
 // A new account, as the service describes it while sign-up is on the waitlist.
 const ADA = { user: { email: "ada@example.com", waiting: true, admin: false } };
 
@@ -14,6 +16,16 @@ before(async () => {
 after(() => w.stop());
 beforeEach(() => w.reset());
 
+/** How many requests are counted under `key` in the current `windowMs` window. */
+async function counted(key, windowMs) {
+  const now = Date.now();
+  const row = await w.db
+    .prepare("SELECT count FROM rate_limits WHERE key = ? AND window_start = ?")
+    .bind(key, now - (now % windowMs))
+    .first();
+  return row?.count ?? 0;
+}
+
 /** The other of two six-digit codes. */
 const other = (code) => String((Number(code) + 1) % 1_000_000).padStart(6, "0");
 
@@ -235,6 +247,7 @@ describe("asking for codes", () => {
   });
 
   test("of several asks at once, one sends a code", async () => {
+    await clearOfWindowEnd(HOUR);
     const asks = await Promise.all(
       Array.from({ length: 5 }, () => w.call("/auth/code", { body: { email: "ada@example.com" } })),
     );
@@ -243,9 +256,13 @@ describe("asking for codes", () => {
     // The one code saved is the one emailed.
     const code = w.lastCode("ada@example.com");
     assert.equal((await w.call("/auth/verify", { body: { email: "ada@example.com", code } })).status, 200);
+    // Only the code sent counts against the address.
+    assert.equal(await counted("code:address:ada@example.com", HOUR), 1);
+    assert.equal(await counted("code:address-day:ada@example.com", DAY), 1);
   });
 
   test("five codes an hour per address", async () => {
+    await clearOfWindowEnd(HOUR);
     for (let i = 0; i < 5; i++) {
       await w.codeFor("ada@example.com");
       await w.age("ada@example.com", MINUTE);
@@ -259,6 +276,7 @@ describe("asking for codes", () => {
   });
 
   test("twenty codes a day per address", async () => {
+    await clearOfWindowEnd(DAY);
     const day = 24 * 60 * MINUTE;
     const start = Date.now() - (Date.now() % day);
     await w.db
@@ -272,6 +290,7 @@ describe("asking for codes", () => {
   });
 
   test("thirty codes an hour per network", async () => {
+    await clearOfWindowEnd(HOUR);
     for (let i = 0; i < 30; i++) await w.codeFor(`user${i}@example.com`);
     const asked = await w.call("/auth/code", { body: { email: "ada@example.com" } });
     assert.equal(asked.status, 429);
@@ -281,7 +300,21 @@ describe("asking for codes", () => {
     await w.codeFor("ada@example.com", { ip: "198.51.100.1" });
   });
 
+  test("an IPv6 network is its /64, however its addresses are written", async () => {
+    await clearOfWindowEnd(HOUR);
+    for (let i = 0; i < 30; i++) await w.codeFor(`user${i}@example.com`, { ip: `2001:db8:1:2::${(i + 1).toString(16)}` });
+    for (const ip of ["2001:db8:1:2:ffff:ffff:ffff:ffff", "2001:0db8:0001:0002::1", "2001:db8:1:2:0:0:0:abc"]) {
+      const asked = await w.call("/auth/code", { body: { email: "ada@example.com" }, ip });
+      assert.equal(asked.status, 429, ip);
+      assert.match(asked.body.error, /^Too many sign-in attempts from this network./);
+    }
+    // The next /64, and IPv4, are other networks.
+    await w.codeFor("ada@example.com", { ip: "2001:db8:1:3::1" });
+    await w.codeFor("bob@example.com", { ip: "198.51.100.1" });
+  });
+
   test("sixty checks an hour per network", async () => {
+    await clearOfWindowEnd(HOUR);
     const code = await w.codeFor("ada@example.com");
     const wrong = other(code);
     for (let i = 0; i < 60; i++) await w.call("/auth/verify", { body: { email: `user${i}@example.com`, code: wrong } });
@@ -302,6 +335,13 @@ describe("asking for codes", () => {
       assert.equal(sent.status, 502);
       assert.deepEqual(sent.body, { error: "We could not send the email. Please try again in a moment." });
       assert.equal((await failing.db.prepare("SELECT COUNT(*) AS n FROM sign_in_codes").first()).n, 0);
+      // No code went out, so none counts against the address.
+      const now = Date.now();
+      const left = await failing.db
+        .prepare("SELECT COALESCE(SUM(count), 0) AS n FROM rate_limits WHERE key LIKE 'code:address%' AND window_start >= ?")
+        .bind(now - (now % DAY))
+        .first();
+      assert.equal(left.n, 0);
 
       refuse = false;
       const code = await failing.codeFor("ada@example.com");

api/test/harness.mjs

@@ -16,6 +16,16 @@ export const SITE = "https://preview.floodwall.ai";
 export const SECRET = "test-secret-that-is-at-least-32-characters";
 export const KEY = "re_test_key";
 
+/**
+ * The service counts requests in fixed windows on the real clock (limits.ts). A test that makes
+ * many requests calls this first: if the current window ends within `marginMs`, it waits for the
+ * next one, so the test's requests all land in one window.
+ */
+export async function clearOfWindowEnd(windowMs, marginMs = 30_000) {
+  const left = windowMs - (Date.now() % windowMs);
+  if (left < marginMs) await new Promise((resolve) => setTimeout(resolve, left + 1_000));
+}
+
 /** Starts the Worker. `resend(request)` can stand in for Resend's answer. */
 export async function start({ bindings = {}, resend } = {}) {
   const outbox = [];

api/test/waitlist.test.mjs

@@ -130,7 +130,8 @@ describe("while sign-up is on the waitlist", () => {
     w.outbox.length = 0;
     await w.call("/admin/approve", { body: { email: "ada@example.com" }, cookie: admin });
     const again = await w.call("/admin/approve", { body: { email: "ada@example.com" }, cookie: admin });
-    assert.deepEqual(again.body, { approved: true, emailed: false });
+    // Not a failed email: there was nothing to send.
+    assert.deepEqual(again.body, { approved: true, emailed: false, alreadyApproved: true });
     assert.equal(w.outbox.length, 1);
   });
 
@@ -215,7 +216,13 @@ describe("the SIGNUP_MODE flag", () => {
       assert.equal(await status(w, "ada@example.com"), "approved");
 
       const admin = await w.signIn(GRACE);
-      assert.equal((await w.call("/admin/users", { method: "GET", cookie: admin })).body.signup, "open");
+      const listed = (await w.call("/admin/users", { method: "GET", cookie: admin })).body;
+      assert.equal(listed.signup, "open");
+      // The list shows nobody waiting, as the console lets them all in.
+      assert.equal(listed.users.find((u) => u.email === "old@example.com").status, "approved");
+      assert.ok(listed.users.every((u) => u.status === "approved"));
+      // Stored as waiting still, so the waitlist would hold them again if it came back.
+      assert.equal(await status(w, "old@example.com"), "waiting");
     } finally {
       await w.stop();
     }

docs/404.html

@@ -33,7 +33,7 @@
     <link rel="manifest" href="/site.webmanifest" />
     <link rel="preload" href="/fonts/geist-variable.woff2" as="font" type="font/woff2" crossorigin />
     <link rel="preload" href="/fonts/geist-mono-variable.woff2" as="font" type="font/woff2" crossorigin />
-    <script type="module" crossorigin src="/assets/index-B-aGcOTH.js"></script>
+    <script type="module" crossorigin src="/assets/index-CWghM7vV.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-DEUrhNBj.css">
     <meta name="description" content="A Rust control plane for high-volume, agent-driven DevOps: admission control, policy gates that admit, defer or reject each intent, and a tamper-evident ledger.">
     <link rel="canonical" href="https://floodwall.ai/">

docs/index.html

@@ -33,7 +33,7 @@
     <link rel="manifest" href="/site.webmanifest" />
     <link rel="preload" href="/fonts/geist-variable.woff2" as="font" type="font/woff2" crossorigin />
     <link rel="preload" href="/fonts/geist-mono-variable.woff2" as="font" type="font/woff2" crossorigin />
-    <script type="module" crossorigin src="/assets/index-B-aGcOTH.js"></script>
+    <script type="module" crossorigin src="/assets/index-CWghM7vV.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-DEUrhNBj.css">
     <meta name="description" content="A Rust control plane for high-volume, agent-driven DevOps: admission control, policy gates that admit, defer or reject each intent, and a tamper-evident ledger.">
     <link rel="canonical" href="https://floodwall.ai/">

site/src/components/auth/SessionUnavailable.tsx

@@ -0,0 +1,45 @@
+import { useState } from "react";
+import { AUTH_COPY } from "../../constants/auth";
+import { authCard, pillSolid } from "../../styles/classes";
+import { Button } from "../ui/button";
+import { Card } from "../ui/card";
+import { AuthPage } from "./AuthPage";
+
+const COPY = AUTH_COPY.unavailable;
+
+/**
+ * Shown in place of a signed-in page when the sign-in service couldn't say who is signed in. The
+ * session may well be fine, so this asks again instead of sending the reader to sign in.
+ */
+export function SessionUnavailable({ onRetry }: { onRetry: () => Promise<unknown> }) {
+  const [busy, setBusy] = useState(false);
+  const [status, setStatus] = useState("");
+
+  async function retry() {
+    setBusy(true);
+    setStatus("");
+    try {
+      // Once the service answers, the page shows what it says instead of this.
+      await onRetry();
+    } catch (error) {
+      setStatus(error instanceof Error ? error.message : AUTH_COPY.service.failed);
+    } finally {
+      setBusy(false);
+    }
+  }
+
+  return (
+    <AuthPage title={COPY.title} lead={COPY.lead}>
+      <Card className={authCard}>
+        <div className="flex flex-wrap gap-2">
+          <Button type="button" className={pillSolid} disabled={busy} onClick={() => void retry()}>
+            {busy ? COPY.retrying : COPY.retry}
+          </Button>
+        </div>
+        <p role="status" aria-live="polite" className="text-sm text-muted empty:hidden">
+          {status}
+        </p>
+      </Card>
+    </AuthPage>
+  );
+}