56 lines
10 KiB
Markdown
56 lines
10 KiB
Markdown
# Implementation Plan: Authenticated Shell, Quick Tabs and Change Password 1.01
|
|
|
|
## 1. Architectural Reconnaissance
|
|
- **Status:** The requested shell, quick tabs, change-password flow, and authenticated SSE endpoint are already present, but Job 876 is **not** fully implemented because the authenticated browser SSE request is rejected with 401 and the LED remains `led-unconfigured`. Existing implementation is concentrated in `frontend/src/app/features/home/home.component.ts:127-135`, `frontend/src/app/core/services/event-status.store.ts:51-76`, `frontend/src/app/features/shell/header/shell-header.component.ts:25-35`, and `backend/src/modules/system/system.controller.ts:77-113`.
|
|
- **Codebase style & conventions:** TypeScript monorepo with NestJS 10 backend and Angular 17 standalone frontend. Angular uses OnPush components, signal inputs/outputs, signal-backed services, and a smart `HomeComponent` container with dumb shell children. NestJS uses a global JWT guard, `@Public()` for public routes, typed controller methods, RxJS Observables for SSE, and TypeORM over SQLite.
|
|
- **Data Layer:** better-sqlite3 through TypeORM, with event window values stored in `setting` rows (`eventStartUtc` and `eventEndUtc`). No schema change is indicated. `EventStatusService.getState()` is the authoritative four-state event state machine.
|
|
- **Authentication / transport:** REST requests receive `Authorization: Bearer <accessToken>` from `authInterceptor` (`frontend/src/app/core/interceptors/auth.interceptor.ts:5-11`). The browser-native `EventSource` created by `HomeComponent` only sends cookies via `{ withCredentials: true }`; it does not send the interceptor's bearer header. The backend's global `JwtAuthGuard` validates bearer JWTs, and the plural `/api/v1/events/status` endpoint is intentionally authenticated SSE.
|
|
- **Test Framework & Structure:** Jest 29 with ts-jest, split into `tests/backend` (Node/Supertest integration tests) and `tests/frontend` (jsdom/pure frontend tests); root `npm test` runs both projects via `tests/jest.config.js`. Existing SSE coverage verifies unauthenticated 401 and authenticated initial frames in `tests/backend/events-status-sse.spec.ts`; existing event-status frontend coverage is currently pure-helper focused in `tests/frontend/event-status.store.spec.ts`.
|
|
- **Required Tools & Dependencies:** No new dependency should be required. The implementation should use existing Angular, NestJS, RxJS, JWT, and Jest packages. No database migration or `/data` fixture is needed. `setup.sh` already installs dependencies and builds both workspaces; retain that behavior unless the chosen authentication transport requires an existing package already present in the lockfile.
|
|
|
|
## 2. Impacted Files
|
|
- **To Modify:**
|
|
- `frontend/src/app/features/home/home.component.ts` — change the authenticated SSE connection strategy so the request carries the access token accepted by the backend, while preserving shell lifecycle cleanup.
|
|
- `frontend/src/app/core/services/event-status.store.ts` and/or `frontend/src/app/core/services/event-status.pure.ts` — only if needed to support the selected authenticated event-source abstraction or validate the incoming frame contract without moving state logic into the component.
|
|
- `frontend/src/app/core/interceptors/auth.interceptor.ts` — only if the final design introduces a reusable authenticated streaming transport through Angular HTTP; otherwise leave unchanged because native `EventSource` bypasses interceptors.
|
|
- `backend/src/modules/system/system.controller.ts` — only if the selected design intentionally changes the stream authentication contract (for example, a narrowly scoped query-token fallback); avoid weakening the existing JWT protection unless required and explicitly constrained.
|
|
- `tests/frontend/event-status.store.spec.ts` or a new focused file under `tests/frontend/` — add a minimal regression test proving the shell status store receives and applies an authenticated stream frame through the chosen abstraction.
|
|
- `tests/backend/events-status-sse.spec.ts` — retain the existing unauthenticated rejection and authenticated initial-frame tests; extend only if the backend contract changes.
|
|
- `docs/api/system.md` and `docs/guides/event-window.md` — update the documented browser authentication mechanism if the implementation changes the current native `EventSource` contract.
|
|
- **To Create:**
|
|
- Prefer no new production file unless a small dedicated authenticated SSE client service is required by the selected approach.
|
|
- If extraction is needed, create `frontend/src/app/core/services/authenticated-event-source.service.ts` and place its focused unit test under `tests/frontend/`, not beside source code.
|
|
|
|
## 3. Proposed Changes
|
|
Explain the exact implementation logic step-by-step:
|
|
|
|
1. **Database / Schema Migration:**
|
|
- No migration. Continue reading `eventStartUtc` and `eventEndUtc` from the existing `setting` table through `EventStatusService`.
|
|
- Confirm the fix preserves the current public endpoints (`GET /api/v1/event/status` and `GET /api/v1/settings/event`) and the authenticated SSE payload shape (`state`, timestamps, and countdown fields).
|
|
|
|
2. **Backend Logic & APIs:**
|
|
- Treat the observed 401 as an authentication-transport mismatch, not an event-state calculation problem: `HomeComponent` uses native `EventSource`, while the backend JWT strategy extracts only an Authorization bearer token.
|
|
- Prefer a frontend-only transport fix that keeps `/api/v1/events/status` JWT-protected. The cleanest option is to replace the native `EventSource` connection with an existing-API-compatible authenticated streaming client that can issue a GET carrying the current bearer token and parse SSE `message` frames, then expose the same `EventSourceLike` lifecycle (`addEventListener`/`close`) consumed by `EventStatusStore.start()`.
|
|
- Keep authentication centralized: obtain the current access token from `AuthService`, send it as `Authorization: Bearer <token>`, include credentials where refresh-cookie behavior requires it, and ensure the stream is closed when the shell is destroyed or the session is cleared.
|
|
- Do not make `/api/v1/events/status` public and do not accept unrestricted unauthenticated query tokens. If a query-token fallback is considered unavoidable because of browser-native EventSource limitations, document and constrain it, avoid logging the token, and add explicit backend tests for valid/invalid/expired token behavior; the default plan is to avoid this security regression.
|
|
- Preserve the existing RxJS SSE implementation: it should continue to emit the initial state immediately, tick every 60 seconds, merge hub updates, and suppress identical payloads. No changes to event-state computation are needed.
|
|
|
|
3. **Frontend UI Integration:**
|
|
- In `HomeComponent.ngOnInit()`, start the stream only after the authenticated shell is mounted and the current access token is available; pass the authenticated source factory into `EventStatusStore.start()`.
|
|
- Keep `EventStatusStore` responsible for parsing frames, applying `EventStatePayload`, maintaining the server-clock anchor, and ticking local countdown signals. Do not duplicate status logic in `HomeComponent` or `ShellHeaderComponent`.
|
|
- Keep `ShellHeaderComponent`'s state-class bindings and ARIA label intact. Once the first authenticated frame arrives, `eventStatus.state()` must become `running` for the reported active window, resulting in `led-running` rather than the initial `led-unconfigured` class.
|
|
- Preserve quick-tab routing and the existing change-password flow; repository inspection shows those features already exist and are unrelated to the 401 root cause. Avoid reworking them.
|
|
- Ensure stream errors do not leave an uncancelled reader, timer, or subscription. Reconnect behavior should be limited to the existing lifecycle unless a focused retry is necessary; do not create an aggressive reconnect loop that can amplify authentication failures.
|
|
- Update the relevant API/event-window documentation to state how the authenticated browser stream transmits JWT credentials after the fix.
|
|
|
|
## 4. Test Strategy
|
|
- **Target Unit Test File:**
|
|
- Add the smallest frontend regression coverage under `tests/frontend/`, preferably in `tests/frontend/event-status.store.spec.ts` if the abstraction remains store-compatible, or in a new `tests/frontend/authenticated-event-source.spec.ts` if a dedicated client is extracted.
|
|
- Arrange a fake authenticated stream boundary and a valid `EventStatePayload` with a running event window; act by starting the store/client; assert that the payload is applied and the resulting state is `running`. Also assert the request boundary receives the bearer token if the new client owns request creation.
|
|
- Keep the existing backend tests in `tests/backend/events-status-sse.spec.ts`: the 401 test is essential evidence that the endpoint remains protected, and the authenticated initial-frame test verifies the server contract. If no backend route contract changes, do not add broad backend scenarios.
|
|
- **Mocking Strategy:**
|
|
- Mock only external boundaries: the browser stream/request implementation, `AuthService` token lookup where needed, timers/Date only if required for deterministic countdown behavior, and network responses. Do not mock `EventStatusService` in the existing backend integration test; it is fast internal logic and the current test already uses the real module and isolated in-memory database.
|
|
- Use the repository's Jest configuration and dedicated `tests/` folders. Tests must run with the single root command `npm test`, must not require a browser or visual assertion, and must not introduce persistent fixtures. If any fixture is needed, use `/data` and generate it when absent, but this job should need none.
|
|
- Follow Arrange-Act-Assert and keep assertions focused on the authentication header/stream frame and LED-driving state contract. Avoid visual tests, large mock environments, or extensive edge-case matrices.
|
|
- **Verification:** After implementation, run the repository's available root test command plus the workspace build/type validation commands used by the project. There is no declared lint or typecheck npm script in `package.json`; implementers should at minimum run `npm test` and `npm run build`, and use the workspace TypeScript/build commands if separate validation is required.
|