Skip to content

fix(react-ui): make each OAuth flow own the popup it opened #2014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AngelPaella wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(react-ui): make each OAuth flow own the popup it opened #2014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e035b52
fix(react-ui): make each OAuth flow own the popup it opened
AngelPaella Aug 10, 2026
532b72d
Merge origin/main into fix/eng4-360-oauth-popup-ownership
AngelPaella Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/oauth-popup-ownership.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
"@crossmint/client-sdk-react-ui": patch
---

Fixes the OAuth popup flow losing logins when a second provider is clicked.

All providers share one named popup (`PopupWindow.initEmpty` opens `window.open(..., "popupWindow")`), so a second click wraps the window a first flow is still using in a client of its own. The flow resumes at three points after handing that popup over: the URL await, the auth-material refresh await, and the 2.5s closure poller. Each flow now claims the popup and re-checks the claim at every one of them, so a superseded flow can no longer close the popup, publish its error, or clear the loading state belonging to the flow that replaced it.

Two teardown-ordering fixes come with it:

- Listener teardown happens when a new flow takes the popup over, not at the top of the call. Doing it earlier left a still-live popup with nothing listening across the URL await, dropping a login that had already completed.
- The closure poller no longer removes listeners. The callback page posts its one-time secret and then closes itself, so a tick landing in that gap dropped the material. Takeover and unmount both remove them, so at most one pair is ever alive.

Unmounting mid-flow now closes the popup it opened, instead of leaving a window on screen that nothing will ever close.
190 changes: 178 additions & 12 deletions packages/client/ui/react-ui/src/hooks/use-oauth-window-listener.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,44 +1,78 @@
import { act, renderHook } from "@testing-library/react";
import { act, cleanup, renderHook, waitFor } from "@testing-library/react";
import { afterEach, describe, expect, test, vi } from "vitest";

import type { OAuthProvider } from "@crossmint/common-sdk-auth";

const getOAuthUrl = vi.fn();
const handleRefreshAuthMaterial = vi.fn();

vi.mock("@/hooks", () => ({
useCrossmintAuth: () => ({ crossmintAuth: { handleRefreshAuthMaterial } }),
useCrossmintAuth: () => ({ crossmintAuth: { getOAuthUrl, handleRefreshAuthMaterial } }),
}));

import { useOAuthWindowListener } from "./useOAuthWindowListener";

const OAUTH_URL_MAP = { google: "https://www.crossmint.com/auth/oauth/google" } as Record<OAuthProvider, string>;
// No prefetched URLs, so every click awaits getOAuthUrl.
const NO_PREFETCHED_URLS = { google: "", twitter: "" } as Record<OAuthProvider, string>;
const PREFETCHED_URLS = {
google: "https://oauth.example/google",
twitter: "https://oauth.example/twitter",
} as Record<OAuthProvider, string>;

// Kept outside the render callback so a superseded flow's writes are observable, and so the
// hook's useCallback sees the stable identity OAuthFlowProvider gives it in production.
const setError = vi.fn();

function fakeWindow(): Window {
return { postMessage: vi.fn(), close: vi.fn(), closed: false, location: { href: "" } } as unknown as Window;
return {
postMessage: vi.fn(),
close: vi.fn(),
closed: false,
location: { href: "about:blank" },
} as unknown as Window;
}

// PopupWindow.initEmpty opens a window by name, so every flow wraps the same popup.
function openPopup(): Window {
const popup = fakeWindow();
vi.spyOn(window, "open").mockReturnValue(popup);
return popup;
}

function renderOAuthHook(urls = NO_PREFETCHED_URLS) {
return renderHook(() => useOAuthWindowListener(urls, setError));
}

