feat: Landing Page and Login/Register Modal 1.01

.kilo/plans/849.md

@@ -1,36 +0,0 @@
-# Implementation Plan: Landing Page and Login/Register Modal 1.00
-
-## 1. Architectural Reconnaissance
-- **Status:** The landing page, login/register modal, bootstrap payload flow, and Markdown helper are present, but Job 849 is **not fully implemented**: the reported runtime-function string proves the welcome binding is not receiving the intended rendered HTML at runtime. Existing source currently derives `welcomeHtml` in `frontend/src/app/features/landing/landing.component.ts:46` and binds `[innerHTML]="welcomeHtml"` in `frontend/src/app/features/landing/landing.component.html:9`; this integration needs to be corrected and covered by a focused component-level regression test.
-- **Codebase style & conventions:** Node.js npm workspace monorepo; Angular 17 standalone components with signals, `OnPush`, Angular control-flow blocks, reactive forms, and `inject()` services. The landing container owns bootstrap-derived state and delegates blog HTTP calls to `LandingService`.
-- **Data Layer:** No database or schema change is required. Backend `SystemService.bootstrap` exposes the persisted `welcomeMarkdown` setting, and `BootstrapService` stores it in its signal payload. Blog data is separately fetched from `GET /api/v1/blog/posts`.
-- **Markdown and security path:** `markdown.pure.ts:9-11` converts Markdown using `marked.parse` and sanitizes with DOMPurify; `MarkdownService.render` wraps that output for Angular HTML binding. Preserve sanitization and ensure the template receives the computed signal value, not a signal/function object. Avoid introducing new CSP inline handlers or unsafe dynamic execution; the existing CSP console error should be investigated only insofar as it is caused by the rendering integration, without weakening CSP.
-- **Test Framework & Structure:** Root Jest 29 multi-project configuration in `tests/jest.config.js`; frontend tests use `ts-jest` and `jsdom`, live exclusively under `tests/frontend/`, and run from the root with `npm test` or the focused `npm run test:frontend` command. Existing `landing-markdown.spec.ts` tests the pure renderer only; no current test mounts `LandingComponent` or verifies its `[innerHTML]` output.
-- **Required Tools & Dependencies:** No new runtime or system dependency is expected. Existing `@angular/core`, `@angular/platform-browser`, `marked`, `dompurify`, Jest/jsdom, and ts-jest are sufficient. The implementer should use the existing `setup.sh` (`npm ci`/fallback install plus Angular and Nest builds); do not add a package solely for this fix unless repository inspection during implementation demonstrates an unavoidable Angular testing requirement.
-
-## 2. Impacted Files
-- **To Modify:**
-  - `frontend/src/app/features/landing/landing.component.ts` — correct the component-side value passed to the HTML binding while retaining bootstrap signal reactivity and the existing MarkdownService sanitization boundary.
-  - `frontend/src/app/features/landing/landing.component.html` — update the binding expression if needed so Angular unwraps/consumes the computed signal value correctly and never renders the function object/source.
-  - `frontend/src/app/core/services/markdown.service.ts` — only if needed by the confirmed root cause; keep the public render contract and sanitization behavior stable rather than bypassing Angular security.
-  - `tests/frontend/landing-markdown.spec.ts` — extend with the smallest regression coverage if a pure helper contract is changed; retain the existing XSS/link sanitization assertions.
-  - `tests/frontend/landing.component.spec.ts` — add a dedicated focused component test under the existing dedicated frontend test folder to mount the standalone landing component and verify the rendered welcome card.
-- **To Create:**
-  - `tests/frontend/landing.component.spec.ts` — focused success-path and key safety/error regression tests, unless the implementer chooses to place equivalent coverage in an existing landing test without expanding scope.
-
-## 3. Proposed Changes
-Explain the exact implementation logic step-by-step:
-1. **Database / Schema Migration:** No migration. Confirm the existing bootstrap response contract remains `welcomeMarkdown: string`; use the value already loaded by `BootstrapService.payload()` and preserve the backend setting/default behavior.
-2. **Backend Logic & APIs:** No backend route or service change. Continue relying on `GET /api/v1/bootstrap`, which already returns the welcome Markdown, and do not alter `/api/v1/blog/posts`, authentication routes, login/register modal behavior, or CSP headers unless root-cause investigation proves the CSP message is independently generated by an existing Angular event-handler configuration.
-3. **Frontend UI Integration:**
-   1. Trace Angular template expression semantics for `computed()` values and verify whether the current `[innerHTML]="welcomeHtml"` passes the computed signal object rather than its value in this Angular 17 setup. The likely correction is to consume the signal (`welcomeHtml()`) in the property binding, matching the component’s other signal accesses (`pageTitle()`, `logo()`, `landing.posts()`, etc.).
-   2. Keep `welcomeHtml` as a computed value derived from `bootstrap.payload()?.welcomeMarkdown ?? ''`, so it updates automatically after bootstrap completes and renders an empty sanitized result when the payload is unavailable.
-   3. Continue routing Markdown through `MarkdownService.render` and `renderMarkdownToHtml`; do not substitute raw interpolation, unsanitized `innerHTML`, `eval`, `Function`, or a security bypass around untrusted content. If the current helper’s return type or sanitizer wrapper must change to make Angular accept the binding, make the narrowest compatible change and preserve DOMPurify’s HTML profile.
-   4. Leave the existing landing title, login button, empty blog state, and modal forms unchanged except where template compilation requires a minimal adjacent adjustment. The expected welcome DOM for the supplied payload must contain an `<h1>` with `Welcome` and paragraph text `Create the first admin to get started.` rather than the Angular runtime helper source.
-   5. Check the reported CSP error after the binding fix during automated verification. Do not weaken `script-src-attr 'none'`; if a separate source-level inline-handler issue is identified, replace it with Angular event bindings or document it as unrelated rather than adding an unsafe CSP exception.
-4. **Test Strategy Integration:** Add only minimal automated frontend logic tests: one component test that supplies a mock `BootstrapService` payload containing the job’s Markdown, renders the standalone component with minimal providers/mocks, and asserts the welcome element contains the heading and paragraph and does not contain the runtime-function text; one safety/error-focused assertion may verify empty/null Markdown produces no runtime source and that the existing pure renderer remains sanitized. Mock `LandingService.refresh()` and any unrelated auth/router dependencies as resolved stubs so the test does not perform HTTP, navigation, or UI/browser automation. Keep tests in `tests/frontend/` and make them pass through the existing root `npm test` command.
-
-## 4. Test Strategy
-- **Target Unit Test File:** `tests/frontend/landing.component.spec.ts` for the component/template regression; retain/update `tests/frontend/landing-markdown.spec.ts` only if the renderer contract is touched. No visual, screenshot, browser, network, database, or persistent `/data` fixture is needed.
-- **Mocking Strategy:** Configure the standalone component through Angular `TestBed`; provide a deterministic `BootstrapService` stub with a signal-backed payload, a `LandingService` stub whose `refresh` is a resolved promise and whose posts/loading/error signals represent the empty blog state, plus minimal `AuthService`, `Router`, and `MarkdownService` stubs or real pure Markdown service as appropriate. If HTTP providers are required by transitive dependencies, use Angular’s testing providers rather than manual HTTP mocks and ensure no request is left outstanding. Assert through stable `data-testid` attributes or a small harness/query abstraction, not CSS classes. Verify both the expected sanitized HTML content and absence of the Angular runtime source string.
-- **Verification commands for the implementer:** Run the focused frontend test, then root `npm test`, frontend production build, lint, and typecheck using the repository’s available scripts/configuration. Since no lint/typecheck scripts are present in the inspected root or frontend `package.json`, inspect any project tooling before implementation; if no commands exist, record that limitation rather than inventing dependencies. `setup.sh` already installs dependencies and builds both packages, so no update is planned.

