fix(auth): stop refresh-token rotation race from logging users out

src/auth/getAuth.test.ts

@@ -28,17 +28,30 @@ const makeConfig = (overrides: {
   idTokenClaims?: Record<string, unknown>;
   /** Claims returned by the mocked OIDC userinfo fetch */
   userInfoClaims?: Record<string, unknown>;
+  /** Override the mocked Better Auth getAccessToken behavior */
+  getAccessTokenImpl?: () => Promise<{
+    accessToken?: string;
+    idToken?: string | null;
+  } | null>;
+  /** Override the mocked Better Auth refreshToken behavior */
+  refreshTokenImpl?: () => Promise<{
+    accessToken?: string;
+    idToken?: string | null;
+  } | null>;
 }) => {
   const setCookieCalls: Array<{ name: string; value: string }> = [];
   const encryptCalls: CachedAuthData[] = [];
 
   const authApi = {
     getSession: mock(async () => overrides.session),
-    getAccessToken: mock(async () => ({
-      accessToken: "access-token",
-      idToken: "id-token",
-    })),
-    refreshToken: mock(async () => null),
+    getAccessToken: mock(
+      overrides.getAccessTokenImpl ??
+        (async () => ({
+          accessToken: "access-token",
+          idToken: "id-token",
+        })),
+    ),
+    refreshToken: mock(overrides.refreshTokenImpl ?? (async () => null)),
     signOut: mock(async () => undefined),
   } as unknown as BetterAuthApi;
 
@@ -275,3 +288,74 @@ describe("createGetAuth userinfo org cache", () => {
     expect(fetchSpy(cfg)).toHaveBeenCalledTimes(2);
   });
 });
+
+describe("createGetAuth concurrent refresh handling", () => {
+  /** APIError wrapper Better Auth throws when a rotated refresh token races */
+  const failedTokenError = () =>
+    Object.assign(new Error("Failed to get a valid access token"), {
+      body: { code: "FAILED_TO_GET_ACCESS_TOKEN" },
+    });
+
+  it("single-flights the token fetch across a concurrent request burst", async () => {
+    // An SSR page fires many parallel requests for the SAME session. Each
+    // triggers getAuth, but only ONE token fetch may hit Better Auth/Gatekeeper.
+    // Firing N refreshes is exactly what races refresh-token rotation and logs
+    // users out, so concurrent getAuth calls for one session must share a fetch.
+    let resolveGate: () => void = () => {};
+    const gate = new Promise<void>((r) => {
+      resolveGate = r;
+    });
+
+    const cfg = makeConfig({
+      session: makeSession({ identityProviderId: "idp-sub-123" }),
+      getAccessTokenImpl: async () => {
+        await gate;
+        return { accessToken: "access-token", idToken: "id-token" };
+      },
+    });
+
+    const getAuth = createGetAuth(cfg);
+    const inflight = [getAuth(request), getAuth(request), getAuth(request)];
+    // let all three reach the shared token fetch before it settles
+    await Promise.resolve();
+    resolveGate();
+    const results = await Promise.all(inflight);
+
+    expect(results.every((r) => r?.accessToken === "access-token")).toBe(true);
+    const getAccessToken = (
+      cfg.authApi as unknown as { getAccessToken: ReturnType<typeof mock> }
+    ).getAccessToken;
+    expect(getAccessToken).toHaveBeenCalledTimes(1);
+  });
+
+  it("degrades instead of signing out when a refresh loses the rotation race", async () => {
+    // The BA session itself is valid (getSession succeeded); only the OAuth
+    // token refresh failed because a sibling request already rotated it. This
+    // must NOT destroy the session: return it token-less and let the natural
+    // 401 -> re-auth path recover, rather than force-logging-out a live user.
+    const cfg = makeConfig({
+      session: makeSession({ identityProviderId: "idp-sub-123" }),
+      getAccessTokenImpl: async () => {
+        throw failedTokenError();
+      },
+      refreshTokenImpl: async () => {
+        throw failedTokenError();
+      },
+    });
+
+    const getAuth = createGetAuth(cfg);
+    const result = await getAuth(request);
+
+    // session preserved, just without a fresh access token
+    expect(result).not.toBeNull();
+    expect(result?.accessToken).toBeUndefined();
+    expect(result?.user.identityProviderId).toBe("idp-sub-123");
+    // never tears the session down on a single transient failure
+    const signOut = (
+      cfg.authApi as unknown as { signOut: ReturnType<typeof mock> }
+    ).signOut;
+    expect(signOut).not.toHaveBeenCalled();
+    // and never clears the auth cache cookie
+    expect(cfg.setCookieCalls.some((c) => c.value === "")).toBe(false);
+  });
+});

src/auth/getAuth.ts

@@ -3,6 +3,7 @@ import { ensureFreshAccessToken, isInvalidGrant } from "./token";
 
 import type { AuthCache } from "./cache";
 import type { OidcClient } from "./oidc";
+import type { TokenResult } from "./token";
 import type { OrganizationClaim } from "./types";
 
 /**
@@ -136,9 +137,12 @@ type GetAuthSession = {
  * via `ensureFreshAccessToken`, OIDC-verified ID token decoding,
  * organization claim extraction, and encrypted cookie caching.
  *
- * On `isInvalidGrant`, clears both the session and cache cookie to
- * force re-authentication. On other token errors, returns the session
- * with whatever organizations are cached (graceful degradation).
+ * The token refresh is single-flighted per session so a page-load request
+ * burst never races refresh-token rotation against itself. A refresh failure
+ * (including `isInvalidGrant`) never tears down the still-valid session: it
+ * degrades to a session without a fresh access token and lets the app's
+ * natural 401 -> re-auth path recover a genuinely dead session, rather than
+ * force-logging-out a user who merely lost a benign rotation race.
  * @param config - Auth configuration
  * @returns `getAuth(request)` function that resolves to session or null
  */
