From 9ab0557ce939248dd982d417fc40ec3549d4f4f5 Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 26 May 2026 19:26:00 -0400 Subject: [PATCH 1/3] refactor(ui): Make FactorTwo sub-cards prop-driven for snapshot testability CodeForm, BackupCodeCard, EmailCodeCard, and PhoneCodeCard no longer read context hooks directly. Parent containers (SignInFactorTwo, SignInClientTrust) provide attempt callbacks, prepare functions, and derived state as props. Adds 8 snapshot tests across CodeForm and BackupCodeCard. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../components/SignIn/SignInClientTrust.tsx | 49 + .../src/components/SignIn/SignInFactorTwo.tsx | 88 +- .../SignIn/SignInFactorTwoBackupCodeCard.tsx | 29 +- .../SignIn/SignInFactorTwoCodeForm.tsx | 67 +- .../SignIn/SignInFactorTwoEmailCodeCard.tsx | 14 +- .../SignIn/SignInFactorTwoPhoneCodeCard.tsx | 16 +- ...nFactorTwoBackupCodeCard.snapshot.test.tsx | 28 + .../SignInFactorTwoCodeForm.snapshot.test.tsx | 93 ++ ...orTwoBackupCodeCard.snapshot.test.tsx.snap | 355 +++++ ...InFactorTwoCodeForm.snapshot.test.tsx.snap | 1403 +++++++++++++++++ 10 files changed, 2045 insertions(+), 97 deletions(-) create mode 100644 packages/ui/src/components/SignIn/__tests__/SignInFactorTwoBackupCodeCard.snapshot.test.tsx create mode 100644 packages/ui/src/components/SignIn/__tests__/SignInFactorTwoCodeForm.snapshot.test.tsx create mode 100644 packages/ui/src/components/SignIn/__tests__/__snapshots__/SignInFactorTwoBackupCodeCard.snapshot.test.tsx.snap create mode 100644 packages/ui/src/components/SignIn/__tests__/__snapshots__/SignInFactorTwoCodeForm.snapshot.test.tsx.snap diff --git a/packages/ui/src/components/SignIn/SignInClientTrust.tsx b/packages/ui/src/components/SignIn/SignInClientTrust.tsx index ca3273fc2ff..89decd5f5f2 100644 --- a/packages/ui/src/components/SignIn/SignInClientTrust.tsx +++ b/packages/ui/src/components/SignIn/SignInClientTrust.tsx @@ -1,4 +1,8 @@ +import type { SignInFactor, SignInResource } from '@clerk/shared/types'; +import React from 'react'; + import { withCardStateProvider } from '@/ui/elements/contexts'; +import type { VerificationCodeCardProps } from '@/ui/elements/VerificationCodeCard'; import { LoadingCard } from '@/ui/elements/LoadingCard'; import { withRedirectToAfterSignIn, withRedirectToSignInTask } from '../../common'; @@ -7,10 +11,14 @@ import { SignInFactorTwoAlternativeMethods } from './SignInFactorTwoAlternativeM import { SignInFactorTwoEmailCodeCard } from './SignInFactorTwoEmailCodeCard'; import { SignInFactorTwoEmailLinkCard } from './SignInFactorTwoEmailLinkCard'; import { SignInFactorTwoPhoneCodeCard } from './SignInFactorTwoPhoneCodeCard'; +import { useHandleSecondFactorResult, useHandleUserLockedError } from './useHandleAttemptResult'; import { useSecondFactorSelection } from './useSecondFactorSelection'; +import { isResetPasswordStrategy } from './utils'; function SignInClientTrustInternal(): JSX.Element { const signIn = useCoreSignIn(); + const handleSecondFactorResult = useHandleSecondFactorResult(); + const handleUserLockedError = useHandleUserLockedError(); const { currentFactor, factorAlreadyPrepared, @@ -20,6 +28,36 @@ function SignInClientTrustInternal(): JSX.Element { toggleAllStrategies, } = useSecondFactorSelection(signIn.supportedSecondFactors); + const isResettingPassword = + isResetPasswordStrategy(signIn.firstFactorVerification?.strategy) && + signIn.firstFactorVerification?.status === 'verified'; + + const handleAttemptCode: VerificationCodeCardProps['onCodeEntryFinishedAction'] = React.useCallback( + (code: string, resolve: () => Promise, reject: (err: unknown) => void) => { + if (!currentFactor) { + return; + } + signIn + .attemptSecondFactor({ strategy: currentFactor.strategy as any, code }) + .then(async res => { + await resolve(); + return handleSecondFactorResult(res); + }) + .catch(err => { + if (handleUserLockedError(err)) { + return; + } + return reject(err); + }); + }, + [signIn, currentFactor, handleSecondFactorResult, handleUserLockedError], + ); + + const handlePrepareSecondFactor = React.useCallback( + (factor: SignInFactor) => signIn.prepareSecondFactor(factor as any), + [signIn], + ); + if (!currentFactor) { return ; } @@ -33,6 +71,13 @@ function SignInClientTrustInternal(): JSX.Element { ); } + const codeCardProps = { + onAttemptCode: handleAttemptCode, + avatarUrl: signIn.userData.imageUrl, + isResettingPassword, + showNewDeviceVerificationNotice: false, + } as const; + switch (currentFactor?.strategy) { case 'phone_code': return ( @@ -42,6 +87,8 @@ function SignInClientTrustInternal(): JSX.Element { onFactorPrepare={handleFactorPrepare} factor={currentFactor} onShowAlternativeMethodsClicked={toggleAllStrategies} + onPrepareSecondFactor={handlePrepareSecondFactor as (factor: any) => Promise} + {...codeCardProps} /> ); case 'email_code': @@ -52,6 +99,8 @@ function SignInClientTrustInternal(): JSX.Element { onFactorPrepare={handleFactorPrepare} factor={currentFactor} onShowAlternativeMethodsClicked={toggleAllStrategies} + onPrepareSecondFactor={handlePrepareSecondFactor as (factor: any) => Promise} + {...codeCardProps} /> ); case 'email_link': diff --git a/packages/ui/src/components/SignIn/SignInFactorTwo.tsx b/packages/ui/src/components/SignIn/SignInFactorTwo.tsx index 90bfddd6df7..75fd2aeac69 100644 --- a/packages/ui/src/components/SignIn/SignInFactorTwo.tsx +++ b/packages/ui/src/components/SignIn/SignInFactorTwo.tsx @@ -1,11 +1,13 @@ import { useClerk } from '@clerk/shared/react'; -import React from 'react'; +import type { SignInFactor, SignInResource } from '@clerk/shared/types'; +import React, { useMemo } from 'react'; import { withCardStateProvider } from '@/ui/elements/contexts'; +import type { VerificationCodeCardProps } from '@/ui/elements/VerificationCodeCard'; import { LoadingCard } from '@/ui/elements/LoadingCard'; import { withRedirectToAfterSignIn, withRedirectToSignInTask } from '../../common'; -import { useCoreSignIn, useSignInContext } from '../../contexts'; +import { useCoreSignIn, useEnvironment, useSignInContext } from '../../contexts'; import { useRouter } from '../../router'; import { SignInFactorTwoAlternativeMethods } from './SignInFactorTwoAlternativeMethods'; import { SignInFactorTwoBackupCodeCard } from './SignInFactorTwoBackupCodeCard'; @@ -13,13 +15,18 @@ import { SignInFactorTwoEmailCodeCard } from './SignInFactorTwoEmailCodeCard'; import { SignInFactorTwoEmailLinkCard } from './SignInFactorTwoEmailLinkCard'; import { SignInFactorTwoPhoneCodeCard } from './SignInFactorTwoPhoneCodeCard'; import { SignInFactorTwoTOTPCard } from './SignInFactorTwoTOTPCard'; +import { useHandleSecondFactorResult, useHandleUserLockedError } from './useHandleAttemptResult'; import { useSecondFactorSelection } from './useSecondFactorSelection'; +import { isResetPasswordStrategy } from './utils'; function SignInFactorTwoInternal(): JSX.Element { const clerk = useClerk(); const signIn = useCoreSignIn(); + const env = useEnvironment(); const router = useRouter(); const { afterSignInUrl } = useSignInContext(); + const handleSecondFactorResult = useHandleSecondFactorResult(); + const handleUserLockedError = useHandleUserLockedError(); const { currentFactor, factorAlreadyPrepared, @@ -29,17 +36,64 @@ function SignInFactorTwoInternal(): JSX.Element { toggleAllStrategies, } = useSecondFactorSelection(signIn.supportedSecondFactors); + const isResettingPassword = + isResetPasswordStrategy(signIn.firstFactorVerification?.strategy) && + signIn.firstFactorVerification?.status === 'verified'; + + const showNewDeviceVerificationNotice = useMemo(() => { + const anyAttributeUsedForSecondFactor = Object.values(env.userSettings.attributes).some( + attr => attr.used_for_second_factor, + ); + return signIn.clientTrustState === 'new' && !anyAttributeUsedForSecondFactor; + }, [signIn.clientTrustState, env.userSettings.attributes]); + + const handleAttemptCode: VerificationCodeCardProps['onCodeEntryFinishedAction'] = React.useCallback( + (code: string, resolve: () => Promise, reject: (err: unknown) => void) => { + if (!currentFactor) { + return; + } + signIn + .attemptSecondFactor({ strategy: currentFactor.strategy as any, code }) + .then(async res => { + await resolve(); + return handleSecondFactorResult(res); + }) + .catch(err => { + if (handleUserLockedError(err)) { + return; + } + return reject(err); + }); + }, + [signIn, currentFactor, handleSecondFactorResult, handleUserLockedError], + ); + + const handleAttemptBackupCode = React.useCallback( + async (code: string) => { + try { + const res = await signIn.attemptSecondFactor({ strategy: 'backup_code', code }); + await handleSecondFactorResult(res); + } catch (err) { + if (handleUserLockedError(err)) { + return; + } + throw err; + } + }, + [signIn, handleSecondFactorResult, handleUserLockedError], + ); + + const handlePrepareSecondFactor = React.useCallback( + (factor: SignInFactor) => signIn.prepareSecondFactor(factor as any), + [signIn], + ); + React.useEffect(() => { if (clerk.__internal_setActiveInProgress) { return; } - // If the sign-in doesn't need second factor verification, redirect away. - // Don't redirect for 'complete' status - setActive will handle navigation. if (signIn.status === null || signIn.status === 'needs_identifier' || signIn.status === 'needs_first_factor') { - // If the user is already signed in (e.g. multi-session app, page reload after - // successful verification), redirect forward to afterSignInUrl instead of - // back to sign-in start. if (clerk.isSignedIn) { void router.navigate(afterSignInUrl); } else { @@ -62,6 +116,13 @@ function SignInFactorTwoInternal(): JSX.Element { ); } + const codeCardProps = { + onAttemptCode: handleAttemptCode, + avatarUrl: signIn.userData.imageUrl, + isResettingPassword, + showNewDeviceVerificationNotice, + } as const; + switch (currentFactor?.strategy) { case 'phone_code': return ( @@ -70,6 +131,8 @@ function SignInFactorTwoInternal(): JSX.Element { onFactorPrepare={handleFactorPrepare} factor={currentFactor} onShowAlternativeMethodsClicked={toggleAllStrategies} + onPrepareSecondFactor={handlePrepareSecondFactor as (factor: any) => Promise} + {...codeCardProps} /> ); case 'totp': @@ -79,10 +142,17 @@ function SignInFactorTwoInternal(): JSX.Element { onFactorPrepare={handleFactorPrepare} factor={currentFactor} onShowAlternativeMethodsClicked={toggleAllStrategies} + {...codeCardProps} /> ); case 'backup_code': - return ; + return ( + + ); case 'email_code': return ( Promise} + {...codeCardProps} /> ); case 'email_link': diff --git a/packages/ui/src/components/SignIn/SignInFactorTwoBackupCodeCard.tsx b/packages/ui/src/components/SignIn/SignInFactorTwoBackupCodeCard.tsx index 47c28ff912b..9efb873f6cf 100644 --- a/packages/ui/src/components/SignIn/SignInFactorTwoBackupCodeCard.tsx +++ b/packages/ui/src/components/SignIn/SignInFactorTwoBackupCodeCard.tsx @@ -1,4 +1,3 @@ -import type { SignInResource } from '@clerk/shared/types'; import React from 'react'; import { Card } from '@/ui/elements/Card'; @@ -8,42 +7,28 @@ import { Header } from '@/ui/elements/Header'; import { handleError } from '@/ui/utils/errorHandler'; import { useFormControl } from '@/ui/utils/useFormControl'; -import { useCoreSignIn } from '../../contexts'; import { Col, descriptors, localizationKeys } from '../../customizables'; -import { useHandleSecondFactorResult, useHandleUserLockedError } from './useHandleAttemptResult'; -import { isResetPasswordStrategy } from './utils'; type SignInFactorTwoBackupCodeCardProps = { onShowAlternativeMethodsClicked: React.MouseEventHandler; + onAttemptBackupCode: (code: string) => Promise; + isResettingPassword: boolean; }; export const SignInFactorTwoBackupCodeCard = (props: SignInFactorTwoBackupCodeCardProps) => { - const { onShowAlternativeMethodsClicked } = props; - const signIn = useCoreSignIn(); + const { onShowAlternativeMethodsClicked, onAttemptBackupCode, isResettingPassword } = props; const card = useCardState(); const codeControl = useFormControl('code', '', { type: 'text', label: localizationKeys('formFieldLabel__backupCode'), isRequired: true, }); - const handleSecondFactorResult = useHandleSecondFactorResult(); - const handleUserLockedError = useHandleUserLockedError(); - - const isResettingPassword = (resource: SignInResource) => - isResetPasswordStrategy(resource.firstFactorVerification?.strategy) && - resource.firstFactorVerification?.status === 'verified'; const handleBackupCodeSubmit: React.FormEventHandler = e => { e.preventDefault(); - return signIn - .attemptSecondFactor({ strategy: 'backup_code', code: codeControl.value }) - .then(handleSecondFactorResult) - .catch(err => { - if (handleUserLockedError(err)) { - return; - } - handleError(err, [codeControl], card.setError); - }); + return onAttemptBackupCode(codeControl.value).catch(err => { + handleError(err, [codeControl], card.setError); + }); }; return ( @@ -53,7 +38,7 @@ export const SignInFactorTwoBackupCodeCard = (props: SignInFactorTwoBackupCodeCa & { showClientTrustNotice?: boolean; @@ -18,6 +15,10 @@ export type SignInFactorTwoCodeCard = Pick void; prepare?: () => Promise; + onAttemptCode: VerificationCodeCardProps['onCodeEntryFinishedAction']; + avatarUrl: string | undefined; + isResettingPassword: boolean; + showNewDeviceVerificationNotice: boolean; }; type SignInFactorTwoCodeFormProps = SignInFactorTwoCodeCard & { @@ -27,84 +28,44 @@ type SignInFactorTwoCodeFormProps = SignInFactorTwoCodeCard & { resendButton?: LocalizationKey; }; -const isResettingPassword = (resource: SignInResource) => - isResetPasswordStrategy(resource.firstFactorVerification?.strategy) && - resource.firstFactorVerification?.status === 'verified'; - export const SignInFactorTwoCodeForm = (props: SignInFactorTwoCodeFormProps) => { - const env = useEnvironment(); - const signIn = useCoreSignIn(); const card = useCardState(); - const handleSecondFactorResult = useHandleSecondFactorResult(); - const handleUserLockedError = useHandleUserLockedError(); - - // Only show the new device verification notice if the user is new - // and no attributes are explicitly used for second factor. - // Retained for backwards compatibility. - const showNewDeviceVerificationNotice = useMemo(() => { - const anyAttributeUsedForSecondFactor = Object.values(env.userSettings.attributes).some( - attr => attr.used_for_second_factor, - ); - return signIn.clientTrustState === 'new' && !anyAttributeUsedForSecondFactor; - }, [signIn.clientTrustState, env.userSettings.attributes]); React.useEffect(() => { if (props.factorAlreadyPrepared) { return; } - void prepare?.(); + void prepareWrapped?.(); }, []); - const prepare = props.prepare + const prepareWrapped = props.prepare ? () => { return props .prepare?.() .then(() => props.onFactorPrepare()) - .catch(err => { - if (handleUserLockedError(err)) { - return; - } - handleError(err, [], card.setError); - }); + .catch(err => handleError(err, [], card.setError)); } : undefined; - const action: VerificationCodeCardProps['onCodeEntryFinishedAction'] = (code, resolve, reject) => { - signIn - .attemptSecondFactor({ strategy: props.factor.strategy, code }) - .then(async res => { - await resolve(); - return handleSecondFactorResult(res); - }) - .catch(err => { - if (handleUserLockedError(err)) { - return; - } - return reject(err); - }); - }; - return ( - {isResettingPassword(signIn) && ( + {props.isResettingPassword && ( Promise; +}; export const SignInFactorTwoEmailCodeCard = (props: SignInFactorTwoEmailCodeCardProps) => { - const signIn = useCoreSignIn(); + const { onPrepareSecondFactor, ...rest } = props; const prepare = () => { const { emailAddressId, strategy } = props.factor; - return signIn.prepareSecondFactor({ emailAddressId, strategy }); + return onPrepareSecondFactor({ emailAddressId, strategy } as EmailCodeFactor); }; return ( Promise; +}; export const SignInFactorTwoPhoneCodeCard = (props: SignInFactorTwoPhoneCodeCardProps) => { - const signIn = useCoreSignIn(); + const { onPrepareSecondFactor, ...rest } = props; const prepare = () => { - // TODO: Why does the BE throw an error if I simply pass - // the whole factor? const { phoneNumberId, strategy } = props.factor; - return signIn.prepareSecondFactor({ phoneNumberId, strategy }); + return onPrepareSecondFactor({ phoneNumberId, strategy } as PhoneCodeFactor); }; return ( { + const defaultProps = { + onShowAlternativeMethodsClicked: vi.fn() as any, + onAttemptBackupCode: vi.fn().mockResolvedValue(undefined), + isResettingPassword: false, + }; + + it('renders default state', () => { + const { container } = renderForSnapshot(); + expect(container).toMatchSnapshot(); + }); + + it('renders with reset password subtitle', () => { + const { container } = renderForSnapshot( + , + ); + expect(container).toMatchSnapshot(); + }); +}); diff --git a/packages/ui/src/components/SignIn/__tests__/SignInFactorTwoCodeForm.snapshot.test.tsx b/packages/ui/src/components/SignIn/__tests__/SignInFactorTwoCodeForm.snapshot.test.tsx new file mode 100644 index 00000000000..8f3bccd9a86 --- /dev/null +++ b/packages/ui/src/components/SignIn/__tests__/SignInFactorTwoCodeForm.snapshot.test.tsx @@ -0,0 +1,93 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { renderForSnapshot } from '@/test/render-for-snapshot'; + +import { localizationKeys } from '../../../localization'; +import { SignInFactorTwoCodeForm } from '../SignInFactorTwoCodeForm'; + +describe('SignInFactorTwoCodeForm snapshots', () => { + const defaultProps = { + factor: { + strategy: 'phone_code' as const, + phoneNumberId: 'pn_123', + safeIdentifier: '+1 (555) 123-4567', + }, + factorAlreadyPrepared: true, + onFactorPrepare: vi.fn(), + onAttemptCode: vi.fn(), + avatarUrl: undefined, + isResettingPassword: false, + showNewDeviceVerificationNotice: false, + cardTitle: localizationKeys('signIn.phoneCodeMfa.title'), + cardSubtitle: localizationKeys('signIn.phoneCodeMfa.subtitle'), + inputLabel: localizationKeys('signIn.phoneCodeMfa.formTitle'), + resendButton: localizationKeys('signIn.phoneCodeMfa.resendButton'), + }; + + it('renders phone code MFA card', () => { + const { container } = renderForSnapshot(); + expect(container).toMatchSnapshot(); + }); + + it('renders with reset password subtitle', () => { + const { container } = renderForSnapshot( + , + ); + expect(container).toMatchSnapshot(); + }); + + it('renders with new device verification notice', () => { + const { container } = renderForSnapshot( + , + ); + expect(container).toMatchSnapshot(); + }); + + it('renders with client trust notice', () => { + const { container } = renderForSnapshot( + , + ); + expect(container).toMatchSnapshot(); + }); + + it('renders TOTP card', () => { + const { container } = renderForSnapshot( + , + ); + expect(container).toMatchSnapshot(); + }); + + it('renders email code MFA card', () => { + const { container } = renderForSnapshot( + , + ); + expect(container).toMatchSnapshot(); + }); +}); diff --git a/packages/ui/src/components/SignIn/__tests__/__snapshots__/SignInFactorTwoBackupCodeCard.snapshot.test.tsx.snap b/packages/ui/src/components/SignIn/__tests__/__snapshots__/SignInFactorTwoBackupCodeCard.snapshot.test.tsx.snap new file mode 100644 index 00000000000..2cd18e0a269 --- /dev/null +++ b/packages/ui/src/components/SignIn/__tests__/__snapshots__/SignInFactorTwoBackupCodeCard.snapshot.test.tsx.snap @@ -0,0 +1,355 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`SignInFactorTwoBackupCodeCard snapshots > renders default state 1`] = ` +
+
+
+
+
+ + TestApp + +
+
+

