diff options
Diffstat (limited to 'packages')
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/static/hub-client.js | 27 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/harness/auth_race_probe.py | 154 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_auth_race.py | 62 |
3 files changed, 242 insertions, 1 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/hub-client.js b/packages/meshbay-hub/src/meshbay_hub/static/hub-client.js index eead457..81185a5 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/hub-client.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/hub-client.js @@ -188,11 +188,23 @@ async function _clearKeyDB() { } function loadAuth() { + let auth; try { - return JSON.parse(localStorage.getItem(AUTH_KEY)); + auth = JSON.parse(localStorage.getItem(AUTH_KEY)); } catch { return null; } + // A session is an identity and a token. An object carrying only the token is + // what a sign-out racing a renewal used to write (see `refreshAccessToken`), + // and it is worse than no session: the app renders the signed-in interface + // from it and throws on the first field it reads. That fix stops new ones + // being written; this one lets a browser already holding one heal itself on + // the next load, instead of needing somebody to find the reset button. + if (auth && (!auth.username || !auth.userId)) { + try { localStorage.removeItem(AUTH_KEY); } catch { /* private mode */ } + return null; + } + return auth; } function saveAuth(auth) { @@ -276,6 +288,19 @@ async function refreshAccessToken() { return null; } const data = await r.json(); + // Signing out while this was in flight is not rare — it is the ordinary + // shape of a tab left open: the idle watch signs out at the same moment + // the renewal fires, one second apart in the hub's log. `_auth` is null + // by now, and `{ ..._auth }` spreads null to `{}` without complaining, so + // what got written back was a token and *no identity at all*: an object + // the app believes is a session, renders the signed-in interface from, + // and throws on at the first field it reads — `username[0]`, a blank page + // on every load afterwards, in localStorage, surviving everything but a + // reset of the site's data. + // + // A sign-out that arrives during a renewal wins. There is nothing here + // worth saving over it. + if (!_auth) return null; setAuth({ ..._auth, token: data.access_token, diff --git a/packages/meshbay-hub/tests/harness/auth_race_probe.py b/packages/meshbay-hub/tests/harness/auth_race_probe.py new file mode 100644 index 0000000..f3abb83 --- /dev/null +++ b/packages/meshbay-hub/tests/harness/auth_race_probe.py @@ -0,0 +1,154 @@ +#!/usr/bin/env python3 +""" +Signing out while a token renewal is in flight must not leave a half a session. + +A tab left open overnight is exactly this race: the idle watch signs the browser +out at the same moment the renewal fires — one second apart in the hub's log. +`refreshAccessToken` then came back to an `_auth` that was already null and +wrote `{ ..._auth, token, refreshToken }`; spreading null is silent, so what +landed in localStorage was a token with **no identity at all**. + +The app believes that object is a session. It renders the signed-in interface +from it and throws on the first field it reads — `user.username[0]` — so every +load afterwards is a blank page, and it is stored, so it survives reloads, +cache clearing and the device restarting. A reader has no way back but clearing +the site's data, which nothing on screen used to mention. + +Driven against the shipped `hub-client.js` in a real browser, with `fetch` +stubbed so the renewal can be held open across the sign-out. + + auth_race_probe.py + +Prints JSON. +""" + +import http.server +import json +import socketserver +import subprocess +import sys +import tempfile +import threading +import time +from pathlib import Path + +STATIC = Path(__file__).resolve().parents[2] / "src" / "meshbay_hub" / "static" +PORT = 8767 +RECORDS = [] +socketserver.TCPServer.allow_reuse_address = True + +PAGE = r"""<!doctype html><html><head><meta charset=utf-8></head><body> +<script type="module"> +import { loadAuth, setAuth, refreshAccessToken } from '/hub-client.js'; + +const post = (o) => fetch.__real('/log', { method: 'POST', body: JSON.stringify(o) }); +const out = {}; + +// Kept aside before the stub goes in: the report has to get out. +const realFetch = window.fetch.bind(window); +window.fetch.__real = realFetch; + +(async () => { + try { + // ── the renewal, held open across a sign-out ──────────────────────────── + let release; + const gate = new Promise((r) => { release = r; }); + window.fetch = async (url) => { + if (String(url).indexOf('/v1/users/token/refresh') >= 0) { + await gate; + return { ok: true, json: async () => ({ + access_token: 'renewed-access', refresh_token: 'renewed-refresh' }) }; + } + return { ok: false, status: 404, json: async () => ({}) }; + }; + window.fetch.__real = realFetch; + + setAuth({ username: 'someone', userId: 'u-1', token: 'old', refreshToken: 'r-1' }); + const inFlight = refreshAccessToken(); + // The idle watch, landing while the request is out. + setAuth(null); + release(); + await inFlight; + + out.afterTheRace = localStorage.getItem('mb_auth'); + + // ── a browser already holding one heals itself ────────────────────────── + localStorage.setItem('mb_auth', JSON.stringify({ token: 'x', refreshToken: 'y' })); + out.poisonedLoads = loadAuth(); + out.poisonedLeftBehind = localStorage.getItem('mb_auth'); + + // ── and a real session is still a session ─────────────────────────────── + localStorage.setItem('mb_auth', JSON.stringify( + { username: 'someone', userId: 'u-1', token: 't', refreshToken: 'r', role: 'user' })); + const good = loadAuth(); + out.goodLoads = good && good.username; + out.goodLeftAlone = !!localStorage.getItem('mb_auth'); + + post(out); + } catch (err) { + post({ error: String((err && err.stack) || err) }); + } +})(); +</script></body></html>""" + + +class H(http.server.BaseHTTPRequestHandler): + def log_message(self, *a): + pass + + def do_POST(self): + length = int(self.headers.get("Content-Length") or 0) + if self.path == "/log": + RECORDS.append(json.loads(self.rfile.read(length).decode())) + else: + self.rfile.read(length) + self.send_response(204) + self.end_headers() + + def _send(self, body: bytes, ctype: str) -> None: + self.send_response(200) + self.send_header("Content-Type", ctype) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def do_GET(self): + path = self.path.split("?")[0] + if path == "/": + self._send(PAGE.encode(), "text/html; charset=utf-8") + return + asset = (STATIC / path.lstrip("/")).resolve() + if not str(asset).startswith(str(STATIC)) or not asset.is_file(): + self.send_response(404) + self.end_headers() + return + self._send(asset.read_bytes(), "text/javascript") + + +def main() -> int: + with socketserver.TCPServer(("127.0.0.1", PORT), H) as srv: + threading.Thread(target=srv.serve_forever, daemon=True).start() + with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as profile: + proc = subprocess.Popen( + ["google-chrome", "--headless=new", "--disable-gpu", "--no-sandbox", + f"--user-data-dir={profile}", f"http://127.0.0.1:{PORT}/"], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + for _ in range(300): + if RECORDS: + break + time.sleep(0.1) + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait() + if not RECORDS: + print(json.dumps({"error": "no measurement"}), file=sys.stderr) + return 1 + print(json.dumps(RECORDS[0], indent=1)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/packages/meshbay-hub/tests/test_auth_race.py b/packages/meshbay-hub/tests/test_auth_race.py new file mode 100644 index 0000000..32e7e49 --- /dev/null +++ b/packages/meshbay-hub/tests/test_auth_race.py @@ -0,0 +1,62 @@ +""" +A sign-out during a token renewal must not leave half a session behind. + +A tab left open overnight is exactly this race: the idle watch signs the browser +out at the same moment the renewal fires — the hub's log has the two requests +one second apart. `refreshAccessToken` came back to an `_auth` that was already +null and wrote `{ ..._auth, token, refreshToken }`. Spreading null is silent, so +what landed in localStorage was a token with **no identity at all**. + +That object is worse than no session. The app believes it is one, renders the +signed-in interface from it, and throws on the first field it reads — +`user.username[0]`, which is "Cannot read properties of undefined (reading +'0')". Every load afterwards is blank, and because it is stored it survives +reloads, clearing the cache and restarting the device. Reported from a phone, +diagnosed from a screenshot of `boot-guard.js` naming the error on screen. + +Two fixes, and both are needed: the sign-out now wins the race, and `loadAuth` +treats an identity-less object as signed out, so a browser already holding one +heals on its next load instead of needing someone to find the reset button. +""" + +import json +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +HARNESS = Path(__file__).parent / "harness" / "auth_race_probe.py" + + +@pytest.fixture(scope="module") +def out(): + if shutil.which("google-chrome") is None: + pytest.skip("Chrome is not available") + proc = subprocess.run([sys.executable, str(HARNESS)], + capture_output=True, text=True, timeout=120) + data = json.loads(proc.stdout) + assert "error" not in data, f"probe failed: {proc.stdout}{proc.stderr}" + return data + + +def test_a_sign_out_during_a_renewal_wins(out): + # Not "writes something harmless": writes nothing. The sign-out is the + # later intention and there is nothing in a renewal worth keeping over it. + assert out["afterTheRace"] is None, ( + f"a session survived the sign-out: {out['afterTheRace']}") + + +def test_what_used_to_be_written_is_recognised_as_not_a_session(out): + # The healing half. Without it every browser already holding one stays + # blank for a year, and the only way out is a menu nobody finds. + assert out["poisonedLoads"] is None, out["poisonedLoads"] + assert out["poisonedLeftBehind"] is None, ( + "the identity-less object was left in storage to be read again") + + +def test_a_real_session_still_loads_untouched(out): + # The guard above must not sign anybody out. This is the case that says so. + assert out["goodLoads"] == "someone", out["goodLoads"] + assert out["goodLeftAlone"], "a valid session was cleared" |