RunicGateway/docs
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

docs(website): unique, changeable, verifiable email addresses (engagement Phase 1b)

Browse Source 
Companion to website#<pr>. Records Phase 1b as built, and corrects two things
the plan got wrong before anyone builds on them.

ENGAGEMENT.md
  - Phase 1b step 2 said to pin the index to a case-insensitive collation. Every
    _ci collation MariaDB offers here is also accent-insensitive, so that index
    would refuse jose@x.com once josé@x.com existed and the de-duplication would
    have cleared a legitimate account's address. The as-built block records the
    generated-column design that shipped instead, and the second-order version of
    the same bug that a seeded fixture caught in the de-dupe query itself.
  - §0.6 named two callers of isDuplicateUsername(). There are five, and the
    three it omits fail worse than the two it names.

BACKEND_DESIGN.md — the users table (already stale: it predated the player
account work), plus email_verifications and email_dedupe_report, and the five
new routes.

UPGRADE_NOTES.md — an operator entry, because the de-duplication is the kind of
quiet change this file exists for: nothing breaks, and the affected users find
out the next time they try to reset a password.

api-route-inventory.json — regenerated from the manifest; still ungated.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
claude
2026-08-29 01:54:05 -05:00
parent e833874e8b
commit 84481ee6b1
4 changed files with 236 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

101
website/ENGAGEMENT.md
View File