@@ -165,6 +169,15 @@ function createGetAuth(config: GetAuthConfig) {
     { organizations: OrganizationClaim[]; expiry: number }
   >();
 
+  // Single-flight the token refresh per session. An SSR page load fires many
+  // parallel requests carrying the SAME session cookie; without deduping, each
+  // one independently refreshes the single-use OAuth token, and all but the
+  // winner present a just-rotated token -> the IDP treats it as reuse and the
+  // user is logged out in a burst. Sharing one in-flight refresh per session
+  // collapses the burst to a single rotation. Keyed by the BA session token;
+  // entries are removed as soon as the refresh settles.
+  const refreshInFlight = new Map<string, Promise<TokenResult | null>>();
+
   return async function getAuth(
     request: Request,
   ): Promise<GetAuthSession | null> {
@@ -194,53 +207,67 @@ function createGetAuth(config: GetAuthConfig) {
       // resolve. Organizations are intentionally not part of the cache.
       const hasCachedData = !!identityProviderId;
 
-      // Get tokens from Gatekeeper via Better Auth
+      // Get tokens from Gatekeeper via Better Auth. Concurrent requests for the
+      // same session share one refresh (single-flight) so a page-load burst
+      // never races refresh-token rotation against itself.
+      const sessionKey =
+        (session.session?.token as string | undefined) ?? session.user.id;
       try {
-        const tokenResult = await ensureFreshAccessToken({
-          getAccessToken: async () => {
-            try {
-              const result = await authApi.getAccessToken({
-                body: { providerId },
-                headers: request.headers,
-              });
+        let inFlight = refreshInFlight.get(sessionKey);
+        if (!inFlight) {
+          inFlight = ensureFreshAccessToken({
+            getAccessToken: async () => {
+              try {
+                const result = await authApi.getAccessToken({
+                  body: { providerId },
+                  headers: request.headers,
+                });
+
+                if (!result?.accessToken) {
+                  console.warn(
+                    `${logPrefix} getAccessToken returned no token`,
+                    {
+                      hasResult: !!result,
+                    },
+                  );
+                }
 
-              if (!result?.accessToken) {
-                console.warn(`${logPrefix} getAccessToken returned no token`, {
-                  hasResult: !!result,
+                return result;
+              } catch (err) {
+                const body =
+                  err && typeof err === "object" && "body" in err
+                    ? (err as { body: unknown }).body
+                    : undefined;
+                console.error(`${logPrefix} getAccessToken failed:`, {
+                  code:
+                    body && typeof body === "object" && "code" in body
+                      ? (body as { code: string }).code
+                      : "unknown",
+                  message: err instanceof Error ? err.message : String(err),
                 });
+                throw err;
               }
-
-              return result;
-            } catch (err) {
-              const body =
-                err && typeof err === "object" && "body" in err
-                  ? (err as { body: unknown }).body
-                  : undefined;
-              console.error(`${logPrefix} getAccessToken failed:`, {
-                code:
-                  body && typeof body === "object" && "code" in body
-                    ? (body as { code: string }).code
-                    : "unknown",
-                message: err instanceof Error ? err.message : String(err),
-              });
-              throw err;
-            }
-          },
-          refreshToken: async () => {
-            try {
-              return await authApi.refreshToken({
-                body: { providerId },
-                headers: request.headers,
-              });
-            } catch (err) {
-              console.error(
-                `${logPrefix} refreshToken failed:`,
-                err instanceof Error ? err.message : String(err),
-              );
-              throw err;
-            }
-          },
-        });
+            },
+            refreshToken: async () => {
+              try {
+                return await authApi.refreshToken({
+                  body: { providerId },
+                  headers: request.headers,
+                });
+              } catch (err) {
+                console.error(
+                  `${logPrefix} refreshToken failed:`,
+                  err instanceof Error ? err.message : String(err),
+                );
+                throw err;
+              }
+            },
+          }).finally(() => {
+            refreshInFlight.delete(sessionKey);
+          });
+          refreshInFlight.set(sessionKey, inFlight);
+        }
+        const tokenResult = await inFlight;
         accessToken = tokenResult?.accessToken;
 
         if (!accessToken) {
@@ -364,19 +391,20 @@ function createGetAuth(config: GetAuthConfig) {
           });
         }
       } catch (err) {
-        console.error(`${logPrefix} Token fetch error:`, err);
-
+        // A failed token refresh must NOT tear down a still-valid session. The
+        // BA session (getSession above) is intact; the refresh most often fails
+        // only because a concurrent SSR request already rotated the single-use
+        // refresh token. Signing out here logged users out in bursts on every
+        // app re-open. Degrade instead: serve the session without a fresh
+        // access token. A genuinely dead session self-heals on the next request
+        // via the app's 401 -> re-auth redirect, without punishing the benign
+        // rotation race.
         if (isInvalidGrant(err)) {
           console.warn(
-            `${logPrefix} Stale OAuth tokens, clearing session for re-auth`,
+            `${logPrefix} Token refresh failed (likely a rotation race); serving session without a fresh access token`,
           );
-          try {
-            await authApi.signOut({ headers: request.headers });
-          } catch {
-            // Sign-out may fail if session is already corrupt
-          }
-          setCookie(authCache.cookieName, "", { maxAge: 0, path: "/" });
-          return null;
+        } else {
+          console.error(`${logPrefix} Token fetch error:`, err);
         }
       }