From d6dd07aed29461c0a56acb15e9b5f58b7c4e7b90 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Thu, 2 Nov 2023 13:10:49 +0200 Subject: [PATCH 1/8] feat(clerk-js): Add default allowed redirect origins if no options is passed --- packages/clerk-js/src/core/clerk.test.ts | 25 ++++++++++++++++++++++++ packages/clerk-js/src/core/clerk.ts | 22 +++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index 29425358950..e77bc091dab 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -1,7 +1,9 @@ +import { parsePublishableKey } from '@clerk/shared'; import type { ActiveSessionResource, SignInJSON, SignUpJSON, TokenResource } from '@clerk/types'; import { waitFor } from '@testing-library/dom'; import { mockNativeRuntime } from '../testUtils'; +import { getETLDPlusOneFromFrontendApi } from '../utils'; import { Clerk } from './clerk'; import { eventBus, events } from './events'; import type { AuthConfig, DisplayConfig, Organization } from './resources/internal'; @@ -402,6 +404,29 @@ describe('Clerk singleton', () => { expect(document.cookie).toContain(mockJwt); }); + + it('contains the default allowed origin values', async () => { + const sut = new Clerk(frontendApi); + await sut.load(); + + const frontendApiStr = parsePublishableKey(sut.publishableKey)?.frontendApi || sut.frontendApi; + + expect(sut.allowedRedirectOrigins).toEqual([ + window.location.origin, + `${window.location.origin}/*`, + `https://*.${getETLDPlusOneFromFrontendApi(frontendApiStr)}`, + `https://*.${getETLDPlusOneFromFrontendApi(frontendApiStr)}/*`, + ]); + }); + + it('contains only the allowedRedirectOrigins options given', async () => { + const sut = new Clerk(frontendApi); + await sut.load({ + allowedRedirectOrigins: ['https://test.host'], + }); + + expect(sut.allowedRedirectOrigins).toEqual(['https://test.host']); + }); }); describe('.signOut()', () => { diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index f22d044b15c..1d6e3c984c6 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -65,6 +65,7 @@ import { createPageLifecycle, errorThrower, getClerkQueryParam, + getETLDPlusOneFromFrontendApi, hasExternalAccountSignUpError, ignoreEventValue, inActiveBrowserTab, @@ -255,6 +256,25 @@ export class Clerk implements ClerkInterface { public isReady = (): boolean => this.#isReady; + get allowedRedirectOrigins(): (string | RegExp)[] | undefined { + if (!this.#options.allowedRedirectOrigins) { + const origins = []; + if (inBrowser()) { + origins.push(window.location.origin); + origins.push(window.location.origin + '/*'); + } + + const frontendApi = parsePublishableKey(this.publishableKey)?.frontendApi || this.frontendApi; + + origins.push(`https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}`); + origins.push(`https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}/*`); + + return origins; + } + + return this.#options.allowedRedirectOrigins; + } + public load = async (options?: ClerkOptions): Promise => { if (this.#isReady) { return; @@ -265,6 +285,8 @@ export class Clerk implements ClerkInterface { ...options, }; + this.#options.allowedRedirectOrigins = this.allowedRedirectOrigins; + if (this.#options.standardBrowser) { this.#isReady = await this.#loadInStandardBrowser(); } else { From f06ca61ad44e206c78a307c12ef9c1d5b17f189e Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Tue, 14 Nov 2023 13:17:14 +0200 Subject: [PATCH 2/8] fix(clerk-js): Use the frontendApi getter --- packages/clerk-js/src/core/clerk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 1d6e3c984c6..81e1684715b 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -264,7 +264,7 @@ export class Clerk implements ClerkInterface { origins.push(window.location.origin + '/*'); } - const frontendApi = parsePublishableKey(this.publishableKey)?.frontendApi || this.frontendApi; + const frontendApi = this.frontendApi; origins.push(`https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}`); origins.push(`https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}/*`); From 0bb3ba3d1d8db4f67aa0c27ac530ce35f9e5d0b2 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Tue, 14 Nov 2023 13:49:03 +0200 Subject: [PATCH 3/8] test(clerk-js): Update tests for allowedRedirectOrigins to use the publishableKey --- packages/clerk-js/src/core/clerk.test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index e77bc091dab..33e6ef7281f 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -1,4 +1,3 @@ -import { parsePublishableKey } from '@clerk/shared'; import type { ActiveSessionResource, SignInJSON, SignUpJSON, TokenResource } from '@clerk/types'; import { waitFor } from '@testing-library/dom'; @@ -406,10 +405,10 @@ describe('Clerk singleton', () => { }); it('contains the default allowed origin values', async () => { - const sut = new Clerk(frontendApi); + const sut = new Clerk(productionPublishableKey); await sut.load(); - const frontendApiStr = parsePublishableKey(sut.publishableKey)?.frontendApi || sut.frontendApi; + const frontendApiStr = sut.frontendApi; expect(sut.allowedRedirectOrigins).toEqual([ window.location.origin, @@ -420,7 +419,7 @@ describe('Clerk singleton', () => { }); it('contains only the allowedRedirectOrigins options given', async () => { - const sut = new Clerk(frontendApi); + const sut = new Clerk(productionPublishableKey); await sut.load({ allowedRedirectOrigins: ['https://test.host'], }); From bc602150036cddf151800783d5da06fbe33c8ea8 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Tue, 14 Nov 2023 14:05:02 +0200 Subject: [PATCH 4/8] chore(repo): Adds Changeset --- .changeset/fast-ads-mix.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .changeset/fast-ads-mix.md diff --git a/.changeset/fast-ads-mix.md b/.changeset/fast-ads-mix.md new file mode 100644 index 00000000000..3a398de72e0 --- /dev/null +++ b/.changeset/fast-ads-mix.md @@ -0,0 +1,11 @@ +--- +'@clerk/clerk-js': minor +--- + +Introducing default values for allowed redirect origins, this change will apply default values for the `allowedRedirectOrigins option`, if there is no value provided the defaults will be similar to the example below. + +Let's assume the host of the application is `test.host`, the origins will be +- `https://test.host/` +- `https://test.host/*` +- `https://*.yourawesomeapp.clerk.accounts.dev/` +- `https://*.yourawesomeapp.clerk.accounts.dev/*` From bf3fabd8915db7597c62272116f3875920b50414 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Thu, 16 Nov 2023 16:28:46 +0200 Subject: [PATCH 5/8] refactor(clerk-js): Extracted createAllowedRedirectOrigins origin getter to a utility function --- .changeset/fast-ads-mix.md | 3 +- packages/clerk-js/src/core/clerk.test.ts | 23 -------------- packages/clerk-js/src/core/clerk.ts | 26 +++------------- .../clerk-js/src/utils/__tests__/url.test.ts | 31 +++++++++++++++++++ packages/clerk-js/src/utils/url.ts | 19 ++++++++++++ 5 files changed, 56 insertions(+), 46 deletions(-) diff --git a/.changeset/fast-ads-mix.md b/.changeset/fast-ads-mix.md index 3a398de72e0..a7a4522bec9 100644 --- a/.changeset/fast-ads-mix.md +++ b/.changeset/fast-ads-mix.md @@ -6,6 +6,5 @@ Introducing default values for allowed redirect origins, this change will apply Let's assume the host of the application is `test.host`, the origins will be - `https://test.host/` -- `https://test.host/*` +- `https://yourawesomeapp.clerk.accounts.dev/` - `https://*.yourawesomeapp.clerk.accounts.dev/` -- `https://*.yourawesomeapp.clerk.accounts.dev/*` diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index 33e6ef7281f..6eaed0c8f87 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -403,29 +403,6 @@ describe('Clerk singleton', () => { expect(document.cookie).toContain(mockJwt); }); - - it('contains the default allowed origin values', async () => { - const sut = new Clerk(productionPublishableKey); - await sut.load(); - - const frontendApiStr = sut.frontendApi; - - expect(sut.allowedRedirectOrigins).toEqual([ - window.location.origin, - `${window.location.origin}/*`, - `https://*.${getETLDPlusOneFromFrontendApi(frontendApiStr)}`, - `https://*.${getETLDPlusOneFromFrontendApi(frontendApiStr)}/*`, - ]); - }); - - it('contains only the allowedRedirectOrigins options given', async () => { - const sut = new Clerk(productionPublishableKey); - await sut.load({ - allowedRedirectOrigins: ['https://test.host'], - }); - - expect(sut.allowedRedirectOrigins).toEqual(['https://test.host']); - }); }); describe('.signOut()', () => { diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 81e1684715b..3c25cf2a243 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -60,12 +60,12 @@ import { appendAsQueryParams, buildURL, completeSignUpFlow, + createAllowedRedirectOrigins, createBeforeUnloadTracker, createCookieHandler, createPageLifecycle, errorThrower, getClerkQueryParam, - getETLDPlusOneFromFrontendApi, hasExternalAccountSignUpError, ignoreEventValue, inActiveBrowserTab, @@ -256,25 +256,6 @@ export class Clerk implements ClerkInterface { public isReady = (): boolean => this.#isReady; - get allowedRedirectOrigins(): (string | RegExp)[] | undefined { - if (!this.#options.allowedRedirectOrigins) { - const origins = []; - if (inBrowser()) { - origins.push(window.location.origin); - origins.push(window.location.origin + '/*'); - } - - const frontendApi = this.frontendApi; - - origins.push(`https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}`); - origins.push(`https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}/*`); - - return origins; - } - - return this.#options.allowedRedirectOrigins; - } - public load = async (options?: ClerkOptions): Promise => { if (this.#isReady) { return; @@ -285,7 +266,10 @@ export class Clerk implements ClerkInterface { ...options, }; - this.#options.allowedRedirectOrigins = this.allowedRedirectOrigins; + this.#options.allowedRedirectOrigins = createAllowedRedirectOrigins( + this.#options.allowedRedirectOrigins, + this.frontendApi, + ); if (this.#options.standardBrowser) { this.#isReady = await this.#loadInStandardBrowser(); diff --git a/packages/clerk-js/src/utils/__tests__/url.test.ts b/packages/clerk-js/src/utils/__tests__/url.test.ts index 29fec96f5fe..40f6283dd43 100644 --- a/packages/clerk-js/src/utils/__tests__/url.test.ts +++ b/packages/clerk-js/src/utils/__tests__/url.test.ts @@ -3,6 +3,7 @@ import type { SignUpResource } from '@clerk/types'; import { appendAsQueryParams, buildURL, + createAllowedRedirectOrigins, getAllETLDs, getETLDPlusOneFromFrontendApi, getSearchParameterFromHash, @@ -460,3 +461,33 @@ describe('isAllowedRedirectOrigin', () => { expect(warnMock).toHaveBeenCalledTimes(Number(!expected)); // Number(boolean) evaluates to 0 or 1 }); }); + +describe('createAllowedRedirectOrigins', () => { + it('contains the default allowed origin values if no value is provided', async () => { + const frontendApi = 'https://somename.clerk.accounts.dev'; + const allowedRedirectOriginsValuesUndefined = createAllowedRedirectOrigins(undefined, frontendApi); + const allowedRedirectOriginsValuesEmptyArray = createAllowedRedirectOrigins([], frontendApi); + + expect(allowedRedirectOriginsValuesUndefined).toEqual([ + 'http://localhost', + `https://${getETLDPlusOneFromFrontendApi(frontendApi)}`, + `https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}`, + ]); + + expect(allowedRedirectOriginsValuesEmptyArray).toEqual([ + 'http://localhost', + `https://${getETLDPlusOneFromFrontendApi(frontendApi)}`, + `https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}`, + ]); + }); + + it('contains only the allowedRedirectOrigins options given', async () => { + const frontendApi = 'https://somename.clerk.accounts.dev'; + const allowedRedirectOriginsValues = createAllowedRedirectOrigins( + ['https://test.host', 'https://*.test.host'], + frontendApi, + ); + + expect(allowedRedirectOriginsValues).toEqual(['https://test.host', 'https://*.test.host']); + }); +}); diff --git a/packages/clerk-js/src/utils/url.ts b/packages/clerk-js/src/utils/url.ts index 61b787d53c3..4e3bedc51df 100644 --- a/packages/clerk-js/src/utils/url.ts +++ b/packages/clerk-js/src/utils/url.ts @@ -350,3 +350,22 @@ export const isAllowedRedirectOrigin = (_url: string, allowedRedirectOrigins: Ar } return isAllowed; }; + +export function createAllowedRedirectOrigins( + allowedRedirectOrigins: Array | undefined, + frontendApi: string, +): (string | RegExp)[] | undefined { + if (!allowedRedirectOrigins || allowedRedirectOrigins.length === 0) { + const origins = []; + if (typeof window !== 'undefined' && !!window.location) { + origins.push(window.location.origin); + } + + origins.push(`https://${getETLDPlusOneFromFrontendApi(frontendApi)}`); + origins.push(`https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}`); + + return origins; + } + + return allowedRedirectOrigins; +} From 7dc3a711767eee6be6d011120ab72b3b7ed200c8 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Thu, 16 Nov 2023 23:48:39 +0200 Subject: [PATCH 6/8] refactor(clerk-js): Reafactored createAllowedRedirectOrigins to return early --- packages/clerk-js/src/utils/url.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/clerk-js/src/utils/url.ts b/packages/clerk-js/src/utils/url.ts index 4e3bedc51df..57f566edfe7 100644 --- a/packages/clerk-js/src/utils/url.ts +++ b/packages/clerk-js/src/utils/url.ts @@ -355,17 +355,17 @@ export function createAllowedRedirectOrigins( allowedRedirectOrigins: Array | undefined, frontendApi: string, ): (string | RegExp)[] | undefined { - if (!allowedRedirectOrigins || allowedRedirectOrigins.length === 0) { - const origins = []; - if (typeof window !== 'undefined' && !!window.location) { - origins.push(window.location.origin); - } - - origins.push(`https://${getETLDPlusOneFromFrontendApi(frontendApi)}`); - origins.push(`https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}`); + if (Array.isArray(allowedRedirectOrigins) && !!allowedRedirectOrigins.length) { + return allowedRedirectOrigins; + } - return origins; + const origins = []; + if (typeof window !== 'undefined' && !!window.location) { + origins.push(window.location.origin); } - return allowedRedirectOrigins; + origins.push(`https://${getETLDPlusOneFromFrontendApi(frontendApi)}`); + origins.push(`https://*.${getETLDPlusOneFromFrontendApi(frontendApi)}`); + + return origins; } From a0a7b81caab46ab86b57cf91fc57ca52fb443254 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Thu, 16 Nov 2023 23:49:20 +0200 Subject: [PATCH 7/8] chore(repo): Update Changeset --- .changeset/fast-ads-mix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/fast-ads-mix.md b/.changeset/fast-ads-mix.md index a7a4522bec9..3fee680f3ef 100644 --- a/.changeset/fast-ads-mix.md +++ b/.changeset/fast-ads-mix.md @@ -2,7 +2,7 @@ '@clerk/clerk-js': minor --- -Introducing default values for allowed redirect origins, this change will apply default values for the `allowedRedirectOrigins option`, if there is no value provided the defaults will be similar to the example below. +Introducing default values for `allowedRedirectOrigins`. If no value is provided, default values similar to the example below will apply. Let's assume the host of the application is `test.host`, the origins will be - `https://test.host/` From 419ead7c74d56ec5dec522f285a305dd3204bcee Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Fri, 17 Nov 2023 12:16:53 +0200 Subject: [PATCH 8/8] chore(clerk-js): Remove unused import from tests --- packages/clerk-js/src/core/clerk.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index 6eaed0c8f87..29425358950 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -2,7 +2,6 @@ import type { ActiveSessionResource, SignInJSON, SignUpJSON, TokenResource } fro import { waitFor } from '@testing-library/dom'; import { mockNativeRuntime } from '../testUtils'; -import { getETLDPlusOneFromFrontendApi } from '../utils'; import { Clerk } from './clerk'; import { eventBus, events } from './events'; import type { AuthConfig, DisplayConfig, Organization } from './resources/internal';