docs(website): record split PR 1 — admin users, account, invites, auth providers
Documentation half of the first of five domain-split PRs (API_V2_PLAN.md § Phase 2). BACKEND_DESIGN.md - §2 folder structure: admin/ now shows index.js (shared gate + mount table) and the four capability routers with their route counts, prefixes and extra gates; admin.routes.js is labelled as the 82-route residual that goes away with PR 5. - §4 /admin heading: was "admin.routes.js -> admin.controller.js", now points at admin/index.js and notes staffOnly, which the old heading omitted. - The "planned change" note becomes "in progress" with what has landed. API_V2_PLAN.md - Status: planning -> in progress; PR 1 marked landed in the sequencing list. - New "PR 1 — as landed" section: the route-count table (6+15+3+4+82 = 110) and three findings for PRs 2-5 — why the self-service /shard/* routes stay with the shard capability despite their Admin · Account tag, why a prefix mount must not be "simplified" to a pathless one (a bare use(gate) would then run for requests headed to later mounts), and that routes.guards.json came back zero-diff too. - New section on the swagger path-normalization prerequisite and its consequence: with sorted path keys, a pure route move produces no spec diff, so the spec becomes a third zero-diff gate alongside the manifest and guards files. - Correction to step 6: PROJECT_TREE.md is auto-generated by the sync-project-tree workflow since website#98 and must not be hand-edited in split PRs. api-route-inventory.json is unchanged — verified still byte-identical to server/routes.manifest.json (200 public + 2 internal), which is the point. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
# Website API — router domain split + CSP hardening
|
||||
|
||||
Status: **planning** · Target repo: `website/` · Docs owner: this file + `BACKEND_DESIGN.md`
|
||||
Status: **in progress** — PR 0 (route manifest), CSP report-only, and split PR 1 of 5 have landed ·
|
||||
Target repo: `website/` · Docs owner: this file + `BACKEND_DESIGN.md`
|
||||
|
||||
> **This file replaces the earlier "API v2" plan** (auth merge → CSP → domain split, with a parallel
|
||||
> `/api/v2` mount and an `/api/mobile` facade). Three of those four pieces are **not being built**:
|
||||
@@ -303,6 +304,63 @@ Because the auth model is untouched and the URLs are frozen, each PR is a **pure
|
||||
with a green test suite and a zero-diff route manifest as its acceptance criteria** — which is what
|
||||
makes grouped PRs actually reviewable.
|
||||
|
||||
> **Step 6 correction:** `PROJECT_TREE.md` is no longer hand-edited. Since website#98 it is
|
||||
> auto-generated by the `sync-project-tree` CI workflow, which opens its own docs PR after a merge to
|
||||
> `main`. Leave it alone in split PRs. `BACKEND_DESIGN.md` §2/§4 are still manual.
|
||||
|
||||
### PR 1 — as landed
|
||||
|
||||
`admin/index.js` owns the shared `noindex, isLoggedIn, staffOnly` gate and the mount table, and
|
||||
declares no routes itself. The gate sits **ahead of every mount**, so a capability router extracted in
|
||||
a later PR cannot silently ship without it. Route counts:
|
||||
|
||||
| Router | Routes | Prefix | Extra gate |
|
||||
|---|---|---|---|
|
||||
| `account.router.js` | 6 | `/admin/account` | none — self-service, an editor manages their own 2FA |
|
||||
| `users.router.js` | 15 | `/admin/users` | `adminOnly` at router level |
|
||||
| `invites.router.js` | 3 | `/admin/invites` | `adminOnly` per route |
|
||||
| `authProviders.router.js` | 4 | `/admin/auth` (routes are `/providers[/:id]`) | `adminOnly` per route |
|
||||
| `admin.routes.js` (residual) | 82 | group root, mounted last | unchanged |
|
||||
|
||||
6 + 15 + 3 + 4 + 82 = the 110 inventoried admin routes. None of the four prefixes appears in the
|
||||
residual file, so nothing depends on mount ordering.
|
||||
|
||||
Three findings worth carrying into PRs 2–5:
|
||||
|
||||
- **The self-service `/shard/*` routes stay with `shard` (PR 4), despite their `Admin · Account`
|
||||
swagger tag.** The invariant is *prefix ownership*, not tag agreement: one router owns `/shard`, so
|
||||
splitting six routes off it by capability would mean two routers mounting under the same prefix and
|
||||
an ordering hazard for no gain. Retag them in PR 4 if the tag still grates.
|
||||
- **`usersRouter.use(adminOnly)` is exactly equivalent to the old `adminRouter.use('/users', adminOnly)`**
|
||||
now that the router is mounted at `/users` — but only because it is mounted at a prefix. Under a
|
||||
*pathless* mount, a bare `use(gate)` would run for every request passing through en route to a later
|
||||
mount, 403-ing an editor on `/admin/posts`. Do not "simplify" a prefix mount away.
|
||||
- **`routes.guards.json` came back zero-diff too**, not just the manifest — no route lost or gained a
|
||||
gate. Worth checking both every time; the guards file is the one that would catch a dropped
|
||||
`adminOnly` that the method+path freeze cannot see.
|
||||
|
||||
The OpenAPI spec was also byte-for-byte unchanged, which required a prerequisite fix — see below.
|
||||
|
||||
### The swagger path-normalization prerequisite (landed before PR 1)
|
||||
|
||||
swagger-autogen builds a path by string-concatenating the mount prefix with the route argument, so a
|
||||
capability router mounted at `/users` whose collection route is `router.get('/')` documents as
|
||||
`/api/v1/admin/users/` — advertising a URL no client calls while dropping the one the SPA, the Android
|
||||
app and the Discord bot all do. Express is indifferent; the published spec is not. It also emits path
|
||||
keys in *router-traversal order*, so moving a route between files rewrote most of the ~5k-line
|
||||
committed artifact even when the API was provably unchanged — burying the one line a reviewer needs.
|
||||
|
||||
Both are fixed once in `server/swagger/swagger.js`, which post-processes the generator's output to
|
||||
strip trailing slashes and sort path keys (throwing on a collision rather than silently dropping an
|
||||
operation). It shipped as its own PR ahead of PR 1, verified inert by the regenerated spec being
|
||||
byte-for-byte the sorted form of the previously committed one — 198 operations, none added or removed.
|
||||
`#swagger.path` was rejected as the fix: it bypasses the mount prefix, so every route would hardcode
|
||||
its absolute path in a comment that silently lies the moment a mount moves.
|
||||
|
||||
**Consequence for PRs 2–5: the swagger diff is now a signal.** With sorting in place, a pure route
|
||||
move produces *no* spec diff at all, so any diff there means an annotation actually changed. Treat
|
||||
`swagger-output.json`, `routes.manifest.json` and `routes.guards.json` as three zero-diff gates.
|
||||
|
||||
### PR 0 — the route manifest (prerequisite of the first split PR) — **landed**
|
||||
|
||||
> **Status: shipped.** `server/scripts/routeManifest.js` + `npm run routes:manifest`,
|
||||
@@ -390,7 +448,7 @@ deliberate `+1` in the manifest — which is exactly the mechanism working as de
|
||||
data**, not on code: watch the `csp` log tag for `frame-ancestors` reports across one release before
|
||||
flipping. Also decide there whether `/api/csp-report` is retired with the report-only twin or kept
|
||||
as a `report-to` group on the enforced policy.
|
||||
4. **PR 1 — admin:** `users`, `account`, `invites`, `auth` (providers).
|
||||
4. **PR 1 — admin:** `users`, `account`, `invites`, `auth` (providers). ✅ landed
|
||||
5. **PR 2 — admin:** `moderation`, `bot-activity`, `activity`.
|
||||
6. **PR 3 — admin (content):** `posts`, `pages`, `wiki`, `uploads`.
|
||||
7. **PR 4 — admin (ops/config):** `shard`, `uo-link`, `email`, `discord-bot`, `settings`, `site-mode`,
|
||||
|
||||
Reference in New Issue
Block a user