Skip to content

Commit 16b7d03

Browse files
committed
fix(clerk-js): validate window navigation protocols and honor allowedRedirectProtocols
1 parent 52d310c commit 16b7d03

16 files changed

Lines changed: 336 additions & 25 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
'@clerk/react': patch
4+
'@clerk/shared': patch
5+
'@clerk/ui': patch
6+
---
7+
8+
Fix missing redirect URL protocol validation for Clerk UI browser navigations, including the multi-session add-account flow.
9+
10+
Internal browser navigations now consistently honor configured redirect protocols and fail closed across mixed ClerkJS/UI bundle versions.

packages/clerk-js/bundlewatch.config.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
"files": [
33
{ "path": "./dist/clerk.js", "maxSize": "549KB" },
44
{ "path": "./dist/clerk.browser.js", "maxSize": "74KB" },
5-
{ "path": "./dist/clerk.legacy.browser.js", "maxSize": "114KB" },
5+
{ "path": "./dist/clerk.legacy.browser.js", "maxSize": "116KB" },
66
{ "path": "./dist/clerk.no-rhc.js", "maxSize": "316KB" },
7-
{ "path": "./dist/clerk.native.js", "maxSize": "72KB" },
7+
{ "path": "./dist/clerk.native.js", "maxSize": "74KB" },
88
{ "path": "./dist/vendors*.js", "maxSize": "7KB" },
99
{ "path": "./dist/coinbase*.js", "maxSize": "36KB" },
1010
{ "path": "./dist/base-account-sdk*.js", "maxSize": "207KB" },

packages/clerk-js/src/core/clerk.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,7 +1916,7 @@ export class Clerk implements ClerkInterface {
19161916

19171917
if (customNavigate) {
19181918
debugLogger.info(`Clerk is navigating to: ${to}`);
1919-
return await customNavigate(to, { windowNavigate });
1919+
return await customNavigate(to, { windowNavigate: this.__internal_windowNavigate });
19201920
}
19211921

19221922
// No window.location and no custom router - can't navigate
@@ -1955,13 +1955,13 @@ export class Clerk implements ClerkInterface {
19551955

19561956
// Custom protocol URLs have an origin value of 'null'. In many cases, this indicates deep-linking and we want to ensure the customNavigate function is used if available.
19571957
if ((toURL.origin !== 'null' && toURL.origin !== window.location.origin) || !customNavigate) {
1958-
windowNavigate(toURL);
1958+
this.__internal_windowNavigate(toURL);
19591959
return;
19601960
}
19611961

19621962
const metadata = {
19631963
...(options?.metadata ? { __internal_metadata: options?.metadata } : {}),
1964-
windowNavigate,
1964+
windowNavigate: this.__internal_windowNavigate,
19651965
};
19661966
// React router only wants the path, search or hash portion.
19671967
return await customNavigate(stripOrigin(toURL), metadata);
@@ -3578,6 +3578,21 @@ export class Clerk implements ClerkInterface {
35783578
return allowedProtocols;
35793579
}
35803580

3581+
/**
3582+
* Primary `window.location.href` navigation chokepoint for `@clerk/clerk-js` and `@clerk/ui`.
3583+
* By default the resolved URL is validated against the customer-supplied
3584+
* `allowedRedirectProtocols` option (the static `ALLOWED_PROTOCOLS` ∪ the customer extension),
3585+
* so internal callers honor customer protocols automatically.
3586+
*
3587+
* Pass `useStaticAllowlistOnly: true` to opt out of the customer extension when a call site
3588+
* must reject any protocol the customer added. There is no current internal consumer of the
3589+
* opt-out; it exists for future security-critical paths.
3590+
*/
3591+
public __internal_windowNavigate = (to: URL | string, opts?: { useStaticAllowlistOnly?: boolean }): void => {
3592+
const allowedProtocols = opts?.useStaticAllowlistOnly ? ALLOWED_PROTOCOLS : this.#allowedRedirectProtocols;
3593+
windowNavigate(to, { allowedProtocols });
3594+
};
3595+
35813596
#isLoaded(): this is LoadedClerk {
35823597
return this.client !== undefined;
35833598
}

packages/clerk-js/src/core/resources/SignIn.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
} from '@clerk/shared/internal/clerk-js/passkeys';
88
import { createValidatePassword } from '@clerk/shared/internal/clerk-js/passwords/password';
99
import { getClerkQueryParam } from '@clerk/shared/internal/clerk-js/queryParams';
10-
import { windowNavigate } from '@clerk/shared/internal/clerk-js/windowNavigate';
1110
import { Poller } from '@clerk/shared/poller';
1211
import type {
1312
AttemptFirstFactorParams,
@@ -392,7 +391,7 @@ export class SignIn extends BaseResource implements SignInResource {
392391
});
393392
}
394393

