From 6562665c80f96f30fef95af83a0abcf71f41795f Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Mon, 17 Aug 2026 17:38:00 +0200 Subject: fix(hub): a session that renews itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported: after an hour of watching a film, every action answers "token expired or invalid", with signing out and back in as the only way on. Reopening the tab the next day did the same. The access token lasts an hour and the refresh token thirty days, and nothing used the second one. `hubFetch` reported a 401 like any other error, and watching a film is precisely an hour in which the hub hears nothing at all, because the video travels over WebRTC. So the token aged out with no request to notice, and a tab reopened the next morning presented a stale token with a perfectly good refresh token sitting beside it in localStorage. Underneath was the reason it could not be recovered from. The hub *rotates*: the refresh endpoint revokes the token presented, returns a replacement, and treats a revoked one presented again as theft, revoking the whole family. The client kept only the access token out of that response. So the refresh token was spent on first use and the second attempt did not merely fail — it destroyed the family. Which is exactly the reported symptom. Renewal now happens on a margin, on returning to the tab, on mount, and on a 401 with the request replayed. Concurrent renewals share one request: two 401s racing would otherwise present the same refresh token twice, and the hub cannot tell that from theft, so the remedy would have been worse than the fault. A refusal signs out cleanly rather than leaving a session that fails every call while looking signed in. The lifetime goes to four hours, which is not what makes long sessions work — renewal is — but is what someone has to notice by if renewal itself breaks. An hour was less than a feature film. Twelve was considered and declined: it widens the window in which a leaked token cannot be turned off, and it lets the renewal path go a whole day between uses, which is how it came to be broken here without anyone noticing. Production sets this in its own hub.toml, so both moved. The tests run the shipped code against a hub that enforces rotation, because a stub that accepted the same refresh token twice would have passed against the broken client. Checked that dropping the rotated token reproduces the revoked family, so the guard is guarding something. Also widens the orphan-setter rule to ignore `setX` functions declared in the module: `setAuth` is not a hook setter, and a rule that cries wolf is one somebody eventually silences. Verified it still catches a real orphan. --- packages/meshbay-hub/src/meshbay_hub/config.py | 13 +- packages/meshbay-hub/src/meshbay_hub/static/app.js | 188 +++++++++++++++-- .../meshbay-hub/tests/harness/session_harness.mjs | 165 +++++++++++++++ packages/meshbay-hub/tests/test_session_renewal.py | 227 +++++++++++++++++++++ .../meshbay-hub/tests/test_transport_contracts.py | 10 +- 5 files changed, 583 insertions(+), 20 deletions(-) create mode 100644 packages/meshbay-hub/tests/harness/session_harness.mjs create mode 100644 packages/meshbay-hub/tests/test_session_renewal.py (limited to 'packages') diff --git a/packages/meshbay-hub/src/meshbay_hub/config.py b/packages/meshbay-hub/src/meshbay_hub/config.py index 31912d1..b508e45 100644 --- a/packages/meshbay-hub/src/meshbay_hub/config.py +++ b/packages/meshbay-hub/src/meshbay_hub/config.py @@ -46,7 +46,18 @@ class HubIdentityConfig: @dataclass class JWTConfig: - access_token_ttl: int = 3600 # 1 hour + # How long an access token stays good. It is not the session — the refresh + # token below is, and the SPA renews against it well before this runs out, + # so a film or a working day never meets this number. + # + # What it does bound is a token that leaks: revoking a member or suspending + # an account both take effect at once (the hub reloads the account on every + # request, and pushes signed revocations to nodes), but there is no way to + # kill one issued token short of that. Four hours keeps the window short + # while renewing several times a day — which also means the renewal path is + # exercised constantly rather than twice, and cannot rot unnoticed the way + # it did when nothing used it at all. + access_token_ttl: int = 14400 # 4 hours refresh_token_ttl: int = 86400 * 30 # 30 days diff --git a/packages/meshbay-hub/src/meshbay_hub/static/app.js b/packages/meshbay-hub/src/meshbay_hub/static/app.js index 198bc78..cb7a462 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/app.js @@ -11,6 +11,14 @@ import * as downloads from './downloads.js'; const HUB = ''; const AUTH_KEY = 'mb_auth'; +// Renew an access token with this much life left rather than waiting for it to +// fail. Generous against a one-hour token: a film is watched without the hub +// hearing a word, and coming back to a tab that has been asleep for an hour +// should not cost a round trip before the first click works. +const TOKEN_RENEW_MARGIN_S = 600; +// How often to look. Cheap — it reads a timestamp out of the token and almost +// always does nothing. +const TOKEN_CHECK_MS = 60000; const THEME_KEY = 'mb_theme'; const IDB_NAME = 'meshbay'; const IDB_VERSION = 1; @@ -171,6 +179,94 @@ function saveAuth(auth) { } } +// ── Session ────────────────────────────────────────────────────────────────── +// +// The access token lasts an hour and the refresh token thirty days. Nothing was +// using the second: `hubFetch` reported a 401 as an error like any other, so an +// hour of watching a film — during which the hub hears nothing, because the +// video comes over WebRTC — ended with "token expired or invalid" and no way +// out but signing out and back in. Reopening the tab the next day did the same, +// with a perfectly good refresh token sitting in localStorage beside the stale +// access one. +// +// This lives outside the component because `hubFetch` is a plain function and +// has to be able to renew a token mid-request without every caller passing the +// machinery down to it. + +let _auth = loadAuth(); +let _onAuthChange = null; // set by App, so the UI follows a background renewal +let _refreshing = null; // in flight, shared: see refreshAccessToken + +function setAuth(auth) { + _auth = auth; + saveAuth(auth); + if (_onAuthChange) _onAuthChange(auth); +} + +/** Seconds until this JWT expires, or null if it says nothing useful. */ +function tokenLifeLeft(token) { + try { + const payload = JSON.parse(atob(token.split('.')[1].replace(/-/g, '+').replace(/_/g, '/'))); + if (!payload.exp) return null; + return payload.exp - Math.floor(Date.now() / 1000); + } catch { + return null; // not a JWT we can read; treat as unknown, never as expired + } +} + +/** + * Trade the refresh token for a new pair. + * + * The hub rotates: it revokes the token presented and returns a new one, and a + * revoked token presented again revokes the whole family. So the new one must + * be stored — the previous code kept only the access token and dropped its + * replacement, which burned the refresh token on first use and locked the + * account out of renewal on the second. That is why signing out and in was the + * only way back. + * + * Concurrent callers share one request. Two 401s racing would otherwise send + * the same refresh token twice, and the second would look exactly like theft. + */ +async function refreshAccessToken() { + if (!_auth || !_auth.refreshToken) return null; + if (_refreshing) return _refreshing; + _refreshing = (async () => { + try { + const r = await fetch(HUB + '/v1/users/token/refresh', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ refresh_token: _auth.refreshToken }), + }); + if (!r.ok) { + // Expired, revoked, or the family was torn down. Nothing to salvage: + // sign out cleanly rather than leave a session that fails every call. + setAuth(null); + return null; + } + const data = await r.json(); + setAuth({ + ..._auth, + token: data.access_token, + refreshToken: data.refresh_token || _auth.refreshToken, + }); + return data.access_token; + } catch { + return null; // offline: keep the session, the next call can try again + } finally { + _refreshing = null; + } + })(); + return _refreshing; +} + +/** Renew before it bites, rather than after. */ +async function ensureFreshToken() { + if (!_auth || !_auth.token) return null; + const left = tokenLifeLeft(_auth.token); + if (left !== null && left > TOKEN_RENEW_MARGIN_S) return _auth.token; + return refreshAccessToken(); +} + // ── Theme ──────────────────────────────────────────────────────────────────── function getInitialTheme() { @@ -188,13 +284,26 @@ function resolveTheme(pref) { // ── Hub API ────────────────────────────────────────────────────────────────── -async function hubFetch(path, { method = 'GET', body, token } = {}) { +async function hubFetch(path, { method = 'GET', body, token, _retried } = {}) { const headers = {}; if (body) headers['Content-Type'] = 'application/json'; - if (token) headers['Authorization'] = `Bearer ${token}`; + // Prefer the token the session currently holds. Callers read theirs from + // React state, which is a render behind a renewal that happened in the + // background — and sending the stale one would 401 for no reason. + const bearer = token && _auth && _auth.token ? _auth.token : token; + if (bearer) headers['Authorization'] = `Bearer ${bearer}`; const opts = { method, headers }; if (body) opts.body = JSON.stringify(body); const r = await fetch(HUB + path, opts); + if (r.status === 401 && bearer && !_retried) { + // The one case worth a second attempt: the access token aged out while + // nothing was talking to the hub. Renew once and replay. If the renewal + // fails it signs out, and the replay below is skipped. + const fresh = await refreshAccessToken(); + if (fresh) { + return hubFetch(path, { method, body, token: fresh, _retried: true }); + } + } if (!r.ok) { const err = await r.json().catch(() => ({ detail: r.statusText })); const detail = Array.isArray(err.detail) @@ -1198,11 +1307,18 @@ function GroupPage({ groupId, group, token, username, userId, onRefreshAuth, setStatus('connecting'); const nodeId = nodesData.nodes[0].node_id; - const transport = new window.MeshBayTransport('', token); + // Renewed here rather than taken from the prop. This effect no longer + // re-runs when the token rotates (see the dependency list below), so + // the captured one can be older than the session's — and it is used to + // sign the offer to the hub, where an expired one is a 401 and no + // connection at all. Renewals are shared, so if one is already in + // flight this waits for it instead of starting a second. + const live = (await ensureFreshToken()) || token; + const transport = new window.MeshBayTransport('', live); transportRef.current = transport; const ack = await transport.connect( - nodeId, token, groupId, null, sessionKeys, _bundleKey, username, + nodeId, live, groupId, null, sessionKeys, _bundleKey, username, userId, _pendingJoinCode); _pendingJoinCode = null; if (cancelled) return; @@ -1291,7 +1407,18 @@ function GroupPage({ groupId, group, token, username, userId, onRefreshAuth, // `group` object, which the hub poll re-creates, and re-running this effect // means tearing down the WebRTC connection. groupId is here, so a real group // change still re-captures it. - }, [groupId, token, retryKey]); + // + // Neither is the token itself, only whether there is one. It used to be a + // dependency and that was harmless while a token never changed during a + // session — it only expired. Now that the session renews itself, the string + // rotates, and this effect tore the WebRTC connection down and rebuilt it + // every time. Worst on arrival: a stored token past its life is renewed the + // instant the page mounts, which is exactly when the group page is + // negotiating ICE, so the connection was abandoned mid-handshake and the + // node sat in `connecting` for ever. The live token is read inside + // `connect()` instead. Signing out unmounts this page; signing in mounts + // it; nothing in between should disturb a working connection. + }, [groupId, Boolean(token), retryKey]); const downloadFile = useCallback(async (entry) => { const transport = transportRef.current; @@ -4214,6 +4341,33 @@ function App() { const resolved = resolveTheme(theme); + // Keep the session alive without anyone having to think about it. + useEffect(() => { + // A renewal can happen inside hubFetch, well away from any render. This is + // how the component learns about it — including a failed one, which sets + // null and lands on the login page instead of failing every later call. + _onAuthChange = (auth) => setUser(auth); + + // On mount above all: a tab reopened tomorrow holds an hour-old access + // token and a refresh token good for a month, and used to greet its owner + // with "invalid token" rather than spending the second on renewing it. + ensureFreshToken(); + + const timer = setInterval(ensureFreshToken, TOKEN_CHECK_MS); + // A backgrounded tab has its timers throttled hard, so the check above may + // not have run for the whole time it was away. Coming back is exactly when + // the token is most likely to be stale. + const onVisible = () => { + if (document.visibilityState === 'visible') ensureFreshToken(); + }; + document.addEventListener('visibilitychange', onVisible); + return () => { + _onAuthChange = null; + clearInterval(timer); + document.removeEventListener('visibilitychange', onVisible); + }; + }, []); + useEffect(() => { document.documentElement.className = `theme-${resolved}`; localStorage.setItem(THEME_KEY, theme); @@ -4320,15 +4474,18 @@ function App() { } const me = await hubFetch('/v1/users/me', { token }); const u = { username, userId: me.user_id, token, refreshToken, role: me.role }; + // setAuth, not saveAuth: it is the one writer that also updates the copy + // hubFetch renews from. Storing the session without it left the renewal + // path with no refresh token to present. + setAuth(u); setUser(u); - saveAuth(u); }, logout: () => { // Navigating away leaves transfers running; signing out does not. They // are moving data on tokens that are about to stop being ours. transfers.reset(); + setAuth(null); setUser(null); - saveAuth(null); setGroups([]); navigate('/login'); }, @@ -4337,16 +4494,13 @@ function App() { // Group membership is baked into the access token at login and the hub does not // push updates, so someone invited after they signed in carries a token that // says they are in nothing. Refreshing re-reads membership from the database. - const refreshAuth = useCallback(async () => { - if (!user || !user.refreshToken) return null; - const data = await hubFetch('/v1/users/token/refresh', { - method: 'POST', body: { refresh_token: user.refreshToken }, - }); - const u = { ...user, token: data.access_token }; - setUser(u); - saveAuth(u); - return data.access_token; - }, [user]); + // Goes through refreshAccessToken like everything else. It used to call the + // endpoint here and keep only the access token, dropping the rotated refresh + // token that came back with it — so the refresh token was spent on first use, + // and presenting the spent one again revoked the whole family. Which is how + // a session that should last a month ended at "invalid token" with signing + // out as the only way back. + const refreshAuth = useCallback(() => refreshAccessToken(), []); let page; if (route === '/login' || route === '/register') { diff --git a/packages/meshbay-hub/tests/harness/session_harness.mjs b/packages/meshbay-hub/tests/harness/session_harness.mjs new file mode 100644 index 0000000..f0d0cb2 --- /dev/null +++ b/packages/meshbay-hub/tests/harness/session_harness.mjs @@ -0,0 +1,165 @@ +// Run the shipped session code against a hub that behaves like the real one. +// +// The defect this guards was not subtle once seen: the hub rotates refresh +// tokens — it revokes the one presented and returns a replacement, and a +// revoked token presented again revokes the entire family — and the client kept +// only the access token out of that response. So the refresh token was spent on +// first use and the second attempt locked the account out of renewal +// altogether, leaving signing out and back in as the only way to carry on. +// +// The fake hub below enforces exactly that rule, which is the point: a stub +// that happily accepts the same refresh token twice would have passed against +// the broken client. +import { readFileSync } from 'fs'; + +const app = readFileSync(process.argv[2], 'utf8'); +const cfg = JSON.parse(process.argv[3] || '{}'); +const { scenario = 'expired-access' } = cfg; + +// ── The pieces of app.js under test, lifted as text ────────────────────────── +const between = (from, to) => { + const i = app.indexOf(from); + if (i < 0) throw new Error(`not found: ${from}`); + const j = app.indexOf(to, i); + if (j < 0) throw new Error(`not found: ${to}`); + return app.slice(i, j); +}; +const sessionBlock = between('let _auth = loadAuth();', '\n// ── Theme'); +const hubFetchFn = between('async function hubFetch(', '\n// ── Router'); + +// ── The world ──────────────────────────────────────────────────────────────── +const store = new Map(); +const localStorage = { + getItem: (k) => (store.has(k) ? store.get(k) : null), + setItem: (k, v) => store.set(k, v), + removeItem: (k) => store.delete(k), +}; +const AUTH_KEY = 'mb_auth'; +const HUB = ''; +const TOKEN_RENEW_MARGIN_S = Number(app.match(/const TOKEN_RENEW_MARGIN_S = (\d+)/)[1]); + +const jwt = (secondsLeft) => { + const payload = Buffer.from(JSON.stringify({ + exp: Math.floor(Date.now() / 1000) + secondsLeft, + })).toString('base64url'); + return `head.${payload}.sig`; +}; + +// The hub. One live refresh token at a time; presenting a revoked one kills the +// family, as the real endpoint does. +let live = 'RT-1'; +let issued = 1; +let familyRevoked = false; +const seen = []; +let refreshCalls = 0; + +const fakeFetch = async (url, opts = {}) => { + if (url.endsWith('/v1/users/token/refresh')) { + refreshCalls++; + const presented = JSON.parse(opts.body).refresh_token; + seen.push(presented); + if (familyRevoked || presented !== live) { + familyRevoked = true; + return { ok: false, status: 401, + json: async () => ({ detail: 'Token reuse detected — family revoked' }) }; + } + live = `RT-${++issued}`; + return { + ok: true, status: 200, + json: async () => ({ access_token: jwt(3600), refresh_token: live, + token_type: 'bearer', expires_in: 3600 }), + }; + } + // Any other endpoint: 401 unless the bearer is a token that has life left. + const auth = (opts.headers || {})['Authorization'] || ''; + const token = auth.replace('Bearer ', ''); + let alive = false; + try { + alive = JSON.parse(Buffer.from(token.split('.')[1], 'base64url').toString()).exp + > Math.floor(Date.now() / 1000); + } catch { alive = false; } + calls.push({ url, token, alive }); + if (!alive) { + return { ok: false, status: 401, json: async () => ({ detail: 'Token expired or invalid' }) }; + } + return { ok: true, status: 200, json: async () => ({ ok: true }) }; +}; +const calls = []; + +let _bundleKey = null; +const _clearKeyDB = () => {}; +function loadAuth() { + try { return JSON.parse(localStorage.getItem(AUTH_KEY)); } catch { return null; } +} +function saveAuth(auth) { + if (auth) localStorage.setItem(AUTH_KEY, JSON.stringify(auth)); + else { localStorage.removeItem(AUTH_KEY); _bundleKey = null; _clearKeyDB(); } +} + +// The session starts with an access token that expired an hour ago and a +// refresh token that is still perfectly good — a tab reopened the next morning. +const startingLife = scenario === 'fresh-access' ? 3600 + : scenario === 'nearly-expired' ? 60 + : -60; +localStorage.setItem(AUTH_KEY, JSON.stringify({ + username: 'someone', userId: 'u1', role: 'user', + token: jwt(startingLife), refreshToken: 'RT-1', +})); + +const ctx = { + localStorage, AUTH_KEY, HUB, TOKEN_RENEW_MARGIN_S, + loadAuth, saveAuth, fetch: fakeFetch, atob: (s) => Buffer.from(s, 'base64').toString('binary'), + console, +}; +const fns = new Function(...Object.keys(ctx), + sessionBlock + '\n' + hubFetchFn + + '\n return {hubFetch, refreshAccessToken, ensureFreshToken, tokenLifeLeft,' + + ' getAuth: () => _auth};')(...Object.values(ctx)); + +// ── The run ────────────────────────────────────────────────────────────────── +const out = { scenario }; +const stored = () => JSON.parse(localStorage.getItem(AUTH_KEY) || 'null'); + +if (scenario === 'concurrent') { + // Two requests hit a dead token at the same instant. If each renews on its + // own, the second presents a token the first has already spent, and the hub + // tears down the family. + const before = stored().token; + // Either may reject once the family is gone; the run still has to report. + await Promise.allSettled([ + fns.hubFetch('/v1/groups/mine', { token: before }), + fns.hubFetch('/v1/users/me', { token: before }), + ]); +} else if (scenario === 'repeat') { + // Renew several times over. Each response carries a new refresh token, and + // keeping the old one would be caught on the very next attempt. + for (let i = 0; i < 4; i++) { + await fns.refreshAccessToken(); + // Age the access token again so the next one really has to renew. A + // rejected renewal signs the session out, and there is then nothing left + // to age — which is the outcome under test, not a reason to stop. + const a = stored(); + if (!a) break; + localStorage.setItem(AUTH_KEY, JSON.stringify({ ...a, token: jwt(-60) })); + const live_ = fns.getAuth(); + if (live_) live_.token = jwt(-60); + } +} else if (scenario === 'refresh-rejected') { + localStorage.setItem(AUTH_KEY, JSON.stringify({ ...stored(), refreshToken: 'RT-STALE' })); + fns.getAuth().refreshToken = 'RT-STALE'; + try { await fns.hubFetch('/v1/groups/mine', { token: stored().token }); } + catch (e) { out.threw = String(e.message); } +} else { + await fns.ensureFreshToken(); + await fns.hubFetch('/v1/groups/mine', { token: stored() ? stored().token : null }); +} + +out.refreshCalls = refreshCalls; +out.refreshTokensPresented = seen; +out.familyRevoked = familyRevoked; +out.signedOut = stored() === null; +out.storedRefreshToken = stored() ? stored().refreshToken : null; +out.apiCalls = calls.map(c => ({ url: c.url, accepted: c.alive })); +out.everRejected = calls.some(c => !c.alive); +out.finalCallAccepted = calls.length ? calls[calls.length - 1].alive : null; +console.log(JSON.stringify(out)); diff --git a/packages/meshbay-hub/tests/test_session_renewal.py b/packages/meshbay-hub/tests/test_session_renewal.py new file mode 100644 index 0000000..38db67f --- /dev/null +++ b/packages/meshbay-hub/tests/test_session_renewal.py @@ -0,0 +1,227 @@ +""" +Staying signed in. + +Reported: after an hour or so of watching a film, every action answers "token +expired or invalid", and the only way back is signing out and in again. Also on +simply reopening the tab the next day. + +Both come from the same place. The access token lasts an hour and the refresh +token thirty days, but nothing used the second one. `hubFetch` reported a 401 as +an error like any other, and watching a film is precisely the activity during +which the hub hears nothing at all — the video travels over WebRTC — so the hour +ran out with no request to notice. + +Underneath that was a worse one. The hub *rotates*: `/v1/users/token/refresh` +revokes the token presented, returns a replacement, and treats a revoked token +presented again as theft, revoking the whole family. The client kept only the +access token out of that response and dropped the new refresh token. So the +refresh token was spent on first use, and the second attempt did not merely fail +— it destroyed the family, which is why signing out and back in was the only +cure. + +These run the shipped `hubFetch`, `refreshAccessToken` and `ensureFreshToken` +against a fake hub that enforces the rotation rule. That rule is the point: a +stub which accepted the same refresh token twice would have passed against the +broken client. +""" + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +APP = STATIC / "app.js" +HARNESS = Path(__file__).parent / "harness" / "session_harness.mjs" + +pytestmark = pytest.mark.skipif( + shutil.which("node") is None or not APP.exists(), + reason="node or the SPA sources are not available") + + +def _run(scenario: str, app: Path = APP) -> dict: + proc = subprocess.run( + ["node", str(HARNESS), str(app), json.dumps({"scenario": scenario})], + capture_output=True, text=True, timeout=60) + assert proc.returncode == 0, f"{proc.stdout}\n{proc.stderr}" + return json.loads(proc.stdout.strip().splitlines()[-1]) + + +@pytest.fixture(scope="module") +def broken(tmp_path_factory): + """The client as it shipped: the rotated refresh token dropped.""" + out = tmp_path_factory.mktemp("session") / "broken.js" + src = APP.read_text() + replaced = src.replace( + " refreshToken: data.refresh_token || _auth.refreshToken,", + " refreshToken: _auth.refreshToken,") + assert replaced != src, ( + "could not reconstruct the defect — the line it hinged on has moved, " + "and the A/B below would be comparing the fix against itself") + out.write_text(replaced) + return out + + +# ── The reported symptom ────────────────────────────────────────────────────── + +def test_an_expired_access_token_renews_itself(app=None): + """A tab reopened the next morning, or a film watched for an hour.""" + r = _run("expired-access") + assert not r["signedOut"], "the session was thrown away instead of renewed" + assert r["finalCallAccepted"], "the request was not replayed after renewing" + assert r["refreshCalls"] == 1 + + +def test_renewal_survives_being_needed_more_than_once(): + """The defect underneath the symptom. + + The hub hands back a new refresh token every time and revokes the old one. + Keep the old one and the second renewal is read as theft. + """ + r = _run("repeat") + assert not r["familyRevoked"], ( + f"the family was revoked after presenting {r['refreshTokensPresented']} " + "— the rotated token is not being stored") + assert not r["signedOut"] + assert len(set(r["refreshTokensPresented"])) == len(r["refreshTokensPresented"]), ( + f"the same refresh token was presented twice: {r['refreshTokensPresented']}") + + +def test_the_defect_is_reproduced_by_dropping_the_rotated_token(broken): + """Otherwise the test above proves nothing. + + This is the shipped behaviour, and it ends exactly where the report did: + signed out, with nothing but signing back in to be done about it. + """ + r = _run("repeat", app=broken) + assert r["familyRevoked"], ( + "dropping the rotated refresh token no longer breaks anything, so the " + "test above is not guarding what it claims") + assert r["signedOut"] + assert r["refreshTokensPresented"] == ["RT-1", "RT-1"] + + +# ── Renewing exactly once ───────────────────────────────────────────────────── + +def test_two_requests_racing_share_one_renewal(): + """Two 401s at the same instant must not present the token twice. + + They would each renew, the second presenting what the first had already + spent — which the hub cannot tell from a stolen token, and answers by + revoking the family. The failure mode is worse than the problem. + """ + r = _run("concurrent") + assert r["refreshCalls"] == 1, ( + f"{r['refreshCalls']} renewals for one expiry") + assert not r["familyRevoked"] + assert r["finalCallAccepted"], "the queued requests were not replayed" + + +def test_a_valid_token_is_left_alone(): + """Renewing on every call would be its own kind of broken.""" + r = _run("fresh-access") + assert r["refreshCalls"] == 0, "a token with an hour left was renewed anyway" + assert r["finalCallAccepted"] + + +# ── When there is genuinely nothing left ────────────────────────────────────── + +def test_an_unusable_refresh_token_signs_out_cleanly(): + """Thirty days later, or after the family was revoked. + + There is nothing to salvage, and the alternative is a session that fails + every call for ever while looking signed in. + """ + r = _run("refresh-rejected") + assert r["signedOut"], ( + "the session survived a refusal, so every later call fails with no way " + "for the person to understand why") + assert r["storedRefreshToken"] is None + assert r["refreshCalls"] == 1, "it kept trying a token the hub had refused" + + +# ── The lifetimes themselves ────────────────────────────────────────────────── + +def test_the_access_token_outlives_a_film_on_its_own(): + """Not because it has to — renewal covers any length — but as a floor. + + Renewal makes the access token's life invisible in normal use. This exists + for the abnormal one: if renewal fails, a token this short is how long + someone has before they notice. An hour was less than a feature film. + """ + from meshbay_hub.config import JWTConfig + ttl = JWTConfig().access_token_ttl + assert ttl >= 4 * 3600, ( + f"{ttl / 3600:.1f} h does not cover a long film if renewal fails") + assert ttl <= 12 * 3600, ( + f"{ttl / 3600:.1f} h is a long time for a leaked token to stay usable, " + "and it lets the renewal path go a whole day without being exercised — " + "which is how it came to be broken without anyone noticing") + + +def test_the_session_is_much_longer_than_the_token(): + """The two must not be confused: the session is the refresh token.""" + from meshbay_hub.config import JWTConfig + cfg = JWTConfig() + assert cfg.refresh_token_ttl >= 7 * 86400 + assert cfg.refresh_token_ttl > cfg.access_token_ttl * 20, ( + "the refresh token is barely longer than the access token, so renewing " + "buys almost nothing and signing in again comes round just as fast") + + +# ── The margin ──────────────────────────────────────────────────────────────── + +def test_renewal_happens_before_expiry_not_after(): + """A margin, so the first click after a long film does not pay for a 401.""" + src = APP.read_text() + import re + margin = int(re.search(r"const TOKEN_RENEW_MARGIN_S = (\d+)", src).group(1)) + assert margin >= 300, ( + f"{margin} s of margin against a one-hour token is thin: a backgrounded " + "tab has its timers throttled and may not check for minutes") + assert "visibilitychange" in src, ( + "nothing re-checks when the tab comes back, which is exactly when the " + "token is most likely to have aged out unnoticed") + + +# ── What renewal must not disturb ───────────────────────────────────────────── + +def test_renewing_does_not_tear_down_the_webrtc_connection(): + """The regression that renewal introduced. + + The effect that dials the node listed `token` among its dependencies. That + was harmless while a token never changed during a session — it only ran + out. Once the session renews itself the string rotates, and the effect tore + the connection down and rebuilt it each time. Worst on arrival: a stored + token past its life is renewed the instant the page mounts, which is when + the group page is negotiating ICE, so the browser abandoned the handshake + and the node sat in `connecting` for ever. + + Signing in or out must still re-run it, so the dependency is whether there + is a token, not which one. + """ + src = APP.read_text() + i = src.index("means tearing down the WebRTC connection") + deps = src[i:src.index(");", i)] + assert "Boolean(token)" in deps, ( + "the WebRTC effect depends on the token's value again — every renewal " + "drops the connection, and one landing mid-handshake never recovers") + assert "[groupId, token," not in deps + + +def test_the_connection_signs_its_offer_with_a_live_token(): + """The other half: not re-running means the captured token can be stale. + + It signs the offer relayed through the hub, where an expired one is a 401 + and no connection at all. + """ + src = APP.read_text() + connect = src[src.index("const connect = async () => {"):] + connect = connect[:connect.index("\n };")] + assert "await ensureFreshToken()" in connect, ( + "the offer is signed with whatever token the effect captured, which is " + "no longer refreshed by a re-run") + assert "new window.MeshBayTransport('', live)" in connect, ( + "the transport is built with the captured token rather than the live one") diff --git a/packages/meshbay-hub/tests/test_transport_contracts.py b/packages/meshbay-hub/tests/test_transport_contracts.py index fb92fcd..8d975d7 100644 --- a/packages/meshbay-hub/tests/test_transport_contracts.py +++ b/packages/meshbay-hub/tests/test_transport_contracts.py @@ -187,8 +187,14 @@ def test_no_setter_survives_the_state_it_belonged_to(app): # to their object, not to this component. called = set(re.findall(r"(?