AI Implementation feature(893): Admin Area General Settings and Categories 1.11 (#33)

This commit was merged in pull request #33.
This commit is contained in:
2026-07-22 16:46:01 +00:00
parent c0a01eb58e
commit 56555d272f
13 changed files with 566 additions and 104 deletions
-87
View File
@@ -1,87 +0,0 @@
# Implementation Plan: Admin Area General Settings and Categories 1.10 — Public Landing Modal Reactivity to `registrationsEnabled`
## 1. Architectural Reconnaissance
- **Codebase style & conventions:** TypeScript everywhere. Monorepo with `backend/` (NestJS + TypeORM + SQLite) and `frontend/` (Angular standalone components, signals, OnPush change detection). Login is a single `LandingComponent` with a `mode = 'login' | 'register' | 'closed'` modal. Tests live in `tests/{backend,frontend}/` and run via `npm test` (jest projects). Pure helpers live next to the component that uses them (e.g. `frontend/src/app/features/landing/login-modal.service.ts`, `frontend/src/app/features/admin/general.pure.ts`).
- **Data Layer:** SQLite via TypeORM. The `registrationsEnabled` flag is stored in the `setting` table under `SETTINGS_KEYS.REGISTRATIONS_ENABLED` (`'true'` / `'false'`). It is read by `SystemService.bootstrap()` to produce the public `BootstrapPayload` and by `AdminGeneralService.getSettings()` for the admin view. The `register` endpoint (`POST /api/v1/auth/register`) reads the same key to gate `REGISTRATIONS_DISABLED`.
- **Test Framework & Structure:** Jest (`tests/jest.config.js`) with two projects (`backend` / `frontend`). Backend tests use `supertest` + an in-memory DB (`process.env.DATABASE_PATH=':memory:'`). Frontend tests use `jsdom` and `ts-jest`. Tests are pure (no Angular TestBed); they import only the pure helper functions and the shared event-source contract. Run with `npm test` from the repo root.
- **Required Tools & Dependencies:** No new runtime dependencies. The fix reuses existing `SseHubService`, the public `event/stream` SSE endpoint, the existing `BootstrapService.refresh()` method, and the `landing-markdown` / `landing-modal` helpers. `setup.sh` already installs dev deps (`ts-jest`, `supertest`, `jsdom`, `typescript`) and rebuilds `better-sqlite3` — no edits needed.
## 2. Impacted Files
- **To Modify:**
- `backend/src/modules/admin/general.service.ts` — include `registrationsEnabled` in the emitted `general` SSE hub payload so listeners can decide whether to refresh bootstrap.
- `backend/src/modules/system/system.controller.ts` (`eventStream`) — flatten the `general` hub event into a `{ topic: 'general', themeKey, registrationsEnabled }` shape so the public SSE actually carries the flag instead of dropping it.
- `frontend/src/app/core/services/bootstrap-event.service.ts` (NEW) — listed under "To Create" below; the public-facing SSE listener that converts `{ topic: 'general' }` frames into `BootstrapService.refresh()` calls. If the implementer prefers to keep it inline, the wiring lives in `frontend/src/app/app.component.ts` and `frontend/src/app/features/landing/landing.component.ts`.
- `frontend/src/app/app.component.ts` — start the public-event listener once at app boot (after `bootstrap.load()`) so every route benefits from it.
- `frontend/src/app/features/landing/landing.component.ts` — on `ngOnInit`, additionally call `bootstrap.refresh()` so the modal opens with the latest server state (covers the user who lands on `/login` after the in-memory cache has been mutated by another tab/process).
- `docs/api/system.md` and `docs/guides/landing-page.md` — update the public-event flatten description and the "Reacting to admin changes" step so the tester flow matches the new flow.
- **To Create:**
- `frontend/src/app/core/services/bootstrap-event.service.ts` — pure, provided-in-root service that subscribes to `/api/v1/event/stream` (via the existing `EventSource`-style helper or `fetch` + `ReadableStream`), filters frames with `topic === 'general'`, and calls `bootstrap.refresh()` for each one. Exposes `start()` / `stop()` so the app can hook it to Angular's lifecycle.
- `tests/frontend/bootstrap-event.spec.ts` — pure unit tests covering: a `general` frame triggers `bootstrap.refresh`; a non-`general` frame does not; malformed JSON is ignored; the listener is idempotent under repeated server pushes.
- `tests/backend/admin-general-event.spec.ts` — backend unit test that asserts `AdminGeneralService.updateSettings()` emits `{ topic: 'general', themeKey, registrationsEnabled }` (so the public SSE listener can rely on the shape).
- `tests/backend/events-status-sse-bootstrap.spec.ts` — backend integration test that asserts `/api/v1/event/stream` forwards the `general` hub payload with the `topic` preserved (independent of the legacy event-status fields).
## 3. Proposed Changes
### 3.1 Backend — propagate the `general` SSE payload with the `registrationsEnabled` flag
1. **`backend/src/modules/admin/general.service.ts`** — change the `general` topic emission so it carries both the new `themeKey` and the new `registrationsEnabled` boolean, computed from the same payload the handler just persisted:
```ts
this.hub.emitEvent({
topic: 'general',
themeKey: payload.themeKey,
registrationsEnabled: payload.registrationsEnabled,
});
```
This keeps the existing shape (`topic: 'general'`) additive — non-breaking for any current consumer.
2. **`backend/src/modules/system/system.controller.ts`** — rewrite the `hub$` mapper inside `eventStream()` so it forwards the `general` topic's payload verbatim instead of projecting it onto the legacy event-status fields. The legacy `@Public() @Sse('event/stream')` consumer (the landing page status badge) must keep receiving its existing `status/countdownMs/startUtc/endUtc/serverNowUtc` shape, so the change is:
- Add a `try { … } catch {}` wrapper that detects payloads matching `{ topic: 'general' }` and forwards them as-is (with a `startWith` so the public stream still emits legacy frames on its 1-second tick).
- Keep the existing flattening for non-`general` hub events so the legacy contract is preserved for the scoreboard / event-status consumers.
- Include the new `general` payload in the legacy public stream's `MessageEvent` so the public tab picks it up.
3. **No new endpoint** — we reuse the existing public `GET /api/v1/event/stream` SSE channel and prove via the new test that it now carries the `general` topic.
### 3.2 Frontend — listen to the public SSE and refresh bootstrap on `general`
1. **Create `frontend/src/app/core/services/bootstrap-event.service.ts`** — a thin, root-provided service that:
- Opens `GET /api/v1/event/stream` once via `fetch` + `ReadableStream` (mirroring `frontend/src/app/core/services/authenticated-event-source.service.ts` but for a public endpoint, so no `Authorization` header).
- Parses `data: {…}` frames, JSON-parses each, and dispatches:
- If `payload.topic === 'general'`, call `BootstrapService.refresh()` and re-apply the theme (the `refresh()` already does both — see `BootstrapService.fetchAndApply` lines 73-83).
- Otherwise, ignore (the frame is the legacy event-status payload, which the existing shell already handles via the authenticated `/events/status` stream).
- Exposes `start()` and `stop()` methods; `start()` is idempotent (no-op if already running) and `stop()` aborts the in-flight fetch and clears listeners.
- Catches malformed JSON frames and surface errors silently (we never want a broken SSE to break the public landing page).
2. **Modify `frontend/src/app/app.component.ts`** — after `await this.bootstrap.load()` and `await this.auth.restoreSession()`, call `bootstrapEvent.start()` so the listener is active for every route, including `/login`. Stop the listener on `DestroyRef.onDestroy` (in practice it lives for the life of the SPA, but the explicit teardown avoids leaks in tests).
3. **Modify `frontend/src/app/features/landing/landing.component.ts`** — in `ngOnInit`, after `void this.landing.refresh()`, also `void this.bootstrap.refresh()`. This is the direct fix for the "reopening the UI modal still exposed no Register control" symptom: even if the cross-tab SSE was either silenced or dropped, the public landing page that the user opens right now fetches the latest bootstrap payload before rendering the modal. The `registrationsEnabled` computed will then re-evaluate and the `@if (registrationsEnabled())` block will render the "Register here" link.
4. **`AuthService.register()` and `LandingComponent.submitRegister()`** — no changes are needed since the existing API endpoint already returns `201` when registrations are enabled, and the existing `buildLoginFailureMessage` already maps `REGISTRATIONS_DISABLED` to the user-facing string. The whole fix is upstream of those.
### 3.3 Docs — update the contract
1. **`docs/api/system.md`** — replace the "`event/stream` public flatten" paragraph with the new contract: the public stream now forwards `general` topic payloads (with `topic`, `themeKey`, `registrationsEnabled`) interleaved with the legacy event-status frames, and the legacy frames are still emitted on the 1-second tick for the original consumer.
2. **`docs/guides/landing-page.md`** — add a "Refresh on admin change" subsection documenting that the public landing page calls `bootstrap.refresh()` on `ngOnInit` AND on any `general` SSE frame, so the modal's Register button is consistent with the API.
## 4. Test Strategy
- **Target Backend Test Files:**
- `tests/backend/admin-general-event.spec.ts` — pure service test using a fake `SseHubService` (a `Subject<any>` is captured) that asserts `AdminGeneralService.updateSettings()` emits `{ topic: 'general', themeKey: '...', registrationsEnabled: true }` after the admin persists the new value. Mirrors the existing `tests/backend/admin-general-service.spec.ts` "persists all keys and emits a settings event" test (lines 223-247).
- `tests/backend/events-status-sse-bootstrap.spec.ts` — full Nest app integration test (mirrors `tests/backend/events-status-sse.spec.ts`) that primes a first admin, upserts a `general` event via the public `SseHubService`, subscribes to `/api/v1/event/stream`, and asserts the SSE delivers a `data: {"topic":"general", ...}` frame with `registrationsEnabled:true` and `themeKey:classic`. Also asserts that the legacy event-status frame is still emitted on the 1-second tick so the contract is not broken.
- **Target Frontend Test Files:**
- `tests/frontend/bootstrap-event.spec.ts` — pure unit tests that:
1. Construct an in-memory EventSource substitute (a `fetch` mock returning a `ReadableStream` of SSE-shaped strings, mirroring the `authenticated-event-source.spec.ts` approach).
2. Drive a `general` frame (`data: {"topic":"general","themeKey":"classic","registrationsEnabled":true}\n\n`) and assert the injected `BootstrapService.refresh()` has been called.
3. Drive a non-`general` frame (`data: {"status":"running",...}\n\n`) and assert `refresh()` is NOT called.
4. Drive a malformed JSON frame and assert the listener does not throw and does not call `refresh()`.
5. Drive two consecutive `general` frames and assert `refresh()` is called twice (idempotent under repeated server pushes).
- `tests/landing-modal-registration.spec.ts` (extend `tests/frontend/landing-modal.spec.ts` if preferred) — adds a tiny pure helper `landingModalShowsRegister(registrationsEnabled: boolean)` that mirrors the `@if (registrationsEnabled())` predicate and asserts it returns `true` exactly when `registrationsEnabled` is `true`. This locks the contract that the modal's switch button is only visible when the bootstrap payload says it should be — i.e. the "modal exposes a Register control iff the API will accept a registration" invariant.
- **Mocking Strategy:**
- Backend tests: use the existing `tests/backend/db-helper.ts` to seed a single admin via `POST /api/v1/auth/register-first-admin`. The `SseHubService` is consumed directly (subscribe to `hub.event$()`) to assert the payload shape without going through the full app — fast and isolated. The full Nest integration test reuses the bootstrapping pattern from `tests/backend/events-status-sse.spec.ts`.
- Frontend tests: stub `fetch` with a `ReadableStream` that emits SSE-encoded frames (mirrors `tests/frontend/authenticated-event-source.spec.ts`'s `makeCaptureableFetch`). Inject a `BootstrapService` whose `refresh()` is a `jest.fn()` that resolves to a minimal `BootstrapPayload`. The Angular `DestroyRef` is faked with a no-op `onDestroy` callback so the test owns the listener lifecycle.
- No TestBed, no Karma, no visual assertions. All tests run via `npm test` from the repo root and finish in well under a second each.
- **Reusable test fixtures from `/data`:** the `/data/hipctf/db.sqlite` file is used by the running server, not by the tests. The test fixtures are created on-the-fly by `tests/backend/db-helper.ts` via `initDb(app)` so no persistent state is required. If the implementer prefers to seed from `/data`, the helper can be extended to copy the SQLite file only when `process.env.USE_DATA_DB === 'true'` (the plan does not require this).
+37
View File
@@ -0,0 +1,37 @@
# Implementation Plan: Admin Area General Settings and Categories 1.11
## 1. Architectural Reconnaissance
- **Codebase style & conventions:** Node.js npm-workspace monorepo with strict TypeScript. The backend is NestJS 10 with thin controllers and repository-backed services; the frontend is Angular 17 using standalone, OnPush components, signals, typed `HttpClient` calls, and embedded `AdminCategoriesComponent` rendering on both `/admin/categories` and `/admin/general`. Category API responses are mapped from entities into `CategoryView`, and list ordering is `LOWER(abbreviation)` plus abbreviation as a deterministic tie-breaker.
- **Data Layer:** TypeORM 0.3 over `better-sqlite3`, `synchronize: false` in the application, explicit migrations registered in `backend/src/database/database.module.ts`, and startup migration execution through `DatabaseInitService.init()` before listening. The persistent production database defaults to `/data/hipctf/db.sqlite`. The current entity and list service require `created_at`/`updated_at`, but legacy databases can have the earlier six-column `category` table. Although migration `1700000000200` attempts to repair timestamps and `1700000000300` defines the canonical CRY/MSC/PWN/REV/WEB/HW rows, the observed persistent database proves a new forward migration is required: already-recorded migrations must never be edited or relied on to rerun.
- **Test Framework & Structure:** Root Jest 29 multi-project configuration with `ts-jest`; backend tests live in `tests/backend/` and run via `npm test` (or focused `npm run test:backend`). Existing migration tests use an isolated in-memory SQLite database. Extend this dedicated test area with a focused legacy-schema migration regression rather than UI/visual tests; the Angular component and service wiring already render icon, abbreviation, description, edit, and delete actions from a successful response.
- **Required Tools & Dependencies:** No new system tools, global CLIs, npm packages, or `/data` fixtures are required. Existing Node/npm, TypeScript, TypeORM, `better-sqlite3`, NestJS testing, and Jest dependencies are sufficient; `setup.sh` should remain unchanged. Verification should use the root single-command suite (`npm test`) and existing build command (`npm run build`).
## 2. Impacted Files
- **To Modify:**
- `backend/src/database/database.module.ts` — register the new forward repair migration after `UpdateSystemCategoryKeys1700000000300` so existing `/data` databases receive it on restart.
- `tests/backend/migrations.spec.ts` — register the migration in the fresh-schema migration list and strengthen canonical system-category assertions to require exactly CRY, HW, MSC, PWN, REV, WEB once each with populated metadata.
- `tests/backend/database-init.spec.ts` — assert startup initialization yields the exact canonical set and remains duplicate-free across repeated initialization/startup behavior.
- `backend/src/config/env.schema.ts` — replace the obsolete crypto/forensics/pwn/web/misc/osint seed constants with the canonical system-key metadata, keeping seed definitions consistent with the repaired database model for fresh installs and future reuse.
- **To Create:**
- `backend/src/database/migrations/1700000000400-RepairCategorySchemaAndSystemCategories.ts` — forward-only, idempotent compatibility migration for legacy category schemas and stale system-category rows.
- `tests/backend/category-repair-migration.spec.ts` — focused in-memory regression starting from the exact legacy six-column schema and CRY/FOR/MSC/OSI/PWN/WEB data described by the Job.
## 3. Proposed Changes
1. **Database / Schema Migration:**
- Add a new migration with a timestamp/name greater than `1700000000300`; do not modify the semantics of migrations that may already be recorded in persistent databases.
- Inspect `PRAGMA table_info("category")`; add `created_at` and `updated_at` only when absent, then backfill blank/null values with an ISO UTC SQLite timestamp. This makes TypeORM `SELECT c.*` valid before any category rows are read through `CategoryEntity`.
- Reconcile system rows transactionally against one canonical definition ordered by the required sort key: CRY (Cryptography), HW (Hardware), MSC (Misc), PWN (Pwn), REV (Reverse Engineering), WEB (Web), each with non-empty description and icon path.
- Preserve user-created rows (`system_key IS NULL`). For system rows, remove obsolete FOR/OSI records, update existing canonical-key rows to canonical name/abbreviation/description/icon metadata, and insert only missing canonical keys. Handle abbreviation collisions deterministically: reuse an existing row with the canonical abbreviation only when it can safely become that system row; otherwise avoid destructive changes to unrelated user data and fail migration with a clear conflict rather than silently corrupting references.
- Ensure uniqueness indexes for non-null `system_key` and `abbreviation` exist after reconciliation. Make the SQL guarded/upsert-like so rerunning the migration logic in a test, or restarting after it has been recorded, leaves exactly one row per canonical system key and no duplicates.
- Keep rollback conservative: reverse only schema/index additions that can be safely removed, or explicitly make `down` non-destructive for canonical data so rollback cannot delete user/challenge relationships.
2. **Backend Logic & APIs:**
- Keep `AdminCategoriesService.list()` and `CategoryEntity` timestamp fields intact; once startup repair aligns the physical schema, TypeORM can select all declared columns and map the existing `CategoryView` without the `c.created_at` 500.
- Keep `GET /api/v1/admin/categories` controller/service contracts unchanged. Confirm the service returns repaired rows alphabetically by lowercased abbreviation with deterministic tie-breaking and exposes `iconPath`, `abbreviation`, `description`, `isSystem`, and timestamps.
- Align `SYSTEM_CATEGORY_KEYS`/`SYSTEM_CATEGORY_META` with CRY/HW/MSC/PWN/REV/WEB so a fresh database and any future seed consumer cannot recreate FOR/OSI. Use one canonical metadata representation where practical to prevent the seed and repair migration from drifting.
- Tighten startup verification to validate the canonical six system keys rather than only logging a total category count; retain user-created categories in the total and report missing/duplicate canonical rows explicitly.
3. **Frontend UI Integration:**
- No frontend code change is planned. `AdminGeneralComponent` already embeds `AdminCategoriesComponent`; `AdminService.listCategories()` already calls the correct endpoint; and the category template already renders icon, abbreviation, description, pencil, and trash controls for every returned row. Repairing the API restores both `/admin/general` and `/admin/categories` without introducing a competing Angular effect or touching the reported NG0600 symptom.
## 4. Test Strategy
- **Target Unit Test File:** `tests/backend/category-repair-migration.spec.ts`, plus minimal assertion updates in `tests/backend/migrations.spec.ts` and `tests/backend/database-init.spec.ts`.
- **Mocking Strategy:** Use a real isolated `better-sqlite3` in-memory `DataSource`/`QueryRunner`; do not mock TypeORM or use the persistent `/data` database. Construct the exact legacy `category` table with only `id`, `system_key`, `name`, `abbreviation`, `description`, and `icon_path`, seed CRY/FOR/MSC/OSI/PWN/WEB, run the new migration, and assert: timestamp columns exist and are populated; a TypeORM repository/list query no longer throws; the system abbreviations are exactly `[CRY, HW, MSC, PWN, REV, WEB]` in API sort order; each row has icon and description metadata; and invoking the repair twice leaves six unique system rows. Keep the migration suite's fresh-database assertion equally exact, and close every DataSource after the tests. No browser, visual confirmation, network service, or heavyweight frontend fixture is needed.