AI Implementation feature(881): Authenticated Shell, Quick Tabs and Change Password 1.05 (#20)
This commit was merged in pull request #20.
This commit is contained in:
@@ -1,30 +0,0 @@
|
||||
# Implementation Plan: Authenticated Shell LED Visibility Fix
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
- **Codebase style & conventions:** Node.js monorepo with an Angular 17 standalone-component SPA and NestJS backend. The authenticated shell uses OnPush components, signal inputs/outputs, inline Angular templates/styles, and theme CSS custom properties. `HomeComponent` owns event-stream lifecycle and passes `EventStatusStore.state()` into the presentational `ShellHeaderComponent`. Quick tabs and change-password behavior are already implemented and are not implicated in this regression.
|
||||
- **Data Layer:** TypeORM-backed backend settings provide `eventStartUtc` and `eventEndUtc`; no schema or persistence changes are required. The backend event-state calculation and authenticated SSE payload are already functioning: the shell receives the correct `running`, `countdown`, `stopped`, or `unconfigured` state and applies the matching class.
|
||||
- **Test Framework & Structure:** Root Jest 29 multi-project configuration with `ts-jest`; frontend tests run in jsdom from the dedicated `tests/frontend/` directory. `npm test` runs all backend and frontend tests, while `npm run test:frontend` runs the frontend project only. Existing `tests/frontend/shell-led.spec.ts` is a source-structure assertion and does not instantiate the component or verify the rendered style binding.
|
||||
- **Required Tools & Dependencies:** No new system tools, global CLIs, npm packages, persistent `/data` assets, or `setup.sh` changes are required. Existing Angular, Jest, jsdom, and TypeScript dependencies are sufficient.
|
||||
|
||||
## 2. Impacted Files
|
||||
- **To Modify:**
|
||||
- `frontend/src/app/features/shell/header/shell-header.component.ts` — remove the conflicting transparent base declaration and bind the state-specific background color directly on the LED element so the browser paints the themed color reliably.
|
||||
- `tests/frontend/shell-led.spec.ts` — replace/extend brittle source-only assertions with a focused automated regression covering the style value produced for each event state and preserving the accessible state label.
|
||||
- **To Create:** None.
|
||||
|
||||
## 3. Proposed Changes
|
||||
1. **Database / Schema Migration:** None. Event timestamps, settings storage, REST/SSE contracts, and backend state computation remain unchanged.
|
||||
2. **Backend Logic & APIs:** None. `/api/v1/events/status` already emits the correct state, and `EventStatusStore` already forwards it to the shell.
|
||||
3. **Frontend UI Integration:**
|
||||
1. In `ShellHeaderComponent`, define a typed state-to-theme-color mapping for all `EventState` values: `running` → `var(--color-success)`, `countdown` → `var(--color-warning)`, `stopped` → `var(--color-danger)`, and `unconfigured` → `var(--color-secondary)`.
|
||||
2. Replace the unused `ledClass` computed value with a computed background-color value derived from `eventState()`; keep the existing state class bindings because they are useful selectors and preserve the current DOM contract.
|
||||
3. Bind the computed value directly to the LED span's `style.background-color`. This gives the element an explicit non-transparent author value while continuing to resolve the active theme token at runtime, matching the reported proof that a direct targeted declaration makes the dot visible.
|
||||
4. Remove `background-color: transparent` from the component base `.led` rule, and remove the redundant component state-color declarations if the inline binding becomes the single authoritative color source. Retain only the shape/layout styling (`inline-block`, 10×10 dimensions, circular radius, border, alignment).
|
||||
5. Remove the global LED state rules from `frontend/src/styles.css` if they are no longer consumed, avoiding duplicate ownership and future cascade ambiguity; keep canonical theme token declarations untouched.
|
||||
6. Preserve the existing `data-testid`, four state classes, and `aria-label="Event status: <state>"`, so status remains exposed textually rather than by color alone.
|
||||
7. Do not modify `HomeComponent`, `QuickTabsComponent`, change-password components, event store, routes, SSE transport, or backend services because repository tracing shows those layers already deliver the correct state and shell behavior.
|
||||
|
||||
## 4. Test Strategy
|
||||
- **Target Unit Test File:** `tests/frontend/shell-led.spec.ts`.
|
||||
- **Mocking Strategy:** No backend, HTTP, SSE, timers, filesystem fixtures, browser automation, or persistent data. Exercise a small exported pure state-to-color helper (or equivalent component mapping) with table-driven assertions for the four states, and retain focused source/template assertions that the LED consumes that value through `[style.background-color]`, keeps each state class binding, and keeps the dynamic `aria-label`. This directly prevents a regression to a transparent base color while remaining fast and CLI-only under `npm test`.
|
||||
- **Verification Commands for the implementer:** Run `npm run test:frontend`, `npm test`, and `npm run build` from `/repo`. The root build compiles both Angular and NestJS; no separate lint or typecheck script is defined.
|
||||
@@ -0,0 +1,45 @@
|
||||
# Implementation Plan: Job 881 — Cross-Tab Authenticated Shell Invalidation
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
- **Codebase style & conventions:** TypeScript monorepo with an Angular standalone-component SPA and NestJS API. Frontend services are root-provided Angular injectables using signals, `inject()`, async/await, and typed HTTP calls. Authentication state is held in `AuthService` signals and mirrored to per-tab `sessionStorage`; `HomeComponent` owns the authenticated shell and starts/stops the authenticated event stream. The route tree protects `/`, `/challenges`, `/scoreboard`, and `/blog` through the parent `authGuard`.
|
||||
- **Data Layer:** No database/schema change is required. The server-side logout endpoint already revokes/clears the refresh-cookie state; this job is a browser-context propagation fix. Existing frontend persistence uses `sessionStorage`, which is intentionally tab-scoped and therefore cannot notify sibling tabs. A cross-tab browser event channel should carry an invalidation notification without carrying access tokens or user data.
|
||||
- **Test Framework & Structure:** Root Jest 29 with two projects in `tests/jest.config.js`; frontend tests run in jsdom through `npm test` or `npm run test:frontend`. Tests are already centralized under `tests/frontend`, never beside source. Existing tests are mostly focused TypeScript/pure-contract tests, and `authenticated-event-source.spec.ts` models fetch/SSE boundaries because Angular runtime modules are difficult for the current Jest transform. Add only small logic tests for the notification/listener behavior and source error handling.
|
||||
- **Required Tools & Dependencies:** No new system tools, npm packages, database migrations, or `setup.sh` changes are expected. `BroadcastChannel` and/or the `storage` event are browser platform APIs. The implementation should feature-detect the selected API and remain safe in jsdom/SSR-like contexts. Existing Node/npm/Jest/TypeScript setup is sufficient.
|
||||
|
||||
## 2. Impacted Files
|
||||
- **To Modify:**
|
||||
- `frontend/src/app/core/services/auth.service.ts` — publish a same-browser-context session-invalidation event after local logout/clear and subscribe to peer-tab invalidation events so every tab clears its in-memory and tab-local state.
|
||||
- `frontend/src/app/core/services/authenticated-event-source.service.ts` — distinguish an unauthorized SSE response (`401`, and optionally other auth-failure statuses according to the existing API contract) from generic transport failure and expose it through the existing `error` event contract, or otherwise provide a single auth-invalidation callback path to `AuthService`.
|
||||
- `frontend/src/app/core/services/event-status.store.ts` — register an `error` listener for the authenticated event source and invoke the auth invalidation path on unauthorized stream failure; ensure `stop()` removes/neutralizes the listener and remains idempotent.
|
||||
- `frontend/src/app/features/home/home.component.ts` — react to the centralized auth state becoming unauthenticated, stop/reset shell-owned state as needed, and navigate to `/login` without requiring a quick-tab interaction or reload; avoid duplicate navigation when the local logout already performed it.
|
||||
- **To Create:**
|
||||
- `tests/frontend/auth-cross-tab.spec.ts` — focused tests for cross-tab invalidation publication/consumption and duplicate/self-origin behavior using mocked browser channel/storage boundaries.
|
||||
- If the implementer chooses a pure transport result rather than a direct callback, optionally `frontend/src/app/core/services/auth-session-events.pure.ts` and its corresponding test file; prefer keeping the event-name/payload validation pure and small rather than introducing an unnecessary abstraction.
|
||||
- **Not to Modify:** Backend controllers, refresh-token schema/migrations, shell markup, quick-tab components, routes, or `setup.sh`; the server invalidation and route protection already exist.
|
||||
|
||||
## 3. Proposed Changes
|
||||
1. **Database / Schema Migration:**
|
||||
- No migration. Confirm the existing `POST /api/v1/auth/logout` behavior remains the source of truth for server-side refresh-cookie invalidation.
|
||||
- Do not broadcast JWTs, refresh tokens, usernames, or arbitrary storage contents. Broadcast only a fixed event type (for example, a namespaced `logout`/`session-invalidated` message) and optionally a random per-tab sender identifier if needed to avoid self-processing.
|
||||
|
||||
2. **Backend Logic & APIs:**
|
||||
- No backend endpoint changes. The existing authenticated logout endpoint is called by `AuthService.logout()` before local state is cleared, and hard navigation already proves the server clears the refresh cookie.
|
||||
- Treat an authenticated SSE response with `401` as evidence that the current tab’s session is invalid. Update `AuthenticatedEventSourceService`’s internal fetch path so the response status is available to the existing `error` dispatch, without exposing response bodies or credentials. Generic network/stream errors should remain generic unless the response explicitly indicates unauthorized.
|
||||
- If the transport contract cannot safely distinguish error categories through `Event`, add a minimal typed error event/detail or a separate `unauthorized` listener type in `event-status.pure.ts`; update all implementations and tests consistently. Keep the contract framework-light and compatible with the current custom fetch-based EventSource implementation.
|
||||
|
||||
3. **Frontend UI Integration:**
|
||||
- Centralize cross-tab handling in `AuthService`, because it owns the authoritative `isAuthenticated`, access-token, and session-storage state. Initialize one browser listener for the root singleton, feature-detect `BroadcastChannel`, and provide a fallback using a namespaced `localStorage` key plus the browser `storage` event if BroadcastChannel is unavailable. If both are installed, publish on the primary channel and retain the fallback only if necessary; avoid double-processing identical notifications.
|
||||
- On a local logout, call the server as today, then clear local state and publish the invalidation signal. Ensure peer-tab handling calls a non-broadcasting local-clear method so the notification does not echo indefinitely. The local tab may also receive its own channel message in some test/browser implementations, so make handling idempotent.
|
||||
- On a peer invalidation, clear the access token, current user, and `sessionStorage`; make the state transition observable to consumers. If `AuthService.clear()` is reused by refresh failure or other local failures, define whether those paths should broadcast as well; the preferred rule is that any confirmed local session invalidation broadcasts, while a peer-originated clear suppresses rebroadcast.
|
||||
- Wire the authenticated SSE lifecycle to the same invalidation path. `EventStatusStore.start()` should attach both message and error listeners. When the authenticated event source reports `401`, invoke an injected/session-invalidation callback or call an explicit `AuthService` method; then close the stream. Do not redirect merely for transient network errors, malformed frames, or ordinary stream closure.
|
||||
- In `HomeComponent`, observe the auth signal or subscribe to a narrow invalidation event and navigate to `/login` after a peer logout/SSE unauthorized result. Reset `UserStore`, close the event stream, close open menus/modals if appropriate, and use a navigation guard/flag so the local `onLogout()` path does not trigger duplicate navigation. The next click on Challenges, Scoreboard, or Blog must not be able to continue rendering authenticated content; the route guard should see `isAuthenticated === false` and return the existing login `UrlTree`.
|
||||
- Ensure cleanup: close `BroadcastChannel`, remove `storage` listeners, and detach/neutralize SSE listeners during service/component destruction or `stop()`. Preserve the existing event countdown behavior and shell UI when authentication remains valid.
|
||||
- Keep browser API access guarded for non-browser/test environments. Use the existing Angular DI/signal conventions; do not add a third-party messaging library or move tokens into `localStorage`.
|
||||
|
||||
## 4. Test Strategy
|
||||
- **Target Unit Test File:** `tests/frontend/auth-cross-tab.spec.ts` for the core event protocol and AuthService behavior. Extend `tests/frontend/authenticated-event-source.spec.ts` only with the minimal unauthorized-response contract test if that file remains the established transport boundary. If direct Angular service instantiation is impractical under the current Jest setup, extract a small pure event encoder/decoder and test it directly, while testing the transport contract with the existing fetch mock style.
|
||||
- **Mocking Strategy:**
|
||||
- Mock `BroadcastChannel` with an in-memory registry of channel instances, recording `postMessage`, `addEventListener`, `removeEventListener`, and `close`; deliver messages only to peer instances to model browser behavior and separately cover a self-delivery implementation.
|
||||
- Mock `window.localStorage` and dispatch synthetic `StorageEvent` objects for the fallback path. Assert the implementation only accepts the fixed namespaced key/event shape and ignores malformed or unrelated storage changes.
|
||||
- Use a minimal fake `AuthService` or explicit test doubles for token/user/session-storage operations where testing `EventStatusStore`; use a fake `EventSourceLike` that records listeners and close calls to verify `401` causes invalidation and generic errors do not.
|
||||
- Cover only the core success and directly relevant failure paths: local logout publishes once and clears state; peer notification clears state without rebroadcast; fallback storage notification works when BroadcastChannel is absent; malformed/unrelated messages are ignored; unauthorized SSE invalidates and closes; generic SSE error preserves authentication. Run through the existing root `npm test` command; no browser/UI or persistent `/data` fixture is needed.
|
||||
Reference in New Issue
Block a user