Commit Graph
3 Commits
Author SHA1 Message Date
timothyandClaude Opus 4.8 e8c3481ea5 fix(api): #295 PR1 — fold in fix-commit re-review (2nd Codex round)
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 8s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m24s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 11m9s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Fix-commit re-review confirmed the 1st-round fixes resolved and caught a 2nd round:

- HIGH — env-seed vs. setup race: an attacker could claim admin in the startup
  window before LocalAdminSeedService runs, and the seed's insert would then be
  swallowed (attacker credential persists, defeating env recovery). Fixed
  structurally: the setup-claim endpoint is CLOSED (409) whenever
  Auth:LocalAdmin:Password is configured — the env seed owns the credential, so
  there's no claim to race (also strengthens the setup-claim TOFU posture).
  Config.setupRequired reflects it.
- LOW — a concurrent setup race-loser now returns 409 (not 422); ClaimLocalAdmin's
  DbUpdateException catch re-checks existence and rethrows genuine/transient DB
  errors instead of masking them as "already configured".
- MEDIUM (accepted, documented) — two simultaneous authenticated password changes
  are a non-serializable lost-update; accepted for a single-admin system
  (self-healing via re-login, implausible timing).

+3 AuthController tests (env-seed closes setup / setupRequired gating). Full
ErsatzTV.Tests green (1506); no generated drift. Docs updated.

Refs #295

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 16:52:23 +02:00
timothyandClaude Opus 4.8 6ac5150fd0 fix(api): #295 PR1 — fold in cold-fork + Codex review findings
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 6s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m22s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m17s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Independent review (cold fork = MERGEABLE-WITH-NITS; Codex = BLOCKED, caught
concurrency defects the fork missed). All actionable findings folded in:

- HIGH (Codex) atomic first-claim-wins: ClaimLocalAdmin now writes the three
  credential rows in ONE transaction guarded by the unique ConfigElement.Key
  index (lost race -> DbUpdateException -> 409), so concurrent claims can't
  produce a mixed-state credential.
- HIGH (Codex) consistent login snapshot: VerifyLocalAdminLogin reads hash+stamp
  in one query and drops rehash-on-verify, so a login racing a password change
  can't capture a stamp newer than the hash it verified (concurrent change ->
  old password fails, or the issued cookie carries the pre-change stamp ->
  revoked next request).
- MEDIUM (Codex) env-seed migration race: LocalAdminSeedService is now a RunOnce
  BackgroundService that awaits SystemStartup.WaitForDatabase (the migrator is a
  BackgroundService; registration order didn't guarantee the schema) + try/catch.
- MEDIUM (fork M1) ForwardedHeaders: reverted the strict-opt-in flip — it would
  regress /iptv M3U/XMLTV/HLS absolute-URL generation (Request.Scheme) behind a
  proxy without KnownProxies. Kept #285 behavior; KnownProxies still recommended.
- LOW (Codex/fork) require X-CSRF on /api/auth/logout + /password (the
  [SkipApiAuthorization] surface isn't covered by the filter's CSRF check;
  closes forced-logout CSRF).
- ChangeLocalAdminPassword also writes hash+stamp atomically. Input length caps
  on username/password.

Deferred with a tracked gate: MEDIUM (Codex) side-effecting [RequiresAuthentication]
GETs (troubleshoot playback/archive) aren't CSRF-covered -> #301, gates PR2
(latent in PR1: the SPA still uses the machine key).

Verify: full ErsatzTV.Tests green (1501); no OpenAPI/generated drift. Docs updated
(api-conventions §9, decisions.md).

Refs #295 #301

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 16:40:25 +02:00
timothyandClaude Opus 4.8 0b23d4b6b1 feat(api): #295 PR1 — browser SPA session auth (session-OR-key gate, server-only)
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m28s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m42s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Implements the ratified #295 design (PR1, server-only, backward compatible). The
/api surface now accepts a valid X-Api-Key (machine) OR an authenticated session
(browser cookie, local login or OIDC), gated by the evolved ApiAuthorizationFilter
(renamed from ApiKeyAuthorizationFilter; same fail-closed EndpointRequiresKey
predicate). Machine/key behavior is byte-identical and the SPA keeps working via
its stored key — the SPA login flow lands in PR2.

- ApiAuthorizationFilter: key-first (CSRF-immune) then session; session-authed
  mutations require the X-CSRF header (403 otherwise). Attributes renamed
  [RequiresApiKey]->[RequiresAuthentication], [SkipApiKeyAuthorization]->[SkipApiAuthorization].
- Cookie scheme ctv-session always registered (Lax/SameAsRequest/14d sliding, 401 not
  redirect for /api); OIDC handler revived when configured (profile scope, userinfo,
  auth-method claim); UseAuthentication/UseAuthorization/UseRateLimiter revived in the
  legacy MapWhen branch.
- Local admin = single credential in ConfigElement rows (username / PBKDF2 hash via
  Microsoft.Extensions.Identity.Core / rotating security stamp) — NO DB migration.
  Password change rotates the stamp; CookieSecurityStampValidator revokes stale local
  sessions. Env-seed recovery (Auth:LocalAdmin:*) via LocalAdminSeedService.
- AuthController /api/auth/{config,session,setup,login,logout,password} + browser-nav
  GET /auth/oidc/login; excluded from OpenAPI (machine-audience spec). Per-IP rate limit
  on login/setup/password; dummy-hash verify (no user enumeration).
- ForwardedHeaders now strict opt-in: X-Forwarded-* ignored unless KnownProxies/Networks
  configured (rate-limiter IP + cookie-Secure integrity). Deployment: operators behind a
  proxy must set ForwardedHeaders:KnownProxies.
- Tests: session/CSRF filter cases + 17 Application/Auth handler tests; full ErsatzTV.Tests
  green (1499). No OpenAPI/generated-artifact drift.
- Docs: api-conventions section 9 rewritten; decisions.md entry (supersedes #206 inert-OIDC note).

Refs #295 #197 #206 #58

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 16:14:54 +02:00