.kilo/plans/873.md

@@ -0,0 +1,39 @@
+# Implementation Plan: Landing Page and Login/Register Modal 1.01
+
+## Status
+
+This job is **not fully implemented**. The repository already contains the landing page, login/register modal, public registration endpoint, and a nominal ten-per-minute limiter, but the reported end-to-end behavior demonstrates an off-by-one/account-count defect that must be isolated and corrected.
+
+## 1. Architectural Reconnaissance
+- **Codebase style & conventions:** Node.js npm-workspace monorepo using TypeScript, NestJS 10, Angular 17 standalone components, async service methods, Angular signals, reactive forms, Zod DTO validation, and Jest. Controllers are thin and delegate business logic to services; the Angular landing component owns modal/form state while `AuthService` owns HTTP calls and session setup.
+- **Data Layer:** TypeORM over `better-sqlite3`; the `user` table has a unique username and stores player/admin accounts. Multi-step registration/session creation runs in a `DataSource.transaction`. No schema migration is expected because rate-limit state is currently in-memory and no user schema change is needed.
+- **Relevant existing flow:** `POST /api/v1/auth/register` in `backend/src/modules/auth/auth.controller.ts` extracts the request IP and calls `AuthService.registerPlayer`. `AuthService.registerPlayer` checks `RegistrationRateLimitService`, checks `registrationsEnabled`, checks username uniqueness, hashes with Argon2id, inserts a player, records a hit, and mints a session. `RegistrationRateLimitService` currently allows fewer than ten retained timestamps. The Angular `/login` route renders `LandingComponent`, whose registration modal calls `AuthService.register`, signs the user in on success, and maps `RATE_LIMITED` errors through `buildLoginFailureMessage`.
+- **Test Framework & Structure:** Root `npm test` runs Jest projects defined in `tests/jest.config.js`; backend tests run in `tests/backend/`, frontend logic tests run in `tests/frontend/`, and tests are not colocated with source. Existing registration integration coverage is in `tests/backend/auth-register.spec.ts`; pure limiter coverage is in `tests/backend/registration-rate-limit.spec.ts`; frontend error-message coverage is in `tests/frontend/landing-modal.spec.ts`.
+- **Required Tools & Dependencies:** No new dependency or system tool is indicated. Existing Node/npm, NestJS, Angular, Jest, TypeORM, SQLite, Supertest, cookie-parser, cookiejar, and Argon2 setup are sufficient. Do not add Redis or another external rate-limit store for this single-process requirement. `setup.sh` already installs dependencies and builds both workspaces; it should remain unchanged unless implementation introduces a dependency, in which case update the lockfile and setup installation path rather than adding runtime infrastructure unnecessarily. Persistent `/data` storage is not needed because limiter state is intentionally ephemeral and tests use `DATABASE_PATH=:memory:`.
+
+## 2. Impacted Files
+- **To Modify:**
+  - `backend/src/common/services/registration-rate-limit.service.ts` — make the fixed-window boundary and/or reservation semantics explicit so exactly ten successful registrations are admitted and the eleventh is rejected.
+  - `backend/src/modules/auth/auth.service.ts` — adjust the registration admission/recording sequence if investigation confirms concurrent requests can pass separate `isAllowed` checks before either records, or otherwise centralize an atomic consume operation.
+  - `tests/backend/registration-rate-limit.spec.ts` — add a focused regression test for the exact ten-success/eleventh-rejection/rollover contract, preferably at the service level and with deterministic timestamps.
+  - `tests/backend/auth-register.spec.ts` — strengthen the existing public registration integration assertion to verify that the rejected request does not create a user and that a request after the window succeeds, without creating a large test matrix.
+- **To Create:** None expected. Keep tests in the existing dedicated `tests/backend/` folder.
+
+## 3. Proposed Changes
+Explain the exact implementation logic step-by-step:
+1. **Database / Schema Migration:** No migration. Preserve the `user` table and existing unique username constraint. The fix must ensure a rejected registration fails before user insertion; successful registrations remain player rows with `role='player'` and `status='enabled'`.
+2. **Backend Logic & APIs:**
+   1. Reproduce the reported sequence using deterministic limiter time and the same IP representation used by the controller (`req.ip`, commonly `::ffff:127.0.0.1`). Distinguish a true limiter boundary bug from test contamination or a race between `isAllowed()` and `record()`.
+   2. Replace the externally split check-then-record path with an atomic “consume/admit” operation, or add equivalent synchronization inside `RegistrationRateLimitService`. The operation must prune timestamps older than the one-minute window, admit exactly when the retained count is below ten, and reserve/record the timestamp as part of the same operation. A blocked call must not append a timestamp.
+   3. Define the rollover boundary consistently: timestamps with age at least 60,000 ms are expired, so the first request after the minute rolls over is admitted. Use an injectable/deterministic `now` argument in the pure service path to avoid sleeping in unit tests; production calls use `Date.now()`.
+   4. Call the atomic admission operation at the correct point in `AuthService.registerPlayer`. Preserve existing validation, registration-enabled, username-conflict, Argon2id, transaction, response, CSRF, and refresh-cookie behavior. Decide and document whether failed validation/disabled/duplicate attempts count based on the existing intended semantics; the job’s acceptance criterion concerns successful account creation, so avoid charging the window for requests that cannot create an account unless repository requirements explicitly require otherwise.
+   5. Check the legacy first-admin/setup callers because they share `RegistrationRateLimitService`. Preserve their protection and ensure the API error remains `429 RATE_LIMITED`; use the existing public-registration message exactly: `Too many registration attempts; please wait a minute before trying again.`
+   6. Do not change the landing-page route or modal unless backend regression analysis exposes a client contract issue. The existing `LandingComponent` already switches login/register modes, calls `AuthService.register`, establishes the session, and routes to `/`; the existing error helper handles the registration rate-limit response as a generic wait message because the server text contains no numeric seconds.
+3. **Frontend UI Integration:** No frontend code change is expected for this server-side off-by-one defect. If implementation changes the API error payload to include a numeric retry duration, update `login-modal.service.ts` and its focused test only to preserve the documented user-facing message; otherwise leave the already implemented landing page and modal intact.
+
+## 4. Test Strategy
+- **Target Unit Test File:** `tests/backend/registration-rate-limit.spec.ts` for deterministic limiter behavior: admit exactly ten calls, reject the eleventh without increasing the retained count, isolate IPs, and admit after a timestamp reaches the one-minute expiry boundary. Keep the existing service tests minimal and extend rather than creating a new test hierarchy.
+- **Target Integration Test File:** `tests/backend/auth-register.spec.ts` for one focused public API regression: enable registrations, issue ten unique valid registrations from one agent/IP, assert each returns `201`, submit the eleventh with a unique username and assert `429 RATE_LIMITED`, query the repository or count users to prove no eleventh account exists, then invoke the limiter with deterministic time or use a narrowly controlled clock/expiry strategy to verify the next request after rollover succeeds. Avoid real 31/60-second sleeps.
+- **Frontend Test File:** No new frontend test unless the error contract changes. Existing `tests/frontend/landing-modal.spec.ts` already covers the registration rate-limit mapping and should remain green.
+- **Mocking Strategy:** Unit-test the limiter without mocks. For backend integration, use the existing Nest `AppModule`, in-memory SQLite initialization via `initDb`, real TypeORM persistence, CSRF cookie priming, and Supertest as already established; mock only time if required by the implementation, using Jest fake timers or an injected clock rather than mocking repositories. Ensure app/database resources are closed and avoid shared limiter state between tests by using a fresh service/app or clearing the service state through its public lifecycle/design. All assertions should be CLI-verifiable with the existing root `npm test` command.
+- **Acceptance assertions:** There are exactly ten successful `201` registrations in the one-minute window; the eleventh returns HTTP 429 with code `RATE_LIMITED` and the required message; the rejected username is absent from `user`; the first request after the one-minute window succeeds with `201`; and no unrelated landing/login behavior regresses.

