timothy/ersatztv
1
0
Fork 0
Code Issues 57 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

#295 PR2: SPA session cutover + #301 side-effecting-GET POST-ification #316

Merged
timothy merged 9 commits from feat/295-spa-auth into main 2026-07-12 22:04:54 +02:00
Conversation 2 Commits 9 Files Changed 46
+2820 -843
timothy commented 2026-07-12 20:46:31 +02:00
Owner
Copy Link
Copy Source

Fixes #295 (PR2 — the SPA cutover) and #301 (side-effecting-GET CSRF hole).

PR1 shipped the entire server auth surface (session-OR-key filter, AuthController, local-admin store, OIDC, cookie). This PR moves the browser onto the session and closes the #301 CSRF gap.

What changed

SPA is cookie-only; machine key is external/MCP-only

  • web/src/api/client.ts no longer sends X-Api-Key; it relies on the same-origin session cookie and sets X-Csrf on every mutating verb centrally.
  • New boot gate web/src/AuthGate.tsx (wraps <App/> in main.tsx, not a route → no parity rows): GET /api/auth/config → Setup (first-run local-admin claim) / GET /api/auth/session → Login (local form + OIDC "Sign in with SSO" when enabled) / the app. Publishes AuthContext.
  • ApiKeyScreen repurposed to machine-key management — reads the key from the new GET /api/auth/machine-key (session-gated), masked with Reveal + Copy; plus a local-admin password-change card. Logout via a UserMenu in the TopBar.
  • 401 signal now drives re-login (passive banner, consults the nav guard); login/change-password suppress it for inline handling.

#301 — POST-ify the side-effecting GETs (so the filter's session-mutation CSRF gate covers them, no new machinery)

  • GET /api/troubleshoot/playback.m3u8 → POST /api/troubleshoot/playback/start returning 200 { url } (the open /iptv manifest the player loads — no header injection into hls.js/native-HLS needed).
  • archive/sample GETs → POST (SPA downloads via a fetch-blob helper, never window.open).
  • Removing the HEAD variants also fixed a latent DeleteOnClose-on-HEAD artifact-destruction bug.
  • Standing rule added: never add a side-effecting GET/HEAD under /api.

Verification

  • C# ErsatzTV.Tests: 1529/1529 green. Web: 731/731 green, lint + build + check:api clean. OpenAPI regenerated (v1.json/v1.d.ts/endpoint-index.md).
  • Two integration bugs caught + fixed during assembly: the MachineKey wire field ({ apiKey } not { key }, mocked unit tests missed it) and stale OpenApiErrorResponseContractTests cases pointing at the removed route.

Docs (same PR)

