Let accounts go by a name (FW-602)

api/README.md

@@ -95,17 +95,26 @@ which the site shows as it is.
 | Request | Body | Answer |
 |---|---|---|
 | `POST /auth/code` | `{ "email": "ada@example.com" }` | `200 { "sent": true }`; `400` not an address; `429` too soon or too many, with `Retry-After`; `502` the email couldn't be sent |
-| `POST /auth/verify` | `{ "email": "...", "code": "123456" }` | `200 { "user": <user> }` and the session cookie; `400` or `401` wrong or expired code; `429` too many checks |
+| `POST /auth/verify` | `{ "email": "...", "code": "123456", "name"?: "..." }` | `200 { "user": <user> }` and the session cookie; `400` or `401` wrong or expired code, or `400` a name that can't be kept; `429` too many checks |
 | `GET /auth/session` | | `200 { "user": <user> }`, or `{ "user": null }` when signed out |
+| `POST /auth/profile` | `{ "name": "..." }` | `200 { "user": <user> }`; an empty name clears it; `400` a name that can't be kept; `401` signed out |
 | `POST /auth/signout` | `{}` | `200 { "signedOut": true }`, and the cookie is cleared |
 | `GET /admin/users` | | `200 { "signup": "waitlist" \| "open", "users": [...] }`, the waiting ones first; `401` signed out; `403` not an admin |
 | `POST /admin/approve` | `{ "email": "..." }` | `200 { "approved": true, "emailed": true \| false }`; `404` no such account; `401`, `403` as above |
 | `POST /admin/remove` | `{ "email": "..." }` | `200 { "removed": true }`; `400` an admin; `404` no such account; `401`, `403` as above |
 
-`<user>` is `{ "email": "...", "waiting": false, "admin": false }`. `waiting` is
-true while the account is on the waitlist; the site then shows `/waitlist`
-instead of the console. Anything added to the service later that only
-approved accounts may use should check `waiting` the same way.
+`<user>` is `{ "email": "...", "name": "..." | null, "waiting": false, "admin": false }`.
+`waiting` is true while the account is on the waitlist; the site then shows
+`/waitlist` instead of the console. Anything added to the service later that
+only approved accounts may use should check `waiting` the same way.
+
+`name` is what the account goes by, or `null` until one is given. Sign-up can
+send it with the code (`/auth/verify`): it names a new account, or one with no
+name yet, and never replaces a name the account has. `/auth/profile` changes
+it. A name is trimmed and its runs of spaces made one; it can be up to 80
+characters, and can't hold control characters or the bidirectional overrides
+that would make it display as something else. The admin listing carries each
+account's `name` too.
 
 Without `RESEND_API_KEY`, or an `AUTH_SECRET` of at least 32 characters,
 `/auth/code` and `/auth/verify` answer `503`.

api/migrations/0003_profile_name.sql

@@ -0,0 +1,4 @@
+-- The name an account goes by, given at sign-up or on the account page. NULL
+-- until it is given: the site shows the address instead.
+
+ALTER TABLE users ADD COLUMN name TEXT;

api/src/admin.ts

