Skip to content

fix(auth/connection): soft-signout on refresh-token 401, immediate reconnect on visibility/online - #749

Open
lucletoffe wants to merge 2 commits into
bulwarkmail:mainfrom
lucletoffe:fix/session-restore-and-reconnect
Open

fix(auth/connection): soft-signout on refresh-token 401, immediate reconnect on visibility/online#749
lucletoffe wants to merge 2 commits into
bulwarkmail:mainfrom
lucletoffe:fix/session-restore-and-reconnect

Conversation

@lucletoffe

Copy link
Copy Markdown

Two related resilience fixes for flaky networks and expiring sessions:

  1. Soft sign-out instead of full logout on refresh-token 401. When the server rejects a refresh token (or a basic-auth session cookie fails to decrypt), the current behavior evicts the whole account entry — identities, subscriptions and cached settings are lost and the user has to re-add the account from scratch. This PR keeps the account entry and marks it signed-out (softSignOut(accountId, reason)), so the user just re-authenticates from the switcher. logout() is unchanged for user-initiated sign-out. Unit tests included (stores/__tests__/auth-store-soft-signout.test.ts).

  2. Immediate reconnect on visibilitychange/online. After a laptop resume or a network blip, the client currently waits for the next scheduled retry before re-establishing the EventSource/JMAP connection. This PR forces an immediate reconnect attempt when the tab becomes visible or the browser reports connectivity back (hooks/use-resume-connectivity.ts), with a test covering the debounce.

Rebased on current main (conflict with the new async logout() signature resolved), tsc --noEmit clean.

The 'Attempting to reconnect…' banner clears only when the keep-alive
ping succeeds. That ping runs every 30 s, but on failure it backs off
exponentially up to ~5 minutes between attempts (pingSkipRemaining =
2^failureCount, capped at 10). So after a laptop resume, a Wi-Fi switch,
or the user returning from lunch, the banner can sit for several
minutes even though the server is reachable again.

This adds a JMAPClient.resumeConnectivity() entry point that bypasses
the skip counter and either fires an immediate ping (session already
open) or a full reconnect (session never established). On success it
clears the banner via the connectionChange callback and wipes the
backoff counters; on failure it stays silent and lets the next
scheduled tick retry.

A new hook wires resumeConnectivity() to three browser signals — tab
visibility flipping to 'visible', window 'online', window 'focus' —
which together cover the situations where we know something changed.
The hook is a no-op without an authenticated client.

- lib/jmap/client-interface.ts: extend IJMAPClient with
  resumeConnectivity() (+ DemoJMAPClient no-op that re-emits connected).
- lib/jmap/client.ts: implement resumeConnectivity(), including the
  ping/reconnect fallthrough and the skip-counter reset.
- hooks/use-resume-connectivity.ts: bind visibilitychange/online/focus
  to the active client.
- app/(main)/[locale]/page.tsx: mount the hook alongside
  useIdentitySync so it runs for the whole authenticated shell.
- lib/jmap/__tests__/resume-connectivity.test.ts: 6 tests covering
  ping success, ping→reconnect fallthrough, missing apiUrl,
  intentional disconnect, rate-limited, and both-attempts-fail.
Symptom Luc hits: the 'Attempting to connect' banner sticks, he
reloads, and the login screen greets him with an empty form — every
tag preference, identity, subscription and cached message list gone.
Even 'remembered' accounts vanish because a full logout evicts them
from useAccountStore.

Root cause: refreshAccessToken() treated any 401 from
/api/auth/token as a definitive session end and called get().logout(),
which then calls accountStore.removeAccount(). That's the right
response when the user actually signs out, wrong when the server
rotated / restarted / temporarily lost its refresh grant. Real users
don't want to re-add a mail server just because a token expired.

Soft-signout keeps the account row. It:
- disconnects the JMAP client for that account and evicts its
  per-account store snapshot from memory,
- marks the account { needsReauth: true, hasError: true } so the
  switcher can offer 'Sign in again to <email>',
- deletes the failed session/refresh cookies (leaving them causes an
  infinite re-fail loop on the next restore),
- switches to another connected account if there is one, otherwise
  clears the shell and redirects to login (the account row is still
  in useAccountStore, ready for a one-tap re-auth once the UI wires it).

Two call sites now route through softSignOut:
- refreshAccessToken(): 401 with a bound activeAccountId — the common
  case. When accountId is somehow null we fall back to logout() for
  defence in depth.
- checkAuth() restoration: the 'definitive rejection' path used to
  evictAccount + removeAccount. Same treatment as above: mark
  needsReauth, drop cookies, keep the row.

Login flow (basic + OAuth) already re-uses an existing account entry
via accountStore.addAccount → updateAccount pattern; the updateAccount
call now also clears needsReauth so a successful re-login re-arms the
switcher label.

- stores/account-store.ts: AccountEntry.needsReauth?: boolean
- stores/auth-store.ts:
  - AuthState.softSignOut(accountId, reason?)
  - refreshAccessToken() 401 branch: softSignOut(activeId) if bound,
    else fall through to logout()
  - checkAuth() 401 branch: updateAccount({ needsReauth: true }) +
    cookie delete, no more accountStore.removeAccount()
  - login success path clears needsReauth on the updateAccount step
- 5 tests in stores/__tests__/auth-store-soft-signout.test.ts covering
  the flag flip, cookie deletion, cookieSlot preservation, the 401
  refresh integration, and the login-success re-arm.

Pre-existing flake noted: lib/__tests__/jmap-client-resilience.test.ts
one test ("fires with true on successful ping during keep-alive")
fails intermittently in the full suite but passes when run alone. Not
caused by this change — reproduced on main with these changes stashed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant