diff --git a/website/API_V2_PLAN.md b/website/API_V2_PLAN.md index 03557a3..76eca46 100644 --- a/website/API_V2_PLAN.md +++ b/website/API_V2_PLAN.md @@ -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`, diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 6318e63..724ba2a 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -29,11 +29,15 @@ Public contact email: **UOMysticmoon@gmail.com** Skeleton from the spec, with a small number of justified additions marked **(+)**. -> **Planned change:** the monolithic route files below (`admin.routes.js` especially, 1552 lines / -> 110 routes) are being split into one router file per business capability — **in place, with every -> URL unchanged**. This section and §4 get updated as each split PR lands. See +> **In progress:** the monolithic route files below (`admin.routes.js` especially, originally 1552 +> lines / 110 routes) are being split into one router file per business capability — **in place, with +> every URL unchanged**. This section and §4 get updated as each split PR lands. See > [API_V2_PLAN.md](./API_V2_PLAN.md) § Phase 2. > +> **Landed so far:** admin `users`, `account`, `invites` and `auth/providers` (28 routes) now live in +> their own routers under `admin/`, behind a new `admin/index.js`. The remaining 82 admin routes are +> still in `admin.routes.js`, and `public/` and `player/` are untouched. +> > "Every URL unchanged" is enforced mechanically, not by review: `server/scripts/routeManifest.js` > (`npm run routes:manifest`) walks the live Express stack and writes the sorted > `{ method, path }` freeze to `server/routes.manifest.json`, mirrored here as @@ -56,7 +60,20 @@ server/ v1.router.js mounts /auth /public /admin auth/ auth.routes.js + auth.controller.js public/ public.routes.js + public.controller.js - admin/ admin.routes.js + admin.controller.js + admin/ index.js mounts the capability routers below at their + own prefixes; owns the shared + `noindex, isLoggedIn, staffOnly` gate and + declares no routes itself + account.router.js (6) /admin/account — self-service, no adminOnly + users.router.js (15) /admin/users — adminOnly + invites.router.js (3) /admin/invites — adminOnly + authProviders.router.js (4) /admin/auth — adminOnly + admin.routes.js (82) everything not yet split, mounted + last at the group root; goes away + when the final split PR lands + admin.controller.js + the per-capability controllers + (already domain-split; the split PRs re-wire + routes, not logic) model/ users/ users.model.js + users.db.js posts/ posts.model.js + posts.db.js (news/five-on-friday/newsletter/screenshots) @@ -440,7 +457,12 @@ expiry (~5 min); `/exchange` is rate-limited per-IP. The bridge tables self-prun Public content GETs pass through the **siteMode** gate (§5). -### /admin (admin.routes.js → admin.controller.js) — all behind `isLoggedIn` + `noindex` +### /admin (admin/index.js → the capability routers in §2) — all behind `isLoggedIn` + `noindex` + `staffOnly` + +`admin/index.js` applies the shared gate and mounts each capability router at the prefix it owns; +`users`, `invites` and `auth/providers` add `adminOnly` on top. Routes not yet extracted still live +in `admin.routes.js`, mounted last at the group root. The URLs below are unaffected by which file a +route currently sits in — that is the property the route manifest freezes. | Method | Path | Purpose | |---|---|---| | GET | `/dashboard` | current mode, last change time + who, content counts, recent activity |