Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/electron-replace-stale-oauth-flow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/electron': patch
---

Fix OAuth sign-in staying blocked after a user abandons the browser tab. Starting a new OAuth sign-in now replaces the pending one instead of failing with `Clerk: an OAuth flow is already pending.` until the 3-minute callback timeout. An unsupported OAuth URL no longer cancels a pending flow, a replaced flow's browser launch failure no longer rejects the new flow, and replacing a flow while its browser is still launching no longer causes an unhandled promise rejection in the main process.
Comment thread
steve-cooks marked this conversation as resolved.
109 changes: 109 additions & 0 deletions packages/electron/src/main/__tests__/create-clerk-bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -705,4 +705,113 @@ describe('createClerkBridge', () => {
await expect(openPromise).resolves.toEqual({ callbackUrl: 'my-app://renderer/?code=123' });
expect(rendererWindow.focus).not.toHaveBeenCalled();
});

it('replaces a pending OAuth flow when a new one starts', async () => {
vi.mocked(shell.openExternal).mockResolvedValue(undefined);
createClerkBridge({
storage,
renderer: {
host: 'renderer',
scheme: 'my-app',
},
});

const openHandler = vi.mocked(ipcMain.handle).mock.calls.find(([channel]) => {
return channel === OAUTH_TRANSPORT_CHANNELS.open;
})?.[1];
const firstPromise = openHandler?.(mainFrameEvent, 'https://accounts.example.com/oauth?attempt=1');
const secondPromise = openHandler?.(mainFrameEvent, 'https://accounts.example.com/oauth?attempt=2');
const openUrlListener = getAppListener<(event: Electron.Event, url: string) => void>('open-url');

await expect(firstPromise).rejects.toThrow('replaced by a new one');
openUrlListener?.({ preventDefault: vi.fn() } as unknown as Electron.Event, 'my-app://renderer/?code=123');
await expect(secondPromise).resolves.toEqual({ callbackUrl: 'my-app://renderer/?code=123' });
});

it('keeps a pending OAuth flow when a new request has an unsupported URL', async () => {
vi.mocked(shell.openExternal).mockResolvedValue(undefined);
createClerkBridge({
storage,
renderer: {
host: 'renderer',
scheme: 'my-app',
},
});

const openHandler = vi.mocked(ipcMain.handle).mock.calls.find(([channel]) => {
return channel === OAUTH_TRANSPORT_CHANNELS.open;
})?.[1];
const pendingPromise = openHandler?.(mainFrameEvent, 'https://accounts.example.com/oauth');

await expect(openHandler?.(mainFrameEvent, 'file:///etc/passwd')).rejects.toThrow('unsupported OAuth URL protocol');

const openUrlListener = getAppListener<(event: Electron.Event, url: string) => void>('open-url');
openUrlListener?.({ preventDefault: vi.fn() } as unknown as Electron.Event, 'my-app://renderer/?code=123');
await expect(pendingPromise).resolves.toEqual({ callbackUrl: 'my-app://renderer/?code=123' });
});

it("does not let a replaced flow's launch failure reject the new flow", async () => {
let rejectFirstLaunch: (error: Error) => void = () => {};
vi.mocked(shell.openExternal)
.mockImplementationOnce(
() =>
new Promise<void>((_resolve, reject) => {
rejectFirstLaunch = reject;
}),
)
.mockResolvedValueOnce(undefined);
createClerkBridge({
storage,
renderer: {
host: 'renderer',
scheme: 'my-app',
},
});

const openHandler = vi.mocked(ipcMain.handle).mock.calls.find(([channel]) => {
return channel === OAUTH_TRANSPORT_CHANNELS.open;
})?.[1];
const firstPromise = openHandler?.(mainFrameEvent, 'https://accounts.example.com/oauth?attempt=1');
const secondPromise = openHandler?.(mainFrameEvent, 'https://accounts.example.com/oauth?attempt=2');

rejectFirstLaunch(new Error('launch failed'));
await expect(firstPromise).rejects.toThrow('replaced by a new one');

const openUrlListener = getAppListener<(event: Electron.Event, url: string) => void>('open-url');
openUrlListener?.({ preventDefault: vi.fn() } as unknown as Electron.Event, 'my-app://renderer/?code=123');
await expect(secondPromise).resolves.toEqual({ callbackUrl: 'my-app://renderer/?code=123' });
});

it('does not leave an unhandled rejection when a flow is replaced while its browser is launching', async () => {
let resolveFirstLaunch: () => void = () => {};
vi.mocked(shell.openExternal)
.mockImplementationOnce(
() =>
new Promise<void>(resolve => {
resolveFirstLaunch = resolve;
}),
)
.mockResolvedValueOnce(undefined);
createClerkBridge({
storage,
renderer: {
host: 'renderer',
scheme: 'my-app',
},
});

const openHandler = vi.mocked(ipcMain.handle).mock.calls.find(([channel]) => {
return channel === OAUTH_TRANSPORT_CHANNELS.open;
})?.[1];
const firstPromise = openHandler?.(mainFrameEvent, 'https://accounts.example.com/oauth?attempt=1');
const secondPromise = openHandler?.(mainFrameEvent, 'https://accounts.example.com/oauth?attempt=2');
await new Promise(resolve => setTimeout(resolve, 0));

resolveFirstLaunch();
await expect(firstPromise).rejects.toThrow('replaced by a new one');

const openUrlListener = getAppListener<(event: Electron.Event, url: string) => void>('open-url');
openUrlListener?.({ preventDefault: vi.fn() } as unknown as Electron.Event, 'my-app://renderer/?code=123');
await expect(secondPromise).resolves.toEqual({ callbackUrl: 'my-app://renderer/?code=123' });
Comment thread
steve-cooks marked this conversation as resolved.
});
});
14 changes: 8 additions & 6 deletions packages/electron/src/main/oauth-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,27 +123,29 @@ export function setupOAuthTransportIpcHandlers(options: OAuthTransportOptions):
throw new Error("Clerk: OAuth request did not originate from a window's main frame.");
}

if (pendingOAuthFlow) {
throw new Error('Clerk: an OAuth flow is already pending.');
}

assertExternalOAuthUrl(url);
disposePendingOAuthFlow(new Error('Clerk: OAuth flow was replaced by a new one.'));
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const window = BrowserWindow.fromWebContents(event.sender);

let flow: PendingOAuthFlow | null = null;
const callbackPromise = new Promise<{ callbackUrl: string }>((resolve, reject) => {
const timeout = setTimeout(() => {
disposePendingOAuthFlow();
reject(new Error('Clerk: OAuth callback timed out.'));
}, CALLBACK_TIMEOUT_MS);

pendingOAuthFlow = { resolve, reject, timeout, window };
flow = { resolve, reject, timeout, window };
pendingOAuthFlow = flow;
});
callbackPromise.catch(() => {});

try {
await shell.openExternal(url);
} catch (err) {
disposePendingOAuthFlow(err instanceof Error ? err : new Error(String(err)));
if (pendingOAuthFlow === flow) {
disposePendingOAuthFlow(err instanceof Error ? err : new Error(String(err)));
}
}

return callbackPromise;
Expand Down