backend/src/common/services/registration-rate-limit.service.ts

@@ -18,4 +18,15 @@ export class RegistrationRateLimitService {
     arr.push(now);
     this.hits.set(ip, arr);
   }
-}
+
+  tryConsume(ip: string, now: number = Date.now()): boolean {
+    const arr = (this.hits.get(ip) ?? []).filter((t) => now - t < WINDOW_MS);
+    if (arr.length >= MAX_PER_WINDOW) {
+      this.hits.set(ip, arr);
+      return false;
+    }
+    arr.push(now);
+    this.hits.set(ip, arr);
+    return true;
+  }
+}

backend/src/modules/auth/auth.service.ts

@@ -100,7 +100,7 @@ export class AuthService implements OnModuleInit {
   async registerFirstAdmin(username: string, password: string, ip: string): Promise<LoginResponse> {
     validatePassword(password, this.config);
 
-    if (!this.registrationRateLimit.isAllowed(ip)) {
+    if (!this.registrationRateLimit.tryConsume(ip)) {
       throw new ApiError(ERROR_CODES.RATE_LIMITED, 'Too many registrations from this IP; try again later.', 429);
     }
 
@@ -128,7 +128,6 @@ export class AuthService implements OnModuleInit {
       });
       await manager.save(user);
 
-      this.registrationRateLimit.record(ip);
       return this.mintSession(manager, user);
     });
   }
