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

Open
  • Is something wrong?
#830 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
76/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
react, typescript
Domain
authentication, frontend

Research direction

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.

Written by the indexing model from the issue text.

Description

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.

Dominant language
TypeScript
Stars
1
Forks
94
Avg merge
1h 2m
Merged PRs (30d)
25

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from MergeFi/frontend

All issues in MergeFi/frontend

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.