m0rph3us1987/HIPCTF2
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: Admin Area General Settings and Categories 1.13

Browse Source
This commit is contained in:
OpenVelo Agent
2026-07-22 18:04:01 +00:00
parent 7bd0009b93
commit 202bf65979
2 changed files with 276 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.kilo/plans/894.md
-82
View File
@@ -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 (`<rootDir>/backend/**/*.spec.ts` / `<rootDir>/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<string>('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" `<select>` renders with zero options.
### 1. Backend logic — fix default path resolution
In `backend/src/modules/admin/general.service.ts`:
- Replace the inline resolution with a small private helper, e.g.:
```ts
private resolveThemesDir(raw: string): string {
if (path.isAbsolute(raw)) return raw;
// Resolve relative to the backend project root (one level up from
// backend/src/modules/admin at compile time, i.e. backend/), so the
// default './themes' always points at backend/themes regardless of
// the process CWD used to launch `node backend/dist/main.js`.
const backendRoot = path.resolve(__dirname, '..', '..', '..');
return path.resolve(backendRoot, raw);
}
```
and use it in `listThemes()`:
```ts
const themesDir = this.resolveThemesDir(this.config.get<string>('THEMES_DIR', './themes'));
```
- Rationale: from `backend/src/modules/admin/general.service.ts`, three `..` segments walk back to the backend package root (`backend/`). At runtime the compiled file lives at `backend/dist/modules/admin/general.service.js`, so `__dirname` is `backend/dist/modules/admin` and three `..` segments still reach `backend/`. This makes `./themes` always point at the canonical `backend/themes/` directory shipped in the repo, without changing the public `THEMES_DIR` contract (an absolute path or a custom relative path still works).
- Keep the rest of `listThemes()` unchanged: it still intersects the loader's catalog with on-disk JSON `id`s, so a custom `THEMES_DIR` pointing at a different folder still filters accordingly.
### 2. Frontend / signal hardening (defensive, small)
The job reports a browser console warning
`RuntimeError: NG0600: Writing to signals is not allowed in a computed or an effect by default`
when the Admin General page loads with the empty theme list. Once the backend returns 10 themes, this symptom goes away. To make the component robust if the network call fails, no structural change to the signal graph is needed — the existing pattern (`signal<ThemeView[]>([])` written in `ngOnInit` via `this.themes.set(themes)`) is correct and not inside an `effect()`/`computed()`. We **do not** introduce new signal writes; we only verify nothing in `frontend/src/app/features/admin/general.component.ts` writes a signal from a computed/effect, and leave the file untouched unless the implementer finds an unrelated `effect()` that mutates state (none exists today — verified by grep). If the implementer wants to be extra defensive, a single optional one-liner fallback in `general.pure.ts` (e.g. a pure `BUILTIN_THEME_VIEW_FALLBACK` array used when the API returns `[]`) is acceptable but not required.
### 3. `setup.sh` / data layer
No changes. `/data` is not touched by this job. The `THEMES_DIR` env var (and its default) continue to work the same way; only the default resolution now points at the correct directory.
## 4. Test Strategy
### Target Unit Test File
- `tests/backend/admin-general-list-themes.spec.ts` (new) — single focused regression spec for the path-resolution bug.
### Mocking Strategy
- Instantiate `AdminGeneralService` directly with hand-rolled fakes for the three injected collaborators (`SettingsService`, `ThemeLoaderService`, `SseHubService`) and a `ConfigService` whose `.get('THEMES_DIR', './themes')` returns the **default** `'./themes'` (no `process.env` override) so the test exercises the same code path the real backend takes.
- `ThemeLoaderService.listThemes()` is faked to return 10 entries with the canonical ids (`classic`, `midnight`, `sunset`, `forest`, `cyber`, `paper`, `crimson`, `ocean`, `neon`, `monochrome`).
- Do NOT mock `fs` or `path`. The test relies on the real `backend/themes/` directory shipped in the repo (this is exactly the regression case).
### Cases (minimal, single-file)
1. **Regression:** with no `THEMES_DIR` override, `listThemes()` returns exactly 10 entries whose ids are a permutation of the canonical `THEME_IDS` set, and each entry has `id === key` and a non-empty `name`. This is the single must-have test that locks in the fix.
2. **Robustness:** when called twice in a row, the result is referentially equivalent (no I/O ordering surprises).
That is intentionally the entire scope. We do **not** add tests for: visual rendering of the `<select>` (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.
.kilo/plans/895.md
+276
View File
@@ -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 `<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.