Skip to content

Commit 846a145

Browse files
authored
fix(ui): avoid re-preparing pending code verifications (#8548)
1 parent 1701e0f commit 846a145

7 files changed

Lines changed: 105 additions & 15 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/ui': patch
3+
---
4+
5+
Avoid sending duplicate verification codes when persisted email or phone code verifications are already pending.

integration/tests/email-code.test.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { test } from '@playwright/test';
1+
import { expect, test } from '@playwright/test';
22

33
import type { Application } from '../models/application';
44
import { appConfigs } from '../presets';
@@ -39,6 +39,40 @@ test.describe('sign up and sign in with email code @generic', () => {
3939
await u.po.expect.toBeSignedIn();
4040
});
4141

42+
test('does not re-send the email code when the verification step is reloaded', async ({ page, context }) => {
43+
const u = createTestUtils({ app, page, context });
44+
const reloadUser = createTestUtils({ app }).services.users.createFakeUser({ fictionalEmail: true });
45+
46+
try {
47+
await u.po.signUp.goTo();
48+
await u.po.signUp.signUpWithEmailAndPassword({
49+
email: reloadUser.email,
50+
password: reloadUser.password,
51+
});
52+
await u.po.signUp.waitForEmailVerificationScreen();
53+
54+
// Count any /prepare_verification calls that happen after we land on the OTP screen.
55+
// Before the fix, reloading would re-mount SignUpEmailCodeCard and trigger a duplicate prepare.
56+
let prepareCount = 0;
57+
await page.context().route('**/prepare_verification*', async route => {
58+
prepareCount += 1;
59+
await route.continue();
60+
});
61+
62+
await page.reload();
63+
await u.po.signUp.waitForEmailVerificationScreen();
64+
// Give clerk-js time to rehydrate the persisted sign-up and (formerly) call prepare on mount.
65+
await page.waitForTimeout(1000);
66+
67+
expect(prepareCount).toBe(0);
68+
69+
await u.po.signUp.enterTestOtpCode({ awaitPrepare: false });
70+
await u.po.expect.toBeSignedIn();
71+
} finally {
72+
await reloadUser.deleteIfExists();
73+
}
74+
});
75+
4276
// TODO: remove
4377
test.skip('sign in has been moved to sign in flow test suite, this will be removed', async () => {});
4478
});

packages/ui/src/components/SignIn/SignInFactorOneCodeForm.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,16 @@ export const SignInFactorOneCodeForm = (props: SignInFactorOneCodeFormProps) =>
4040
const supportEmail = useSupportEmail();
4141
const clerk = useClerk();
4242

43+
const factorChannel = 'channel' in props.factor ? props.factor.channel : undefined;
44+
const normalizedFactorChannel = factorChannel === 'sms' ? undefined : factorChannel;
45+
const normalizedVerificationChannel =
46+
signIn.firstFactorVerification.channel === 'sms' ? undefined : signIn.firstFactorVerification.channel;
47+
const hasPendingFactorVerification =
48+
signIn.firstFactorVerification.status === 'unverified' &&
49+
signIn.firstFactorVerification.strategy === props.factor.strategy &&
50+
normalizedFactorChannel === normalizedVerificationChannel;
4351
const shouldAvoidPrepare = signIn.firstFactorVerification.status === 'verified' && props.factorAlreadyPrepared;
52+
const shouldAvoidInitialPrepare = shouldAvoidPrepare || hasPendingFactorVerification;
4453

