nandi/frq · b8610a1

nandi/frqpublic⑂ Fork 0

⑂ b8610a1

Spending the refresh token, which nothing ever did

`resume` stored a `refresh_token` and no code path reached for it. An
access token is good for minutes -- the profile says under thirty and
recommends five -- and the app stays open for longer, so a reader who
signed in and came back held a session that looked complete and a PDS
that refused it. That is the 401 at `com.atproto.server.getSession`
from `prepare`, and before the last commit it was also the silent slide
into a guest id while signed in.

`prepare` now treats a refusal as what it nearly always is: an expired
token. Spend the refresh, ask again. The replacement is saved before
anything else can fail, because refresh tokens are single-use and one
lost in flight is a session that cannot be recovered.

When the refresh will not trade either, the session is cleared rather
than kept. "Signed in, and nothing works" is a worse place to leave
somebody than a sign-in button.

`nim/web/test/session.js` covers it offline with a stub PDS, and joins
`just test web`. Reverting the retry reproduces the reported error
verbatim.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nandi committed 2026-09-20T12:39:56-07:00 Browse files

b8610a1 parent: 7b4d21d

modified flutter/web/frq_oauth.js +53 -3 Viewed

@@ -305,6 +305,10 @@
305	305	accessJwt: t.access_token,
306	306	refresh: t.refresh_token \|\| '',
307	307	pds: pending.pds,
	308	+ // Where a refresh goes. Discovery would find it again, but it is two
	309	+ // round trips to learn something already known, on the path taken
	310	+ // every time the app opens.
	311	+ token: pending.token,
308	312	dpopNonce: who.nonce \|\| '',
309	313	};
310	314	drop(PENDING);
@@ -322,6 +326,42 @@
322	326	dpop().forget();
323	327	}
324	328
	329	+ // Trade the refresh token for a fresh access token.
	330	+ //
	331	+ // This was the missing half of the session: `resume` stored a
	332	+ // `refresh_token` and nothing ever spent it. An access token is good for
	333	+ // minutes -- the profile says under thirty and recommends five -- so a
	334	+ // reader who signed in and came back an hour later had a session that
	335	+ // looked complete, a PDS that refused it, and a client that shrugged and
	336	+ // connected as a guest.
	337	+ //
	338	+ // The new refresh token replaces the old one and the old one is spent:
	339	+ // they are single-use, so it is saved before anything else can fail.
	340	+ async function refresh() {
	341	+ const s = saved();
	342	+ if (!s \|\| !s.refresh) throw new Error('not signed in');
	343	+ const where = s.token \|\| (await discover(s.pds)).token;
	344	+ const r = await postForm(where, form([
	345	+ ['grant_type', 'refresh_token'],
	346	+ ['refresh_token', s.refresh],
	347	+ ['client_id', clientId()],
	348	+ ]), null);
	349	+ if (r.status !== 200) {
	350	+ // The refresh token is gone or was revoked, and no retry will bring it
	351	+ // back. Clearing the session is what turns "signed in, and nothing
	352	+ // works" into a sign-in button.
	353	+ forget();
	354	+ throw new Error('The session has expired; sign in again (' +
	355	+ r.status + '): ' + r.body);
	356	+ }
	357	+ const t = JSON.parse(r.body);
	358	+ s.accessJwt = t.access_token;
	359	+ if (t.refresh_token) s.refresh = t.refresh_token;
	360	+ s.token = where;
	361	+ save(SESSION, s);
	362	+ return s;
	363	+ }
	364	+
