8 Commits

Author	SHA1	Message	Date
Dragan Spiridonov	56327d0931	decide: browser sessions are read-only permanently; drop the dead step-up client ... Decision: browser-side admin is not wanted. `BROWSER_SIGNIN_SCOPE` stays `sensing:read`, and the escalate-on-demand design sketched while this was open is not being built. Destructive operations — training, model delete, recording delete — keep their home in the CLI, where `--admin` is explicit and typed by a person. Routing them through a browser would mean either asking every user to consent to delete capability in order to watch a stream, or building a second consent flow to avoid that. Consequences, now settled rather than open: - The UI's admin controls are unreachable from a Cognitum browser session, and that is intended. The manual token-paste field is unchanged and still carries whatever authority the pasted token has, so nothing that worked before stops working. - REMOVED the client-side step-up redirect from ui/services/api.service.js. It caught an RFC 6750 challenge that can never be issued to a browser, and it ended in `return new Promise(() => {})` — so if any other 401 had ever grown that header, every caller awaiting it would have hung forever with no error and no timeout. Dead code with a trap in it is worse than no code. - KEPT ADMIN_REVERIFY_SECS as a server-side backstop. Fail-closed and free, so if the requested scope is ever widened the freshness requirement is already in place. Documented at its definition as a backstop specifically so nobody reads its passing tests as evidence the control is exercised — the tests reach it through a crate-internal seam that mints an admin cookie the real flow does not produce. ADR-271 also stops hedging on the session TTL: chosen is A at one hour. Option B (server-side refresh-token store) is not built, and the ADR now names the residual instead of implying it is closed — within one hour a revoked Cognitum grant still reads sensing data through an existing browser session. There is no introspection endpoint, so nothing short of B closes that, and one hour is the size of the hole we accepted. Adds `the_cookie_max_age_matches_the_session_expiry`, which the previous ADR revision asked for and nobody had written: Max-Age and the payload's `exp` are two independent expressions of one lifetime, and drift means either the browser presents a session we reject or we hold authority the browser discarded. Verified: workspace 176 suites clean, UI 22, api.service.js parses. Co-Authored-By: Ruflo & AQE	2026-07-23 14:47:31 +02:00
Dragan Spiridonov	1ed0bc57ef	docs(adr): correct a false claim about browser scope; pin the decision in code ... A cross-vendor pre-merge sweep found no merge blockers, but did surface an error in ADR-271 that I wrote — and a coherence gap behind it. ADR-271 P2 stated that capping the browser session at `sensing:read` was "considered and rejected, because the dashboard genuinely performs admin operations". That is wrong. `/oauth/start` (main.rs:9206) already requests `SENSING_READ` and nothing else, deliberately, with a comment saying so. The browser session is ALREADY read-only, so the breakage I claimed capping would cause is simply the current behaviour. Two consequences, now stated in the ADR instead of left to be discovered: 1. The UI's admin controls do not work from a browser OAuth session. `model.service.js:136` issues DELETE /api/v1/models/{id}, which 401s. Admin work needs the CLI (`login --admin`) or a pasted admin bearer. A gap in a new feature, not a regression — the token-paste path is unchanged. 2. The ADMIN_REVERIFY_SECS step-up control added in the previous commit guards a case that cannot currently arise. No browser session holds `sensing:admin`, so the freshness branch never fires in production. Its tests pass because the crate-internal seam mints an admin cookie the real flow never produces. That second point is worth being blunt about: it is the same shape as several defects this branch already fixed — correct code, green tests, unreachable call site. The difference is that here the guard is deliberately ahead of the need rather than mistakenly behind it, and saying which one it is matters. So the constant is now named `BROWSER_SIGNIN_SCOPE` rather than inlined, with the cost of widening it documented at the definition, and two tests: `browser_sign_in_stays_read_only_until_someone_decides_otherwise` pins the value, and `the_authorize_url_actually_carries_that_scope` proves it reaches the wire — asserting on the constant alone would pass even if `begin` were called with something else, which is exactly the isolation failure being guarded against. The ADR also records the coherent way to add browser-side admin if wanted: escalate-on-demand via the RFC 6750 challenge, keeping least privilege by default rather than asking every user to consent to delete capability to watch a stream. Not bundled here — it needs a scope parameter and a UI affordance. Sweep verdict: no merge blockers. Two other non-blocking risks it raised are accurate and unchanged: concurrent JWKS refresh at the stale boundary is not atomic (a duplicated idempotent GET, already documented as an accepted cost), and the service worker's SHELL_ASSETS use root-relative paths while the UI mounts under /ui, so offline shell precaching is incomplete — pre-existing, unrelated to this branch. Verified: workspace 176 suites clean. Co-Authored-By: Ruflo & AQE	2026-07-23 14:31:01 +02:00
Dragan Spiridonov	89cceaf835	fix(auth): close P1/P2/P3 — JWKS stall, 12h session, cookie shadowing ... All three deferred findings from the qe-court round. Each fix is guarded by a test confirmed to FAIL against the old behaviour. P1 — JWKS: self-inflicted stall, and a blocking fetch on a tokio worker. `fetched_at` advances only on SUCCESS, and the only rate limiter sat behind `if fresh`. So once the TTL elapsed after the last successful fetch, `fresh` was permanently false, the limiter was never consulted, and EVERY request performed its own blocking 3s-timeout fetch. A Pi that loses WAN stalled itself 300s later with no attacker present; an attacker could force the same state by flooding tokens with an unknown `kid`. Now: `last_attempt_at` is recorded BEFORE every fetch regardless of outcome, and gates the stale path too; a stale-but-present key is served rather than erroring, which is the offline tolerance this module always claimed. Measured by the new test: 26 outbound fetches before, 1 after. Kept as TWO independent limiters. Merging them looks tidy and is wrong — a routine refetch would then suppress the unknown-`kid` path for 30s and delay pickup of a key rotation inside the TTL. I made that mistake first; two existing tests caught it. The blocking call also now runs in `spawn_blocking` at the verify boundary, matching what `main.rs` already does for the token exchange, where the comment reads "the same mistake this codebase had to fix in jwks.rs". The hot verification path had never been given the same treatment. A panicked task fails closed. P2 — session lifetime, per decision: 1 hour, plus step-up. SESSION_TTL_SECS 12h -> 1h, and privileged (`sensing:admin`) actions now require the user to have authenticated within ADMIN_REVERIFY_SECS (5 min), tracked by a new `auth_time` claim. Reads ride the full session; only the routes where a stale session does damage are re-verified, so a dashboard whose main use is watching a live stream does not re-auth hourly. `auth_time` is `#[serde(default)]`, so a cookie issued before the field existed reads as 0 — infinitely stale. Such a session keeps working for reads and cannot perform privileged actions. Fail-closed and self-healing on next sign-in. The refusal carries an RFC 6750 `WWW-Authenticate` error code, because the client's correct response differs from a plain 401: the user IS signed in and needs to prove it again. `api.service.js` acts on that and redirects through `/oauth/start` — otherwise a stale-session delete surfaces as a generic "Request failed" with no hint that signing in again fixes it. P3 — cookie shadowing. `read_cookie` returned the FIRST match, and RFC 6265 §5.4 sends longer-`Path` cookies first. Cookies are not isolated by port or scheme, so any other service on the host — or a plain-HTTP MITM injecting Set-Cookie — could plant `ruview_session=<their own validly signed session>; Path=/ui`. The victim sent both, the attacker's first, and it verified because it genuinely was signed: silent session takeover, with `/oauth/status` reporting the attacker's account. The signature was doing its job throughout, which is why "it's signed" never answered this. `__Host-` would, but requires `Secure`, and RuView is routinely reached over plain HTTP on a LAN. So both credential paths now accept only when EXACTLY ONE candidate verifies. An attacker can still cause a refusal by planting a second valid cookie — a nuisance — but no longer a takeover. Planting junk changes nothing, so this does not become a trivial DoS. Tests: +1 jwks (26-vs-1 fetch amplification), +4 step-up, +4 shadowing, +1 duplicate-name reader. Mutation-verified: reverting the stale-path guard gives 26 fetches; reverting to first-match cookie reads fails `a_shadowing_cookie_cannot_silently_take_over_the_session`. Verified: workspace 176 suites clean under CI flags, ruview-auth 62+25+2 with --all-features, UI 22. ADR-271: P1/P2/P3 marked RESOLVED with the analysis retained, since it explains why each fix has the shape it does. Co-Authored-By: Ruflo & AQE	2026-07-23 13:13:42 +02:00
Dragan Spiridonov	6ce50d5158	test(auth): cover browser sign-in; ADR-271/272 corrections and remediation plans ... Browser sign-in was the newest security surface in this PR and had no executable evidence behind it: browser_session.rs was 534 lines with 13 tests, every one of which hit a private helper (sign, unsign, cookie, read_cookie, is_live, has_scope). No test called issue, from_cookie_header, begin, verifier_for_callback or is_configured, and no test anywhere presented a session cookie to the gate. +10 tests in browser_session, +6 in bearer_auth. Three mutants the adversarial review named, each now verified dead by actually applying the mutation: (a) delete the `state` comparison in verifier_for_callback -> a_callback_whose_state_does_not_match_is_refused FAILED Without it the callback accepts a code from a flow the user never started: login CSRF, victim silently lands in the attacker's session. (b) `session.is_live().then_some(session)` -> `Some(session)` -> an_expired_session_cookie_does_not_authenticate FAILED -> an_expired_browser_session_is_refused FAILED `is_live` was already unit-tested; nothing asserted the CALLER consults it. Same "tested in isolation, call site untested" shape as the earlier refresh-never-invoked defect. (c) `session.has_scope(required)` -> `true` -> a_read_scoped_browser_session_cannot_delete_or_train FAILED Without it any browser session could delete models and start training. Mutant (c) initially appeared to SURVIVE. It did not — there are two has_scope call sites and the first substitution only hit one. Mutating the one in `session_or_unauthorized` kills the test. That accident confirmed a separate finding: the cookie branch inside require_bearer is unreachable when an Authorization header is present, because the OAuth step returns on both arms. It fails closed, so it is not a hole, but "try the next credential" is what the code reads like. Pinned by a_bad_bearer_beats_a_good_cookie_rather_than_falling_back. Adds two crate-internal test seams (init_secret_for_tests, test_cookie_value). test_cookie_value signs through the same path as `issue`, so tests presenting a cookie exercise real verification rather than a test-only bypass. ADR-271: - The "browser cannot obtain an OAuth token" section asserted `grep -ril "oauth|cognitum|pkce" ui/` returns nothing. It now returns three files, invalidated by commits in this same PR. Marked superseded, original retained under a fold, replaced with what actually ships. - Records the two deferred decisions with designs rather than patches: P1 the blocking JWKS fetch on a tokio worker (whose rate limiter is bypassed on exactly the stale path that matters, because fetched_at updates only on success — so after the TTL every request fetches, and a Pi that loses WAN stalls itself with no attacker present); P2 the 12-hour session from a 15-minute token, with three costed options. Capping the session to sensing:read was considered and rejected: the dashboard genuinely issues DELETE /api/v1/models/{id}. - P3 records that dropping `__Host-` costs origin-integrity, not just Secure — read_cookie takes the FIRST match and cookies are not port-scoped, so a same-host writer can shadow a session. Forgery was never the threat that prefix addresses. - Documents redirect_uri's hardcoded default and the unconsumed CLI credential as known-incomplete, per decision to leave both as-is. ADR-272: corrected a claim that would mislead users into a 401. It stated the Python client DOES send Authorization: Bearer on the handshake; ws.py passes no headers at all (zero occurrences of extra_headers or Authorization), so every published client 401s once auth is enabled. Server-side decision unchanged. Verified: sensing-server 566 + 179 + 7 + 5 + 8 + 4 + 16 pass under CI flags, ruview-auth 61 + 25 + 2 with --all-features, auth_wiring 7. Co-Authored-By: Ruflo & AQE	2026-07-23 12:08:13 +02:00
Dragan Spiridonov	c72bbc15dd	fix(auth): close /api/field bypass, two fail-opens, and a self-disarming test ... Findings from a qe-court adversarial round (4 prosecutors across 2 vendors). Each was verified against the code before being accepted; the ones below reproduced, the rest are reported in the PR thread rather than acted on. FATAL — `/api/field` was reachable with no credential, on both listeners. The gate protected `/api/v1/*` by prefix. `/api/field` is the REST sibling of `/ws/field` and serves the same signed FieldEvent stream — live presence, pose, vitals. `/ws/field` was gated in this PR; its twin one path segment over was not. Measured with RUVIEW_API_TOKEN set and no credential supplied: /api/v1/models 401 (control) /ws/field 401 (gated by this PR) /api/field 200 on :8080 AND :8765 Fixed by inverting the gate to deny-by-default with an explicit anonymous allowlist (`/`, `/ui`, `/health`, `/oauth/`). A route added at a new path is now gated because nobody exposed it, rather than exposed because nobody protected it — the same inversion already applied to the scope gate. FATAL — the wiring test disarmed itself exactly when it mattered. `Server::start` returned an Option that all five tests turned into `return`, so a server that failed to boot produced "5 passed" with zero assertions run, and cargo swallows the skip line without --nocapture. The one test that observes real wiring — the guard against both shipped bypasses — was silent for any change that breaks startup, including a boot panic in the auth path. It now panics with the child's stderr. MAJOR — a malformed client-id list silently disabled the audience check. An empty allowlist is the opt-out sentinel in verify.rs. `RUVIEW_OAUTH_CLIENT_IDS=","` is non-empty, passes the guard, then filters to an empty Vec — turning the audience boundary off with no log and admitting a token minted for any other Cognitum product. Only a literal `*` may opt out now; anything else that parses to nothing warns and falls back to the default. Same fail-open shape as the scope denylist this PR already had to invert. MAJOR — credentials were world-readable for a window on every refresh. `fs::write` creates at 0666 & !umask (0644 by default), and both writers chmodded afterwards. The existing permissions test asserted on the FINAL file and passed throughout. Affected the CLI refresh token (rotated with reuse detection — a thief who presents it first takes the session family) and the browser session secret (the HMAC key for every session; stealing it forges any account at any scope). Both now create with mode 0600 via OpenOptions. MAJOR — ui/sw.js cached authenticated API responses. Closing the /oauth/ leg left the /api/ leg open. `networkFirst` cached every successful response, keyed by URL alone, purged by nothing at sign-out: sign in as A, load sensing data, sign out, sign in as B, lose the network, and B is served A's data with no authorization check. API responses are now network-only — which is also the correct behaviour for a live sensing dashboard, where replaying a stale reading can show a room occupied after the person left — plus a cache purge on sign-out. CI — 40 of ruview-auth's 87 tests never ran. The workspace runs --no-default-features, which switches off the `login` and `pkce` features. Measured: 47 tests vs 87. The whole interactive sign-in path — credential storage, single-flight refresh, the file lock, the loopback callback — was green locally and never executed in CI. Added an --all-features step. (Checked the sensing-server for the same problem and did NOT find it: bearer_auth's 49 and browser_session's 15 do run under CI flags.) Tests: +2 wiring tests (one fails against the old gate with "http port served /api/field to an anonymous caller", passes after), +3 UI service-worker tests, +3 CLI scope tests, +1 temp-file permission test, +1 client-id parsing test. wifi-densepose-cli/src/auth.rs had zero tests and builds its own scope string, so the library's least-privilege test said nothing about what the CLI requests. Verified: ruview-auth 61+25+2 pass (--all-features), sensing-server bearer_auth 50, browser_session 15, auth_wiring 7, workspace 25 suites clean, UI 22. Co-Authored-By: Ruflo & AQE	2026-07-23 11:48:59 +02:00
Dragan Spiridonov	43737941cb	fix(auth): two Set-Cookie headers were collapsing into one; clear the spent transaction ... Two bugs, one of which I introduced while fixing the other and caught only by checking the actual response. 1. THE SPENT TRANSACTION COOKIE WAS NEVER CLEARED ON SUCCESS. `clear_transaction` was only used on error paths, so after a successful sign-in `ruview_oauth_txn` lingered for its full 10-minute TTL and every subsequent request carried a dead cookie. Visible in the logs as `names=ruview_oauth_txn,ruview_session`. Now cleared alongside issuing the session, and on logout too. 2. THE FIX FOR (1) BROKE SIGN-IN, AND THE TEST CAUGHT IT. Axum's array-of-tuples response form REPLACES same-name headers rather than appending. Adding a second `Set-Cookie` silently overwrote the first, so the logout response emitted only the transaction clear and — had this shipped — the CALLBACK would have emitted only the transaction clear too, dropping the session cookie and making a successful OAuth round-trip a no-op. Caught by looking at the real response headers rather than trusting the change: `curl -D-` showed one Set-Cookie where there should have been two. Now uses `axum::response::AppendHeaders`. Both cookies verified present. Two tests pin this: one DOCUMENTS the footgun (the array form collapses two Set-Cookie headers into one) and one asserts AppendHeaders emits both. The first exists so the next person to reach for the tidier-looking array form finds out here instead of in production. Also adds a cache-busting query to both redirect targets (`/ui/?signed_in=<ms>` and `?signed_out=<ms>`). Landing on the same URL let the browser restore the page from the back/forward cache with a stale panel, which is why signing in appeared not to work until a hard refresh — the session was established correctly every time, the page simply was not re-fetched. Tests: 549 sensing-server lib. Co-Authored-By: Ruflo & AQE	2026-07-23 10:46:43 +02:00
Dragan Spiridonov	9299a3b137	feat(ui): Sign in with Cognitum, zero-config session secret, executed JS tests ... Three loose ends from the browser sign-in work. --- 1. The last mile: a button --- The server endpoints existed but nothing in ui/ linked to them, so the feature was unreachable. QuickSettings gains a "Cognitum Account" panel that renders from `GET /oauth/status` and offers Sign in / Sign out. `/oauth/status` is a new endpoint and is deliberately UNGATED — a signed-OUT browser cannot ask a gated API whether sign-in is available. It returns only capability flags and, when a session exists, who it belongs to. Never a credential. The panel distinguishes four states rather than showing a button that might 404: signed in; sign-in available; auth on but OAuth off (points at the existing static-token panel); no auth required. A 404 from /oauth/status means a server predating this work and says so plainly. Sign-in is a full-page navigation, not fetch(): the server replies 302 and the browser must follow it carrying the transaction cookie. An XHR would follow the redirect invisibly and land nowhere. --- 2. RUVIEW_SESSION_SECRET no longer required --- Previously `/oauth/start` returned 503 unless an operator invented a secret — a footgun, since they set RUVIEW_OAUTH_ISSUER, expect sign-in, and get a 503 naming an env var they have never heard of. Now resolved in order: env var, then `<data_dir>/session-secret`, then generate one and persist it 0600 (temp file, chmod before rename — the discipline used for the CLI's credentials). Persisted rather than in-memory so a restart does not silently sign everyone out. The env var still wins, which is what a multi-instance deployment needs: several servers must share a secret or a session issued by one is rejected by the next. If the file cannot be written we log the reason and continue with an in-memory secret rather than refusing sign-in outright. Verified with NO configuration at all: secret generated, file mode 0600, /oauth/start returns 302, /oauth/status reports browser_signin: true. --- 3. The JavaScript is now executed by tests --- `ui/services/ws-ticket.test.mjs`, 9 tests, node:test built-in — no new dependency and no package.json needed. Run: `node --test ui/services/`. Covers: no token means no fetch and an unchanged URL; the bearer reaches the Authorization header and NEVER the URL; `?` vs `&` when a query already exists; ticket URL-encoding; 404 treated as a pre-ADR-272 server (the property that lets one UI work against old and new servers, and therefore lets the legacy escape hatch be removed later); 503 and network failure swallowed rather than breaking the connect path; and that a fresh ticket is minted per call, since tickets are single-use and caching one fails on the second reconnect. STATED PRECISELY, because the distinction matters: this EXECUTES the module in Node with stubbed fetch/localStorage. It is more than the `node --check` it replaces and less than a browser — no real WebSocket upgrade, no real cookies, no page wiring. "The UI JavaScript has never been run" is no longer true of this module. "Browser-tested" still is not. Tests: 547 sensing-server lib, 5 wiring integration, 87 ruview-auth, 9 JS. Co-Authored-By: Ruflo & AQE	2026-07-23 10:07:49 +02:00
Dragan Spiridonov	347698b67c	feat(auth): browser sign-in — /oauth/start, /oauth/callback, session cookie ... Closes the gap adversarial review found: `wifi-densepose login` writes ~/.ruview/credentials.json, which a BROWSER CANNOT READ. The UI therefore had no way to obtain a Cognitum token at all, and the WebSocket ticket mechanism ADR-272 built "for browsers" was only exercisable with the legacy static shared secret OAuth was meant to replace. The ADRs described a browser story that did not exist. Ported from cognitum-one/freetokens (src/auth/oauth.ts, live at freetokens.cognitum.one), whose shape is not the obvious one and is the whole point: THE BROWSER NEVER HOLDS AN OAUTH TOKEN. The server generates the PKCE verifier and state, keeps them in an HMAC-signed cookie, performs the code exchange itself, verifies the token, and issues its OWN session cookie carrying an assertion — subject, account, scope, expiry — not a credential. So the access token cannot be read by an XSS, cannot sit in localStorage, and cannot leak through a URL. A stolen session cookie is useless against Cognitum or any sibling service. GET /oauth/start -> 302 to auth.cognitum.one + signed transaction cookie GET /oauth/callback -> constant-time state check, exchange, verify, session GET /oauth/logout -> clears the local session (not the Cognitum session) Verified against the running binary: /oauth/start returns the same 302 + HttpOnly/SameSite=Lax/Max-Age=600 shape freetokens does live; a forged `state` is refused 400; a forged session cookie is refused 401. DELIBERATE DEVIATION from freetokens: no `__Host-` cookie prefix. That prefix REQUIRES `Secure`, and RuView is routinely reached at http://localhost or over plain HTTP on a LAN, where such a cookie is never sent and sign-in would fail silently. `Secure` is set only when the request actually arrived over TLS (direct or via x-forwarded-proto). Every other attribute matches; the HMAC is what protects the value. Other decisions worth stating: - The callback verifies through the SAME `verify_access_token` every other request uses — signature, audience (client_id), typ, expiry, scope. A sign-in path must not be a softer path. - The session cookie is checked LAST in the middleware, after bearer and ticket: it is the weakest-bound credential, so a presented bearer should win. - A browser session requests `sensing:read` only. Admin work goes through the CLI's explicit `--admin`. - The token exchange runs in `spawn_blocking` — `ureq` is blocking, and parking an async worker is the mistake this codebase just had to fix in jwks.rs. - `/oauth/*` sits outside `/api/v1/*` on purpose: gating the routes you use to obtain a credential would deadlock. PKCE moved out from behind the `login` feature into its own light `pkce` feature (rand + sha2 + base64, no HTTP stack), so the server can build an authorize URL without pulling in the client-side login machinery. `login` now implies `pkce`. Tests: 13 new browser_session unit tests — signature round-trip, tampered payload, wrong secret, malformed cookie values, HttpOnly/SameSite/Secure attributes, exact scope matching with no implied escalation, a cookie name that merely ends with the target not matching, multi-scope URL encoding, and the core property that a session cookie never contains the access token. Totals: 547 sensing-server lib + 5 wiring integration, 87 ruview-auth. Co-Authored-By: Ruflo & AQE	2026-07-23 09:57:15 +02:00