From 9299a3b137577c17da670646d30880461a82a020 Mon Sep 17 00:00:00 2001 From: Dragan Spiridonov Date: Thu, 23 Jul 2026 10:07:49 +0200 Subject: [PATCH] feat(ui): Sign in with Cognitum, zero-config session secret, executed JS tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `/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 --- ui/services/ws-ticket.test.mjs | 112 ++++++++++++++++++ ui/utils/quick-settings.js | 73 ++++++++++++ .../src/browser_session.rs | 78 +++++++++++- .../wifi-densepose-sensing-server/src/main.rs | 32 +++++ 4 files changed, 293 insertions(+), 2 deletions(-) create mode 100644 ui/services/ws-ticket.test.mjs diff --git a/ui/services/ws-ticket.test.mjs b/ui/services/ws-ticket.test.mjs new file mode 100644 index 00000000..00f04176 --- /dev/null +++ b/ui/services/ws-ticket.test.mjs @@ -0,0 +1,112 @@ +// Executed tests for the WebSocket ticket helper (ADR-272). +// +// Run: node --test ui/services/ +// +// WHAT THIS IS AND IS NOT. +// This EXECUTES the module in Node with stubbed `fetch` and `localStorage`. It +// is strictly more than the `node --check` syntax pass this file replaces, and +// strictly less than a browser: it does not exercise a real WebSocket upgrade, +// real cookie handling, or the page wiring. The claim "the UI JavaScript has +// never been run" is no longer true of this module; "browser-tested" still is +// not. Both statements matter and neither should be rounded up. + +import { test, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; + +const STORAGE_KEY = 'ruview-api-token'; + +// --- stubs installed before the module under test is imported --------------- +let stored = {}; +let fetchCalls = []; +let fetchImpl = async () => ({ ok: true, status: 200, json: async () => ({ ticket: 'T' }) }); + +globalThis.localStorage = { + getItem: (k) => (k in stored ? stored[k] : null), + setItem: (k, v) => { stored[k] = String(v); }, + removeItem: (k) => { delete stored[k]; }, +}; +globalThis.fetch = async (...args) => { fetchCalls.push(args); return fetchImpl(...args); }; + +const { withWsTicket } = await import('./ws-ticket.js'); + +beforeEach(() => { + stored = {}; + fetchCalls = []; + fetchImpl = async () => ({ ok: true, status: 200, json: async () => ({ ticket: 'T' }) }); +}); + +test('with no stored token the URL is returned unchanged and nothing is fetched', async () => { + // Auth is off, so no ticket is needed. Minting one would be a pointless + // round-trip on every reconnect. + const url = await withWsTicket('ws://host/ws/sensing'); + assert.equal(url, 'ws://host/ws/sensing'); + assert.equal(fetchCalls.length, 0); +}); + +test('a minted ticket is appended and the bearer is sent only in the header', async () => { + stored[STORAGE_KEY] = 'secret-bearer'; + const url = await withWsTicket('ws://host/ws/sensing'); + + assert.equal(url, 'ws://host/ws/sensing?ticket=T'); + // The long-lived bearer must never reach a URL — only the bounded ticket does. + assert.ok(!url.includes('secret-bearer'), `bearer leaked into URL: ${url}`); + + const [path, init] = fetchCalls[0]; + assert.equal(path, '/api/v1/ws-ticket'); + assert.equal(init.method, 'POST'); + assert.equal(init.headers.Authorization, 'Bearer secret-bearer'); +}); + +test('an existing query string gets & rather than a second ?', async () => { + stored[STORAGE_KEY] = 'b'; + const url = await withWsTicket('ws://host/ws/sensing?foo=1'); + assert.equal(url, 'ws://host/ws/sensing?foo=1&ticket=T'); +}); + +test('the ticket value is URL-encoded', async () => { + stored[STORAGE_KEY] = 'b'; + fetchImpl = async () => ({ ok: true, status: 200, json: async () => ({ ticket: 'a+b/c=d' }) }); + const url = await withWsTicket('ws://host/ws/sensing'); + assert.ok(url.endsWith('ticket=a%2Bb%2Fc%3Dd'), url); +}); + +test('404 means a server predating ADR-272, so connect without a ticket', async () => { + // That server still exempts WebSockets, so an unticketed connect is correct. + // This is what lets one UI work against both old and new servers, which is + // what makes the legacy escape hatch removable rather than permanent. + stored[STORAGE_KEY] = 'b'; + fetchImpl = async () => ({ ok: false, status: 404, json: async () => ({}) }); + assert.equal(await withWsTicket('ws://host/ws/sensing'), 'ws://host/ws/sensing'); +}); + +test('a 503 does not append a ticket and does not throw', async () => { + // Ticket store exhausted. The socket attempt will fail on its own and the + // caller's reconnect logic handles it; throwing here would duplicate that. + stored[STORAGE_KEY] = 'b'; + fetchImpl = async () => ({ ok: false, status: 503, json: async () => ({}) }); + assert.equal(await withWsTicket('ws://host/ws/sensing'), 'ws://host/ws/sensing'); +}); + +test('a network failure is swallowed rather than breaking the connect path', async () => { + stored[STORAGE_KEY] = 'b'; + fetchImpl = async () => { throw new Error('offline'); }; + assert.equal(await withWsTicket('ws://host/ws/sensing'), 'ws://host/ws/sensing'); +}); + +test('a 200 with no ticket field yields no ticket', async () => { + stored[STORAGE_KEY] = 'b'; + fetchImpl = async () => ({ ok: true, status: 200, json: async () => ({}) }); + assert.equal(await withWsTicket('ws://host/ws/sensing'), 'ws://host/ws/sensing'); +}); + +test('a fresh ticket is minted per call and never reused', async () => { + // Tickets are single-use and expire in ~30s, so caching one across reconnects + // fails on the second attempt. + stored[STORAGE_KEY] = 'b'; + let n = 0; + fetchImpl = async () => ({ ok: true, status: 200, json: async () => ({ ticket: `T${++n}` }) }); + + assert.equal(await withWsTicket('ws://h/ws/sensing'), 'ws://h/ws/sensing?ticket=T1'); + assert.equal(await withWsTicket('ws://h/ws/sensing'), 'ws://h/ws/sensing?ticket=T2'); + assert.equal(fetchCalls.length, 2, 'each connect attempt must mint its own'); +}); diff --git a/ui/utils/quick-settings.js b/ui/utils/quick-settings.js index afae95eb..33c91212 100644 --- a/ui/utils/quick-settings.js +++ b/ui/utils/quick-settings.js @@ -74,6 +74,16 @@ export class QuickSettings { +
+
Cognitum Account
+
+ Checking\u2026 +
+ + +
+
+
API Access
@@ -233,3 +243,66 @@ export class QuickSettings { this.panel?.remove(); } } + +// ---- Cognitum browser sign-in (ADR-271) ------------------------------------- +// +// `/oauth/status` is intentionally UNGATED: a signed-out browser cannot ask a +// gated endpoint whether sign-in is available. It returns capability flags and, +// when a session exists, who it belongs to — never a credential. +// +// Sign-in is a full-page navigation, not fetch(): the server replies 302 to +// auth.cognitum.one, and the browser must follow it and carry the transaction +// cookie. An XHR would follow the redirect invisibly and land nowhere useful. +export async function refreshSignInPanel(root = document) { + const status = root.querySelector('#qs-signin-status'); + const signIn = root.querySelector('#qs-signin'); + const signOut = root.querySelector('#qs-signout'); + if (!status || !signIn || !signOut) return null; + + let info; + try { + const resp = await fetch('/oauth/status', { credentials: 'same-origin' }); + // 404 = a server predating ADR-271. Say so plainly rather than offering a + // button that will 404. + if (resp.status === 404) { + status.textContent = 'This server does not support Cognitum sign-in.'; + signIn.hidden = true; + signOut.hidden = true; + return null; + } + if (!resp.ok) throw new Error(`status ${resp.status}`); + info = await resp.json(); + } catch (err) { + status.textContent = `Could not reach the server (${err.message}).`; + signIn.hidden = true; + signOut.hidden = true; + return null; + } + + if (info.signed_in) { + status.textContent = `Signed in${info.account ? ` as ${info.account}` : ''}${ + info.scope ? ` \u2014 ${info.scope}` : '' + }`; + signIn.hidden = true; + signOut.hidden = false; + } else if (info.browser_signin) { + status.textContent = info.auth_required + ? 'This server requires sign-in.' + : 'Optional: sign in to use your Cognitum account.'; + signIn.hidden = false; + signOut.hidden = true; + } else if (info.auth_required) { + // Auth is on but OAuth is not — the static-token panel below is the path. + status.textContent = 'This server uses a shared API token (see API Access below).'; + signIn.hidden = true; + signOut.hidden = true; + } else { + status.textContent = 'This server does not require sign-in.'; + signIn.hidden = true; + signOut.hidden = true; + } + + signIn.onclick = () => { window.location.href = '/oauth/start'; }; + signOut.onclick = () => { window.location.href = '/oauth/logout'; }; + return info; +} diff --git a/v2/crates/wifi-densepose-sensing-server/src/browser_session.rs b/v2/crates/wifi-densepose-sensing-server/src/browser_session.rs index f5fe5ad1..d2663668 100644 --- a/v2/crates/wifi-densepose-sensing-server/src/browser_session.rs +++ b/v2/crates/wifi-densepose-sensing-server/src/browser_session.rs @@ -31,6 +31,7 @@ //! arrived over TLS. Every other attribute — `HttpOnly`, `SameSite=Lax`, //! `Path=/` — matches, and the signature is what actually protects the value. +use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; use hmac::{Hmac, Mac}; @@ -155,13 +156,86 @@ pub enum SessionError { InvalidToken(String), } -fn secret() -> Result { - std::env::var(SESSION_SECRET_ENV) +/// Process-wide secret, resolved once. +static SECRET: std::sync::OnceLock> = std::sync::OnceLock::new(); + +/// Resolve the signing secret: env first, then a persisted file, then generate. +/// +/// Requiring an operator to invent a secret before browser sign-in works is a +/// footgun — they set `RUVIEW_OAUTH_ISSUER`, expect sign-in, and get a 503 that +/// names an env var they have never heard of. A single-host appliance has no +/// reason to need that step, so we generate one and persist it `0600` next to +/// the server's other state. +/// +/// 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. +pub fn init_secret(data_dir: &Path) { + let resolved = std::env::var(SESSION_SECRET_ENV) .ok() .filter(|s| !s.trim().is_empty()) + .map(|s| { + tracing::info!("browser session secret: from {SESSION_SECRET_ENV}"); + s + }) + .or_else(|| load_or_create_secret(data_dir)); + let _ = SECRET.set(resolved); +} + +fn load_or_create_secret(data_dir: &Path) -> Option { + let path = data_dir.join("session-secret"); + if let Ok(existing) = std::fs::read_to_string(&path) { + let trimmed = existing.trim().to_string(); + if !trimmed.is_empty() { + tracing::info!(path = %path.display(), "browser session secret: loaded"); + return Some(trimmed); + } + } + let mut bytes = [0u8; 32]; + rand::RngCore::fill_bytes(&mut rand::rngs::OsRng, &mut bytes); + let generated = b64(&bytes); + if let Err(e) = write_secret(&path, &generated) { + tracing::warn!( + path = %path.display(), + error = %e, + "could not persist a browser session secret; sessions will not survive a restart. \ + Set {SESSION_SECRET_ENV} to fix this permanently." + ); + // Still usable this run — better than refusing sign-in outright. + return Some(generated); + } + tracing::info!(path = %path.display(), "browser session secret: generated"); + Some(generated) +} + +fn write_secret(path: &Path, value: &str) -> std::io::Result<()> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir)?; + } + let tmp = path.with_extension("tmp"); + std::fs::write(&tmp, value)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + // Restrict BEFORE publishing the path, as with the CLI's credentials. + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o600))?; + } + std::fs::rename(&tmp, path) +} + +fn secret() -> Result { + SECRET + .get() + .and_then(|s| s.clone()) .ok_or(SessionError::NotConfigured) } +/// Is browser sign-in usable on this server? +pub fn is_configured() -> bool { + secret().is_ok() +} + /// Begin sign-in: where to redirect, and the cookie to set. pub fn begin(issuer: &str, client_id: &str, scope: &str, secure: bool) -> Result<(String, String), SessionError> { let secret = secret()?; diff --git a/v2/crates/wifi-densepose-sensing-server/src/main.rs b/v2/crates/wifi-densepose-sensing-server/src/main.rs index 7fdfd5e3..147dd721 100644 --- a/v2/crates/wifi-densepose-sensing-server/src/main.rs +++ b/v2/crates/wifi-densepose-sensing-server/src/main.rs @@ -7673,6 +7673,10 @@ async fn main() { // ADR-044 §5.3: load persisted runtime config from the data directory. let data_dir = std::path::PathBuf::from("data"); let runtime_config = load_runtime_config(&data_dir); + // ADR-271: resolve (or generate + persist) the browser-session signing key + // before any request can arrive. Zero-config for a single appliance; the + // env var still wins for a multi-instance deployment that must share one. + wifi_densepose_sensing_server::browser_session::init_secret(&data_dir); info!( "Loaded runtime config: dedup_factor={:.2}", runtime_config.dedup_factor @@ -8087,6 +8091,10 @@ async fn main() { .route("/oauth/start", get(oauth_start)) .route("/oauth/callback", get(oauth_callback)) .route("/oauth/logout", get(oauth_logout)) + // Ungated on purpose: a signed-OUT browser needs to discover whether + // sign-in is available, and it cannot ask a gated endpoint that. + // Returns only capability + who-you-are, never a credential. + .route("/oauth/status", get(oauth_status)) .route("/api/v1/stream/pose", get(ws_pose_handler)) // Sensing WebSocket on the HTTP port so the UI can reach it without a second port .route("/ws/sensing", get(ws_sensing_handler)) @@ -9321,3 +9329,27 @@ async fn oauth_logout(headers: axum::http::HeaderMap) -> axum::response::Respons ) .into_response() } + +/// `GET /oauth/status` — what a signed-out browser needs to render the right UI. +/// +/// Deliberately ungated and deliberately thin: capability flags and, if a live +/// session exists, who it belongs to. No token, no scope escalation hints, no +/// server configuration beyond "is sign-in possible here". +async fn oauth_status( + axum::Extension(auth): axum::Extension, + headers: axum::http::HeaderMap, +) -> axum::Json { + use wifi_densepose_sensing_server::browser_session as bs; + let session = headers + .get(axum::http::header::COOKIE) + .and_then(|v| v.to_str().ok()) + .and_then(bs::from_cookie_header); + axum::Json(serde_json::json!({ + "auth_required": auth.is_enabled(), + "oauth_enabled": auth.oauth_enabled(), + "browser_signin": auth.oauth_enabled() && bs::is_configured(), + "signed_in": session.is_some(), + "account": session.as_ref().map(|s| s.account_id.clone()), + "scope": session.as_ref().map(|s| s.scope.clone()), + })) +}