Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
9 changes: 9 additions & 0 deletions .changeset/electron-virtual-router-navigation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@clerk/electron': patch
'@clerk/shared': patch
'@clerk/ui': patch
---

Keep Clerk's navigation inside the renderer. `ClerkProvider` now always supplies `routerPush`/`routerReplace`, so Clerk routes through your application's router when you provide one, and never navigates the window to an internal `/CLERK-ROUTER/VIRTUAL/...` path — which no custom protocol handler can serve, and which reloaded the renderer and dropped the user out of sign-in.

Applications that worked around this by passing no-op router functions, or by filtering `CLERK-ROUTER/VIRTUAL` out themselves, can remove those workarounds.
5 changes: 5 additions & 0 deletions .changeset/signin-transport-transfer-next-step.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Route an OAuth transfer to the sign-up continue step when sign-in uses a native OAuth transport. The callback previously navigated with hash-style URLs (`<sign-up-url>#/continue`) that the in-place component router cannot resolve, landing transferred sign-ups on the start card where submitting created a fresh sign-up without the verified external account.
2 changes: 0 additions & 2 deletions integration/templates/electron-vite/src/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ function App() {
<ClerkProvider
publishableKey={PUBLISHABLE_KEY}
__internal_clerkUIUrl={CLERK_UI_URL}
routerPush={() => {}}
routerReplace={() => {}}
>
<main data-testid='electron-app'>
<Show when='signed-out'>
Expand Down
21 changes: 21 additions & 0 deletions integration/tests/electron/basic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ type ElectronWindow = Window & {
tokenCache?: Partial<Record<'clearToken' | 'getToken' | 'saveToken', unknown>>;
oauthTransport?: Partial<Record<'getRedirectUrl' | 'open', unknown>>;
};
__hardNavigations?: string[];
};

test.describe('electron basic auth @electron', () => {
Expand Down Expand Up @@ -74,4 +75,24 @@ test.describe('electron basic auth @electron', () => {
test('keeps the signed-out state after relaunch', async ({ electronPage }) => {
await expect(electronPage.locator('.cl-signIn-root')).toBeVisible({ timeout: 30_000 });
});

test('never hard navigates the renderer during sign-in', async ({ electronPage }) => {
const { signIn } = createPageObjects({ page: electronPage, useTestingToken: false });

await electronPage.evaluate(() => {
(window as ElectronWindow).__hardNavigations = [];
addEventListener('clerk:beforeunload', () => {
(window as ElectronWindow).__hardNavigations?.push(location.href);
});
});

await signIn.waitForMounted();
await signIn.setIdentifier(fakeUser.email!);
await signIn.continue();
await signIn.setPassword(fakeUser.password);
await signIn.continue();

await expect(electronPage.locator('[data-testid="user-id"]')).toHaveText(/^user_/, { timeout: 30_000 });
await expect(electronPage.evaluate(() => (window as ElectronWindow).__hardNavigations)).resolves.toEqual([]);
});
});
71 changes: 71 additions & 0 deletions packages/electron/src/react/__tests__/ClerkProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,77 @@ describe('Electron ClerkProvider', () => {
});
});

describe('router handlers', () => {
const renderWithRouter = (props: Record<string, unknown> = {}) => {
renderToStaticMarkup(
<ClerkProvider
publishableKey='pk_test_provider'
{...props}
>
<span>App</span>
</ClerkProvider>,
);

return {
routerPush: capturedProviderProps?.routerPush as (to: string, metadata?: unknown) => void,
routerReplace: capturedProviderProps?.routerReplace as (to: string, metadata?: unknown) => void,
};
};

it('always supplies both handlers so clerk-js never falls back to a window navigation', () => {
const { routerPush, routerReplace } = renderWithRouter();

expect(routerPush).toBeTypeOf('function');
expect(routerReplace).toBeTypeOf('function');
});

it('absorbs virtual router paths instead of forwarding them to the application router', () => {
const push = vi.fn();
const replace = vi.fn();
const windowNavigate = vi.fn();
const { routerPush, routerReplace } = renderWithRouter({ routerPush: push, routerReplace: replace });

routerPush('/CLERK-ROUTER/VIRTUAL/sign-up#/continue', { windowNavigate });
routerReplace('/CLERK-ROUTER/VIRTUAL/sign-in#/factor-two', { windowNavigate });

expect(push).not.toHaveBeenCalled();
expect(replace).not.toHaveBeenCalled();
expect(windowNavigate).not.toHaveBeenCalled();
});

it('absorbs virtual router paths even without an application router', () => {
const windowNavigate = vi.fn();
const { routerPush } = renderWithRouter();

routerPush('/CLERK-ROUTER/VIRTUAL/sign-up#/continue', { windowNavigate });

expect(windowNavigate).not.toHaveBeenCalled();
});

it('forwards real destinations to the application router', () => {
const push = vi.fn();
const replace = vi.fn();
const windowNavigate = vi.fn();
const { routerPush, routerReplace } = renderWithRouter({ routerPush: push, routerReplace: replace });

routerPush('/settings/connections', { windowNavigate });
routerReplace('/dashboard', { windowNavigate });

expect(push).toHaveBeenCalledWith('/settings/connections', { windowNavigate });
expect(replace).toHaveBeenCalledWith('/dashboard', { windowNavigate });
expect(windowNavigate).not.toHaveBeenCalled();
});

it('falls back to the host navigation for real destinations when no application router is provided', () => {
const windowNavigate = vi.fn();
const { routerPush } = renderWithRouter();

routerPush('/settings/connections', { windowNavigate });

expect(windowNavigate).toHaveBeenCalledWith('/settings/connections');
});
});

