From d8f16c4f79ea1352d428d71dd4e800d1530aa00f Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 8 May 2026 15:52:02 +0100 Subject: [PATCH] docs(admin): design for admin OpenAPI coverage (#7693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the design doc for documenting `/admin-auth/*` and `/admin/update/status` in the OpenAPI spec so the typed client generated by #7695 (`admin/src/api/ schema.d.ts`) gains admin call-sites the day it lands. This PR is stacked on #7695 — codegen rails must merge first. Schema-only, no call-site migrations (those are an explicit follow-up named in #7693). Co-Authored-By: Claude Opus 4.7 (1M context) --- ...6-05-08-issue-7693-admin-openapi-design.md | 274 ++++++++++++++++++ 1 file changed, 274 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-08-issue-7693-admin-openapi-design.md diff --git a/docs/superpowers/specs/2026-05-08-issue-7693-admin-openapi-design.md b/docs/superpowers/specs/2026-05-08-issue-7693-admin-openapi-design.md new file mode 100644 index 000000000..984a418da --- /dev/null +++ b/docs/superpowers/specs/2026-05-08-issue-7693-admin-openapi-design.md @@ -0,0 +1,274 @@ +# Issue 7693 — Document admin endpoints in the OpenAPI spec + +**Status:** design approved 2026-05-08 +**Issue:** https://github.com/ether/etherpad/issues/7693 +**Stacks on:** PR #7695 (`chore/admin-typesafe-api-7638-upstream`) — codegen rails +**Related:** #7601 (introduced `/admin/update/status`); #7607 (Tier 2 update endpoints, in-flight) + +## Goal + +Add OpenAPI definitions for the admin endpoints currently consumed by the admin +UI so the typed client generated by PR #7695 (`admin/src/api/schema.d.ts`) +gains admin call-sites the day it lands. + +This PR adds the schema only. **No call-sites migrate** — that is the explicit +follow-up named in #7693. + +## Scope + +In: + +- `POST /admin-auth/` — login + session check (consumed by `LoginScreen.tsx` + and `App.tsx`). +- `GET /admin/update/status` — Tier 1 update banner data (consumed by + `UpdateBanner.tsx` and `UpdatePage.tsx`; introduced by #7601, merged on + develop). + +Out: + +- `/admin/update/{apply,cancel,acknowledge,log}` — Tier 2 endpoints from the + in-flight `feat/7607-auto-update-tier2-manual-click` branch. That PR amends + `openapi-admin.ts` when it lands. +- The admin SPA static-file route (`/admin/{*filename}`) — not an API. +- `/admin/socket.io/*` — websocket; out of OpenAPI scope. +- `/api/version-status` — already public, belongs in the public spec, not the + admin spec. +- Migrating any of the four admin `fetch()` call-sites to `$api`. + +## Architecture + +### File layout (new files marked NEW) + +``` +src/node/hooks/express/ +├── openapi.ts unchanged — APIHandler-driven public spec +└── openapi-admin.ts NEW — hand-authored OpenAPI 3.0 doc for admin routes + +src/tests/backend/specs/ +└── openapi-admin.ts NEW — Mocha specs asserting document shape + +admin/scripts/ +├── dump-spec.ts MODIFIED — also import generateAdminDefinition, +│ deep-merge into one document, write merged JSON +├── merge-openapi.mjs NEW — focused deep-merge with collision detection +├── __tests__/ +│ └── merge-openapi.test.mjs NEW — node --test unit specs for the merge +└── gen-api.mjs unchanged — still calls dump-spec.ts then + openapi-typescript on the resulting JSON +``` + +`openapi-admin.ts` is a **static OpenAPI document** (no APIHandler reflection). +Hand-authored because admin routes aren't registered through APIHandler — they +are plain Express handlers. This keeps `openapi.ts`'s 771-line generator +untouched and avoids tangling two different generation strategies in one +module. + +### Why merge in `dump-spec.ts` rather than at `openapi-typescript` time + +`openapi-typescript` only accepts one input. We could run it twice and emit +two `.d.ts` files, but the chosen design (Section 3, codegen) is a single +merged `schema.d.ts` so the admin UI's `$api` instance has one `paths` +interface covering both surfaces. The merge therefore happens at JSON-dump +time, before `openapi-typescript` runs. + +## OpenAPI document contents + +### Info & security schemes + +```yaml +openapi: 3.0.2 +info: + title: Etherpad Admin API + version: + description: | + Authenticated administrative endpoints consumed by the Etherpad admin UI. + Distinct from the public /api/{version}/* surface served by openapi.json. + +components: + securitySchemes: + basicAuth: + type: http + scheme: basic + sessionCookie: + type: apiKey + in: cookie + name: express_sid +``` + +`basicAuth` covers the login POST to `/admin-auth/`. `sessionCookie` covers +post-login admin sessions established by `express-session` (cookie name +`express_sid` is the Etherpad default; if a deployment overrides it the spec +remains structurally correct — only the documented cookie name shifts). + +The two schemes coexist on `/admin-auth/`; only `sessionCookie` applies on +`/admin/update/status`. + +### Paths + +#### `POST /admin-auth/` — `verifyAdminAccess` + +- **Security:** `[{ basicAuth: [] }, { sessionCookie: [] }, {}]` — Basic *or* + session cookie *or* none. The empty object documents that the server + accepts the request without auth and replies `401`. +- **Responses:** + - `200` — admin verified (Basic logged in, or session cookie was valid for + an admin user). Empty body. + - `401` — no auth presented and no session. Empty body. + - `403` — auth presented or session present, but the user is not an admin. + Empty body. +- **Description:** notes that POST with `Authorization: Basic …` establishes + an admin session; POST with no auth header verifies an existing one. + +This single-operation modeling matches reality: the route is one +middleware-terminated path that branches on what the client sends. Two +operations on the same path would imply different server behavior the +admin UI does not actually depend on. + +#### `GET /admin/update/status` — `getUpdateStatus` + +- **Security:** `[{ sessionCookie: [] }, {}]` — cookie when + `updates.requireAdminForStatus=true`, otherwise anonymous OK. The + conditional is documented in the description; clients that depend on + receiving the full diagnostic payload should send the session cookie. +- **Responses:** + - `200` — JSON body matching the `UpdateStatus` schema below. + - `401` / `403` — only emitted when `updates.requireAdminForStatus=true`. + +Response schema `UpdateStatus` mirrors the runtime shape returned by +`src/node/hooks/express/updateStatus.ts`: + +```yaml +UpdateStatus: + type: object + required: [currentVersion, installMethod, tier, vulnerableBelow, execution, lockHeld] + properties: + currentVersion: { type: string } + latest: { $ref: '#/components/schemas/UpdateLatest', nullable: true } + lastCheckAt: { type: string, format: date-time, nullable: true } + installMethod: { type: string, enum: [git, pnpm, docker, snap, unknown] } + tier: { type: string, enum: [off, '1', '2'] } + policy: { $ref: '#/components/schemas/UpdatePolicy', nullable: true } + vulnerableBelow: { type: string, nullable: true } + execution: { $ref: '#/components/schemas/UpdateExecution' } + lastResult: { $ref: '#/components/schemas/UpdateResult', nullable: true } + lockHeld: { type: boolean } +``` + +Sub-schemas (`UpdateLatest`, `UpdatePolicy`, `UpdateExecution`, +`UpdateResult`) are declared in `components.schemas` so the generated client +gets full nesting. Their fields mirror the source types in +`src/node/updater/state.ts` and `src/node/updater/UpdatePolicy.ts`. Where the +handler sanitizes the payload for non-admins (`sanitizeExecution`, +`sanitizeLastResult`), the schemas mark the redacted fields as optional so +both shapes validate. + +### Public exposure (runtime) + +`openapi-admin.ts` exports an `expressPreSession` hook that mounts: + +``` +GET /admin/openapi.json (CORS: *) +``` + +for parity with `/api/openapi.json` and `/rest/openapi.json`. The hook +registers the route in `expressPreSession`, which runs before +`expressCreateServer` (where `admin.ts` registers the SPA wildcard +`/admin/{*filename}`). The earlier registration ensures +`/admin/openapi.json` resolves before the wildcard catches it. + +Codegen does not depend on this route — `dump-spec.ts` calls +`generateAdminDefinition()` in-process. The route exists for downstream +tooling (Postman, swagger-ui, third-party clients). + +## Codegen merge + +`merge-openapi.mjs` exports one function: + +```js +mergeOpenAPI(publicDoc, adminDoc) -> mergedDoc +``` + +Rules: + +| Section | Rule | +| ------------------------------ | ----------------------------------------------------------------- | +| `paths` | Union by path key. Collision throws. | +| `components.schemas` | Union by name. Collision throws. | +| `components.parameters` | Union by name. Collision throws. | +| `components.responses` | Union by name. Collision throws. | +| `components.securitySchemes` | Union by name. Collision throws. | +| `security` (root) | Public spec's root `security` is preserved; admin paths declare their own per-operation security so admin requirements never apply to public paths. | +| `info`, `servers` | Public spec wins. | + +Throwing on collision is intentional: silent overwrite is a footgun, and the +backend test below catches collisions before merge runs in CI. + +## Tests + +### Backend — `src/tests/backend/specs/openapi-admin.ts` + +Mocha specs against `generateAdminDefinition()`. No live HTTP. + +- Document is valid OpenAPI 3.0 (smoke check via `openapi-schema-validation`, + already in `node_modules`). +- `paths['/admin-auth/'].post.operationId === 'verifyAdminAccess'` and + declares responses `200`, `401`, `403`. +- `paths['/admin/update/status'].get.operationId === 'getUpdateStatus'` and + references `#/components/schemas/UpdateStatus`. +- `components.securitySchemes` contains `basicAuth` and `sessionCookie`. +- `components.schemas.UpdateStatus.properties` contains every property name + emitted by `updateStatus.ts:res.json({...})`. Cross-checked by importing + the same handler and asserting key parity. This is the regression net for + spec/handler drift. +- Admin operationIds and admin path keys do not collide with the public + spec (cross-loaded via `generateDefinitionForVersion`). Cross-collision + is impossible today (admin paths start with `/admin`, public paths are + flat or `/createGroup`-style), but the test fails loudly if a future + rename breaks the assumption. + +### Codegen merge — `admin/scripts/__tests__/merge-openapi.test.mjs` + +Node `--test` runner (already used by #7695 for `client.test.ts`). + +- Two minimal docs merge into the expected union. +- Path collision throws. +- Schema-name collision throws. +- Public root `security` is preserved when admin doc declares no root + security. +- Per-operation security on admin paths survives the merge unchanged. + +### No frontend tests this PR + +No call-sites migrate, so there is nothing UI-observable to assert. +Migration PRs add Playwright coverage when they touch each fetch. + +## Risks & mitigations + +| Risk | Mitigation | +| ---------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | +| `UpdateStatus` schema drifts from `updateStatus.ts` over time | Backend spec cross-checks property names against the handler. Tier 2 PR amends both spec and handler in one change. | +| Tier 2 (#7607) rebase conflicts with the new `openapi-admin.ts` | This PR adds only `/admin/update/status`. Tier 2 appends new entries — no conflict on the existing one. | +| `merge-openapi.mjs` silently overwrites a duplicate | Throws on collision. Backend spec cross-checks against the public spec. | +| `/admin/openapi.json` collides with `/admin/{*filename}` SPA wildcard | `openapi-admin.ts` registers in `expressPreSession`; `admin.ts` registers in `expressCreateServer`. Earlier hook wins. Backend smoke test confirms 200 + JSON content-type. | +| #7695 changes shape before it merges, breaking our base | This PR is stacked on #7695's branch. Rebase when #7695 rebases. PR description documents the dependency. | +| `express_sid` is not the actual cookie name in some deployments | Documented; spec is structurally correct; deployments that override it can still consume a typed client. | + +## Rollout + +1. Branch `feat/7693-admin-openapi` from `chore/admin-typesafe-api-7638-upstream`. +2. Add `openapi-admin.ts`, `merge-openapi.mjs`; modify `dump-spec.ts`. +3. Add backend spec and merge unit tests. +4. Open PR #7693 as **draft**, base set to `chore/admin-typesafe-api-7638-upstream`. +5. When PR #7695 merges to develop, change base to `develop`, rebase, mark + ready for review. +6. Follow-up PR (separately tracked) migrates the four admin `fetch()` + sites: `LoginScreen.tsx`, `App.tsx`, `UpdateBanner.tsx`, `UpdatePage.tsx`. + +## Open question deferred to implementation + +The `express_sid` cookie name is the documented default but Etherpad +deployments can override it via settings. Implementation will read the +configured name at spec-generation time (or document the override path) so +the spec reflects the running configuration. If reading the configured name +is awkward at codegen time (it requires booting Settings), the spec keeps +the default and notes the override in the description.