mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-21 17:18:37 +00:00
docs(admin): design for admin OpenAPI coverage (#7693)
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) <noreply@anthropic.com>
This commit is contained in:
John McLear
parent
40fe8a4c23
commit
d8f16c4f79
1 changed files with 274 additions and 0 deletions
|
|
@ -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: <getEpVersion()>
|
||||
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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue