Skip to content

Commit

Permalink
feat(clerk-js): Improve redirects on OAuth callback (#1563)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kostaspt authored Aug 23, 2023
1 parent 873dee6 commit 4764e40
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 5 deletions.
7 changes: 7 additions & 0 deletions .changeset/tender-icons-bake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@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.
60 changes: 59 additions & 1 deletion packages/clerk-js/src/core/clerk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -700,6 +707,7 @@ describe('Clerk singleton', () => {
signIn: new SignIn(null),
signUp: new SignUp({
status: 'missing_requirements',
missing_fields: [],
verifications: {
external_account: {
status: 'unverified',
Expand Down Expand Up @@ -1111,6 +1119,7 @@ describe('Clerk singleton', () => {
signIn: new SignIn(null),
signUp: new SignUp({
status: 'missing_requirements',
missing_fields: ['last_name'],
verifications: {
external_account: {
status: 'verified',
Expand All @@ -1134,6 +1143,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()', () => {
Expand Down
24 changes: 22 additions & 2 deletions packages/clerk-js/src/core/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import type {
SignOut,
SignOutCallback,
SignOutOptions,
SignUpField,
SignUpProps,
SignUpResource,
UnsubscribeCallback,
Expand All @@ -53,6 +54,7 @@ import type {
} from '@clerk/types';

import type { MountComponentRenderer } from '../ui/Components';
import { completeSignUpFlow } from '../ui/components/SignUp/util';
import {
appendAsQueryParams,
buildURL,
Expand Down Expand Up @@ -835,6 +837,7 @@ export default class Clerk implements ClerkInterface {
const { externalAccount } = signUp.verifications;
const su = {
status: signUp.status,
missingFields: signUp.missingFields,
externalAccountStatus: externalAccount.status,
externalAccountErrorCode: externalAccount.error?.code,
externalAccountSessionId: externalAccount.error?.meta?.sessionId,
Expand Down Expand Up @@ -874,6 +877,23 @@ export default class Clerk implements ClerkInterface {
buildURL({ base: displayConfig.signUpUrl, hashPath: '/continue' }, { stringify: true }),
);

const navigateToNextStepSignUp = ({ missingFields }: { missingFields: SignUpField[] }) => {
if (missingFields.length) {
return navigateToContinueSignUp();
}

return completeSignUpFlow({
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';

Expand Down Expand Up @@ -903,7 +923,7 @@ export default class Clerk implements ClerkInterface {
beforeEmit: navigateAfterSignUp,
});
case 'missing_requirements':
return navigateToContinueSignUp();
return navigateToNextStepSignUp({ missingFields: res.missingFields });
default:
clerkOAuthCallbackDidNotCompleteSignInSignUp('sign in');
}
Expand Down Expand Up @@ -939,7 +959,7 @@ export default class Clerk implements ClerkInterface {
}

if (su.externalAccountStatus === 'verified' && su.status === 'missing_requirements') {
return navigateToContinueSignUp();
return navigateToNextStepSignUp({ missingFields: signUp.missingFields });
}

return navigateToSignIn();
Expand Down
2 changes: 2 additions & 0 deletions packages/clerk-js/src/ui/components/SignUp/SignUp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ function SignUpRoutes(): JSX.Element {
redirectUrl={signUpContext.redirectUrl}
secondFactorUrl={signUpContext.secondFactorUrl}
continueSignUpUrl='../continue'
verifyEmailAddressUrl='../verify-email-address'
verifyPhoneNumberUrl='../verify-phone-number'
/>
</Route>
<Route path='verify'>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ describe('SignUpContinue', () => {
f.startSignUpWithEmailAddress();
});
render(<SignUpContinue />, { 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);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
10 changes: 10 additions & 0 deletions packages/types/src/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

/**
Expand Down

0 comments on commit 4764e40

Please sign in to comment.