395-
return this.authenticateWithRedirectOrPopup(params, windowNavigate);
394+
return this.authenticateWithRedirectOrPopup(params, SignIn.clerk.__internal_windowNavigate);
396395
};
397396

398397
public authenticateWithPopup = async (params: AuthenticateWithPopupParams): Promise<void> => {
@@ -1199,7 +1198,7 @@ class SignInFuture implements SignInFutureResource {
11991198
// Pick up the modified SignIn resource
12001199
await this.#resource.reload();
12011200
} else {
1202-
windowNavigate(externalVerificationRedirectURL);
1201+
SignIn.clerk.__internal_windowNavigate(externalVerificationRedirectURL);
12031202
}
12041203
}
12051204
});

packages/clerk-js/src/core/resources/SignUp.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { inBrowser } from '@clerk/shared/browser';
22
import { type ClerkError, ClerkRuntimeError, isCaptchaError, isClerkAPIResponseError } from '@clerk/shared/error';
33
import { createValidatePassword } from '@clerk/shared/internal/clerk-js/passwords/password';
4-
import { windowNavigate } from '@clerk/shared/internal/clerk-js/windowNavigate';
54
import { Poller } from '@clerk/shared/poller';
65
import type {
76
AttemptEmailAddressVerificationParams,
@@ -462,7 +461,7 @@ export class SignUp extends BaseResource implements SignUpResource {
462461
});
463462
}
464463

465-
return this.authenticateWithRedirectOrPopup(params, windowNavigate);
464+
return this.authenticateWithRedirectOrPopup(params, SignUp.clerk.__internal_windowNavigate);
466465
};
467466

468467
public authenticateWithPopup = async (
@@ -1082,7 +1081,7 @@ class SignUpFuture implements SignUpFutureResource {
10821081
// Pick up the modified SignUp resource
10831082
await this.#resource.reload();
10841083
} else {
1085-
windowNavigate(externalVerificationRedirectURL);
1084+
SignUp.clerk.__internal_windowNavigate(externalVerificationRedirectURL);
10861085
}
10871086
}
10881087
});

packages/react/src/isomorphicClerk.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,10 @@ export class IsomorphicClerk implements IsomorphicLoadedClerk {
265265
return this.clerkjs?.__internal_getOption ? this.clerkjs?.__internal_getOption(key) : this.options[key];
266266
}
267267