325	365	// Mint the proof freeq will present to the PDS on our behalf.
326	366	//
327	367	// For `GET {pds}/xrpc/com.atproto.server.getSession` and bound to the
@@ -333,9 +373,19 @@
333	373	// access token lives about an hour, and the only thing that comes back
334	374	// through IRC when it has expired is a bare failure.
335	375	async function prepare() {
336		- const s = saved();
	376	+ let s = saved();
337	377	if (!s) throw new Error('not signed in');
338		- const who = await whoami(s.pds, s.accessJwt);
	378	+ let who;
	379	+ try {
	380	+ who = await whoami(s.pds, s.accessJwt);
	381	+ } catch (e) {
	382	+ // Asking who the token belongs to is also how its age is discovered.
	383	+ // A refusal here is nearly always an expired access token, so spend
	384	+ // the refresh token and ask once more; if that fails it throws, and
	385	+ // saying so beats connecting as somebody else.
	386	+ s = await refresh();
	387	+ who = await whoami(s.pds, s.accessJwt);
	388	+ }
339	389	if (who.handle) s.handle = who.handle;
340	390	if (who.did) s.did = who.did;
341	391	if (who.nonce) s.dpopNonce = who.nonce;
@@ -345,5 +395,5 @@
345	395	return s;
346	396	}
347	397
348		- window.frqOauth = { begin, resume, saved, forget, prepare };
	398	+ window.frqOauth = { begin, resume, saved, forget, prepare, refresh };
349	399	})();

        accessJwt: t.access_token,
        refresh: t.refresh_token || '',
        pds: pending.pds,
++      // Where a refresh goes. Discovery would find it again, but it is two
++      // round trips to learn something already known, on the path taken
++      // every time the app opens.
++      token: pending.token,
        dpopNonce: who.nonce || '',
      };
      drop(PENDING);
      dpop().forget();
    }
++  // Trade the refresh token for a fresh access token.
++  //
++  // This was the missing half of the session: `resume` stored a
++  // `refresh_token` and nothing ever spent it. An access token is good for
++  // minutes -- the profile says under thirty and recommends five -- so a
++  // reader who signed in and came back an hour later had a session that
++  // looked complete, a PDS that refused it, and a client that shrugged and
++  // connected as a guest.
++  //
++  // The new refresh token replaces the old one and the old one is spent:
++  // they are single-use, so it is saved before anything else can fail.
++  async function refresh() {
++    const s = saved();
++    if (!s || !s.refresh) throw new Error('not signed in');
++    const where = s.token || (await discover(s.pds)).token;
++    const r = await postForm(where, form([
++      ['grant_type', 'refresh_token'],
++      ['refresh_token', s.refresh],
++      ['client_id', clientId()],
++    ]), null);
++    if (r.status !== 200) {
++      // The refresh token is gone or was revoked, and no retry will bring it
++      // back. Clearing the session is what turns "signed in, and nothing
++      // works" into a sign-in button.
++      forget();
++      throw new Error('The session has expired; sign in again (' +
++        r.status + '): ' + r.body);
++    }
++    const t = JSON.parse(r.body);
++    s.accessJwt = t.access_token;
++    if (t.refresh_token) s.refresh = t.refresh_token;
++    s.token = where;
++    save(SESSION, s);
++    return s;
++  }
++
    // Mint the proof freeq will present to the PDS on our behalf.
    //
    // For `GET {pds}/xrpc/com.atproto.server.getSession` and bound to the
    // access token lives about an hour, and the only thing that comes back
    // through IRC when it has expired is a bare failure.
    async function prepare() {
--    const s = saved();
++    let s = saved();
      if (!s) throw new Error('not signed in');
--    const who = await whoami(s.pds, s.accessJwt);
++    let who;
++    try {
++      who = await whoami(s.pds, s.accessJwt);
++    } catch (e) {
++      // Asking who the token belongs to is also how its age is discovered.
++      // A refusal here is nearly always an expired access token, so spend
++      // the refresh token and ask once more; if that fails it throws, and
++      // saying so beats connecting as somebody else.
++      s = await refresh();
++      who = await whoami(s.pds, s.accessJwt);
++    }
      if (who.handle) s.handle = who.handle;
      if (who.did) s.did = who.did;
      if (who.nonce) s.dpopNonce = who.nonce;
      return s;
    }
--  window.frqOauth = { begin, resume, saved, forget, prepare };
++  window.frqOauth = { begin, resume, saved, forget, prepare, refresh };
  })();

modified justfile +2 -1 Viewed

@@ -109,7 +109,8 @@ test suite="all" *args:
109	109	just _flutter layout test ;;
110	110	nim) just _nim-test "$@" ;;
111	111	web) just _nim-js
112		- exec node nim/web/test/smoke.js build/web/frq_core.js ;;
	112	+ node nim/web/test/smoke.js build/web/frq_core.js
	113	+ exec node nim/web/test/session.js ;;