@@ -29,6 +29,7 @@ async function emailFrom(request: Request) {
 
 type AccountRow = {
   email: string;
+  name: string | null;
   status: string;
   created_at: number;
   last_sign_in_at: number;
@@ -40,14 +41,15 @@ export async function listUsers(request: Request, env: Env, now: number) {
   await requireAdmin(request, env, now);
   const admins = adminEmails(env);
   const { results } = await env.DB.prepare(
-    `SELECT email, status, created_at, last_sign_in_at, approved_at, approved_by FROM users
+    `SELECT email, name, status, created_at, last_sign_in_at, approved_at, approved_by FROM users
      ORDER BY status = 'waiting' DESC, created_at DESC LIMIT ?`,
   )
     .bind(LIST_LIMIT)
     .all<AccountRow>();
   const open = isOpen(env);
   const users = results.map((row) => ({
     email: row.email,
+    name: row.name,
     // 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.

api/src/auth.ts

@@ -6,6 +6,7 @@
 //   POST /auth/code     { email }        emails a code
 //   POST /auth/verify   { email, code }  checks it and starts a session
 //   GET  /auth/session                   who the session cookie belongs to
+//   POST /auth/profile  { name }         changes the signed-in account's name
 //   POST /auth/signout                   ends the session
 
 import type { Env } from "./env";
@@ -19,15 +20,16 @@ import {
   DAY,
   LIMITS,
   MESSAGES,
+  NAME_MAX_LENGTH,
   RESEND_COOLDOWN_MS,
   SESSION_TTL_MS,
 } from "./rules";
 import { codeHash, isToken, newCode, newToken, sameHash, tokenHash } from "./tokens";
 
-/** What the site is told about the signed-in account. */
-export type PublicUser = { email: string; waiting: boolean; admin: boolean };
+/** What the site is told about the signed-in account. `name` is null until one is given. */
+export type PublicUser = { email: string; name: string | null; waiting: boolean; admin: boolean };
 
-type UserRow = { id: string; email: string; status: string };
+type UserRow = { id: string; email: string; status: string; name: string | null };
 
 /** Whether anyone who signs in may use the console; anything but "open" keeps the waitlist. */
 export function isOpen(env: Env) {
@@ -42,7 +44,23 @@ export function adminEmails(env: Env) {
 /** An account waits only while sign-up is on the waitlist, until approved; admins never wait. */
 function publicUser(row: UserRow, env: Env): PublicUser {
   const admin = adminEmails(env).has(row.email);
-  return { email: row.email, waiting: !admin && !isOpen(env) && row.status !== "approved", admin };
+  return { email: row.email, name: row.name, waiting: !admin && !isOpen(env) && row.status !== "approved", admin };
+}
+
+// Characters a name can't hold: control characters, and the bidirectional
+// overrides and isolates that would let a name display as something else.
+const NOT_IN_A_NAME = /[\p{Cc}\u202a-\u202e\u2066-\u2069]/u;
+
+/**
+ * A name as it is kept: trimmed, with runs of spaces made one, or null for none (absent or empty).
+ * Anything else, or a name longer than NAME_MAX_LENGTH characters, is refused.
+ */
+export function readName(value: unknown) {
+  if (value === undefined || value === null) return null;
+  if (typeof value !== "string") throw new HttpError(400, MESSAGES.badName);
+  const name = value.normalize("NFC").replace(/\s+/gu, " ").trim();
+  if ([...name].length > NAME_MAX_LENGTH || NOT_IN_A_NAME.test(name)) throw new HttpError(400, MESSAGES.badName);
+  return name || null;
 }
 
 /** An address in the form accounts are kept under, or null if it isn't one. */
@@ -148,6 +166,9 @@ export async function verifyCode(request: Request, env: Env, now: number) {
   const email = normalizeEmail(body.email);
   const code = typeof body.code === "string" ? body.code.trim() : "";
   if (!email || code.length !== CODE_LENGTH || !/^\d+$/.test(code)) throw new HttpError(400, MESSAGES.wrongCode);
+  // Sign-up may send the name to start the account with; it is checked before
+  // the code, so a name that can't be kept doesn't use up a try.
+  const name = readName(body.name);
 
   // Every check uses up one of the code's tries, right or wrong.
   const live = await db
@@ -176,14 +197,16 @@ export async function verifyCode(request: Request, env: Env, now: number) {
   // A new account joins the waitlist unless sign-up is open or it is an
   // admin's; an existing one keeps its status. Either way the address is
   // proven, so the session starts: a waiting account sees the waitlist and
-  // nothing else.
+  // nothing else. A name sent along names a new account, or one without a
+  // name yet; it never replaces a name the account already has.
   const user = await db
     .prepare(
-      `INSERT INTO users (id, email, created_at, last_sign_in_at, status) VALUES (?1, ?2, ?3, ?3, ?4)
-       ON CONFLICT (email) DO UPDATE SET last_sign_in_at = excluded.last_sign_in_at
-       RETURNING id, email, status`,
+      `INSERT INTO users (id, email, created_at, last_sign_in_at, status, name) VALUES (?1, ?2, ?3, ?3, ?4, ?5)
+       ON CONFLICT (email) DO UPDATE SET
+         last_sign_in_at = excluded.last_sign_in_at, name = COALESCE(users.name, excluded.name)
+       RETURNING id, email, status, name`,
     )
-    .bind(crypto.randomUUID(), email, now, isOpen(env) || adminEmails(env).has(email) ? "approved" : "waiting")
+    .bind(crypto.randomUUID(), email, now, isOpen(env) || adminEmails(env).has(email) ? "approved" : "waiting", name)
     .first<UserRow>();
   const token = newToken();
   await db
@@ -200,7 +223,7 @@ export async function sessionUser(request: Request, env: Env, now: number) {
   const token = readSessionCookie(request);
   if (!token || !isToken(token)) return null;
   const row = await env.DB.prepare(
-    `SELECT users.id, users.email, users.status FROM sessions JOIN users ON users.id = sessions.user_id
+    `SELECT users.id, users.email, users.status, users.name FROM sessions JOIN users ON users.id = sessions.user_id
      WHERE sessions.id = ? AND sessions.expires_at > ?`,
   )
     .bind(await tokenHash(token), now)
@@ -212,6 +235,19 @@ export async function currentSession(request: Request, env: Env, now: number) {
   return json({ user: await sessionUser(request, env, now) });
 }
 
+/** Sets or clears (an empty name) the signed-in account's name, and answers with the account. */
+export async function updateProfile(request: Request, env: Env, now: number) {
+  const name = readName((await readJson(request)).name);
+  const signedIn = await sessionUser(request, env, now);
+  if (!signedIn) throw new HttpError(401, MESSAGES.signedOut);
+  const row = await env.DB.prepare("UPDATE users SET name = ? WHERE email = ? RETURNING id, email, status, name")
+    .bind(name, signedIn.email)
+    .first<UserRow>();
+  // Removed by an admin between the two steps: signed out as far as anyone knows.
+  if (!row) throw new HttpError(401, MESSAGES.signedOut);
+  return json({ user: publicUser(row, env) });
+}
+
 export async function signOut(request: Request, env: Env) {
   const token = readSessionCookie(request);
   if (token && isToken(token)) {

api/src/index.ts

@@ -2,7 +2,7 @@
 // and sweeps expired rows on its daily schedule.
 
 import { approveUser, listUsers, removeUser } from "./admin";
-import { currentSession, sendCode, signOut, sweep, verifyCode } from "./auth";
+import { currentSession, sendCode, signOut, sweep, updateProfile, verifyCode } from "./auth";
 import type { Env } from "./env";
 import { allowedOrigin, HttpError, json, preflight, withCors } from "./http";
 import { MESSAGES } from "./rules";
@@ -21,6 +21,8 @@ async function route(request: Request, env: Env, origin: string | null) {
       return verifyCode(request, env, now);
     case "GET /auth/session":
       return currentSession(request, env, now);
+    case "POST /auth/profile":
+      return updateProfile(request, env, now);
     case "POST /auth/signout":
       return signOut(request, env);
     case "GET /admin/users":

api/src/rules.ts

@@ -6,6 +6,8 @@ export const CODE_TTL_MS = 10 * 60_000;
 export const CODE_MAX_ATTEMPTS = 5;
 export const RESEND_COOLDOWN_MS = 60_000;
 export const SESSION_TTL_MS = 30 * 24 * 60 * 60_000;
+/** The longest name an account can go by, in characters. */
+export const NAME_MAX_LENGTH = 80;
 
 const HOUR = 60 * 60_000;
 export const DAY = 24 * HOUR;
@@ -34,6 +36,7 @@ export function waitText(ms: number) {
 export const MESSAGES = {
   badRequest: "That request could not be read.",
   badEmail: "Enter a valid email address.",
+  badName: `Enter a name of up to ${NAME_MAX_LENGTH} characters, or leave it empty.`,
   wrongCode: "That code is wrong or has expired. Check the newest code, or ask for a new one.",
   wait: (seconds: number) => `You can ask for a new code in ${seconds} s.`,
   tooManyFromNetwork: (wait: string) => `Too many sign-in attempts from this network. Try again in ${wait}.`,

api/test/auth.test.mjs

@@ -7,7 +7,7 @@ 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 } };
+const ADA = { user: { email: "ada@example.com", name: null, waiting: true, admin: false } };
 
 let w;
 before(async () => {

api/test/profile.test.mjs

@@ -0,0 +1,134 @@
+import assert from "node:assert/strict";
+import { after, before, beforeEach, describe, test } from "node:test";
+import { start } from "./harness.mjs";
+
+const BAD_NAME = "Enter a name of up to 80 characters, or leave it empty.";
+
+let w;
+before(async () => {
+  // The waitlist on, whatever is deployed, so a waiting account can be tried too.
+  w = await start({ bindings: { ADMIN_EMAILS: "grace@example.com", SIGNUP_MODE: "waitlist" } });
+});
+after(() => w.stop());
+beforeEach(() => w.reset());
+
+/** Signs in with `name` sent along, as /signup does; returns the answer and the cookie. */
+async function signUp(email, name) {
+  const code = await w.codeFor(email);
+  const verified = await w.call("/auth/verify", { body: { email, code, name } });
+  return { ...verified, cookie: verified.headers.get("Set-Cookie")?.split(";")[0] };
+}
+const session = async (cookie) => (await w.call("/auth/session", { method: "GET", cookie })).body.user;
+const stored = async (email) => (await w.db.prepare("SELECT name FROM users WHERE email = ?").bind(email).first())?.name;
+
+describe("the name an account goes by", () => {
+  test("is null until one is given", async () => {
+    const cookie = await w.signIn("ada@example.com");
+    assert.equal((await session(cookie)).name, null);
+    assert.equal(await stored("ada@example.com"), null);
+  });
+
+  test("sign-up can give it, tidied", async () => {
+    const { status, body, cookie } = await signUp("ada@example.com", "  Ada \t  Lovelace ");
+    assert.equal(status, 200);
+    assert.equal(body.user.name, "Ada Lovelace");
+    assert.equal((await session(cookie)).name, "Ada Lovelace");
+  });
+
+  test("an empty name at sign-up is none", async () => {
+    const { body } = await signUp("ada@example.com", "   ");
+    assert.equal(body.user.name, null);
+  });
+
+  test("signing in again never replaces it, but fills it in if there is none", async () => {
+    await signUp("ada@example.com", "Ada Lovelace");
+    assert.equal((await signUp("ada@example.com", "Someone Else")).body.user.name, "Ada Lovelace");
+    await w.signIn("bob@example.com");
+    assert.equal((await signUp("bob@example.com", "Bob")).body.user.name, "Bob");
+  });
+
+  test("a name that can't be kept is refused before the code is tried", async () => {
+    const code = await w.codeFor("ada@example.com");
+    for (const name of ["x".repeat(81), "Ada\u0000", "Ada\nLovelace\u0007", "‮ecalevoL adA", 42, ["Ada"]]) {
+      const refused = await w.call("/auth/verify", { body: { email: "ada@example.com", code, name } });
+      assert.equal(refused.status, 400, JSON.stringify(name));
+      assert.equal(refused.body.error, BAD_NAME);
+    }
+    // The code still has all its tries: refusing the name didn't use one.
+    const { attempts } = await w.db.prepare("SELECT attempts FROM sign_in_codes WHERE email = ?").bind("ada@example.com").first();
+    assert.equal(attempts, 0);
+    assert.equal((await w.call("/auth/verify", { body: { email: "ada@example.com", code, name: "Ada" } })).status, 200);
+  });
+
+  test("80 characters fit, counted as characters, not bytes", async () => {
+    const name = "é".repeat(40) + "李".repeat(39) + "😀";
+    assert.equal([...name].length, 80);
+    assert.equal((await signUp("ada@example.com", name)).body.user.name, name);
+  });
+
+  test("scripts that join with zero-width joiners keep them", async () => {
+    // Malayalam, Hindi and emoji sequences use U+200C and U+200D inside a name.
+    const name = "ന്‍ क्‌ष 👩‍💻";
+    assert.ok(name.includes("‍") && name.includes("‌"));
+    assert.equal((await signUp("ada@example.com", name)).body.user.name, name);
+  });
+});
+
+describe("POST /auth/profile", () => {
+  test("changes the signed-in account's name, and answers with the account", async () => {
+    const cookie = await w.signIn("ada@example.com");
+    const changed = await w.call("/auth/profile", { body: { name: " Ada  King " }, cookie });
+    assert.equal(changed.status, 200);
+    assert.deepEqual(changed.body, { user: { email: "ada@example.com", name: "Ada King", waiting: true, admin: false } });
+    assert.equal((await session(cookie)).name, "Ada King");
+  });
+
+  test("an empty name clears it", async () => {
+    const { cookie } = await signUp("ada@example.com", "Ada");
+    assert.equal((await w.call("/auth/profile", { body: { name: "" }, cookie })).body.user.name, null);
+    assert.equal(await stored("ada@example.com"), null);
+  });
+
+  test("a waiting account can name itself too", async () => {
+    const cookie = await w.signIn("ada@example.com");
+    assert.equal((await session(cookie)).waiting, true);
+    assert.equal((await w.call("/auth/profile", { body: { name: "Ada" }, cookie })).status, 200);
+  });
+
+  test("refuses a name that can't be kept, and changes nothing", async () => {
+    const { cookie } = await signUp("ada@example.com", "Ada");
+    const refused = await w.call("/auth/profile", { body: { name: "x".repeat(81) }, cookie });
+    assert.equal(refused.status, 400);
+    assert.equal(refused.body.error, BAD_NAME);
+    assert.equal(await stored("ada@example.com"), "Ada");
+  });
+
+  test("needs a session", async () => {
+    const answer = await w.call("/auth/profile", { body: { name: "Ada" } });
+    assert.equal(answer.status, 401);
+    assert.equal(answer.body.error, "Sign in first.");
+  });
+
+  test("changes only its own account", async () => {
+    const ada = await w.signIn("ada@example.com");
+    await signUp("bob@example.com", "Bob");
+    await w.call("/auth/profile", { body: { name: "Ada" }, cookie: ada });
+    assert.equal(await stored("bob@example.com"), "Bob");
+  });
+
+  test("is refused from another site, before anything is read", async () => {
+    const cookie = await w.signIn("ada@example.com");
+    const foreign = await w.call("/auth/profile", { body: { name: "Mallory" }, cookie, origin: "https://evil.example" });
+    assert.equal(foreign.status, 403);
+    assert.equal(await stored("ada@example.com"), null);
+  });
+
+  test("the admin listing carries each account's name", async () => {
+    await signUp("ada@example.com", "Ada Lovelace");
+    await w.signIn("bob@example.com");
+    const admin = await w.signIn("grace@example.com");
+    const { users } = (await w.call("/admin/users", { method: "GET", cookie: admin })).body;
+    assert.equal(users.find((u) => u.email === "ada@example.com").name, "Ada Lovelace");
+    assert.equal(users.find((u) => u.email === "bob@example.com").name, null);
+  });
+});