4554
const cacheKey = useMemo(() => {
4655
const factor = props.factor;
@@ -82,7 +91,7 @@ export const SignInFactorOneCodeForm = (props: SignInFactorOneCodeFormProps) =>
8291
.catch(err => handleError(err, [], card.setError));
8392
};
8493

85-
useFetch(shouldAvoidPrepare ? undefined : () => signIn?.prepareFirstFactor(props.factor), cacheKey, {
94+
useFetch(shouldAvoidInitialPrepare ? undefined : () => signIn?.prepareFirstFactor(props.factor), cacheKey, {
8695
staleTime: 100,
8796
onSuccess: () => props.onFactorPrepare(),
8897
onError: err => handleError(err, [], card.setError),

packages/ui/src/components/SignIn/__tests__/SignInFactorOneCodeForm.test.tsx

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -102,19 +102,22 @@ describe('SignInFactorOneCodeForm', () => {
102102
);
103103
});
104104

105-
it('still calls prepare when factor is already prepared but verification not verified', async () => {
106-
const { wrapper } = await createFixtures(f => {
105+
it('skips automatic prepare when the same factor verification is pending', async () => {
106+
const { wrapper, fixtures } = await createFixtures(f => {
107107
f.withPhoneNumber();
108108
f.startSignInWithPhoneNumber({ supportPhoneCode: true });
109109
});
110+
fixtures.signIn.firstFactorVerification.status = 'unverified';
111+
fixtures.signIn.firstFactorVerification.strategy = 'phone_code';
112+
fixtures.signIn.firstFactorVerification.channel = 'sms';
110113

111114
const props = {
112115
factor: {
113116
strategy: 'phone_code' as const,
114117
phoneNumberId: 'idn_123',
115118
safeIdentifier: '+1234567890',
116119
},
117-
factorAlreadyPrepared: true,
120+
factorAlreadyPrepared: false,
118121
onFactorPrepare: vi.fn(),
119122
cardTitle: localizationKeys('signIn.phoneCode.title'),
120123
cardSubtitle: localizationKeys('signIn.phoneCode.subtitle'),
@@ -124,16 +127,18 @@ describe('SignInFactorOneCodeForm', () => {
124127

125128
renderWithProviders(<SignInFactorOneCodeForm {...props} />, { wrapper });
126129

127-
expect(vi.mocked(useFetch)).toHaveBeenCalledWith(expect.any(Function), expect.any(Object), expect.any(Object));
130+
expect(vi.mocked(useFetch)).toHaveBeenCalledWith(undefined, expect.any(Object), expect.any(Object));
128131
});
129132
});
130133

131134
describe('shouldAvoidPrepare Logic', () => {
132-
it('still calls prepare when factor is already prepared but verification not verified', async () => {
133-
const { wrapper } = await createFixtures(f => {
135+
it('allows prepare when the same factor verification is expired', async () => {
136+
const { wrapper, fixtures } = await createFixtures(f => {
134137
f.withPhoneNumber();
135138
f.startSignInWithPhoneNumber({ supportPhoneCode: true });
136139
});
140+
fixtures.signIn.firstFactorVerification.status = 'expired';
141+
fixtures.signIn.firstFactorVerification.strategy = 'phone_code';
137142

138143
const propsWithFactorPrepared = {
139144
...defaultProps,
@@ -142,11 +147,7 @@ describe('SignInFactorOneCodeForm', () => {
142147

143148
renderWithProviders(<SignInFactorOneCodeForm {...propsWithFactorPrepared} />, { wrapper });
144149

145-
expect(vi.mocked(useFetch)).toHaveBeenCalledWith(
146-
expect.any(Function), // fetcher should still be a function because shouldAvoidPrepare requires BOTH conditions
147-
expect.any(Object),
148-
expect.any(Object),
149-
);
150+
expect(vi.mocked(useFetch)).toHaveBeenCalledWith(expect.any(Function), expect.any(Object), expect.any(Object));
150151
});
151152

152153
it('allows prepare when factor is not already prepared', async () => {

packages/ui/src/components/SignUp/SignUpEmailCodeCard.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ export const SignUpEmailCodeCard = () => {
1111
const card = useCardState();
1212

1313
const emailVerificationStatus = signUp.verifications.emailAddress.status;
14+
const hasPendingEmailCodeVerification =
15+
emailVerificationStatus === 'unverified' && signUp.verifications.emailAddress.strategy === 'email_code';
1416
const shouldAvoidPrepare = !signUp.status || emailVerificationStatus === 'verified';
17+
const shouldAvoidInitialPrepare = shouldAvoidPrepare || hasPendingEmailCodeVerification;
1518

1619
const prepare = () => {
1720
if (shouldAvoidPrepare) {
@@ -24,7 +27,7 @@ export const SignUpEmailCodeCard = () => {
2427

2528
// TODO: Introduce a useMutation to handle mutating requests
2629
useFetch(
27-
shouldAvoidPrepare ? undefined : () => signUp.prepareEmailAddressVerification({ strategy: 'email_code' }),
30+
shouldAvoidInitialPrepare ? undefined : () => signUp.prepareEmailAddressVerification({ strategy: 'email_code' }),
2831
{
2932
name: 'prepare',
3033
strategy: 'email_code',

packages/ui/src/components/SignUp/SignUpPhoneCodeCard.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ export const SignUpPhoneCodeCard = withCardStateProvider(() => {
1515
const channel = signUp.verifications.phoneNumber.channel;
1616

1717
const phoneVerificationStatus = signUp.verifications.phoneNumber.status;
18+
const hasPendingPhoneCodeVerification =
19+
phoneVerificationStatus === 'unverified' && signUp.verifications.phoneNumber.strategy === 'phone_code';
1820
const shouldAvoidPrepare = !signUp.status || phoneVerificationStatus === 'verified';
1921
const isAlternativePhoneCodeProvider = !!channel && channel !== 'sms';
2022

@@ -34,7 +36,7 @@ export const SignUpPhoneCodeCard = withCardStateProvider(() => {
3436
useFetch(
3537
// If an alternative phone code provider is used, we skip the prepare step
3638
// because the verification is already created on the Start screen
37-
shouldAvoidPrepare || isAlternativePhoneCodeProvider
39+
shouldAvoidPrepare || hasPendingPhoneCodeVerification || isAlternativePhoneCodeProvider
3840
? undefined
3941
: () => signUp.preparePhoneNumberVerification({ strategy: 'phone_code', channel: undefined }),
4042
{

packages/ui/src/components/SignUp/__tests__/SignUpVerifyEmail.test.tsx

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,42 @@ describe('SignUpVerifyEmail', () => {
5757
);
5858
});
5959

60+
it('does not prepare a new email code when the persisted email verification is pending', async () => {
61+
const { wrapper, fixtures } = await createFixtures(f => {
62+
f.withEmailAddress({ required: true, verifications: ['email_code'] });
63+
f.startSignUpWithEmailAddress({
64+
emailAddress: 'pending-code@clerk.com',
65+
supportEmailLink: false,
66+
supportEmailCode: true,
67+
});
68+
});
69+
fixtures.signUp.prepareEmailAddressVerification.mockResolvedValue(fixtures.signUp);
70+
71+
const { findByText } = render(<SignUpVerifyEmail />, { wrapper });
72+
await waitFor(async () => expect(await findByText(/Verify your email/i)).toBeInTheDocument());
73+
74+
expect(fixtures.signUp.prepareEmailAddressVerification).not.toHaveBeenCalled();
75+
});
76+
77+
it('prepares a new email code when the persisted email verification is expired', async () => {
78+
const { wrapper, fixtures } = await createFixtures(f => {
79+
f.withEmailAddress({ required: true, verifications: ['email_code'] });
80+
f.startSignUpWithEmailAddress({
81+
emailAddress: 'expired-code@clerk.com',
82+
supportEmailLink: false,
83+
supportEmailCode: true,
84+
emailVerificationStatus: 'expired',
85+
});
86+
});
87+
fixtures.signUp.prepareEmailAddressVerification.mockRejectedValue(null);
88+
89+
render(<SignUpVerifyEmail />, { wrapper });
90+
91+
await waitFor(() =>
92+
expect(fixtures.signUp.prepareEmailAddressVerification).toHaveBeenCalledWith({ strategy: 'email_code' }),
93+
);
94+
});
95+
6096
it('clicking on the edit icon navigates to the previous route', async () => {
6197
const { wrapper, fixtures } = await createFixtures(f => {
6298
f.withEmailAddress({ required: true });

0 commit comments

Comments
 (0)