feat(send): recover revoked sessions via silent refresh before forced logout

Address review on #960: on an x-logout (a Keycloak-revoked access token), the
frontend now attempts a silent refresh before tearing the session down, and
only forces logout when the refresh token is also gone. A transient refresh
error keeps the session (fail open).

- auth-store: new recoverOrForceLogout() runs the deduped silent refresh;
  refreshAccessToken now records whether a failure was genuine so the decision
  keys off the failure kind, not isLoggedIn (which can be false during an
  extension cold-start and would otherwise turn a network blip into a logout).
- api.ts / trpc.ts: on x-logout, recover and retry the request once with the
  fresh token instead of unconditionally forcing logout. In api.ts x-logout is
  branched ahead of the plain-401 path so there is exactly one refresh.

The OIDC refresh token lives only client-side (oidc-client-ts), so this
recovery is necessarily on the frontend; the backend only signals revocation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: aaspinwall

packages/send/frontend/src/lib/api.ts

@@ -129,8 +129,38 @@ export class ApiConnection {
       return null;
     }
 
-    // Handle authentication errors
-    if (resp.status === 401) {
+    // Handle authentication errors. x-logout always rides on a 401, so branch on
+    // it first and keep the two paths mutually exclusive — a revoked session
+    // gets exactly one recovery attempt, never a second refresh from the plain
+    // 401 branch below.
+    if (resp.headers?.get?.('x-logout')) {
+      // The backend flags a revoked OIDC session (logout / password change /
+      // admin force-logout). Try to recover with the refresh token before
+      // tearing down; recoverOrForceLogout forces logout only if the refresh
+      // token is also dead, and keeps the session on a transient error (#974).
+      try {
+        const { useAuthStore } =
+          await import('@send-frontend/stores/auth-store');
+        const authStore = useAuthStore();
+        const recovered = await authStore.recoverOrForceLogout();
+
+        if (recovered && requestHeaders['Authorization']) {
+          // Session rolled forward — retry once with the freshly-rotated token.
+          const newToken = await authStore.getAccessToken();
+          if (newToken) {
+            opts.headers['Authorization'] = `Bearer ${newToken}`;
+            resp = await fetch(url, opts);
+          }
+        } else if (!recovered) {
+          // Genuine logout (state already cleared) or a transient error (session
+          // kept, fail open) — either way this request is done.
+          return null;
+        }
+      } catch (error) {
+        console.error('Forced-logout handling failed:', error);
+        return null;
+      }
+    } else if (resp.status === 401) {
       // If we're using OIDC and get 401, try to refresh the token
       if (requestHeaders['Authorization']) {
         try {
@@ -166,20 +196,6 @@ export class ApiConnection {
       }
     }
 
-    // The backend sets x-logout when the OIDC session is no longer active
-    // (logout / password change / admin force-logout). Clear local auth and
-    // stop — regardless of the response status (#960).
-    if (resp.headers?.get?.('x-logout')) {
-      try {
-        const { useAuthStore } =
-          await import('@send-frontend/stores/auth-store');
-        await useAuthStore().handleForcedLogout();
-      } catch (error) {
-        console.error('Forced-logout handling failed:', error);
-      }
-      return null;
-    }
-
     if (!resp.ok) {
       // Surface the status/body for the caller's diagnostics before discarding
       // the response. Reading the body is safe here because we return null

packages/send/frontend/src/lib/trpc.ts

@@ -98,25 +98,42 @@ export async function fetchWithLogoutCheck(
   url: RequestInfo | URL,
   options: RequestInit
 ): Promise<Response> {
-  const headers = new Headers(options.headers);
-  try {
-    const { useAuthStore } = await import('@send-frontend/stores/auth-store');
-    if (!headers.has('Authorization')) {
-      const token = await useAuthStore().getAccessToken();
-      if (token) {
-        headers.set('Authorization', `Bearer ${token}`);
+  // Attach the current OIDC access token (unless the caller already set one).
+  // Rebuilt for the post-refresh retry so it picks up the freshly-rotated token.
+  async function buildHeaders(): Promise<Headers> {
+    const headers = new Headers(options.headers);
+    try {
+      const { useAuthStore } = await import('@send-frontend/stores/auth-store');
+      if (!headers.has('Authorization')) {
+        const token = await useAuthStore().getAccessToken();
+        if (token) {
+          headers.set('Authorization', `Bearer ${token}`);
+        }
       }
+    } catch {
+      // No token available — fall back to cookie auth.
     }
-  } catch {
-    // No token available — fall back to cookie auth.
+    return headers;
   }
 
-  const res = await fetch(url, { ...options, headers, credentials: 'include' });
+  const res = await fetch(url, {
+    ...options,
+    headers: await buildHeaders(),
+    credentials: 'include',
+  });
 
   if (res.headers?.get?.('x-logout')) {
     try {
       const { useAuthStore } = await import('@send-frontend/stores/auth-store');
-      await useAuthStore().handleForcedLogout();
+      // A revoked access token may just need refreshing — recover and retry
+      // rather than forcing logout when the refresh token is still alive (#974).
+      if (await useAuthStore().recoverOrForceLogout()) {
+        return await fetch(url, {
+          ...options,
+          headers: await buildHeaders(),
+          credentials: 'include',
+        });
+      }
     } catch (error) {
       console.error('Forced-logout handling failed:', error);
     }

packages/send/frontend/src/stores/auth-store.ts

@@ -79,6 +79,14 @@ export const useAuthStore = defineStore('auth', () => {
   // invalid_grant, which reads as a spurious logout.
   let inFlightRefresh: Promise<User | null> | null = null;
 
+  // Outcome of the most recently completed refresh: `true` when it failed
+  // genuinely (refresh token revoked/expired), `false` on success or a transient
+  // error. recoverOrForceLogout reads this to decide force-logout vs fail-open,
+  // rather than inferring from isLoggedIn — which can already be false (e.g. an
+  // extension cold-start request fires before checkAuthStatus flips it true) and
+  // would otherwise turn a network blip into a spurious logout.
+  let lastRefreshFailedGenuinely = false;
+
   /**
    * Notify the add-on background that the session is over so its menu reverts
    * to logged-out. Only meaningful inside Thunderbird, where the token-bridge
@@ -107,6 +115,7 @@ export const useAuthStore = defineStore('auth', () => {
     if (inFlightRefresh) return inFlightRefresh;
 
     inFlightRefresh = (async () => {
+      lastRefreshFailedGenuinely = false;
       try {
         const user = await userManager.signinSilent();
         currentUser.value = user;
@@ -118,6 +127,7 @@ export const useAuthStore = defineStore('auth', () => {
         return user;
       } catch (error) {
         if (isGenuineAuthFailure(error)) {
+          lastRefreshFailedGenuinely = true;
           console.warn(
             `Silent token refresh failed — session ended (${
               (error as { error?: string }).error
@@ -442,6 +452,39 @@ export const useAuthStore = defineStore('auth', () => {
     return user?.access_token ?? null;
   }
 
+  /**
+   * The backend reported the current access token revoked (x-logout, #960).
+   * Before tearing the session down, try a silent refresh: a revoked/expired
+   * *access* token can often be replaced using a still-valid *refresh* token,
+   * so the session keeps rolling instead of bouncing the user to login (PR #974
+   * review). Only force logout when the refresh token is also gone; on a
+   * transient refresh error keep the session (fail open).
+   *
+   * Goes through refreshAccessToken() — an unconditional signinSilent — rather
+   * than getAccessToken(), because the token behind x-logout is still within
+   * its lifetime (the backend exp-gates the signal), so getAccessToken() would
+   * hand back the same stale, revoked token without refreshing.
+   *
+   * @returns `true` if the session was recovered — the caller should retry the
+   * request with the fresh token — and `false` otherwise (forced logout on a
+   * genuine failure, or session kept on a transient error).
+   */
+  async function recoverOrForceLogout(): Promise<boolean> {
+    const user = await refreshAccessToken();
+    if (user && !user.expired) {
+      return true; // refresh token still valid → session continues
+    }
+    // Force logout ONLY on a genuine refresh failure (refresh token gone). A
+    // transient error keeps the session (fail open). We read the failure kind
+    // refreshAccessToken just recorded rather than inferring from isLoggedIn —
+    // there is no await between its resolution above and this read, so the flag
+    // still reflects this refresh.
+    if (lastRefreshFailedGenuinely) {
+      await handleForcedLogout(); // genuine failure → clear state and redirect
+    }
+    return false;
+  }
+
   async function loadUser() {
     // Always check for a stored user instead of getting it from
     // the userManager.
@@ -648,6 +691,7 @@ export const useAuthStore = defineStore('auth', () => {
     getAccessToken,
     logoutFromOIDC,
     handleForcedLogout,
+    recoverOrForceLogout,
     refreshToken,
     loginToKeyCloak, // Alias for loginToOIDC
 

packages/send/frontend/src/test/lib/api.test.ts

@@ -9,12 +9,18 @@ import { afterEach, describe, expect, it, vi } from 'vitest';
 
 // Stub the auth store so api.call's dynamic imports (getAccessToken and the
 // x-logout handler) don't pull in pinia/oidc at test time.
-const { mockForcedLogout } = vi.hoisted(() => ({ mockForcedLogout: vi.fn() }));
+const { mockGetAccessToken, mockRecover, mockRefreshToken } = vi.hoisted(
+  () => ({
+    mockGetAccessToken: vi.fn<() => Promise<string | null>>(async () => null),
+    mockRecover: vi.fn<() => Promise<boolean>>(async () => false),
+    mockRefreshToken: vi.fn<() => Promise<string | null>>(async () => null),
+  })
+);
 vi.mock('@send-frontend/stores/auth-store', () => ({
   useAuthStore: () => ({
-    getAccessToken: async () => null,
-    handleForcedLogout: mockForcedLogout,
-    refreshToken: async () => null,
+    getAccessToken: mockGetAccessToken,
+    recoverOrForceLogout: mockRecover,
+    refreshToken: mockRefreshToken,
   }),
 }));
 
@@ -137,19 +143,22 @@ describe('ApiConnection.call — onFailure diagnostics', () => {
   });
 });
 
-describe('ApiConnection.call — x-logout forced logout (#960)', () => {
+describe('ApiConnection.call — x-logout session recovery (#960/#974)', () => {
   afterEach(() => {
     vi.unstubAllGlobals();
     vi.restoreAllMocks();
-    mockForcedLogout.mockClear();
+    mockGetAccessToken.mockReset().mockResolvedValue(null);
+    mockRecover.mockReset().mockResolvedValue(false);
+    mockRefreshToken.mockReset().mockResolvedValue(null);
   });
 
-  it('clears the session and returns null when the response has x-logout', async () => {
-    mockFetch(
+  it('returns null without retrying when recovery fails (refresh token dead)', async () => {
+    mockRecover.mockResolvedValue(false);
+    const fetchFn = mockFetch(
       () =>
         ({
-          ok: true,
-          status: 200,
+          ok: false,
+          status: 401,
           headers: { get: (k: string) => (k === 'x-logout' ? '1' : null) },
           json: async () => ({ ok: true }),
         }) as unknown as Response
@@ -158,11 +167,50 @@ describe('ApiConnection.call — x-logout forced logout (#960)', () => {
     const api = new ApiConnection(SERVER);
     const result = await api.call('uploads', {}, 'POST');
 
-    expect(mockForcedLogout).toHaveBeenCalledTimes(1);
+    expect(mockRecover).toHaveBeenCalledTimes(1);
+    expect(fetchFn).toHaveBeenCalledTimes(1); // no retry
     expect(result).toBeNull();
   });
 
-  it('does not force logout when the header is absent', async () => {
+  it('retries with a fresh token and returns the retry body when recovery succeeds', async () => {
+    // Present an OIDC bearer token so the retry path (which needs an existing
+    // Authorization header) is taken.
+    mockGetAccessToken.mockResolvedValueOnce('stale'); // initial request token
+    mockRecover.mockResolvedValue(true);
+    mockGetAccessToken.mockResolvedValueOnce('fresh'); // token used on retry
+
+    let call = 0;
+    const fetchFn = mockFetch(() => {
+      call += 1;
+      if (call === 1) {
+        return {
+          ok: false,
+          status: 401,
+          headers: { get: (k: string) => (k === 'x-logout' ? '1' : null) },
+          json: async () => ({ stale: true }),
+        } as unknown as Response;
+      }
+      return {
+        ok: true,
+        status: 200,
+        headers: { get: () => null },
+        json: async () => ({ ok: true }),
+      } as unknown as Response;
+    });
+
+    const api = new ApiConnection(SERVER);
+    const result = await api.call('uploads', {}, 'POST');
+
+    expect(mockRecover).toHaveBeenCalledTimes(1);
+    expect(fetchFn).toHaveBeenCalledTimes(2); // retried once
+    const retryOpts = (fetchFn.mock.calls[1] as unknown[])[1] as {
+      headers: Record<string, string>;
+    };
+    expect(retryOpts.headers['Authorization']).toBe('Bearer fresh');
+    expect(result).toEqual({ ok: true });
+  });
+
+  it('does not attempt recovery when the header is absent', async () => {
     mockFetch(
       () =>
         ({
@@ -176,7 +224,7 @@ describe('ApiConnection.call — x-logout forced logout (#960)', () => {
     const api = new ApiConnection(SERVER);
     const result = await api.call('uploads', {}, 'POST');
 
-    expect(mockForcedLogout).not.toHaveBeenCalled();
+    expect(mockRecover).not.toHaveBeenCalled();
     expect(result).toEqual({ ok: true });
   });
 });

packages/send/frontend/src/test/lib/trpc.test.ts

@@ -7,14 +7,14 @@ import {
 } from '@send-frontend/lib/trpc';
 import { TRPC_WS_PATH } from '@send-frontend/lib/config';
 
-const { mockGetAccessToken, mockForcedLogout } = vi.hoisted(() => ({
+const { mockGetAccessToken, mockRecover } = vi.hoisted(() => ({
   mockGetAccessToken: vi.fn(),
-  mockForcedLogout: vi.fn(),
+  mockRecover: vi.fn(async () => false),
 }));
 vi.mock('@send-frontend/stores/auth-store', () => ({
   useAuthStore: () => ({
     getAccessToken: mockGetAccessToken,
-    handleForcedLogout: mockForcedLogout,
+    recoverOrForceLogout: mockRecover,
   }),
 }));
 
@@ -107,19 +107,46 @@ describe('fetchWithLogoutCheck (#960)', () => {
     expect(mockGetAccessToken).not.toHaveBeenCalled();
   });
 
-  it('forces logout when the response carries x-logout', async () => {
+  it('recovers (no retry) and returns the original response when recovery fails', async () => {
     mockGetAccessToken.mockResolvedValue(null);
-    vi.stubGlobal(
-      'fetch',
-      vi.fn(() =>
-        Promise.resolve({
-          headers: { get: (k: string) => (k === 'x-logout' ? '1' : null) },
-        } as unknown as Response)
-      )
-    );
+    mockRecover.mockResolvedValue(false);
+    const original = {
+      headers: { get: (k: string) => (k === 'x-logout' ? '1' : null) },
+    } as unknown as Response;
+    const fetchFn = vi.fn(() => Promise.resolve(original));
+    vi.stubGlobal('fetch', fetchFn);
+
+    const res = await fetchWithLogoutCheck('https://x/trpc', {});
+
+    expect(mockRecover).toHaveBeenCalledTimes(1);
+    expect(fetchFn).toHaveBeenCalledTimes(1); // no retry
+    expect(res).toBe(original);
+  });
+
+  it('retries once with a refreshed token when recovery succeeds', async () => {
+    mockRecover.mockResolvedValue(true);
+    // First call: initial request has no token; retry: getAccessToken yields one.
+    mockGetAccessToken.mockResolvedValueOnce(null).mockResolvedValueOnce('fresh');
+
+    const inits: RequestInit[] = [];
+    let call = 0;
+    const fetchFn = vi.fn((_url, opts) => {
+      inits.push(opts);
+      call += 1;
+      return Promise.resolve({
+        headers: {
+          get: (k: string) => (call === 1 && k === 'x-logout' ? '1' : null),
+        },
+      } as unknown as Response);
+    });
+    vi.stubGlobal('fetch', fetchFn);
 
     await fetchWithLogoutCheck('https://x/trpc', {});
 
-    expect(mockForcedLogout).toHaveBeenCalledTimes(1);
+    expect(mockRecover).toHaveBeenCalledTimes(1);
+    expect(fetchFn).toHaveBeenCalledTimes(2); // retried once
+    expect((inits[1].headers as Headers).get('Authorization')).toBe(
+      'Bearer fresh'
+    );
   });
 });