docs(shard): record the REST projection gap the Part A smoke test found

The live five-rung smoke test of the visibility framework found that Part
A enforced it on the SSE path and on /guilds + /governors, but not on the
remaining public REST reads - so one event was projected live and served
verbatim from stored history.

link/v3.md gains 3.6.1 with the full list (the anonymous acct/webId leak
on /feed, the flattened ownerAcct on /idoc, the dead `houses` field
rules, /feed ignoring live config, the empty-allowlist fall-through, and
the Date-to-{} projection bug), plus the rule it leaves behind: a read
path that returns shard data and does not project is a bug, and every new
Part B/C surface must gate its kind set on live config rather than on
PUBLIC_KINDS.

3.5 also corrected: the table is NOT seeded on boot. An absent row means
"use the compiled default", which keeps the defaults in one place instead
of duplicating them into a seeder that could drift.

BACKEND_DESIGN.md 6.5 records the same as a security contract: rule 1
locks a field by meaning rather than spelling; PUBLIC_KINDS is a
module-load constant and must not answer per-caller questions;
projectFeature walks arrays and plain objects only.

SHARD_VISIBILITY.md gets the admin-facing version - that stored history
answers the same way the live stream does, and that turning live updates
off stops the push, not the reading.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-07-28 10:52:13 -05:00
parent 5cb77595aa
commit 35ad440bad
4 changed files with 207 additions and 10 deletions

View File

