From c596cf997e5c44ee7110ac96cbdb108a2b8050d6 Mon Sep 17 00:00:00 2001 From: Konstantinos Pittas Date: Tue, 8 Aug 2023 15:27:29 +0300 Subject: [PATCH 1/7] feat(clerk-js): Improve redirects on OAuth callback Currently, if you try to sign up with a provider that allows unverified accounts, the flow appears to be broken because it asks for the email after the OAuth callback. With this change, it will navigate to the appropriate change when needed. --- .changeset/tender-icons-bake.md | 2 + packages/clerk-js/src/core/clerk.test.ts | 4 ++ packages/clerk-js/src/core/clerk.ts | 24 ++++++++- .../src/ui/components/SignUp/SignUp.tsx | 2 + .../SignUp/__tests__/SignUpContinue.test.tsx | 4 +- .../ui/components/SignUp/signUpFormHelpers.ts | 3 +- .../clerk-js/src/ui/components/SignUp/util.ts | 50 +++++++++++++------ packages/types/src/clerk.ts | 10 ++++ 8 files changed, 80 insertions(+), 19 deletions(-) create mode 100644 .changeset/tender-icons-bake.md diff --git a/.changeset/tender-icons-bake.md b/.changeset/tender-icons-bake.md new file mode 100644 index 0000000000..a845151cc8 --- /dev/null +++ b/.changeset/tender-icons-bake.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index ce4ef203cd..e7c64abce5 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -700,6 +700,8 @@ describe('Clerk singleton', () => { signIn: new SignIn(null), signUp: new SignUp({ status: 'missing_requirements', + missing_fields: [], + unverified_fields: [], verifications: { external_account: { status: 'unverified', @@ -1111,6 +1113,8 @@ describe('Clerk singleton', () => { signIn: new SignIn(null), signUp: new SignUp({ status: 'missing_requirements', + missing_fields: ['last_name'], + unverified_fields: [], verifications: { external_account: { status: 'verified', diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index ed0f6e621d..6d54189cf2 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -53,6 +53,7 @@ import type { } from '@clerk/types'; import type { MountComponentRenderer } from '../ui/Components'; +import { handleMissingRequirements } from '../ui/components/SignUp/util'; import { appendAsQueryParams, buildURL, @@ -834,6 +835,8 @@ export default class Clerk implements ClerkInterface { const { externalAccount } = signUp.verifications; const su = { status: signUp.status, + missingFields: signUp.missingFields, + unverifiedFields: signUp.unverifiedFields, externalAccountStatus: externalAccount.status, externalAccountErrorCode: externalAccount.error?.code, externalAccountSessionId: externalAccount.error?.meta?.sessionId, @@ -873,6 +876,23 @@ export default class Clerk implements ClerkInterface { buildURL({ base: displayConfig.signUpUrl, hashPath: '/continue' }, { stringify: true }), ); + const navigateToNextStepSignUp = () => { + if (su.unverifiedFields.length === 0 || su.missingFields.length !== 0) { + return navigateToContinueSignUp(); + } + + return handleMissingRequirements({ + signUp, + verifyEmailPath: + params.verifyEmailAddressUrl || + buildURL({ base: displayConfig.signUpUrl, hashPath: '/verify-email-address' }, { stringify: true }), + verifyPhonePath: + params.verifyPhoneNumberUrl || + buildURL({ base: displayConfig.signUpUrl, hashPath: '/verify-phone-number' }, { stringify: true }), + navigate, + }); + }; + const userExistsButNeedsToSignIn = su.externalAccountStatus === 'transferable' && su.externalAccountErrorCode === 'external_account_exists'; @@ -902,7 +922,7 @@ export default class Clerk implements ClerkInterface { beforeEmit: navigateAfterSignUp, }); case 'missing_requirements': - return navigateToContinueSignUp(); + return navigateToNextStepSignUp(); default: clerkOAuthCallbackDidNotCompleteSignInSignUp('sign in'); } @@ -938,7 +958,7 @@ export default class Clerk implements ClerkInterface { } if (su.externalAccountStatus === 'verified' && su.status === 'missing_requirements') { - return navigateToContinueSignUp(); + return navigateToNextStepSignUp(); } return navigateToSignIn(); diff --git a/packages/clerk-js/src/ui/components/SignUp/SignUp.tsx b/packages/clerk-js/src/ui/components/SignUp/SignUp.tsx index d85ff69995..2384a4056e 100644 --- a/packages/clerk-js/src/ui/components/SignUp/SignUp.tsx +++ b/packages/clerk-js/src/ui/components/SignUp/SignUp.tsx @@ -44,6 +44,8 @@ function SignUpRoutes(): JSX.Element { redirectUrl={signUpContext.redirectUrl} secondFactorUrl={signUpContext.secondFactorUrl} continueSignUpUrl='../continue' + verifyEmailAddressUrl='../verify-email-address' + verifyPhoneNumberUrl='../verify-phone-number' /> 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 838bd5f830..80aef9c658 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 @@ -42,7 +42,9 @@ describe('SignUpContinue', () => { f.startSignUpWithEmailAddress(); }); render(, { wrapper }); - screen.getByText(/email address/i); + // Because the email address is already set, it should not be shown, + // as we're in PSU mode and it's not a missing field. + expect(screen.queryByText(/email address/i)).not.toBeInTheDocument(); screen.getByText(/password/i); }); diff --git a/packages/clerk-js/src/ui/components/SignUp/signUpFormHelpers.ts b/packages/clerk-js/src/ui/components/SignUp/signUpFormHelpers.ts index dd35b65e39..3fcf7a24da 100644 --- a/packages/clerk-js/src/ui/components/SignUp/signUpFormHelpers.ts +++ b/packages/clerk-js/src/ui/components/SignUp/signUpFormHelpers.ts @@ -50,12 +50,13 @@ export function determineActiveFields(fieldProps: FieldDeterminationProps): Fiel // If continuing with an existing sign-up, show only fields absolutely necessary to minimize fiction export function minimizeFieldsForExistingSignup(fields: Fields, signUp: SignUpResource) { if (signUp) { + const hasEmailFilled = !!signUp.emailAddress; const hasVerifiedEmail = signUp.verifications?.emailAddress?.status == 'verified'; const hasVerifiedPhone = signUp.verifications?.phoneNumber?.status == 'verified'; const hasVerifiedExternalAccount = signUp.verifications?.externalAccount?.status == 'verified'; const hasVerifiedWeb3Wallet = signUp.verifications?.web3Wallet?.status == 'verified'; - if (hasVerifiedEmail) { + if (hasEmailFilled || hasVerifiedEmail) { delete fields.emailAddress; } diff --git a/packages/clerk-js/src/ui/components/SignUp/util.ts b/packages/clerk-js/src/ui/components/SignUp/util.ts index 6d9742ad30..4b6ca9efab 100644 --- a/packages/clerk-js/src/ui/components/SignUp/util.ts +++ b/packages/clerk-js/src/ui/components/SignUp/util.ts @@ -10,6 +10,32 @@ type CompleteSignUpFlowProps = { redirectUrlComplete?: string; }; +export const handleMissingRequirements = ({ + signUp, + verifyEmailPath, + verifyPhonePath, + navigate, + redirectUrl = '', + redirectUrlComplete = '', +}: CompleteSignUpFlowProps): Promise | undefined => { + if (signUp.missingFields.some(mf => mf === 'saml')) { + return signUp.authenticateWithRedirect({ + strategy: 'saml', + redirectUrl, + redirectUrlComplete, + continueSignUp: true, + }); + } + + if (signUp.unverifiedFields?.includes('email_address') && verifyEmailPath) { + return navigate(verifyEmailPath); + } + if (signUp.unverifiedFields?.includes('phone_number') && verifyPhonePath) { + return navigate(verifyPhonePath); + } + return; +}; + export const completeSignUpFlow = ({ signUp, verifyEmailPath, @@ -19,24 +45,18 @@ export const completeSignUpFlow = ({ redirectUrl = '', redirectUrlComplete = '', }: CompleteSignUpFlowProps): Promise | undefined => { - if (signUp.status === 'complete') { - return handleComplete && handleComplete(); - } else if (signUp.status === 'missing_requirements') { - if (signUp.missingFields.some(mf => mf === 'saml')) { - return signUp.authenticateWithRedirect({ - strategy: 'saml', + switch (signUp.status) { + case 'complete': + return handleComplete && handleComplete(); + case 'missing_requirements': + return handleMissingRequirements({ + signUp, + verifyEmailPath, + verifyPhonePath, + navigate, redirectUrl, redirectUrlComplete, - continueSignUp: true, }); - } - - if (signUp.unverifiedFields?.includes('email_address') && verifyEmailPath) { - return navigate(verifyEmailPath); - } - if (signUp.unverifiedFields?.includes('phone_number') && verifyPhonePath) { - return navigate(verifyPhonePath); - } } return; }; diff --git a/packages/types/src/clerk.ts b/packages/types/src/clerk.ts index 155edd9c5a..59110b940f 100644 --- a/packages/types/src/clerk.ts +++ b/packages/types/src/clerk.ts @@ -451,6 +451,16 @@ export type HandleOAuthCallbackParams = { * Full URL or path to navigate after an incomplete sign up. */ continueSignUpUrl?: string | null; + + /** + * Full URL or path to navigate after requesting email verification. + */ + verifyEmailAddressUrl?: string | null; + + /** + * Full URL or path to navigate after requesting phone verification. + */ + verifyPhoneNumberUrl?: string | null; }; /** From 47f7c4f36c1044bc1bd4a9c453114f46aaf96f5d Mon Sep 17 00:00:00 2001 From: Konstantinos Pittas Date: Thu, 10 Aug 2023 13:13:30 +0300 Subject: [PATCH 2/7] feat(clerk-js): Improve check for next step --- packages/clerk-js/src/core/clerk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 6d54189cf2..0bf254f2f3 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -877,7 +877,7 @@ export default class Clerk implements ClerkInterface { ); const navigateToNextStepSignUp = () => { - if (su.unverifiedFields.length === 0 || su.missingFields.length !== 0) { + if (su.missingFields.length) { return navigateToContinueSignUp(); } From a8d77966ef3640fd388f5fbeeeece39c3f582e94 Mon Sep 17 00:00:00 2001 From: Konstantinos Pittas Date: Thu, 10 Aug 2023 13:13:47 +0300 Subject: [PATCH 3/7] feat(clerk-js): Add test --- packages/clerk-js/src/core/clerk.test.ts | 49 ++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index e7c64abce5..da587d4b42 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -1138,6 +1138,55 @@ describe('Clerk singleton', () => { expect(mockNavigate).toHaveBeenCalledWith('/sign-up#/continue'); }); }); + + it('redirects user to the verify-email-address url if the external account has unverified email and there are no missing requirements', async () => { + mockEnvironmentFetch.mockReturnValue( + Promise.resolve({ + authConfig: {}, + userSettings: mockUserSettings, + displayConfig: mockDisplayConfig, + isSingleSession: () => false, + isProduction: () => false, + isDevelopmentOrStaging: () => true, + }), + ); + + mockClientFetch.mockReturnValue( + Promise.resolve({ + activeSessions: [], + signIn: new SignIn(null), + signUp: new SignUp({ + status: 'missing_requirements', + missing_fields: [], + unverified_fields: ['email_address'], + verifications: { + email_address: { + status: 'unverified', + strategy: 'from_oauth_google', + next_action: 'needs_attempt', + }, + external_account: { + status: 'verified', + strategy: 'oauth_google', + external_verification_redirect_url: '', + error: null, + }, + }, + } as any as SignUpJSON), + }), + ); + + const sut = new Clerk(frontendApi); + await sut.load({ + navigate: mockNavigate, + }); + + await sut.handleRedirectCallback(); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith('/sign-up#/verify-email-address'); + }); + }); }); describe('.handleMagicLinkVerification()', () => { From 64ffc860d177e2d81a07d2c5c31d3326c167e54a Mon Sep 17 00:00:00 2001 From: Konstantinos Pittas Date: Thu, 10 Aug 2023 13:48:52 +0300 Subject: [PATCH 4/7] fix(clerk-js): Tests and clean-up leftovers --- packages/clerk-js/src/core/clerk.test.ts | 11 ++++++++--- packages/clerk-js/src/core/clerk.ts | 10 +++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index da587d4b42..8155b45eda 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -593,7 +593,14 @@ describe('Clerk singleton', () => { }), ); - const mockSignUpCreate = jest.fn().mockReturnValue(Promise.resolve({ status: 'missing_requirements' })); + const mockSignUpCreate = jest.fn().mockReturnValue( + Promise.resolve( + new SignUp({ + status: 'missing_requirements', + missing_fields: ['phone_number'], + } as any as SignUpJSON), + ), + ); const sut = new Clerk(frontendApi); await sut.load({ @@ -701,7 +708,6 @@ describe('Clerk singleton', () => { signUp: new SignUp({ status: 'missing_requirements', missing_fields: [], - unverified_fields: [], verifications: { external_account: { status: 'unverified', @@ -1114,7 +1120,6 @@ describe('Clerk singleton', () => { signUp: new SignUp({ status: 'missing_requirements', missing_fields: ['last_name'], - unverified_fields: [], verifications: { external_account: { status: 'verified', diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 0bf254f2f3..9a0350d59b 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -44,6 +44,7 @@ import type { SignOut, SignOutCallback, SignOutOptions, + SignUpField, SignUpProps, SignUpResource, UnsubscribeCallback, @@ -836,7 +837,6 @@ export default class Clerk implements ClerkInterface { const su = { status: signUp.status, missingFields: signUp.missingFields, - unverifiedFields: signUp.unverifiedFields, externalAccountStatus: externalAccount.status, externalAccountErrorCode: externalAccount.error?.code, externalAccountSessionId: externalAccount.error?.meta?.sessionId, @@ -876,8 +876,8 @@ export default class Clerk implements ClerkInterface { buildURL({ base: displayConfig.signUpUrl, hashPath: '/continue' }, { stringify: true }), ); - const navigateToNextStepSignUp = () => { - if (su.missingFields.length) { + const navigateToNextStepSignUp = ({ missingFields }: { missingFields: SignUpField[] }) => { + if (missingFields.length) { return navigateToContinueSignUp(); } @@ -922,7 +922,7 @@ export default class Clerk implements ClerkInterface { beforeEmit: navigateAfterSignUp, }); case 'missing_requirements': - return navigateToNextStepSignUp(); + return navigateToNextStepSignUp({ missingFields: res.missingFields }); default: clerkOAuthCallbackDidNotCompleteSignInSignUp('sign in'); } @@ -958,7 +958,7 @@ export default class Clerk implements ClerkInterface { } if (su.externalAccountStatus === 'verified' && su.status === 'missing_requirements') { - return navigateToNextStepSignUp(); + return navigateToNextStepSignUp({ missingFields: signUp.missingFields }); } return navigateToSignIn(); From 7841cbd8b4e37860c34efb7c04895838ca52616c Mon Sep 17 00:00:00 2001 From: Konstantinos Pittas Date: Thu, 10 Aug 2023 14:33:40 +0300 Subject: [PATCH 5/7] refactor(clerk-js): Use completeSignUpFlow as it is --- packages/clerk-js/src/core/clerk.ts | 4 +- .../clerk-js/src/ui/components/SignUp/util.ts | 50 ++++++------------- 2 files changed, 17 insertions(+), 37 deletions(-) diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 9a0350d59b..e84e72ba68 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -54,7 +54,7 @@ import type { } from '@clerk/types'; import type { MountComponentRenderer } from '../ui/Components'; -import { handleMissingRequirements } from '../ui/components/SignUp/util'; +import { completeSignUpFlow } from '../ui/components/SignUp/util'; import { appendAsQueryParams, buildURL, @@ -881,7 +881,7 @@ export default class Clerk implements ClerkInterface { return navigateToContinueSignUp(); } - return handleMissingRequirements({ + return completeSignUpFlow({ signUp, verifyEmailPath: params.verifyEmailAddressUrl || diff --git a/packages/clerk-js/src/ui/components/SignUp/util.ts b/packages/clerk-js/src/ui/components/SignUp/util.ts index 4b6ca9efab..6d9742ad30 100644 --- a/packages/clerk-js/src/ui/components/SignUp/util.ts +++ b/packages/clerk-js/src/ui/components/SignUp/util.ts @@ -10,32 +10,6 @@ type CompleteSignUpFlowProps = { redirectUrlComplete?: string; }; -export const handleMissingRequirements = ({ - signUp, - verifyEmailPath, - verifyPhonePath, - navigate, - redirectUrl = '', - redirectUrlComplete = '', -}: CompleteSignUpFlowProps): Promise | undefined => { - if (signUp.missingFields.some(mf => mf === 'saml')) { - return signUp.authenticateWithRedirect({ - strategy: 'saml', - redirectUrl, - redirectUrlComplete, - continueSignUp: true, - }); - } - - if (signUp.unverifiedFields?.includes('email_address') && verifyEmailPath) { - return navigate(verifyEmailPath); - } - if (signUp.unverifiedFields?.includes('phone_number') && verifyPhonePath) { - return navigate(verifyPhonePath); - } - return; -}; - export const completeSignUpFlow = ({ signUp, verifyEmailPath, @@ -45,18 +19,24 @@ export const completeSignUpFlow = ({ redirectUrl = '', redirectUrlComplete = '', }: CompleteSignUpFlowProps): Promise | undefined => { - switch (signUp.status) { - case 'complete': - return handleComplete && handleComplete(); - case 'missing_requirements': - return handleMissingRequirements({ - signUp, - verifyEmailPath, - verifyPhonePath, - navigate, + if (signUp.status === 'complete') { + return handleComplete && handleComplete(); + } else if (signUp.status === 'missing_requirements') { + if (signUp.missingFields.some(mf => mf === 'saml')) { + return signUp.authenticateWithRedirect({ + strategy: 'saml', redirectUrl, redirectUrlComplete, + continueSignUp: true, }); + } + + if (signUp.unverifiedFields?.includes('email_address') && verifyEmailPath) { + return navigate(verifyEmailPath); + } + if (signUp.unverifiedFields?.includes('phone_number') && verifyPhonePath) { + return navigate(verifyPhonePath); + } } return; }; From 0e3acf8650af9cb07cb6bea3f3dd5c3776cd4610 Mon Sep 17 00:00:00 2001 From: Konstantinos Pittas Date: Tue, 22 Aug 2023 16:14:06 +0300 Subject: [PATCH 6/7] chore(clerk-js): Update changeset description --- .changeset/tender-icons-bake.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.changeset/tender-icons-bake.md b/.changeset/tender-icons-bake.md index a845151cc8..6ca460d239 100644 --- a/.changeset/tender-icons-bake.md +++ b/.changeset/tender-icons-bake.md @@ -1,2 +1,5 @@ --- +"@clerk/clerk-js": minor --- + +Improve redirects on OAuth callback. Now, if you try to sign up with a provider that allows unverified accounts, it will navigate to the appropriate change when needed, fixing the broken flow. From f0d2c6c9d427e599e7e7e5701f2bf2afc2cdd713 Mon Sep 17 00:00:00 2001 From: Konstantinos Pittas Date: Wed, 23 Aug 2023 11:13:16 +0300 Subject: [PATCH 7/7] fix(clerk-js): Update changeset description --- .changeset/tender-icons-bake.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.changeset/tender-icons-bake.md b/.changeset/tender-icons-bake.md index 6ca460d239..ceb2e4a03c 100644 --- a/.changeset/tender-icons-bake.md +++ b/.changeset/tender-icons-bake.md @@ -1,5 +1,7 @@ --- -"@clerk/clerk-js": minor +'@clerk/clerk-js': minor +'@clerk/types': minor --- -Improve redirects on OAuth callback. Now, if you try to sign up with a provider that allows unverified accounts, it will navigate to the appropriate change when needed, fixing the broken flow. +Improve redirects on OAuth callback. Now, if you try to sign up with a provider that allows unverified accounts, it will +navigate to the appropriate change when needed, fixing the broken flow.