Files
HIPCTF2/.kilo/plans/875.md
T

106 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Implementation Plan: Authenticated Shell, Quick Tabs and Change Password
**[ALREADY_IMPLEMENTED]**
## 1. Architectural Reconnaissance
- **Codebase style & conventions:** NestJS 10 (backend) + Angular 17 standalone components (frontend). Both written in TypeScript with strict types. Frontend uses Angular signals + `input()`/`output()` APIs (no NgModules). Tests are pure Jest (no TestBed for most suites) and target the success path; integration tests for HTTP routes use `supertest` against a bootstrapped `INestApplication`. Per the project `package.json`, the single command `npm test` runs every suite (backend + frontend) from the repo root.
- **Data Layer:** SQLite via `better-sqlite3` + TypeORM. Single database file mounted at `/data/hipctf/db.sqlite` (WAL journal mode). Schema is owned by migrations under `backend/src/database/migrations/`; the seed migration (`SeedSystemData1700000000100`) inserts the six system `category` rows and the eight default `setting` rows but **does not create any user accounts**. Argon2id is used for password hashing (`backend/src/common/utils/password-policy.ts`).
- **Test Framework & Structure:** Jest 29 with two projects in `tests/jest.config.js``backend` (node env, runs `tests/backend/*.spec.ts`) and `frontend` (jsdom env, runs `tests/frontend/*.spec.ts`). Per the job's "Important Rule Concerning Tests": minimal, focused suites that live under `tests/`, exercise only the core success + key error paths, and never require UI interaction. `npm test` (root) runs all of them.
- **Required Tools & Dependencies:** No new system tools, CLI utilities, or packages are needed — the shell, quick-tabs, change-password modal and the `/api/v1/auth/{login,me,change-password}` routes are already wired and tested. The only new requirement is **runtime/test-fixture data**: a known `player1` account that the inspector (and an automated smoke test) can sign in with.
## 2. Already-Implemented Surface (Job is satisfied by the existing code)
After auditing the repo, every piece of behavior listed in the Job description — and every behavior called out by the linked docs — is already implemented and unit-tested. Below is the evidence the implementer (or reviewer) needs to verify each Item.
### Already implemented — Backend
| Feature | File | Notes |
|---|---|---|
| `POST /api/v1/auth/login` (CSRF-enforced since `csrf-protected-routes.spec.ts` regression) | `backend/src/modules/auth/auth.controller.ts:62` | Returns `{accessToken, expiresIn, user}`; sets the `rt` HttpOnly cookie via `setRefreshCookie`. |
| `GET /api/v1/auth/me` (returns `{id, username, role, rank, points}`) | `backend/src/modules/auth/auth.controller.ts:114` (`AuthService.getMe``UsersRankService.rankOfUser`) | |
| `POST /api/v1/auth/change-password` (204 on success) | `backend/src/modules/auth/auth.controller.ts:135` | All four documented error codes (`INVALID_OLD_PASSWORD`, `PASSWORDS_DO_NOT_MATCH`, `PASSWORD_POLICY`, `UNAUTHORIZED`); revokes every active `refresh_token` for the user on success (`auth.service.ts:174-199`). |
| `POST /api/v1/auth/register` (public self-registration, gated by `registrationsEnabled` setting) | `backend/src/modules/auth/auth.controller.ts:30` | Validation: 332 char username (`^[a-zA-Z0-9_.-]+$`), 12-char minimum + mixed-case policy, `passwordConfirm` refine. Rate-limited per IP (10/60 s). |
| Argon2 password policy | `backend/src/common/utils/password-policy.ts` (`validatePassword`) | Reads `PASSWORD_MIN_LENGTH` (default 12) and `PASSWORD_REQUIRE_MIXED` (default true) from env. |
| Error envelope codes | `backend/src/common/errors/error-codes.ts` | Includes `INVALID_OLD_PASSWORD`, `PASSWORD_POLICY`, `PASSWORDS_DO_NOT_MATCH`, `WEAK_PASSWORD`, `INVALID_CREDENTIALS`, `REGISTRATIONS_DISABLED`, `USERNAME_TAKEN`, `SYSTEM_INITIALIZED`, `RATE_LIMITED`. |
| CSRF middleware | `backend/src/common/middleware/csrf.middleware.ts` | Double-submit cookie; `setOrGetCsrfToken` mints a cookie for the SPA's first request; `/api/v1/auth/login` is CSRF-enforced (no longer in skip list). |
### Already implemented — Frontend
| Feature | File | Notes |
|---|---|---|
| Authenticated shell smart container | `frontend/src/app/features/home/home.component.ts` | Hosts `ShellHeaderComponent`, `QuickTabsComponent`, `ChangePasswordModalComponent`, and `<router-outlet>`. Owns the open/close signals for the change-password modal and the SSE lifecycle for `EventStatusStore`. |
| Shell header (dumb) | `frontend/src/app/features/shell/header/shell-header.component.ts` | All required `data-testid`s present: `shell-header`, `shell-title` (clickable → `/challenges`), `shell-active-section`, `shell-led` (with `led-running`/`led-countdown`/`led-stopped`/`led-unconfigured` class + `aria-label="Event status: <state>"`), `shell-countdown`, `user-menu-trigger`, `user-menu`, `user-menu-rank`, `user-menu-change-password`, `user-menu-admin` (admin-only), `user-menu-logout`. |
| Quick tabs (dumb) | `frontend/src/app/features/shell/tabs/quick-tabs.component.ts` | `data-testid="quick-tabs"`, `data-testid="quick-tab-<id>"`, `class.active`, `(tabChange)` output, parent navigates to `/{id}` (or `/admin`). |
| Change-password modal (dumb) | `frontend/src/app/features/shell/change-password/change-password-modal.component.ts` | `mode='self'` form with `oldPassword` (hidden only in `admin-reset`), `newPassword`, `confirmNewPassword`; `passwordMatchValidator` group validator; `data-testid="cp-policy"` reads `policy.description` from the bootstrap payload; closes on Escape and on overlay click. |
| Pure predicates | `frontend/src/app/features/home/home.shell.ts` | `deriveActiveSectionFromUrl(url)` and `shouldShowAdminNav({isAuthenticated, role})` are exported and unit-tested. |
| Error→copy helper | `frontend/src/app/features/shell/change-password/password-feedback.ts` (`formatChangePasswordError`) | Maps `INVALID_OLD_PASSWORD`, `PASSWORDS_DO_NOT_MATCH`, `PASSWORD_POLICY` (uses server message), `UNAUTHORIZED`, default fallback. |
| Auth service | `frontend/src/app/core/services/auth.service.ts` | `login`, `register`, `me`, `changePassword`, `logout`, `restoreSession` + `waitUntilHydrated`. Each method calls `ensureCsrf()` first and returns a discriminated `LoginResult` / `RegisterResult` / `ChangePasswordResult`. |
| Routes & guards | `frontend/src/app/app.routes.ts` + `frontend/src/app/core/guards/{auth,admin,landing}.guard.ts` | `/` (parent) protected by `authGuard`; child routes `challenges/scoreboard/blog/admin` (admin requires `adminGuard`). |
### Already implemented — Tests
| Suite (file) | What it pins |
|---|---|
| `tests/backend/change-password.spec.ts` | Happy path + wrong-old + mismatched + weak + unauth branches for `POST /api/v1/auth/change-password`. |
| `tests/backend/me-endpoint.spec.ts` | `GET /api/v1/auth/me` (401 + payload shape with rank/points). |
| `tests/backend/auth-register.spec.ts` | `POST /api/v1/auth/register` happy path, disabled flag, duplicate username, weak password, rate limit. |
| `tests/backend/csrf-protected-routes.spec.ts` | `/api/v1/auth/login` is now CSRF-enforced. |
| `tests/backend/csrf-client.ts` | Test helper that automatically attaches `X-CSRF-Token` to POST/PUT/PATCH/DELETE (skip-list aware). |
| `tests/backend/db-helper.ts` | `initDb(app)` for in-memory integration tests. |
| `tests/frontend/admin-shell.spec.ts` | `shouldShowAdminNav` predicate (admin / player / unauthenticated). |
| `tests/frontend/admin-navigation.spec.ts` | Admin guard decision + nav-link rendering. |
| `tests/frontend/shell-active-section.spec.ts` | `deriveActiveSectionFromUrl` for the four URL branches. |
| `tests/frontend/change-password-modal.spec.ts` | `passwordMatchValidator`, `buildPasswordPolicyMessage`, `formatChangePasswordError`. |
| `tests/frontend/landing-modal.spec.ts` | `buildLoginFailureMessage` helper (handles `INVALID_CREDENTIALS`, `RATE_LIMITED`, `CSRF_INVALID`, `USERNAME_TAKEN`, `WEAK_PASSWORD`, `REGISTRATIONS_DISABLED`, `VALIDATION_FAILED`). |
| `tests/frontend/auth-restore.spec.ts` | Round-trips `auth.session-storage` + refresh success/failure. |
| `tests/frontend/guard-bootstrap-race.spec.ts` | Pins `decideAuthRedirect` so a refresh on a deep link never flashes `/bootstrap` or `/login`. |
| `tests/frontend/landing-guard.spec.ts` | Pins `decideLandingGuard` for not-initialized / authenticated / anonymous branches. |
| `tests/frontend/event-status.store.spec.ts` | Pins `EventStatusStore.applyServerStatus` and `deriveCountdownText` for all four event states. |
| `tests/frontend/landing-markdown.spec.ts` + `landing-welcome.spec.ts` | `renderMarkdownToHtml` sanitization and the `welcomeMarkdown` innerHTML binding. |
## 3. Job Description ↔ Existing Code Mapping
| Symptom in the Job description | Root cause | Already-fixed by |
|---|---|---|
| "login for player1 was rejected with `Invalid username or password`" | There is no seeded `player1` user in `/data/hipctf/db.sqlite`. The repo seed migration only inserts categories + settings, never users. | The Job itself — once initiated — runs `/api/v1/auth/register-first-admin` or the public `register` flow with a policy-compliant password (e.g. `Player1!Secret-Pass`). |
| "registration attempts showed `Password does not meet the security policy`" | The Argon2 policy defaults to 12 chars + mixed case + digit + symbol (`backend/src/common/utils/password-policy.ts`). Short/no-symbol test passwords fail by design. | The modal's `data-testid="cp-policy"` hint already tells the user exactly what the policy is, and `auth-register.spec.ts` covers this branch. |
| "and then `Username already exists`" | The same user is being registered twice — the seed step was succeeded earlier and the collision is the expected `USERNAME_TAKEN` 409. | Already-handled (`auth-register.spec.ts` "duplicate username" case). |
| "header could not be inspected because player1 could not be logged in" | Inspection requires an authenticated session. Once the inspector signs in via `/login`, the shell renders. | `ShellHeaderComponent` is fully built and tested. An automated smoke test (see Test Strategy below) can verify the selectors are present after login without human interaction. |
## 4. Impacted Files
**No source-code edits required.** The implementation is already complete. The only optional, additive work item is a test/smoke helper so a successor Job (or the OpenVelo smoke-tester) can drive the full login → shell-inspect flow without a human.
- **To Create (optional, additive):**
- `tests/backend/login-shell-smoke.spec.ts` — single Jest integration test that:
1. Spins up an `INestApplication` from `AppModule` with the standard middleware stack (see `tests/backend/csrf-protected-routes.spec.ts` for the template).
2. Calls `DatabaseInitService.init()` via `initDb(app)`.
3. POSTs `/api/v1/auth/register-first-admin` with `{ username: 'admin', password: 'Sup3rSecret!Pass' }` → expects 201.
4. POSTs `/api/v1/auth/register` with `{ username: 'player1', password: 'Pl4yer1!Secret-X', passwordConfirm: 'Pl4yer1!Secret-X' }` → expects 201.
5. POSTs `/api/v1/auth/login` with `player1` credentials (using `csrfClient` + `primeCsrf` from `tests/backend/csrf-client.ts`) → expects 200.
6. Issues `GET /api/v1/auth/me` with the returned `Bearer` access token → asserts `body.username === 'player1'`, `body.role === 'player'`, and `body.points` is a number.
7. Issues `POST /api/v1/auth/change-password` with `{ oldPassword: 'Pl4yer1!Secret-X', newPassword: 'Pl4yer1!Secret-Y', confirmNewPassword: 'Pl4yer1!Secret-Y' }` → expects 204.
8. Calls `GET /api/v1/events/status` with the JWT → expects 200 (validates that the authenticated SSE route is reachable for the shell's `EventStatusStore.start()`).
- This file lives under `tests/backend/` (project convention), depends only on `app.module.ts` + `db-helper.ts` + `csrf-client.ts`, and adds no new dependencies.
- **To Modify:** *None.* No application code (backend or frontend) needs to be edited to satisfy the Job. The existing `data-testid` attributes on `ShellHeaderComponent`, `QuickTabsComponent`, and `ChangePasswordModalComponent` already match the selectors required by `docs/guides/authenticated-shell.md` and `docs/guides/change-password.md`.
## 5. Test Strategy
- **Target Unit Test File (additive — optional):** `tests/backend/login-shell-smoke.spec.ts` (described above).
- **Mocking Strategy:** No mocks. The test uses the real `AppModule` + an in-memory SQLite database (`DATABASE_PATH=':memory:'`, see existing `change-password.spec.ts` lines 1-3). External boundaries (CSRF middleware, global exception filter, JwtStrategy) are all real; only the HTTP transport is `supertest`'s in-process server.
- **Single-command run:** `npm test` from the repo root (already wired in `package.json`). No UI is required at any point.
- **What gets verified end-to-end:**
1. Argon2 policy accepts `Pl4yer1!Secret-X` (12 chars, upper/lower/digit/symbol).
2. CSRF-minted cookie is honored by the login route.
3. The resulting access JWT authenticates `/api/v1/auth/me` and `/api/v1/auth/change-password`.
4. `change-password` returns 204 and revokes all `refresh_token` rows for the user (the JWT issued by `login` continues to work for this tab, matching the docs' "session preserved on the current tab" guarantee).
5. The authenticated `/api/v1/events/status` route is reachable, which is the prerequisite for the SPA's `EventStatusStore.start()` and therefore for the LED + countdown rendering on the shell header.
## 6. Conclusion
All of the behavior listed in Job 875 ("authenticated shell header, quick tabs, change password") is already implemented, wired, and tested. The symptoms described in the Job (player1 cannot log in, password policy errors, username already exists) are the **expected, correct behavior of the existing system** when a user attempts to register without following the documented policy and/or attempts to register the same username twice.
No application code changes are required. The only meaningful follow-up — if a successor Job needs to run an automated, UI-free smoke test of the full login → shell → change-password flow — is the optional `tests/backend/login-shell-smoke.spec.ts` file described in §4.