diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2bbdc260..a85b6a97 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -204,7 +204,7 @@ jobs: node-version: '22' - name: Run UI unit tests - run: node --test ui/sw.test.mjs ui/services/ws-ticket.test.mjs + run: node --test ui/sw.test.mjs ui/services/ws-ticket.test.mjs ui/services/websocket.service.test.mjs # Unit and Integration Tests # Python pytest matrix — runs against the archived v1 Python tree. diff --git a/ui/pose-fusion.html b/ui/pose-fusion.html index 39cacc23..7fd6cfd1 100644 --- a/ui/pose-fusion.html +++ b/ui/pose-fusion.html @@ -196,6 +196,6 @@ - + diff --git a/ui/pose-fusion/js/main.js b/ui/pose-fusion/js/main.js index 58348f4b..7b7eb4af 100644 --- a/ui/pose-fusion/js/main.js +++ b/ui/pose-fusion/js/main.js @@ -10,6 +10,7 @@ import { CnnEmbedder } from './cnn-embedder.js?v=13'; import { FusionEngine } from './fusion-engine.js?v=13'; import { PoseDecoder } from './pose-decoder.js?v=13'; import { CanvasRenderer } from './canvas-renderer.js?v=13'; +import { withWsTicket } from '../../services/ws-ticket.js'; // === State === let mode = 'dual'; // 'dual' | 'video' | 'csi' @@ -114,7 +115,9 @@ function init() { const url = wsUrlInput.value.trim(); if (!url) return; connectWsBtn.textContent = 'Connecting...'; - const ok = await csiSimulator.connectLive(url); + // ADR-272: exchange the stored bearer for a single-use ?ticket= before the + // upgrade — a browser cannot set an Authorization header on a WebSocket. + const ok = await csiSimulator.connectLive(await withWsTicket(url)); connectWsBtn.textContent = ok ? '✓ Connected' : 'Connect'; if (ok) { connectWsBtn.classList.add('active'); @@ -136,10 +139,19 @@ function init() { }); csiCnn.tryLoadWasm(wasmBase); - // Auto-connect to local sensing server WebSocket if available - const defaultWsUrl = 'ws://localhost:8765/ws/sensing'; + // Auto-connect to local sensing server WebSocket if available. + // Served from the Docker image the sensing stream lives on :3001 (same + // port mapping sensing.service.js uses); the standalone dev server stays + // on :8765. + const wsPortMap = { '3000': '3001' }; + const mappedPort = wsPortMap[window.location.port]; + const defaultWsUrl = mappedPort + ? `ws://${window.location.hostname}:${mappedPort}/ws/sensing` + : 'ws://localhost:8765/ws/sensing'; if (wsUrlInput) wsUrlInput.value = defaultWsUrl; - csiSimulator.connectLive(defaultWsUrl).then(ok => { + // ADR-272: exchange the stored bearer for a single-use ?ticket= before the + // upgrade — a browser cannot set an Authorization header on a WebSocket. + withWsTicket(defaultWsUrl).then(u => csiSimulator.connectLive(u)).then(ok => { if (ok && connectWsBtn) { connectWsBtn.textContent = '✓ Live ESP32'; connectWsBtn.classList.add('active'); diff --git a/ui/services/websocket.service.js b/ui/services/websocket.service.js index c27cea4f..4c9ee587 100644 --- a/ui/services/websocket.service.js +++ b/ui/services/websocket.service.js @@ -2,6 +2,7 @@ import { API_CONFIG, buildWsUrl } from '../config/api.config.js'; import { backendDetector } from '../utils/backend-detector.js'; +import { withWsTicket } from './ws-ticket.js'; export class WebSocketService { constructor() { @@ -115,8 +116,19 @@ export class WebSocketService { } async createWebSocketWithTimeout(url) { + // ADR-272: the server gates /ws/* and /api/v1/stream/* behind bearer auth, + // and a browser cannot set an Authorization header on an upgrade request. + // Exchange the stored bearer for a single-use ?ticket= here — immediately + // before the socket opens, on every attempt — so reconnects each get a + // fresh ticket. Also strip any long-lived `token` param a caller put in + // the URL (e.g. pose.service.js): the bearer itself must never travel in + // a query string. + const urlObj = new URL(url); + urlObj.searchParams.delete('token'); + const connectUrl = await withWsTicket(urlObj.toString()); + return new Promise((resolve, reject) => { - const ws = new WebSocket(url); + const ws = new WebSocket(connectUrl); const timeout = setTimeout(() => { ws.close(); reject(new Error(`Connection timeout after ${this.config.connectionTimeout}ms`)); diff --git a/ui/services/websocket.service.test.mjs b/ui/services/websocket.service.test.mjs new file mode 100644 index 00000000..55cd0da1 --- /dev/null +++ b/ui/services/websocket.service.test.mjs @@ -0,0 +1,98 @@ +// Executed regression test for issue #1461: the pose/event WebSocket path +// (unlike sensing.service.js) opened a bare `new WebSocket(url)` with no +// ADR-272 ticket exchange, and never stripped the long-lived bearer that +// pose.service.js puts on the URL as `?token=`. Both meant the pose stream +// 401'd whenever RUVIEW_API_TOKEN was set. +// +// Run: node --test ui/sw.test.mjs ui/services/ws-ticket.test.mjs ui/services/websocket.service.test.mjs +// +// This EXECUTES createWebSocketWithTimeout in Node with a stub `WebSocket` +// class and stubbed `fetch`/`localStorage`, so it verifies the actual ticket +// exchange + token stripping wiring, not just that the file parses. + +import { test, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; + +const STORAGE_KEY = 'ruview-api-token'; + +let stored = {}; +let fetchCalls = []; +let fetchImpl = async () => ({ ok: true, status: 200, json: async () => ({ ticket: 'T' }) }); +let lastConstructedUrl = null; + +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); }; + +// Minimal WebSocket stub: records the URL it was opened with and fires +// `onopen` on the next microtask — by then createWebSocketWithTimeout's +// Promise executor has already assigned `ws.onopen`, so this resolves the +// real promise the way a successful upgrade would, instead of leaving the +// 10s connection-timeout timer dangling for the whole test run. +class FakeWebSocket { + constructor(url) { + lastConstructedUrl = url; + this.url = url; + this.readyState = 0; // CONNECTING + queueMicrotask(() => { if (this.onopen) this.onopen(); }); + } + close() {} +} +globalThis.WebSocket = FakeWebSocket; + +const { WebSocketService } = await import('./websocket.service.js'); + +beforeEach(() => { + stored = {}; + fetchCalls = []; + fetchImpl = async () => ({ ok: true, status: 200, json: async () => ({ ticket: 'T' }) }); + lastConstructedUrl = null; +}); + +test('with no stored bearer, the socket opens with the URL unchanged', async () => { + const svc = new WebSocketService(); + await svc.createWebSocketWithTimeout('ws://host/api/v1/stream/pose?min_confidence=0.3'); + assert.equal(lastConstructedUrl, 'ws://host/api/v1/stream/pose?min_confidence=0.3'); + assert.equal(fetchCalls.length, 0, 'no ticket should be minted when auth is off'); +}); + +test('a stored bearer is exchanged for a single-use ticket before the socket opens', async () => { + stored[STORAGE_KEY] = 'secret-bearer'; + const svc = new WebSocketService(); + await svc.createWebSocketWithTimeout('ws://host/api/v1/stream/pose?min_confidence=0.3'); + + const url = new URL(lastConstructedUrl); + assert.equal(url.searchParams.get('ticket'), 'T'); + assert.equal(url.searchParams.get('min_confidence'), '0.3', 'other params must survive'); + assert.ok(!lastConstructedUrl.includes('secret-bearer'), `bearer leaked into URL: ${lastConstructedUrl}`); + + const [path, init] = fetchCalls[0]; + assert.equal(path, '/api/v1/ws-ticket'); + assert.equal(init.headers.Authorization, 'Bearer secret-bearer'); +}); + +test('a stray ?token= from a caller (pose.service.js) is stripped, not forwarded', async () => { + // Regression for #1461: pose.service.js puts the long-lived bearer on the + // URL as `?token=`. That must never reach the actual WebSocket upgrade — + // the ticket exchange above is the only credential that belongs in the URL. + stored[STORAGE_KEY] = 'secret-bearer'; + const svc = new WebSocketService(); + await svc.createWebSocketWithTimeout('ws://host/api/v1/stream/pose?token=secret-bearer&max_fps=30'); + + const url = new URL(lastConstructedUrl); + assert.equal(url.searchParams.get('token'), null, 'token param must be stripped'); + assert.equal(url.searchParams.get('ticket'), 'T', 'a real ticket must replace it'); + assert.equal(url.searchParams.get('max_fps'), '30', 'unrelated params must survive'); + assert.ok(!lastConstructedUrl.includes('secret-bearer')); +}); + +test('with no ADR-272 endpoint on the server (404), the socket still opens without a ticket', async () => { + stored[STORAGE_KEY] = 'secret-bearer'; + fetchImpl = async () => ({ ok: false, status: 404, json: async () => ({}) }); + const svc = new WebSocketService(); + await svc.createWebSocketWithTimeout('ws://host/api/v1/stream/pose'); + assert.equal(lastConstructedUrl, 'ws://host/api/v1/stream/pose'); +});