Skip to content

Commit 95178cf

Browse files
committed
fix(cloud): #96 review, round two — another window's sign-in; a cancelled request starts no renewal
1. A sign-in in ANOTHER window could still be replayed into. Round one bound a request to the host's sessionGeneration, which counts the sessions THIS window has been through. The stored tokens belong to every window: one that signs in as someone else changes them without moving this window's count. A 401 arriving after that found "a different token for the same gateway", and the old account's prompt went out again on the new account's token — an inline edit in flight, or the nodes still to come of a Sketch run. What every window can see, and a renewal does not change, is who the token says it is for. providers/session.js gains jwtSubject: `sub`, read the way `exp` already is, not verified. A different stored token is a renewal only if it names the subject the refused one named — checked where the count is, before refreshing and again before re-sending. Tokens that name no subject cannot be told apart this way and are left to the count; one that stops, or starts, naming a subject is not taken for the same account. The same-account case still works across windows: a token another window renewed is used, with no second refresh of it. 2. A request cancelled while the stored session was being read still started a renewal. Cancel, Stop or the next keystroke could land during that read; recovery went on to start the refresh before it looked at the signal, and for ghost text used up the minute's one renewal. The signal is now checked as soon as the read returns: a request nobody is waiting for starts nothing. Verified: 48 suites, 924 cases, 0 failing — on develop, which this branch now contains (d056588: #92 and #95 have merged). session.test.js goes from 15 to 19 cases, authRetry.test.js from 39 to 45, authRetryCallers.test.js from 72 to 76: another window signing in as someone else while an inline edit is out, and half-way through a Sketch run, against the shipped host code; another window renewing the same session; a keystroke landing during the read. On the module as last pushed, the new cases fail and the rest pass. 55 breakages — each rule removed, each caller reverted, each option dropped — fail a case each. node --check is clean, and tsc --checkJs reports nothing new. Not run in the editor. The subject check rests on the access token being a JWT that carries `sub`, as the session tests model it; the backend is not in this repository.
1 parent 125f1ae commit 95178cf

6 files changed

Lines changed: 238 additions & 25 deletions

File tree

‎extensions/levelcode-ai/extension.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,8 @@ async function refreshGatewayToken() {
671671
* ONE instance for the window, so that requests which fail together wait on a single renewal. And it
672672
* is told which session this window is on (sessionGeneration), for the reason the refresh keeps that
673673
* count: a request belongs to the session it was sent in. A token stored by a LATER sign-in is not a
674-
* renewal of the old one, and is never put on a request the earlier session sent.
674+
* renewal of the old one, and is never put on a request the earlier session sent. (A sign-in made in
675+
* ANOTHER window this count cannot see; for that, authRetry goes by who the tokens say they are for.)
675676
*/
676677
const authRetry = createAuthRetry({ prepProviderRequest, refreshGatewayToken, isAuthError, sessionGeneration: () => sessionGeneration, dbg });
677678

‎extensions/levelcode-ai/providers/authRetry.js‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
* - Only a GATEWAY request is renewed. A 401 from the user's own provider is a wrong key, and no
2121
* amount of refreshing a cloud session fixes that: it is rethrown untouched.
2222
* - Only an auth failure, only before any of the answer has arrived (a second send would repeat
23-
* it), and not once the caller has aborted. One retry: the second failure is the answer.
23+
* it), and not once the caller has aborted — a request nobody is waiting for starts no renewal
24+
* either, wherever in the recovery the abort lands. One retry: the second failure is the answer.
2425
* - It never ends a session. One thing does — the refresh endpoint answering 401, inside the
2526
* host's own refresh. This only FINDS OUT, by asking the host for a provider again: `signedOut`
2627
* means the session is over, and the caller gets the sentence the chat shows — coded
@@ -41,7 +42,10 @@
4142
* sign-in as someone else, or a change of cloud host, the stored token is not this request's.
4243
* Putting it on the request would send one host's credential to another, or replay one
4344
* account's prompt as another's — so recovery stops there, before refreshing and again before
44-
* re-sending, and the request's own error stands.
45+
* re-sending, and the request's own error stands. Two things say whose a token is. The host
46+
* counts the sessions THIS window has been through. What another window does to the store
47+
* they share no window can count — but the tokens name their subject, and a token that names
48+
* a different one is another account's, whoever stored it.
4549
* - A request nobody asked for (ghost text) may START a renewal once a minute at most. It is
4650
* sent on every pause in typing; a gateway that keeps answering 401 must not turn typing into
4751
* a stream of refreshes, each one rotating the session's credentials. Waiting on a renewal
@@ -50,7 +54,7 @@
5054
// @ts-check
5155
'use strict';
5256