268+
public __internal_windowNavigate: Clerk['__internal_windowNavigate'] = (to, opts) => {
269+
this.clerkjs?.__internal_windowNavigate?.(to, opts);
270+
};
271+
268272
constructor(options: IsomorphicClerkOptions) {
269273
this.#publishableKey = options?.publishableKey;
270274
this.#proxyUrl = options?.proxyUrl;
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
import { ALLOWED_PROTOCOLS, CLERK_BEFORE_UNLOAD_EVENT, windowNavigate } from '../windowNavigate';
4+
5+
describe('windowNavigate', () => {
6+
let originalLocation: Location;
7+
let hrefSetter: ReturnType<typeof vi.fn>;
8+
let warnSpy: ReturnType<typeof vi.spyOn>;
9+
let eventSpy: ReturnType<typeof vi.fn>;
10+
11+
beforeEach(() => {
12+
originalLocation = window.location;
13+
hrefSetter = vi.fn();
14+
Object.defineProperty(window, 'location', {
15+
configurable: true,
16+
value: new Proxy(
17+
{ href: 'https://example.com/' },
18+
{
19+
set: (target, prop, value) => {
20+
if (prop === 'href') {
21+
hrefSetter(value);
22+
(target as any).href = value;
23+
return true;
24+
}
25+
(target as any)[prop] = value;
26+
return true;
27+
},
28+
},
29+
),
30+
});
31+
warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
32+
eventSpy = vi.fn();
33+
window.addEventListener(CLERK_BEFORE_UNLOAD_EVENT, eventSpy);
34+
});
35+
36+
afterEach(() => {
37+
window.removeEventListener(CLERK_BEFORE_UNLOAD_EVENT, eventSpy);
38+
Object.defineProperty(window, 'location', {
39+
configurable: true,
40+
value: originalLocation,
41+
});
42+
warnSpy.mockRestore();
43+
});
44+
45+
it.each([
46+
['absolute https URL', 'https://example.com/dashboard'],
47+
['absolute http URL', 'http://example.com/dashboard'],
48+
['relative path', '/sign-in'],
49+
['wails protocol', 'wails://app/route'],
50+
['chrome-extension protocol', 'chrome-extension://abc/route'],
51+
])('navigates to %s', (_label, to) => {
52+
windowNavigate(to);
53+
expect(hrefSetter).toHaveBeenCalledTimes(1);
54+
expect(eventSpy).toHaveBeenCalledTimes(1);
55+
expect(warnSpy).not.toHaveBeenCalled();
56+
});
57+
58+
it.each([
59+
['javascript', 'javascript:alert(1)'],
60+
['data', 'data:text/html,<script>alert(1)</script>'],
61+
['file', 'file:///etc/passwd'],
62+
['vbscript', 'vbscript:msgbox(1)'],
63+
['mixed-case JavaScript', 'JavaScript:alert(1)'],
64+
['upper-case JAVASCRIPT', 'JAVASCRIPT:alert(1)'],
65+
['leading-whitespace javascript', ' javascript:alert(1)'],
66+
['leading-tab javascript', '\tjavascript:alert(1)'],
67+
['leading-newline javascript', '\njavascript:alert(1)'],
68+
])('blocks %s: protocol and does not navigate', (_label, to) => {
69+
windowNavigate(to);
70+
expect(hrefSetter).not.toHaveBeenCalled();
71+
expect(eventSpy).not.toHaveBeenCalled();
72+
expect(warnSpy).toHaveBeenCalledTimes(1);
73+
});
74+
75+
it('blocks javascript: URLs that the URL parser normalizes via the base URL', () => {
76+
windowNavigate('javascript:alert(location.origin)//');
77+
expect(hrefSetter).not.toHaveBeenCalled();
78+
expect(eventSpy).not.toHaveBeenCalled();
79+
expect(warnSpy).toHaveBeenCalledTimes(1);
80+
});
81+
82+
it.each([
83+
['scheme-relative //host', '//evil.example/path'],
84+
['scheme-relative ///host', '///evil.example/path'],
85+
['backslash \\\\host', '\\\\evil.example\\path'],
86+
['mixed /\\host', '/\\evil.example/path'],
87+
['mixed \\/host', '\\/evil.example/path'],
88+
['leading-whitespace scheme-relative', ' //evil.example/path'],
89+
['leading-tab scheme-relative', '\t//evil.example/path'],
90+
])('blocks %s and does not navigate', (_label, to) => {
91+
windowNavigate(to);
92+
expect(hrefSetter).not.toHaveBeenCalled();
93+
expect(eventSpy).not.toHaveBeenCalled();
94+
expect(warnSpy).toHaveBeenCalledTimes(1);
95+
});
96+
97+
it('still rejects scheme-relative URLs when an extended allowlist is supplied', () => {
98+
windowNavigate('//evil.example/path', {
99+
allowedProtocols: [...ALLOWED_PROTOCOLS, 'slack:'],
100+
});
101+
expect(hrefSetter).not.toHaveBeenCalled();
102+
expect(eventSpy).not.toHaveBeenCalled();
103+
expect(warnSpy).toHaveBeenCalledTimes(1);
104+
});
105+
106+
it('honors a caller-supplied extended allowlist for custom protocols', () => {
107+
windowNavigate('slack://channel/123', {
108+
allowedProtocols: [...ALLOWED_PROTOCOLS, 'slack:'],
109+
});
110+
expect(hrefSetter).toHaveBeenCalledTimes(1);
111+
expect(hrefSetter).toHaveBeenCalledWith('slack://channel/123');
112+
expect(eventSpy).toHaveBeenCalledTimes(1);
113+
expect(warnSpy).not.toHaveBeenCalled();
114+
});
115+
116+
it('still rejects disallowed protocols when an extended allowlist is supplied', () => {
117+
windowNavigate('javascript:alert(1)', {
118+
allowedProtocols: [...ALLOWED_PROTOCOLS, 'slack:'],
119+
});
120+
expect(hrefSetter).not.toHaveBeenCalled();
121+
expect(eventSpy).not.toHaveBeenCalled();
122+
expect(warnSpy).toHaveBeenCalledTimes(1);
123+
});
124+
});

packages/shared/src/internal/clerk-js/windowNavigate.ts

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,48 @@ export const ALLOWED_PROTOCOLS = [
1111
'chrome-extension:',
1212
];
1313

14+
export type WindowNavigateOptions = {
15+
/**
16+
* Protocol allowlist applied to the resolved URL. Defaults to `ALLOWED_PROTOCOLS`. Pass an
17+
* extended list (e.g. `Clerk`'s `#allowedRedirectProtocols`) to honor the customer-supplied
18+
* `allowedRedirectProtocols` option.
19+
*/
20+
allowedProtocols?: ReadonlyArray<string>;
21+
};
22+
23+
const SCHEME_RELATIVE_PREFIX = /^[/\\][/\\]/;
24+
1425
/**
1526
* Helper utility to navigate via window.location.href. Also dispatches a clerk:beforeunload custom event.
1627
*
17-
* Note that this utility should **never** be called with a user-provided URL. We make no specific checks against the contents of the URL here and assume it is safe. Use `Clerk.navigate()` instead for user-provided URLs.
28+
* Navigations whose protocol is not in the allowlist (e.g. `javascript:`, `data:`) are aborted.
29+
* Scheme-relative inputs (`//host`, `\\host`) are also rejected: they adopt the base URL's scheme,
30+
* which is always in the allowlist, so they would otherwise pass the protocol check while
31+
* redirecting cross-origin.
32+
*
33+
* Callers that have already validated against an extended allowlist should pass it via
34+
* `options.allowedProtocols` so legitimate custom protocols (Wails, Tauri, etc.) are honored.
35+
*
36+
* @deprecated Use `clerk.__internal_windowNavigate` instead. It honors the customer-supplied
37+
* `allowedRedirectProtocols` option by default, so internal call sites can't accidentally
38+
* bypass it by forgetting to pass `options.allowedProtocols`. The bare export will be removed
39+
* in the next major version.
1840
*/
19-
export function windowNavigate(to: URL | string): void {
41+
export function windowNavigate(to: URL | string, options?: WindowNavigateOptions): void {
42+
if (typeof to === 'string' && SCHEME_RELATIVE_PREFIX.test(to.trim())) {
43+
console.warn(
44+
`Clerk: scheme-relative navigation to "${to}" is not allowed. Provide a same-origin path or an absolute URL.`,
45+
);
46+
return;
47+
}
2048
const toURL = new URL(to, window.location.href);
49+
const allowedProtocols = options?.allowedProtocols ?? ALLOWED_PROTOCOLS;
50+
if (!allowedProtocols.includes(toURL.protocol)) {
51+
console.warn(
52+
`Clerk: "${toURL.protocol}" is not a valid navigation protocol. Aborting navigation. If you think this is a mistake, please open an issue.`,
53+
);
54+
return;
55+
}
2156
window.dispatchEvent(new CustomEvent(CLERK_BEFORE_UNLOAD_EVENT));
2257
window.location.href = toURL.href;
2358
}

packages/shared/src/types/clerk.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,16 @@ export interface Clerk {
288288
*/
289289
__internal_getOption<K extends keyof ClerkOptions>(key: K): ClerkOptions[K];
290290

291+
/**
292+
* @internal
293+
* Primary `window.location.href` navigation chokepoint for `@clerk/clerk-js` and `@clerk/ui`.
294+
* By default the resolved URL is validated against the customer-supplied
295+
* `allowedRedirectProtocols` option (static defaults ∪ the customer extension).
296+
* Disallowed protocols and scheme-relative inputs (`//host`) are rejected with a console warning.
297+
* Pass `useStaticAllowlistOnly: true` to opt out of the customer extension.
298+
*/
299+
__internal_windowNavigate: (to: URL | string, opts?: { useStaticAllowlistOnly?: boolean }) => void;
300+
291301
frontendApi: string;
292302

293303
/** Your Clerk [Publishable Key](!publishable-key). */

packages/ui/src/components/UserButton/useMultisessionActions.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { navigateIfTaskExists } from '@clerk/shared/internal/clerk-js/sessionTasks';
2-
import { windowNavigate } from '@clerk/shared/internal/clerk-js/windowNavigate';
32
import { useClerk, usePortalRoot } from '@clerk/shared/react';
43
import type { SignedInSessionResource, UserButtonProps, UserResource } from '@clerk/shared/types';
54

65
import { useEnvironment } from '@/ui/contexts';
76
import { useCardState } from '@/ui/elements/contexts';
87
import { sleep } from '@/ui/utils/sleep';
8+
import { clerkWindowNavigate } from '@/ui/utils/windowNavigate';
99

1010
import { useMultipleSessions } from '../../hooks/useMultipleSessions';
1111
import { useRouter } from '../../router';
@@ -22,7 +22,8 @@ type UseMultisessionActionsParams = {
2222
} & Pick<UserButtonProps, 'userProfileMode' | 'appearance' | 'userProfileProps'>;
2323

2424
export const useMultisessionActions = (opts: UseMultisessionActionsParams) => {
25-
const { setActive, signOut, openUserProfile } = useClerk();
25+
const clerk = useClerk();
26+
const { setActive, signOut, openUserProfile } = clerk;
2627
const card = useCardState();
2728
const { signedInSessions, otherSessions } = useMultipleSessions({ user: opts.user });
2829
const { navigate } = useRouter();
@@ -101,7 +102,7 @@ export const useMultisessionActions = (opts: UseMultisessionActionsParams) => {
101102
};
102103

103104
const handleAddAccountClicked = () => {
104-
windowNavigate(opts.signInUrl || window.location.href);
105+
clerkWindowNavigate(clerk, opts.signInUrl || window.location.href);
105106
return sleep(2000);
106107
};
107108

0 commit comments

Comments
 (0)