refactor(api): collapse /admin/account and /player/account onto /auth/me/account
All checks were successful
PR Checks / bot-tests (pull_request) Successful in 26s
PR Checks / client-build (pull_request) Successful in 27s
PR Checks / server-tests (pull_request) Successful in 10m32s

Self-service account security had three URL surfaces onto one controller. All
three mounted the same `admin/account.controller.js` handlers; each of the three
router files carried a header comment apologising for the arrangement.

`/auth/me/account` was already a strict superset, which settles which to keep:

  /admin/account   6 routes  noindex, isLoggedIn, staffOnly
  /player/account  8 routes  noindex, requireAuth
  /auth/me/account 10 routes noindex, requireAuth

Neither of the deleted surfaces carried recovery codes, and /admin/account
carried no username or password change at all — so client.js already called
/auth/me/account/recovery-codes/* for two operations on a screen it otherwise
served from /admin/account. The split was leaking before this change.

Gating is equivalent where it overlapped: /player and /auth/me apply identical
`noindex, requireAuth`, and `staffOnly` on /admin/account was strictly narrower
while buying nothing, since every handler is self-scoped to req.user.id. There
is no CSRF layer to differ.

  - 14 routes deleted, 0 added, no handler changed.
  - account.controller.js moves router/v1/admin/ -> router/v1/auth/, beside the
    one router that still reaches it.
  - Web client: 14 call sites move onto a root-level api.myAccount /
    api.changeUsername / ... group, matching the /auth/me methods already there.
  - Android app: no change. MeApi.kt was already 100% /auth/me/account/*.
  - Two swagger tags, `Admin · Account` and `Player`, were declared only by the
    deleted routes and go with them. The orphaned `AccountStatus` schema goes
    too; `PlayerAccount` is re-described as the any-role /auth/me/account shape
    (the name is kept so existing $refs resolve).

Breaking to the published OpenAPI surface, accepted deliberately: both consumers
are in this org, and deprecate-then-delete would leave the next phase deciding
whether to add routes to surfaces already marked for removal.

Verification: routes.manifest.json shows exactly 14 deletions and 0 additions.
The OpenAPI spec loses the same 14 paths with zero surviving path definitions
changed; its large textual diff is pure reordering, because removing the
first-mounted router shifts every later path. 1203 server tests, 288 client
tests, 53 bot tests green; check:modules, check:hosts and routes:manifest
--check all pass.

Design of record: docs/website/ENGAGEMENT.md Phase 1a. This lands ahead of
engagement Phase 1b, which adds a self-service email field — written once here
rather than three times.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-08-29 00:49:25 -05:00
parent f5aa32e0ed
commit 6e61146678
20 changed files with 89 additions and 1428 deletions

View File

@@ -3,19 +3,19 @@
//
// This file owns exactly two things: the gate every player route shares, and the
// mount table. No route is declared here. Each capability router mounts at the
// prefix it already owned inside the old monolithic player.routes.js, so the
// emitted URL set is byte-identical — proved by a zero-line diff in
// server/routes.manifest.json (`npm run routes:manifest`).
// prefix it already owned inside the old monolithic player.routes.js.
//
// Self-service account security (`/player/account/*`) used to be mounted here. It
// is gone: `/auth/me/account/*` is the single canonical self surface for every
// role, and this group's copy was a strictly smaller duplicate of it.
//
// **Staff are a superset of players.** This group is open to any authenticated
// account, not just role 'player': every read/write is self-scoped to req.user.id,
// and a staff member has every player ability plus their staff tools on top.
// Adding a requireRole('player') here would 403 an admin off their own characters
// (it happened once — see docs/website/BACKEND_DESIGN.md). Staff also reach the
// identical self-scoped handlers under /admin/shard and /auth/me/account; those
// are alternative URLs onto the same controllers, not duplicated logic — and
// both of those live in module-uo now, which changes where they are defined and
// nothing about which URLs answer.
// identical self-scoped handlers under /admin/shard, which lives in module-uo now
// — that changes where they are defined and nothing about which URLs answer.
//
// See docs/website/API_V2_PLAN.md § Phase 2 for the split.
@@ -24,7 +24,6 @@ const express = require('express')
const { requireAuth } = require('../../../auth/session.middleware')
const noindex = require('../../../middleware/noindex')
const accountRouter = require('./account.router')
const appealsRouter = require('./appeals.router')
const teamsRouter = require('./teams.router')
const teamForumRouter = require('./teamForum.router')
@@ -39,7 +38,6 @@ const playerRouter = express.Router()
// silently ship without it.
playerRouter.use(noindex, requireAuth)
playerRouter.use('/account', accountRouter)
playerRouter.use('/appeals', appealsRouter)
playerRouter.use('/teams', teamsRouter)
// Same prefix, second router. The forum and the leader-exercised grant flow are a