fix(saas): give auth-bootstrap data fetching a single owner (#7194)

## The problem

Three pieces of user data (pro status, avatar metadata, profile picture)
were being fetched from four different places: `initializeAuth` on
mount, the `SIGNED_IN` handler, the `TOKEN_REFRESHED` handler, and the
post-upgrade path.

On a fresh login the first two both see a session, so everything got
fetched twice. It didn't stop after login either — Supabase re-fires
`SIGNED_IN` on token refresh and tab-visibility wakeups, so a refresh
that emitted both events cost around 7 Supabase reads.

## The fix

All four call sites now go through one `loadUserData(session)` that is
idempotent per identity.

The guard key is `user.id` + `is_anonymous`:

- not the access token, which changes on every refresh and would defeat
the guard entirely
- the anonymous flag matters because a guest to authenticated upgrade
keeps the same user id, and that is the one case where the data
genuinely does need reloading

**Per login: 6 fetches to 3. A repeat `SIGNED_IN` or `TOKEN_REFRESHED`
fetches nothing.** The tests count real calls rather than asserting on
shape.

## Two behaviour changes worth naming

- `initializeAuth` now awaits the full load, so the initial spinner also
waits on the profile-picture URL. Net login is still faster, since an
entire duplicate pass is gone.
- A tab-wake `SIGNED_IN` no longer revalidates entitlements. That
revalidation was accidental rather than designed — `refreshProStatus()`
is the intended path, and post-checkout is already handled by
`CheckoutContext`.

## Scope

Supabase-origin traffic only. This does not touch the ~20 authenticated
requests hitting `SupabaseAuthenticationFilter`, because those go to the
Stirling backend rather than the hosted Supabase project. That is a
separate problem and is unmeasured, so it needs measuring before
anything is optimised. Remaining items (a double `/api/v1/team/my`
fetch, an effect keyed on `[user]` identity in `FolderContext`, the
`portalAccess` spinner flash, and caching the auth filter's per-request
Postgres round-trips) are tracked separately.

## Verification

```
npx tsc --noEmit --project editor/src/saas/tsconfig.json   # exit 0
npx eslint --max-warnings=0 editor/src/saas/auth            # exit 0
npx prettier --check editor/src/saas/auth/                  # clean
npx vitest run --project saas                               # 75 passed (20 files)
```
This commit is contained in:
ConnorYoh
2026-08-06 13:05:24 +00:00
committed by GitHub
parent ad8830b645
commit 74c53001cf
2 changed files with 427 additions and 77 deletions
@@ -0,0 +1,339 @@
import { act, render, waitFor } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { Session, User } from "@supabase/supabase-js";
/**
* Request-count tests for {@link AuthProvider}'s data loading. It used to fetch
* pro status, avatar metadata and the picture from two places at once, and
* Supabase re-fires SIGNED_IN on token refresh and tab wakeups, so it kept
* happening. These pin the call counts.
*/
type AuthCallback = (event: string, session: Session | null) => void;
const rpc = vi.fn();
const createSignedUrl = vi.fn();
const storageFrom = vi.fn((_bucket: string) => ({ createSignedUrl }));
const getSession = vi.fn();
const onAuthStateChange = vi.fn();
const unsubscribe = vi.fn();
vi.mock("@app/auth/supabase", () => ({
supabase: {
auth: {
getSession: () => getSession(),
onAuthStateChange: (cb: AuthCallback) => onAuthStateChange(cb),
refreshSession: vi
.fn()
.mockResolvedValue({ data: { session: null }, error: null }),
signOut: vi.fn().mockResolvedValue({ error: null }),
},
rpc: (...args: unknown[]) => rpc(...args),
storage: { from: (bucket: string) => storageFrom(bucket) },
},
debugAuthEvents: vi.fn(),
}));
const syncOAuthAvatar = vi.fn();
const getProfilePictureMetadata = vi.fn();
vi.mock("@app/services/avatarSyncService", () => ({
syncOAuthAvatar: (...args: unknown[]) => syncOAuthAvatar(...args),
getProfilePictureMetadata: (...args: unknown[]) =>
getProfilePictureMetadata(...args),
getProviderAvatarUrl: () => null,
}));
const synchronizeUserUpgrade = vi.fn();
vi.mock("@app/services/userService", () => ({
synchronizeUserUpgrade: (...args: unknown[]) =>
synchronizeUserUpgrade(...args),
}));
// Imported after the mocks so the provider picks them up.
const { AuthProvider, useAuth } = await import("./UseSession");
/** Surfaces `loading` so a test can assert on it rather than on the container. */
function LoadingProbe() {
const { loading } = useAuth();
return <span data-testid="loading">{String(loading)}</span>;
}
const USER_ID = "11111111-2222-3333-4444-555555555555";
function makeSession(
overrides: { token?: string; userId?: string; anonymous?: boolean } = {},
): Session {
const user = {
id: overrides.userId ?? USER_ID,
email: "someone@example.com",
is_anonymous: overrides.anonymous ?? false,
app_metadata: { provider: "google" },
user_metadata: { full_name: "Some One" },
} as unknown as User;
return {
access_token: overrides.token ?? "token-1",
refresh_token: "refresh-1",
expires_in: 3600,
token_type: "bearer",
user,
} as unknown as Session;
}
/** Total requests the provider makes per user-data load. */
function callCounts() {
return {
proStatus: rpc.mock.calls.length,
metadata: getProfilePictureMetadata.mock.calls.length,
picture: createSignedUrl.mock.calls.length,
avatarSync: syncOAuthAvatar.mock.calls.length,
};
}
function renderProvider() {
let authCallback: AuthCallback = () => {};
onAuthStateChange.mockImplementation((cb: AuthCallback) => {
authCallback = cb;
return { data: { subscription: { unsubscribe } } };
});
const utils = render(
<AuthProvider>
<LoadingProbe />
</AuthProvider>,
);
/**
* Deliver an auth event and let its work finish. The provider defers with
* setTimeout(0), so a microtask flush is not enough: without draining real
* macrotasks the assertions run before any refetch and prove nothing.
*/
const fire = async (event: string, s: Session | null) => {
await act(async () => {
authCallback(event, s);
await new Promise((resolve) => setTimeout(resolve, 0));
await new Promise((resolve) => setTimeout(resolve, 0));
});
};
return { ...utils, fire };
}
describe("AuthProvider user-data loading", () => {
beforeEach(() => {
vi.clearAllMocks();
vi.useRealTimers();
sessionStorage.clear();
rpc.mockResolvedValue({ data: true, error: null });
createSignedUrl.mockResolvedValue({
data: { signedUrl: "https://example.test/avatar" },
error: null,
});
getProfilePictureMetadata.mockResolvedValue(null);
syncOAuthAvatar.mockResolvedValue(false);
synchronizeUserUpgrade.mockResolvedValue(undefined);
getSession.mockResolvedValue({
data: { session: makeSession() },
error: null,
});
});
it("fetches each piece of user data exactly once on login", async () => {
const { fire } = renderProvider();
await waitFor(() => expect(createSignedUrl).toHaveBeenCalled());
// The SIGNED_IN that follows a fresh login must not repeat the work.
await fire("SIGNED_IN", makeSession());
expect(callCounts()).toEqual({
proStatus: 1,
metadata: 1,
picture: 1,
avatarSync: 1,
});
});
it("does not refetch when SIGNED_IN repeats with a new access token", async () => {
const { fire } = renderProvider();
await waitFor(() => expect(createSignedUrl).toHaveBeenCalled());
const before = callCounts();
// What a tab-visibility wakeup or token refresh looks like: same user,
// different token.
await fire("SIGNED_IN", makeSession({ token: "token-2" }));
expect(callCounts()).toEqual(before);
});
it("does not refetch on TOKEN_REFRESHED for the same identity", async () => {
const { fire } = renderProvider();
await waitFor(() => expect(createSignedUrl).toHaveBeenCalled());
const before = callCounts();
await fire("TOKEN_REFRESHED", makeSession({ token: "token-3" }));
expect(callCounts()).toEqual(before);
});
it("keeps loading false across repeat auth events", async () => {
// Guards the Landing -> HomePage unmount: toggling `loading` on a wakeup
// would tear down the tree on every tab switch.
const { fire, getByTestId } = renderProvider();
await waitFor(() =>
expect(getByTestId("loading").textContent).toBe("false"),
);
await fire("SIGNED_IN", makeSession({ token: "token-4" }));
expect(getByTestId("loading").textContent).toBe("false");
await fire("TOKEN_REFRESHED", makeSession({ token: "token-5" }));
expect(getByTestId("loading").textContent).toBe("false");
expect(callCounts().proStatus).toBe(1);
});
it("clears the initial spinner without waiting for the avatar upload", async () => {
// syncOAuthAvatar re-uploads the provider image on a first login. Gating
// `loading` on it would stall account creation behind an image upload.
let releaseSync = () => {};
syncOAuthAvatar.mockImplementationOnce(
() =>
new Promise<boolean>((resolve) => {
releaseSync = () => resolve(false);
}),
);
const { getByTestId } = renderProvider();
await waitFor(() =>
expect(getByTestId("loading").textContent).toBe("false"),
);
// The picture read chains behind the sync, so it has not run yet either.
expect(createSignedUrl).not.toHaveBeenCalled();
await act(async () => {
releaseSync();
await new Promise((resolve) => setTimeout(resolve, 0));
});
});
it("refetches after a guest upgrade, which keeps the same user id", async () => {
// The upgrade path is the one case where the id is unchanged but the data
// must be reloaded - hence keying on is_anonymous, not the id alone.
getSession.mockResolvedValue({
data: { session: makeSession({ anonymous: true }) },
error: null,
});
sessionStorage.setItem("pendingUpgrade", "true");
sessionStorage.setItem("upgradeProvider", "google");
const { fire } = renderProvider();
await waitFor(() => expect(rpc).toHaveBeenCalledTimes(1));
await fire("USER_UPDATED", makeSession({ anonymous: false }));
await waitFor(() => expect(rpc).toHaveBeenCalledTimes(2));
expect(synchronizeUserUpgrade).toHaveBeenCalledWith("google");
});
it("does not drop an upgrade that lands while the guest load is in flight", async () => {
// Coalescing on "something is in flight" alone would hand the upgrade the
// guest's promise and never fetch the real user's data.
getSession.mockResolvedValue({
data: { session: makeSession({ anonymous: true }) },
error: null,
});
sessionStorage.setItem("pendingUpgrade", "true");
sessionStorage.setItem("upgradeProvider", "google");
let releaseGuestLoad = () => {};
const guestLoadBlocked = new Promise<void>((resolve) => {
releaseGuestLoad = resolve;
});
rpc.mockImplementationOnce(async () => {
await guestLoadBlocked;
return { data: false, error: null };
});
const { fire } = renderProvider();
await waitFor(() => expect(rpc).toHaveBeenCalledTimes(1));
// Deliver the upgrade with the guest load still deliberately unsettled, so
// the guard genuinely has an in-flight load to reason about.
await fire("USER_UPDATED", makeSession({ anonymous: false }));
await waitFor(() => expect(rpc).toHaveBeenCalledTimes(2), {
timeout: 1000,
});
// Let the abandoned guest load settle inside act, so its trailing state
// updates do not land after the test finishes.
await act(async () => {
releaseGuestLoad();
await new Promise((resolve) => setTimeout(resolve, 0));
await new Promise((resolve) => setTimeout(resolve, 0));
});
});
it("reloads for the same user after a sign-out", async () => {
const { fire } = renderProvider();
await waitFor(() => expect(createSignedUrl).toHaveBeenCalled());
await fire("SIGNED_OUT", null);
await fire("SIGNED_IN", makeSession());
await waitFor(() => expect(rpc).toHaveBeenCalledTimes(2));
});
it("keeps the initial spinner up when SIGNED_IN wins the race with initializeAuth", async () => {
// initializeAuth yields at `await getSession()`, so SIGNED_IN can land
// first. Marking the identity loaded up front would then clear the spinner
// while pro status was still in flight.
const session = makeSession();
let releaseSession = () => {};
getSession.mockReturnValueOnce(
new Promise<{ data: { session: Session }; error: null }>((resolve) => {
releaseSession = () => resolve({ data: { session }, error: null });
}),
);
let releaseProStatus = () => {};
rpc.mockImplementationOnce(
() =>
new Promise<{ data: boolean; error: null }>((resolve) => {
releaseProStatus = () => resolve({ data: true, error: null });
}),
);
const { getByTestId, fire } = renderProvider();
// SIGNED_IN lands first and starts the load; pro status stays unsettled.
await fire("SIGNED_IN", session);
expect(rpc).toHaveBeenCalledTimes(1);
// initializeAuth must now adopt that in-flight load, not short-circuit.
await act(async () => {
releaseSession();
await new Promise((resolve) => setTimeout(resolve, 0));
await new Promise((resolve) => setTimeout(resolve, 0));
});
expect(getByTestId("loading").textContent).toBe("true");
// Adopted, not restarted.
expect(rpc).toHaveBeenCalledTimes(1);
await act(async () => {
releaseProStatus();
await new Promise((resolve) => setTimeout(resolve, 0));
await new Promise((resolve) => setTimeout(resolve, 0));
});
await waitFor(() =>
expect(getByTestId("loading").textContent).toBe("false"),
);
expect(rpc).toHaveBeenCalledTimes(1);
});
});
+88 -77
View File
@@ -2,6 +2,7 @@ import {
createContext,
useContext,
useEffect,
useRef,
useState,
ReactNode,
useCallback,
@@ -247,6 +248,78 @@ export function AuthProvider({ children }: { children: ReactNode }) {
await fetchProfilePictureMetadata();
}, [fetchProfilePictureMetadata]);
// Refs, not state: the auth effect below has an empty dep array.
const loadedForRef = useRef<string | null>(null);
const inFlightRef = useRef<{ key: string; promise: Promise<void> } | null>(
null,
);
/**
* Sole owner of the per-user data fetching: mount-time init and every auth
* event route through here. Idempotent per identity, since Supabase re-fires
* SIGNED_IN and TOKEN_REFRESHED on token refresh and tab wakeups; pass
* `force` for a genuine reload. The returned promise excludes the profile
* picture, so awaiting it never blocks on an image download.
*/
const loadUserData = useCallback(
(
sessionToLoad: Session | null,
opts?: { force?: boolean },
): Promise<void> => {
const user = sessionToLoad?.user;
if (!user) {
loadedForRef.current = null;
return Promise.resolve();
}
// Not the access token, which changes every refresh. The anonymous flag
// matters: a guest upgrade keeps the same id and must still refetch.
const key = `${user.id}:${Boolean(user.is_anonymous)}`;
if (!opts?.force && loadedForRef.current === key)
return Promise.resolve();
// The second of a concurrent init/SIGNED_IN pair adopts this promise so it
// still awaits the load. A forced reload must not: the guest upgrade
// would be silently dropped.
if (!opts?.force && inFlightRef.current?.key === key)
return inFlightRef.current.promise;
const run = (async () => {
// Off the awaited path: a first login re-uploads the provider avatar.
// The signed-URL read chains behind it because reading first 404s and
// silently falls back to the provider photo.
const avatarSync = syncOAuthAvatar(user).catch((err) => {
console.debug("[Auth Debug] Failed to sync OAuth avatar:", err);
return false;
});
void avatarSync
.then(() => fetchProfilePicture(sessionToLoad))
.catch((err) => {
console.debug("[Auth Debug] Failed to fetch profile picture:", err);
});
await Promise.all([
fetchProStatus(sessionToLoad),
fetchProfilePictureMetadata(sessionToLoad),
]);
// Only on success: set up front, a concurrent caller short-circuits on
// it and returns to a still-empty state. Also lets a failure retry.
loadedForRef.current = key;
})()
.catch((err) => {
console.debug("[Auth Debug] Failed to load user data:", err);
})
.finally(() => {
// Only clear our own entry; a newer load may have superseded us.
if (inFlightRef.current?.promise === run) inFlightRef.current = null;
});
inFlightRef.current = { key, promise: run };
return run;
},
[fetchProStatus, fetchProfilePictureMetadata, fetchProfilePicture],
);
const refreshSession = async () => {
try {
setLoading(true);
@@ -312,23 +385,8 @@ export function AuthProvider({ children }: { children: ReactNode }) {
});
setSession(data.session);
// Fetch pro status, profile picture metadata, and profile picture using the session from the response
if (data.session?.user) {
// Sync OAuth avatar in background; fetch the picture once the
// sync settles instead of guessing with a fixed delay.
syncOAuthAvatar(data.session.user)
.catch((err) => {
console.debug(
"[Auth Debug] Failed to sync OAuth avatar on init:",
err,
);
return false;
})
.then(() => fetchProfilePicture(data.session));
await fetchProStatus(data.session);
await fetchProfilePictureMetadata(data.session);
}
// Awaited so the spinner does not clear before pro status is known.
await loadUserData(data.session);
}
} catch (err) {
console.error(
@@ -374,58 +432,13 @@ export function AuthProvider({ children }: { children: ReactNode }) {
setIsPro(null);
setProfilePictureUrl(null);
setProfilePictureMetadata(null);
} else if (event === "SIGNED_IN") {
console.debug("[Auth Debug] User signed in successfully");
if (newSession?.user) {
// Note: we deliberately do NOT toggle `loading` here. Supabase
// also fires SIGNED_IN on tab visibility / token-refresh wakeups
// (per its docs: "SIGNED_IN is fired when a user signs in OR
// when the access token is refreshed"), and gating the UI on
// `loading` would unmount Landing -> HomePage every time the
// user switches tabs back. Initial-mount loading is handled by
// `initializeAuth` above; downstream fetches expose their own
// null/loading states.
// Sync OAuth avatar in background (don't block other fetches)
const avatarSync = syncOAuthAvatar(newSession.user).catch(
(err) => {
console.debug(
"[Auth Debug] Failed to sync OAuth avatar:",
err,
);
return false;
},
);
// Fetch user data in parallel
Promise.all([
fetchProStatus(newSession),
fetchProfilePictureMetadata(newSession),
]).then(() => {
// Fetch the picture once the avatar sync settles.
avatarSync.then(() => {
fetchProfilePicture(newSession).finally(() => {
console.debug(
"[Auth Debug] User data fully loaded after sign in",
);
});
});
});
}
} else if (event === "TOKEN_REFRESHED") {
console.debug("[Auth Debug] Token refreshed");
// Optionally refresh pro status, profile picture metadata, and profile picture on token refresh
if (newSession?.user) {
Promise.all([
fetchProStatus(newSession),
fetchProfilePictureMetadata(newSession),
fetchProfilePicture(newSession),
]).then(() => {
console.debug(
"[Auth Debug] User data refreshed after token refresh",
);
});
}
loadedForRef.current = null;
} else if (event === "SIGNED_IN" || event === "TOKEN_REFRESHED") {
console.debug("[Auth Debug] Signed in or token refreshed");
// Deliberately does not touch `loading`: Supabase also fires
// SIGNED_IN on tab wakeups, and gating the UI on it would unmount
// Landing -> HomePage on every tab switch. Pinned by a test.
void loadUserData(newSession);
} else if (event === "USER_UPDATED") {
console.debug("[Auth Debug] User updated");
@@ -454,14 +467,8 @@ export function AuthProvider({ children }: { children: ReactNode }) {
"[Auth Debug] User upgrade synchronized successfully",
);
// Refresh pro status, profile picture metadata, and profile picture after upgrade
if (newSession?.user) {
return Promise.all([
fetchProStatus(newSession),
fetchProfilePictureMetadata(newSession),
fetchProfilePicture(newSession),
]);
}
// Forced: same user id, so the guest's data must be replaced.
return loadUserData(newSession, { force: true });
})
.then(() => {
console.debug(
@@ -484,6 +491,10 @@ export function AuthProvider({ children }: { children: ReactNode }) {
mounted = false;
subscription.unsubscribe();
};
// Empty and load-bearing: must subscribe once. The closures are recreated
// when `session` changes, so listing deps would re-subscribe on every auth
// event; every call above passes its session explicitly instead. No lint
// rule enforces this, so do not "fix" these deps.
}, []);
const { t } = useTranslation();