From eeb2b4a680998791b573541214eca0c5f52e4163 Mon Sep 17 00:00:00 2001 From: Tom Milewski Date: Thu, 14 Dec 2023 19:43:29 -0500 Subject: [PATCH] refactor(clerk-js,nextjs,clerk-react,shared,types): Remove hashing and third-party cookie based session syncing for development instances. --- .changeset/famous-penguins-bow.md | 7 + .changeset/modern-buses-sort.md | 10 ++ .changeset/modern-mayflies-sort.md | 6 + .../clerk-js/src/core/clerk.redirects.test.ts | 135 +++++++--------- packages/clerk-js/src/core/clerk.test.ts | 148 +++++------------- packages/clerk-js/src/core/clerk.ts | 15 +- packages/clerk-js/src/core/devBrowser.ts | 2 +- .../clerk-js/src/core/resources/AuthConfig.ts | 2 - packages/nextjs/src/server/authMiddleware.ts | 6 +- packages/react/src/isomorphicClerk.ts | 7 +- .../shared/src/__tests__/devbrowser.test.ts | 44 +++--- packages/shared/src/devBrowser.ts | 35 +---- packages/types/src/authConfig.ts | 6 - packages/types/src/clerk.ts | 10 +- 14 files changed, 161 insertions(+), 272 deletions(-) create mode 100644 .changeset/famous-penguins-bow.md create mode 100644 .changeset/modern-buses-sort.md create mode 100644 .changeset/modern-mayflies-sort.md diff --git a/.changeset/famous-penguins-bow.md b/.changeset/famous-penguins-bow.md new file mode 100644 index 00000000000..d5a294ee3b5 --- /dev/null +++ b/.changeset/famous-penguins-bow.md @@ -0,0 +1,7 @@ +--- +'@clerk/types': major +--- + +- Remove `BuildUrlWithAuthParams` type +- `AuthConfigResource` no longer has a `urlBasedSessionSyncing` property +- `buildUrlWithAuth` no longer accepts an `options` argument of `BuildUrlWithAuthParams`. diff --git a/.changeset/modern-buses-sort.md b/.changeset/modern-buses-sort.md new file mode 100644 index 00000000000..2fb6eb6a655 --- /dev/null +++ b/.changeset/modern-buses-sort.md @@ -0,0 +1,10 @@ +--- +'@clerk/chrome-extension': major +'@clerk/clerk-js': major +'@clerk/nextjs': major +'@clerk/shared': major +'@clerk/clerk-react': major +'@clerk/types': major +--- + +Remove hashing and third-party cookie functionality related to development instance session syncing in favor of URL-based session syncing with query parameters. diff --git a/.changeset/modern-mayflies-sort.md b/.changeset/modern-mayflies-sort.md new file mode 100644 index 00000000000..814b36f9a8e --- /dev/null +++ b/.changeset/modern-mayflies-sort.md @@ -0,0 +1,6 @@ +--- +'@clerk/clerk-js': major +'@clerk/clerk-react': major +--- + +- `buildUrlWithAuth` no longer accepts an `options` argument. diff --git a/packages/clerk-js/src/core/clerk.redirects.test.ts b/packages/clerk-js/src/core/clerk.redirects.test.ts index 33b757ca064..c79f9be886f 100644 --- a/packages/clerk-js/src/core/clerk.redirects.test.ts +++ b/packages/clerk-js/src/core/clerk.redirects.test.ts @@ -1,17 +1,17 @@ import { Clerk } from './clerk'; -import type { AuthConfig, DisplayConfig } from './resources/internal'; +import type { DevBrowser } from './devBrowser'; +import type { DisplayConfig } from './resources/internal'; import { Client, Environment } from './resources/internal'; const mockClientFetch = jest.fn(); const mockEnvironmentFetch = jest.fn(); -const mockUsesUrlBasedSessionSync = jest.fn(); jest.mock('./resources/Client'); jest.mock('./resources/Environment'); // Because Jest, don't ask me why... jest.mock('./devBrowser', () => ({ - createDevBrowser: () => ({ + createDevBrowser: (): DevBrowser => ({ clear: jest.fn(), setup: jest.fn(), getDevBrowserJWT: jest.fn(() => 'deadbeef'), @@ -57,7 +57,9 @@ const developmentPublishableKey = 'pk_test_Y2xlcmsuYWJjZWYuMTIzNDUuZGV2LmxjbGNsZ const productionPublishableKey = 'pk_live_Y2xlcmsuYWJjZWYuMTIzNDUucHJvZC5sY2xjbGVyay5jb20k'; describe('Clerk singleton - Redirects', () => { - let mockNavigate = jest.fn(); + const mockNavigate = jest.fn((to: string) => Promise.resolve(to)); + const mockedLoadOptions = { routerPush: mockNavigate, routerReplace: mockNavigate }; + let mockWindowLocation; let mockHref: jest.Mock; @@ -90,10 +92,10 @@ describe('Clerk singleton - Redirects', () => { activeSessions: [], }), ); - - mockNavigate = jest.fn((to: string) => Promise.resolve(to)); }); + afterEach(() => mockNavigate.mockReset()); + describe('.redirectTo(SignUp|SignIn|UserProfile|AfterSignIn|AfterSignUp|CreateOrganization|OrganizationProfile)', () => { let clerkForProductionInstance: Clerk; let clerkForDevelopmentInstance: Clerk; @@ -102,7 +104,6 @@ describe('Clerk singleton - Redirects', () => { beforeEach(async () => { mockEnvironmentFetch.mockReturnValue( Promise.resolve({ - authConfig: { urlBasedSessionSyncing: true } as AuthConfig, userSettings: mockUserSettings, displayConfig: mockDisplayConfigWithSameOrigin, isProduction: () => false, @@ -110,17 +111,11 @@ describe('Clerk singleton - Redirects', () => { }), ); - mockUsesUrlBasedSessionSync.mockReturnValue(true); - clerkForProductionInstance = new Clerk(productionPublishableKey); - await clerkForProductionInstance.load({ - routerPush: mockNavigate, - }); - clerkForDevelopmentInstance = new Clerk(developmentPublishableKey); - await clerkForDevelopmentInstance.load({ - routerPush: mockNavigate, - }); + + await clerkForProductionInstance.load(mockedLoadOptions); + await clerkForDevelopmentInstance.load(mockedLoadOptions); }); afterEach(() => { @@ -129,57 +124,57 @@ describe('Clerk singleton - Redirects', () => { it('redirects to signInUrl', async () => { await clerkForProductionInstance.redirectToSignIn({ redirectUrl: 'https://www.example.com/' }); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); - await clerkForDevelopmentInstance.redirectToSignIn({ redirectUrl: 'https://www.example.com/' }); + + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); expect(mockNavigate).toHaveBeenNthCalledWith(2, '/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); }); it('redirects to signUpUrl', async () => { await clerkForProductionInstance.redirectToSignUp({ redirectUrl: 'https://www.example.com/' }); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); - await clerkForDevelopmentInstance.redirectToSignUp({ redirectUrl: 'https://www.example.com/' }); + + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); expect(mockNavigate).toHaveBeenNthCalledWith(2, '/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); }); it('redirects to userProfileUrl', async () => { await clerkForProductionInstance.redirectToUserProfile(); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/user-profile'); - await clerkForDevelopmentInstance.redirectToUserProfile(); + + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/user-profile'); expect(mockNavigate).toHaveBeenNthCalledWith(2, '/user-profile'); }); it('redirects to afterSignUp', async () => { await clerkForProductionInstance.redirectToAfterSignUp(); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/'); - await clerkForDevelopmentInstance.redirectToAfterSignUp(); + + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/'); expect(mockNavigate).toHaveBeenNthCalledWith(2, '/'); }); it('redirects to afterSignIn', async () => { await clerkForProductionInstance.redirectToAfterSignIn(); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/'); - await clerkForDevelopmentInstance.redirectToAfterSignIn(); + + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/'); expect(mockNavigate).toHaveBeenNthCalledWith(2, '/'); }); it('redirects to create organization', async () => { await clerkForProductionInstance.redirectToCreateOrganization(); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/create-organization'); - await clerkForDevelopmentInstance.redirectToCreateOrganization(); + + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/create-organization'); expect(mockNavigate).toHaveBeenNthCalledWith(2, '/create-organization'); }); it('redirects to organization profile', async () => { await clerkForProductionInstance.redirectToOrganizationProfile(); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/organization-profile'); - await clerkForDevelopmentInstance.redirectToOrganizationProfile(); + + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/organization-profile'); expect(mockNavigate).toHaveBeenNthCalledWith(2, '/organization-profile'); }); }); @@ -188,7 +183,6 @@ describe('Clerk singleton - Redirects', () => { beforeEach(async () => { mockEnvironmentFetch.mockReturnValue( Promise.resolve({ - authConfig: { urlBasedSessionSyncing: true } as AuthConfig, userSettings: mockUserSettings, displayConfig: mockDisplayConfigWithDifferentOrigin, isProduction: () => false, @@ -196,79 +190,64 @@ describe('Clerk singleton - Redirects', () => { }), ); - mockUsesUrlBasedSessionSync.mockReturnValue(true); - clerkForProductionInstance = new Clerk(productionPublishableKey); - await clerkForProductionInstance.load({ - routerPush: mockNavigate, - }); - clerkForDevelopmentInstance = new Clerk(developmentPublishableKey); - await clerkForDevelopmentInstance.load({ - routerPush: mockNavigate, - }); + + await clerkForProductionInstance.load(mockedLoadOptions); + await clerkForDevelopmentInstance.load(mockedLoadOptions); }); afterEach(() => { mockEnvironmentFetch.mockRestore(); }); + const host = 'http://another-test.host'; + it('redirects to signInUrl', async () => { await clerkForProductionInstance.redirectToSignIn({ redirectUrl: 'https://www.example.com/' }); - expect(mockHref).toHaveBeenNthCalledWith( - 1, - 'http://another-test.host/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F', - ); - await clerkForDevelopmentInstance.redirectToSignIn({ redirectUrl: 'https://www.example.com/' }); + + expect(mockHref).toHaveBeenNthCalledWith(1, `${host}/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F`); + expect(mockHref).toHaveBeenNthCalledWith( 2, - 'http://another-test.host/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F__clerk_db_jwt[deadbeef]', + `${host}/sign-in?__clerk_db_jwt=deadbeef#/?redirect_url=https%3A%2F%2Fwww.example.com%2F`, ); }); it('redirects to signUpUrl', async () => { await clerkForProductionInstance.redirectToSignUp({ redirectUrl: 'https://www.example.com/' }); - expect(mockHref).toHaveBeenNthCalledWith( - 1, - 'http://another-test.host/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F', - ); - await clerkForDevelopmentInstance.redirectToSignUp({ redirectUrl: 'https://www.example.com/' }); + + expect(mockHref).toHaveBeenNthCalledWith(1, `${host}/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F`); expect(mockHref).toHaveBeenNthCalledWith( 2, - 'http://another-test.host/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F__clerk_db_jwt[deadbeef]', + `${host}/sign-up?__clerk_db_jwt=deadbeef#/?redirect_url=https%3A%2F%2Fwww.example.com%2F`, ); }); it('redirects to userProfileUrl', async () => { await clerkForProductionInstance.redirectToUserProfile(); - expect(mockHref).toHaveBeenNthCalledWith(1, 'http://another-test.host/user-profile'); - await clerkForDevelopmentInstance.redirectToUserProfile(); - expect(mockHref).toHaveBeenNthCalledWith(2, 'http://another-test.host/user-profile#__clerk_db_jwt[deadbeef]'); + + expect(mockHref).toHaveBeenNthCalledWith(1, `${host}/user-profile`); + expect(mockHref).toHaveBeenNthCalledWith(2, `${host}/user-profile?__clerk_db_jwt=deadbeef`); }); it('redirects to create organization', async () => { await clerkForProductionInstance.redirectToCreateOrganization(); - expect(mockHref).toHaveBeenNthCalledWith(1, 'http://another-test.host/create-organization'); - await clerkForDevelopmentInstance.redirectToCreateOrganization(); - expect(mockHref).toHaveBeenNthCalledWith( - 2, - 'http://another-test.host/create-organization#__clerk_db_jwt[deadbeef]', - ); + + expect(mockHref).toHaveBeenNthCalledWith(1, `${host}/create-organization`); + expect(mockHref).toHaveBeenNthCalledWith(2, `${host}/create-organization?__clerk_db_jwt=deadbeef`); }); it('redirects to organization profile', async () => { await clerkForProductionInstance.redirectToOrganizationProfile(); - expect(mockHref).toHaveBeenNthCalledWith(1, 'http://another-test.host/organization-profile'); - await clerkForDevelopmentInstance.redirectToOrganizationProfile(); - expect(mockHref).toHaveBeenNthCalledWith( - 2, - 'http://another-test.host/organization-profile#__clerk_db_jwt[deadbeef]', - ); + + expect(mockHref).toHaveBeenNthCalledWith(1, `${host}/organization-profile`); + expect(mockHref).toHaveBeenNthCalledWith(2, `${host}/organization-profile?__clerk_db_jwt=deadbeef`); }); }); }); @@ -278,10 +257,8 @@ describe('Clerk singleton - Redirects', () => { let clerkForDevelopmentInstance: Clerk; beforeEach(async () => { - mockUsesUrlBasedSessionSync.mockReturnValue(true); mockEnvironmentFetch.mockReturnValue( Promise.resolve({ - authConfig: { urlBasedSessionSyncing: true } as AuthConfig, userSettings: mockUserSettings, displayConfig: mockDisplayConfigWithDifferentOrigin, isProduction: () => false, @@ -290,22 +267,20 @@ describe('Clerk singleton - Redirects', () => { ); clerkForProductionInstance = new Clerk(productionPublishableKey); - await clerkForProductionInstance.load({ - routerPush: mockNavigate, - }); - clerkForDevelopmentInstance = new Clerk(developmentPublishableKey); - await clerkForDevelopmentInstance.load({ - routerPush: mockNavigate, - }); + + await clerkForProductionInstance.load(mockedLoadOptions); + await clerkForDevelopmentInstance.load(mockedLoadOptions); }); + const host = 'https://app.example.com'; + it('redirects to the provided url with __clerk_db_jwt in the url', async () => { - await clerkForProductionInstance.redirectWithAuth('https://app.example.com'); - expect(mockHref).toHaveBeenNthCalledWith(1, 'https://app.example.com/'); + await clerkForProductionInstance.redirectWithAuth(host); + await clerkForDevelopmentInstance.redirectWithAuth(host); - await clerkForDevelopmentInstance.redirectWithAuth('https://app.example.com'); - expect(mockHref).toHaveBeenNthCalledWith(2, 'https://app.example.com/#__clerk_db_jwt[deadbeef]'); + expect(mockHref).toHaveBeenNthCalledWith(1, `${host}/`); + expect(mockHref).toHaveBeenNthCalledWith(2, `${host}/?__clerk_db_jwt=deadbeef`); }); }); }); diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index 7bdc6f08420..5f490e3537f 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -3,22 +3,22 @@ import { waitFor } from '@testing-library/dom'; import { mockNativeRuntime } from '../testUtils'; import { Clerk } from './clerk'; +import type { DevBrowser } from './devBrowser'; import { eventBus, events } from './events'; -import type { AuthConfig, DisplayConfig, Organization } from './resources/internal'; +import type { DisplayConfig, Organization } from './resources/internal'; import { BaseResource, Client, EmailLinkErrorCode, Environment, SignIn, SignUp } from './resources/internal'; import { SessionCookieService } from './services'; import { mockJwt } from './test/fixtures'; const mockClientFetch = jest.fn(); const mockEnvironmentFetch = jest.fn(); -const mockUsesUrlBasedSessionSync = jest.fn(); jest.mock('./resources/Client'); jest.mock('./resources/Environment'); // Because Jest, don't ask me why... jest.mock('./devBrowser', () => ({ - createDevBrowser: () => ({ + createDevBrowser: (): DevBrowser => ({ clear: jest.fn(), setup: jest.fn(), getDevBrowserJWT: jest.fn(() => 'deadbeef'), @@ -36,7 +36,7 @@ Environment.getInstance = jest.fn().mockImplementation(() => { const oldWindowLocation = window.location; const setWindowQueryParams = (params: Array<[string, string]>) => { - // @ts-ignore + // @ts-expect-error - Forcing delete on non-optional property for testing purposes delete window.location; const u = new URL(oldWindowLocation.href); params.forEach(([k, v]) => u.searchParams.append(k, v)); @@ -48,7 +48,8 @@ describe('Clerk singleton', () => { const developmentPublishableKey = 'pk_test_Y2xlcmsuYWJjZWYuMTIzNDUuZGV2LmxjbGNsZXJrLmNvbSQ'; const productionPublishableKey = 'pk_live_Y2xlcmsuYWJjZWYuMTIzNDUucHJvZC5sY2xjbGVyay5jb20k'; - let mockNavigate = jest.fn(); + const mockNavigate = jest.fn((to: string) => Promise.resolve(to)); + const mockedLoadOptions = { routerPush: mockNavigate, routerReplace: mockNavigate }; const mockDisplayConfig = { signInUrl: 'http://test.host/sign-in', @@ -59,10 +60,6 @@ describe('Clerk singleton', () => { organizationProfileUrl: 'http://test.host/organization-profile', } as DisplayConfig; - const mockAuthConfig = { - urlBasedSessionSyncing: true, - } as AuthConfig; - const mockUserSettings = { signUp: { captcha_enabled: false, @@ -115,7 +112,6 @@ describe('Clerk singleton', () => { mockEnvironmentFetch.mockReturnValue( Promise.resolve({ - authConfig: mockAuthConfig, userSettings: mockUserSettings, displayConfig: mockDisplayConfig, isSingleSession: () => false, @@ -130,10 +126,11 @@ describe('Clerk singleton', () => { }), ); - mockNavigate = jest.fn((to: string) => Promise.resolve(to)); eventBus.off(events.TokenUpdate); }); + afterEach(() => mockNavigate.mockReset()); + describe('initialize', () => { it('should consider publishableKey readonly', () => { const sut = new Clerk(productionPublishableKey); @@ -535,7 +532,7 @@ describe('Clerk singleton', () => { }); it('uses window location if a custom navigate is defined but destination has different origin', async () => { - await sut.load({ routerPush: mockNavigate }); + await sut.load(mockedLoadOptions); const toUrl = 'https://www.origindifferent.com/'; await sut.navigate(toUrl); expect(mockHref).toHaveBeenCalledWith(toUrl); @@ -543,7 +540,7 @@ describe('Clerk singleton', () => { }); it('wraps custom navigate method in a promise if provided and it sync', async () => { - await sut.load({ routerPush: mockNavigate }); + await sut.load(mockedLoadOptions); const toUrl = 'http://test.host/path#hash'; const res = sut.navigate(toUrl); expect(res.then).toBeDefined(); @@ -563,7 +560,7 @@ describe('Clerk singleton', () => { }); it('logs navigation custom navigation when routerDebug is enabled', async () => { - await sut.load({ routerPush: mockNavigate, routerDebug: true }); + await sut.load({ ...mockedLoadOptions, routerDebug: true }); const toUrl = 'http://test.host/path#hash'; const res = sut.navigate(toUrl); expect(res.then).toBeDefined(); @@ -625,9 +622,7 @@ describe('Clerk singleton', () => { .mockReturnValue(Promise.resolve({ status: 'complete', createdSessionId: '123' })); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); if (!sut.client) { fail('we should always have a client'); } @@ -690,9 +685,7 @@ describe('Clerk singleton', () => { ); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); if (!sut.client) { fail('we should always have a client'); } @@ -758,9 +751,7 @@ describe('Clerk singleton', () => { .mockReturnValue(Promise.resolve({ status: 'complete', createdSessionId: '123' })); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); if (!sut.client) { fail('we should always have a client'); } @@ -816,9 +807,7 @@ describe('Clerk singleton', () => { const mockSetActive = jest.fn(); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive; await sut.handleRedirectCallback(); @@ -868,9 +857,7 @@ describe('Clerk singleton', () => { .mockReturnValue(Promise.resolve({ status: 'complete', createdSessionId: '123' })); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); if (!sut.client) { fail('we should always have a client'); } @@ -916,9 +903,7 @@ describe('Clerk singleton', () => { ); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); await sut.handleRedirectCallback(); @@ -959,9 +944,7 @@ describe('Clerk singleton', () => { ); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.handleRedirectCallback({ secondFactorUrl: '/custom-2fa', @@ -1015,9 +998,7 @@ describe('Clerk singleton', () => { }); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive as any; sut.handleRedirectCallback({ @@ -1072,9 +1053,7 @@ describe('Clerk singleton', () => { }); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive as any; sut.handleRedirectCallback({ @@ -1126,9 +1105,7 @@ describe('Clerk singleton', () => { ); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); await sut.handleRedirectCallback(); @@ -1176,9 +1153,7 @@ describe('Clerk singleton', () => { ); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); await sut.handleRedirectCallback(); @@ -1220,9 +1195,7 @@ describe('Clerk singleton', () => { ); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); await sut.handleRedirectCallback(); @@ -1269,9 +1242,7 @@ describe('Clerk singleton', () => { ); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); await sut.handleRedirectCallback(); @@ -1303,9 +1274,7 @@ describe('Clerk singleton', () => { ); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); await sut.handleRedirectCallback(); @@ -1349,9 +1318,7 @@ describe('Clerk singleton', () => { const mockSignInCreate = jest.fn().mockReturnValue(Promise.resolve({ status: 'needs_first_factor' })); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); if (!sut.client) { fail('we should always have a client'); } @@ -1410,9 +1377,7 @@ describe('Clerk singleton', () => { ); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); if (!sut.client) { fail('we should always have a client'); } @@ -1463,9 +1428,7 @@ describe('Clerk singleton', () => { const mockSignInCreate = jest.fn().mockReturnValue(Promise.resolve({ status: 'needs_first_factor' })); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); if (!sut.client) { fail('we should always have a client'); } @@ -1503,9 +1466,7 @@ describe('Clerk singleton', () => { const mockSignInCreate = jest.fn().mockReturnValue(Promise.resolve({ status: 'needs_new_password' })); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); if (!sut.client) { fail('we should always have a client'); } @@ -1544,9 +1505,7 @@ describe('Clerk singleton', () => { const mockSetActive = jest.fn(); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive; const redirectUrlComplete = '/redirect-to'; @@ -1575,9 +1534,7 @@ describe('Clerk singleton', () => { const mockSetActive = jest.fn(); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive; const redirectUrl = '/2fa'; @@ -1608,9 +1565,7 @@ describe('Clerk singleton', () => { const mockSetActive = jest.fn(); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive; const redirectUrlComplete = '/redirect-to'; @@ -1639,9 +1594,7 @@ describe('Clerk singleton', () => { const mockSetActive = jest.fn(); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive; const redirectUrl = '/next-up'; @@ -1666,9 +1619,7 @@ describe('Clerk singleton', () => { const mockSetActive = jest.fn(); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive; await expect(async () => { @@ -1690,9 +1641,7 @@ describe('Clerk singleton', () => { const mockSetActive = jest.fn(); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive; await expect(async () => { @@ -1716,9 +1665,7 @@ describe('Clerk singleton', () => { ); const mockSetActive = jest.fn(); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive; const res = { ping: 'ping' }; const cb = () => { @@ -1741,9 +1688,7 @@ describe('Clerk singleton', () => { ); const mockSetActive = jest.fn(); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive; await expect(async () => { await sut.handleEmailLinkVerification({}); @@ -1768,9 +1713,7 @@ describe('Clerk singleton', () => { ); const mockSetActive = jest.fn(); const sut = new Clerk(productionPublishableKey); - await sut.load({ - routerPush: mockNavigate, - }); + await sut.load(mockedLoadOptions); sut.setActive = mockSetActive; await expect(async () => { @@ -1841,8 +1784,7 @@ describe('Clerk singleton', () => { }); describe('buildUrlWithAuth', () => { - it('builds an absolute url from a relative url in development for url based session syncing', async () => { - mockUsesUrlBasedSessionSync.mockReturnValue(true); + it('builds an absolute url from a relative url in development', async () => { const sut = new Clerk(developmentPublishableKey); await sut.load(); @@ -1859,25 +1801,22 @@ describe('Clerk singleton', () => { }); it('uses the hash to propagate the dev_browser JWT by default on dev', async () => { - mockUsesUrlBasedSessionSync.mockReturnValue(true); const sut = new Clerk(developmentPublishableKey); await sut.load(); const url = sut.buildUrlWithAuth('https://example.com/some-path'); - expect(url).toBe('https://example.com/some-path#__clerk_db_jwt[deadbeef]'); + expect(url).toBe('https://example.com/some-path?__clerk_db_jwt=deadbeef'); }); it('uses the query param to propagate the dev_browser JWT if specified by option on dev', async () => { - mockUsesUrlBasedSessionSync.mockReturnValue(true); const sut = new Clerk(developmentPublishableKey); await sut.load(); - const url = sut.buildUrlWithAuth('https://example.com/some-path', { useQueryParam: true }); + const url = sut.buildUrlWithAuth('https://example.com/some-path'); expect(url).toBe('https://example.com/some-path?__clerk_db_jwt=deadbeef'); }); it('uses the query param to propagate the dev_browser JWT to Account Portal pages on dev - non-kima', async () => { - mockUsesUrlBasedSessionSync.mockReturnValue(true); const sut = new Clerk(developmentPublishableKey); await sut.load(); @@ -1886,7 +1825,6 @@ describe('Clerk singleton', () => { }); it('uses the query param to propagate the dev_browser JWT to Account Portal pages on dev - kima', async () => { - mockUsesUrlBasedSessionSync.mockReturnValue(true); const sut = new Clerk(developmentPublishableKey); await sut.load(); @@ -1897,13 +1835,13 @@ describe('Clerk singleton', () => { describe('Organizations', () => { it('getOrganization', async () => { - // @ts-ignore + // @ts-expect-error - Mocking a protected method BaseResource._fetch = jest.fn().mockResolvedValue({}); const sut = new Clerk(developmentPublishableKey); await sut.getOrganization('some-org-id'); - // @ts-ignore + // @ts-expect-error - Mocking a protected method expect(BaseResource._fetch).toHaveBeenCalledWith({ method: 'GET', path: '/organizations/some-org-id', diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index c15842b0de9..d988a494bd8 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -17,7 +17,6 @@ import { eventComponentMounted, TelemetryCollector } from '@clerk/shared/telemet import type { ActiveSessionResource, AuthenticateWithMetamaskParams, - BuildUrlWithAuthParams, Clerk as ClerkInterface, ClerkAPIError, ClerkOptions, @@ -702,7 +701,7 @@ export class Clerk implements ClerkInterface { return await customNavigate(stripOrigin(toURL)); }; - public buildUrlWithAuth(to: string, options?: BuildUrlWithAuthParams): string { + public buildUrlWithAuth(to: string): string { if (this.#instanceType === 'production') { return to; } @@ -718,10 +717,7 @@ export class Clerk implements ClerkInterface { return clerkMissingDevBrowserJwt(); } - // Use query param for Account Portal pages so that SSR can access the dev_browser JWT - const asQueryParam = !!options?.useQueryParam || isDevAccountPortalOrigin(toURL.hostname); - - return setDevBrowserJWTInURL(toURL, devBrowserJwt, asQueryParam).href; + return setDevBrowserJWTInURL(toURL, devBrowserJwt).href; } public buildSignInUrl(options?: SignInRedirectOptions): string { @@ -1329,7 +1325,7 @@ export class Clerk implements ClerkInterface { * For multi-domain we want to avoid retrieving a fresh JWT from FAPI, and we need to get the token as a result of multi-domain session syncing. */ if (this.#instanceType === 'production') { - await this.#devBrowser.clear(); + this.#devBrowser.clear(); } else { await this.#devBrowser.setup(); } @@ -1386,7 +1382,7 @@ export class Clerk implements ClerkInterface { break; } catch (err) { if (isError(err, 'dev_browser_unauthenticated')) { - await this.#devBrowser.clear(); + this.#devBrowser.clear(); await this.#devBrowser.setup(); } else if (!isValidBrowserOnline()) { console.warn(err); @@ -1579,8 +1575,7 @@ export class Clerk implements ClerkInterface { return false; } - const buildUrlWithAuthParams: BuildUrlWithAuthParams = { useQueryParam: true }; - await this.navigate(this.buildUrlWithAuth(redirectUrl, buildUrlWithAuthParams)); + await this.navigate(this.buildUrlWithAuth(redirectUrl)); return true; }; } diff --git a/packages/clerk-js/src/core/devBrowser.ts b/packages/clerk-js/src/core/devBrowser.ts index 18a329fe959..5ccf7ac5cfe 100644 --- a/packages/clerk-js/src/core/devBrowser.ts +++ b/packages/clerk-js/src/core/devBrowser.ts @@ -48,7 +48,7 @@ export function createDevBrowser({ frontendApi, fapiClient }: CreateDevBrowserOp fapiClient.onBeforeRequest(request => { const devBrowserJWT = getDevBrowserJWT(); if (devBrowserJWT && request?.url) { - request.url = setDevBrowserJWTInURL(request.url, devBrowserJWT, true); + request.url = setDevBrowserJWTInURL(request.url, devBrowserJWT); } }); diff --git a/packages/clerk-js/src/core/resources/AuthConfig.ts b/packages/clerk-js/src/core/resources/AuthConfig.ts index 4a5287c16db..2d176e87d7c 100644 --- a/packages/clerk-js/src/core/resources/AuthConfig.ts +++ b/packages/clerk-js/src/core/resources/AuthConfig.ts @@ -4,7 +4,6 @@ import { BaseResource } from './internal'; export class AuthConfig extends BaseResource implements AuthConfigResource { singleSessionMode!: boolean; - urlBasedSessionSyncing!: boolean; public constructor(data: AuthConfigJSON) { super(); @@ -13,7 +12,6 @@ export class AuthConfig extends BaseResource implements AuthConfigResource { protected fromJSON(data: AuthConfigJSON | null): this { this.singleSessionMode = data ? data.single_session_mode : true; - this.urlBasedSessionSyncing = data ? data.url_based_session_syncing : false; return this; } } diff --git a/packages/nextjs/src/server/authMiddleware.ts b/packages/nextjs/src/server/authMiddleware.ts index ddabf813a4a..ff075b32406 100644 --- a/packages/nextjs/src/server/authMiddleware.ts +++ b/packages/nextjs/src/server/authMiddleware.ts @@ -15,7 +15,6 @@ import { PUBLISHABLE_KEY, SECRET_KEY } from './constants'; import { informAboutProtectedRouteInfo, receivedRequestForIgnoredRoute } from './errors'; import { redirectToSignIn } from './redirect'; import type { NextMiddlewareResult, WithAuthOptions } from './types'; -import { isDevAccountPortalOrigin } from './url'; import { apiEndpointUnauthorizedNextResponse, decorateRequest, @@ -327,10 +326,7 @@ const appendDevBrowserOnCrossOrigin = (req: WithClerkUrl, res: Resp // Next.js 12.1+ allows redirects only to absolute URLs const url = new URL(location); - // Use query param for Account Portal pages so that SSR can access the dev_browser JWT - const asQueryParam = isDevAccountPortalOrigin(url.hostname); - - const urlWithDevBrowser = setDevBrowserJWTInURL(url, dbJwt, asQueryParam); + const urlWithDevBrowser = setDevBrowserJWTInURL(url, dbJwt); return NextResponse.redirect(urlWithDevBrowser.href, res); } diff --git a/packages/react/src/isomorphicClerk.ts b/packages/react/src/isomorphicClerk.ts index c5cbb5ebdfb..f4b7df167e5 100644 --- a/packages/react/src/isomorphicClerk.ts +++ b/packages/react/src/isomorphicClerk.ts @@ -4,7 +4,6 @@ import type { TelemetryCollector } from '@clerk/shared/telemetry'; import type { ActiveSessionResource, AuthenticateWithMetamaskParams, - BuildUrlWithAuthParams, Clerk, ClientResource, CreateOrganizationParams, @@ -113,8 +112,6 @@ type IsomorphicLoadedClerk = Without< // TODO: Align return type buildOrganizationProfileUrl: () => string | void; // TODO: Align return type - buildUrlWithAuth: (to: string, opts?: BuildUrlWithAuthParams | undefined) => string | void; - // TODO: Align return type buildAfterSignInUrl: () => string | void; // TODO: Align return type buildAfterSignUpUrl: () => string | void; @@ -308,8 +305,8 @@ export class IsomorphicClerk implements IsomorphicLoadedClerk { } }; - buildUrlWithAuth = (to: string, opts?: BuildUrlWithAuthParams | undefined): string | void => { - const callback = () => this.clerkjs?.buildUrlWithAuth(to, opts) || ''; + buildUrlWithAuth = (to: string): string | void => { + const callback = () => this.clerkjs?.buildUrlWithAuth(to) || ''; if (this.clerkjs && this.#loaded) { return callback(); } else { diff --git a/packages/shared/src/__tests__/devbrowser.test.ts b/packages/shared/src/__tests__/devbrowser.test.ts index 8c5a340ba60..0a656eb872c 100644 --- a/packages/shared/src/__tests__/devbrowser.test.ts +++ b/packages/shared/src/__tests__/devbrowser.test.ts @@ -3,22 +3,26 @@ import { getDevBrowserJWTFromURL, setDevBrowserJWTInURL } from '../devBrowser'; const DUMMY_URL_BASE = 'http://clerk-dummy'; describe('setDevBrowserJWTInURL(url, jwt)', () => { - const testCases: Array<[string, string, boolean, string]> = [ - ['', 'deadbeef', false, '#__clerk_db_jwt[deadbeef]'], - ['foo', 'deadbeef', false, 'foo#__clerk_db_jwt[deadbeef]'], - ['/foo', 'deadbeef', false, '/foo#__clerk_db_jwt[deadbeef]'], - ['#foo', 'deadbeef', false, '#foo__clerk_db_jwt[deadbeef]'], - ['/foo?bar=42#qux', 'deadbeef', false, '/foo?bar=42#qux__clerk_db_jwt[deadbeef]'], - ['/foo#__clerk_db_jwt[deadbeef]', 'deadbeef', false, '/foo#__clerk_db_jwt[deadbeef]'], - ['/foo?bar=42#qux__clerk_db_jwt[deadbeef]', 'deadbeef', false, '/foo?bar=42#qux__clerk_db_jwt[deadbeef]'], - ['/foo', 'deadbeef', true, '/foo?__clerk_db_jwt=deadbeef'], - ['/foo?bar=42', 'deadbeef', true, '/foo?bar=42&__clerk_db_jwt=deadbeef'], + const testCases: Array<[string, string, string]> = [ + ['', 'deadbeef', '?__clerk_db_jwt=deadbeef'], + ['foo', 'deadbeef', 'foo?__clerk_db_jwt=deadbeef'], + ['/foo', 'deadbeef', '/foo?__clerk_db_jwt=deadbeef'], + ['#foo', 'deadbeef', '?__clerk_db_jwt=deadbeef#foo'], + ['/foo?bar=42#qux', 'deadbeef', '/foo?bar=42&__clerk_db_jwt=deadbeef#qux'], + ['/foo#__clerk_db_jwt[deadbeef2]', 'deadbeef', '/foo?__clerk_db_jwt=deadbeef#__clerk_db_jwt[deadbeef2]'], + [ + '/foo?bar=42#qux__clerk_db_jwt[deadbeef2]', + 'deadbeef', + '/foo?bar=42&__clerk_db_jwt=deadbeef#qux__clerk_db_jwt[deadbeef2]', + ], + ['/foo', 'deadbeef', '/foo?__clerk_db_jwt=deadbeef'], + ['/foo?bar=42', 'deadbeef', '/foo?bar=42&__clerk_db_jwt=deadbeef'], ]; test.each(testCases)( 'sets the dev browser JWT at the end of the provided url. Params: url=(%s), jwt=(%s), expected url=(%s)', - (input, paramName, asQueryParam, expected) => { - expect(setDevBrowserJWTInURL(new URL(input, DUMMY_URL_BASE), paramName, asQueryParam).href).toEqual( + (input, paramName, expected) => { + expect(setDevBrowserJWTInURL(new URL(input, DUMMY_URL_BASE), paramName).href).toEqual( new URL(expected, DUMMY_URL_BASE).href, ); }, @@ -27,7 +31,7 @@ describe('setDevBrowserJWTInURL(url, jwt)', () => { const oldHistory = globalThis.history; -describe('getDevBrowserJWTFromURL(url,)', () => { +describe('getDevBrowserJWTFromURL(url)', () => { const replaceStateMock = jest.fn(); beforeEach(() => { @@ -53,11 +57,15 @@ describe('getDevBrowserJWTFromURL(url,)', () => { const testCases: Array<[string, string, null | string]> = [ ['', '', null], ['foo', '', null], - ['#__clerk_db_jwt[deadbeef]', 'deadbeef', ''], - ['foo#__clerk_db_jwt[deadbeef]', 'deadbeef', 'foo'], - ['/foo#__clerk_db_jwt[deadbeef]', 'deadbeef', '/foo'], - ['#foo__clerk_db_jwt[deadbeef]', 'deadbeef', '#foo'], - ['/foo?bar=42#qux__clerk_db_jwt[deadbeef]', 'deadbeef', '/foo?bar=42#qux'], + ['?__clerk_db_jwt=deadbeef', 'deadbeef', ''], + ['foo?__clerk_db_jwt=deadbeef', 'deadbeef', 'foo'], + ['/foo?__clerk_db_jwt=deadbeef', 'deadbeef', '/foo'], + ['?__clerk_db_jwt=deadbeef#foo', 'deadbeef', '#foo'], + [ + '/foo?bar=42&__clerk_db_jwt=deadbeef#qux__clerk_db_jwt[deadbeef2]', + 'deadbeef', + '/foo?bar=42#qux__clerk_db_jwt[deadbeef2]', + ], ]; test.each(testCases)( diff --git a/packages/shared/src/devBrowser.ts b/packages/shared/src/devBrowser.ts index 63bec3084c6..cec2382a294 100644 --- a/packages/shared/src/devBrowser.ts +++ b/packages/shared/src/devBrowser.ts @@ -1,32 +1,19 @@ export const DEV_BROWSER_JWT_KEY = '__clerk_db_jwt'; export const DEV_BROWSER_JWT_HEADER = 'Clerk-Db-Jwt'; -const DEV_BROWSER_JWT_MARKER_REGEXP = /__clerk_db_jwt\[(.*)\]/; - // Sets the dev_browser JWT in the hash or the search -export function setDevBrowserJWTInURL(url: URL, jwt: string, asQueryParam: boolean): URL { +export function setDevBrowserJWTInURL(url: URL, jwt: string): URL { const resultURL = new URL(url); - // extract & strip existing jwt from hash - const jwtFromHash = extractDevBrowserJWTFromHash(resultURL.hash); - resultURL.hash = resultURL.hash.replace(DEV_BROWSER_JWT_MARKER_REGEXP, ''); - if (resultURL.href.endsWith('#')) { - resultURL.hash = ''; - } - // extract & strip existing jwt from search const jwtFromSearch = resultURL.searchParams.get(DEV_BROWSER_JWT_KEY); resultURL.searchParams.delete(DEV_BROWSER_JWT_KEY); // Existing jwt takes precedence - const jwtToSet = jwtFromHash || jwtFromSearch || jwt; + const jwtToSet = jwtFromSearch || jwt; if (jwtToSet) { - if (asQueryParam) { - resultURL.searchParams.append(DEV_BROWSER_JWT_KEY, jwtToSet); - } else { - resultURL.hash = resultURL.hash + `${DEV_BROWSER_JWT_KEY}[${jwtToSet}]`; - } + resultURL.searchParams.set(DEV_BROWSER_JWT_KEY, jwtToSet); } return resultURL; @@ -38,19 +25,10 @@ export function setDevBrowserJWTInURL(url: URL, jwt: string, asQueryParam: boole export function getDevBrowserJWTFromURL(url: URL): string { const resultURL = new URL(url); - // extract & strip existing jwt from hash - const jwtFromHash = extractDevBrowserJWTFromHash(resultURL.hash); - resultURL.hash = resultURL.hash.replace(DEV_BROWSER_JWT_MARKER_REGEXP, ''); - if (resultURL.href.endsWith('#')) { - resultURL.hash = ''; - } - // extract & strip existing jwt from search - const jwtFromSearch = resultURL.searchParams.get(DEV_BROWSER_JWT_KEY) || ''; + const jwt = resultURL.searchParams.get(DEV_BROWSER_JWT_KEY) || ''; resultURL.searchParams.delete(DEV_BROWSER_JWT_KEY); - const jwt = jwtFromHash || jwtFromSearch; - // eslint-disable-next-line valid-typeof if (jwt && typeof globalThis.history !== undefined) { globalThis.history.replaceState(null, '', resultURL.href); @@ -58,8 +36,3 @@ export function getDevBrowserJWTFromURL(url: URL): string { return jwt; } - -function extractDevBrowserJWTFromHash(hash: string): string { - const matches = hash.match(DEV_BROWSER_JWT_MARKER_REGEXP); - return matches ? matches[1] : ''; -} diff --git a/packages/types/src/authConfig.ts b/packages/types/src/authConfig.ts index f6ea57ad2b0..698e664ec8d 100644 --- a/packages/types/src/authConfig.ts +++ b/packages/types/src/authConfig.ts @@ -5,10 +5,4 @@ export interface AuthConfigResource extends ClerkResource { * Enabled single session configuration at the instance level. */ singleSessionMode: boolean; - - /** - * Denotes if the instance will use the new mode for syncing development sessions which uses URL - * decoration instead of third-party cookies. - */ - urlBasedSessionSyncing: boolean; } diff --git a/packages/types/src/clerk.ts b/packages/types/src/clerk.ts index 962dcbdafc3..67ffefcfd13 100644 --- a/packages/types/src/clerk.ts +++ b/packages/types/src/clerk.ts @@ -308,9 +308,8 @@ export interface Clerk { * Decorates the provided url with the auth token for development instances. * * @param {string} to - * @param opts A {@link BuildUrlWithAuthParams} object */ - buildUrlWithAuth(to: string, opts?: BuildUrlWithAuthParams): string; + buildUrlWithAuth(to: string): string; /** * Returns the configured url where is mounted or a custom sign-in page is rendered. @@ -492,13 +491,6 @@ export type HandleOAuthCallbackParams = { export type HandleSamlCallbackParams = HandleOAuthCallbackParams; -export type BuildUrlWithAuthParams = { - /** - * Controls if dev browser JWT is added as a query param - */ - useQueryParam?: boolean | null; -}; - export type CustomNavigation = (to: string, options?: NavigateOptions) => Promise | void; export type ClerkThemeOptions = DeepSnakeToCamel>;