Skip to content

Commit

Permalink
feat(clerk-js): Add allowedRedirectProtocols (#4705)
Browse files Browse the repository at this point in the history
  • Loading branch information
BRKalow authored Dec 3, 2024
1 parent 1c51045 commit 4e5e7f4
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 15 deletions.
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 @@ -960,7 +960,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 @@ -974,7 +974,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 @@ -2111,4 +2112,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.
*/
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 @@ -711,9 +711,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

0 comments on commit 4e5e7f4

Please sign in to comment.