From 4e5e7f463c12893a21cb3b5f9317fc3f2945879b Mon Sep 17 00:00:00 2001 From: Bryce Kalow Date: Tue, 3 Dec 2024 14:56:51 -0600 Subject: [PATCH] feat(clerk-js): Add allowedRedirectProtocols (#4705) --- .changeset/blue-teachers-remember.md | 6 +++++ .../clerk-js/src/core/__tests__/clerk.test.ts | 23 ++++++++++++++++--- packages/clerk-js/src/core/clerk.ts | 15 ++++++++++-- packages/clerk-js/src/utils/windowNavigate.ts | 18 +++++++-------- packages/types/src/clerk.ts | 6 ++++- 5 files changed, 53 insertions(+), 15 deletions(-) create mode 100644 .changeset/blue-teachers-remember.md diff --git a/.changeset/blue-teachers-remember.md b/.changeset/blue-teachers-remember.md new file mode 100644 index 0000000000..309183c3ff --- /dev/null +++ b/.changeset/blue-teachers-remember.md @@ -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. diff --git a/packages/clerk-js/src/core/__tests__/clerk.test.ts b/packages/clerk-js/src/core/__tests__/clerk.test.ts index 25d5f8978a..63df480e13 100644 --- a/packages/clerk-js/src/core/__tests__/clerk.test.ts +++ b/packages/clerk-js/src/core/__tests__/clerk.test.ts @@ -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', @@ -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 () => { @@ -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 () => { @@ -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()', () => { diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 41b3f9929d..a6b1ecf461 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -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.`, ); @@ -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; } @@ -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; + } } diff --git a/packages/clerk-js/src/utils/windowNavigate.ts b/packages/clerk-js/src/utils/windowNavigate.ts index 2ebf7de9d9..051fb9bfb3 100644 --- a/packages/clerk-js/src/utils/windowNavigate.ts +++ b/packages/clerk-js/src/utils/windowNavigate.ts @@ -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:', @@ -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; } diff --git a/packages/types/src/clerk.ts b/packages/types/src/clerk.ts index 37fc952139..9265e8f63b 100644 --- a/packages/types/src/clerk.ts +++ b/packages/types/src/clerk.ts @@ -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; + /** + * 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; /** * This option defines that the application is a satellite application. */