fix(ui): service worker cached /oauth/status, freezing browser sign-in
Browser sign-in worked, but the settings panel kept offering "Sign in with
Cognitum" afterwards; only a hard reload showed the true state. The server was
correct throughout.
Root cause: `ui/sw.js` routed cache-first as its CATCH-ALL for every path
outside `/api/` and `/health/`. `/oauth/status` therefore had its first
(signed-out) response stored in the Cache API and replayed to the page forever.
The Cache API is not the HTTP cache and ignores `Cache-Control` entirely, so the
`no-store, no-cache, must-revalidate` the server already sends could not prevent
it. A hard reload bypasses the service worker, which is why that alone appeared
to fix it, and why signing out re-poisoned the entry.
Fixes, in order of blast radius:
- `/oauth/` is never handled by the worker. Not network-first — that still
writes a copy, which would be replayed the moment the server is briefly
unreachable, silently reinstating a stale sign-in state.
- Cache-first is now an ALLOWLIST (navigations and static asset extensions)
rather than the catch-all. This is the underlying defect: any endpoint added
outside `/api/` was frozen on its first response. Unrecognised paths now go
to the network untouched.
- `CACHE_NAME` bumped to `ruview-v2`, so `activate` evicts the poisoned
`ruview-v1` from browsers that already ran the old worker. Verified: the
cache list went from `[ruview-v1]` to `[ruview-v2]` on update.
- Shell lookups use `ignoreSearch` and store a search-less key, so the
`?signed_in=<ms>` the callback redirects to does not mint a fresh, never-hit
cache entry per sign-in.
Also: re-check sign-in state on `pageshow` (bfcache restore, where no script
re-runs) and on `visibilitychange` (signed in or out in another tab). Opening
the panel alone is not sufficient — it may already be open.
Verification, in a real browser driven end-to-end:
- Reproduced with a valid session cookie: server returned `signed_in: true` to
curl while the page's own fetch got a cached `signed_in: false`, and the
request never appeared in the server log at all.
- Confirmed the cached entry existed: `caches.open('ruview-v1').match(
'/oauth/status')` returned the signed-out body.
- After the fix, on a NORMAL (not hard) reload the panel reads
"Signed in as <account> - sensing:read" with Sign out shown.
Tests: `ui/sw.test.mjs` loads the real `sw.js` with stubbed worker globals and
asserts the routing decision per path. 4 of its 10 tests fail against the
pre-fix worker and pass after; the other 6 pin pre-existing guards (non-GET,
websocket upgrade, cross-origin, API paths, static assets, navigation) and pass
in both, so they track behaviour rather than the rewrite.
CI: adds a `ui-tests` job. Nothing ran the UI JavaScript before, so both this
suite and the ADR-272 ws-ticket suite would have rotted unexecuted — which is
the same blind spot that let this defect ship.
Removes the temporary `/oauth/status` cookie logging and the panel console
diagnostic added while tracking this down.
Co-Authored-By: Ruflo & AQE
This commit is contained in:
parent
43737941cb
commit
9b9754778f
|
|
@ -171,6 +171,28 @@ jobs:
|
||||||
- name: ADR-135 calibration witness proof (determinism guard)
|
- name: ADR-135 calibration witness proof (determinism guard)
|
||||||
run: bash scripts/verify-calibration-proof.sh
|
run: bash scripts/verify-calibration-proof.sh
|
||||||
|
|
||||||
|
# Browser-facing JavaScript.
|
||||||
|
#
|
||||||
|
# These run the dashboard's own modules in Node with stubbed browser globals.
|
||||||
|
# They exist because the Rust suite cannot see them at all: two ADR-271/272
|
||||||
|
# defects (a service worker caching /oauth/status, and the WebSocket ticket
|
||||||
|
# helper) lived entirely in `ui/` and were invisible to a fully green
|
||||||
|
# workspace. Blocking, and fast — no browser, no install step.
|
||||||
|
ui-tests:
|
||||||
|
name: UI JavaScript Tests
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
steps:
|
||||||
|
- name: Checkout code
|
||||||
|
uses: actions/checkout@v4
|
||||||
|
|
||||||
|
- name: Set up Node
|
||||||
|
uses: actions/setup-node@v4
|
||||||
|
with:
|
||||||
|
node-version: '22'
|
||||||
|
|
||||||
|
- name: Run UI unit tests
|
||||||
|
run: node --test ui/sw.test.mjs ui/services/ws-ticket.test.mjs
|
||||||
|
|
||||||
# Unit and Integration Tests
|
# Unit and Integration Tests
|
||||||
# Python pytest matrix — runs against the archived v1 Python tree.
|
# Python pytest matrix — runs against the archived v1 Python tree.
|
||||||
# `continue-on-error: true` for the same reason as code-quality above:
|
# `continue-on-error: true` for the same reason as code-quality above:
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
// Executed tests for the WebSocket ticket helper (ADR-272).
|
// Executed tests for the WebSocket ticket helper (ADR-272).
|
||||||
//
|
//
|
||||||
// Run: node --test ui/services/
|
// Run: node --test ui/sw.test.mjs ui/services/ws-ticket.test.mjs
|
||||||
|
// (a directory argument does not work — Node resolves it as a module)
|
||||||
//
|
//
|
||||||
// WHAT THIS IS AND IS NOT.
|
// WHAT THIS IS AND IS NOT.
|
||||||
// This EXECUTES the module in Node with stubbed `fetch` and `localStorage`. It
|
// This EXECUTES the module in Node with stubbed `fetch` and `localStorage`. It
|
||||||
|
|
|
||||||
45
ui/sw.js
45
ui/sw.js
|
|
@ -1,7 +1,27 @@
|
||||||
// RuView Service Worker - Offline caching for the dashboard shell
|
// RuView Service Worker - Offline caching for the dashboard shell
|
||||||
// Strategy: Network-first for API calls, Cache-first for static assets
|
// Strategy: Network-first for API calls, Cache-first for static assets
|
||||||
|
|
||||||
const CACHE_NAME = 'ruview-v1';
|
// Bumped from v1: an older SW cached `/oauth/status` cache-first, so browsers
|
||||||
|
// that ran it hold a permanently signed-out answer. `activate` deletes every
|
||||||
|
// cache whose name is not CACHE_NAME, so bumping is what evicts it from clients
|
||||||
|
// already in the field. Bump again if a future change poisons the cache.
|
||||||
|
const CACHE_NAME = 'ruview-v2';
|
||||||
|
|
||||||
|
// Requests whose response depends on the caller's credentials. These must never
|
||||||
|
// be served from the Cache API.
|
||||||
|
//
|
||||||
|
// The Cache API is NOT the HTTP cache: it ignores `Cache-Control` completely, so
|
||||||
|
// the `no-store, no-cache, must-revalidate` the server already sends on
|
||||||
|
// `/oauth/status` has no effect here. A cached signed-out response was returned
|
||||||
|
// to the page forever, and only a hard reload — which bypasses the service
|
||||||
|
// worker entirely — showed the true state. (ADR-271.)
|
||||||
|
const NEVER_CACHE_PREFIXES = ['/oauth/'];
|
||||||
|
|
||||||
|
// What may be served cache-first. Previously cache-first was the *catch-all* for
|
||||||
|
// everything outside `/api/` and `/health/`, which meant any endpoint added at a
|
||||||
|
// new path was frozen on first response. An allowlist fails safe instead: an
|
||||||
|
// unrecognised path goes to the network untouched.
|
||||||
|
const STATIC_ASSET = /\.(?:js|mjs|css|html|json|png|jpe?g|gif|svg|ico|webp|woff2?|ttf|map)$/i;
|
||||||
const SHELL_ASSETS = [
|
const SHELL_ASSETS = [
|
||||||
'/',
|
'/',
|
||||||
'/index.html',
|
'/index.html',
|
||||||
|
|
@ -74,25 +94,40 @@ self.addEventListener('fetch', (event) => {
|
||||||
// Skip cross-origin requests
|
// Skip cross-origin requests
|
||||||
if (url.origin !== self.location.origin) return;
|
if (url.origin !== self.location.origin) return;
|
||||||
|
|
||||||
|
// Credentialed endpoints: hands off entirely. Not networkFirst — that still
|
||||||
|
// writes a copy into the cache, which would be replayed the moment the server
|
||||||
|
// is briefly unreachable, silently reinstating a stale sign-in state.
|
||||||
|
if (NEVER_CACHE_PREFIXES.some((prefix) => url.pathname.startsWith(prefix))) return;
|
||||||
|
|
||||||
// API calls: network-first with cache fallback
|
// API calls: network-first with cache fallback
|
||||||
if (url.pathname.startsWith('/api/') || url.pathname.startsWith('/health/')) {
|
if (url.pathname.startsWith('/api/') || url.pathname.startsWith('/health/')) {
|
||||||
event.respondWith(networkFirst(request));
|
event.respondWith(networkFirst(request));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Static assets: cache-first with network fallback
|
// Static assets and the app shell: cache-first with network fallback.
|
||||||
event.respondWith(cacheFirst(request));
|
if (request.mode === 'navigate' || STATIC_ASSET.test(url.pathname)) {
|
||||||
|
event.respondWith(cacheFirst(request));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Anything else is left alone and goes to the network as normal.
|
||||||
});
|
});
|
||||||
|
|
||||||
async function cacheFirst(request) {
|
async function cacheFirst(request) {
|
||||||
const cached = await caches.match(request);
|
// `ignoreSearch` so the shell is a single entry. Sign-in redirects back to
|
||||||
|
// `/ui/?signed_in=<ms>`, which would otherwise mint a fresh cache entry per
|
||||||
|
// sign-in and never hit any of them again.
|
||||||
|
const cached = await caches.match(request, { ignoreSearch: true });
|
||||||
if (cached) return cached;
|
if (cached) return cached;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const response = await fetch(request);
|
const response = await fetch(request);
|
||||||
if (response.ok) {
|
if (response.ok) {
|
||||||
const cache = await caches.open(CACHE_NAME);
|
const cache = await caches.open(CACHE_NAME);
|
||||||
cache.put(request, response.clone());
|
// Store under the search-less URL to match how it is looked up above.
|
||||||
|
const key = new URL(request.url);
|
||||||
|
key.search = '';
|
||||||
|
cache.put(key.toString(), response.clone());
|
||||||
}
|
}
|
||||||
return response;
|
return response;
|
||||||
} catch {
|
} catch {
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,164 @@
|
||||||
|
// Executed tests for the service worker's request routing (ADR-271/272).
|
||||||
|
//
|
||||||
|
// WHY THIS EXISTS.
|
||||||
|
// Browser sign-in appeared to fail: after a successful OAuth round-trip the
|
||||||
|
// settings panel still offered "Sign in with Cognitum", and only a hard reload
|
||||||
|
// showed the truth. The server was correct throughout — `/oauth/status` fell
|
||||||
|
// through to the service worker's cache-first catch-all, so the first
|
||||||
|
// (signed-out) response was stored in the Cache API and replayed forever.
|
||||||
|
//
|
||||||
|
// The Cache API is not the HTTP cache. It ignores `Cache-Control` entirely, so
|
||||||
|
// the `no-store` the server already sent could not prevent this. Nothing in the
|
||||||
|
// Rust suite, the UI unit tests, or a curl probe could observe it: curl has no
|
||||||
|
// service worker, and a hard reload bypasses one. It was only visible by
|
||||||
|
// driving a real browser.
|
||||||
|
//
|
||||||
|
// These tests load the REAL `sw.js` with stubbed worker globals and assert on
|
||||||
|
// which strategy each path is routed to.
|
||||||
|
//
|
||||||
|
// Run: node --test ui/sw.test.mjs ui/services/ws-ticket.test.mjs
|
||||||
|
// (a directory argument does not work — Node resolves it as a module)
|
||||||
|
|
||||||
|
import { test, beforeEach } from 'node:test';
|
||||||
|
import assert from 'node:assert/strict';
|
||||||
|
import { readFileSync } from 'node:fs';
|
||||||
|
import { fileURLToPath } from 'node:url';
|
||||||
|
import vm from 'node:vm';
|
||||||
|
|
||||||
|
const SW_SOURCE = readFileSync(fileURLToPath(new URL('./sw.js', import.meta.url)), 'utf8');
|
||||||
|
const ORIGIN = 'http://127.0.0.1:8099';
|
||||||
|
|
||||||
|
/** Load sw.js in a fresh context and return its registered `fetch` listener. */
|
||||||
|
function loadServiceWorker() {
|
||||||
|
const listeners = {};
|
||||||
|
const cachePuts = [];
|
||||||
|
|
||||||
|
const cacheStub = {
|
||||||
|
addAll: async () => {},
|
||||||
|
put: async (req, res) => { cachePuts.push(String(req.url ?? req)); return undefined; },
|
||||||
|
keys: async () => [],
|
||||||
|
match: async () => undefined,
|
||||||
|
};
|
||||||
|
|
||||||
|
const sandbox = {
|
||||||
|
self: {
|
||||||
|
addEventListener: (name, fn) => { listeners[name] = fn; },
|
||||||
|
location: { origin: ORIGIN },
|
||||||
|
skipWaiting: () => {},
|
||||||
|
clients: { claim: () => {} },
|
||||||
|
},
|
||||||
|
caches: {
|
||||||
|
open: async () => cacheStub,
|
||||||
|
keys: async () => [],
|
||||||
|
delete: async () => true,
|
||||||
|
match: async () => undefined,
|
||||||
|
},
|
||||||
|
// Never actually reached: every assertion below inspects the routing
|
||||||
|
// decision, not the network.
|
||||||
|
fetch: async () => ({ ok: true, clone: () => ({}) }),
|
||||||
|
URL,
|
||||||
|
Response: class { constructor(body, init) { this.body = body; Object.assign(this, init); } },
|
||||||
|
console,
|
||||||
|
};
|
||||||
|
vm.createContext(sandbox);
|
||||||
|
vm.runInContext(SW_SOURCE, sandbox);
|
||||||
|
|
||||||
|
return { listeners, cachePuts, sandbox };
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Route one request and report the decision.
|
||||||
|
* `handled: false` means the SW called neither respondWith nor cache — the
|
||||||
|
* request goes to the network untouched, which is the only safe outcome for a
|
||||||
|
* credentialed endpoint.
|
||||||
|
*/
|
||||||
|
function route(path, { method = 'GET', mode = 'cors', headers = {} } = {}) {
|
||||||
|
const { listeners } = loadServiceWorker();
|
||||||
|
let handled = false;
|
||||||
|
const request = {
|
||||||
|
url: `${ORIGIN}${path}`,
|
||||||
|
method,
|
||||||
|
mode,
|
||||||
|
headers: { get: (k) => headers[k] ?? headers[k.toLowerCase()] ?? null },
|
||||||
|
};
|
||||||
|
listeners.fetch({ request, respondWith: () => { handled = true; } });
|
||||||
|
return handled;
|
||||||
|
}
|
||||||
|
|
||||||
|
let sw;
|
||||||
|
beforeEach(() => { sw = loadServiceWorker(); });
|
||||||
|
|
||||||
|
// --- the defect this file exists for ----------------------------------------
|
||||||
|
|
||||||
|
test('/oauth/status is never handled by the service worker', () => {
|
||||||
|
// The exact request whose cached signed-out copy made sign-in look broken.
|
||||||
|
assert.equal(route('/oauth/status'), false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('every /oauth/ path bypasses the worker, not just status', () => {
|
||||||
|
// start/callback/logout all carry or clear credentials. A cached redirect or
|
||||||
|
// Set-Cookie replayed later is worse than a cached status.
|
||||||
|
for (const p of ['/oauth/start', '/oauth/callback?code=x&state=y', '/oauth/logout']) {
|
||||||
|
assert.equal(route(p), false, `${p} must go straight to the network`);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
// --- the underlying defect: cache-first was the catch-all -------------------
|
||||||
|
|
||||||
|
test('an unrecognised path is left to the network rather than cached', () => {
|
||||||
|
// This is what made the OAuth bug possible in the first place: any endpoint
|
||||||
|
// added outside /api/ was silently frozen on its first response. An
|
||||||
|
// allowlist means the next such endpoint is safe by default.
|
||||||
|
assert.equal(route('/some/future/endpoint'), false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('static assets are still served cache-first', () => {
|
||||||
|
// The offline shell is the point of the worker; the fix must not disable it.
|
||||||
|
for (const p of ['/app.js', '/style.css', '/components/TabManager.js', '/icons/logo.svg']) {
|
||||||
|
assert.equal(route(p), true, `${p} should be cache-first`);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a navigation request is still served cache-first', () => {
|
||||||
|
assert.equal(route('/ui/', { mode: 'navigate' }), true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('API paths are still handled, so offline fallback survives', () => {
|
||||||
|
assert.equal(route('/api/v1/models'), true);
|
||||||
|
assert.equal(route('/health/live'), true);
|
||||||
|
});
|
||||||
|
|
||||||
|
// --- pre-existing guards, pinned so the rewrite did not drop them -----------
|
||||||
|
|
||||||
|
test('non-GET requests are ignored', () => {
|
||||||
|
assert.equal(route('/api/v1/models', { method: 'POST' }), false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('websocket upgrades are ignored', () => {
|
||||||
|
assert.equal(route('/ws/sensing', { headers: { Upgrade: 'websocket' } }), false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('cross-origin requests are ignored', () => {
|
||||||
|
const { listeners } = loadServiceWorker();
|
||||||
|
let handled = false;
|
||||||
|
listeners.fetch({
|
||||||
|
request: {
|
||||||
|
url: 'https://auth.cognitum.one/oauth/authorize',
|
||||||
|
method: 'GET',
|
||||||
|
mode: 'cors',
|
||||||
|
headers: { get: () => null },
|
||||||
|
},
|
||||||
|
respondWith: () => { handled = true; },
|
||||||
|
});
|
||||||
|
assert.equal(handled, false);
|
||||||
|
});
|
||||||
|
|
||||||
|
// --- cache hygiene -----------------------------------------------------------
|
||||||
|
|
||||||
|
test('the cache name is bumped so clients holding the poisoned v1 evict it', () => {
|
||||||
|
// `activate` deletes every cache whose name !== CACHE_NAME. Browsers that
|
||||||
|
// already ran the old worker hold a signed-out /oauth/status in `ruview-v1`;
|
||||||
|
// only a name change removes it for them.
|
||||||
|
assert.ok(!/['"]ruview-v1['"]/.test(SW_SOURCE), 'CACHE_NAME must not still be ruview-v1');
|
||||||
|
assert.ok(/CACHE_NAME\s*=\s*['"]ruview-v[2-9]/.test(SW_SOURCE));
|
||||||
|
});
|
||||||
|
|
@ -77,7 +77,7 @@ export class QuickSettings {
|
||||||
<div class="qs-section">
|
<div class="qs-section">
|
||||||
<div class="qs-section-title">Cognitum Account</div>
|
<div class="qs-section-title">Cognitum Account</div>
|
||||||
<div class="qs-row" style="flex-direction: column; align-items: stretch; gap: 6px;">
|
<div class="qs-row" style="flex-direction: column; align-items: stretch; gap: 6px;">
|
||||||
<span id="qs-signin-status" style="font-size: 0.9em; opacity: 0.85;">Checking\u2026</span>
|
<span id="qs-signin-status" style="font-size: 0.9em; opacity: 0.85;">Checking...</span>
|
||||||
<div style="display: flex; gap: 8px;">
|
<div style="display: flex; gap: 8px;">
|
||||||
<button class="qs-btn" id="qs-signin" hidden>Sign in with Cognitum</button>
|
<button class="qs-btn" id="qs-signin" hidden>Sign in with Cognitum</button>
|
||||||
<button class="qs-btn-danger" id="qs-signout" hidden>Sign out</button>
|
<button class="qs-btn-danger" id="qs-signout" hidden>Sign out</button>
|
||||||
|
|
@ -113,6 +113,16 @@ export class QuickSettings {
|
||||||
// Bind events
|
// Bind events
|
||||||
this.panel.querySelector('.qs-close').addEventListener('click', () => this.close());
|
this.panel.querySelector('.qs-close').addEventListener('click', () => this.close());
|
||||||
|
|
||||||
|
// Re-check sign-in state whenever the page could be showing a stale view:
|
||||||
|
// `pageshow` fires on a back/forward-cache restore (where no script re-runs
|
||||||
|
// and no fetch would otherwise happen), and `visibilitychange` covers
|
||||||
|
// signing in or out in another tab. Opening the panel alone is not enough —
|
||||||
|
// the panel may already be open, or the page may be restored wholesale.
|
||||||
|
window.addEventListener('pageshow', () => { void refreshSignInPanel(this.panel); });
|
||||||
|
document.addEventListener('visibilitychange', () => {
|
||||||
|
if (!document.hidden) void refreshSignInPanel(this.panel);
|
||||||
|
});
|
||||||
|
|
||||||
// ADR-271 sign-in. Bound here as well as in refreshSignInPanel so a click
|
// ADR-271 sign-in. Bound here as well as in refreshSignInPanel so a click
|
||||||
// works even if the status fetch has not resolved yet.
|
// works even if the status fetch has not resolved yet.
|
||||||
this.panel.querySelector('#qs-signin')
|
this.panel.querySelector('#qs-signin')
|
||||||
|
|
@ -293,7 +303,7 @@ export async function refreshSignInPanel(root = document) {
|
||||||
|
|
||||||
if (info.signed_in) {
|
if (info.signed_in) {
|
||||||
status.textContent = `Signed in${info.account ? ` as ${info.account}` : ''}${
|
status.textContent = `Signed in${info.account ? ` as ${info.account}` : ''}${
|
||||||
info.scope ? ` \u2014 ${info.scope}` : ''
|
info.scope ? ` - ${info.scope}` : ''
|
||||||
}`;
|
}`;
|
||||||
signIn.hidden = true;
|
signIn.hidden = true;
|
||||||
signOut.hidden = false;
|
signOut.hidden = false;
|
||||||
|
|
|
||||||
|
|
@ -9355,10 +9355,10 @@ async fn oauth_status(
|
||||||
headers: axum::http::HeaderMap,
|
headers: axum::http::HeaderMap,
|
||||||
) -> axum::Json<serde_json::Value> {
|
) -> axum::Json<serde_json::Value> {
|
||||||
use wifi_densepose_sensing_server::browser_session as bs;
|
use wifi_densepose_sensing_server::browser_session as bs;
|
||||||
let session = headers
|
let raw = headers
|
||||||
.get(axum::http::header::COOKIE)
|
.get(axum::http::header::COOKIE)
|
||||||
.and_then(|v| v.to_str().ok())
|
.and_then(|v| v.to_str().ok());
|
||||||
.and_then(bs::from_cookie_header);
|
let session = raw.and_then(bs::from_cookie_header);
|
||||||
axum::Json(serde_json::json!({
|
axum::Json(serde_json::json!({
|
||||||
"auth_required": auth.is_enabled(),
|
"auth_required": auth.is_enabled(),
|
||||||
"oauth_enabled": auth.oauth_enabled(),
|
"oauth_enabled": auth.oauth_enabled(),
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue