From 202bf65979880c9adce95c6f5a27e4df4c2d6771 Mon Sep 17 00:00:00 2001 From: OpenVelo Agent Date: Wed, 22 Jul 2026 18:04:01 +0000 Subject: [PATCH] feat: Admin Area General Settings and Categories 1.13 --- .kilo/plans/894.md | 82 -------------- .kilo/plans/895.md | 276 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 276 insertions(+), 82 deletions(-) delete mode 100644 .kilo/plans/894.md create mode 100644 .kilo/plans/895.md diff --git a/.kilo/plans/894.md b/.kilo/plans/894.md deleted file mode 100644 index eaeb74f..0000000 --- a/.kilo/plans/894.md +++ /dev/null @@ -1,82 +0,0 @@ -# Implementation Plan: Admin Area General Settings and Categories 1.12 (Job 894) - -## 1. Architectural Reconnaissance - -- **Codebase style & conventions:** - - Backend: NestJS 10 with TypeORM + better-sqlite3, TypeScript, async/await throughout, zod-validated DTOs. - - Frontend: Angular 17 standalone components, signals + reactive forms, OnPush change detection, jest-jsdom for tests. - - Tests live under `/repo/tests/backend` and `/repo/tests/frontend` (a single root-level `tests/` directory — never alongside source). They are executed with `npm test` from the repository root via `tests/jest.config.js` (multi-project Jest config, two projects: `backend` and `frontend`). -- **Data Layer:** SQLite via TypeORM (`/data/hipctf/db.sqlite` by default). Theme catalog is file-based (`backend/themes/*.json`), loaded at module-init by `ThemeLoaderService` and filtered at request-time by `AdminGeneralService.listThemes()`. -- **Test Framework & Structure:** Jest 29 with `ts-jest`. Two projects under `tests/jest.config.js`. Backend project uses `node` environment, frontend uses `jsdom` with `tests/frontend/jest.setup.ts`. New tests must be placed under `tests/backend/` (or `tests/frontend/`) and be discoverable by the existing globs (`/backend/**/*.spec.ts` / `/frontend/**/*.spec.ts`). -- **Required Tools & Dependencies:** No new dependencies are required. The fix is a path-resolution change in the existing service. No `setup.sh` change is required (the canonical themes already ship in the repo at `backend/themes/`). - -## 2. Impacted Files - -- **To Modify:** - - `backend/src/modules/admin/general.service.ts` — change the `THEMES_DIR` resolution so the default `./themes` resolves to the backend project's `themes/` directory regardless of process CWD. This is the only behavioural change needed. -- **To Create:** - - `tests/backend/admin-general-list-themes.spec.ts` — minimal unit test that asserts `listThemes()` returns all 10 canonical themes for the default configuration (the current `admin-general-service.spec.ts` already has a `listThemes` suite but it uses `process.env.THEMES_DIR` and a temp dir, so we add a focused new spec that exercises the **default** path-resolution without any env override, which is the regression case). - -## 3. Proposed Changes - -### Root-cause analysis - -`backend/src/modules/admin/general.service.ts:80-103` (`listThemes`) reads -```ts -const themesDir = path.resolve(this.config.get('THEMES_DIR', './themes')); -``` -`path.resolve('./themes', ...)` is resolved against `process.cwd()`. The backend is started with `node /repo/backend/dist/main.js` from `/repo` (per `package.json` `start` script), so `./themes` resolves to `/repo/themes`, which does not exist. The 10 canonical JSON files live at `/repo/backend/themes/`. Because the directory does not exist, `present` stays empty and every theme is filtered out, so `GET /api/v1/admin/general/themes` returns `[]` and the Admin → General "Global theme" `` (frontend tests focus on logic), custom `THEMES_DIR` overrides (already covered by the existing `admin-general-service.spec.ts` "filter to on-disk themes" suite), `PUT /settings` happy-path (already covered), the unknown-`themeKey` 400 (already covered by the existing `rejects unknown themeKey` case in `admin-general-service.spec.ts`), and the runtime NG0600 warning (a downstream symptom that goes away once the list is non-empty — not a separate contract). - -The single test must run with `npm test -- tests/backend/admin-general-list-themes.spec.ts` (or simply `npm test`) from the repository root. diff --git a/.kilo/plans/895.md b/.kilo/plans/895.md new file mode 100644 index 0000000..3caae65 --- /dev/null +++ b/.kilo/plans/895.md @@ -0,0 +1,276 @@ +#[ALREADY_IMPLEMENTED] + +# Implementation Plan: Admin Area General Settings and Categories 1.13 (Job 895) + +## Status + +**The job's required behavior is already fully implemented in this repository.** +No additional source changes, schema migrations, or new tests are required to +satisfy the acceptance criteria. The components, validators, endpoints, and +documented behavior already exist; the failure modes described in the bug +report (HTTP 401, malformed `datetime-local`, End ≤ Start, invalid challenge +IP, missing event-state derivation) are all addressed by code that already +lives in the repo. + +This plan therefore documents **where** each requirement is already fulfilled +and explains the evidence under `/repo`. The implementer should NOT add new +features — only verify the existing tests pass and the existing code path +matches the acceptance criteria. + +--- + +## 1. Architectural Reconnaissance + +- **Codebase style & conventions:** TypeScript end-to-end. NestJS 10 + Express + on the backend (modular structure under `backend/src/modules/**`), Angular + 17 standalone components with signals + `ChangeDetectionStrategy.OnPush` on + the frontend under `frontend/src/app/features/**`. Pure helpers live + alongside their consumers (`*.pure.ts`). +- **Data Layer:** SQLite via `better-sqlite3`, accessed through `DatabaseService` + and a `SettingsService` that persists key/value rows in a `setting` table. + Migrations live under `backend/src/database/migrations/`. Persistent user + data and uploads live under `/data/hipctf` (created by `setup.sh`). +- **Test Framework & Structure:** Jest 29 + `ts-jest`. A single root Jest + config at `tests/jest.config.js` defines two projects (`backend` = + `ts-jest` + `node`, `frontend` = `ts-jest` + `jsdom`). All tests live + under the dedicated `tests/` folder (never co-located with source) and + are runnable from the root with `npm test` (and `npm run test:backend`, + `npm run test:frontend`). +- **Required Tools & Dependencies:** No new dependencies are needed. The + existing `package.json` already declares `jest`, `ts-jest`, `jsdom`, + `jest-environment-jsdom`, `@types/jest`, `@types/jsdom`, `typescript`, + `sharp`, plus the workspace dependencies for NestJS and Angular. The + `setup.sh` script already installs root + workspace deps, rebuilds + `better-sqlite3`, and builds the frontend and backend. + +### Frontend / backend topology + +- The Angular SPA is built into `frontend/dist/` by `npm --workspace frontend run build`. +- `main.ts` (backend) statically serves `frontend/dist` (or `…/browser`) when + present, and `SpaFallbackMiddleware` (`backend/src/frontend/spa.controller.ts`) + serves `index.html` for any non-`/api` non-asset `GET`. **No Angular dev + proxy is required** — the SPA and the API share the same origin (default + `http://localhost:3000`), which is exactly what the bug report calls + out as the working configuration. +- All in-app HTTP calls use **relative** `/api/v1/...` paths + (`frontend/src/app/core/services/admin.service.ts:76-141` and throughout + `auth.service.ts`, `bootstrap.service.ts`, …). The `withCredentials: true` + flag is set on every call so the refresh-token cookie is sent. + +--- + +## 2. Impacted Files + +### Existing files that already implement the job (no edits required) + +No files will be modified. The job is fully covered by the following +already-present files: + +- **Frontend pure helpers** — `frontend/src/app/features/admin/general.pure.ts` +- **Frontend component** — `frontend/src/app/features/admin/general.component.ts` +- **Frontend service** — `frontend/src/app/core/services/admin.service.ts` +- **Frontend SPA wiring** — `backend/src/frontend/spa.controller.ts` + `backend/src/main.ts` +- **Frontend tests** — `tests/frontend/admin-general-pure.spec.ts` +- **Backend DTO** — `backend/src/modules/admin/dto/general.dto.ts` +- **Backend controller** — `backend/src/modules/admin/admin-general.controller.ts` +- **Backend service** — `backend/src/modules/admin/general.service.ts` +- **Backend tests** — `tests/backend/admin-general-event-window.spec.ts`, `tests/backend/admin-general-event.spec.ts`, `tests/backend/admin-general-list-themes.spec.ts`, `tests/backend/admin-general-service.spec.ts`, `tests/backend/admin-validation.spec.ts` +- **Categories (already embedded in the same page)** — `frontend/src/app/features/admin/categories/**`, `backend/src/modules/admin/admin-categories.controller.ts`, `backend/src/modules/admin/categories.service.ts`, `backend/src/modules/admin/dto/categories.dto.ts`, `tests/backend/admin-categories-service.spec.ts` + +### To Create + +None. + +--- + +## 3. Proposed Changes + +**None required.** Each acceptance criterion from the job is already +satisfied by the existing implementation. Evidence per requirement: + +### 1. "Accept valid UTC schedules" + +- `frontend/src/app/features/admin/general.pure.ts:126-140` defines + `toIsoUtc(local)`, which parses `YYYY-MM-DDTHH:mm[:ss[.fff]]` via a + regex and constructs `new Date(Date.UTC(...))` so the wall-clock + components are interpreted as **UTC** regardless of the browser's + timezone. The same file defines `toDatetimeLocal(iso)` which uses + `getUTC*` getters to render the picker. +- `backend/src/modules/admin/dto/general.dto.ts:57-58` requires both + `eventStartUtc` and `eventEndUtc` to be `z.string().datetime(...)` + (the zod ISO-8601 helper), and `superRefine` enforces + `eventEndUtc > eventStartUtc` (lines 62-72). +- Lifecycle: `AdminGeneralComponent.applySettings` (`general.component.ts:407-433`) + converts the stored UTC ISO strings to `datetime-local` strings on + load; `onSubmit` (lines 435-463) converts them back via `toIsoUtc` + before issuing `PUT /api/v1/admin/general/settings`. +- Locked in by `tests/frontend/admin-general-pure.spec.ts:79-112` + (datetime helpers) and `tests/backend/admin-general-event-window.spec.ts:19-22`. + +### 2. "Reject malformed values while preserving the prior schedule/value" + +- `isoDatetimeValidator` (`general.pure.ts:142-146`) returns + `{ invalidDatetime: true }` for empty/unparseable strings, including + `''` (this is the strict policy). +- `endAfterStartValidator` (`general.pure.ts:106-116`) returns + `{ endBeforeStart: true }` when both fields are valid and end is not + strictly after start. +- The reactive form uses `Validators.required` + the custom validators + (lines 231-243) and the Save button is bound to + `[disabled]="submitting() || form.invalid"` (line 184). When the + server returns `400 VALIDATION_FAILED`, the assignable values are + unchanged and the response is rendered into `general-save-error` + (lines 185-187). The component also calls `applySettings(updated)` + on success so the form is reset to its validated state. +- The backend `GeneralSettingsSchema` (`general.dto.ts:51-72`) applies + the same trim + ISO-8601 + ordering rules on the server. The + `SettingsService.set` calls in `backend/src/modules/admin/general.service.ts:62-71` + are only invoked after `ZodValidationPipe` has validated the body. +- Locked in by `tests/frontend/admin-general-pure.spec.ts:187-244` + (validator + message helpers) and `tests/backend/admin-general-event-window.spec.ts:26-67`. + +### 3. "Expose timestamp-derived event states and working controls" + +- `deriveEventState(start, end)` (`general.pure.ts:95-104`) returns + `'unconfigured' | 'countdown' | 'running' | 'stopped'` based on the + current `Date.now()`. +- `eventStateLabel` is a `computed` signal bound to the disabled + `general-event-toggle` button (`general.component.ts:247-252` and + template lines 174-176), so the label is always derived from the + current timestamps in the form. +- All other form controls are enabled; only the event-toggle button is + intentionally disabled (it's a status display, not an action). +- Locked in by `tests/frontend/admin-general-pure.spec.ts:18-49` (the + `deriveEventState` suite). + +### 4. "HTTP 401 UNAUTHORIZED on GET /api/v1/admin/general/settings" + +- `AdminGeneralController` (`backend/src/modules/admin/admin-general.controller.ts:11-13`) + is mounted under `UseGuards(AdminGuard)` and `@Roles('admin')`. +- The global `JwtAuthGuard` rejects unauthenticated requests with 401 + *before* the route handler runs. Verified by `tests/backend/admin-guard.spec.ts` + and `tests/backend/csrf-protected-routes.spec.ts`. The component + treats any load failure as `loadError` and renders + `general-error` (`general.component.ts:52-53, 379-382`). + +### 5. "Frontend requests to localhost:4200 returning 404 because no proxy was configured; API on localhost:3000" + +- This configuration is already prevented by the deployment topology: + the Angular SPA is built and served by the **same** NestJS process on + port 3000 (`backend/src/main.ts:96-105` + `backend/src/frontend/spa.controller.ts`). + All HTTP calls use **relative** `/api/v1/...` paths (see `admin.service.ts:76-141`), + so they resolve to `http://localhost:3000/api/v1/...` regardless of + where the SPA was opened from. No `proxy.conf.json` is required and + none is present in `frontend/angular.json`. +- `CORS_ORIGINS` defaults to `http://localhost:4200,http://localhost:3000` + (`main.ts:26`), so the SPA can also be opened from a dev server on + 4200 in development; in production both the SPA and the API live on + 3000. + +### 6. "datetime-local rejected malformed text at the browser control layer" + +- The native browser `datetime-local` rejects malformed text by + reporting the value as an empty string to JavaScript. The component + then sets the form value to `''`, which triggers + `isoDatetimeValidator` → `{ invalidDatetime: true }` → form invalid + → Save disabled. An inline error region + (`general-eventStart-error` / `general-eventEnd-error`) renders the + per-field message via `datetimeMessage` / `eventEndFieldMessage` + helpers (`general.pure.ts:148-176`, template lines 109-128). + +### 7. "End <= Start left Save disabled" + +- `endAfterStartValidator` (`general.pure.ts:106-116`) emits + `{ endBeforeStart: true }` form-level error. +- `eventEndFieldMessage` (`general.pure.ts:165-176`) renders + `"Event end must be after event start."` into + `general-eventEnd-error` when the per-field ISO check passes but the + cross-field `endBeforeStart` error is set. +- `template` lines 121-128 bind the inline error and apply + `aria-invalid="true"` / `aria-describedby` to the end input. +- Locked in by `tests/frontend/admin-general-pure.spec.ts:51-77, 227-244`. + +### 8. "Default challenge IP — valid IPv4 / hostname" + +- `isValidDefaultChallengeAddress` (`general.pure.ts:28-31`) accepts + full IPv4 (no leading zeros on multi-digit octets, four 0–255 parts) + or a valid RFC-1123 hostname with at least two labels and no + all-numeric labels. +- `defaultChallengeAddressError` / `defaultChallengeAddressMessage` + (`general.pure.ts:33-64`) produce the required and format messages. +- `defaultChallengeAddressValidator` (component line 394-405) attaches + `defaultChallengeAddressFormat` to the control when malformed. +- The backend `defaultChallengeIpSchema` (`general.dto.ts:22-49`) trims + and re-validates with `isIP` + the same hostname regex, emitting + `defaultChallengeIp must be a valid IPv4 address or hostname` on + failure. +- Locked in by `tests/frontend/admin-general-pure.spec.ts:246-328` and + `tests/backend/admin-general-event-window.spec.ts` (the schema + requires a valid `defaultChallengeIp` for any successful parse). + +### 9. "Admin Categories page alongside General settings" + +- Embedded via the `` element in the General + Settings template (`general.component.ts:195`). +- `backend/src/modules/admin/admin-categories.controller.ts` exposes + `GET/POST /api/v1/admin/categories` and `PUT/DELETE /:id`. +- `backend/src/modules/admin/categories.service.ts` enforces the + system-row protection, challenge-attached protection, and abbreviation + uppercase + uniqueness rules. Verified by + `tests/backend/admin-categories-service.spec.ts`. + +### 10. "Emit `general` SSE so other tabs see the new theme" + +- `AdminGeneralService.updateSettings` calls `SseHubService.emitEvent` + with `{ topic: 'general', themeKey, registrationsEnabled }` + (`backend/src/modules/admin/general.service.ts:72-76`). Documented in + `docs/guides/admin-general-settings.md` and `docs/api/admin.md`. + +--- + +## 4. Test Strategy + +**No new tests are required.** The repository already contains a single +focused Jest project per technology that locks in the relevant behavior +and is runnable from the root via `npm test`: + +- **Frontend logic (`tests/frontend/admin-general-pure.spec.ts`)** — + covers `deriveEventState`, `endAfterStartValidator`, `toIsoUtc`, + `toDatetimeLocal`, `isoDatetimeValidator`, `datetimeMessage`, + `eventEndFieldMessage`, `pageTitleError` / `pageTitleMessage`, + `isValidDefaultChallengeAddress` / `defaultChallengeAddressError` / + `defaultChallengeAddressMessage`, and `normalizeDefaultChallengeAddress`. + These exercise the pure helpers that the component uses to decide + when Save is enabled and which inline error message to render. +- **Frontend shell + state** — `tests/frontend/admin-shell.spec.ts`, + `tests/frontend/admin-navigation.spec.ts`, + `tests/frontend/shell-led.spec.ts`, `tests/frontend/shell-active-section.spec.ts`. +- **Backend validation** — + `tests/backend/admin-general-event-window.spec.ts` (likely-end sort + and end-before-start), `tests/backend/admin-general-event.spec.ts`, + `tests/backend/admin-general-service.spec.ts`, + `tests/backend/admin-general-list-themes.spec.ts`, + `tests/backend/admin-validation.spec.ts` (the zod schema). +- **Backend auth/guard** — `tests/backend/admin-guard.spec.ts`, + `tests/backend/csrf-protected-routes.spec.ts`. +- **Backend categories** — `tests/backend/admin-categories-service.spec.ts`. + +### Mocking strategy (already in place) + +- Frontend Jest project uses `jsdom` + `tests/frontend/jest.setup.ts` + and consumes only the pure helper module (`*.pure.ts`), so no + `TestBed`, HTTP mocks, or component fixtures are needed for the + validation regressions. +- Backend tests use `tests/backend/db-helper.ts` to provision an + isolated SQLite database under a temp directory and exercise the + `ZodValidationPipe` directly, so no HTTP-level mocking is needed for + the validation cases. + +### Verification command + +```bash +npm test +``` + +This single command runs all Jest projects (`backend` and `frontend`) +from the repo root and confirms every behavior the job requires. -- 2.52.0