113	114	dart) just _nim-lib
114	115	exec "{{tc}}" exec -- bash -c \
115	116	'cd dart/frq_core && dart pub get && dart test -r expanded' ;;

                  just _flutter layout test ;;
          nim)    just _nim-test "$@" ;;
          web)    just _nim-js
--                exec node nim/web/test/smoke.js build/web/frq_core.js ;;
++                node nim/web/test/smoke.js build/web/frq_core.js
++                exec node nim/web/test/session.js ;;
          dart)   just _nim-lib
                  exec "{{tc}}" exec -- bash -c \
                      'cd dart/frq_core && dart pub get && dart test -r expanded' ;;

added nim/web/test/session.js +112 -0 Viewed

new file mode 100644
@@ -0,0 +1,112 @@
	1	+// The sign-in that outlives its access token.
	2	+//
	3	+// node nim/web/test/session.js
	4	+//
	5	+// Offline: `fetch` is a stub here and no network is touched. What it covers
	6	+// is the half of the session that had no caller -- `resume` stored a
	7	+// `refresh_token` and nothing ever spent it, so a reader who came back an
	8	+// hour later held a session that looked complete and a PDS that refused it.
	9	+// An access token is good for minutes; the app is open for longer.
	10	+const fs = require('fs');
	11	+const assert = require('assert');
	12	+
	13	+let store = {};
	14	+global.localStorage = {
	15	+ getItem(k) { return k in store ? store[k] : null; },
	16	+ setItem(k, v) { store[k] = String(v); },
	17	+ removeItem(k) { delete store[k]; },
	18	+};
	19	+global.window = {
	20	+ location: { origin: 'http://127.0.0.1:8000', hostname: '127.0.0.1',
	21	+ pathname: '/', search: '', assign() {} },
	22	+ history: { replaceState() {} },
	23	+};
	24	+global.crypto = require('crypto').webcrypto;
	25	+global.btoa = (s) => Buffer.from(s, 'binary').toString('base64');
	26	+global.TextEncoder = require('util').TextEncoder;
	27	+global.URLSearchParams = URLSearchParams;
	28	+
	29	+// The PDS and the authorization server, as far as this cares: an access
	30	+// token is accepted only while it is the current one.
	31	+let live = 'fresh-token';
	32	+let refreshes = 0;
	33	+let refreshValid = true;
	34	+const calls = [];
	35	+global.fetch = async (url, opts) => {
	36	+ const body = String((opts && opts.body) \|\| '');
	37	+ calls.push(String(url));
	38	+ const reply = (status, obj, headers) => ({
	39	+ status: status,
	40	+ text: async () => JSON.stringify(obj),
	41	+ headers: { get: (h) => (headers \|\| {})[h.toLowerCase()] \|\| null },
	42	+ });
	43	+ if (String(url).endsWith('/xrpc/com.atproto.server.getSession')) {
	44	+ const bearer = (opts.headers.Authorization \|\| '').replace('DPoP ', '');
	45	+ if (bearer !== live) return reply(401, { error: 'InvalidToken' });
	46	+ return reply(200, { did: 'did:plc:abc', handle: 'someone.example' },
	47	+ { 'dpop-nonce': 'n1' });
	48	+ }
	49	+ if (String(url).endsWith('/token')) {
	50	+ assert.ok(body.includes('grant_type=refresh_token'), 'a refresh grant');
	51	+ assert.ok(body.includes('refresh_token=r1'), 'spends the stored token');
	52	+ if (!refreshValid) return reply(400, { error: 'invalid_grant' });
	53	+ refreshes += 1;
	54	+ live = 'second-token';
	55	+ return reply(200, { access_token: live, refresh_token: 'r2',
	56	+ scope: 'atproto transition:generic' });
	57	+ }
	58	+ throw new Error('unexpected request: ' + url);
	59	+};
	60	+
	61	+(0, eval)(fs.readFileSync('flutter/web/frq_dpop.js', 'utf8'));
	62	+(0, eval)(fs.readFileSync('flutter/web/frq_oauth.js', 'utf8'));
	63	+
	64	+const ok = (name) => console.log(' ok ' + name);
	65	+const session = (over) => Object.assign({
	66	+ did: 'did:plc:abc', handle: 'someone.example', accessJwt: 'fresh-token',
	67	+ refresh: 'r1', pds: 'https://pds.example',
	68	+ token: 'https://pds.example/token', dpopNonce: '',
	69	+}, over \|\| {});
	70	+
	71	+(async () => {
	72	+ // A token the PDS still accepts is used as it is.
	73	+ store = {}; live = 'fresh-token'; refreshes = 0;
	74	+ localStorage.setItem('frq:oauth:session', JSON.stringify(session()));
	75	+ let s = await window.frqOauth.prepare();
	76	+ assert.equal(refreshes, 0);
	77	+ assert.ok(s.dpopProof, 'a proof for freeq to present');
	78	+ ok('a live token is not refreshed');
	79	+
	80	+ // The nonce the PDS asked for is kept, so the proof minted at connect
	81	+ // carries one already.
	82	+ assert.equal(window.frqOauth.saved().dpopNonce, 'n1');
	83	+ ok('and the nonce it answered with is kept');
	84	+
	85	+ // The case that was silently broken.
	86	+ store = {}; live = 'fresh-token'; refreshes = 0; refreshValid = true;
	87	+ localStorage.setItem('frq:oauth:session',
	88	+ JSON.stringify(session({ accessJwt: 'stale' })));
	89	+ s = await window.frqOauth.prepare();
	90	+ assert.equal(refreshes, 1, 'the refresh token was spent');
	91	+ assert.equal(s.accessJwt, 'second-token');
	92	+ assert.equal(s.handle, 'someone.example', 'still knows who it is');
	93	+ ok('a token the PDS refuses is refreshed, once');
	94	+
	95	+ // Single-use: the replacement is stored, or the next refresh spends a
	96	+ // token that is already gone.
	97	+ assert.equal(window.frqOauth.saved().refresh, 'r2');
	98	+ ok('and the new refresh token replaces the spent one');
	99	+
	100	+ // No refresh left. Connecting as a guest here is the bug this is named
	101	+ // after; the session goes and the reader is asked to sign in.
	102	+ store = {}; live = 'fresh-token'; refreshValid = false;
	103	+ localStorage.setItem('frq:oauth:session',
	104	+ JSON.stringify(session({ accessJwt: 'stale' })));
	105	+ let threw = '';
	106	+ try { await window.frqOauth.prepare(); } catch (e) { threw = String(e); }
	107	+ assert.ok(/expired/.test(threw), 'says so: ' + threw);
	108	+ assert.equal(window.frqOauth.saved(), null, 'and the session is cleared');
	109	+ ok('a refusal that no refresh fixes ends the session, loudly');
	110	+
	111	+ console.log('all ok');
	112	+})().catch((e) => { console.error(e); process.exit(1); });

