AuthContext.refresh() has no staleness guard: an in-flight resolve can resurrect a signed-out session or restore the previous account after a cross-tab switch

オープン
  • 何か問題がありますか？
#830 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
3/5
見積もり時間
1〜2日
初心者へのやさしさ
76/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
react, typescript
領域
authentication, frontend

調査の方向性

Start with src/context/AuthContext.tsx, especially refresh, handleTokenChangedElsewhere, and logout; then check src/lib/api.ts to see how requests read tokens and handle abort signals. Reproduce the delayed refresh and cross-tab logout or account switch described in the issue. Done means an outdated resolve cannot overwrite newer authentication state, and retries use the token subject they actually request.

索引モデルが issue の本文から書いたものです。

説明

bug security

Location

src/context/AuthContext.tsx:72-118 (refresh), together with :132-149 (handleTokenChangedElsewhere) and :120-130 (mount hydration).

Problem

refresh() writes setUser(profile) unconditionally when its request chain finally resolves (:92). Nothing records which resolve attempt it was, so there is no way for a later state change to invalidate an earlier in-flight one. The inFlight counter next to it only coordinates the loading flag; it does not order or cancel the writes.

The retry loop makes the window large, not small: up to 3 attempts with 200ms and 400ms sleeps between them (:85-114), on top of the 20s request timeout in apiRequest. A single refresh() can legitimately still be running several seconds after it started.

Three concrete failure paths:

1. Cross-tab logout is undone. handleTokenChangedElsewhere(null) (:134-137) does setUser(null); setDegraded(false); and nothing else — it does not cancel the in-flight resolve. If a refresh() was running when the other tab logged out, its /users/:id response (the token was still valid server-side when the request was issued) lands afterwards and calls setUser(profile). The tab is signed back in, with a token that no longer exists in localStorage — so every subsequent apiRequest goes out unauthenticated and 401s while the UI renders the full signed-in chrome.

2. Cross-tab account switch resolves backwards. Tab A is mid-refresh() for user X (say it is on retry 2). Tab B signs in as user Y, which fires the storage event, which starts a second refresh(). The second one resolves quickly; the first one resolves after it and overwrites user with X's profile. The tab now renders user X's identity, dashboards and — via WalletContext's user?.stellarAddress effect (src/context/WalletContext.tsx:239-256) — user X's payout address, while the stored token belongs to user Y.

3. The decoded subject goes stale across retries. const subject = getTokenSubject(token) is read once, before the loop (:84). Retries reuse it, but apiRequest re-reads the token from storage on every call (src/lib/api.ts:109). If the token changes between attempts, attempt 2 requests /users/<old user id> with the new user's bearer token. The backend answers 403 for another user's profile, which lands in the 401 || 403 branch and calls clearToken(); setUser(null) — a hard, silent logout of the account that just signed in, which is exactly the failure #4 was closed to prevent.

Reproduction (path 1, the easiest to see)

  1. Sign in. Open the app in two tabs.
  2. In tab A, throttle the network (or point /users/:id at a stub that delays ~3s) and trigger a re-resolve — reloading tab A is enough, since mount hydration calls refresh().
  3. While that request is in flight, click "Log out" in tab B.
  4. Tab A briefly renders signed-out, then flips back to signed-in when the delayed response lands.
  5. Click anything that calls the API in tab A: it fails with 401, because localStorage no longer holds a token.

Suggested fix

Give every resolve a generation number and drop writes from a superseded one:

const resolveGen = useRef(0);

const refresh = useCallback(async () => {
  const gen = ++resolveGen.current;
  const commit = <T,>(fn: () => T) => (gen === resolveGen.current ? fn() : undefined);
  ...
  for (let attempt = 0; attempt < MAX_RETRIES; attempt++) {
    if (gen !== resolveGen.current) return null;          // abandoned mid-retry
    const token = getToken();                              // re-read per attempt
    if (!token) { commit(() => setUser(null)); return null; }
    const subject = getTokenSubject(token);                // re-derive per attempt
    ...
    commit(() => { setUser(profile); setDegraded(false); });
  }
}, []);

and bump resolveGen.current in logout() and in the newValue === null branch of handleTokenChangedElsewhere, so a cross-tab logout invalidates anything in flight. Passing an AbortSignal through to apiRequest (it already honours init.signal) would additionally stop the wasted requests rather than just ignoring their results.

主要言語
TypeScript
スター
1
フォーク
94
平均マージ
1時間 2分
マージ済み PR（30日）
25

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

MergeFi/frontend のほかの issue

MergeFi/frontend の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。