@@ -0,0 +1,126 @@
# Handoff — Trusted Devices & MFA: remaining work
> Written 2026-07-22 at the end of the backend/web implementation session, for a
> fresh session to continue. Canonical design: `docs/website/TRUSTED_DEVICES_MFA.md`.
> App-side plan: `docs/android/PLAN.md §4.1.1`. This doc is the "what's left + how".
## 1. Status at handoff
**Done, in review, and live-smoke-tested** (real MariaDB + AVD):
- Backend (schema, session service, web + mobile login, self-service + admin
endpoints, invalidation), web client UI, admin front-end UI, OpenAPI spec,
33 server tests — all in **website PR #93** (branch `feature/trusted-devices-mfa`).
- Docs (BACKEND_DESIGN §3/§4/§6, TRUSTED_DEVICES_MFA.md, PLAN §4.1.1) — **docs PR #32**
(branch `docs/trusted-devices-mfa`).
- Live smoke test passed end-to-end: trusted-device TOTP-skip (web + mobile
`X-Trust-Token`), recovery-code login (single-use), cap 409, admin MFA reset,
audit logging, and a real 2FA login through the Android app on an emulator.
**Not done — the two remaining items below.**
## 2. Remaining item A — merge gate (no code)
- **website#93** and **docs#32** need CI green + review, then merge to `main`.
- CI (`.gitea/workflows/pr-checks.yml`) runs server tests, client build, bot install.
- Nothing to build here; just get them reviewed/merged. The Android work should land
**after** #93 merges so the app builds against the merged contract.
## 3. Remaining item B — Android app implementation (the real work)
The backend is fully ready and additive; the existing app is unaffected (verified).
The app just needs to *consume* the new endpoints. Own PR in the **`android-app`**
repo, branch `feature/trusted-devices-mfa`. Package id `com.runicgateway.app`.
### 3.1 Scope (from PLAN §4.1.1)
1. **Login/TOTP screen:** on the existing `401 { totpRequired }` step, add
- a **"Trust this device"** checkbox, and
- a **"use a recovery code instead"** toggle (send `recoveryCode` instead of `code`).
2. **Trust token storage:** when a login response carries `trustToken`, store it in
**EncryptedSharedPreferences** (same secure store as the bearer tokens — never
plain prefs/logs). On subsequent logins send it as the **`X-Trust-Token`** header
so the server skips the TOTP prompt.
3. **Cap handling:** a login response with `{ trustLimitReached: true, devices }`
means show the device list and prompt the user to revoke one
(`DELETE /auth/me/trusted-devices/:id`) then retry trusting.
4. **Account screens:**
- **Trusted Devices**: list (`GET /auth/me/trusted-devices`), revoke one, untrust
all; "trust this device" (`POST /auth/me/trusted-devices` → returns `trustToken`
for native — store it).
- **Recovery Codes**: show the one-time batch returned by TOTP enable; remaining
count (`GET …/recovery-codes/status`); regenerate (`POST …/recovery-codes/generate`,
password step-up) with a show-once display + copy/share.
5. **Invalidation:** on logout / dead-refresh sign-out / Settings→Server switch,
**clear the stored `trustToken`** alongside the bearer tokens. (A server-side
password change/reset or TOTP disable already revokes it.)
6. **Tests:** JVM `:app:testDebugUnitTest` — DTO decode for the new fields, and
repository logic (trust-token persist/clear, recoveryCode vs code branch).
### 3.2 Exact API contract the app consumes
`POST /auth/mobile/login` — body `{ username, password, code?, recoveryCode?, trustDevice?, device_name? }`, optional header `X-Trust-Token: <token>`
- `200``{ accessToken, refreshToken, expiresIn, user:{id,username,role}, trustToken?, trustLimitReached?, devices? }`
- `trustToken` present only when `trustDevice:true` was accepted (store it).
- `trustLimitReached:true` + `devices[]` when at the cap (login still succeeded).
- `401``{ totpRequired:true, message }` (missing/invalid 2nd factor) — reveal the
code field (existing behavior); or generic `{ message }` for bad credentials.
- A valid `X-Trust-Token` bound to the user makes a code unnecessary → straight `200`.
Self-service (Bearer access token):
- `GET /auth/me/trusted-devices``[{ id, platform, deviceName, userAgent, createdAt, lastUsedAt, expiresAt }]`
- `POST /auth/me/trusted-devices` body `{ deviceName? }``{ trusted:true, trustToken }` (native) | `409 { error:'trusted_device_limit', devices }`
- `DELETE /auth/me/trusted-devices/:id``{ revoked:boolean }`
- `DELETE /auth/me/trusted-devices``{ revoked:number }`
- `GET /auth/me/account/recovery-codes/status``{ remaining:number }`
- `POST /auth/me/account/recovery-codes/generate` body `{ currentPassword? }``{ recoveryCodes:[string] }`
- `POST /auth/me/account/totp/enable` body `{ code }``{ totp_enabled:true, recoveryCodes:[string] }` (codes shown once)
The OpenAPI spec (`website/server/swagger/swagger-output.json`, schemas `TrustedDevice`,
`TrustDeviceResult`, `TrustedDeviceLimit`, `RecoveryCodes`) is the source of truth.
### 3.3 Where it likely goes in the app
The app already handles `totpRequired` (reveals a code field — verified live), so the
auth surface exists. Extend the existing auth Retrofit API + repository + login
ViewModel/screen, add a secure `trustToken` accessor to the encrypted token store,
and add two account screens. Explore `android-app/app/src/main/java/com/runicgateway/app/`
(auth/data/core modules) at the start — don't assume file names.
## 4. Environment & how-to (verified this session)
- **DB:** Docker container `uomm-db`, MariaDB on host port **3307**, db `uomysticmoon`,
user `uomm` (password: `docker exec uomm-db printenv MARIADB_PASSWORD`).
- **Run the server:** `cd website/server && node src/server.js` (uses `.env`, already
points at 127.0.0.1:3307). It ensures schema + seeds on boot. For cap testing set
`MAX_TRUSTED_DEVICES=2`; recovery count via `RECOVERY_CODE_COUNT`.
- Pre-existing noise: `uo-link-socket … Unsupported state` decrypt errors are an old
encrypted `uo_link_config` row, unrelated — ignore.
- **Server tests:** `cd website/server && DB_HOST=127.0.0.1 DB_PORT=59999 node --test`
(dead port by design; models stubbed). Client: `cd website/client && npm test`.
- **TOTP codes for manual testing:** from `website/server`,
`node -e "process.stdout.write(require('speakeasy').totp({secret:'<BASE32>',encoding:'base32'}))"`.
- **Android build:** JDK 21; `cd android-app && ./gradlew :app:assembleDebug -Pksp.incremental=false`.
Unit tests: `./gradlew :app:testDebugUnitTest -Pksp.incremental=false`.
- **Emulator:** SDK at `~/AppData/Local/Android/Sdk`; AVDs `Medium_Phone_API_36.1`,
`s22_ultra`. `adb install -r -g app/build/outputs/apk/debug/app-debug.apk`.
- **⚠ Cleartext HTTP gotcha (dev only):** the app blocks plain HTTP to `10.0.2.2`
(`CLEARTEXT communication … not permitted`). Cleartext to `127.0.0.1` **is** allowed,
so for local testing run `adb reverse tcp:3000 tcp:3000` and set the app's server URL
to `http://127.0.0.1:3000` (its default). Production uses HTTPS via the proxy — not a
code issue. (Consider whether v1 wants a `network_security_config` dev exception; not
required for the feature.)
## 5. Conventions (CLAUDE.md)
- Branch from up-to-date `main`; commit/push as **`wtclaude`** using the token at
`C:\Users\colby\.gitea_token_claude` via `http.extraHeader` (never inline in URL).
- Conventional Commits; end commit messages with the `Co-Authored-By: Claude …` +
`Claude-Session:` trailers. PR template requires ticking the **AI-assisted** box
(tool: Claude Code) and the license box.
- Use `mcp__gitea__*` for PRs/issues. GPL-3.0-or-later.
## 6. First moves for the fresh session
1. Check whether website#93 / docs#32 have merged (`mcp__gitea__pull_request_read`).
2. In `android-app`: sync `main`, `git checkout -b feature/trusted-devices-mfa`.
3. Explore the app's auth module; implement §3 against the §3.2 contract.
4. Test with the local server + emulator via the §4 cleartext workaround.
5. Open a single `android-app` PR; update `docs/android/PLAN.md` if the app design
deviates from §4.1.1.