53-
const { SESSION_EXPIRED_MESSAGE } = require('./session');
57+
const { SESSION_EXPIRED_MESSAGE, jwtSubject } = require('./session');
5458

5559
/** How often requests the user did not ask for may START a renewal. */
5660
const BACKGROUND_RENEWAL_INTERVAL_MS = 60 * 1000;
@@ -138,9 +142,11 @@ function createAuthRetry(host) {
138142
*
139143
* 'ended' — the session is over: the host answers `signedOut`
140144
* 'superseded' — the request is no longer this session's to recover. The session it was sent
141-
* in has been replaced (a sign-in, a sign-out), or the host would now send it
142-
* somewhere else: another gateway URL, the user's own provider, nowhere.
143-
* Whatever token is stored for THAT is not put on this request.
145+
* in has been replaced — a sign-in or a sign-out here (the host's count), a
146+
* sign-in as someone else in another window (the stored token names another
147+
* subject) — or the host would now send it somewhere else: another gateway
148+
* URL, the user's own provider, nowhere. Whatever token is stored for THAT is
149+
* not put on this request.
144150
* 'renewed' — same session, same gateway, a different token in place: `apiKey`
145151
* 'same' — the token that was refused is still the one stored
146152
*
@@ -153,7 +159,12 @@ function createAuthRetry(host) {
153159
if (cur && !cur.ok && cur.reason === 'signedOut') { return { state: 'ended' }; }
154160
const sameGateway = !!cur && cur.ok && cur.gateway && cur.providerId === req.providerId && cur.baseURL === req.baseURL;
155161
if (!sameGateway || generation() !== sessionOf.get(req)) { return { state: 'superseded' }; }
156-
return cur.apiKey && cur.apiKey !== sent ? { state: 'renewed', apiKey: cur.apiKey } : { state: 'same' };
162+
if (!cur.apiKey || cur.apiKey === sent) { return { state: 'same' }; }
163+
// A different token. The store is shared with the other windows, and their sign-ins are not in
164+
// this window's count: it is a renewal only if it is for whoever the refused one was for. (Tokens
165+
// that name no subject cannot be told apart this way, and are left to the checks above.)
166+
if (jwtSubject(cur.apiKey) !== jwtSubject(sent)) { return { state: 'superseded' }; }
167+
return { state: 'renewed', apiKey: cur.apiKey };
157168
}
158169

159170
/**
@@ -165,6 +176,9 @@ function createAuthRetry(host) {
165176
*/
166177
async function recover(req, sent, o) {
167178
let cur = await look(req, sent);
179+
// Cancel, Stop or the next keystroke can land while that read is out. A request nobody is waiting
180+
// for any more starts no renewal — and, for ghost text, does not use up the minute.
181+
if (aborted(o.signal)) { return { outcome: 'aborted' }; }
168182
if (cur.state === 'renewed') { return { outcome: 'already-renewed', apiKey: cur.apiKey }; }
169183
if (cur.state !== 'same') { return { outcome: cur.state }; } // ended, or no longer this request's: nothing to refresh
170184
// The limit is on STARTING a renewal. One that is already out costs nothing to wait on — and the

‎extensions/levelcode-ai/providers/session.js‎

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,51 @@ const REFRESH_TIMEOUT_MS = 10 * 1000;
2626
const SESSION_EXPIRED_MESSAGE = 'Your LevelCode Cloud session has expired. Sign in again to continue.';
2727

2828
/**
29-
* The `exp` claim of a JWT as epoch milliseconds, or null when the token is not a JWT, carries no
30-
* `exp`, or is unreadable. Never throws: a malformed token is a reason to re-check with the server,
31-
* not a reason to crash the editor.
29+
* The payload of a JWT, or null when the token is not a JWT or is unreadable. Never throws.
3230
* @param {string|null|undefined} token
33-
* @returns {number|null}
31+
* @returns {any}
3432
*/
35-
function jwtExpiresAt(token) {
33+
function jwtPayload(token) {
3634
try {
3735
const parts = String(token || '').split('.');
3836
if (parts.length !== 3) { return null; }
3937
const b64 = parts[1].replace(/-/g, '+').replace(/_/g, '/');
4038
const payload = JSON.parse(Buffer.from(b64 + '='.repeat((4 - b64.length % 4) % 4), 'base64').toString('utf8'));
41-
const exp = Number(payload && payload.exp);
42-
return Number.isFinite(exp) && exp > 0 ? exp * 1000 : null;
39+
return payload && typeof payload === 'object' ? payload : null;
4340
} catch { return null; }
4441
}
4542

43+
/**
44+
* The `exp` claim of a JWT as epoch milliseconds, or null when the token is not a JWT, carries no
45+
* `exp`, or is unreadable. Never throws: a malformed token is a reason to re-check with the server,
46+
* not a reason to crash the editor.
47+
* @param {string|null|undefined} token
48+
* @returns {number|null}
49+
*/
50+
function jwtExpiresAt(token) {
51+
const payload = jwtPayload(token);
52+
const exp = Number(payload && payload.exp);
53+
return Number.isFinite(exp) && exp > 0 ? exp * 1000 : null;
54+
}
55+
56+
/**
57+
* Who a JWT says it is for: its `sub` claim, as a string — or null when the token is not a JWT, is
58+
* unreadable, or names no subject. Read, not verified, like `exp`: it is only asking the token.
59+
*
60+
* It is the one thing about a session that every window can see and that a renewal does not change.
61+
* The stored tokens belong to all the windows, and a window only counts the sign-ins it makes
62+
* itself; but two tokens that name different subjects are two accounts, whichever window stored the
63+
* second one.
64+
* @param {string|null|undefined} token
65+
* @returns {string|null}
66+
*/
67+
function jwtSubject(token) {
68+
const payload = jwtPayload(token);
69+
const sub = payload ? payload.sub : undefined;
70+
if (typeof sub === 'string') { return sub || null; }
71+
return typeof sub === 'number' && Number.isFinite(sub) ? String(sub) : null;
72+
}
73+
4674
/**
4775
* Whether an access token should be refreshed before use: it expires within the margin, has
4876
* already expired, or cannot be read at all (an unreadable token is treated as expired — the
@@ -97,4 +125,4 @@ function isSessionExpiredError(e) {
97125
return status === 401 || /\bAPI 401\b|token_expired|refresh_expired|signature has expired/i.test(msg);
98126
}
99127

100-
module.exports = { EXPIRY_MARGIN_MS, REFRESH_TIMEOUT_MS, SESSION_EXPIRED_MESSAGE, jwtExpiresAt, accessNeedsRefresh, classifyRefresh, isSessionExpiredError };
128+
module.exports = { EXPIRY_MARGIN_MS, REFRESH_TIMEOUT_MS, SESSION_EXPIRED_MESSAGE, jwtExpiresAt, jwtSubject, accessNeedsRefresh, classifyRefresh, isSessionExpiredError };

‎extensions/levelcode-ai/test/authRetry.test.js‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* - requests that fail together wait on one renewal, and one refused on a token that has been
1414
* renewed since is simply sent again
1515
* - a request stays with the session and the gateway it was sent on: a token stored by a later
16-
* sign-in, or for another cloud host, is never put on it
16+
* sign-in — in this window or in another — or for another cloud host, is never put on it
1717
* - a request nobody asked for (ghost text) cannot turn typing into a stream of refreshes — but
1818
* is never kept from waiting on a renewal that is already out
1919
*
@@ -42,6 +42,15 @@ function within(promise, ms, what) {
4242
return Promise.race([promise, late]).finally(() => clearTimeout(timer));
4343
}
4444

45+
/** An unsigned JWT with the given payload: a real access token says who it is for. */
46+
function jwt(payload) {
47+
const b64 = (o) => Buffer.from(JSON.stringify(o)).toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '');
48+
return b64({ alg: 'HS256', typ: 'JWT' }) + '.' + b64(payload) + '.sig';
49+
}
50+
/** Ada's n-th access token, and one of Bo's. */
51+
const ada = (n) => jwt({ sub: 'ada', exp: 1_800_000_000 + n });
52+
const BO = jwt({ sub: 'bo', exp: 1_800_000_000 });
53+
4554
/** The gateway's answer to a lapsed access token, as the openai adapter throws it. */
4655
const gateway401 = () => Object.assign(new Error('LevelCode Cloud API 401: Signature has expired'), { status: 401 });
4756

@@ -365,6 +374,53 @@ async function test(name, fn) { await fn(); n++; console.log(' ok - ' + name);
365374
assert.deepStrictEqual(w.dbg, ['auth.retry superseded']);
366375
});
367376

377+
// What another WINDOW does to the store they share is in nobody's count. The tokens say whose they are.
378+
await test('another WINDOW signed in as someone else: this window counted nothing, the token names another subject — not replayed', async () => {
379+
const w = window_({ token: ada(1) });
380+
const req = w.request();
381+
w.token = BO; // the shared store, changed elsewhere: no sign-in here, same host, same provider
382+
const e = await failure(w.authRetry(req, w.send));
383+
assert.ok(/API 401/.test(e.message), 'the request\'s own error');
384+
assert.deepStrictEqual(w.sent, [ada(1)], 'Ada\'s prompt is not sent again on Bo\'s token');
385+
assert.strictEqual(w.renewals, 0, 'and Bo\'s session is not refreshed for it');
386+
assert.deepStrictEqual(w.dbg, ['auth.retry superseded']);
387+
});
388+
389+
await test('another window RENEWED the same account: the same subject, so it is sent again on that token', async () => {
390+
const w = window_({ token: ada(1) });
391+
const req = w.request();
392+
w.token = ada(2);
393+
assert.strictEqual(await w.authRetry(req, w.send), 'answered on ' + ada(2));
394+
assert.deepStrictEqual({ renewals: w.renewals, dbg: w.dbg }, { renewals: 0, dbg: ['auth.retry already-renewed'] });
395+
});
396+
397+
await test('the renewal itself comes back as someone else\'s (the other window signed in just before it): not this request\'s', async () => {
398+
const w = window_({ token: ada(1), renewal: async () => { w.token = BO; return 'claim'; } });
399+
const e = await failure(w.authRetry(w.request(), w.send));
400+
assert.ok(/API 401/.test(e.message));
401+
assert.deepStrictEqual({ sent: w.sent, dbg: w.dbg }, { sent: [ada(1)], dbg: ['auth.retry superseded'] });
402+
});
403+
404+
await test('a renewal within the account is taken as one, by tokens that say who they are for', async () => {
405+
const w = window_({ token: ada(1), renewal: async () => { w.token = ada(2); return 'claim'; } });
406+
assert.strictEqual(await w.authRetry(w.request(), w.send), 'answered on ' + ada(2));
407+
});
408+
409+
await test('a token that cannot be read as the same subject is not taken for the same account', async () => {
410+
for (const [sentOn, stored] of [[ada(1), 'opaque-token'], ['opaque-token', ada(2)], [jwt({ sub: 7 }), jwt({ sub: '8' })]]) {
411+
const w = window_({ token: sentOn });
412+
const req = w.request();
413+
w.token = stored;
414+
await failure(w.authRetry(req, w.send));
415+
assert.deepStrictEqual(w.sent, [sentOn], JSON.stringify([sentOn, stored]));
416+
}
417+
// …while 7 and '7' are one subject: a server may write the id either way.
418+
const w = window_({ token: jwt({ sub: 7, exp: 1 }) });
419+
const req = w.request();
420+
w.token = jwt({ sub: '7', exp: 2 });
421+
assert.strictEqual(await w.authRetry(req, w.send), 'answered on ' + w.token);
422+
});
423+
368424
await test('the same URL under another provider is not the same gateway', async () => {
369425
const stored = { ok: true, gateway: true, providerId: 'openai', apiKey: 'access-1', baseURL: 'https://cloud.test/ai' };
370426
const authRetry = createAuthRetry({
@@ -416,6 +472,18 @@ async function test(name, fn) { await fn(); n++; console.log(' ok - ' + name);
416472
assert.strictEqual(w.renewals, 1);
417473
});
418474

475+
await test('aborted while the stored session is being READ: no renewal is started — and ghost text\'s minute is not used up', async () => {
476+
const w = window_({ renewal: 'fail' });
477+
const ac = new AbortController();
478+
w.onPrep = (nth) => { if (nth === 1) { ac.abort(); } }; // the keystroke lands while authRetry is looking at what is stored
479+
const e = await failure(w.authRetry(w.request(), w.send, { background: true, signal: ac.signal }));
480+
assert.ok(/API 401/.test(e.message));
481+
assert.deepStrictEqual({ renewals: w.renewals, dbg: w.dbg }, { renewals: 0, dbg: ['auth.retry aborted'] });
482+
w.onPrep = null;
483+
await failure(w.authRetry(w.request(), w.send, { background: true })); // the next pause, well inside the minute
484+
assert.deepStrictEqual({ renewals: w.renewals, dbg: w.dbg }, { renewals: 1, dbg: ['auth.retry aborted', 'auth.retry failed'] }, 'it may still start the minute\'s one renewal');
485+
});
486+
419487
await test('aborted AFTER the renewal landed, before the re-send: the new token is kept, the request is not sent', async () => {
420488
const w = window_();
421489
const ac = new AbortController();

0 commit comments

Comments
 (0)