HIPCTF2/.kilo/plans/895.md

#[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 `<app-admin-categories />` 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.