From 435ab5055290defda48dc9163b3b5fd93503891e Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Fri, 15 Dec 2023 22:32:59 +0200 Subject: [PATCH 1/5] feat(clerk-js): Retheme Sign Up (#2376) * feat(clerk-js): Retheme Sign Up * feat(clerk-js): Retheme Sign Up * chore(repo): Add Changeset * test(clerk-js): Fix tests --- .changeset/odd-eels-ring.md | 2 + .../src/ui/common/EmailLinkStatusCard.tsx | 19 ++-- .../clerk-js/src/ui/common/SSOCallback.tsx | 1 + .../SignIn/SignInAccountSwitcher.tsx | 95 ++++++++++--------- .../ui/components/SignUp/SignUpContinue.tsx | 74 ++++++++------- .../SignUp/__tests__/SignUpContinue.test.tsx | 2 +- .../SignUp/__tests__/SignUpStart.test.tsx | 2 +- .../__tests__/SignUpVerifyEmail.test.tsx | 2 +- .../__tests__/SignUpVerifyPhone.test.tsx | 2 +- .../clerk-js/src/ui/elements/LoadingCard.tsx | 1 + packages/localizations/src/en-US.ts | 14 +-- 11 files changed, 114 insertions(+), 100 deletions(-) create mode 100644 .changeset/odd-eels-ring.md diff --git a/.changeset/odd-eels-ring.md b/.changeset/odd-eels-ring.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/odd-eels-ring.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/clerk-js/src/ui/common/EmailLinkStatusCard.tsx b/packages/clerk-js/src/ui/common/EmailLinkStatusCard.tsx index 94496b1207c..8531448a913 100644 --- a/packages/clerk-js/src/ui/common/EmailLinkStatusCard.tsx +++ b/packages/clerk-js/src/ui/common/EmailLinkStatusCard.tsx @@ -35,14 +35,17 @@ export const EmailLinkStatusCard = (props: EmailLinkStatusCardProps) => { return ( - {card.error} - - - - - - - + + {card.error} + + + + + + + + + ); diff --git a/packages/clerk-js/src/ui/common/SSOCallback.tsx b/packages/clerk-js/src/ui/common/SSOCallback.tsx index 0dcc5b3251d..40d7f4bf47d 100644 --- a/packages/clerk-js/src/ui/common/SSOCallback.tsx +++ b/packages/clerk-js/src/ui/common/SSOCallback.tsx @@ -37,6 +37,7 @@ export const SSOCallbackCard = (props: HandleOAuthCallbackParams | HandleSamlCal {card.error} + ); diff --git a/packages/clerk-js/src/ui/components/SignIn/SignInAccountSwitcher.tsx b/packages/clerk-js/src/ui/components/SignIn/SignInAccountSwitcher.tsx index c3173f9b2f2..61831d76228 100644 --- a/packages/clerk-js/src/ui/components/SignIn/SignInAccountSwitcher.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/SignInAccountSwitcher.tsx @@ -25,54 +25,57 @@ const _SignInAccountSwitcher = () => { return ( - {card.error} - - Signed out - Select account to continue to {applicationName} - - - - {activeSessions.map(s => ( - ({ height: theme.sizes.$16, justifyContent: 'flex-start' })} + + {card.error} + + Signed out + Select account to continue to {applicationName} + + + + {activeSessions.map(s => ( + ({ height: theme.sizes.$16, justifyContent: 'flex-start' })} + > + + + ))} + + + ({ color: theme.colors.$blackAlpha500 })} + /> + } + onClick={handleAddAccountClicked} > - - - ))} - - - ({ color: theme.colors.$blackAlpha500 })} - /> - } - onClick={handleAddAccountClicked} - > - Add account - - ({ color: theme.colors.$blackAlpha500 })} - /> - } - onClick={handleSignOutAllClicked} - > - Sign out of all accounts - + Add account + + ({ color: theme.colors.$blackAlpha500 })} + /> + } + onClick={handleSignOutAllClicked} + > + Sign out of all accounts + + - + + ); diff --git a/packages/clerk-js/src/ui/components/SignUp/SignUpContinue.tsx b/packages/clerk-js/src/ui/components/SignUp/SignUpContinue.tsx index 96ded091d86..ffb13ce9a72 100644 --- a/packages/clerk-js/src/ui/components/SignUp/SignUpContinue.tsx +++ b/packages/clerk-js/src/ui/components/SignUp/SignUpContinue.tsx @@ -160,42 +160,46 @@ function _SignUpContinue() { return ( - {card.error} - - - - - - - {(showOauthProviders || showWeb3Providers) && ( - + {card.error} + + + + + + + {(showOauthProviders || showWeb3Providers) && ( + + )} + - )} - - - - - - - - - - + + + + + + + + + + + + ); diff --git a/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpContinue.test.tsx b/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpContinue.test.tsx index ff242d817a3..f588cd654cf 100644 --- a/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpContinue.test.tsx +++ b/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpContinue.test.tsx @@ -82,7 +82,7 @@ describe('SignUpContinue', () => { }); render(, { wrapper }); - const signInLink = screen.getByText('Have an account?').nextElementSibling; + const signInLink = screen.getByText('Already have an account?').nextElementSibling; expect(signInLink?.textContent).toBe('Sign in'); expect(signInLink?.tagName.toUpperCase()).toBe('A'); expect(signInLink?.getAttribute('href')).toMatch(fixtures.environment.displayConfig.signInUrl); diff --git a/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpStart.test.tsx b/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpStart.test.tsx index 6b95a1137f5..aa0de597504 100644 --- a/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpStart.test.tsx +++ b/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpStart.test.tsx @@ -141,7 +141,7 @@ describe('SignUpStart', () => { }); render(, { wrapper }); - const signInLink = screen.getByText('Have an account?').nextElementSibling; + const signInLink = screen.getByText('Already have an account?').nextElementSibling; expect(signInLink?.textContent).toBe('Sign in'); expect(signInLink?.tagName.toUpperCase()).toBe('A'); expect(signInLink?.getAttribute('href')).toMatch(fixtures.environment.displayConfig.signInUrl); diff --git a/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpVerifyEmail.test.tsx b/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpVerifyEmail.test.tsx index cb21ba46a6d..9cd0189dc5c 100644 --- a/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpVerifyEmail.test.tsx +++ b/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpVerifyEmail.test.tsx @@ -54,7 +54,7 @@ describe('SignUpVerifyEmail', () => { render(, { wrapper }); screen.getByText(/Verify your email/i); - screen.getByText(/to continue to TestApp/i); + screen.getByText(/Enter the verification code sent to your email/i); }); it('clicking on the edit icon navigates to the previous route', async () => { diff --git a/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpVerifyPhone.test.tsx b/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpVerifyPhone.test.tsx index 60d45fa38bd..bf5d6ddfe7e 100644 --- a/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpVerifyPhone.test.tsx +++ b/packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpVerifyPhone.test.tsx @@ -29,7 +29,7 @@ describe('SignUpVerifyPhone', () => { }); render(, { wrapper }); screen.getByText(/Verify your phone/i); - screen.getByText(/to continue to TestApp/i); + screen.getByText(/Enter the verification code sent to your phone/i); }); it('clicking on the edit icon navigates to the previous route', async () => { diff --git a/packages/clerk-js/src/ui/elements/LoadingCard.tsx b/packages/clerk-js/src/ui/elements/LoadingCard.tsx index e8a78567156..5c099f0b46d 100644 --- a/packages/clerk-js/src/ui/elements/LoadingCard.tsx +++ b/packages/clerk-js/src/ui/elements/LoadingCard.tsx @@ -34,6 +34,7 @@ export const LoadingCard = withCardStateProvider(() => { {card.error} + ); }); diff --git a/packages/localizations/src/en-US.ts b/packages/localizations/src/en-US.ts index cd11e60856a..6fcbaaccbfa 100644 --- a/packages/localizations/src/en-US.ts +++ b/packages/localizations/src/en-US.ts @@ -88,8 +88,8 @@ export const enUS: LocalizationResource = { signUp: { start: { title: 'Create your account', - subtitle: 'to continue to {{applicationName}}', - actionText: 'Have an account?', + subtitle: 'Welcome! Please fill in the details to get started.', + actionText: 'Already have an account?', actionLink: 'Sign in', }, emailLink: { @@ -112,22 +112,22 @@ export const enUS: LocalizationResource = { }, emailCode: { title: 'Verify your email', - subtitle: 'to continue to {{applicationName}}', + subtitle: 'Enter the verification code sent to your email', formTitle: 'Verification code', formSubtitle: 'Enter the verification code sent to your email address', resendButton: "Didn't receive a code? Resend", }, phoneCode: { title: 'Verify your phone', - subtitle: 'to continue to {{applicationName}}', + subtitle: 'Enter the verification code sent to your phone', formTitle: 'Verification code', formSubtitle: 'Enter the verification code sent to your phone number', resendButton: "Didn't receive a code? Resend", }, continue: { title: 'Fill in missing fields', - subtitle: 'to continue to {{applicationName}}', - actionText: 'Have an account?', + subtitle: 'Please fill in the remaining details to continue.', + actionText: 'Already have an account?', actionLink: 'Sign in', }, }, @@ -135,7 +135,7 @@ export const enUS: LocalizationResource = { start: { title: 'Sign in to {{applicationName}}', subtitle: 'Welcome back! Please sign in to continue', - actionText: 'No account?', + actionText: 'Don’t have an account?', actionLink: 'Sign up', actionLink__use_email: 'Use email', actionLink__use_phone: 'Use phone', From 5f58a22746aba94f76bef5dbbc94fa93ea3b0b7e Mon Sep 17 00:00:00 2001 From: Tom Milewski Date: Fri, 15 Dec 2023 16:01:23 -0500 Subject: [PATCH 2/5] refactor(clerk-js,nextjs,clerk-react,shared,types): Remove hashing and (#2367) 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 32305d9b18f..844a07a47db 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>; From a9fe242be4dbaaa02c6643fea0688f1fb23f23e7 Mon Sep 17 00:00:00 2001 From: Dimitris Klouvas Date: Fri, 15 Dec 2023 23:02:38 +0200 Subject: [PATCH 3/5] chore(*): Improve @clerk/backend DX [Part 6 - token and jwt utils return values] (#2377) * chore(backend,types): Introduce ReturnWithError and use it as `verifyToken` return value * chore(backend,types): Make /jwt subpath utilies to return `ReturnWithError` type * chore(backend,types): Rename and move `ReturnWithError` move types to backend `JwtReturnType` --- .changeset/dull-ants-argue.md | 14 +++ .changeset/mighty-rice-marry.md | 5 + .changeset/proud-trees-yell.md | 23 ++++ packages/backend/README.md | 25 ++--- .../backend/src/__tests__/exports.test.ts | 1 + packages/backend/src/errors.ts | 2 + .../backend/src/jwt/__tests__/signJwt.test.ts | 11 +- .../src/jwt/__tests__/verifyJwt.test.ts | 91 ++++++++------- packages/backend/src/jwt/signJwt.ts | 14 ++- packages/backend/src/jwt/types.ts | 9 ++ packages/backend/src/jwt/verifyJwt.ts | 104 +++++++++++------- .../src/tokens/__tests__/verify.test.ts | 8 +- packages/backend/src/tokens/handshake.ts | 22 ++-- packages/backend/src/tokens/request.ts | 70 +++++++----- packages/backend/src/tokens/verify.ts | 57 ++++++---- packages/backend/src/util/testUtils.ts | 6 + packages/nextjs/src/server/getAuth.ts | 8 +- 17 files changed, 305 insertions(+), 165 deletions(-) create mode 100644 .changeset/dull-ants-argue.md create mode 100644 .changeset/mighty-rice-marry.md create mode 100644 .changeset/proud-trees-yell.md create mode 100644 packages/backend/src/jwt/types.ts diff --git a/.changeset/dull-ants-argue.md b/.changeset/dull-ants-argue.md new file mode 100644 index 00000000000..5a65fd09620 --- /dev/null +++ b/.changeset/dull-ants-argue.md @@ -0,0 +1,14 @@ +--- +'@clerk/backend': major +--- + +Change return value of `verifyToken()` from `@clerk/backend` to `{ data, error}`. +To replicate the current behaviour use this: +```typescript +import { verifyToken } from '@clerk/backend' + +const { data, error } = await verifyToken(...); +if(error){ + throw error; +} +``` \ No newline at end of file diff --git a/.changeset/mighty-rice-marry.md b/.changeset/mighty-rice-marry.md new file mode 100644 index 00000000000..b9f461e5f62 --- /dev/null +++ b/.changeset/mighty-rice-marry.md @@ -0,0 +1,5 @@ +--- +'@clerk/types': minor +--- + +Introduce new `ResultWithError` type in `@clerk/types` diff --git a/.changeset/proud-trees-yell.md b/.changeset/proud-trees-yell.md new file mode 100644 index 00000000000..84af419ccfd --- /dev/null +++ b/.changeset/proud-trees-yell.md @@ -0,0 +1,23 @@ +--- +'@clerk/backend': major +'@clerk/nextjs': major +'@clerk/types': major +--- + +Change return values of `signJwt`, `hasValidSignature`, `decodeJwt`, `verifyJwt` +to return `{ data, error }`. Example of keeping the same behavior using those utilities: +```typescript +import { signJwt, hasValidSignature, decodeJwt, verifyJwt } from '@clerk/backend/jwt'; + +const { data, error } = await signJwt(...) +if (error) throw error; + +const { data, error } = await hasValidSignature(...) +if (error) throw error; + +const { data, error } = decodeJwt(...) +if (error) throw error; + +const { data, error } = await verifyJwt(...) +if (error) throw error; +``` \ No newline at end of file diff --git a/packages/backend/README.md b/packages/backend/README.md index b8f7f821a5d..7e31d3d0ab0 100644 --- a/packages/backend/README.md +++ b/packages/backend/README.md @@ -103,7 +103,7 @@ Verifies a Clerk generated JWT (i.e. Clerk Session JWT and Clerk JWT templates). ```js import { verifyToken } from '@clerk/backend'; -verifyToken(token, { +const { result, error } = await verifyToken(token, { issuer: '...', authorizedParties: '...', }); @@ -114,11 +114,10 @@ verifyToken(token, { Verifies a Clerk generated JWT (i.e. Clerk Session JWT and Clerk JWT templates). The key needs to be provided in the options. ```js -import { verifyJwt } from '@clerk/backend'; +import { verifyJwt } from '@clerk/backend/jwt'; -verifyJwt(token, { +const { result, error } = verifyJwt(token, { key: JsonWebKey | string, - issuer: '...', authorizedParties: '...', }); ``` @@ -128,9 +127,9 @@ verifyJwt(token, { Decodes a JWT. ```js -import { decodeJwt } from '@clerk/backend'; +import { decodeJwt } from '@clerk/backend/jwt'; -decodeJwt(token); +const { result, error } = decodeJwt(token); ``` #### hasValidSignature(jwt: Jwt, key: JsonWebKey | string) @@ -138,9 +137,9 @@ decodeJwt(token); Verifies that the JWT has a valid signature. The key needs to be provided. ```js -import { hasValidSignature } from '@clerk/backend'; +import { hasValidSignature } from '@clerk/backend/jwt'; -hasValidSignature(token, jwk); +const { result, error } = await hasValidSignature(token, jwk); ``` #### debugRequestState(requestState) @@ -148,7 +147,7 @@ hasValidSignature(token, jwk); Generates a debug payload for the request state ```js -import { debugRequestState } from '@clerk/backend'; +import { debugRequestState } from '@clerk/backend/internal'; debugRequestState(requestState); ``` @@ -158,7 +157,7 @@ debugRequestState(requestState); Builds the AuthObject when the user is signed in. ```js -import { signedInAuthObject } from '@clerk/backend'; +import { signedInAuthObject } from '@clerk/backend/internal'; signedInAuthObject(jwtPayload, options); ``` @@ -168,7 +167,7 @@ signedInAuthObject(jwtPayload, options); Builds the empty AuthObject when the user is signed out. ```js -import { signedOutAuthObject } from '@clerk/backend'; +import { signedOutAuthObject } from '@clerk/backend/internal'; signedOutAuthObject(); ``` @@ -178,7 +177,7 @@ signedOutAuthObject(); Removes sensitive private metadata from user and organization resources in the AuthObject ```js -import { sanitizeAuthObject } from '@clerk/backend'; +import { sanitizeAuthObject } from '@clerk/backend/internal'; sanitizeAuthObject(authObject); ``` @@ -188,7 +187,7 @@ sanitizeAuthObject(authObject); Removes any `private_metadata` and `privateMetadata` attributes from the object to avoid leaking sensitive information to the browser during SSR. ```js -import { prunePrivateMetadata } from '@clerk/backend'; +import { prunePrivateMetadata } from '@clerk/backend/internal'; prunePrivateMetadata(obj); ``` diff --git a/packages/backend/src/__tests__/exports.test.ts b/packages/backend/src/__tests__/exports.test.ts index 44aaa2935d7..b76db3f0f66 100644 --- a/packages/backend/src/__tests__/exports.test.ts +++ b/packages/backend/src/__tests__/exports.test.ts @@ -18,6 +18,7 @@ export default (QUnit: QUnit) => { module('subpath /errors exports', () => { test('should not include a breaking change', assert => { const exportedApiKeys = [ + 'SignJWTError', 'TokenVerificationError', 'TokenVerificationErrorAction', 'TokenVerificationErrorCode', diff --git a/packages/backend/src/errors.ts b/packages/backend/src/errors.ts index 15a7580f465..32bf6fe170c 100644 --- a/packages/backend/src/errors.ts +++ b/packages/backend/src/errors.ts @@ -60,3 +60,5 @@ export class TokenVerificationError extends Error { })`; } } + +export class SignJWTError extends Error {} diff --git a/packages/backend/src/jwt/__tests__/signJwt.test.ts b/packages/backend/src/jwt/__tests__/signJwt.test.ts index 22fa071cd3b..35e31e4879a 100644 --- a/packages/backend/src/jwt/__tests__/signJwt.test.ts +++ b/packages/backend/src/jwt/__tests__/signJwt.test.ts @@ -9,6 +9,7 @@ import { publicJwks, signingJwks, } from '../../fixtures'; +import { assertOk } from '../../util/testUtils'; import { signJwt } from '../signJwt'; import { verifyJwt } from '../verifyJwt'; @@ -26,22 +27,24 @@ export default (QUnit: QUnit) => { }); test('signs a JWT with a JWK formatted secret', async assert => { - const jwt = await signJwt(payload, signingJwks, { + const { data } = await signJwt(payload, signingJwks, { algorithm: mockJwtHeader.alg, header: mockJwtHeader, }); + assertOk(assert, data); - const verifiedPayload = await verifyJwt(jwt, { key: publicJwks }); + const { data: verifiedPayload } = await verifyJwt(data, { key: publicJwks }); assert.deepEqual(verifiedPayload, payload); }); test('signs a JWT with a pkcs8 formatted secret', async assert => { - const jwt = await signJwt(payload, pemEncodedSignKey, { + const { data } = await signJwt(payload, pemEncodedSignKey, { algorithm: mockJwtHeader.alg, header: mockJwtHeader, }); + assertOk(assert, data); - const verifiedPayload = await verifyJwt(jwt, { key: pemEncodedPublicKey }); + const { data: verifiedPayload } = await verifyJwt(data, { key: pemEncodedPublicKey }); assert.deepEqual(verifiedPayload, payload); }); }); diff --git a/packages/backend/src/jwt/__tests__/verifyJwt.test.ts b/packages/backend/src/jwt/__tests__/verifyJwt.test.ts index 39c9e9007f2..29b36e7c43e 100644 --- a/packages/backend/src/jwt/__tests__/verifyJwt.test.ts +++ b/packages/backend/src/jwt/__tests__/verifyJwt.test.ts @@ -1,3 +1,4 @@ +import type { Jwt } from '@clerk/types'; import type QUnit from 'qunit'; import sinon from 'sinon'; @@ -11,66 +12,72 @@ import { signedJwt, someOtherPublicKey, } from '../../fixtures'; +import { assertOk } from '../../util/testUtils'; import { decodeJwt, hasValidSignature, verifyJwt } from '../verifyJwt'; export default (QUnit: QUnit) => { const { module, test } = QUnit; + const invalidTokenError = { + reason: 'token-invalid', + message: 'Invalid JWT form. A JWT consists of three parts separated by dots.', + }; module('hasValidSignature(jwt, key)', () => { test('verifies the signature with a JWK formatted key', async assert => { - assert.true(await hasValidSignature(decodeJwt(signedJwt), publicJwks)); + const { data: decodedResult } = decodeJwt(signedJwt); + assertOk(assert, decodedResult); + const { data: signatureResult } = await hasValidSignature(decodedResult, publicJwks); + assert.true(signatureResult); }); test('verifies the signature with a PEM formatted key', async assert => { - assert.true(await hasValidSignature(decodeJwt(signedJwt), pemEncodedPublicKey)); + const { data: decodedResult } = decodeJwt(signedJwt); + assertOk(assert, decodedResult); + const { data: signatureResult } = await hasValidSignature(decodedResult, pemEncodedPublicKey); + assert.true(signatureResult); }); test('it returns false if the key is not correct', async assert => { - assert.false(await hasValidSignature(decodeJwt(signedJwt), someOtherPublicKey)); + const { data: decodedResult } = decodeJwt(signedJwt); + assertOk(assert, decodedResult); + const { data: signatureResult } = await hasValidSignature(decodedResult, someOtherPublicKey); + assert.false(signatureResult); }); }); module('decodeJwt(jwt)', () => { test('decodes a valid JWT', assert => { - const { header, payload } = decodeJwt(mockJwt); - assert.propEqual(header, mockJwtHeader); - assert.propEqual(payload, mockJwtPayload); - // TODO: @dimkl assert signature is instance of Uint8Array + const { data } = decodeJwt(mockJwt); + assertOk(assert, data); + + assert.propEqual(data.header, mockJwtHeader); + assert.propEqual(data.payload, mockJwtPayload); + // TODO(@dimkl): assert signature is instance of Uint8Array }); - test('throws an error if null is given as jwt', assert => { - assert.throws( - () => decodeJwt('null'), - new Error('Invalid JWT form. A JWT consists of three parts separated by dots.'), - ); + test('returns an error if null is given as jwt', assert => { + const { error } = decodeJwt('null'); + assert.propContains(error, invalidTokenError); }); - test('throws an error if undefined is given as jwt', assert => { - assert.throws( - () => decodeJwt('undefined'), - new Error('Invalid JWT form. A JWT consists of three parts separated by dots.'), - ); + test('returns an error if undefined is given as jwt', assert => { + const { error } = decodeJwt('undefined'); + assert.propContains(error, invalidTokenError); }); - test('throws an error if empty string is given as jwt', assert => { - assert.throws( - () => decodeJwt('undefined'), - new Error('Invalid JWT form. A JWT consists of three parts separated by dots.'), - ); + test('returns an error if empty string is given as jwt', assert => { + const { error } = decodeJwt(''); + assert.propContains(error, invalidTokenError); }); test('throws an error if invalid string is given as jwt', assert => { - assert.throws( - () => decodeJwt('undefined'), - new Error('Invalid JWT form. A JWT consists of three parts separated by dots.'), - ); + const { error } = decodeJwt('whatever'); + assert.propContains(error, invalidTokenError); }); test('throws an error if number is given as jwt', assert => { - assert.throws( - () => decodeJwt('42'), - new Error('Invalid JWT form. A JWT consists of three parts separated by dots.'), - ); + const { error } = decodeJwt('42'); + assert.propContains(error, invalidTokenError); }); }); @@ -90,8 +97,8 @@ export default (QUnit: QUnit) => { issuer: mockJwtPayload.iss, authorizedParties: ['https://accounts.inspired.puma-74.lcl.dev'], }; - const payload = await verifyJwt(mockJwt, inputVerifyJwtOptions); - assert.propEqual(payload, mockJwtPayload); + const { data } = await verifyJwt(mockJwt, inputVerifyJwtOptions); + assert.propEqual(data, mockJwtPayload); }); test('returns the valid JWT payload if valid key & issuer method & azp is given', async assert => { @@ -100,8 +107,8 @@ export default (QUnit: QUnit) => { issuer: (iss: string) => iss.startsWith('https://clerk'), authorizedParties: ['https://accounts.inspired.puma-74.lcl.dev'], }; - const payload = await verifyJwt(mockJwt, inputVerifyJwtOptions); - assert.propEqual(payload, mockJwtPayload); + const { data } = await verifyJwt(mockJwt, inputVerifyJwtOptions); + assert.propEqual(data, mockJwtPayload); }); test('returns the valid JWT payload if valid key & issuer & list of azp (with empty string) is given', async assert => { @@ -110,12 +117,18 @@ export default (QUnit: QUnit) => { issuer: mockJwtPayload.iss, authorizedParties: ['', 'https://accounts.inspired.puma-74.lcl.dev'], }; - const payload = await verifyJwt(mockJwt, inputVerifyJwtOptions); - assert.propEqual(payload, mockJwtPayload); + const { data } = await verifyJwt(mockJwt, inputVerifyJwtOptions); + assert.propEqual(data, mockJwtPayload); }); - // todo('returns the reason of the failure when verifications fail', assert => { - // assert.true(true); - // }); + test('returns the reason of the failure when verifications fail', async assert => { + const inputVerifyJwtOptions = { + key: mockJwks.keys[0], + issuer: mockJwtPayload.iss, + authorizedParties: ['', 'https://accounts.inspired.puma-74.lcl.dev'], + }; + const { error } = await verifyJwt('invalid-jwt', inputVerifyJwtOptions); + assert.propContains(error, invalidTokenError); + }); }); }; diff --git a/packages/backend/src/jwt/signJwt.ts b/packages/backend/src/jwt/signJwt.ts index 5f3d2f8ef44..ada988a077b 100644 --- a/packages/backend/src/jwt/signJwt.ts +++ b/packages/backend/src/jwt/signJwt.ts @@ -1,7 +1,9 @@ +import { SignJWTError } from '../errors'; import runtime from '../runtime'; import { base64url } from '../util/rfc4648'; import { getCryptoAlgorithm } from './algorithms'; import { importKey } from './cryptoKeys'; +import type { JwtReturnType } from './types'; export interface SignJwtOptions { algorithm?: string; @@ -32,7 +34,7 @@ export async function signJwt( payload: Record, key: string | JsonWebKey, options: SignJwtOptions, -): Promise { +): Promise> { if (!options.algorithm) { throw new Error('No algorithm specified'); } @@ -53,7 +55,11 @@ export async function signJwt( const encodedPayload = encodeJwtData(payload); const firstPart = `${encodedHeader}.${encodedPayload}`; - const signature = await runtime.crypto.subtle.sign(algorithm, cryptoKey, encoder.encode(firstPart)); - - return `${firstPart}.${base64url.stringify(new Uint8Array(signature), { pad: false })}`; + try { + const signature = await runtime.crypto.subtle.sign(algorithm, cryptoKey, encoder.encode(firstPart)); + const encodedSignature = `${firstPart}.${base64url.stringify(new Uint8Array(signature), { pad: false })}`; + return { data: encodedSignature }; + } catch (error) { + return { error: new SignJWTError((error as Error)?.message) }; + } } diff --git a/packages/backend/src/jwt/types.ts b/packages/backend/src/jwt/types.ts new file mode 100644 index 00000000000..f0b34b58dc1 --- /dev/null +++ b/packages/backend/src/jwt/types.ts @@ -0,0 +1,9 @@ +export type JwtReturnType = + | { + data: R; + error?: undefined; + } + | { + data?: undefined; + error: E; + }; diff --git a/packages/backend/src/jwt/verifyJwt.ts b/packages/backend/src/jwt/verifyJwt.ts index 8e0dc3a81bb..acd48b40076 100644 --- a/packages/backend/src/jwt/verifyJwt.ts +++ b/packages/backend/src/jwt/verifyJwt.ts @@ -17,27 +17,40 @@ import { assertSubClaim, } from './assertions'; import { importKey } from './cryptoKeys'; +import type { JwtReturnType } from './types'; const DEFAULT_CLOCK_SKEW_IN_SECONDS = 5 * 1000; -export async function hasValidSignature(jwt: Jwt, key: JsonWebKey | string) { +export async function hasValidSignature(jwt: Jwt, key: JsonWebKey | string): Promise> { const { header, signature, raw } = jwt; const encoder = new TextEncoder(); const data = encoder.encode([raw.header, raw.payload].join('.')); const algorithm = getCryptoAlgorithm(header.alg); - const cryptoKey = await importKey(key, algorithm, 'verify'); - - return runtime.crypto.subtle.verify(algorithm.name, cryptoKey, signature, data); + try { + const cryptoKey = await importKey(key, algorithm, 'verify'); + + const verified = await runtime.crypto.subtle.verify(algorithm.name, cryptoKey, signature, data); + return { data: verified }; + } catch (error) { + return { + error: new TokenVerificationError({ + reason: TokenVerificationErrorReason.TokenInvalidSignature, + message: (error as Error)?.message, + }), + }; + } } -export function decodeJwt(token: string): Jwt { +export function decodeJwt(token: string): JwtReturnType { const tokenParts = (token || '').toString().split('.'); if (tokenParts.length !== 3) { - throw new TokenVerificationError({ - reason: TokenVerificationErrorReason.TokenInvalid, - message: `Invalid JWT form. A JWT consists of three parts separated by dots.`, - }); + return { + error: new TokenVerificationError({ + reason: TokenVerificationErrorReason.TokenInvalid, + message: `Invalid JWT form. A JWT consists of three parts separated by dots.`, + }), + }; } const [rawHeader, rawPayload, rawSignature] = tokenParts; @@ -63,7 +76,7 @@ export function decodeJwt(token: string): Jwt { const payload = JSON.parse(decoder.decode(base64url.parse(rawPayload, { loose: true }))); const signature = base64url.parse(rawSignature, { loose: true }); - return { + const data = { header, payload, signature, @@ -73,7 +86,9 @@ export function decodeJwt(token: string): Jwt { signature: rawSignature, text: token, }, - }; + } satisfies Jwt; + + return { data }; } export type VerifyJwtOptions = { @@ -86,47 +101,54 @@ export type VerifyJwtOptions = { export async function verifyJwt( token: string, { audience, authorizedParties, clockSkewInMs, key }: VerifyJwtOptions, -): Promise { +): Promise> { const clockSkew = clockSkewInMs || DEFAULT_CLOCK_SKEW_IN_SECONDS; - const decoded = decodeJwt(token); + const { data: decoded, error } = decodeJwt(token); + if (error) { + return { error }; + } const { header, payload } = decoded; + try { + // Header verifications + const { typ, alg } = header; - // Header verifications - const { typ, alg } = header; - - assertHeaderType(typ); - assertHeaderAlgorithm(alg); - - // Payload verifications - const { azp, sub, aud, iat, exp, nbf } = payload; - - assertSubClaim(sub); - assertAudienceClaim([aud], [audience]); - assertAuthorizedPartiesClaim(azp, authorizedParties); - assertExpirationClaim(exp, clockSkew); - assertActivationClaim(nbf, clockSkew); - assertIssuedAtClaim(iat, clockSkew); + assertHeaderType(typ); + assertHeaderAlgorithm(alg); - let signatureValid: boolean; + // Payload verifications + const { azp, sub, aud, iat, exp, nbf } = payload; - try { - signatureValid = await hasValidSignature(decoded, key); + assertSubClaim(sub); + assertAudienceClaim([aud], [audience]); + assertAuthorizedPartiesClaim(azp, authorizedParties); + assertExpirationClaim(exp, clockSkew); + assertActivationClaim(nbf, clockSkew); + assertIssuedAtClaim(iat, clockSkew); } catch (err) { - throw new TokenVerificationError({ - action: TokenVerificationErrorAction.EnsureClerkJWT, - reason: TokenVerificationErrorReason.TokenVerificationFailed, - message: `Error verifying JWT signature. ${err}`, - }); + return { error: err as TokenVerificationError }; + } + + const { data: signatureValid, error: signatureError } = await hasValidSignature(decoded, key); + if (signatureError) { + return { + error: new TokenVerificationError({ + action: TokenVerificationErrorAction.EnsureClerkJWT, + reason: TokenVerificationErrorReason.TokenVerificationFailed, + message: `Error verifying JWT signature. ${signatureError}`, + }), + }; } if (!signatureValid) { - throw new TokenVerificationError({ - reason: TokenVerificationErrorReason.TokenInvalidSignature, - message: 'JWT signature is invalid.', - }); + return { + error: new TokenVerificationError({ + reason: TokenVerificationErrorReason.TokenInvalidSignature, + message: 'JWT signature is invalid.', + }), + }; } - return payload; + return { data: payload }; } diff --git a/packages/backend/src/tokens/__tests__/verify.test.ts b/packages/backend/src/tokens/__tests__/verify.test.ts index bd32177b7c9..9034facec74 100644 --- a/packages/backend/src/tokens/__tests__/verify.test.ts +++ b/packages/backend/src/tokens/__tests__/verify.test.ts @@ -26,19 +26,19 @@ export default (QUnit: QUnit) => { }); test('verifies the provided session JWT', async assert => { - const payload = await verifyToken(mockJwt, { + const { data } = await verifyToken(mockJwt, { apiUrl: 'https://api.clerk.test', secretKey: 'a-valid-key', authorizedParties: ['https://accounts.inspired.puma-74.lcl.dev'], skipJwksCache: true, }); - assert.propEqual(payload, mockJwtPayload); + assert.propEqual(data, mockJwtPayload); assert.ok(fakeFetch.calledOnce); }); test('verifies the token by fetching the JWKs from Backend API when secretKey is provided ', async assert => { - const payload = await verifyToken(mockJwt, { + const { data } = await verifyToken(mockJwt, { secretKey: 'a-valid-key', authorizedParties: ['https://accounts.inspired.puma-74.lcl.dev'], skipJwksCache: true, @@ -52,7 +52,7 @@ export default (QUnit: QUnit) => { 'Clerk-Backend-SDK': '@clerk/backend', }, }); - assert.propEqual(payload, mockJwtPayload); + assert.propEqual(data, mockJwtPayload); }); }); }; diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 1dfd8b41bb9..4689f5ac64c 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -6,7 +6,10 @@ import { loadClerkJWKFromLocal, loadClerkJWKFromRemote } from './keys'; import type { VerifyTokenOptions } from './verify'; async function verifyHandshakeJwt(token: string, { key }: VerifyJwtOptions): Promise<{ handshake: string[] }> { - const decoded = decodeJwt(token); + const { data: decoded, error } = decodeJwt(token); + if (error) { + throw error; + } const { header, payload } = decoded; @@ -16,14 +19,11 @@ async function verifyHandshakeJwt(token: string, { key }: VerifyJwtOptions): Pro assertHeaderType(typ); assertHeaderAlgorithm(alg); - let signatureValid: boolean; - - try { - signatureValid = await hasValidSignature(decoded, key); - } catch (err) { + const { data: signatureValid, error: signatureError } = await hasValidSignature(decoded, key); + if (signatureError) { throw new TokenVerificationError({ reason: TokenVerificationErrorReason.TokenVerificationFailed, - message: `Error verifying handshake token. ${err}`, + message: `Error verifying handshake token. ${signatureError}`, }); } @@ -46,8 +46,12 @@ export async function verifyHandshakeToken( ): Promise<{ handshake: string[] }> { const { secretKey, apiUrl, apiVersion, jwksCacheTtlInMs, jwtKey, skipJwksCache } = options; - const { header } = decodeJwt(token); - const { kid } = header; + const { data, error } = decodeJwt(token); + if (error) { + throw error; + } + + const { kid } = data.header; let key; diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 507261ecfa7..f0df811a617 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -1,5 +1,4 @@ import { parsePublishableKey } from '@clerk/shared/keys'; -import type { JwtPayload } from '@clerk/types'; import { constants } from '../constants'; import type { TokenCarrier } from '../errors'; @@ -60,7 +59,7 @@ function assertSignInUrlFormatAndOrigin(_signInUrl: string, origin: string) { function isRequestEligibleForHandshake(authenticateContext: { secFetchDest?: string; accept?: string }) { const { accept, secFetchDest } = authenticateContext; - // NOTE: we could also check sec-fetch-mode === navigate here, but according to the spec, sec-fetch-dest: document should indicate that the request is the result of a user navigation. + // NOTE: we could also check sec-fetch-mode === navigate here, but according to the spec, sec-fetch-dest: document should indicate that the request is the data of a user navigation. if (secFetchDest === 'document') { return true; } @@ -145,37 +144,41 @@ export async function authenticateRequest( return signedOut(authenticateContext, AuthErrorReason.SessionTokenMissing, '', headers); } - let verifyResult: JwtPayload; + const { data, error } = await verifyToken(sessionToken, authenticateContext); + if (data) { + return signedIn(authenticateContext, data, headers); + } - try { - verifyResult = await verifyToken(sessionToken, authenticateContext); - } catch (err) { - if ( - err instanceof TokenVerificationError && - instanceType === 'development' && - (err.reason === TokenVerificationErrorReason.TokenExpired || - err.reason === TokenVerificationErrorReason.TokenNotActiveYet) - ) { - err.tokenCarrier = 'cookie'; - // This probably means we're dealing with clock skew - console.error( - `Clerk: Clock skew detected. This usually means that your system clock is inaccurate. Clerk will attempt to account for the clock skew in development. + if ( + instanceType === 'development' && + (error.reason === TokenVerificationErrorReason.TokenExpired || + error.reason === TokenVerificationErrorReason.TokenNotActiveYet) + ) { + error.tokenCarrier = 'cookie'; + // This probably means we're dealing with clock skew + console.error( + `Clerk: Clock skew detected. This usually means that your system clock is inaccurate. Clerk will attempt to account for the clock skew in development. To resolve this issue, make sure your system's clock is set to the correct time (e.g. turn off and on automatic time synchronization). --- -${err.getFullMessage()}`, - ); +${error.getFullMessage()}`, + ); - // Retry with a generous clock skew allowance (1 day) - verifyResult = await verifyToken(sessionToken, { ...authenticateContext, clockSkewInMs: 86_400_000 }); - } else { - throw err; + // Retry with a generous clock skew allowance (1 day) + const { data: retryResult, error: retryError } = await verifyToken(sessionToken, { + ...authenticateContext, + clockSkewInMs: 86_400_000, + }); + if (retryResult) { + return signedIn(authenticateContext, retryResult, headers); } + + throw retryError; } - return signedIn(authenticateContext, verifyResult!, headers); + throw error; } function handleMaybeHandshakeStatus( @@ -204,8 +207,12 @@ ${err.getFullMessage()}`, const { sessionTokenInHeader } = authenticateContext; try { - const verifyResult = await verifyToken(sessionTokenInHeader!, authenticateContext); - return await signedIn(options, verifyResult); + const { data, error } = await verifyToken(sessionTokenInHeader!, authenticateContext); + if (error) { + throw error; + } + // use `await` to force this try/catch handle the signedIn invocation + return await signedIn(options, data); } catch (err) { return handleError(err, 'header'); } @@ -294,17 +301,22 @@ ${err.getFullMessage()}`, return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.ClientUATWithoutSessionToken, ''); } - const decodeResult = decodeJwt(sessionToken!); + const { data: decodeResult, error: decodedError } = decodeJwt(sessionToken!); + if (decodedError) { + return handleError(decodedError, 'cookie'); + } if (decodeResult.payload.iat < clientUat) { return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.SessionTokenOutdated, ''); } try { - const verifyResult = await verifyToken(sessionToken!, authenticateContext); - if (verifyResult) { - return signedIn(authenticateContext, verifyResult); + const { data, error } = await verifyToken(sessionToken!, authenticateContext); + if (error) { + throw error; } + // use `await` to force this try/catch handle the signedIn invocation + return await signedIn(authenticateContext, data); } catch (err) { return handleError(err, 'cookie'); } diff --git a/packages/backend/src/tokens/verify.ts b/packages/backend/src/tokens/verify.ts index 90d1d716cc0..7d76a0a2212 100644 --- a/packages/backend/src/tokens/verify.ts +++ b/packages/backend/src/tokens/verify.ts @@ -3,6 +3,7 @@ import type { JwtPayload } from '@clerk/types'; import { TokenVerificationError, TokenVerificationErrorAction, TokenVerificationErrorReason } from '../errors'; import type { VerifyJwtOptions } from '../jwt'; import { decodeJwt, verifyJwt } from '../jwt'; +import type { JwtReturnType } from '../jwt/types'; import type { LoadClerkJWKFromRemoteOptions } from './keys'; import { loadClerkJWKFromLocal, loadClerkJWKFromRemote } from './keys'; @@ -13,7 +14,10 @@ export type VerifyTokenOptions = Pick; -export async function verifyToken(token: string, options: VerifyTokenOptions): Promise { +export async function verifyToken( + token: string, + options: VerifyTokenOptions, +): Promise> { const { secretKey, apiUrl, @@ -26,28 +30,39 @@ export async function verifyToken(token: string, options: VerifyTokenOptions): P skipJwksCache, } = options; - const { header } = decodeJwt(token); + const { data: decodedResult, error: decodedError } = decodeJwt(token); + if (decodedError) { + return { error: decodedError }; + } + + const { header } = decodedResult; const { kid } = header; - let key; - - if (jwtKey) { - key = loadClerkJWKFromLocal(jwtKey); - } else if (secretKey) { - // Fetch JWKS from Backend API using the key - key = await loadClerkJWKFromRemote({ secretKey, apiUrl, apiVersion, kid, jwksCacheTtlInMs, skipJwksCache }); - } else { - throw new TokenVerificationError({ - action: TokenVerificationErrorAction.SetClerkJWTKey, - message: 'Failed to resolve JWK during verification.', - reason: TokenVerificationErrorReason.JWKFailedToResolve, + try { + let key; + + if (jwtKey) { + key = loadClerkJWKFromLocal(jwtKey); + } else if (secretKey) { + // Fetch JWKS from Backend API using the key + key = await loadClerkJWKFromRemote({ secretKey, apiUrl, apiVersion, kid, jwksCacheTtlInMs, skipJwksCache }); + } else { + return { + error: new TokenVerificationError({ + action: TokenVerificationErrorAction.SetClerkJWTKey, + message: 'Failed to resolve JWK during verification.', + reason: TokenVerificationErrorReason.JWKFailedToResolve, + }), + }; + } + + return await verifyJwt(token, { + audience, + authorizedParties, + clockSkewInMs, + key, }); + } catch (error) { + return { error: error as TokenVerificationError }; } - - return await verifyJwt(token, { - audience, - authorizedParties, - clockSkewInMs, - key, - }); } diff --git a/packages/backend/src/util/testUtils.ts b/packages/backend/src/util/testUtils.ts index b12f26fcd7a..9dabe5c75e7 100644 --- a/packages/backend/src/util/testUtils.ts +++ b/packages/backend/src/util/testUtils.ts @@ -84,3 +84,9 @@ const mockHeadersGet = (key: string) => { return null; }; + +// used instead of the explicitly invoking assert.ok to avoid +// type casting the data param +export function assertOk(assert: Assert, data: unknown): asserts data is T { + assert.ok(data); +} diff --git a/packages/nextjs/src/server/getAuth.ts b/packages/nextjs/src/server/getAuth.ts index 9f372fde0ce..075c86dc69e 100644 --- a/packages/nextjs/src/server/getAuth.ts +++ b/packages/nextjs/src/server/getAuth.ts @@ -119,5 +119,11 @@ export const buildClerkProps: BuildClerkProps = (req, initState = {}) => { const parseJwt = (req: RequestLike) => { const cookieToken = getCookie(req, constants.Cookies.Session); const headerToken = getHeader(req, 'authorization')?.replace('Bearer ', ''); - return decodeJwt(cookieToken || headerToken || ''); + const { data, error } = decodeJwt(cookieToken || headerToken || ''); + + if (error) { + throw error; + } + + return data; }; From c5e16d1cd00cc96615fc779e671f7fd7d41721f1 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Sat, 16 Dec 2023 00:01:55 +0200 Subject: [PATCH 4/5] fix(nextjs): Type of `auth().protect()` when user is signed out (#2378) --- packages/nextjs/src/app-router/server/auth.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/app-router/server/auth.ts b/packages/nextjs/src/app-router/server/auth.ts index 6d0d4df10ac..03850723841 100644 --- a/packages/nextjs/src/app-router/server/auth.ts +++ b/packages/nextjs/src/app-router/server/auth.ts @@ -41,7 +41,7 @@ type AuthSignedOut = AuthObjectWithoutResources< * This function is experimental as it throws a Nextjs notFound error if user is not authenticated or authorized. * In the future we would investigate a way to throw a more appropriate error that clearly describes the not authorized of authenticated status. */ - protect: never; + protect: () => never; } >; From 3b9e8015f3eac63b9dcc8223ca8b43a3cbe5a041 Mon Sep 17 00:00:00 2001 From: Stefanos Anagnostou Date: Sat, 16 Dec 2023 00:02:19 +0200 Subject: [PATCH 5/5] feat(clerk-js): Retheme `` (#2379) * fix(clerk-js): Fix application logo in OrganizationList * feat(clerk-js): Retheme CreateOrganization component --- .changeset/honest-pigs-smoke.md | 2 + .../CreateOrganization/CreateOrganization.tsx | 10 +- .../CreateOrganizationForm.tsx | 95 +++++++++++-------- .../CreateOrganizationPage.tsx | 37 +++++--- .../OrganizationList/OrganizationListPage.tsx | 49 ++-------- .../__tests__/OrganizationList.test.tsx | 4 +- .../clerk-js/src/ui/elements/FormContent.tsx | 12 ++- .../clerk-js/src/ui/foundations/borders.ts | 1 + 8 files changed, 101 insertions(+), 109 deletions(-) create mode 100644 .changeset/honest-pigs-smoke.md diff --git a/.changeset/honest-pigs-smoke.md b/.changeset/honest-pigs-smoke.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/honest-pigs-smoke.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/clerk-js/src/ui/components/CreateOrganization/CreateOrganization.tsx b/packages/clerk-js/src/ui/components/CreateOrganization/CreateOrganization.tsx index 6fd05c64e61..e0ed90ea9b3 100644 --- a/packages/clerk-js/src/ui/components/CreateOrganization/CreateOrganization.tsx +++ b/packages/clerk-js/src/ui/components/CreateOrganization/CreateOrganization.tsx @@ -3,7 +3,7 @@ import type { CreateOrganizationModalProps } from '@clerk/types'; import { withOrganizationsEnabledGuard } from '../../common'; import { ComponentContext, withCoreUserGuard } from '../../contexts'; import { Flow } from '../../customizables'; -import { ProfileCard, withCardStateProvider } from '../../elements'; +import { withCardStateProvider } from '../../elements'; import { Route, Switch } from '../../router'; import type { CreateOrganizationCtx } from '../../types'; import { CreateOrganizationPage } from './CreateOrganizationPage'; @@ -23,13 +23,7 @@ const _CreateOrganization = () => { }; const AuthenticatedRoutes = withCoreUserGuard(() => { - return ( - ({ width: t.sizes.$120 })}> - - - - - ); + return ; }); export const CreateOrganization = withOrganizationsEnabledGuard( diff --git a/packages/clerk-js/src/ui/components/CreateOrganization/CreateOrganizationForm.tsx b/packages/clerk-js/src/ui/components/CreateOrganization/CreateOrganizationForm.tsx index bb7928de20a..9c14221ba16 100644 --- a/packages/clerk-js/src/ui/components/CreateOrganization/CreateOrganizationForm.tsx +++ b/packages/clerk-js/src/ui/components/CreateOrganization/CreateOrganizationForm.tsx @@ -3,12 +3,12 @@ import type { OrganizationResource } from '@clerk/types'; import React from 'react'; import { useWizard, Wizard } from '../../common'; -import { Icon } from '../../customizables'; +import { Col, Icon, Text } from '../../customizables'; import { Form, FormButtonContainer, FormContent, IconButton, SuccessPage, useCardState } from '../../elements'; -import { QuestionMark, Upload } from '../../icons'; +import { Upload } from '../../icons'; import type { LocalizationKey } from '../../localization'; import { localizationKeys } from '../../localization'; -import { colors, createSlug, handleError, useFormControl } from '../../utils'; +import { createSlug, handleError, useFormControl } from '../../utils'; import { InviteMembersForm } from '../OrganizationProfile/InviteMembersForm'; import { InvitationsSentMessage } from '../OrganizationProfile/InviteMembersPage'; import { OrganizationProfileAvatarUploader } from '../OrganizationProfile/OrganizationProfileAvatarUploader'; @@ -19,8 +19,8 @@ type CreateOrganizationFormProps = { onCancel?: () => void; onComplete?: () => void; flow: 'default' | 'organizationList'; - startPage: { - headerTitle: LocalizationKey; + startPage?: { + headerTitle?: LocalizationKey; headerSubtitle?: LocalizationKey; }; }; @@ -111,45 +111,59 @@ export const CreateOrganizationForm = (props: CreateOrganizationFormProps) => { ({ minHeight: t.sizes.$60 })} + sx={t => ({ minHeight: t.sizes.$60, gap: 0 })} > - await setFile(file)} - onAvatarRemove={file ? onAvatarRemove : null} - avatarPreviewPlaceholder={ - ({ - transitionDuration: theme.transitionDuration.$controls, - })} - /> - } - sx={theme => ({ - width: theme.sizes.$12, - height: theme.sizes.$12, - borderRadius: theme.radii.$md, - backgroundColor: theme.colors.$avatarBackground, - ':hover': { - backgroundColor: colors.makeTransparent(theme.colors.$avatarBackground, 0.2), - svg: { - transform: 'scale(1.2)', + + ({ + textAlign: 'left', + marginBottom: t.space.$2, + color: t.colors.$blackAlpha700, + })} + > + Logo + + await setFile(file)} + onAvatarRemove={file ? onAvatarRemove : null} + avatarPreviewPlaceholder={ + ({ + color: t.colors.$blackAlpha400, + transitionDuration: t.transitionDuration.$controls, + })} + /> + } + sx={t => ({ + width: t.sizes.$16, + height: t.sizes.$16, + borderRadius: t.radii.$md, + border: `${t.borders.$dashed} ${t.colors.$blackAlpha200}`, + backgroundColor: t.colors.$blackAlpha50, + ':hover': { + backgroundColor: t.colors.$blackAlpha50, + svg: { + transform: 'scale(1.2)', + }, }, - }, - })} - /> - } - /> + })} + /> + } + /> + { { + { @@ -12,18 +12,27 @@ export const CreateOrganizationPage = withCardStateProvider(() => { const { mode, navigateAfterCreateOrganization, skipInvitationScreen } = useCreateOrganizationContext(); return ( - { - if (mode === 'modal') { - closeCreateOrganization(); - } - }} - /> + ({ width: t.sizes.$120 })}> + ({ + padding: `${t.space.$4} ${t.space.$5}`, + })} + > + + + ({ padding: `${t.space.$5}` })}> + { + if (mode === 'modal') { + closeCreateOrganization(); + } + }} + /> + + + ); }); diff --git a/packages/clerk-js/src/ui/components/OrganizationList/OrganizationListPage.tsx b/packages/clerk-js/src/ui/components/OrganizationList/OrganizationListPage.tsx index eeaaa4606cf..6d484e98c00 100644 --- a/packages/clerk-js/src/ui/components/OrganizationList/OrganizationListPage.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationList/OrganizationListPage.tsx @@ -1,18 +1,9 @@ -import { useOrganization, useOrganizationList, useUser } from '@clerk/shared/react'; +import { useOrganizationList } from '@clerk/shared/react'; import { useState } from 'react'; import { useEnvironment, useOrganizationListContext } from '../../contexts'; import { Box, Col, descriptors, Flex, localizationKeys, Spinner } from '../../customizables'; -import { - Action, - Card, - Header, - OrganizationAvatar, - SecondaryActions, - useCardState, - UserAvatar, - withCardStateProvider, -} from '../../elements'; +import { Action, Card, Header, SecondaryActions, useCardState, withCardStateProvider } from '../../elements'; import { useInView } from '../../hooks'; import { Add } from '../../icons'; import { CreateOrganizationForm } from '../CreateOrganization/CreateOrganizationForm'; @@ -87,7 +78,6 @@ export const OrganizationListPage = withCardStateProvider(() => { }); const OrganizationListFlows = ({ showListInitially }: { showListInitially: boolean }) => { - const environment = useEnvironment(); const { navigateAfterSelectOrganization, skipInvitationScreen } = useOrganizationListContext(); const [isCreateOrganizationFlow, setCreateOrganizationFlow] = useState(!showListInitially); return ( @@ -99,18 +89,19 @@ const OrganizationListFlows = ({ showListInitially }: { showListInitially: boole {isCreateOrganizationFlow && ( ({ - padding: `${t.space.$8}`, + padding: `${t.space.$none} ${t.space.$5} ${t.space.$5}`, })} > + ({ + padding: `${t.space.$4} ${t.space.$5}`, + })} + > + + navigateAfterSelectOrganization(org).then(() => setCreateOrganizationFlow(false)) } @@ -129,8 +120,6 @@ const OrganizationListPageList = (props: { onCreateOrganizationClick: () => void const { ref, userMemberships, userSuggestions, userInvitations } = useOrganizationListInView(); const { hidePersonal } = useOrganizationListContext(); - const { organization } = useOrganization(); - const { user } = useUser(); const isLoading = userMemberships?.isLoading || userInvitations?.isLoading || userSuggestions?.isLoading; const hasNextPage = userMemberships?.hasNextPage || userInvitations?.hasNextPage || userSuggestions?.hasNextPage; @@ -145,24 +134,6 @@ const OrganizationListPageList = (props: { onCreateOrganizationClick: () => void padding: `${t.space.$none} ${t.space.$8}`, })} > - ({ paddingBottom: t.space.$6 })} - > - {organization && ( - t.sizes.$8} - /> - )} - {!organization && ( - t.sizes.$8} - /> - )} - { await waitFor(() => { // Header expect(queryByRole('heading', { name: /Create organization/i })).toBeInTheDocument(); - // Subheader - expect(queryByText('to continue to TestApp')).toBeInTheDocument(); - // + expect(queryByText('Personal account')).not.toBeInTheDocument(); // Form fields of CreateOrganizationForm diff --git a/packages/clerk-js/src/ui/elements/FormContent.tsx b/packages/clerk-js/src/ui/elements/FormContent.tsx index 585d312aa63..061e4c64ade 100644 --- a/packages/clerk-js/src/ui/elements/FormContent.tsx +++ b/packages/clerk-js/src/ui/elements/FormContent.tsx @@ -6,7 +6,7 @@ import type { PropsOfComponent } from '../styledSystem'; import { Card, Header, useCardState } from './index'; type PageProps = PropsOfComponent & { - headerTitle: LocalizationKey | string; + headerTitle?: LocalizationKey | string; headerTitleTextVariant?: PropsOfComponent['textVariant']; breadcrumbTitle?: LocalizationKey; Breadcrumbs?: React.ComponentType | null; @@ -37,10 +37,12 @@ export const FormContent = (props: PageProps) => { > {card.error} - + {headerTitle && ( + + )} {headerSubtitle && (