++// The sign-in that outlives its access token.
++//
++//     node nim/web/test/session.js
++//
++// Offline: `fetch` is a stub here and no network is touched. What it covers
++// is the half of the session that had no caller -- `resume` stored a
++// `refresh_token` and nothing ever spent it, so a reader who came back an
++// hour later held a session that looked complete and a PDS that refused it.
++// An access token is good for minutes; the app is open for longer.
++const fs = require('fs');
++const assert = require('assert');
++
++let store = {};
++global.localStorage = {
++  getItem(k) { return k in store ? store[k] : null; },
++  setItem(k, v) { store[k] = String(v); },
++  removeItem(k) { delete store[k]; },
++};
++global.window = {
++  location: { origin: 'http://127.0.0.1:8000', hostname: '127.0.0.1',
++              pathname: '/', search: '', assign() {} },
++  history: { replaceState() {} },
++};
++global.crypto = require('crypto').webcrypto;
++global.btoa = (s) => Buffer.from(s, 'binary').toString('base64');
++global.TextEncoder = require('util').TextEncoder;
++global.URLSearchParams = URLSearchParams;
++
++// The PDS and the authorization server, as far as this cares: an access
++// token is accepted only while it is the current one.
++let live = 'fresh-token';
++let refreshes = 0;
++let refreshValid = true;
++const calls = [];
++global.fetch = async (url, opts) => {
++  const body = String((opts && opts.body) || '');
++  calls.push(String(url));
++  const reply = (status, obj, headers) => ({
++    status: status,
++    text: async () => JSON.stringify(obj),
++    headers: { get: (h) => (headers || {})[h.toLowerCase()] || null },
++  });
++  if (String(url).endsWith('/xrpc/com.atproto.server.getSession')) {
++    const bearer = (opts.headers.Authorization || '').replace('DPoP ', '');
++    if (bearer !== live) return reply(401, { error: 'InvalidToken' });
++    return reply(200, { did: 'did:plc:abc', handle: 'someone.example' },
++                 { 'dpop-nonce': 'n1' });
++  }
++  if (String(url).endsWith('/token')) {
++    assert.ok(body.includes('grant_type=refresh_token'), 'a refresh grant');
++    assert.ok(body.includes('refresh_token=r1'), 'spends the stored token');
++    if (!refreshValid) return reply(400, { error: 'invalid_grant' });
++    refreshes += 1;
++    live = 'second-token';
++    return reply(200, { access_token: live, refresh_token: 'r2',
++                        scope: 'atproto transition:generic' });
++  }
++  throw new Error('unexpected request: ' + url);
++};
++
++(0, eval)(fs.readFileSync('flutter/web/frq_dpop.js', 'utf8'));
++(0, eval)(fs.readFileSync('flutter/web/frq_oauth.js', 'utf8'));
++
++const ok = (name) => console.log('  ok   ' + name);
++const session = (over) => Object.assign({
++  did: 'did:plc:abc', handle: 'someone.example', accessJwt: 'fresh-token',
++  refresh: 'r1', pds: 'https://pds.example',
++  token: 'https://pds.example/token', dpopNonce: '',
++}, over || {});
++
++(async () => {
++  // A token the PDS still accepts is used as it is.
++  store = {}; live = 'fresh-token'; refreshes = 0;
++  localStorage.setItem('frq:oauth:session', JSON.stringify(session()));
++  let s = await window.frqOauth.prepare();
++  assert.equal(refreshes, 0);
++  assert.ok(s.dpopProof, 'a proof for freeq to present');
++  ok('a live token is not refreshed');
++
++  // The nonce the PDS asked for is kept, so the proof minted at connect
++  // carries one already.
++  assert.equal(window.frqOauth.saved().dpopNonce, 'n1');
++  ok('and the nonce it answered with is kept');
++
++  // The case that was silently broken.
++  store = {}; live = 'fresh-token'; refreshes = 0; refreshValid = true;
++  localStorage.setItem('frq:oauth:session',
++    JSON.stringify(session({ accessJwt: 'stale' })));
++  s = await window.frqOauth.prepare();
++  assert.equal(refreshes, 1, 'the refresh token was spent');
++  assert.equal(s.accessJwt, 'second-token');
++  assert.equal(s.handle, 'someone.example', 'still knows who it is');
++  ok('a token the PDS refuses is refreshed, once');
++
++  // Single-use: the replacement is stored, or the next refresh spends a
++  // token that is already gone.
++  assert.equal(window.frqOauth.saved().refresh, 'r2');
++  ok('and the new refresh token replaces the spent one');
++
++  // No refresh left. Connecting as a guest here is the bug this is named
++  // after; the session goes and the reader is asked to sign in.
++  store = {}; live = 'fresh-token'; refreshValid = false;
++  localStorage.setItem('frq:oauth:session',
++    JSON.stringify(session({ accessJwt: 'stale' })));
++  let threw = '';
++  try { await window.frqOauth.prepare(); } catch (e) { threw = String(e); }
++  assert.ok(/expired/.test(threw), 'says so: ' + threw);
++  assert.equal(window.frqOauth.saved(), null, 'and the session is cleared');
++  ok('a refusal that no refresh fixes ends the session, loudly');
++
++  console.log('all ok');
++})().catch((e) => { console.error(e); process.exit(1); });