@@ -136,7 +135,7 @@ export class AuthService implements OnModuleInit {
   async registerPlayer(dto: RegisterDto, ip: string): Promise<LoginResponse> {
     validatePassword(dto.password, this.config);
 
-    if (!this.registrationRateLimit.isAllowed(ip)) {
+    if (!this.registrationRateLimit.tryConsume(ip)) {
       throw new ApiError(
         ERROR_CODES.RATE_LIMITED,
         'Too many registration attempts; please wait a minute before trying again.',
@@ -174,7 +173,6 @@ export class AuthService implements OnModuleInit {
       });
       await manager.save(user);
 
-      this.registrationRateLimit.record(ip);
       return this.mintSession(manager, user);
     });
   }

tests/backend/registration-rate-limit.spec.ts

@@ -32,6 +32,25 @@ describe('RegistrationRateLimitService', () => {
     for (let i = 0; i < 10; i++) s.record('1.1.1.1', now + i);
     expect(s.isAllowed('2.2.2.2', now + 10)).toBe(true);
   });
+
+  it('tryConsume admits exactly 10 and rejects the 11th without double-recording', () => {
+    const s = new RegistrationRateLimitService();
+    const now = 1_000_000;
+    for (let i = 0; i < 10; i++) {
+      expect(s.tryConsume('1.1.1.1', now + i)).toBe(true);
+    }
+    expect(s.tryConsume('1.1.1.1', now + 10)).toBe(false);
+  });
+
+  it('tryConsume admits again once the window rolls over', () => {
+    const s = new RegistrationRateLimitService();
+    const base = 1_000_000;
+    for (let i = 0; i < 10; i++) {
+      expect(s.tryConsume('1.1.1.1', base + i)).toBe(true);
+    }
+    expect(s.tryConsume('1.1.1.1', base + 10)).toBe(false);
+    expect(s.tryConsume('1.1.1.1', base + 60_000)).toBe(true);
+  });
 });
 
 describe('register-first-admin registration rate limit (integration)', () => {
@@ -77,4 +96,4 @@ describe('register-first-admin registration rate limit (integration)', () => {
       .send({ username: 'noone', password: 'Sup3rSecret!Pass' })
       .expect(429);
   });
-});
+});