function deliverAuthMaterial(source: Window) {
const event = new MessageEvent("message", {
data: { event: "authMaterialFromPopupCallback", data: { oneTimeSecret: "one-time-secret" } },
function deliver(source: Window, event: string, data: unknown) {
const messageEvent = new MessageEvent("message", {
data: { event, data },
origin: "https://www.crossmint.com",
});
// jsdom's MessageEvent only accepts a real WindowProxy as `source`, so install the stub directly.
Object.defineProperty(event, "source", { value: source });
window.dispatchEvent(event);
Object.defineProperty(messageEvent, "source", { value: source });
window.dispatchEvent(messageEvent);
}

function deliverAuthMaterial(source: Window, oneTimeSecret = "one-time-secret") {
deliver(source, "authMaterialFromPopupCallback", { oneTimeSecret });
}

describe("useOAuthWindowListener", () => {
afterEach(() => {
// Before clearing the spies, not after: unmounting tears a live flow down, and testing
// library's auto-cleanup runs late enough that those calls would land in the next test.
cleanup();
vi.restoreAllMocks();
getOAuthUrl.mockReset();
handleRefreshAuthMaterial.mockClear();
setError.mockClear();
});

describe("when another window on the page sends the callback", () => {
test("takes auth material only from the popup it opened", async () => {
const popup = fakeWindow();
vi.spyOn(window, "open").mockReturnValue(popup);
const popup = openPopup();

const { result } = renderHook(() => useOAuthWindowListener(OAUTH_URL_MAP, vi.fn()));
const { result } = renderOAuthHook(PREFETCHED_URLS);
await act(async () => {
await result.current.createPopupAndSetupListeners("google" as OAuthProvider);
});
Expand All @@ -54,4 +88,136 @@ describe("useOAuthWindowListener", () => {
expect(handleRefreshAuthMaterial).toHaveBeenCalledWith("one-time-secret");
});
});

describe("when a second provider is clicked while the first URL is still resolving", () => {
test("leaves one listener pair on the shared popup, so the secret is redeemed once", async () => {
const popup = openPopup();
const resolvers: Array<(url: string) => void> = [];
getOAuthUrl.mockImplementation(() => new Promise<string>((resolve) => resolvers.push(resolve)));

const { result } = renderOAuthHook();

await act(async () => {
result.current.createPopupAndSetupListeners("google" as OAuthProvider);
result.current.createPopupAndSetupListeners("twitter" as OAuthProvider);
await waitFor(() => expect(resolvers).toHaveLength(2));
resolvers[0]("https://oauth.example/google");
resolvers[1]("https://oauth.example/twitter");
});

await waitFor(() => expect(popup.location.href).toBe("https://oauth.example/twitter"));

await act(async () => {
deliverAuthMaterial(popup);
});
expect(handleRefreshAuthMaterial).toHaveBeenCalledTimes(1);
});

test("leaves the popup, the error and the loading state to the flow that took over", async () => {
const popup = openPopup();
const resolvers: Array<(url: string) => void> = [];
const rejecters: Array<(error: Error) => void> = [];
getOAuthUrl.mockImplementation(
() =>
new Promise<string>((resolve, reject) => {
resolvers.push(resolve);
rejecters.push(reject);
})
);

const { result } = renderOAuthHook();

await act(async () => {
result.current.createPopupAndSetupListeners("google" as OAuthProvider);
result.current.createPopupAndSetupListeners("twitter" as OAuthProvider);
await waitFor(() => expect(resolvers).toHaveLength(2));
resolvers[1]("https://oauth.example/twitter");
});
await act(async () => {
rejecters[0](new Error("google 500"));
});

expect(popup.location.href).toBe("https://oauth.example/twitter");
expect(popup.close).not.toHaveBeenCalled();
expect(setError).not.toHaveBeenCalledWith("google 500");
expect(result.current.activeOAuthProvider).toBe("twitter");
});
});

describe("when a second provider is clicked after the first flow is fully set up", () => {
test("drops the first flow's listeners, so the secret is redeemed once", async () => {
const popup = openPopup();
const { result } = renderOAuthHook(PREFETCHED_URLS);

await act(async () => {
await result.current.createPopupAndSetupListeners("google" as OAuthProvider);
});
await act(async () => {
await result.current.createPopupAndSetupListeners("twitter" as OAuthProvider);
});

await act(async () => {
deliverAuthMaterial(popup);
});

expect(handleRefreshAuthMaterial).toHaveBeenCalledTimes(1);
expect(popup.location.href).toBe("https://oauth.example/twitter");
});
});

describe("when the popup reports auth material", () => {
test("redeems the secret, then closes the popup and clears the loading state", async () => {
const popup = openPopup();
const { result } = renderOAuthHook(PREFETCHED_URLS);

await act(async () => {
await result.current.createPopupAndSetupListeners("google" as OAuthProvider);
});
await act(async () => {
deliverAuthMaterial(popup, "secret");
});

expect(handleRefreshAuthMaterial).toHaveBeenCalledWith("secret");
expect(popup.close).toHaveBeenCalled();
expect(result.current.activeOAuthProvider).toBeNull();
});
});

describe("when the provider unmounts mid-flow", () => {
test("closes the popup it opened and stops redeeming what arrives after", async () => {
const popup = openPopup();
const { result, unmount } = renderOAuthHook(PREFETCHED_URLS);

await act(async () => {
await result.current.createPopupAndSetupListeners("google" as OAuthProvider);
});
unmount();

expect(popup.close).toHaveBeenCalled();

await act(async () => {
deliverAuthMaterial(popup);
});
expect(handleRefreshAuthMaterial).not.toHaveBeenCalled();
});

test("does not publish an error for the flow it abandoned", async () => {
openPopup();
const rejecters: Array<(error: Error) => void> = [];
getOAuthUrl.mockImplementation(() => new Promise<string>((_, reject) => rejecters.push(reject)));

const { result, unmount } = renderOAuthHook();

await act(async () => {
result.current.createPopupAndSetupListeners("google" as OAuthProvider);
await waitFor(() => expect(rejecters).toHaveLength(1));
});
unmount();
await act(async () => {
rejecters[0](new Error("google 500"));
});

expect(setError).not.toHaveBeenCalledWith("google 500");
});
});
});
76 changes: 61 additions & 15 deletions packages/client/ui/react-ui/src/hooks/useOAuthWindowListener.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useCallback } from "react";
import { useEffect, useRef, useState, useCallback } from "react";
import type { OAuthProvider } from "@crossmint/common-sdk-auth";
import { PopupWindow } from "@crossmint/client-sdk-window";
import { useCrossmintAuth } from "@/hooks";
Expand All @@ -10,9 +10,33 @@ export const useOAuthWindowListener = (oauthUrlMap: OAuthUrlMap, setError: (erro
const { crossmintAuth } = useCrossmintAuth();
// Track which OAuth provider's window is currently being interacted with
const [activeOAuthProvider, setActiveOAuthProvider] = useState<OAuthProvider | null>(null);
const cleanupRef = useRef<(() => void) | null>(null);
const popupRef = useRef<PopupWindow<IncomingEvents, OutgoingEvents> | null>(null);
// Every click claims the next id. Because all flows share one named popup, each resumption
// point below has to check it still holds the claim before touching the popup or the state.
const flowIdRef = useRef(0);
const mountedRef = useRef(true);

useEffect(() => {
mountedRef.current = true;

return () => {
mountedRef.current = false;
cleanupRef.current?.();
cleanupRef.current = null;
// Nothing is left to adopt this popup, so abort the flow instead of leaving a window
// on screen that no longer has a listener to close it.
popupRef.current?.window?.close();
popupRef.current = null;
};
}, []);

const createPopupAndSetupListeners = useCallback(
async (provider: OAuthProvider, providerLoginHint?: string) => {
// Claim the flow before the first await.
const flowId = ++flowIdRef.current;
const ownsPopup = () => flowIdRef.current === flowId && mountedRef.current;

setActiveOAuthProvider(provider);
setError(null);

Expand All @@ -27,6 +51,7 @@ export const useOAuthWindowListener = (oauthUrlMap: OAuthUrlMap, setError: (erro
height: 700,
incomingEvents,
});
popupRef.current = popup;

const prefetchedUrl = oauthUrlMap[provider];
const resolvedUrl = prefetchedUrl || (await crossmintAuth?.getOAuthUrl(provider));
Expand All @@ -35,9 +60,17 @@ export const useOAuthWindowListener = (oauthUrlMap: OAuthUrlMap, setError: (erro
}
baseUrl = new URL(resolvedUrl);
} catch (e) {
popup?.window?.close();
setActiveOAuthProvider(null);
setError(e instanceof Error ? e.message : "Failed to start OAuth login");
if (ownsPopup()) {
popup?.window?.close();
setActiveOAuthProvider(null);
setError(e instanceof Error ? e.message : "Failed to start OAuth login");
}
return;
}

// PopupWindow.initEmpty opens a named window, so the later click reused this
// popup. Leave it to that flow rather than closing it or navigating it again.
if (!ownsPopup()) {
return;
}

Expand All @@ -59,26 +92,33 @@ export const useOAuthWindowListener = (oauthUrlMap: OAuthUrlMap, setError: (erro
});
}

// Drop the previous flow's listeners only now that this one is taking the popup over.
// Doing it before the await above would leave its still-live popup with nothing listening.
cleanupRef.current?.();

if (popup.window != null) {
popup.window.location.href = baseUrl.toString();
}

// Listen on the popup itself: it is the window that sends these events, and its
// transport drops anything from another sender.
let stopListening = () => {
// reassigned below, once there is something to unsubscribe
};

// transport drops anything from another sender. Every flow wraps the same reused
// window in its own client, so the takeover above is what keeps one pair alive.
const handleAuthMaterial = async (data: { oneTimeSecret: string }) => {
await crossmintAuth?.handleRefreshAuthMaterial(data.oneTimeSecret);
stopListening();
if (!ownsPopup()) {
Comment on lines 106 to +108

@greptile-apps greptile-apps Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Ownership checked after auth mutation

When a popup's redemption remains pending and the user starts another provider flow, handleRefreshAuthMaterial stores the superseded flow's tokens and invokes its refresh callback before ownsPopup() runs, causing the abandoned provider to publish shared authentication state.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/client/ui/react-ui/src/hooks/useOAuthWindowListener.ts
Line: 112-114

Comment:
**Ownership checked after auth mutation**

When a popup's redemption remains pending and the user starts another provider flow, `handleRefreshAuthMaterial` stores the superseded flow's tokens and invokes its refresh callback before `ownsPopup()` runs, causing the abandoned provider to publish shared authentication state.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

return;
}
cleanup();
popup.window?.close();
setActiveOAuthProvider(null);
};

const handleError = (data: { error: string }) => {
if (!ownsPopup()) {
return;
}
setError(data.error);
stopListening();
cleanup();
popup.window?.close();
setActiveOAuthProvider(null);
};
Expand All @@ -87,18 +127,24 @@ export const useOAuthWindowListener = (oauthUrlMap: OAuthUrlMap, setError: (erro
const errorListenerId = popup.on("errorFromPopupCallback", handleError);
// Add a check for manual window closure
// Ideally we should find a more explicit way of doing this, but I think this is fine for now.
// The listeners deliberately stay registered: a callback page posts its secret and then closes
// itself, so tearing them down here would drop material that is already in flight. Takeover
// and unmount both remove them, so at most one pair is ever alive.
const checkWindowClosure = setInterval(() => {
if (popup.window?.closed) {
stopListening();
setActiveOAuthProvider(null);
clearInterval(checkWindowClosure);
if (ownsPopup()) {
setActiveOAuthProvider(null);
}
}
Comment on lines 133 to 139

@greptile-apps greptile-apps Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Closed popup poller stays active

After the user manually closes the popup, this branch clears only activeOAuthProvider; the interval, closed popup reference, and message listeners remain alive until another flow starts or the provider unmounts, causing recurring wakeups and retained resources for the remaining provider lifetime.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/client/ui/react-ui/src/hooks/useOAuthWindowListener.ts
Line: 139-145

Comment:
**Closed popup poller stays active**

After the user manually closes the popup, this branch clears only `activeOAuthProvider`; the interval, closed popup reference, and message listeners remain alive until another flow starts or the provider unmounts, causing recurring wakeups and retained resources for the remaining provider lifetime.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

}, 2500); // Check every 2.5 seconds

stopListening = () => {
const cleanup = () => {
clearInterval(checkWindowClosure);
popup.off(authMaterialListenerId);
popup.off(errorListenerId);
clearInterval(checkWindowClosure);
};
cleanupRef.current = cleanup;
},
[oauthUrlMap, crossmintAuth, setError]
);
Expand Down
Loading