AI Implementation feature(848): First-Start Create Admin Modal 1.07 #10
@@ -1,107 +0,0 @@
|
||||
# Implementation Plan: First-Start Create Admin Modal 1.06 — Bootstrap Race Fix
|
||||
|
||||
## 0. Status
|
||||
|
||||
**[ALREADY_IMPLEMENTED]**
|
||||
|
||||
The race described in Job 847 is already fixed in the current codebase. Two
|
||||
near-simultaneous `POST /api/v1/setup/create-admin` requests can no longer
|
||||
both succeed: exactly one returns HTTP 201 and the other returns HTTP 409
|
||||
`SYSTEM_INITIALIZED`. No new code or test work is required.
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
|
||||
- **Codebase style & conventions:** TypeScript, NestJS 10+ (modular, DI,
|
||||
controllers + services), class-based services, TypeORM with the
|
||||
`DataSource.transaction` API, `zod` DTOs guarded by
|
||||
`ZodValidationPipe`, a project-wide `ApiError` / `ERROR_CODES` enum,
|
||||
and Argon2id password hashing.
|
||||
- **Data Layer:** SQLite (in-memory for tests, file-based for runtime),
|
||||
TypeORM entities; the bootstrap flow wraps all admin creation inside
|
||||
a single `DataSource.transaction` so the `adminCount > 0` guard and
|
||||
the user insert commit atomically.
|
||||
- **Test Framework & Structure:** Jest. Tests live exclusively under
|
||||
`/repo/tests/` (separated into `tests/backend/` and `tests/frontend/`),
|
||||
never alongside source. Backed by a `csrfClient` helper in
|
||||
`tests/backend/csrf-client.ts` and an `initDb(app)` helper that uses
|
||||
SQL migrations to provision the in-memory database. Tests are
|
||||
runnable from the repo root via the standard `npm test` pipeline.
|
||||
- **Required Tools & Dependencies:** None beyond the existing stack
|
||||
(NestJS, TypeORM, better-sqlite3, argon2, zod, Jest, Supertest,
|
||||
uuid). No new package, system tool, or `setup.sh` change is needed.
|
||||
|
||||
## 2. Impacted Files
|
||||
|
||||
The implementation already exists in these files; no further edits are
|
||||
required:
|
||||
|
||||
- **To Modify:** None.
|
||||
- **To Create:** None.
|
||||
- **Existing files that already contain the fix (read-only references):**
|
||||
- `backend/src/modules/setup/setup.service.ts:23,39-97,111-130` —
|
||||
`SetupService.createAdmin` runs every bootstrap attempt through
|
||||
`runBootstrap()`, which serializes callers on a per-process promise
|
||||
chain (`#bootstrapChain`) so two concurrent transactions cannot
|
||||
both observe an empty `user` table.
|
||||
- `backend/src/modules/setup/setup.service.ts:60-95` — Each
|
||||
serialized call executes inside `dataSource.transaction(...)`,
|
||||
performs the `adminCount > 0` check first, throws
|
||||
`ApiError(SYSTEM_INITIALIZED, 409)` on the loser, then performs the
|
||||
`username` uniqueness check, Argon2id hash, and `UserEntity`
|
||||
insert on the winner.
|
||||
- `backend/src/modules/setup/setup.module.ts` — Wires
|
||||
`RegistrationRateLimitService`, `JwtService`, `ConfigService`,
|
||||
`DataSource`, and the user/refresh-token repositories into
|
||||
`SetupService`.
|
||||
- `backend/src/common/errors/api-error.ts`,
|
||||
`backend/src/common/errors/error-codes.ts` — Provide `SYSTEM_INITIALIZED`
|
||||
and the 409 mapping consumed by `GlobalExceptionFilter`.
|
||||
- `tests/backend/setup-create-admin.spec.ts:121-149` — Existing
|
||||
regression test `"only one of two concurrent first-admin requests
|
||||
succeeds; the other receives a controlled conflict"` already
|
||||
reproduces the race described in the Job and asserts
|
||||
`[201, 409]`, the loser's `SYSTEM_INITIALIZED` code, exactly one
|
||||
admin row, and exactly one `refresh_token` row.
|
||||
|
||||
## 3. Proposed Changes
|
||||
|
||||
No changes. The Job's expected behaviour is already enforced by two
|
||||
cooperating layers:
|
||||
|
||||
1. **Database / Schema Migration:** None. The existing `user.role`
|
||||
index plus the in-transaction `adminCount` check are sufficient.
|
||||
2. **Backend Logic & APIs:** `SetupService.createAdmin` already wraps
|
||||
the whole bootstrap pipeline in `runBootstrap()` which serializes
|
||||
callers on a single process-local promise chain, and each task
|
||||
itself runs inside a `DataSource.transaction` whose first action is
|
||||
the `adminCount > 0` guard. The controller path
|
||||
`POST /api/v1/setup/create-admin` already maps the
|
||||
`SYSTEM_INITIALIZED` `ApiError` to HTTP 409 via
|
||||
`GlobalExceptionFilter`.
|
||||
3. **Frontend UI Integration:** No change needed. The frontend service
|
||||
`frontend/src/app/features/setup/setup-create-admin.service.ts`
|
||||
already switches on `CreateAdminResult` and surfaces 409 as the
|
||||
"system already initialized" inline error with a Retry affordance.
|
||||
|
||||
## 4. Test Strategy
|
||||
|
||||
The required regression test already exists and is the test that proves
|
||||
the fix. No additional tests are planned because:
|
||||
|
||||
- The failing scenario in the Job (two concurrent `adminAlpha` /
|
||||
`adminBeta` requests both returning 201 with admin rows + two
|
||||
`refresh_token` rows) is exactly the scenario covered by
|
||||
`tests/backend/setup-create-admin.spec.ts:121-149`.
|
||||
- Adding a second concurrent-race test would duplicate coverage without
|
||||
exercising new behavior, violating the "minimal, highly-focused tests"
|
||||
constraint.
|
||||
|
||||
If a future run of `npm test` ever fails this spec, the implementer
|
||||
should investigate `SetupService.runBootstrap` and the transactional
|
||||
`adminCount` guard (not add tests).
|
||||
|
||||
### Verification commands (for the implementer only — not for execution here)
|
||||
|
||||
- `npm test -- tests/backend/setup-create-admin.spec.ts` — confirms
|
||||
201/409 outcome, single admin row, single `refresh_token` row.
|
||||
- `npm test` — full Jest run from the repo root.
|
||||
@@ -0,0 +1,148 @@
|
||||
# Implementation Plan: Job 848 — First-Start Create Admin Modal 1.07
|
||||
|
||||
## Status
|
||||
|
||||
**[ALREADY_IMPLEMENTED]**
|
||||
|
||||
The First-Start Create Admin Modal feature is fully implemented end-to-end in
|
||||
this repository. The user-facing symptom reported in the job ("HIPCTF heading +
|
||||
'Frontend not built.' paragraph, no bootstrap dialog, no form controls, no
|
||||
submit button") is produced by the SPA fallback in
|
||||
`backend/src/frontend/spa.controller.ts:27` whenever
|
||||
`frontend/dist/browser/index.html` does not exist on disk at the moment
|
||||
the request is served.
|
||||
|
||||
That `dist/browser/index.html` is now present (rebuilt during the current
|
||||
session), so navigating to `http://localhost:3000/` will serve the Angular
|
||||
SPA, which redirects to `/bootstrap` and renders the create-admin modal.
|
||||
|
||||
No source code changes, no schema changes, and no new tests are required.
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
|
||||
- **Codebase style & conventions:**
|
||||
- Monorepo with npm workspaces (`backend/`, `frontend/`).
|
||||
- Backend: NestJS controllers + services, TypeORM repositories,
|
||||
`@nestjs/config`, argon2 for password hashing, class-validator /
|
||||
`ZodValidationPipe` for input, `ApiError` + `GlobalExceptionFilter`
|
||||
for the uniform `{ code, message }` error envelope.
|
||||
- Frontend: Angular 17+ standalone components, `ChangeDetectionStrategy.OnPush`,
|
||||
Angular Signals, reactive forms (`ReactiveFormsModule`), HTTP via
|
||||
`HttpClient` with `withCredentials: true`, separate
|
||||
`BootstrapService` (initialised flag) and `AuthService` (session token).
|
||||
- Persistence: SQLite via TypeORM, file path comes from
|
||||
`process.env.DATABASE_PATH` (default `/data/hipctf/hipctf.db`),
|
||||
`FRONTEND_DIST` env var names the built SPA directory.
|
||||
- **Data Layer:** SQLite (TypeORM). No schema change is needed.
|
||||
- **Test Framework & Structure:**
|
||||
- Jest with two projects (`backend`, `frontend`) configured in
|
||||
`tests/jest.config.js`.
|
||||
- Backend tests use `ts-jest` against an in-memory SQLite DB
|
||||
(`process.env.DATABASE_PATH = ':memory:'`) via
|
||||
`tests/backend/db-helper.ts`.
|
||||
- Frontend tests use `jest-environment-jsdom` with
|
||||
`tests/frontend/jest.setup.ts` providing Angular `TestBed` +
|
||||
`provideHttpClientTesting`.
|
||||
- All tests live under `tests/` and run via a single root command:
|
||||
`npm test` (alias for `jest --config tests/jest.config.js`).
|
||||
- **Required Tools & Dependencies:** No new tools. Existing `npm run build`
|
||||
in `setup.sh` rebuilds `frontend/dist/browser/index.html`, which is the
|
||||
actual fix for the symptom. No new packages are required.
|
||||
|
||||
## 2. Impacted Files
|
||||
|
||||
- **To Modify:** None.
|
||||
- **To Create:** None.
|
||||
|
||||
### Where the feature already lives (for reference / verification)
|
||||
|
||||
Backend:
|
||||
- `backend/src/modules/setup/setup.module.ts`
|
||||
- `backend/src/modules/setup/setup.controller.ts` — `POST /api/v1/setup/create-admin`.
|
||||
- `backend/src/modules/setup/setup.service.ts:39-97` — `createAdmin()` inside a
|
||||
TypeORM transaction with serialized bootstrap via `runBootstrap()`
|
||||
(lines 99-130) — guarantees that two concurrent callers cannot both
|
||||
observe an empty admin table; the loser gets
|
||||
`409 SYSTEM_INITIALIZED`.
|
||||
- `backend/src/modules/setup/dto/create-admin.dto.ts`
|
||||
- `backend/src/common/errors/error-codes.ts` — `SYSTEM_INITIALIZED`,
|
||||
`USERNAME_TAKEN`, `WEAK_PASSWORD`, `VALIDATION_FAILED`, `RATE_LIMITED`.
|
||||
- `backend/src/frontend/spa.controller.ts:11-28` — SPA fallback that emits
|
||||
the placeholder HTML **only** when no built `index.html` is found.
|
||||
|
||||
Frontend:
|
||||
- `frontend/src/app/features/setup/setup-create-admin.component.ts` — the
|
||||
modal component (`data-testid="setup-submit"`, `setup-retry`, etc.).
|
||||
- `frontend/src/app/features/setup/setup-create-admin.component.html`
|
||||
- `frontend/src/app/features/setup/setup-create-admin.component.css`
|
||||
- `frontend/src/app/features/setup/setup-create-admin.service.ts` —
|
||||
`POST /api/v1/setup/create-admin` with CSRF preflight + error mapping.
|
||||
- `frontend/src/app/features/setup/setup-create-admin.validators.ts` —
|
||||
`passwordMatchValidator`.
|
||||
- `frontend/src/app/features/setup/setup-create-admin.field-errors.ts` —
|
||||
pure helpers `usernameError`, `passwordError`, `confirmError`.
|
||||
- `frontend/src/app/core/services/bootstrap.service.ts` — exposes
|
||||
`initialized()`, `markInitialized()`, and `passwordPolicy()`.
|
||||
- `frontend/src/app/core/services/auth.service.ts` — `setSession()` used
|
||||
on success.
|
||||
- `frontend/src/app/app.routes.ts` — `/bootstrap` route, guarded by
|
||||
`authGuard` which redirects to `/bootstrap` while `initialized() === false`.
|
||||
|
||||
Existing tests that already cover this Job:
|
||||
- `tests/backend/setup-create-admin.spec.ts`
|
||||
- `tests/backend/bootstrap.integration.spec.ts`
|
||||
- `tests/backend/registration-rate-limit.spec.ts`
|
||||
- `tests/backend/api-error.spec.ts`
|
||||
- `tests/frontend/setup-create-admin.spec.ts`
|
||||
- `tests/frontend/guard-bootstrap-race.spec.ts`
|
||||
- `tests/frontend/auth-restore.spec.ts`
|
||||
|
||||
## 3. Proposed Changes
|
||||
|
||||
No code changes. The only operational fix is to keep
|
||||
`frontend/dist/browser/index.html` in place.
|
||||
|
||||
If a fresh container runs without it, `setup.sh` already invokes
|
||||
`npm --workspace frontend run build` (line 11) which produces the Angular
|
||||
bundle under `frontend/dist/browser/index.html`. The
|
||||
`SpaFallbackMiddleware` looks for the file in both
|
||||
`frontend/dist/index.html` and `frontend/dist/browser/index.html`, so the
|
||||
standard build output is resolved without any extra wiring.
|
||||
|
||||
## 4. Test Strategy
|
||||
|
||||
No new tests. The relevant behaviour is already covered:
|
||||
|
||||
- **Backend:** `tests/backend/setup-create-admin.spec.ts` exercises
|
||||
`POST /api/v1/setup/create-admin` happy path + each documented error
|
||||
(`SYSTEM_INITIALIZED`, `USERNAME_TAKEN`, `WEAK_PASSWORD`,
|
||||
`VALIDATION_FAILED`, `RATE_LIMITED`).
|
||||
- **Backend concurrency:** the `runBootstrap()` chain in
|
||||
`SetupService` is what serializes concurrent first-admin requests;
|
||||
`tests/backend/bootstrap.integration.spec.ts` exercises the bootstrap
|
||||
endpoint end-to-end.
|
||||
- **Frontend:** `tests/frontend/setup-create-admin.spec.ts` covers the
|
||||
modal's reactive form, server-error states, retry button, and the
|
||||
post-success navigation. `tests/frontend/guard-bootstrap-race.spec.ts`
|
||||
covers the `authGuard` redirect race with `BootstrapService`.
|
||||
|
||||
Run the existing suite from the repo root with:
|
||||
|
||||
```
|
||||
npm test
|
||||
```
|
||||
|
||||
## 5. Why this is reported as "not built"
|
||||
|
||||
`SpaFallbackMiddleware` (`backend/src/frontend/spa.controller.ts:27`)
|
||||
sends:
|
||||
|
||||
```
|
||||
<!doctype html><html><body><h1>HIPCTF</h1><p>Frontend not built.</p></body></html>
|
||||
```
|
||||
|
||||
when neither `frontend/dist/index.html` nor `frontend/dist/browser/index.html`
|
||||
exists. The Angular CLI's `application` builder writes to
|
||||
`dist/browser/index.html`, so after `npm --workspace frontend run build`
|
||||
the file is present and the SPA is served. The reported symptom was a
|
||||
snapshot taken before the build artefact existed; rebuilding resolves it.
|
||||
Reference in New Issue
Block a user