it('defaults allowedRedirectProtocols to the renderer custom scheme', () => {
stubWindowProtocol('clerk:');

Expand Down
29 changes: 29 additions & 0 deletions packages/electron/src/react/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { ClerkProviderProps as ReactClerkProviderProps } from '@clerk/react';
import { InternalClerkProvider as ReactClerkProvider } from '@clerk/react/internal';
import { isVirtualRouterPath } from '@clerk/shared/internal/clerk-js/url';
import { ALLOWED_PROTOCOLS } from '@clerk/shared/internal/clerk-js/windowNavigate';
import { loadClerkUIScript } from '@clerk/shared/loadClerkJsScript';
import type { ClerkUIConstructor } from '@clerk/shared/ui';
Expand Down Expand Up @@ -79,6 +80,30 @@ function createOAuthTransport(): ClerkOAuthTransport | undefined {
};
}

type ClerkRouterFn = NonNullable<ReactClerkProviderProps['routerPush']>;

/** Always supplied, so clerk-js never reaches `window.location` and reloads the renderer. */
function createRouterHandlers(
routerPush: ClerkRouterFn | undefined,
routerReplace: ClerkRouterFn | undefined,
): { routerPush: ClerkRouterFn; routerReplace: ClerkRouterFn } {
const wrap =
(delegate: ClerkRouterFn | undefined): ClerkRouterFn =>
(to, metadata) => {
if (isVirtualRouterPath(to)) {
return;
}

if (delegate) {
return delegate(to, metadata);
}

metadata?.windowNavigate(to);
};

return { routerPush: wrap(routerPush), routerReplace: wrap(routerReplace) };
}

/**
* Infer the custom renderer scheme registered with `createClerkBridge({ renderer })`.
* Built-in Clerk protocols and local file renderers are not inferred.
Expand All @@ -98,15 +123,19 @@ export function ClerkProvider({
publishableKey,
passkeys,
allowedRedirectProtocols,
routerPush,
routerReplace,
...props
}: ClerkProviderProps): JSX.Element {
const clerk = createClerkInstance(publishableKey, passkeys);
const oauthTransport = createOAuthTransport();
const clerkUI = loadClerkUI(publishableKey, props);
const routerHandlers = createRouterHandlers(routerPush, routerReplace);

return (
<ReactClerkProvider
{...props}
{...routerHandlers}
Clerk={clerk}
__internal_oauthTransport={oauthTransport}
allowedRedirectProtocols={allowedRedirectProtocols ?? defaultAllowedRedirectProtocols()}
Expand Down
7 changes: 7 additions & 0 deletions packages/shared/src/internal/clerk-js/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,13 @@ export const pathFromFullPath = (fullPath: string) => {
return fullPath.replace(/CLERK-ROUTER\/(.*?)\//, '');
};

export const VIRTUAL_ROUTER_BASE_PATH = 'CLERK-ROUTER/VIRTUAL';

/**
* Whether `to` addresses the in-memory component router rather than a real application route.
*/
export const isVirtualRouterPath = (to: string): boolean => to.includes(VIRTUAL_ROUTER_BASE_PATH);

const frontendApiRedirectPathsWithUserInput: string[] = [
'/oauth/authorize', // OAuth2 identify provider flow
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,57 @@ describe('buildSignInOAuthTransportCallbackParams', () => {
unsafeMetadata: { a: 1 },
} as any;

const origin = window.location.origin;

expect(buildSignInOAuthTransportCallbackParams(ctx)).toEqual({
signUpUrl: '/sign-up',
signInUrl: '/sign-in',
signInForceRedirectUrl: '/after-in',
signUpForceRedirectUrl: '/after-up',
continueSignUpUrl: '/continue',
transferable: true,
firstFactorUrl: 'factor-one',
secondFactorUrl: 'factor-two',
resetPasswordUrl: 'reset-password',
// Relative to the SignIn start route; the sign-up gate URL stays absolute (combined-aware).
signInProtectCheckUrl: 'protect-check',
signUpProtectCheckUrl: '/sign-up-protect-check',
// Sign-up steps are path routes on the sign-up component; hash-style URLs would lose their
// hash in the virtual router and land a transferred sign-up on the start card.
continueSignUpUrl: `${origin}/sign-up/continue`,
verifyEmailAddressUrl: `${origin}/sign-up/verify-email-address`,
verifyPhoneNumberUrl: `${origin}/sign-up/verify-phone-number`,
signUpProtectCheckUrl: `${origin}/sign-up/protect-check`,
unsafeMetadata: { a: 1 },
});
});

it('targets the virtual sign-up routes for modal transport callbacks', () => {
const ctx = {
signUpUrl: '/CLERK-ROUTER/VIRTUAL/sign-up',
signInUrl: '/CLERK-ROUTER/VIRTUAL/sign-in',
} as any;

const params = buildSignInOAuthTransportCallbackParams(ctx);
const origin = window.location.origin;

expect(params.continueSignUpUrl).toBe(`${origin}/CLERK-ROUTER/VIRTUAL/sign-up/continue`);
expect(params.verifyEmailAddressUrl).toBe(`${origin}/CLERK-ROUTER/VIRTUAL/sign-up/verify-email-address`);
expect(params.verifyPhoneNumberUrl).toBe(`${origin}/CLERK-ROUTER/VIRTUAL/sign-up/verify-phone-number`);
expect(params.signUpProtectCheckUrl).toBe(`${origin}/CLERK-ROUTER/VIRTUAL/sign-up/protect-check`);
});

it('targets the embedded create subtree in the combined flow', () => {
const ctx = {
signUpUrl: '/sign-in#/create',
signInUrl: '/sign-in',
isCombinedFlow: true,
} as any;

const params = buildSignInOAuthTransportCallbackParams(ctx);

expect(params.continueSignUpUrl).toBe('create/continue');
expect(params.verifyEmailAddressUrl).toBe('create/verify-email-address');
expect(params.verifyPhoneNumberUrl).toBe('create/verify-phone-number');
expect(params.signUpProtectCheckUrl).toBe('create/protect-check');
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

describe('buildSignUpOAuthCallbackParams', () => {
Expand Down
16 changes: 16 additions & 0 deletions packages/ui/src/components/SignIn/buildOAuthCallbackParams.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { buildURL, trimTrailingSlash } from '@clerk/shared/internal/clerk-js/url';
import type { HandleOAuthCallbackParams } from '@clerk/shared/types';

import type { SignInContextType } from '../../contexts/components/SignIn';
Expand All @@ -23,12 +24,27 @@ export function buildSignInOAuthCallbackParams(ctx: SignInContextType): HandleOA
}

export function buildSignInOAuthTransportCallbackParams(ctx: SignInContextType): HandleOAuthCallbackParams {
// Path form, not `#/step`: the in-place component router matches on pathname only and would drop the hash.
const signUpStepUrl = (step: string): string => {
if (ctx.isCombinedFlow) {
return `create/${step}`;
}
const url = buildURL({ base: ctx.signUpUrl }, { stringify: false });
url.pathname = `${trimTrailingSlash(url.pathname)}/${step}`;
url.hash = '';
return url.href;
};

return {
...buildSignInOAuthCallbackParams(ctx),
firstFactorUrl: 'factor-one',
secondFactorUrl: 'factor-two',
resetPasswordUrl: 'reset-password',
signInProtectCheckUrl: 'protect-check',
continueSignUpUrl: signUpStepUrl('continue'),
verifyEmailAddressUrl: signUpStepUrl('verify-email-address'),
verifyPhoneNumberUrl: signUpStepUrl('verify-phone-number'),
signUpProtectCheckUrl: signUpStepUrl('protect-check'),
};
}

Expand Down
4 changes: 3 additions & 1 deletion packages/ui/src/router/VirtualRouter.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { VIRTUAL_ROUTER_BASE_PATH } from '@clerk/shared/internal/clerk-js/url';
import { useClerk } from '@clerk/shared/react';
import React, { useEffect } from 'react';

import { useClerkModalStateParams } from '../hooks';
import { BaseRouter } from './BaseRouter';
export const VIRTUAL_ROUTER_BASE_PATH = 'CLERK-ROUTER/VIRTUAL';

export { VIRTUAL_ROUTER_BASE_PATH };

interface VirtualRouterProps {
startPath: string;
Expand Down
Loading