diff --git a/website/API_V2_PLAN.md b/website/API_V2_PLAN.md index 3a9cca5..a95f987 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: **in progress** — PR 0 (route manifest), CSP report-only, and split PRs 1–2 of 5 have landed · +Status: **domain split complete** — PR 0 (route manifest), CSP report-only and split PRs 1–5 have all +landed; only the CSP enforce PR remains, and it is blocked on soak data rather than on code · 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 @@ -478,6 +479,91 @@ Notes: - **`/admin/shard/pages` vs `/admin/pages` stayed separate**, as PR 3 flagged: the former is the in-game help-page (support) queue and belongs to `shard`; the latter is the CMS page builder. +### PR 5 — as landed + +`public`, `player` and the residual `auth` — 54 routes across three groups, the last split PR. +`public.routes.js`, `player.routes.js` and `auth.routes.js` are all **deleted**, so every one of the +200 routes in the manifest is now declared in a capability router and no monolithic route file +remains anywhere in `router/v1/`. + +| Group | Router | Routes | Prefix | Extra gate | +|---|---|---|---|---| +| `public` | `posts.router.js` | 2 | `/public/posts` | none — `siteMode` per route | +| | `wiki.router.js` | 4 | `/public/wiki` | none — `siteMode` per route | +| | `pages.router.js` | 2 | `/public/pages` | none — `siteMode` per route except the preview | +| | `shard.router.js` | 12 | `/public/shard` | none — never `siteMode` gated | +| | `site.router.js` | 4 | **group root** (`/settings`, `/status`, `/version`, `/contact`) | none | +| `player` | `account.router.js` | 8 | `/player/account` | none beyond the group gate | +| | `shard.router.js` | 8 | `/player/shard` | none beyond the group gate | +| | `appeals.router.js` | 4 | `/player/appeals` | none beyond the group gate | +| `auth` | `login.router.js` | 2 | `/auth/login` | `loginGuards` per route | +| | `register.router.js` | 1 | `/auth/register` | `loginGuards` + `registerLimiter` | +| | `invite.router.js` | 2 | `/auth/invite` | `loginGuards` + `registerLimiter` on accept | +| | `password.router.js` | 3 | `/auth/password` | per-route reset limiters | +| | `session.router.js` | 2 | **group root** (`/logout`, `/me`) | per route | + +24 + 20 + 10 = 54. `auth/`'s other 32 routes (`me` 23, `mobile` 5, `sso` 4) were already in their own +files and did not move. All four gates zero-diff: `routes.manifest.json` (200 public + 2 internal), +`routes.guards.json`, `swagger-output.json` (198 operations), and `docs/website/api-route-inventory.json` +was already in sync. 434 server tests green. + +Notes: + +- **Each group is now a directory with an `index.js`**, matching `admin/`: `public/index.js`, + `player/index.js`, `auth/index.js` own the group gate (where there is one) and the mount table and + declare no routes. `v1.router.js` requires the directories. The four tests that imported the + deleted entry files were repointed. +- **Two of the three groups have no group gate, and that is the security-relevant fact about them.** + `player/index.js` carries `noindex, requireAuth` — authenticated, *any* role, because staff are a + superset of players. `public/index.js` and `auth/index.js` carry **nothing**, deliberately: the + public surface is anonymous by contract (logged-out SPA, Discord bot, and the Android + `ShardStreamClient` on `/public/shard/stream`, none of which send credentials), and `/auth` is where + an anonymous caller *becomes* authenticated. Both index files say so, because the obvious "hardening" + edit to either one is an outage. +- **`GET /auth/me` has a mount-order dependency, and it is the one genuinely non-obvious thing in this + PR.** `authRouter.use('/me', meRouter)` matches the bare path `/me`, not just `/me/*` — so a request + to `GET /auth/me` runs `meRouter`'s (and `notifRouter`'s) `noindex, requireAuth`, matches no route + inside either, and falls through to its own handler. `session.router.js` must therefore stay mounted + **last**. Verified by the counterfactual rather than by reading the mount table: moving the mount to + the top of `auth/index.js` still answers `401`, but the response loses its `X-Robots-Tag` header. No + gate file and neither manifest can see that — only a header assertion can. +- **Two root-mounted routers, on the PR 4 `dashboard.router.js` precedent.** `public/site.router.js` + (`/settings`, `/status`, `/version`, `/contact`) and `auth/session.router.js` (`/logout`, `/me`) hold + the routes that own no path segment. Both are safe at the root **only** because they declare no + router-level middleware — a bare `use(gate)` there runs for every request passing through toward + another mount. Both files say so. +- **`loginGuards` is the PR's one shared module**, the counterpart to PR 3's `imageUpload.js`. The + `[backoffGuard, slowLogin, loginLimiter]` array was defined inline in `auth.routes.js` and spread by + four routes that this PR puts in three different files — plus a fifth, already-duplicated copy in + `sso.routes.js`. It moved to `auth/loginGuards.js` and `sso.routes.js` now imports it too, so there + is one definition rather than five: duplicating a throttling stack is how the copies drift, and the + copy that drifts is the one that stops throttling. It is exported `Object.freeze`d — it is + module-level shared state, and a router that pushed onto it would silently add middleware to every + other login surface. Guard freshness is unaffected: the same three named functions, so + `routes.guards.json` did not move. +- **The `/:param` shadowing check was run again and is clean.** All 86 public/player/auth routes and + all 64 literal paths among them dispatch to their own layer. This was checked in *dispatch* order + against the built stack, since the manifest sorts and therefore cannot see declaration order. The + only ordering-sensitive pair is `GET /public/wiki/{categories,tags}` ahead of `/public/wiki/:slug` — + the public twin of the `admin/wiki.router.js` trap PR 3 found, and `wiki.router.js` says so. The + `/public/pages` preview route also stays ahead of `/:slug`, though at a different depth. +- **Filename deviations from the target tree, both for prefix agreement.** The tree named the public + posts router `news.router.js`; it is `posts.router.js`, matching the `/posts` prefix it owns and its + `admin/posts.router.js` sibling. The tree also implied `sso.router.js` / `mobile.router.js`; those + files already exist as `sso.routes.js` / `mobile.routes.js` and were not renamed — they did not move + in this PR, and churning their names would add diff noise to a PR whose value is being reviewable. +- **`auth/session.router.js` is a deviation the target tree did not anticipate**, the same shape as + PR 2's `activity.router.js`. The tree listed `login.router.js` but had nowhere to put `/logout` and + `GET /me`, which own no prefix. Folding them into `login.router.js` would have forced *that* router + to the group root and given up prefix ownership for the four login routes; a separate root-mounted + singleton file keeps `/login` a real prefix mount. +- **Tag mismatches were left alone again**, on the PR 1 / PR 3 / PR 4 precedent: the acceptance + criterion is a byte-identical spec, so retagging belongs in a PR that is about tags. +- **`public.controller.js` was not split.** Unlike the admin controllers, it is still one file serving + settings/status/version/contact *and* posts/wiki/pages. The plan's rule is that these PRs re-wire + routes, not logic — splitting a controller is a separate change with a separate risk profile, and + bundling it would have cost this PR its "pure mechanical refactor" acceptance criteria. + ### 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 @@ -591,7 +677,9 @@ deliberate `+1` in the manifest — which is exactly the mechanism working as de 7. **PR 4 — admin (ops/config):** `shard`, `uo-link`, `email`, `discord-bot`, `settings`, `site-mode`, `dashboard`. (`activity` went with PR 2 — see § PR 2 — as landed.) **This is the whole residual file** — `admin.routes.js` is deleted here, not by PR 5. ✅ landed -8. **PR 5 — `public/*` + `player/*`** (and the residual `auth/*` grouping). **The only split PR left.** +8. **PR 5 — `public/*` + `player/*`** (and the residual `auth/*` grouping). ✅ landed — **the domain + split is complete.** The only remaining item in this plan is the CSP enforce PR (3), which is + blocked on soak data, not on code. Each PR: **zero-line diff in `routes.manifest.json`**, server tests green (`cd website/server && npm test`), Swagger regenerated, matching `docs/` edit, Conventional Commit, diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 92b3cc9..c1eea89 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -29,17 +29,17 @@ Public contact email: **UOMysticmoon@gmail.com** Skeleton from the spec, with a small number of justified additions marked **(+)**. -> **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. +> **Complete.** The monolithic route files (`admin.routes.js` especially, originally 1552 lines / +> 110 routes) have been split into one router file per business capability — **in place, with every +> URL unchanged**. See [API_V2_PLAN.md](./API_V2_PLAN.md) § Phase 2. > -> **Landed so far:** the **admin group is fully split**. `users`, `account`, `invites`, -> `auth/providers` (PR 1, 28 routes), `moderation`, `bot-activity`, `activity` (PR 2, 18 routes), -> `posts`, `uploads`, `wiki`, `pages` (PR 3, 31 routes) and `shard`, `uo-link`, `email`, -> `discord-bot`, `settings`, `dashboard`/`site-mode` (PR 4, 33 routes) each live in their own router -> under `admin/`, behind `admin/index.js` — **`admin.routes.js` is deleted**. `public/` and `player/` -> are untouched and are the subject of the last split PR. +> `users`, `account`, `invites`, `auth/providers` (PR 1, 28 routes), `moderation`, `bot-activity`, +> `activity` (PR 2, 18 routes), `posts`, `uploads`, `wiki`, `pages` (PR 3, 31 routes) and `shard`, +> `uo-link`, `email`, `discord-bot`, `settings`, `dashboard`/`site-mode` (PR 4, 33 routes) each live +> in their own router under `admin/`, behind `admin/index.js`. PR 5 did the same for `public/` (24), +> `player/` (20) and the residual `auth/` (10). **`admin.routes.js`, `public.routes.js`, +> `player.routes.js` and `auth.routes.js` are all deleted**; each group is now a directory whose +> `index.js` owns the group gate and the mount table and declares no routes of its own. > > "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 @@ -60,9 +60,61 @@ server/ router/ api.router.js mounts /v1 v1/ - v1.router.js mounts /auth /public /admin - auth/ auth.routes.js + auth.controller.js - public/ public.routes.js + public.controller.js + v1.router.js mounts /auth /public /admin /player + auth/ index.js mounts the routers below; no group gate — /auth + is where an anonymous caller becomes + authenticated, so the authenticated parts gate + themselves. Mount order is load-bearing (see + session.router.js) + login.router.js (2) /auth/login + /login/totp — shared + loginGuards stack + register.router.js (1) /auth/register — honours the + player_registration setting + invite.router.js (2) /auth/invite/:token[/accept] — the + token is its own authority, so it + bypasses player_registration + password.router.js (3) /auth/password/forgot + reset/:token + session.router.js (2) POST /logout and GET /me — the two + singletons owning no path segment, so + mounted at the group root, LAST: the + /me sub-routers below also match the + bare /me and supply its noindex header + me.routes.js (23) /auth/me/account*, sessions, trusted + devices — router-level requireAuth + notifications.routes.js (3) /auth/me/devices*, notifications/* + mobile.routes.js + /auth/mobile/* — native bearer login + mobileSso.routes.js (5) + sso.routes.js (4) mounted PATHLESS: owns two prefixes, + /auth/providers and /auth/sso/* + loginGuards.js shared backoff/slow/limiter stack for + every credential-guessing surface + (not a router) + auth.controller.js + invite/passwordReset/sso/mobile controllers + public/ index.js mounts the routers below; **no group gate** — + this surface is anonymous by design (SPA + logged-out, Discord bot, Android ShardStream) + posts.router.js (2) /public/posts/:category[/:idOrSlug] + wiki.router.js (4) /public/wiki — /categories and /tags + MUST precede /:slug + pages.router.js (2) /public/pages — the draft-preview + route precedes /:slug and is + deliberately not site-mode gated + shard.router.js (12) /public/shard/* incl. the anonymous + SSE stream; never site-mode gated + site.router.js (4) /settings /status /version /contact — + the group-root singletons; declares no + router-level middleware + public.controller.js + shard.controller.js + player/ index.js owns the shared `noindex, requireAuth` gate + (authenticated, ANY role — staff are a superset + of players) and the mount table + account.router.js (8) /player/account — credentials, TOTP, + linked identities; handlers shared + with /admin/account and /auth/me + shard.router.js (8) /player/shard — linking + own roster, + vendors, chars, sales, houses + appeals.router.js (4) /player/appeals + shard.controller.js + appeals.controller.js admin/ index.js mounts the capability routers below at their own prefixes; owns the shared `noindex, isLoggedIn, staffOnly` gate and @@ -345,7 +397,12 @@ authenticated endpoints silently. Names are a hint only — `requireRole(...)` r arrow and cannot be observed — but a *missing* `requireAuth` is unambiguous, and the server test suite asserts every `/admin/**` and `/player/**` route still carries it. -### /auth (auth.routes.js → auth.controller.js) +### /auth (auth/index.js → the capability routers in §2) + +No group gate — `/auth` is where an anonymous caller becomes authenticated. The authenticated parts +gate themselves: `me.routes.js` and `notifications.routes.js` each apply `noindex, requireAuth` at +their own router level, and `/sso/:provider/link` carries `requireAuth` per route. + | Method | Path | Auth | Body | Purpose | |---|---|---|---|---| | POST | `/login` | — (rate-limited) | `{username,password}` | verify, set cookie, log `auth.login`, update `last_login_at`. If the account has TOTP **and this browser is a trusted device** (a valid `rg_trust` cookie bound to the user), the TOTP step is **skipped** and a session is issued directly (logs `auth.login.trusted_device`). Otherwise a 2FA account returns `{totpRequired, challenge}`. | @@ -378,9 +435,9 @@ lets a client (the Android app) manage its own account through one surface witho for web back-compat. **The `/player/*` group is self-service, not player-only.** Staff are a **superset** of players — every -player ability plus their staff tools on top — so the whole `/player/*` router (game-account linking, -character/vendor/house reads, credential changes, appeals) sits behind `requireAuth` **only**, never -`requireRole('player')`. Every handler is self-scoped to the caller by `req.user.id`, so an admin/editor/ +player ability plus their staff tools on top — so the whole group (`account.router.js`, +`shard.router.js`, `appeals.router.js`, mounted by `player/index.js`) sits behind the shared +`noindex, requireAuth` gate **only**, never `requireRole('player')`. Every handler is self-scoped to the caller by `req.user.id`, so an admin/editor/ moderator using it sees only their **own** linked accounts and characters (with the pre-existing `isAdmin` bypass still letting a genuine admin read *any* character). Staff also reach the identical self-scoped handlers under `/admin/shard/*` (same controller) for the web admin surface; the two are @@ -475,7 +532,13 @@ web sessions already use. **Authorization code.** Cryptographically random, ≥128 bits, stored **hash-only**, single-use, short expiry (~5 min); `/exchange` is rate-limited per-IP. The bridge tables self-prune (§3). -### /public (public.routes.js → public.controller.js) — all GET, no auth +### /public (public/index.js → the capability routers in §2) — all GET except `/contact`, no auth + +**No group gate, deliberately.** This surface is anonymous by design: the SPA renders it logged-out, +the Discord bot reads it with no credentials, and the Android `ShardStreamClient` consumes +`/public/shard/stream` without an `Authorization` header. Content visibility during maintenance comes +from the per-route **siteMode** middleware (§5), never from an auth gate. + | Method | Path | Notes | |---|---|---| | GET | `/settings` | whitelisted public keys, derived `registration`/`gameAccountSignup` flags, the per-shard **`brand`** block (name, `accent` color, logo/hero/favicon) a client themes itself from — one image runs as any shard, asset fields may be site-relative paths (resolve against the base URL) — and a **`push`** block `{ ntfyUrl }` (M7): the client-facing ntfy relay URL the app's embedded distributor registers its device topic against, from `NTFY_PUBLIC_URL` / first `NTFY_ALLOWED_ORIGINS` (never the internal `NTFY_BASE_URL`); `null` when push isn't configured for the shard. |