View File

@@ -115,9 +115,13 @@ CREATE TABLE IF NOT EXISTS shard_feature_visibility (
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
``` ```
Seeded on boot in `server.js`, one row per feature. **All ten shard features are covered — the four **Not** seeded on boot (this changed during implementation): an **absent row means "use the compiled
new ones and the six that already ship — and every default reproduces today's behavior, so the default"**, so the table starts empty and only ever holds rows an admin has actually touched. The
retrofit is a no-op until an admin changes something.** defaults live in one place — `FEATURES` in `shardVisibility.js` — instead of being duplicated into a
seeder that could drift from it, and a DB blip degrades to those same defaults rather than to
"everything is public". **All ten shard features are covered — the four new ones and the six that
already ship — and every default reproduces today's behavior, so the retrofit is a no-op until an
admin changes something.**
| Feature | Default audience | Sensitive fields (default rung) | | Feature | Default audience | Sensitive fields (default rung) |
|---|---|---| |---|---|---|
@@ -156,10 +160,42 @@ Applied at:
3. **Nav**`GET /api/v1/public/shard/features` returns only the features the calling viewer can 3. **Nav**`GET /api/v1/public/shard/features` returns only the features the calling viewer can
see, so the SPA hides nav entries rather than rendering links that 403. see, so the SPA hides nav entries rather than rendering links that 403.
### 3.6.1 What the first implementation missed (found by the §11 smoke test, fixed)
Part A shipped enforcement on the SSE path and on `/guilds` + `/governors`, but the **remaining public
REST reads never called into it** — so the same event was projected live and served verbatim from
history. Recorded because each miss is a shape the next phase can repeat:
- **`/public/shard/feed` returned the stored payload as-is.** `actor.acct` / `actor.webId` were
readable *anonymously* for every logged kind (`player.death`, `mob.killed`, `skill.gain`,
`guild.join`, …) — broader than the §3.1 leak, which was limited to board holders.
- **`/public/shard/idoc` returned `ownerAcct`.** Rule 1 keyed on the exact strings `acct`/`webId`,
but `shapeHouse` flattens the actor into `ownerAcct` / `ownerName` / `ownerSerial`. The lock is now
on the field's **meaning** — a key that is or ends in `acct`/`webId`, case-insensitively — so
flattened spellings are covered and unwritten shapes fail closed.
- **The `houses` field rules were dead config.** Neither `getIdoc` nor `getHouses` projected, so the
panel offered toggles that did nothing. **Every feature's declared fields must name the keys the
read model actually emits**, not just the wire frame's.
- **`/feed` filtered on `PUBLIC_KINDS`**, a module-load constant derived from the compiled defaults,
so live audience changes never reached it. `visibleKinds(level, config)` resolves the readable set
from live config; it deliberately ignores the `stream` flag, which governs SSE fan-out only (market
history stays readable with its firehose off).
- **`shardEvents.db.list` treated an empty `kinds` array as "no filter"** and fell through to an
unfiltered `SELECT`. A fully-gated config would have dumped the whole event log, staff audit
included. An empty allowlist now serves nothing.
- **`projectValue` recursed into every object**, so a `Date` column came back as `{}`. It walks
arrays and plain objects only. The unit tests used JSON fixtures and could not have caught this —
the live read did, which is the argument for §11's smoke test over tests alone.
**The rule this leaves behind:** *a read path that returns shard data and does not call
`projectFeature` is a bug.* Every new surface in Parts B and C — `/ruleset`, `/points`, `/market`,
`/atlas` — must project, and must gate its kind set on live config rather than on `PUBLIC_KINDS`.
### 3.7 Admin surface ### 3.7 Admin surface
`GET` / `PUT /api/v1/admin/shard/visibility` (admin-only). Validate feature names against the known `GET` / `PUT /api/v1/admin/shard/visibility` (admin-only). Validate feature names against the known
set and rungs against the ladder; reject any attempt to set `acct`/`webId` below `admin`. Writes an set and rungs against the ladder; reject any attempt to set a locked field below `admin` — including
its flattened spellings (`ownerAcct`, `leaderWebId`), see §3.6.1. Writes an
`admin.audit`-style row so visibility changes are traceable. New client panel `admin.audit`-style row so visibility changes are traceable. New client panel
`routes/admin/ShardVisibility.jsx` at `/admin/shard-visibility`, linked from `ShardAdmin.jsx`. `routes/admin/ShardVisibility.jsx` at `/admin/shard-visibility`, linked from `ShardAdmin.jsx`.

View File

@@ -711,6 +711,10 @@ rather than silently ignore:
1. **`acct` and `webId` are admin-only, always.** They are not exposed as configurable fields, and a 1. **`acct` and `webId` are admin-only, always.** They are not exposed as configurable fields, and a
stored row attempting to loosen them is discarded on read as well as rejected on write. A character stored row attempting to loosen them is discarded on read as well as rejected on write. A character
name is visible in game; the account behind it and the website user it links to are not. name is visible in game; the account behind it and the website user it links to are not.
The lock is on the field's **meaning, not one spelling**: `isLockedField(key)` matches a key that
*is* or *ends in* `acct`/`webId`, case-insensitively, so the flattened forms the read models emit
(`shapeHouse``ownerAcct`, `shapeGuild``leaderWebId`) are covered too. An exact-key check was
the original implementation and it let `GET /public/shard/idoc` serve `ownerAcct` anonymously.
2. **A kind absent from `KIND_FEATURE` is never broadcast below `admin`.** Fail closed. This is what 2. **A kind absent from `KIND_FEATURE` is never broadcast below `admin`.** Fail closed. This is what
keeps the kind map a security boundary rather than a convenience filter, and it means a shard that keeps the kind map a security boundary rather than a convenience filter, and it means a shard that
starts emitting an unknown event degrades to staff-only, never to public. starts emitting an unknown event degrades to staff-only, never to public.
@@ -731,14 +735,31 @@ it picks, it fails open on one side.)
| Nav | `GET /public/shard/features` returns only what the caller may reach, so the SPA never renders a link that would 403. Presentation only. | | Nav | `GET /public/shard/features` returns only what the caller may reach, so the SPA never renders a link that would 403. Presentation only. |
Config reads are cached ~5s, so admin changes take effect within seconds **including on already-open Config reads are cached ~5s, so admin changes take effect within seconds **including on already-open
streams**. `PUBLIC_KINDS` still exists and is still exported (`/feed` filtering, `notificationStreams.js`) streams**. `PUBLIC_KINDS` still exists and is still exported (`notificationStreams.js`) but is now
but is now **derived** from the kind map rather than hand-maintained, so the two cannot drift. **derived** from the kind map rather than hand-maintained, so the two cannot drift.
**`PUBLIC_KINDS` is a module-load constant and must not be used to answer "may this caller read this
kind?"** — it is computed from the compiled *defaults*, so it cannot see an admin's changes. Use
`visibleKinds(level, config)`, which resolves against the live config. `/feed` uses it; it originally
used `PUBLIC_KINDS` and consequently kept serving `guild.join` to anonymous callers after an admin had
moved `guilds` to `staff`. `visibleKinds` deliberately ignores the `stream` flag: that governs SSE
fan-out only, so a feature whose live firehose ships off (market) stays readable from stored history.
**Every read path that returns shard data must call `projectFeature`.** The stored-history endpoints
are not exempt — `/feed` returns the same events the stream does, and returning them unprojected
reopens on the REST side exactly what the stream closes. Relatedly, `shardEvents.db.list` treats an
**empty** `kinds` array as "serve nothing", never "no filter"; the fall-through it used to take would
have turned a fully-gated config into a dump of the entire event log.
`projectFeature` walks **arrays and plain objects only**. A `Date`, `Buffer` or other class instance
is passed through as a value — rebuilding one key-by-key yields `{}`, which is the difference between
the pure-JSON wire frames and the DB-backed read models whose rows carry real `Date` columns.
**Defaults reproduce pre-3.0 behavior exactly**, so installing the framework is a no-op until an admin **Defaults reproduce pre-3.0 behavior exactly**, so installing the framework is a no-op until an admin
changes something — with one deliberate exception, which is the leak it was written to close: changes something — with deliberate exceptions, which are the leaks it was written to close.
`/public/shard/guilds` and `/public/shard/governors` previously returned the raw stored payload, whose `/public/shard/guilds`, `/public/shard/governors` and `/public/shard/feed` previously returned the raw
`leader` / `governor` actors carry `acct` and `webId`. Those fields are now stripped for every caller stored payload, whose actors carry `acct` and `webId`; `/public/shard/idoc` returned the flattened
below admin. `ownerAcct`. All are now stripped for every caller below admin.
--- ---

