a8ead65ea8c90ef30691356bfcf3d64231623cdc
8 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 julien
|
6120471b66 |
chore(workspace): tsconfig composite refs + axe-linter false-positive config (#147)
## Summary
Three editor-noise sources flagged by the VS Code TypeScript service + the Deque axe Linter extension, each tamed at the right layer. No runtime behaviour change.
| Source | Fix |
| --- | --- |
| **TS6306** — Referenced project `libs/{shared/ui,shared/state,feature/auth}` must have `composite: true`. Nx 22's lib generator doesn't emit `composite`; modern VS Code TS service flags it. | Add `composite: true` to each lib's `tsconfig.lib.json`, let `nx sync` redirect consumer references in `apps/portal-{shell,admin}/tsconfig.app.json` to point at the `.lib.json` directly. |
| **TS6504** — `moduleResolution: "node"` / `"node10"` deprecated, removed in TS 7.0. Two hits on the BFF tsconfigs. | Add `ignoreDeprecations: "5.0"` on `apps/portal-bff/tsconfig.{app,spec}.json` — the opt-out knob the diagnostic itself suggests. A proper migration to `nodenext`/`node16` is a separate chantier. |
| **axe-core/list (WCAG 1.3.1)** — `<ul>` "must only directly contain `<li>`, `<script>`, or `<template>`" — fires on Angular 17+ `@for` blocks inside lists. Pure static-linter limitation; rendered DOM is fine. | New `.axe-linter.yml` at repo root: `global-disable: [list]`. |
## What lands
### `composite: true` on lib `.lib.json`
[`libs/shared/ui/tsconfig.lib.json`](libs/shared/ui/tsconfig.lib.json), [`libs/shared/state/tsconfig.lib.json`](libs/shared/state/tsconfig.lib.json), [`libs/feature/auth/tsconfig.lib.json`](libs/feature/auth/tsconfig.lib.json) get `composite: true` added. `nx sync` then automatically rewrites consumer references:
```diff
- "path": "../../libs/shared/ui"
+ "path": "../../libs/shared/ui/tsconfig.lib.json"
```
in [`apps/portal-shell/tsconfig.app.json`](apps/portal-shell/tsconfig.app.json) and [`apps/portal-admin/tsconfig.app.json`](apps/portal-admin/tsconfig.app.json). Semantically cleaner — the app references the lib's actual compile config (which produces the `.d.ts` it consumes), not the lib's solution-style root tsconfig.
**Earlier attempt — composite on the solution `tsconfig.json` — silently broke `vitest`**: the Angular Vite plugin chokes on a composite project with `files: []` / `include: []` and falls through, leaving spec files loaded but tests not registered (`"No test suite found in file"`). Moving `composite` to `.lib.json` (the project that actually has inputs) fixes the contract without poking the plugin.
### `ignoreDeprecations: "5.0"`
[`apps/portal-bff/tsconfig.app.json`](apps/portal-bff/tsconfig.app.json) and [`apps/portal-bff/tsconfig.spec.json`](apps/portal-bff/tsconfig.spec.json) — silences `Option 'moduleResolution=node10' is deprecated and will stop functioning in TypeScript 7.0`. The diagnostic suggests `"6.0"` as the value, but TS 5.9 (our pinned version) only accepts `"5.0"`; using `"6.0"` results in `TS5103: Invalid value for '--ignoreDeprecations'` and breaks every spec. `"5.0"` is the current-gen accepted value.
The deprecation is real — TS 7.0 will drop both `"node"` and `"node10"` `moduleResolution` modes. The migration target is `moduleResolution: "nodenext"` paired with matching `module: "nodenext"`, but that interacts non-trivially with Nest's CommonJS pipeline and the BFF's import semantics. Out of scope for a drive-by fix; we'll handle it as a dedicated chantier when TS 7.0 lands on the roadmap.
### `.axe-linter.yml`
New file at repo root:
```yaml
global-disable:
- list
```
The Deque axe Linter VS Code extension reads `.axe-linter.yml` at workspace root. The `list` rule (WCAG 1.3.1) fires false positives on Angular 17+ control-flow syntax — `@for (item of list; ...) { <li>… }` looks like a non-`<li>` child of `<ul>` to a static HTML scanner. The Angular compiler erases those tokens at build time; the rendered DOM is compliant. CI accessibility coverage is provided by `axe-playwright` per [ADR-0016 §"Tooling"](docs/decisions/0016-accessibility-baseline-wcag-aa-targeted-aaa.md) — it runs against the rendered DOM and is unaffected by this disable.
## Notes for the reviewer
- **Why not `composite: true` on every lib?** Per CLAUDE.md "no premature abstractions" — `libs/shared/{tokens,util}` are not currently referenced by any `tsconfig.app.json`, so they don't trigger TS6306. Adding `composite` to them would be future-proofing without a current consumer. When a consumer reference is added, the same one-line fix lands then.
- **Why not migrate `moduleResolution` properly?** The BFF runs on Nest's CommonJS pipeline; `nodenext` brings stricter ESM resolution (`.js` extensions in imports, package `exports` map enforcement) that ripples through. Not a 5-minute change. The `ignoreDeprecations` knob is the textbook defer mechanism for exactly this case.
- **Why disable `list` globally rather than per-file?** The rule's false-positive pattern (`<ul><@for>` / `<ol><@for>`) applies workspace-wide; we use `@for` consistently across `portal-shell` + `portal-admin`. Per-file disables would multiply as new templates land. axe-playwright remains the authoritative check on the rule.
## Test plan
- [x] `pnpm nx run-many -t lint test build --projects=portal-shell,portal-admin,portal-bff,shared-ui,shared-state,feature-auth` — 18/18 tasks pass. **517 specs green** across the affected projects.
- [x] `pnpm nx sync:check` — workspace in sync after the changes; running `sync` again is a no-op.
- [ ] Editor smoke — reopen the workspace in VS Code: the TS6306 errors on lib `tsconfig.json` files should be gone, the two `moduleResolution=node10` deprecation lines on BFF tsconfigs should be silenced, and the `list` rule under `sidebar.html` (`portal-admin`) should no longer surface.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #147
|
||
|
julien
|
71a098b154 |
fix(feature-auth): use AUTH_PATH_PREFIX to skip the /me endpoint in the 401 interceptor (#135)
## Summary Regression introduced by PR #134 / merged minutes ago: opening portal-admin while anonymous fires the bootstrap `/admin/auth/me` → 401 → `bffUnauthorizedInterceptor` triggers `AuthService.refresh()` → which re-fires `/admin/auth/me` → 401 → tight loop that exhausts the BFF's 120/min rate limiter in seconds. The user lands on a `rate_limited` error instead of the "Sign in" panel. ## Root cause [`bffUnauthorizedInterceptor`](libs/feature/auth/src/lib/bff-unauthorized.interceptor.ts) deliberately skips the `/me` endpoint to avoid the loop (the bootstrap `/me` legitimately 401s when anonymous). But the skip target was **hardcoded** to `${bffBaseUrl}/auth/me`: ```ts !req.url.startsWith(`${bffBaseUrl}/auth/me`) ``` When portal-admin overrides `AUTH_PATH_PREFIX` to `/admin/auth` (per PR #134), the bootstrap URL becomes `${bffBaseUrl}/admin/auth/me`, which does **not** start with `${bffBaseUrl}/auth/me`. The interceptor treats it as a regular protected route, calls `refresh()`, and we're off to the races. The reported log shows the rate-limit counter dropping `remaining=3 → 2 → 1 → 0` over four `/me` calls within ~25 ms, all 401. ## Fix Derive the skip URL from `AUTH_PATH_PREFIX` (which the interceptor already has access to via DI — same module as the AuthService): ```ts const meUrl = `${bffBaseUrl}${pathPrefix}/me`; … !req.url.startsWith(meUrl) ``` Same behaviour for portal-shell (the default `/auth` factory keeps the skip URL at `/auth/me`), correct behaviour for portal-admin (`/admin/auth/me`), and any future surface that picks a different prefix inherits the fix automatically. ## Test plan - [x] `pnpm nx test feature-auth` — **30 specs pass** (was 29; +1 regression spec asserting that no follow-up `/me` is issued after a bootstrap 401 when the prefix is `/admin/auth`). - [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean. - [ ] Manual: `pnpm nx serve portal-admin`, visit `http://localhost:4300/`, observe the BFF log — exactly **one** `/admin/auth/me` request (the bootstrap), 401, no follow-up, no rate-limit hit. The home page renders the "No admin session detected" + Sign in button. ## Notes for the reviewer - The bug pattern is symmetric: any future host that overrides `AUTH_PATH_PREFIX` would have hit the same trap. Making the skip target derive from the token is the structural fix; the regression spec pins it. - portal-shell tests still pass without changes — the default factory returns `/auth`, so the existing fixtures continue exercising `${bffBaseUrl}/auth/me` as the skip target. - No SPA-side changes needed in portal-shell or portal-admin app code. The fix is entirely inside the lib. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #135 |
||
|
julien
|
40741ce326 |
feat(portal-admin): spa auth wiring + admin shell skeleton (#134)
## Summary Phase-3a step per [ADR-0020](docs/decisions/0020-portal-admin-app.md) §"Confirmation" item 4 (entry route + admin shell). Wires the existing [`feature-auth`](libs/feature/auth) library against the distinct admin OIDC routes (`/api/admin/auth/*`) the BFF exposes since PR #129, ships a lean header/sidebar/footer chrome with an "Admin" badge so an internal user can never mistake the surface for portal-shell, and gives the landing page a self-test panel that confirms the auth chain end-to-end as soon as an `admin` Entra role gets assigned. ## What lands ### Lib change — `AUTH_PATH_PREFIX` injection token [`libs/feature/auth`](libs/feature/auth/src/lib/auth.config.ts) gains one injection token. AuthService composes URLs as `${bffBaseUrl}${pathPrefix}/{me,login,logout}` instead of hard-coding `/auth/...`. Default factory returns `/auth`, so portal-shell-shaped consumers keep working without an explicit provider — no churn on existing call sites. Admin hosts override with `/admin/auth`. The interceptors (`bffCredentialsInterceptor`, `csrfInterceptor`, `bffUnauthorizedInterceptor`) are **unchanged** — they only care about the BFF base URL, not the path prefix. ### portal-admin wiring - [environment.ts](apps/portal-admin/src/environments/environment.ts) — same shape as portal-shell, same BFF base URL (both SPAs talk to one BFF per ADR-0020 §"Where does the admin app live"). Same CSRF cookie name in v1. - [app.config.ts](apps/portal-admin/src/app/app.config.ts) — `HttpClient` with the standard interceptor chain (credentials → csrf → unauthorized), `AUTH_BFF_BASE_URL` from env, `AUTH_PATH_PREFIX = '/admin/auth'`, `AUTH_CSRF_COOKIE_NAME` from env. ### Admin shell - **[AdminHeader](apps/portal-admin/src/app/components/header/header.ts)** — APF wordmark + persistent "Admin" badge + auth widget. No global search / notifications / help cluster: admins land on tabular workloads, not a discovery dashboard (ADR-0020 §"UX style is data-dense"). - **[AdminSidebar](apps/portal-admin/src/app/components/sidebar/sidebar.ts)** — static menu listing the four ADR-0020 v1 modules. Audit log is a live router link (target of the next PR); the others are `aria-disabled` placeholders with a "Soon" badge so the navigation shape is visible even before they ship. - **[AdminFooter](apps/portal-admin/src/app/components/footer/footer.ts)** — copyright + persistent "Admin surface" tag. Stays in view for long workloads where the header has scrolled off. ### Home — auth self-test panel [apps/portal-admin/src/app/pages/home/](apps/portal-admin/src/app/pages/home/): - Signed-in: shows `displayName`, `username`, `tenant`, `oid` in a monospaced detail list. - Anonymous: "No admin session detected" + a "Sign in via Entra" button that delegates to `AuthService.login()` → 302 through `/api/admin/auth/login`. - Error: "Could not reach the BFF" + retry button. - Roadmap list quoting ADR-0020's v1 catalogue. ## Known limitations (v1, documented) - **CSRF cookie shared between surfaces.** Both portals issue `portal_csrf` on session creation. A user with both portals open will see overwrites on the second sign-in; mutating actions on the first surface will then 403 until refresh. Splitting to `portal_admin_csrf` is a follow-up if the pattern becomes common. - **Shell chrome strings are plain English.** The admin app's `$localize` plumbing is wired (matches portal-shell), but adding markers to the shell here would require regenerating `messages.fr.xlf` and `i18nMissingTranslation=error` fails the prod build on every gap. Full admin i18n is its own follow-up. - **`LayoutStateService` not yet consumed.** No collapse toggle in v1. Theme preference still threads through because both apps read the same `localStorage` key — toggle in portal-shell, see it honoured in portal-admin. - **`ci:perf` only runs against portal-shell.** Admin perf budgets are enforced at build time by `apps/portal-admin/project.json` (`maximumError == maximumWarning` per ADR-0020's relaxed thresholds: 500 KB initial JS / 1.5 MB initial total). Adding admin to `pnpm ci:perf`'s gzip + Lighthouse chain is a follow-up. ## Test plan - [x] `pnpm nx test feature-auth` — **29 specs pass** (was 28; +1 for the `AUTH_PATH_PREFIX` override). - [x] `pnpm nx test portal-admin` — **15 specs pass** (was 1; +14: AdminHeader 5, AdminSidebar 3, AdminFooter 2, Home 4, App 1 expanded). - [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean. - [x] Gzip-budget script run manually against `dist/apps/portal-admin/browser`: 86.65 KB initial / 300 KB budget, 4.46 KB CSS / 150 KB budget, 2.15 KB largest lazy / 100 KB per-chunk budget. Both `en` + `fr` bundles checked thanks to PR #133's multi-locale detection. - [ ] e2e — pending an Entra `admin` role assignment. Once available: `pnpm nx serve portal-admin`, visit `http://localhost:4300`, see "No admin session detected", click "Sign in via Entra", complete the round-trip, land back on `/` with the signed-in payload + `portal_admin_session` cookie set. ## Notes for the reviewer - `AdminHeader` is intentionally narrower than `Header` from portal-shell — no shared base class. The two are likely to evolve in different directions (admin-specific banner with system-status badges, audit-trail link, etc.) and a premature abstraction would be expensive to undo. - `AdminSidebar`'s "Soon" badge is a deliberate signal — without it, an admin who clicked a placeholder link and got a route-not-found would assume the app is broken. The badge sets expectations. - All shell components use `:host-context(.dark)` for dark-mode SCSS instead of `:host(.dark)` — same pattern as portal-shell, since the `.dark` class lives on `<html>` outside the component's view encapsulation. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #134 |
||
|
julien
|
5bbe2304ff |
feat(portal-bff): helmet + env-driven CORS allowlist + double-submit CSRF (#122)
## Summary Phase-2 security baseline that the `main.ts` placeholder note has been advertising since the auth/session work began. Three independent middlewares + their SPA counterparts, all mounted in a single PR because they only become meaningful together. ### Helmet on the BFF `helmet()` with three overrides matching our specific shape: - **HSTS only in production** — dev runs on plain HTTP, HSTS is just noise. - **`crossOriginResourcePolicy: 'cross-origin'`** — the SPA on its own origin reads JSON from the BFF; the default `same-origin` would block it. - **CSP disabled in non-production** — the BFF doesn't render HTML, so CSP on JSON responses is mostly inert, but Helmet's default CSP triggers noisy `connect-src` violations in browser devtools that we don't need. Everything else is Helmet defaults: `X-Frame-Options=SAMEORIGIN`, `X-Content-Type-Options=nosniff`, `Referrer-Policy=no-referrer`, `X-Powered-By` removed, etc. ### CORS allowlist, env-driven `CORS_ALLOWED_ORIGINS` env (comma-separated) is now **mandatory** at boot. The BFF refuses to start without it via `readCorsAllowlist()` — same boot-time validator family as `assertSessionSecret` etc. The previous hardcoded `http://localhost:4200` fallback is gone; getting CORS wrong silently is the kind of "works in dev, breaks in prod" trap the validator is specifically designed to catch. `X-CSRF-Token` is now in the allowed headers. ### Double-submit CSRF - BFF mints a 256-bit `csrfToken` at session creation (`/auth/callback`), stored on `req.session.csrfToken` and mirrored to a JS-readable cookie (`__Host-portal_csrf` prod / `portal_csrf` dev). The cookie is the SPA's read-only view; the server-side session is the source of truth. - `createCsrfMiddleware` (mounted after the session middleware in `main.ts`) compares the `X-CSRF-Token` header with `req.session.csrfToken` using `crypto.timingSafeEqual`. Skips: - safe methods (`GET / HEAD / OPTIONS`), - anonymous requests (no `req.session.user`), - `/api/auth/login` and `/api/auth/callback` (those mint the token themselves). - Mismatch → `403 {"error":"csrf"}` with a structured Pino warn. - SPA's `csrfInterceptor` reads the cookie via `document.cookie` and copies its value into `X-CSRF-Token` on every mutating BFF request. The header is omitted on `GET / HEAD / OPTIONS` (BFF skips them anyway) and on non-BFF origins. - Logout and the absolute-timeout middleware both clear the CSRF cookie alongside the session cookie. ## Notable choices **Session-bound double-submit, not pure cookie-vs-header.** A naive "compare cookie with header" check is defeated when an attacker can plant a cookie (subdomain takeover, etc.). Comparing the header to the server-side session-stored token instead means the attacker would also need to be the authenticated user — which is what CSRF defense is supposed to prevent in the first place. **No CSRF for anonymous mutating routes (v1).** None exist today; we don't have an unauthenticated POST endpoint anywhere. Generating a CSRF token for anonymous sessions would conflict with `saveUninitialized: false` on express-session and add complexity we don't need yet. Anonymous public-form CSRF defenses (site-key, captcha) land if and when those routes ship. **`SameSite=Lax`, not `Strict`, on the CSRF cookie.** Matches the session cookie's policy so the two travel together on the SPA→BFF cross-origin same-site fetch (different ports = different origin, same registrable domain). The double-submit pattern is what gives the protection; `SameSite=Lax` is a belt-and-braces layer. **`csrfInterceptor` runs after `bffCredentialsInterceptor` and before `bffUnauthorizedInterceptor` in the chain.** Order: credentials first (set `withCredentials`), then CSRF (set the header), then unauthorized handling (catch 401s). Forward order, no surprises. **`CORS_ALLOWED_ORIGINS` has no localhost fallback.** I considered keeping the fallback for ergonomics but it makes the BFF silently misconfigured if someone forgets the env. The error message points straight at the file to edit. ## Out of scope (next PRs) - Rate limiting + structured error filter (still in the phase-2 to-do). - CSP fine-tuning when we have actual HTML pages (portal-shell + portal-admin static serving). - CSRF token rotation on idle-extension (today the token lives the session's lifetime; refreshing on each request would invalidate in-flight mutations). ## Test plan - [x] `pnpm nx run-many -t test --projects=portal-bff,feature-auth,portal-shell` clean env → **177 + 28 + 34 = 239/239 pass** (was 144 + 19 + 34 = 197 before; +42 specs across CSRF middleware, CSRF cookie helpers, CORS allowlist parser, csrfInterceptor, and extended auth.controller / absolute-timeout coverage). - [x] `pnpm nx run-many -t lint build --projects=portal-bff,feature-auth,portal-shell` → clean. - [x] **CI clean-env repro** (lesson from prior PRs): every env var unset (including new `CORS_ALLOWED_ORIGINS`) → tests still pass. The BFF refuses to boot without `CORS_ALLOWED_ORIGINS`, which is the intended behaviour. - [x] Prettier-clean. - [ ] Manual smoke against running BFF: - [ ] Sign in → `__Host-portal_csrf` (prod) / `portal_csrf` (dev) cookie set, value matches `audit.events.payload->>actorIdHash`-style traceability via `req.session.csrfToken` in Redis. - [ ] Hit a future POST route from the SPA → request carries `X-CSRF-Token`, BFF accepts. - [ ] Forge a POST without the header (curl) → 403 `{"error":"csrf"}`. - [ ] Sign out → both cookies cleared. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #122 |
||
|
julien
|
177f2f20c0 |
feat(portal-shell): authGuard + BFF http interceptors + /profile demo route (#117)
## Summary Brings the SPA auth track to the level of polish the BFF surface deserves. After #113/#114, the header reflects sign-in state — but the SPA had no protected routes and no global handling of session-state drift. This PR adds three building blocks (one guard, two interceptors) plus one demo consumer. - **`authGuard`** (`CanActivateFn`) — gates routes on `AuthService.state`. Waits out the bootstrap `loading` state, allows when `authenticated`, redirects through `auth.login()` (full-page navigation to the BFF's `/auth/login` → Entra round-trip) when `anonymous` or `error`. - **`bffCredentialsInterceptor`** — flips `withCredentials: true` on every request whose URL starts with `AUTH_BFF_BASE_URL`. Replaces the per-call flag we had on `/me` (#114) with a single point of truth. Future BFF calls inherit it automatically — no chance of forgetting it. - **`bffUnauthorizedInterceptor`** — calls `AuthService.refresh()` when a BFF route (other than `/auth/me` itself) answers 401. Keeps the SPA's auth state in sync after server-side session destruction (absolute-timeout, manual revoke, idle-TTL expiry). - **`/profile`** demo route — first real consumer of the guard. Lazy-loaded component that renders the curated `CurrentUser` payload (display name, username, oid, tid). Exercises the full loop end-to-end: guard waits on /me → BFF answers → SPA renders. ## Notable choices **Lazy `AuthService` resolution in the 401 interceptor.** A naive `inject(AuthService)` at the top of the interceptor caused a circular-construction error: `AuthService`'s own constructor fires the bootstrap `/me`, which goes through the interceptor chain, which tries to inject `AuthService` while it's still being constructed. The fix is to inject the parent `Injector` and resolve `AuthService` lazily inside `catchError` — by the time a 401 actually fires, construction is done. Standard Angular pattern for "interceptor depends on a service that uses HttpClient". **`/auth/me` is excluded from the 401 refresh trigger.** The interceptor's whole job is to catch session-state drift; `/me` is the probe `AuthService.refresh()` itself uses. Without the exclusion, a 401 from /me would call `refresh()` → another /me → another 401 → infinite loop. **On `error` state, the guard still redirects to `/auth/login`.** Could have shown a "can't reach the server" page on the protected route, but the BFF-side login screen surfaces diagnostics more usefully (Entra's own error path) than a generic SPA outage page would. **Per-call `withCredentials: true` removed from `AuthService.refresh()`.** The interceptor now applies it uniformly. The spec that pinned the per-call flag is also gone — that contract moved to `bff-credentials.interceptor.spec.ts` where it belongs. **`profileTitle` + 6 new i18n message ids.** `route.profile.title`, `profile.heading`, `profile.intro`, `profile.field.{displayName,username,oid,tid}` shipped in `messages.fr.xlf` with FR translations. ## Out of scope (next PRs) - A real user-profile feature (settings, preferences, etc.) — `/profile` is just an auth-loop fixture today. - Showing the auth-loading state on protected routes (currently the guard blocks navigation; the user sees the previous route until /me resolves). Acceptable for v1. ## Test plan - [x] `pnpm nx test feature-auth` → **19/19 pass** (was 8; +11 across `auth.guard.spec.ts`, `bff-credentials.interceptor.spec.ts`, `bff-unauthorized.interceptor.spec.ts`). - [x] `pnpm nx test portal-shell` → **34/34 pass** (was 32; +2 for the Profile component). - [x] `pnpm nx lint feature-auth portal-shell` → clean. - [x] `pnpm nx build portal-shell` → clean. Bundle: main 492 kB raw / 131 kB transfer (well under the 300 KB gzip budget per ADR-0017). - [x] **CI clean-env repro** (lesson from #115/#116): `env -u REDIS_URL -u SESSION_* ... pnpm exec nx run-many -t test` → 123 + 19 + 34 = **176/176 pass**. - [ ] Manual smoke against running BFF: - [ ] Anonymous → visit `/profile` → redirect to `/auth/login` → Entra → callback → SPA lands at `/profile` with identity card filled in. - [ ] Trigger an absolute-timeout (set `SESSION_ABSOLUTE_TIMEOUT_SECONDS=5` in BFF `.env`, wait) → next BFF call returns 401 → header flips to "Sign in". --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #117 |
||
|
julien
|
0e4f0fc611 |
fix(portal-shell): send withCredentials on /me so the session cookie crosses SPA→BFF in dev (#114)
## Summary Manual smoke after PR #113 surfaced a dev-only bug: after `/auth/callback` the BFF correctly sets the `portal_session` cookie and redirects to the SPA, but the SPA's next call to `/api/auth/me` comes back **401 with no `cookie:` header at all**. The user lands "back at the portal" but the header still shows "Sign in". **Root cause.** Angular's `HttpClient` via `withFetch()` inherits `fetch`'s default `credentials: 'same-origin'`. In dev, `localhost:4200` (SPA) → `localhost:3000` (BFF) is cross-origin (different ports), so the browser drops the session cookie on the way out. SameSite=Lax is a red herring: both URLs share the registrable domain, so the cookie is still same-site — what was missing was opting the fetch into credentials. **Fix.** Per-call `withCredentials: true` on the /me request. Only /me needs cookies today; login/logout are full-page navigations through `window.location`, which the browser hydrates with cookies regardless. A global `HttpInterceptor` will be the right abstraction once other authenticated BFF endpoints exist — premature for one consumer. **BFF side was already correct.** `enableCors({ credentials: true })` in `main.ts`. Nothing to change. A new spec pins `withCredentials === true` on the /me request so a future refactor can't silently drop the flag and reintroduce the bug. ## Test plan - [x] `pnpm nx test feature-auth` → **9/9 pass** (was 8 before; +1 spec pinning the credentials flag). - [x] `pnpm nx test portal-shell` → **32/32 pass**. - [x] `pnpm nx lint feature-auth portal-shell` → clean. - [x] `pnpm nx build portal-shell` → clean. - [ ] Manual smoke against the running BFF: anonymous landing → click "Sign in" → Entra → callback → SPA lands with avatar + display name in the header (the very last step that failed before this fix). --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #114 |
||
|
julien
|
9a9faf9a31 |
feat(portal-shell): wire SPA auth state to BFF /me + header login/logout widget (#113)
## Summary First user-visible piece of the auth track. The portal-shell SPA now consumes the BFF auth surface (`/api/auth/me`, `/api/auth/login`, `/api/auth/logout`) and the header reflects sign-in state. - `libs/feature/auth` ships an `AuthService` that fetches `/auth/me` on first injection, holds a signal-backed `AuthState` (`loading` / `anonymous` / `authenticated` / `error`) and exposes `currentUser` + `isLoading` computed signals plus `login()` / `logout()` / `refresh()` methods. - The header's right-side widget renders four states: a sign-in button when anonymous, the user avatar (initials, `JD` for "Jane Doe") + display name + sign-out button when authenticated, a loading dot before `/me` resolves, and a "Can't reach the server" chip on non-401 failures. - `login()` / `logout()` go through an injected `AUTH_NAVIGATOR` token whose default calls `window.location.assign(url)`. Specs override it with `vi.fn()` — no `window.location` mocking required. ## Notable choices **Auto-bootstrap on construction, not via `provideAppInitializer`.** The service fires `/me` from its constructor (unawaited) so consuming components transition through the explicit `loading` state. Blocking app boot on the round-trip would push the first paint behind the network call — bad for TTFB, especially on slow links. The header handles `loading` as a first-class state. **Discriminated `AuthState` over flat fields.** A single source of truth (`state()`) with four `kind`s lets templates `switch` and narrow automatically. `currentUser` and `isLoading` are computed conveniences but never out of sync with `state`. **`AUTH_BFF_BASE_URL` + `AUTH_NAVIGATOR` injection tokens.** Decouples the lib from the host's `environment.ts` shape and keeps tests free of `window.location` redefinition (which jsdom resists across multiple specs in the same file — first redefine works, second throws "Cannot redefine property"). The host wires both in `app.config.ts`. **Curated public user type.** `CurrentUser` mirrors the BFF's `/me` response (`oid`, `tid`, `username`, `displayName`) — no `amr`, no internal claims. The shape lives in `auth.types.ts` so feature code can import it without depending on Angular HTTP details. **Distinct `error` state.** A network failure / 5xx surfaces a different UI than "not signed in" — "Can't reach the server" chip vs. sign-in button. Avoids the trap of treating any `/me` failure as "anonymous". ## Out of scope (next PRs) - Route guards (protecting routes from anonymous users). For now the header is the only consumer. - Auto-refresh of the session before idle timeout. - HTTP interceptor that redirects to `/auth/login` on a 401 from any other BFF call. - Per-locale styling polish on the new header strings. ## Test plan - [x] `pnpm nx test feature-auth` → **8/8 pass**. - [x] `pnpm nx test portal-shell` → **32/32 pass** (was 27 before). - [x] `pnpm nx lint portal-shell feature-auth` → clean. - [x] `pnpm nx build portal-shell` → main bundle 488 kB raw / 129.94 kB transfer; well under the 300 kB gzip budget from ADR-0017. - [ ] Manual smoke once the BFF is up: - [ ] Anonymous landing → header shows "Sign in". - [ ] Click "Sign in" → BFF /login → Entra → callback → SPA lands with avatar + display name in the header. - [ ] Click "Sign out" → BFF /logout → Entra logout → back at SPA, header back to "Sign in". --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #113 |
||
 Julien Gautier
|
8de19320c5 |
chore: generate shared and feature libs with module boundaries per ADR-0003
Generate the four phase-4 libraries: - libs/shared/tokens (project name shared-tokens) - plain TS lib via @nx/js:library; will host the a11y design tokens (palette, contrast tiers, spacing, motion) once Tailwind lands in phase 5; consumable by both apps; tagged scope:shared, type:shared. - libs/shared/util (shared-util) - plain TS lib for cross-cutting utility code; tagged scope:shared, type:shared. - libs/shared/ui (shared-ui) - Angular standalone library that will host the spartan-ng components copy-pasted in phase 5; Angular-only so tagged scope:portal-shell, type:shared. unitTestRunner= vitest-analog because vitest-angular requires a buildable lib. - libs/feature/auth (feature-auth) - placeholder Angular standalone feature lib to demonstrate the type:feature pattern; tagged scope:portal-shell, type:feature. @nx/enforce-module-boundaries depConstraints replaced (root eslint.config.mjs) with the rules from ADR-0003: scope:portal-shell -> scope:portal-shell, scope:shared scope:portal-bff -> scope:portal-bff, scope:shared scope:shared -> scope:shared type:app -> type:feature, type:shared type:feature -> type:feature, type:shared type:shared -> type:shared This forbids portal-shell from importing portal-bff code (and vice versa) and prevents shared libs from depending on feature libs. Project names follow the convention of ADR-0003 (feature-<name> / shared-<scope>) by passing --name explicitly to the generator; the Nx 22 default takes only the last directory segment. Sanity check: pnpm nx run-many -t lint and -t test pass for the 8 projects (4 apps/e2e + 4 libs). Side effects from the generators: tsconfig.base.json paths populated with the lib import aliases; nx.json gains vite/playwright plugin entries; .gitignore picks up vitest.config.*.timestamp* (Vitest temp files); package.json gains @analogjs/vitest-angular and related devDeps; pnpm-lock.yaml regenerated. Two eslint-disable comments in portal-bff-e2e support files were trimmed by lint --fix - those files already lint clean without the directive. |