+ Enter a backup code +

+

+ Your backup code is the one you got when setting up two-step authentication. +

+
+
+
+
+ +
+ + Use another method + +
+
+ +
+
+
+
+
+
+
+

+ Secured by +

+ + + +
+
+
+
+
+
+ +`; + +exports[`SignInFactorTwoBackupCodeCard snapshots > renders with reset password subtitle 1`] = ` +
+
+
+
+
+ + TestApp + +
+
+

+ Enter a backup code +

+

+ to reset your password +

+
+
+
+
+ +
+ + Use another method + +
+
+ +
+
+
+
+
+
+
+

+ Secured by +

+ + + +
+
+
+
+
+
+ +`; diff --git a/packages/ui/src/components/SignIn/__tests__/__snapshots__/SignInFactorTwoCodeForm.snapshot.test.tsx.snap b/packages/ui/src/components/SignIn/__tests__/__snapshots__/SignInFactorTwoCodeForm.snapshot.test.tsx.snap new file mode 100644 index 00000000000..7fde276d413 --- /dev/null +++ b/packages/ui/src/components/SignIn/__tests__/__snapshots__/SignInFactorTwoCodeForm.snapshot.test.tsx.snap @@ -0,0 +1,1403 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`SignInFactorTwoCodeForm snapshots > renders TOTP card 1`] = ` +
+
+
+
+
+

+ Two-step verification +

+

+ To continue, please enter the verification code generated by your authenticator app +

+
+
+
+
+
+
+
+