@@ -1,7 +1,8 @@
# The Engagement System — findings and plan
**Status:** design of record. **Phase 1 is built** (website#165 + docs#178, with website#164 as its
prerequisite); everything from Phase 1b on is still design. The scope decisions below are
**Status:** design of record. **Phases 1, 1a and 1b are built** (Phase 1: website#165 + docs#178, with
website#164 as its prerequisite; Phase 1a: website#166 + docs#179); everything from Phase 2 on is still
design. The scope decisions below are
settled; **four of the eight questions in §7.1 were answered by the org lead on 2026-08-28** — Q1, Q3, Q5
and Q7, and Q1's answer added a whole phase (**Phase 1b**, unique email addresses). Q2, Q4, Q6 and
Q8 remain open and block Phases 4, 5b, 2 and 8 respectively. Per CLAUDE.md § Conventions, no implementation
@@ -173,6 +174,21 @@ collision as a username collision the instant the index exists:
The fix is to distinguish the constraint (read the index name off the driver error) before Phase 1b adds
the index — not after.
> **Amended 2026-08-29 (Phase 1b, as built).** This table names **two** callers of
> `isDuplicateUsername()`. There are **five**, and the three it omits fail worse than the two it
> names — `invite.controller.js` accepts an invite to an address already held and fails *after* the
> invitee has clicked the link and chosen a password, while `admin.controller.js` `createUser` and
> `updateUser` had **no catch at all** and turned a duplicate address into an opaque 500 for an admin
> who could see nothing wrong with the form. (`auth/account.controller.js` changeUsername is the
> fifth and is username-only, so it was already correct.) All five are handled; each answers
> differently on purpose, because a public form, an authenticated IdP callback, a half-completed
> invite and an admin screen do not owe the same person the same amount of truth.
>
> The index name is available **only in the driver's message text** — the mariadb connector exposes
> no structured field for it — so the discrimination is a regex over `for key '…'`, with its own
> test. That message also embeds the bound parameters, so on an email collision it *contains the
> address*: a second, independent reason these errors must never be echoed to a client.
**3. SSO auto-provisioning is the source of the duplicates, and CLAUDE.md is stale about it.** CLAUDE.md
states *"identities are never auto-provisioned"*. `provisionSsoPlayer` (`sso.controller.js:197`) does
exactly that whenever `player_registration ∈ {sso, both}`, writing `profile.email` straight into
@@ -1345,7 +1361,7 @@ generated mirror with no CI gate, so it was regenerated wholesale here rather th
---
### Phase 1b — Unique, changeable, verifiable email addresses *(decision 6)*
### Phase 1b — Unique, changeable, verifiable email addresses ✅ *(decision 6)*
**Lands alone, between 1 and 2, and before any engagement mail exists.** It touches registration, SSO
provisioning and the boot-time schema path — three security-sensitive surfaces — and retrofitting
@@ -1364,10 +1380,9 @@ uniqueness *after* a send log and a suppression list hold rows is strictly worse
and `email_verified` to `0`. Multiple `NULL`s are legal under a UNIQUE index, so nobody loses an
account and nothing cascades. The affected accounts are written to an admin-visible report — *who*
was cleared and *what* address they lost — because they are exactly the users who must be contacted.
Then `ALTER TABLE users ADD UNIQUE INDEX IF NOT EXISTS uq_users_email (email)`, pinned to a
case-insensitive collation for the same reason `username` was (`schema.sql:24`): `Foo@x.com` and
`foo@x.com` are one mailbox everywhere that matters, and folding must happen in the index rather than
in application code that can be bypassed.
Then a UNIQUE index — **on a generated `email_norm` column, not on `email`.** The reasoning above
(`Foo@x.com` and `foo@x.com` are one mailbox; folding belongs in the index rather than in bypassable
application code) is right, but the collation this originally named is not: see the amendment below.
3. **A self-serve change-and-verify flow**, which does not exist today (§0.6 finding 4). Set/change
address, a signed time-boxed verification link, `email_verified` set only on link use. It lands on
**`/auth/me/account` and nowhere else** — Phase 1a made that the single self-service surface. SSO's
@@ -1393,6 +1408,78 @@ is used; `Foo@x.com` collides with `foo@x.com`; an upgraded install has the gate
migration is idempotent and re-running `ensureSchema()` is a no-op; **no destructive DDL** — the
de-dupe nulls a column, it never deletes a row.
#### As built (2026-08-29)
**The index is on a generated column, because every `_ci` collation is also accent-insensitive.**
Step 2 above said to pin `email` to a case-insensitive collation "for the same reason `username` was".
Tested against the deployment's own MariaDB 11.8, that is wrong in a way that would have destroyed
data: under **both** `utf8mb4_general_ci` and the server-default `utf8mb4_uca1400_ai_ci`,
`josé@x.com` and `jose@x.com` compare EQUAL. They are different mailboxes. A UNIQUE index over either
collation refuses the second address forever, and the de-duplication below would have nulled a
legitimate account's address and reported it as a duplicate that never was.
The accent-sensitive, case-insensitive collations that would be exactly right
(`utf8mb4_general1400_as_ci`, `utf8mb4_0900_as_ci`) are MariaDB 11.4+ only, so pinning one moves the
"a UNIQUE email can stop a boot" failure of §0.6 to a different trigger. What shipped instead:
```sql
email VARCHAR(255) NULL,
email_norm VARCHAR(255) COLLATE utf8mb4_bin AS (LOWER(email)) STORED,
UNIQUE KEY uq_users_email_norm (email_norm)
```
`LOWER()` under a `_bin` collation folds case without folding accents — verified, not assumed. The
fold still lives in the schema rather than in bypassable application code, which was the point of the
original rule. Multiple NULLs remain legal, which is what lets the de-dupe clear an address without
deleting an account. No foreign key references `users.email`, so the STORED-generated-column trap from
TEAMS.md phase 2 (`ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED` on `ON DELETE SET NULL`) does not apply.
**The de-dupe must group on that same column, and the first version did not.** Written as
`LOWER(u2.email) = LOWER(u.email)`, the comparison uses the *column's* collation — accent-insensitive
— so it over-folds even when the index does not. A seeded fixture caught it nulling `jose@x.com` as a
"duplicate" of `josé@x.com`: the exact defect the index change was made to prevent, reintroduced one
statement later. The migration therefore **adds `email_norm` before de-duplicating and groups on it**,
so the two agree by construction rather than by a hand-matched `COLLATE` clause a later edit can get
wrong. Order in `schema.sql` is load-bearing and commented as such.
**Four decisions taken at build time**, all approved before any code:
| | Decision | Why |
|---|---|---|
| Index folding | generated `LOWER()` column + `_bin` index | above |
| Change flow | **pending column**, live address untouched | a typo cannot silently redirect account-recovery mail. Cost: a pending address reserves nothing, so two users may both be pending on one address and the second to confirm loses — with the same generic failure |
| Re-auth | `currentPassword` required, SSO carve-out | an address is where recovery lands, so repointing it is credential-grade; mirrors `changePassword` |
| Report surface | table + dashboard warning + read route | reuses the Phase 1 G22 shape: narrow, self-clearing, silent on installs it does not concern |
**One deviation from the text above, deliberate:** step 3 says a "signed" link. Every comparable flow
in this codebase (`user_invites`, `password_resets`, `mobile_refresh_tokens`) uses an opaque random
token with only its sha256 at rest, and `email_verifications` matches them rather than introducing a
second token mechanism for one caller.
**`provisionSsoPlayer` now returns `{ user }` or `{ error }`** instead of the user or a bare null. Two
ways to fail need two things said to the person at the browser; the two call sites map `error`
straight onto the `sso_error` code the login pages already render.
**Verified on a live rig**, not only in unit tests — a real MariaDB 11.8 seeded with the pre-upgrade
schema plus three accounts sharing an address, upgraded by booting the real server, with a real SMTP
send into a mail catcher (which also discharges Phase 1's outstanding "no live SMTP send"):
- the upgrade **boots clean**; oldest kept the address, two were nulled and reported with the exact
addresses they lost; `josé@` and `jose@` both survived
- the gate seeded **`off` on the upgrade** and `on` on a fresh install
- the dashboard warning fired with the right count and cleared on acknowledge
- a change request staged the address and **left the live one receiving mail**; the link went only to
the new address; opening it from a session-less client installed the address and **set no cookie**
- a replayed link, and a second account confirming an address the first had just taken, both returned
the **byte-identical** generic 404 — the real reason logged, never returned
- `NEWMAIL@RIG.TEST` was refused at registration as a duplicate of `newmail@rig.test`, while
`néwmail@rig.test` registered successfully beside it
**Left for later, deliberately:** nothing consumes `email_verification_required` yet — the engine that
would honour it is Phase 4 and the deliverability rules are Phase 9. It is seeded and editable now
because the fresh-vs-upgrade distinction is only knowable at the migration that adds it, and
reconstructing "was this install fresh?" afterwards is guesswork.
---
### Phase 2 — The trigger registry and the variable contract