fix(auth): don't eject users to invite screen on transient access-check failure

packages/core/src/auth/auth.test.ts

@@ -1,6 +1,10 @@
 import type { RootLogger } from "@posthog/di/logger";
 import type { IPowerManager } from "@posthog/platform/power-manager";
-import { OAUTH_SCOPE_VERSION } from "@posthog/shared";
+import {
+  NotAuthenticatedError,
+  OAUTH_SCOPE_VERSION,
+  sleepWithBackoff,
+} from "@posthog/shared";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { AuthService } from "./auth";
 import type {
@@ -1342,4 +1346,262 @@ describe("AuthService", () => {
       expect(redeemCallCount).toBe(2);
     });
   });
+
+  describe("code access check resilience", () => {
+    const stubFetchWithCheckAccess = (
+      checkAccess: (callCount: number) => Response,
+    ): { getCheckAccessCalls: () => number } => {
+      let checkAccessCalls = 0;
+      vi.stubGlobal(
+        "fetch",
+        vi.fn(async (input: string | Request) => {
+          const url = typeof input === "string" ? input : input.url;
+
+          if (url.includes("/api/users/@me/")) {
+            return {
+              ok: true,
+              json: vi.fn().mockResolvedValue({
+                uuid: "user-1",
+                organization: { id: "org-1" },
+              }),
+            } as unknown as Response;
+          }
+
+          if (/\/api\/organizations\/[^/]+\/$/.test(url)) {
+            return {
+              ok: true,
+              json: vi.fn().mockResolvedValue({
+                name: "Org 1",
+                teams: [{ id: 42, name: "Project 42" }],
+              }),
+            } as unknown as Response;
+          }
+
+          if (url.includes("/invites/check-access/")) {
+            checkAccessCalls++;
+            return checkAccess(checkAccessCalls);
+          }
+
+          return {
+            ok: true,
+            json: vi.fn().mockResolvedValue({}),
+          } as unknown as Response;
+        }) as unknown as typeof fetch,
+      );
+      return { getCheckAccessCalls: () => checkAccessCalls };
+    };
+
+    const restoreSession = async () => {
+      seedStoredSession({ selectedProjectId: 42 });
+      oauthFlow.refreshToken.mockResolvedValue(
+        mockTokenResponse({
+          accessToken: "access-token",
+          refreshToken: "refresh-token",
+        }),
+      );
+      await service.initialize();
+    };
+
+    it("recovers in the background after transient failures without ejecting the user", async () => {
+      // Fails twice, then succeeds — a real user weathering a blip must end up
+      // with access, never having been flipped to `false`.
+      const { getCheckAccessCalls } = stubFetchWithCheckAccess((call) =>
+        call <= 2
+          ? ({
+              ok: false,
+              status: 403,
+              json: () => Promise.resolve({}),
+            } as unknown as Response)
+          : ({
+              ok: true,
+              status: 200,
+              json: () => Promise.resolve({ has_access: true }),
+            } as unknown as Response),
+      );
+
+      await restoreSession();
+
+      await vi.waitFor(() =>
+        expect(service.getState().hasCodeAccess).toBe(true),
+      );
+      expect(getCheckAccessCalls()).toBe(3);
+    });
+
+    it("fails closed once the retry budget is exhausted", async () => {
+      const { getCheckAccessCalls } = stubFetchWithCheckAccess(
+        () =>
+          ({
+            ok: false,
+            status: 403,
+            json: () => Promise.resolve({}),
+          }) as unknown as Response,
+      );
+
+      await restoreSession();
+
+      // Persistent inability to call home eventually revokes access so the
+      // invite gate can't be bypassed by staying offline.
+      await vi.waitFor(() =>
+        expect(service.getState().hasCodeAccess).toBe(false),
+      );
+      expect(service.getState().status).toBe("authenticated");
+      // 1 awaited attempt + one retry per backoff step.
+      expect(getCheckAccessCalls()).toBe(10);
+      // Each retry waits the next backoff step (attempt 0..8).
+      expect(
+        vi.mocked(sleepWithBackoff).mock.calls.map((call) => call[0]),
+      ).toEqual([0, 1, 2, 3, 4, 5, 6, 7, 8]);
+    });
+
+    it("treats thrown network errors like transient failures and fails closed", async () => {
+      // The check throws (network timeout / DNS failure) rather than returning
+      // a non-2xx — exercises the catch branch, which must behave like a
+      // transient failure: retry, then fail closed.
+      const { getCheckAccessCalls } = stubFetchWithCheckAccess(() => {
+        throw new TypeError("network down");
+      });
+
+      await restoreSession();
+
+      await vi.waitFor(() =>
+        expect(service.getState().hasCodeAccess).toBe(false),
+      );
+      expect(getCheckAccessCalls()).toBe(10);
+    });
+
+    it("does not let a stale retry loop clobber a fresh grant", async () => {
+      // Park the retry loop on its first backoff so we can interleave a newer
+      // authoritative check that succeeds before the stale loop wakes.
+      let releaseSleep!: () => void;
+      const gate = new Promise<void>((resolve) => {
+        releaseSleep = resolve;
+      });
+      vi.mocked(sleepWithBackoff).mockImplementationOnce(() => gate);
+
+      let call = 0;
+      stubFetchWithCheckAccess(() => {
+        call++;
+        // First (restore) attempt fails and schedules the retry loop; every
+        // later check succeeds with access.
+        return call === 1
+          ? ({
+              ok: false,
+              status: 403,
+              json: () => Promise.resolve({}),
+            } as unknown as Response)
+          : ({
+              ok: true,
+              status: 200,
+              json: () => Promise.resolve({ has_access: true }),
+            } as unknown as Response);
+      });
+
+      await restoreSession();
+      // Restore's attempt failed transiently; gate is unresolved (null, not false).
+      expect(service.getState().hasCodeAccess).toBeNull();
+
+      // A fresh authoritative check (token refresh) installs a new session and
+      // resolves access true, superseding the parked loop.
+      await service.refreshAccessToken();
+      expect(service.getState().hasCodeAccess).toBe(true);
+
+      // Wake the stale loop: it must notice it was superseded and NOT fail closed.
+      releaseSleep();
+      await Promise.resolve();
+      await Promise.resolve();
+      expect(service.getState().hasCodeAccess).toBe(true);
+    });
+
+    it("logs out on a 401 instead of retrying", async () => {
+      const { getCheckAccessCalls } = stubFetchWithCheckAccess(
+        () =>
+          ({
+            ok: false,
+            status: 401,
+            json: () => Promise.resolve({}),
+          }) as unknown as Response,
+      );
+
+      await restoreSession();
+
+      // A rejected token is a hard stop: log the user out (no retry, no
+      // fail-closed invite screen) so they re-authenticate.
+      const state = service.getState();
+      expect(state.status).toBe("anonymous");
+      expect(state.hasCodeAccess).toBeNull();
+      expect(getCheckAccessCalls()).toBe(1);
+    });
+
+    it("still gates the user when the server definitively reports no access", async () => {
+      stubFetchWithCheckAccess(
+        () =>
+          ({
+            ok: true,
+            status: 200,
+            json: () => Promise.resolve({ has_access: false }),
+          }) as unknown as Response,
+      );
+
+      await restoreSession();
+
+      expect(service.getState().hasCodeAccess).toBe(false);
+    });
+
+    it("honours a success on the final retry instead of failing closed", async () => {
+      // 1 awaited attempt + 8 transient failures, then the 9th (final) retry
+      // succeeds: the loop must settle to access and never reach the
+      // fail-closed write, exercising the final-iteration boundary.
+      const { getCheckAccessCalls } = stubFetchWithCheckAccess((call) =>
+        call < 10
+          ? ({
+              ok: false,
+              status: 403,
+              json: () => Promise.resolve({}),
+            } as unknown as Response)
+          : ({
+              ok: true,
+              status: 200,
+              json: () => Promise.resolve({ has_access: true }),
+            } as unknown as Response),
+      );
+
+      await restoreSession();
+
+      await vi.waitFor(() =>
+        expect(service.getState().hasCodeAccess).toBe(true),
+      );
+      expect(getCheckAccessCalls()).toBe(10);
+    });
+
+    it("rejects an in-flight refresh that logs out mid-sync on a 401", async () => {
+      // The race the seq guard alone doesn't cover: a 401 from the code-access
+      // check logs out *during* syncAuthenticatedSession, inside the shared
+      // refresh promise. The awaited refresh must reject rather than resolve
+      // with the torn-down session's (now dead) access token.
+      let reject401 = false;
+      stubFetchWithCheckAccess(() =>
+        reject401
+          ? ({
+              ok: false,
+              status: 401,
+              json: () => Promise.resolve({}),
+            } as unknown as Response)
+          : ({
+              ok: true,
+              status: 200,
+              json: () => Promise.resolve({ has_access: true }),
+            } as unknown as Response),
+      );
+
+      await restoreSession();
+      expect(service.getState().status).toBe("authenticated");
+
+      reject401 = true;
+      await expect(service.refreshAccessToken()).rejects.toBeInstanceOf(
+        NotAuthenticatedError,
+      );
+      expect(service.getState().status).toBe("anonymous");
+      expect(service.getState().hasCodeAccess).toBeNull();
+    });
+  });
 });