View File

@@ -80,6 +80,11 @@ game to anyone standing next to them; the **account** behind it is not, and neit
user it's linked to. Publishing those would disclose something the shard itself doesn't, and would user it's linked to. Publishing those would disclose something the shard itself doesn't, and would
tie a player's in-game identity to their forum identity without their consent. tie a player's in-game identity to their forum identity without their consent.
This rule matches the *meaning* of a field, not one spelling of it. Some responses nest the player
who owns a record (`leader.acct`); others flatten it into the row (`ownerAcct`, `leaderWebId`,
`governorAcct`). Every one of those is locked, and the admin API refuses to configure any of them —
so a new response shape can't quietly reopen the hole by naming the field differently.
**2. Unknown event kinds are never broadcast below admin.** **2. Unknown event kinds are never broadcast below admin.**
The live stream maps each event kind to a feature. A kind with no mapping — a new event from a shard The live stream maps each event kind to a feature. A kind with no mapping — a new event from a shard
plugin the site doesn't know yet, say — goes to admins only. It fails closed. This is what keeps the plugin the site doesn't know yet, say — goes to admins only. It fails closed. This is what keeps the
@@ -99,6 +104,15 @@ Three places, one config:
- **Navigation** hides links a viewer can't follow, so they don't hit a wall. This is presentation - **Navigation** hides links a viewer can't follow, so they don't hit a wall. This is presentation
only — the gate is server-side either way. only — the gate is server-side either way.
**Stored history answers the same way the live stream does.** The activity feed reads from the event
log rather than the live stream, but it resolves the *same* question against the *same* config: which
kinds you may read, and which fields survive. So moving a feature up a rung hides it from the history
as well as the stream — there is no back door where yesterday's copy of an event is more revealing
than today's.
One deliberate asymmetry: turning **live updates** off for a feature stops the push, not the reading.
The marketplace ships this way — its history and its pages are public, only the firehose is off.
Changes take effect within about five seconds, **including on streams that are already open**. You Changes take effect within about five seconds, **including on streams that are already open**. You
don't need to restart anything. don't need to restart anything.