Skip to content

Commit af55efb

Browse files
committed
web: the session store belongs to every tab, not to the tab that started it
The refresh token is rotated on use. The browser secret store loaded everything once and rewrote the whole thing on every change, so a second tab held the token the first had already rotated away: its renewal was refused, ended the session the first tab had just renewed, and wrote its start-up snapshot back over the first tab's tokens. Now nothing is cached. A read is a read of what is stored; a change reads, changes ONE key and writes, under a lock shared by the tabs (Web Locks; without it a tab still applies its own changes one at a time). A browser that refuses the write keeps the session in memory for that tab rather than completing a sign-in that is not there. Tests: unit (two tabs over one store, the rotated token, concurrent writers, unreadable store, no key, refused write — each mutation-checked) and a second tab in the end-to-end.
1 parent 325ad4b commit af55efb

3 files changed

Lines changed: 232 additions & 23 deletions

File tree

‎web/main.js‎

Lines changed: 61 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -266,42 +266,82 @@ function createUrlCallbackProvider(URI, callbackRoute) {
266266
* running on this origin, which can call the same key: the editor therefore loads no third-party
267267
* extensions (see docs/WEB.md). Where IndexedDB or WebCrypto is unavailable (some private
268268
* windows), secrets live in memory for the session and the user signs in again after a reload.
269+
*
270+
* The store belongs to every tab of this origin, as the desktop's keychain belongs to every window,
271+
* and the extension relies on that: the refresh token is rotated on use, so a tab that renewed the
272+
* session has to be seen by the next tab that does, or that one presents a token that is no longer
273+
* good and ends a session the first just renewed. So nothing is cached here: a read is a read of
274+
* what is stored now, and a change is read-modify-write of ONE key under a lock shared by the tabs
275+
* (the Web Locks API; without it, changes from one tab are still applied one at a time).
269276
*/
270277
async function createSecretStorage() {
271278
const STORE_KEY = 'levelcode-web.secrets.v1';
279+
const LOCK = 'levelcode-web.secrets';
272280
let key;
273281
try { key = await loadOrCreateKey(); } catch (e) { key = undefined; }
274282

275-
let cache = {};
276-
const fromMemoryOnly = !key;
277-
if (key) {
283+
if (!key) {
284+
const memory = new Map();
285+
return {
286+
type: 'in-memory',
287+
async get(k) { return memory.get(k); },
288+
async set(k, v) { memory.set(k, v); },
289+
async delete(k) { memory.delete(k); },
290+
async keys() { return [...memory.keys()]; },
291+
};
292+
}
293+
294+
/** Everything stored now. Unreadable (key lost, tampered) is dropped: the user signs in again. */
295+
const read = async () => {
296+
let sealed = null;
297+
try { sealed = localStorage.getItem(STORE_KEY); } catch (e) { return {}; }
298+
if (!sealed) { return {}; }
278299
try {
279-
const sealed = localStorage.getItem(STORE_KEY);
280-
if (sealed) { cache = JSON.parse(await unseal(key, sealed)); }
300+
const all = JSON.parse(await unseal(key, sealed));
301+
return all && typeof all === 'object' ? all : {};
281302
} catch (e) {
282-
// An unreadable store (key lost, tampered) is dropped; the user signs in again.
283303
console.warn('[levelcode-web] the saved session could not be read and was cleared', e);
284304
try { localStorage.removeItem(STORE_KEY); } catch (e2) { /* ignore */ }
285-
cache = {};
305+
return {};
286306
}
287-
}
307+
};
288308

289-
let writing = Promise.resolve();
290-
const persist = () => {
291-
if (!key) { return writing; }
292-
writing = writing.then(async () => {
293-
try { localStorage.setItem(STORE_KEY, await seal(key, JSON.stringify(cache))); }
294-
catch (e) { console.error('[levelcode-web] could not persist secrets', e); }
295-
});
296-
return writing;
309+
// One change at a time within this tab, and — where the browser has Web Locks — across tabs.
310+
let queue = Promise.resolve();
311+
const exclusive = (fn) => {
312+
const run = () => (globalThis.navigator && navigator.locks && navigator.locks.request
313+
? navigator.locks.request(LOCK, fn)
314+
: fn());
315+
const next = queue.then(run, run);
316+
queue = next.then(() => undefined, () => undefined);
317+
return next;
297318
};
319+
// When the browser refuses the write (storage disabled or full) this tab keeps the session in memory and
320+
// the user signs in again after a reload: better than a sign-in that completes and is not there.
321+
/** @type {Map<string, string> | null} */
322+
let inMemory = null;
323+
const change = (mutate) => exclusive(async () => {
324+
if (inMemory) {
325+
const all = Object.fromEntries(inMemory);
326+
mutate(all);
327+
inMemory = new Map(Object.entries(all));
328+
return;
329+
}
330+
const all = await read();
331+
mutate(all);
332+
try { localStorage.setItem(STORE_KEY, await seal(key, JSON.stringify(all))); }
333+
catch (e) {
334+
console.error('[levelcode-web] could not persist secrets; keeping them for this tab only', e);
335+
inMemory = new Map(Object.entries(all));
336+
}
337+
});
298338

299339
return {
300-
type: fromMemoryOnly ? 'in-memory' : 'persisted',
301-
async get(k) { return cache[k]; },
302-
async set(k, v) { cache[k] = v; await persist(); },
303-
async delete(k) { delete cache[k]; await persist(); },
304-
async keys() { return Object.keys(cache); },
340+
type: 'persisted',
341+
async get(k) { return inMemory ? inMemory.get(k) : (await read())[k]; },
342+
async set(k, v) { await change((all) => { all[k] = v; }); },
343+
async delete(k) { await change((all) => { delete all[k]; }); },
344+
async keys() { return inMemory ? [...inMemory.keys()] : Object.keys(await read()); },
305345
};
306346
}
307347

‎web/test/e2e-signin-tab.mjs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,12 @@ try {
115115
await sleep(1500);
116116
check('an editor that is not waiting for it leaves it alone', (await orphan.eval(`localStorage.getItem('vscode-web.url-callbacks[7]') !== null`)) === true);
117117
const before = exchanges();
118+
const callsBefore = accountCalls();
118119
await orphan.goto(editorOrigin + '/');
119120
await workbenchReady(orphan);
120121
await orphan.waitFor(() => exchanges() > before, { ms: 60000, label: 'delivery at start-up' }).catch(() => null);
122+
await orphan.waitFor(() => accountCalls() > callsBefore, { ms: 30000, label: 'the second tab\'s account calls' }).catch(() => null);
123+
check('a second tab of the editor is signed in from the store the first one wrote, with no sign-in of its own', accountCalls() > callsBefore && stub.state.signIns === 1, `account calls ${callsBefore} -> ${accountCalls()}, sign-ins ${stub.state.signIns}`);
121124
check('the next start delivers it to the extension (the stand-in refuses the made-up code; the attempt is the proof)', exchanges() === before + 1, `${before} -> ${exchanges()}`);
122125
check('and it is gone from storage', (await orphan.eval(`localStorage.getItem('vscode-web.url-callbacks[7]')`)) === null);
123126

‎web/test/unit/main.test.js‎

Lines changed: 168 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* leaveForSignIn the one command the page offers an extension: it must take the tab to the
99
* account site's /ai/ pages and nowhere else
1010
* createUrlCallbackProvider .drain / .whenListening: what is delivered when the page starts
11+
* createSecretStorage the session store, shared by every tab of the origin
1112
* startReturnFromSignIn ?signin=1 and the return from a sign-in
1213
*--------------------------------------------------------------------------------------------*/
1314
// @ts-check
@@ -19,10 +20,19 @@ const path = require('path');
1920

2021
const SOURCE = fs.readFileSync(path.join(__dirname, '..', '..', 'main.js'), 'utf8');
2122

22-
/** The text of a top-level `function name(...) { ... }` — it ends at the first `}` in column 0. */
23+
/**
24+
* The text of a top-level `function name(...) { ... }` (or `async function`): a one-line function is its
25+
* line, any other ends at the first `}` in column 0.
26+
*/
2327
function extract(name) {
24-
const start = SOURCE.indexOf('\nfunction ' + name + '(');
28+
let start = SOURCE.indexOf('\nfunction ' + name + '(');
29+
if (start < 0) { start = SOURCE.indexOf('\nasync function ' + name + '('); }
2530
assert.ok(start >= 0, 'main.js has no top-level function ' + name);
31+
const eol = SOURCE.indexOf('\n', start + 1);
32+
const line = SOURCE.slice(start + 1, eol);
33+
const opens = (line.match(/\{/g) || []).length;
34+
const closes = (line.match(/\}/g) || []).length;
35+
if (opens > 0 && opens === closes && line.endsWith('}')) { return line + '\n'; }
2636
const end = SOURCE.indexOf('\n}\n', start);
2737
assert.ok(end > start, 'could not find the end of ' + name);
2838
return SOURCE.slice(start + 1, end + 2);
@@ -197,6 +207,162 @@ const ACCOUNT = 'https://levelcode.example';
197207
} finally { t.restore(); }
198208
});
199209

210+
/* ---- the secret store --------------------------------------------------------------------------- */
211+
const nodeCrypto = require('crypto');
212+
const STORE_KEY = 'levelcode-web.secrets.v1';
213+
const SECRET_SOURCE = ['b64', 'unb64', 'seal', 'unseal', 'createSecretStorage'].map(extract).join('\n');
214+
const memoryStorage = () => {
215+
const m = new Map();
216+
return {
217+
m,
218+
getItem: (k) => (m.has(k) ? m.get(k) : null),
219+
setItem: (k, v) => { m.set(k, String(v)); },
220+
removeItem: (k) => { m.delete(k); },
221+
};
222+
};
223+
/** A mutex shared by every "tab" that is handed the same one, as Web Locks is. */
224+
const sharedLocks = () => {
225+
let tail = Promise.resolve();
226+
const held = { max: 0, now: 0 };
227+
return {
228+
held,
229+
request: (_name, fn) => {
230+
const run = async () => { held.now++; held.max = Math.max(held.max, held.now); try { return await fn(); } finally { held.now--; } };
231+
const p = tail.then(run, run);
232+
tail = p.then(() => undefined, () => undefined);
233+
return p;
234+
},
235+
};
236+
};
237+
/** One tab of the editor: its own storage object over the shared localStorage, the shared origin key. */
238+
const tab = async ({ storage, key, locks, failKey = false, noKey = false }) => {
239+
const logs = { error: [], warn: [] };
240+
const factory = new Function('localStorage', 'navigator', 'console', 'loadOrCreateKey', 'crypto', 'TextEncoder', 'TextDecoder', 'btoa', 'atob', 'Uint8Array',
241+
SECRET_SOURCE + '\nreturn createSecretStorage;');
242+
const create = factory(storage, locks ? { locks } : {}, { warn: (...a) => logs.warn.push(a), error: (...a) => logs.error.push(a) },
243+
async () => { if (noKey) { throw new Error('no indexedDB'); } return key; }, nodeCrypto.webcrypto, TextEncoder, TextDecoder, btoa, atob, Uint8Array);
244+
return { store: await create(), logs };
245+
};
246+
const newKey = () => nodeCrypto.webcrypto.subtle.generateKey({ name: 'AES-GCM', length: 256 }, false, ['encrypt', 'decrypt']);
247+
248+
await test('secrets: set, get, keys and delete, and they are there after a reload', async () => {
249+
const storage = memoryStorage();
250+
const key = await newKey();
251+
const a = await tab({ storage, key });
252+
assert.strictEqual(a.store.type, 'persisted');
253+
await a.store.set('access', 'token-AAA');
254+
await a.store.set('refresh', 'token-RRR');
255+
assert.strictEqual(await a.store.get('access'), 'token-AAA');
256+
assert.deepStrictEqual((await a.store.keys()).sort(), ['access', 'refresh']);
257+
await a.store.delete('access');
258+
assert.strictEqual(await a.store.get('access'), undefined);
259+
const reloaded = await tab({ storage, key });
260+
assert.strictEqual(await reloaded.store.get('refresh'), 'token-RRR');
261+
assert.deepStrictEqual(await reloaded.store.keys(), ['refresh']);
262+
});
263+
await test('secrets: what is stored is sealed — neither the value nor its base64 is in it', async () => {
264+
const storage = memoryStorage();
265+
const a = await tab({ storage, key: await newKey() });
266+
await a.store.set('refresh', 'a-very-recognisable-secret-value');
267+
const blob = storage.getItem(STORE_KEY);
268+
assert.ok(blob && blob.length > 20);
269+
assert.ok(!blob.includes('recognisable') && !blob.includes(Buffer.from('a-very-recognisable-secret-value').toString('base64')));
270+
});
271+
await test('secrets: another tab\'s change is seen at once — the rotated refresh token is the one a second tab uses', async () => {
272+
const storage = memoryStorage();
273+
const key = await newKey();
274+
const locks = sharedLocks();
275+
const first = await tab({ storage, key, locks });
276+
await first.store.set('refresh', 'R1');
277+
await first.store.set('access', 'A1');
278+
const second = await tab({ storage, key, locks }); // started while R1 was current
279+
assert.strictEqual(await second.store.get('refresh'), 'R1');
280+
await first.store.set('refresh', 'R2'); // the first tab renews the session
281+
await first.store.set('access', 'A2');
282+
assert.strictEqual(await second.store.get('refresh'), 'R2', 'a second tab must not present the refresh token that was rotated away');
283+
assert.strictEqual(await second.store.get('access'), 'A2');
284+
});
285+
await test('secrets: a change is one key — a tab does not write back what it saw at start-up', async () => {
286+
const storage = memoryStorage();
287+
const key = await newKey();
288+
const locks = sharedLocks();
289+
const first = await tab({ storage, key, locks });
290+
await first.store.set('access', 'A1');
291+
const second = await tab({ storage, key, locks });
292+
await first.store.set('refresh', 'R2'); // after the second tab started
293+
await second.store.set('verifier', 'V'); // an unrelated change in the second tab
294+
assert.strictEqual(await first.store.get('refresh'), 'R2', 'the second tab\'s write did not undo the first\'s');
295+
assert.strictEqual(await first.store.get('verifier'), 'V');
296+
await second.store.delete('access'); // the second tab signs out of the access token
297+
assert.strictEqual(await first.store.get('refresh'), 'R2');
298+
assert.strictEqual(await first.store.get('access'), undefined);
299+
});
300+
await test('secrets: changes made at the same moment in two tabs all land, one at a time', async () => {
301+
const storage = memoryStorage();
302+
const key = await newKey();
303+
const locks = sharedLocks();
304+
const a = await tab({ storage, key, locks });
305+
const b = await tab({ storage, key, locks });
306+
const work = [];
307+
for (let i = 0; i < 12; i++) {
308+
work.push(a.store.set('a' + i, 'x' + i));
309+
work.push(b.store.set('b' + i, 'y' + i));
310+
}
311+
await Promise.all(work);
312+
const keys = await a.store.keys();
313+
assert.strictEqual(keys.length, 24, 'lost: ' + [...Array(12).keys()].flatMap((i) => ['a' + i, 'b' + i]).filter((k) => !keys.includes(k)).join(','));
314+
assert.strictEqual(locks.held.max, 1, 'never two changes inside the lock at once');
315+
});
316+
await test('secrets: with no Web Locks a tab still applies its own changes one at a time', async () => {
317+
const storage = memoryStorage();
318+
const a = await tab({ storage, key: await newKey() });
319+
await Promise.all([...Array(10).keys()].map((i) => a.store.set('k' + i, 'v' + i)));
320+
assert.strictEqual((await a.store.keys()).length, 10);
321+
});
322+
await test('secrets: a store that cannot be read is dropped, with a warning, and the tab carries on', async () => {
323+
const storage = memoryStorage();
324+
storage.setItem(STORE_KEY, 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA');
325+
const a = await tab({ storage, key: await newKey() });
326+
assert.strictEqual(await a.store.get('refresh'), undefined);
327+
assert.strictEqual(storage.getItem(STORE_KEY), null);
328+
assert.strictEqual(a.logs.warn.length >= 1, true);
329+
await a.store.set('refresh', 'R');
330+
assert.strictEqual(await a.store.get('refresh'), 'R');
331+
});
332+
await test('secrets: a key from another browser profile cannot read it — that is a sign-in again, not an error', async () => {
333+
const storage = memoryStorage();
334+
const mine = await tab({ storage, key: await newKey() });
335+
await mine.store.set('refresh', 'R');
336+
const other = await tab({ storage, key: await newKey() });
337+
assert.strictEqual(await other.store.get('refresh'), undefined);
338+
});
339+
await test('secrets: where there is no key the store is in memory, writes nothing, and works for the session', async () => {
340+
const storage = memoryStorage();
341+
const a = await tab({ storage, key: null, noKey: true });
342+
assert.strictEqual(a.store.type, 'in-memory');
343+
await a.store.set('access', 'A');
344+
assert.strictEqual(await a.store.get('access'), 'A');
345+
assert.deepStrictEqual(await a.store.keys(), ['access']);
346+
await a.store.delete('access');
347+
assert.strictEqual(await a.store.get('access'), undefined);
348+
assert.strictEqual(storage.m.size, 0, 'nothing was written to localStorage');
349+
});
350+
await test('secrets: when the browser refuses the write the session is kept for this tab, not lost', async () => {
351+
const storage = memoryStorage();
352+
const key = await newKey();
353+
const a = await tab({ storage, key });
354+
await a.store.set('refresh', 'R1');
355+
const realSet = storage.setItem;
356+
storage.setItem = () => { throw new Error('QuotaExceededError'); };
357+
await a.store.set('access', 'A1'); // refused: kept in memory, with R1 carried over
358+
storage.setItem = realSet;
359+
assert.strictEqual(await a.store.get('access'), 'A1');
360+
assert.strictEqual(await a.store.get('refresh'), 'R1');
361+
await a.store.set('refresh', 'R2');
362+
assert.strictEqual(await a.store.get('refresh'), 'R2');
363+
assert.ok(a.logs.error.length >= 1, 'said so');
364+
});
365+
200366
/* ---- startReturnFromSignIn --------------------------------------------------------------------- */
201367
function makeStart(href, { drained = 0, listening = true, sessionThrows = false } = {}) {
202368
const calls = { replaced: [], drain: [], fired: [], removed: [] };

0 commit comments

Comments
 (0)