diff --git a/docs/adr/ADR-272-websocket-authentication-tickets.md b/docs/adr/ADR-272-websocket-authentication-tickets.md new file mode 100644 index 00000000..48a0bead --- /dev/null +++ b/docs/adr/ADR-272-websocket-authentication-tickets.md @@ -0,0 +1,169 @@ +# ADR-272: WebSocket authentication tickets + +- **Status**: accepted +- **Date**: 2026-07-22 +- **Deciders**: RuView maintainers +- **Tags**: auth, websocket, security, sensing-server +- **Related**: ADR-271 (Cognitum OAuth resource server), ADR-055 (integrated sensing server), PR #1313 (the exemption this supersedes), cognitum-one/dashboard ADR-060 + +## Context + +`bearer_auth` gates `/api/v1/*`. WebSocket upgrade endpoints were exempt, for a +real reason: a browser's `WebSocket` constructor cannot attach an +`Authorization` header to the handshake, so a gated socket is simply +unreachable from page JavaScript. `/ws/sensing` and `/ws/introspection` sat +outside `PROTECTED_PREFIX` entirely; `/api/v1/stream/pose` was added to an +explicit `EXEMPT_PATHS` list by PR #1313. + +The reasoning was sound. The consequence was not, and it was measured rather +than argued. On a server with `RUVIEW_API_TOKEN` set — an operator who believes +authentication is ON — a real WebSocket handshake carrying **no credential at +all**: + +``` +/ws/sensing -> 101 Switching Protocols +/ws/introspection -> 101 Switching Protocols +/api/v1/stream/pose -> 101 Switching Protocols +/api/v1/models -> 401 Unauthorized (control) +``` + +**The control plane was locked and the data plane was open.** `/ws/sensing` +carries the live sensing output — presence, pose, breathing and heart rate. +`/ws/introspection` exposes internal pipeline state. For the ADR-055 desktop +topology (server bundled in the app, loopback only) that is bounded. For the +LAN/hub deployment RuView also supports, anyone who can reach the port can +watch the sensor. + +ADR-271 sharpened the contrast rather than causing it: the REST surface is now +genuinely strong — offline-verified Cognitum tokens, scope-separated +destructive routes — which makes an ungated data plane the obvious way in. + +*Precision about the evidence:* the handshake completing was verified. A +payload frame was not captured in that window, so the finding is "the +connection is established without a credential", not "data was read". + +## Decision + +Gate every WebSocket upgrade. Accept **either** of two credentials, chosen to +match what each kind of client can actually do. + +### 1. Native clients send a bearer on the upgrade + +The Python client, the Rust CLI and the TypeScript MCP client are not browsers +and have never been subject to the header limitation. They send a normal +`Authorization: Bearer` on the handshake. Routing them through a ticket would +add a round-trip and a second credential path for no benefit. + +### 2. Browsers exchange their credential for a single-use ticket + +`POST /api/v1/ws-ticket` is an ordinary authenticated request — where headers +*do* work — and returns an opaque ticket the page appends as +`?ticket=` on the socket URL. + +**A credential in a URL is normally a mistake.** URLs reach access logs, +`Referer` headers and browser history. Three properties bound this one, and all +three are load-bearing: + +| Property | Why it matters | +|---|---| +| **Single use** — consumed on the first upgrade attempt, valid or not | A ticket found in a log is already spent | +| **~30 second TTL** | Long enough to open a socket; not long enough to harvest | +| **Not the credential** — authorizes one WebSocket | Cannot be replayed against `/api/v1/*`, cannot be refreshed, carries no reusable identity | + +The long-lived bearer token is still never placed in a URL. + +A ticket **inherits the issuing principal's scopes**, so a `sensing:read` +session cannot mint one that outranks itself, and a ticket from a token without +`sensing:read` is refused at the upgrade. + +### 3. WebSocket paths are matched by **prefix**, not by an allowlist + +Anything under `/ws/` is treated as an upgrade path, plus the one endpoint that +lives outside it (`/api/v1/stream/pose`). + +This is the most important detail in the ADR. An allowlist means every +WebSocket route added later is ungated until someone remembers to extend it — +the same bug, reintroduced on a delay. It is not hypothetical: +`/ws/train/progress` (ADR-186, arriving with PR #1387) is already referenced by +`ui/services/training.service.js` and would have shipped unauthenticated under +an allowlist. Prefix matching gates it on arrival. + +New WebSocket routes should live under `/ws/` and inherit gating for free. + +### 4. A migration escape hatch, deliberately uncomfortable + +`RUVIEW_WS_LEGACY_UNAUTHENTICATED=1` restores the previous behaviour. Gating +these paths **breaks a browser UI that has not yet been updated to fetch a +ticket**, and not every deployment can update server and UI in lockstep. + +It is a migration aid, not a supported configuration: + +- It logs a warning on every boot naming the actual exposure — "the live + sensing stream — presence, pose and vital signs — is readable by anyone who + can reach this port" — rather than something an operator can skim past. +- Its blast radius is exactly the WebSocket paths. A test pins that it does not + weaken `/api/v1/*`. +- It is read **once at construction**, so changing the environment cannot + silently open the paths on a running server. + +The alternative — a clean break with no hatch — was considered and rejected as +sequencing, not principle: a hard break tempts an operator into turning auth off +entirely, which is strictly worse than a narrow, loudly-announced exception. +The hatch should be removed once the shipped UI fetches tickets. + +### 5. Deployments with auth off are unchanged + +No credential configured ⇒ the middleware is the same no-op it has always been. +Pinned by a test. + +## Consequences + +- The measured hole is closed: all three paths now return `401` to a + credential-less handshake, while a bearer or a valid ticket returns `101`. +- Browser UIs need updating. Shipped in the same change for + `sensing.service.js`, `websocket-client.js` and `observatory/js/main.js` via + a shared `withWsTicket()` helper; a ticket is minted per connection attempt + and never cached, because it is single-use and short-lived. +- A UI running against a server that predates this ADR still works: the helper + treats `404` from `/api/v1/ws-ticket` as "no ticket needed". +- One more round-trip before a browser opens a socket. Negligible against a + stream that then runs for minutes. +- Tickets live in memory, capped at 512 outstanding and self-healing as they + expire, so an authenticated but misbehaving caller cannot grow the store + without bound. In-memory is correct rather than convenient: a ticket + surviving a restart would outlive the server that vouched for it. + +## Supersedes + +PR #1313's `enabled_exempts_pose_stream_websocket`, which asserted the +exemption. Its premise about browsers was correct and is preserved here; its +conclusion is replaced. The test was renamed and inverted rather than deleted, +with the history in its doc comment, and the half that still matters — the +WebSocket rule must not leak to other `/api/v1/*` paths — is kept. + +## Deliberately not done + +- **`/health*` stays ungated.** Orchestrator probes hit it anonymously, and + that is the point of a liveness endpoint. `/health/metrics` is included in + that exemption; if metrics ever carry occupancy-derived values this should be + revisited, because that would make them sensing data wearing an ops label. +- **`/ui/*` stays ungated.** It is static assets; the data behind them is + gated. +- **No revocation of an issued ticket.** It expires in seconds and is + single-use; a revocation path would be more machinery than the exposure + justifies. +- **No ticket for native clients.** They can send a header, so they should. + +## Implementation + +`v2/crates/wifi-densepose-sensing-server/src/ws_ticket.rs` (store), +`src/bearer_auth.rs` (gating), `src/main.rs` (`POST /api/v1/ws-ticket`), +`ui/services/ws-ticket.js` plus the three call sites. + +Tests: 12 store, 9 gating, 4 path-matching. Store coverage includes single-use +enforcement, replay refusal, expiry refusal *and* pruning, 256-bit +unpredictability, cap enforcement and self-healing, and `?myticket=x` not being +read as `?ticket=x`. Gating coverage includes every known WS path refusing an +unauthenticated upgrade, bearer acceptance, ticket single-use, a ticket being +useless against REST, the escape hatch working *and* not weakening REST, and +auth-off behaviour unchanged. diff --git a/ui/observatory/js/main.js b/ui/observatory/js/main.js index 26abbe2c..b0149368 100644 --- a/ui/observatory/js/main.js +++ b/ui/observatory/js/main.js @@ -8,6 +8,7 @@ * - Dot-matrix mist body mass, particle trails, WiFi waves, signal field * - Reflective floor, settings dialog, and practical data HUD */ +import { withWsTicket } from '../../services/ws-ticket.js'; import * as THREE from 'three'; import { OrbitControls } from 'three/addons/controls/OrbitControls.js'; @@ -462,7 +463,7 @@ class Observatory { console.log('[Observatory] Sensing server detected at', base, '→', wsUrl); this.settings.dataSource = 'ws'; this.settings.wsUrl = wsUrl; - this._connectWS(wsUrl); + void this._connectWS(wsUrl); } else { tryNext(i + 1); } @@ -472,10 +473,13 @@ class Observatory { tryNext(0); } - _connectWS(url) { + // async: `/ws/sensing` is gated (ADR-272); mint a single-use ticket first. + async _connectWS(url) { this._disconnectWS(); + let wsUrl = url; + try { wsUrl = await withWsTicket(url); } catch { /* auth off or pre-ADR-272 server */ } try { - this._ws = new WebSocket(url); + this._ws = new WebSocket(wsUrl); this._ws.onopen = () => { console.log('[Observatory] WebSocket connected'); this._hud.updateSourceBadge('ws', this._ws); diff --git a/ui/services/sensing.service.js b/ui/services/sensing.service.js index 2d5635e4..e07a09cd 100644 --- a/ui/services/sensing.service.js +++ b/ui/services/sensing.service.js @@ -1,3 +1,4 @@ +import { withWsTicket } from './ws-ticket.js'; /** * Sensing WebSocket Service * @@ -65,7 +66,7 @@ class SensingService { /** Start the service (connect or simulate). */ start() { - this._connect(); + void this._connect(); } /** Stop the service entirely. */ @@ -120,13 +121,26 @@ class SensingService { // ---- Connection -------------------------------------------------------- - _connect() { + // async because the server gates `/ws/sensing` (ADR-272) and a browser + // cannot set an Authorization header on an upgrade — so we mint a + // single-use ticket first. Minted per connect attempt, never cached: a + // ticket is valid once and expires in seconds, so reusing one across + // reconnects would fail on the second attempt. + async _connect() { if (this._ws && this._ws.readyState <= WebSocket.OPEN) return; this._setState('connecting'); + let url = SENSING_WS_URL; try { - this._ws = new WebSocket(SENSING_WS_URL); + url = await withWsTicket(SENSING_WS_URL); + } catch { + // Ticket minting is best-effort: against a server with auth off, or one + // predating ADR-272, connecting without a ticket is correct. + } + + try { + this._ws = new WebSocket(url); } catch (err) { console.warn('[Sensing] WebSocket constructor failed:', err.message); this._fallbackToSimulation(); @@ -184,7 +198,7 @@ class SensingService { this._reconnectTimer = setTimeout(() => { this._reconnectTimer = null; - this._connect(); + void this._connect(); }, delay); // Only start simulation after several failed attempts so a brief hiccup diff --git a/ui/services/websocket-client.js b/ui/services/websocket-client.js index 208e0780..64f1ca86 100644 --- a/ui/services/websocket-client.js +++ b/ui/services/websocket-client.js @@ -1,3 +1,4 @@ +import { withWsTicket } from './ws-ticket.js'; // WebSocket Client for Three.js Visualization - WiFi DensePose // Default endpoint is `/ws/sensing` on the same host the page was served from. // Callers (e.g. viz.html) usually pass an explicit `url` derived from @@ -47,7 +48,9 @@ export class WebSocketClient { } // Attempt to connect - connect() { + // async: `/ws/*` is gated (ADR-272) and a browser cannot set an + // Authorization header on an upgrade, so mint a single-use ticket first. + async connect() { if (this.state === 'connecting' || this.state === 'connected') { console.warn('[WS-VIZ] Already connected or connecting'); return; @@ -56,8 +59,12 @@ export class WebSocketClient { this._setState('connecting'); console.log(`[WS-VIZ] Connecting to ${this.url}`); + // Per attempt, never cached — a ticket is single-use and short-lived. + let url = this.url; + try { url = await withWsTicket(this.url); } catch { /* auth off or pre-ADR-272 server */ } + try { - this.ws = new WebSocket(this.url); + this.ws = new WebSocket(url); this.ws.binaryType = 'arraybuffer'; this.ws.onopen = () => this._handleOpen(); @@ -235,7 +242,7 @@ export class WebSocketClient { console.log(`[WS-VIZ] Reconnecting in ${delay}ms (attempt ${this.reconnectAttempts}/${this.maxReconnectAttempts})`); this.reconnectTimer = setTimeout(() => { - this.connect(); + void this.connect(); }, delay); } diff --git a/ui/services/ws-ticket.js b/ui/services/ws-ticket.js new file mode 100644 index 00000000..9870562a --- /dev/null +++ b/ui/services/ws-ticket.js @@ -0,0 +1,77 @@ +// Single-use WebSocket tickets (ADR-272). +// +// A browser's WebSocket constructor cannot set an `Authorization` header on the +// upgrade request. That used to mean the sensing WebSocket stayed reachable +// with no credential even when the server had auth switched on — the REST +// control plane was locked while the live sensing stream was open. +// +// The server now gates `/ws/*` and `/api/v1/stream/pose`. Browsers exchange +// their stored bearer token at `POST /api/v1/ws-ticket` — an ordinary request, +// where headers DO work — for a short-lived, single-use ticket, and pass that +// as `?ticket=` on the socket URL. +// +// Why a token in a URL is acceptable here when it normally is not: the ticket +// is consumed on first use, lives ~30 seconds, and authorizes one WebSocket +// and nothing else. It cannot be replayed against /api/v1/*. The long-lived +// bearer token is still never put in a URL. + +import { API_TOKEN_STORAGE_KEY } from './api.service.js'; + +function storedToken() { + try { + return localStorage.getItem(API_TOKEN_STORAGE_KEY) || null; + } catch { + // Private browsing / storage disabled — treat as "no token configured". + return null; + } +} + +/** + * Mint a ticket. Returns null when no token is configured (auth is off, so no + * ticket is needed) or when the server does not offer the endpoint. + */ +async function mintTicket() { + const token = storedToken(); + if (!token) return null; + + try { + const resp = await fetch('/api/v1/ws-ticket', { + method: 'POST', + headers: { Authorization: `Bearer ${token}` }, + }); + // 404 means a server predating ADR-272: it still exempts WebSockets, so + // connecting without a ticket is correct there. Treated as "no ticket + // needed" rather than as an error, so the UI works against both. + if (resp.status === 404) return null; + if (!resp.ok) { + console.warn('[ws-ticket] mint failed:', resp.status); + return null; + } + const body = await resp.json(); + return body.ticket || null; + } catch (err) { + // Offline, or the server is down. The socket attempt will fail on its own + // and the caller's reconnect logic handles it; failing loudly here would + // just duplicate that. + console.warn('[ws-ticket] mint error:', err.message); + return null; + } +} + +/** + * Return `url` with a freshly minted ticket appended, or unchanged when no + * ticket is available or needed. + * + * Always call this immediately before opening the socket — a ticket expires in + * seconds and is valid exactly once, so one must never be cached or reused + * across reconnects. + * + * @param {string} url ws:// or wss:// URL + * @returns {Promise} + */ +export async function withWsTicket(url) { + const ticket = await mintTicket(); + if (!ticket) return url; + const sep = url.includes('?') ? '&' : '?'; + return `${url}${sep}ticket=${encodeURIComponent(ticket)}`; +} diff --git a/v2/crates/wifi-densepose-sensing-server/src/bearer_auth.rs b/v2/crates/wifi-densepose-sensing-server/src/bearer_auth.rs index e4ab06bb..89638caa 100644 --- a/v2/crates/wifi-densepose-sensing-server/src/bearer_auth.rs +++ b/v2/crates/wifi-densepose-sensing-server/src/bearer_auth.rs @@ -86,6 +86,10 @@ pub const PROTECTED_PREFIX: &str = "/api/v1/"; /// /// They are now gated, and accept **either** a bearer (native clients, which /// are not browser-constrained) **or** a single-use ticket (ADR-272). +/// +/// This list is the set that exists today, used for the boot warning and tests. +/// The runtime rule is [`is_ws_path`], which matches by prefix so routes added +/// later are gated without an edit here. pub const WS_PATHS: &[&str] = &[ "/ws/sensing", "/ws/introspection", @@ -109,8 +113,27 @@ fn legacy_ws_unauthenticated() -> bool { ) } +/// WebSocket upgrade paths that do NOT live under [`WS_PREFIX`] and so must be +/// named explicitly. +pub const WS_PATHS_OUTSIDE_PREFIX: &[&str] = &["/api/v1/stream/pose"]; + +/// Everything under here is treated as a WebSocket upgrade. +pub const WS_PREFIX: &str = "/ws/"; + +/// Is this a WebSocket upgrade path? +/// +/// Matched by **prefix**, not by an allowlist, and that choice is the whole +/// point. An allowlist means every WebSocket route added later is ungated until +/// someone remembers to add it here — which is exactly the bug this module just +/// fixed, reintroduced on a delay. `/ws/train/progress` (ADR-186, arriving with +/// PR #1387) is already referenced by `ui/services/training.service.js` and +/// would have shipped unauthenticated under an allowlist. +/// +/// `/api/v1/stream/pose` is the one upgrade endpoint outside the prefix, so it +/// is named. New WebSocket routes should go under `/ws/` and inherit gating for +/// free. fn is_ws_path(path: &str) -> bool { - WS_PATHS.contains(&path) + path.starts_with(WS_PREFIX) || WS_PATHS_OUTSIDE_PREFIX.contains(&path) } /// Cognitum OAuth verification state. Built once at boot and shared. @@ -1229,3 +1252,44 @@ mod ws_gate_tests { ); } } + +#[cfg(test)] +mod ws_path_matching_tests { + use super::*; + + #[test] + fn every_currently_known_websocket_path_matches() { + for p in WS_PATHS { + assert!(is_ws_path(p), "{p} must be recognised as a WebSocket path"); + } + } + + #[test] + fn a_websocket_route_that_does_not_exist_yet_is_already_gated() { + // `/ws/train/progress` arrives with ADR-186 (PR #1387) and is already + // referenced by the UI. Under an exact-match allowlist it would ship + // unauthenticated. Prefix matching means it is gated on arrival. + assert!(is_ws_path("/ws/train/progress")); + assert!(is_ws_path("/ws/anything-added-in-future")); + } + + #[test] + fn ordinary_rest_paths_are_not_treated_as_websockets() { + for p in [ + "/api/v1/models", + "/api/v1/stream/status", // a plain GET, not an upgrade + "/health", + "/ui/index.html", + "/", + ] { + assert!(!is_ws_path(p), "{p} must not be treated as a WebSocket path"); + } + } + + #[test] + fn a_path_merely_starting_with_ws_is_not_the_ws_prefix() { + // `/wsx/...` must not match `/ws/`. + assert!(!is_ws_path("/wsx/sensing")); + assert!(!is_ws_path("/ws")); + } +}