Skip to content
Merged
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
6 changes: 6 additions & 0 deletions .changeset/blue-teachers-remember.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': minor
'@clerk/types': minor
---

Introduce a new `allowedRedirectProtocols` option to pass additional allowed protocols for user-provided redirect validation.
23 changes: 20 additions & 3 deletions packages/clerk-js/src/core/__tests__/clerk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('Clerk singleton', () => {
const productionPublishableKey = 'pk_live_Y2xlcmsuYWJjZWYuMTIzNDUucHJvZC5sY2xjbGVyay5jb20k';

const mockNavigate = jest.fn((to: string) => Promise.resolve(to));
const mockedLoadOptions = { routerPush: mockNavigate, routerReplace: mockNavigate };
const mockedLoadOptions = { routerDebug: true, routerPush: mockNavigate, routerReplace: mockNavigate };

const mockDisplayConfig = {
signInUrl: 'http://test.host/sign-in',
Expand Down Expand Up @@ -686,7 +686,6 @@ describe('Clerk singleton', () => {
const toUrl = 'https://www.origindifferent.com/';
await sut.navigate(toUrl);
expect(mockHref).toHaveBeenCalledWith(toUrl);
expect(logSpy).not.toHaveBeenCalled();
});

it('wraps custom navigate method in a promise if provided and it sync', async () => {
Expand All @@ -696,7 +695,6 @@ describe('Clerk singleton', () => {
expect(res.then).toBeDefined();
expect(mockHref).not.toHaveBeenCalled();
expect(mockNavigate.mock.calls[0][0]).toBe('/path#hash');
expect(logSpy).not.toHaveBeenCalled();
});

it('logs navigation external navigation when routerDebug is enabled', async () => {
Expand All @@ -720,6 +718,25 @@ describe('Clerk singleton', () => {
expect(logSpy).toHaveBeenCalledTimes(1);
expect(logSpy).toHaveBeenCalledWith(`Clerk is navigating to: ${toUrl}`);
});

it('validates the protocol of the provided URL', async () => {
await sut.load({ ...mockedLoadOptions, allowedRedirectProtocols: ['gg:'] });
// allowed protocol
const toUrl = 'gg://some/deeply/nested/path';
await sut.navigate(toUrl);
expect(mockNavigate.mock.calls[0][0]).toBe(toUrl);
expect(logSpy).toHaveBeenCalledTimes(1);
expect(logSpy).toHaveBeenCalledWith(`Clerk is navigating to: ${toUrl}`);

mockNavigate.mockReset();
logSpy.mockReset();

// disallowed protocol
const badUrl = 'evil://some/deeply/nested/path';
await sut.navigate(badUrl);
expect(mockNavigate.mock.calls[0][0]).toBe('/');
expect(logSpy).toHaveBeenCalledTimes(1);
});
});

describe('.handleRedirectCallback()', () => {
Expand Down
15 changes: 13 additions & 2 deletions packages/clerk-js/src/core/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ export class Clerk implements ClerkInterface {

let toURL = new URL(to, window.location.href);

if (!ALLOWED_PROTOCOLS.includes(toURL.protocol)) {
if (!this.#allowedRedirectProtocols.includes(toURL.protocol)) {
console.warn(
`Clerk: "${toURL.protocol}" is not a valid protocol. Redirecting to "/" instead. If you think this is a mistake, please open an issue.`,
);
Expand All @@ -972,7 +972,8 @@ export class Clerk implements ClerkInterface {
console.log(`Clerk is navigating to: ${toURL}`);
}

if (toURL.origin !== window.location.origin || !customNavigate) {
// 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.
if ((toURL.origin !== 'null' && toURL.origin !== window.location.origin) || !customNavigate) {
windowNavigate(toURL);
return;
}
Expand Down Expand Up @@ -2093,4 +2094,14 @@ export class Clerk implements ClerkInterface {
// ignore
}
};

get #allowedRedirectProtocols() {
let allowedProtocols = ALLOWED_PROTOCOLS;

if (this.#options.allowedRedirectProtocols) {
allowedProtocols = allowedProtocols.concat(this.#options.allowedRedirectProtocols);
}

return allowedProtocols;
}
}
18 changes: 9 additions & 9 deletions packages/clerk-js/src/utils/windowNavigate.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
export const CLERK_BEFORE_UNLOAD_EVENT = 'clerk:beforeunload';

/**
* Additional protocols can be provided using the `allowedRedirectProtocols` Clerk option.
*/
export const ALLOWED_PROTOCOLS = [
'http:',
'https:',
Expand All @@ -8,16 +11,13 @@ export const ALLOWED_PROTOCOLS = [
'chrome-extension:',
];

/**
* Helper utility to navigate via window.location.href. Also dispatches a clerk:beforeunload custom event.
*
* 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.
*/
Comment on lines +17 to +18
Copy link
Member

Choose a reason for hiding this comment

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

Should we replace the usage for windowNavigate from SignIn.ts and SignUp.ts with Clerk.navigate() ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we need to, as the url is provided from our API and it will always be external.

export function windowNavigate(to: URL | string): void {
let toURL = new URL(to, window.location.href);

if (!ALLOWED_PROTOCOLS.includes(toURL.protocol)) {
console.warn(
`Clerk: "${toURL.protocol}" is not a valid protocol. Redirecting to "/" instead. If you think this is a mistake, please open an issue.`,
);
toURL = new URL('/', window.location.href);
}

const toURL = new URL(to, window.location.href);
window.dispatchEvent(new CustomEvent(CLERK_BEFORE_UNLOAD_EVENT));
window.location.href = toURL.href;
}
6 changes: 5 additions & 1 deletion packages/types/src/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -706,9 +706,13 @@ export type ClerkOptions = ClerkOptionsNavigation &
/** This URL will be used for any redirects that might happen and needs to point to your primary application on the client-side. This option is optional for production instances and required for development instances. */
signUpUrl?: string;
/**
* Optional array of domains used to validate against the query param of an auth redirect. If no match is made, the redirect is considered unsafe and the default redirect will be used with a warning passed to the console.
* An optional array of domains to validate user-provided redirect URLs against. If no match is made, the redirect is considered unsafe and the default redirect will be used with a warning logged in the console.
*/
allowedRedirectOrigins?: Array<string | RegExp>;
/**
* An optional array of protocols to validate user-provided redirect URLs against. If no match is made, the redirect is considered unsafe and the default redirect will be used with a warning logged in the console.
*/
allowedRedirectProtocols?: Array<string>;
/**
* This option defines that the application is a satellite application.
*/
Expand Down
Loading