spa-conventions.md §5e rewritten (session-auth seams), api-conventions.md §9 (#301 resolved + standing rule + machine-key endpoint), decisions.md new entry, e2e-local.md (fail-closed seeding + browser boot-gate flow). No blazor-route-parity.md/domain-model.md rows (login/setup mint no routes).

Pending before merge

  • Live E2E (setup-claim → login → machine-key → troubleshoot playback POST → archive download → logout)
  • Adversarial review (fork + Codex) — security-critical; Review-verdict: … @ <head> to follow.

🤖 Generated with Claude Code

Fixes #295 (PR2 — the SPA cutover) and #301 (side-effecting-GET CSRF hole). PR1 shipped the entire server auth surface (session-OR-key filter, `AuthController`, local-admin store, OIDC, cookie). **This PR moves the browser onto the session** and closes the #301 CSRF gap. ## What changed **SPA is cookie-only; machine key is external/MCP-only** - `web/src/api/client.ts` no longer sends `X-Api-Key`; it relies on the same-origin session cookie and sets `X-Csrf` on every mutating verb centrally. - New **boot gate** `web/src/AuthGate.tsx` (wraps `<App/>` in `main.tsx`, **not** a route → no parity rows): `GET /api/auth/config` → Setup (first-run local-admin claim) / `GET /api/auth/session` → Login (local form + OIDC "Sign in with SSO" when enabled) / the app. Publishes `AuthContext`. - `ApiKeyScreen` repurposed to **machine-key management** — reads the key from the new `GET /api/auth/machine-key` (session-gated), masked with Reveal + Copy; plus a local-admin password-change card. Logout via a `UserMenu` in the TopBar. - 401 signal now drives re-login (passive banner, consults the nav guard); login/change-password suppress it for inline handling. **#301 — POST-ify the side-effecting GETs** (so the filter's session-mutation CSRF gate covers them, no new machinery) - `GET /api/troubleshoot/playback.m3u8` → **`POST /api/troubleshoot/playback/start`** returning `200 { url }` (the open `/iptv` manifest the player loads — no header injection into hls.js/native-HLS needed). - archive/sample GETs → **POST** (SPA downloads via a fetch-blob helper, never `window.open`). - Removing the HEAD variants also fixed a latent `DeleteOnClose`-on-HEAD artifact-destruction bug. - Standing rule added: never add a side-effecting GET/HEAD under `/api`. ## Verification - C# `ErsatzTV.Tests`: **1529/1529** green. Web: **731/731** green, lint + build + `check:api` clean. OpenAPI regenerated (`v1.json`/`v1.d.ts`/`endpoint-index.md`). - Two integration bugs caught + fixed during assembly: the `MachineKey` wire field (`{ apiKey }` not `{ key }`, mocked unit tests missed it) and stale `OpenApiErrorResponseContractTests` cases pointing at the removed route. ## Docs (same PR) `spa-conventions.md §5e` rewritten (session-auth seams), `api-conventions.md §9` (#301 resolved + standing rule + machine-key endpoint), `decisions.md` new entry, `e2e-local.md` (fail-closed seeding + browser boot-gate flow). No `blazor-route-parity.md`/`domain-model.md` rows (login/setup mint no routes). ## Pending before merge - [ ] Live E2E (setup-claim → login → machine-key → troubleshoot playback POST → archive download → logout) - [ ] Adversarial review (fork + Codex) — security-critical; `Review-verdict: … @ <head>` to follow. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
timothy force-pushed feat/295-spa-auth from e433332428 to 1c3b4ad264 2026-07-12 20:50:38 +02:00 Compare
timothy force-pushed feat/295-spa-auth from 1c3b4ad264 to 071f339a86 2026-07-12 21:28:02 +02:00 Compare
timothy commented 2026-07-12 21:46:24 +02:00
Author
Owner
Copy Link
Copy Source

Adversarial review — complete

Review loop (security gate, so both fork + Codex per rotation, reconciled by Fable, then a fix-commit re-review per the #242 discipline):

  • Fork (initial diff) → MERGEABLE-WITH-NITS (verified CSRF completeness on Troubleshoot/ScriptedSchedule, machine-key gating, boot-gate lockout).
  • Codex (initial diff) → BLOCKED, 4 MAJOR + 1 MINOR — critically caught a side-effecting GET the fork missed (GET /api/graphics-elements?refresh=true writes to the DB), plus lock-race→422-not-409, a fast-completion screen race, machine-key response missing no-store, and an AuthSession type-vs-wire mismatch.
  • Fable reconciliation → ratified Codex's BLOCKED; prescribed the fix plan (rejected 1 fork nit as non-defect, deferred 1 pre-existing to #320).
  • Fixes landed (aa86fb6f backend + 071f339a web): graphics-refresh POST-ified (GET now pure-read), typed LockedError→409, onPlay phase-promotion on POST-200, Cache-Control: no-store on the machine key, AuthSession optional fields. Plus a CI-gate fix (2e056a12): update-openapi.sh now builds before the doc-gen target (the new #303 api-docs gate hit a latent deps.json failure — this PR was the first to change the /api surface and exercise it).
  • Fork re-review (fix commits) → MERGEABLE, all 5 findings verified CLOSED in code; one latent cosmetic nit accepted (hardcoded 409 title on the generic LockedError, single call site).

Live E2E (fresh instance, headless): 5/5 flows PASS — setup-claim → login → machine-key screen (revealed key == $CONFIG/api.key, no-store present) → CSRF gating (403 without X-CSRF, 204 with) → old GET playback.m3u8 → 404 → logout/lockout safety.

CI: all 7 checks green on 2e056a12.

Review-verdict: MERGEABLE @ 2e056a12c1d8c7f45ecae44cd6de9655a4eef29b

## Adversarial review — complete Review loop (security gate, so both fork + Codex per rotation, reconciled by Fable, then a fix-commit re-review per the #242 discipline): - **Fork (initial diff)** → MERGEABLE-WITH-NITS (verified CSRF completeness on Troubleshoot/ScriptedSchedule, machine-key gating, boot-gate lockout). - **Codex (initial diff)** → BLOCKED, 4 MAJOR + 1 MINOR — critically caught a side-effecting GET the fork missed (`GET /api/graphics-elements?refresh=true` writes to the DB), plus lock-race→422-not-409, a fast-completion screen race, machine-key response missing `no-store`, and an `AuthSession` type-vs-wire mismatch. - **Fable reconciliation** → ratified Codex's BLOCKED; prescribed the fix plan (rejected 1 fork nit as non-defect, deferred 1 pre-existing to #320). - **Fixes landed** (`aa86fb6f` backend + `071f339a` web): graphics-refresh POST-ified (GET now pure-read), typed `LockedError`→409, `onPlay` phase-promotion on POST-200, `Cache-Control: no-store` on the machine key, `AuthSession` optional fields. Plus a CI-gate fix (`2e056a12`): `update-openapi.sh` now builds before the doc-gen target (the new #303 api-docs gate hit a latent `deps.json` failure — this PR was the first to change the `/api` surface and exercise it). - **Fork re-review (fix commits)** → **MERGEABLE**, all 5 findings verified CLOSED in code; one latent cosmetic nit accepted (hardcoded 409 title on the generic `LockedError`, single call site). **Live E2E** (fresh instance, headless): 5/5 flows PASS — setup-claim → login → machine-key screen (revealed key == `$CONFIG/api.key`, `no-store` present) → CSRF gating (403 without `X-CSRF`, 204 with) → old `GET playback.m3u8` → 404 → logout/lockout safety. **CI**: all 7 checks green on `2e056a12`. Review-verdict: MERGEABLE @ 2e056a12c1d8c7f45ecae44cd6de9655a4eef29b
timothy force-pushed feat/295-spa-auth from 2e056a12c1 to be9cf5f381 2026-07-12 21:51:27 +02:00 Compare
timothy commented 2026-07-12 21:59:47 +02:00
Author
Owner
Copy Link
Copy Source

Re-anchoring the verdict after rebasing onto the latest main (past #315 + the parallel-session CI chores). The rebase is content-identical to the reviewed 2e056a12 — main's additions were disjoint (docs/scripts/migration-smoke); no code or reviewed surface changed. Full CI is green on the new head.

Review-verdict: MERGEABLE @ be9cf5f381

Re-anchoring the verdict after rebasing onto the latest `main` (past #315 + the parallel-session CI chores). The rebase is **content-identical** to the reviewed `2e056a12` — main's additions were disjoint (docs/scripts/migration-smoke); no code or reviewed surface changed. Full CI is green on the new head. Review-verdict: MERGEABLE @ be9cf5f381755b93e3efd04b51b70e1e57314858
timothy merged commit fa655053ba into main 2026-07-12 22:04:54 +02:00
timothy deleted branch feat/295-spa-auth 2026-07-12 22:04:55 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#316