From 9d7082e7219542b6b2b734d6fae5c68afd0bf3b9 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Fri, 14 Feb 2025 18:01:32 +0200 Subject: [PATCH 1/9] fix(clerk-js): Correctly show alternative verification methods --- .../UserVerification/AlternativeMethods.tsx | 6 ++- .../UVFactorTwoPhoneCodeCard.tsx | 1 - .../UserVerificationFactorOne.tsx | 6 ++- .../src/ui/hooks/useAlternativeStrategies.ts | 42 +++++++++++++++++++ 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx b/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx index a3b1b390eeb..b4a7dfca433 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx @@ -5,7 +5,7 @@ import type { LocalizationKey } from '../../customizables'; import { Col, descriptors, Flex, Flow, localizationKeys } from '../../customizables'; import { ArrowBlockButton, BackLink, Card, Header } from '../../elements'; import { useCardState } from '../../elements/contexts'; -import { useAlternativeStrategies } from '../../hooks/useAlternativeStrategies'; +import { useReverificationAlternativeStrategies } from '../../hooks/useAlternativeStrategies'; import { ChatAltIcon, Email, LockClosedIcon } from '../../icons'; import { formatSafeIdentifier } from '../../utils'; import { useUserVerificationSession } from './useUserVerificationSession'; @@ -29,11 +29,13 @@ const AlternativeMethodsList = (props: AlternativeMethodListProps) => { const { onBackLinkClick, onHavingTroubleClick, onFactorSelected } = props; const card = useCardState(); const { data } = useUserVerificationSession(); - const { firstPartyFactors, hasAnyStrategy } = useAlternativeStrategies({ + const { firstPartyFactors, hasAnyStrategy } = useReverificationAlternativeStrategies({ filterOutFactor: props?.currentFactor, supportedFirstFactors: data?.supportedFirstFactors, }); + console.log('currentFactor -fa', firstPartyFactors); + return ( diff --git a/packages/clerk-js/src/ui/components/UserVerification/UVFactorTwoPhoneCodeCard.tsx b/packages/clerk-js/src/ui/components/UserVerification/UVFactorTwoPhoneCodeCard.tsx index cd790c3e497..3b140106a23 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UVFactorTwoPhoneCodeCard.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UVFactorTwoPhoneCodeCard.tsx @@ -6,7 +6,6 @@ import type { UVFactorTwoCodeCard } from './UVFactorTwoCodeForm'; import { UVFactorTwoCodeForm } from './UVFactorTwoCodeForm'; type UVFactorTwoPhoneCodeCardProps = UVFactorTwoCodeCard & { factor: PhoneCodeFactor }; - export const UVFactorTwoPhoneCodeCard = (props: UVFactorTwoPhoneCodeCardProps) => { const { session } = useSession(); diff --git a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorOne.tsx b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorOne.tsx index 5a1aceabd0d..ab8f31262f8 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorOne.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorOne.tsx @@ -3,7 +3,7 @@ import React, { useEffect } from 'react'; import { useEnvironment } from '../../contexts'; import { ErrorCard, LoadingCard, useCardState, withCardStateProvider } from '../../elements'; -import { useAlternativeStrategies } from '../../hooks/useAlternativeStrategies'; +import { useReverificationAlternativeStrategies } from '../../hooks/useAlternativeStrategies'; import { localizationKeys } from '../../localization'; import { useRouter } from '../../router'; import { determineStartingSignInFactor, factorHasLocalStrategy } from '../SignIn/utils'; @@ -46,7 +46,7 @@ export function _UserVerificationFactorOne(): JSX.Element | null { prevCurrentFactor: undefined, })); - const { hasAnyStrategy } = useAlternativeStrategies({ + const { hasAnyStrategy, hasFirstParty } = useReverificationAlternativeStrategies({ filterOutFactor: currentFactor, supportedFirstFactors: availableFactors, }); @@ -116,6 +116,7 @@ export function _UserVerificationFactorOne(): JSX.Element | null { onFactorPrepare={handleFactorPrepare} onShowAlternativeMethodsClicked={toggleAllStrategies} factor={currentFactor} + showAlternativeMethods={hasFirstParty} /> ); case 'phone_code': @@ -125,6 +126,7 @@ export function _UserVerificationFactorOne(): JSX.Element | null { onFactorPrepare={handleFactorPrepare} onShowAlternativeMethodsClicked={toggleAllStrategies} factor={currentFactor} + showAlternativeMethods={hasFirstParty} /> ); default: diff --git a/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts b/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts index 1d8c814eb7c..9a5288231db 100644 --- a/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts +++ b/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts @@ -35,3 +35,45 @@ export function useAlternativeStrategies({ firstPartyFactors, }; } + +export function useReverificationAlternativeStrategies({ + filterOutFactor, + supportedFirstFactors: _supportedFirstFactors, +}: { + filterOutFactor: SignInFactor | null | undefined; + supportedFirstFactors: SignInFirstFactor[] | null | undefined; +}) { + const { strategies: OAuthStrategies } = useEnabledThirdPartyProviders(); + const supportedFirstFactors = _supportedFirstFactors || []; + + const firstFactors = supportedFirstFactors.filter( + f => f.strategy !== filterOutFactor?.strategy && !isResetPasswordStrategy(f.strategy), + ); + + const shouldAllowForAlternativeStrategies = firstFactors.length + OAuthStrategies.length > 0; + + const firstPartyFactors = supportedFirstFactors + .filter(f => !f.strategy.startsWith('oauth_')) + .filter(f => { + if ( + f.strategy === 'email_code' && + filterOutFactor?.strategy === 'email_code' && + filterOutFactor.emailAddressId === f.emailAddressId + ) { + return false; + } + + return f.strategy === filterOutFactor?.strategy; + }) + .filter(factor => factorHasLocalStrategy(factor)) + // Only include passkey if the device supports it. + // @ts-ignore Types are not public yet. + .filter(factor => (factor.strategy === 'passkey' ? isWebAuthnSupported() : true)) + .sort(allStrategiesButtonsComparator) as T[]; + + return { + hasAnyStrategy: shouldAllowForAlternativeStrategies, + hasFirstParty: firstPartyFactors.length > 0, + firstPartyFactors, + }; +} From c92a0a33c741e4628b98b37e6a7145c5f7b5b71e Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Wed, 19 Feb 2025 17:00:45 +0200 Subject: [PATCH 2/9] Update packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx --- .../src/ui/components/UserVerification/AlternativeMethods.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx b/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx index b4a7dfca433..785cf5cc1b8 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx @@ -34,7 +34,6 @@ const AlternativeMethodsList = (props: AlternativeMethodListProps) => { supportedFirstFactors: data?.supportedFirstFactors, }); - console.log('currentFactor -fa', firstPartyFactors); return ( From 18c99319fdc56cac54df153fc6ed7908a79a8a46 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Fri, 21 Feb 2025 12:54:19 +0200 Subject: [PATCH 3/9] fix(clerk-js): Correctly show alternative methods for user re-verification --- .../UserVerification/AlternativeMethods.tsx | 1 - .../UserVerification/UVFactorOneCodeForm.tsx | 28 ++++++++++--------- .../UVFactorOnePhoneCodeCard.tsx | 1 + .../UserVerification/UVFactorTwoCodeForm.tsx | 2 ++ .../UserVerificationFactorTwo.tsx | 6 ++++ .../UserVerificationFactorTwoTOTP.tsx | 1 + .../src/ui/hooks/useAlternativeStrategies.ts | 20 +++---------- 7 files changed, 29 insertions(+), 30 deletions(-) diff --git a/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx b/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx index 785cf5cc1b8..472d4a7c6b8 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx @@ -34,7 +34,6 @@ const AlternativeMethodsList = (props: AlternativeMethodListProps) => { supportedFirstFactors: data?.supportedFirstFactors, }); - return ( diff --git a/packages/clerk-js/src/ui/components/UserVerification/UVFactorOneCodeForm.tsx b/packages/clerk-js/src/ui/components/UserVerification/UVFactorOneCodeForm.tsx index 94290e55852..572ebac705a 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UVFactorOneCodeForm.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UVFactorOneCodeForm.tsx @@ -54,18 +54,20 @@ export const UVFactorOneCodeForm = (props: UVFactorOneCodeFormProps) => { }; return ( - +
+ +
); }; diff --git a/packages/clerk-js/src/ui/components/UserVerification/UVFactorOnePhoneCodeCard.tsx b/packages/clerk-js/src/ui/components/UserVerification/UVFactorOnePhoneCodeCard.tsx index a2210f7dd37..8d264993871 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UVFactorOnePhoneCodeCard.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UVFactorOnePhoneCodeCard.tsx @@ -15,6 +15,7 @@ export const UVFactorOnePhoneCodeCard = (props: UVFactorOnePhoneCodeCardProps) = cardSubtitle={localizationKeys('reverification.phoneCode.subtitle')} inputLabel={localizationKeys('reverification.phoneCode.formTitle')} resendButton={localizationKeys('reverification.phoneCode.resendButton')} + showAlternativeMethods={props.showAlternativeMethods} />
); diff --git a/packages/clerk-js/src/ui/components/UserVerification/UVFactorTwoCodeForm.tsx b/packages/clerk-js/src/ui/components/UserVerification/UVFactorTwoCodeForm.tsx index b931350c460..4810868a362 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UVFactorTwoCodeForm.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UVFactorTwoCodeForm.tsx @@ -13,6 +13,7 @@ export type UVFactorTwoCodeCard = Pick void; prepare?: () => Promise; + showAlternativeMethods?: boolean; }; type SignInFactorTwoCodeFormProps = UVFactorTwoCodeCard & { @@ -64,6 +65,7 @@ export const UVFactorTwoCodeForm = (props: SignInFactorTwoCodeFormProps) => { safeIdentifier={'safeIdentifier' in props.factor ? props.factor.safeIdentifier : undefined} profileImageUrl={session?.user?.imageUrl} onShowAlternativeMethodsClicked={props.onShowAlternativeMethodsClicked} + showAlternativeMethods={props.showAlternativeMethods} /> ); }; diff --git a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx index efccbc8996b..ec4ed4c0c22 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx @@ -1,3 +1,4 @@ +import { isDeeplyEqual } from '@clerk/shared/react/index'; import type { SessionVerificationResource, SessionVerificationSecondFactor, SignInFactor } from '@clerk/types'; import React, { useEffect } from 'react'; @@ -44,6 +45,9 @@ export function _UserVerificationFactorTwo(): JSX.Element { toggleAllStrategies(); }; + const hasAlternativeStrategies = + (availableFactors && availableFactors.filter(factor => isDeeplyEqual(factor, currentFactor)).length > 0) || false; + useEffect(() => { if (sessionVerification.status === 'needs_first_factor') { void navigate('../'); @@ -72,6 +76,7 @@ export function _UserVerificationFactorTwo(): JSX.Element { onFactorPrepare={handleFactorPrepare} factor={currentFactor} onShowAlternativeMethodsClicked={toggleAllStrategies} + showAlternativeMethods={hasAlternativeStrategies} /> ); case 'totp': @@ -81,6 +86,7 @@ export function _UserVerificationFactorTwo(): JSX.Element { onFactorPrepare={handleFactorPrepare} factor={currentFactor} onShowAlternativeMethodsClicked={toggleAllStrategies} + showAlternativeMethods={hasAlternativeStrategies} /> ); case 'backup_code': diff --git a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwoTOTP.tsx b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwoTOTP.tsx index 238bb50fd1e..6385f2f6c7a 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwoTOTP.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwoTOTP.tsx @@ -14,6 +14,7 @@ export function UserVerificationFactorTwoTOTP(props: UVFactorTwoTOTPCardProps): cardTitle={localizationKeys('reverification.totpMfa.title')} cardSubtitle={localizationKeys('reverification.totpMfa.subtitle')} inputLabel={localizationKeys('reverification.totpMfa.formTitle')} + showAlternativeMethods={props.showAlternativeMethods} />
); diff --git a/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts b/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts index 9a5288231db..89858be68bf 100644 --- a/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts +++ b/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts @@ -1,3 +1,4 @@ +import { isDeeplyEqual } from '@clerk/shared/react/index'; import { isWebAuthnSupported } from '@clerk/shared/webauthn'; import type { SignInFactor, SignInFirstFactor } from '@clerk/types'; @@ -43,29 +44,16 @@ export function useReverificationAlternativeStrategies({ filterOutFactor: SignInFactor | null | undefined; supportedFirstFactors: SignInFirstFactor[] | null | undefined; }) { - const { strategies: OAuthStrategies } = useEnabledThirdPartyProviders(); const supportedFirstFactors = _supportedFirstFactors || []; - const firstFactors = supportedFirstFactors.filter( - f => f.strategy !== filterOutFactor?.strategy && !isResetPasswordStrategy(f.strategy), - ); + const firstFactors = supportedFirstFactors.filter(f => !isResetPasswordStrategy(f.strategy)); - const shouldAllowForAlternativeStrategies = firstFactors.length + OAuthStrategies.length > 0; + const shouldAllowForAlternativeStrategies = firstFactors.length > 0; const firstPartyFactors = supportedFirstFactors .filter(f => !f.strategy.startsWith('oauth_')) - .filter(f => { - if ( - f.strategy === 'email_code' && - filterOutFactor?.strategy === 'email_code' && - filterOutFactor.emailAddressId === f.emailAddressId - ) { - return false; - } - - return f.strategy === filterOutFactor?.strategy; - }) .filter(factor => factorHasLocalStrategy(factor)) + .filter(factor => !isDeeplyEqual(factor, filterOutFactor)) // Only include passkey if the device supports it. // @ts-ignore Types are not public yet. .filter(factor => (factor.strategy === 'passkey' ? isWebAuthnSupported() : true)) From 1e1d5402d88a57fe6d08624f3b41988208268ca9 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Fri, 21 Feb 2025 13:00:28 +0200 Subject: [PATCH 4/9] chore(repo): Add changeset --- .changeset/sharp-sheep-reflect.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/sharp-sheep-reflect.md diff --git a/.changeset/sharp-sheep-reflect.md b/.changeset/sharp-sheep-reflect.md new file mode 100644 index 00000000000..bb22c1488d4 --- /dev/null +++ b/.changeset/sharp-sheep-reflect.md @@ -0,0 +1,5 @@ +--- +"@clerk/clerk-js": patch +--- + +fix(clerk-js): Correctly show alternative methods for user re-verification card From f1de40363c28b76859f9768c092dfdfd5a231a1c Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Fri, 21 Feb 2025 14:00:27 +0200 Subject: [PATCH 5/9] fix(clerk-js): Remove deeply cloned check --- .../UserVerification/AlternativeMethods.tsx | 2 +- .../UserVerification/UVFactorOneCodeForm.tsx | 28 +++++----- .../UserVerificationFactorOne.tsx | 2 +- .../useReverificationAlternativeStrategies.ts | 55 +++++++++++++++++++ .../src/ui/hooks/useAlternativeStrategies.ts | 30 ---------- 5 files changed, 70 insertions(+), 47 deletions(-) create mode 100644 packages/clerk-js/src/ui/components/UserVerification/useReverificationAlternativeStrategies.ts diff --git a/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx b/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx index 472d4a7c6b8..7bf5bf65a75 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/AlternativeMethods.tsx @@ -5,9 +5,9 @@ import type { LocalizationKey } from '../../customizables'; import { Col, descriptors, Flex, Flow, localizationKeys } from '../../customizables'; import { ArrowBlockButton, BackLink, Card, Header } from '../../elements'; import { useCardState } from '../../elements/contexts'; -import { useReverificationAlternativeStrategies } from '../../hooks/useAlternativeStrategies'; import { ChatAltIcon, Email, LockClosedIcon } from '../../icons'; import { formatSafeIdentifier } from '../../utils'; +import { useReverificationAlternativeStrategies } from './useReverificationAlternativeStrategies'; import { useUserVerificationSession } from './useUserVerificationSession'; import { withHavingTrouble } from './withHavingTrouble'; diff --git a/packages/clerk-js/src/ui/components/UserVerification/UVFactorOneCodeForm.tsx b/packages/clerk-js/src/ui/components/UserVerification/UVFactorOneCodeForm.tsx index 572ebac705a..94290e55852 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UVFactorOneCodeForm.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UVFactorOneCodeForm.tsx @@ -54,20 +54,18 @@ export const UVFactorOneCodeForm = (props: UVFactorOneCodeFormProps) => { }; return ( -
- -
+ ); }; diff --git a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorOne.tsx b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorOne.tsx index ab8f31262f8..38ea9164289 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorOne.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorOne.tsx @@ -3,11 +3,11 @@ import React, { useEffect } from 'react'; import { useEnvironment } from '../../contexts'; import { ErrorCard, LoadingCard, useCardState, withCardStateProvider } from '../../elements'; -import { useReverificationAlternativeStrategies } from '../../hooks/useAlternativeStrategies'; import { localizationKeys } from '../../localization'; import { useRouter } from '../../router'; import { determineStartingSignInFactor, factorHasLocalStrategy } from '../SignIn/utils'; import { AlternativeMethods } from './AlternativeMethods'; +import { useReverificationAlternativeStrategies } from './useReverificationAlternativeStrategies'; import { UserVerificationFactorOnePasswordCard } from './UserVerificationFactorOnePassword'; import { useUserVerificationSession, withUserVerificationSessionGuard } from './useUserVerificationSession'; import { UVFactorOneEmailCodeCard } from './UVFactorOneEmailCodeCard'; diff --git a/packages/clerk-js/src/ui/components/UserVerification/useReverificationAlternativeStrategies.ts b/packages/clerk-js/src/ui/components/UserVerification/useReverificationAlternativeStrategies.ts new file mode 100644 index 00000000000..16367dc9510 --- /dev/null +++ b/packages/clerk-js/src/ui/components/UserVerification/useReverificationAlternativeStrategies.ts @@ -0,0 +1,55 @@ +import type { SignInFactor, SignInFirstFactor } from '@clerk/types'; +import { useMemo } from 'react'; + +import { allStrategiesButtonsComparator } from '../../utils'; +import { factorHasLocalStrategy, isResetPasswordStrategy } from '../SignIn/utils'; + +const factorsAreEqual = (a: SignInFactor | null | undefined, b: SignInFactor | null | undefined) => { + if (!a || !b) { + return false; + } + + if (a.strategy === 'email_code' && b.strategy === 'email_code') { + return a.emailAddressId === b.emailAddressId; + } + + if (a.strategy === 'phone_code' && b.strategy === 'phone_code') { + return a.phoneNumberId === b.phoneNumberId; + } + + return a.strategy === b.strategy; +}; + +export function useReverificationAlternativeStrategies({ + filterOutFactor, + supportedFirstFactors, +}: { + filterOutFactor: SignInFactor | null | undefined; + supportedFirstFactors: SignInFirstFactor[] | null | undefined; +}) { + const firstFactors = supportedFirstFactors + ? supportedFirstFactors.filter(f => !isResetPasswordStrategy(f.strategy)) + : []; + const shouldAllowForAlternativeStrategies = firstFactors && firstFactors.length > 0; + + const firstPartyFactors = useMemo( + () => + supportedFirstFactors + ? (supportedFirstFactors + .filter(f => !f.strategy.startsWith('oauth_')) + .filter(factor => factorHasLocalStrategy(factor)) + .filter(factor => !factorsAreEqual(factor, filterOutFactor)) + // Only include passkey if the device supports it. + // @ts-ignore Types are not public yet. + .filter(factor => (factor.strategy === 'passkey' ? isWebAuthnSupported() : true)) + .sort(allStrategiesButtonsComparator) as T[]) + : [], + [supportedFirstFactors, filterOutFactor], + ); + + return { + hasAnyStrategy: shouldAllowForAlternativeStrategies, + hasFirstParty: firstPartyFactors && firstPartyFactors.length > 0, + firstPartyFactors, + }; +} diff --git a/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts b/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts index 89858be68bf..1d8c814eb7c 100644 --- a/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts +++ b/packages/clerk-js/src/ui/hooks/useAlternativeStrategies.ts @@ -1,4 +1,3 @@ -import { isDeeplyEqual } from '@clerk/shared/react/index'; import { isWebAuthnSupported } from '@clerk/shared/webauthn'; import type { SignInFactor, SignInFirstFactor } from '@clerk/types'; @@ -36,32 +35,3 @@ export function useAlternativeStrategies({ firstPartyFactors, }; } - -export function useReverificationAlternativeStrategies({ - filterOutFactor, - supportedFirstFactors: _supportedFirstFactors, -}: { - filterOutFactor: SignInFactor | null | undefined; - supportedFirstFactors: SignInFirstFactor[] | null | undefined; -}) { - const supportedFirstFactors = _supportedFirstFactors || []; - - const firstFactors = supportedFirstFactors.filter(f => !isResetPasswordStrategy(f.strategy)); - - const shouldAllowForAlternativeStrategies = firstFactors.length > 0; - - const firstPartyFactors = supportedFirstFactors - .filter(f => !f.strategy.startsWith('oauth_')) - .filter(factor => factorHasLocalStrategy(factor)) - .filter(factor => !isDeeplyEqual(factor, filterOutFactor)) - // Only include passkey if the device supports it. - // @ts-ignore Types are not public yet. - .filter(factor => (factor.strategy === 'passkey' ? isWebAuthnSupported() : true)) - .sort(allStrategiesButtonsComparator) as T[]; - - return { - hasAnyStrategy: shouldAllowForAlternativeStrategies, - hasFirstParty: firstPartyFactors.length > 0, - firstPartyFactors, - }; -} From 8a87847e4509d91e6b98a18cdcfd14f166f88656 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Wed, 26 Feb 2025 16:16:02 +0200 Subject: [PATCH 6/9] tests(clerk-js): Add test from first factor alternative mnethods --- .../UserVerificationFactorTwo.tsx | 18 +++-- .../__tests__/UVFactorOne.test.tsx | 76 ++++++++++++++++++- .../useReverificationAlternativeStrategies.ts | 21 ++++- 3 files changed, 105 insertions(+), 10 deletions(-) diff --git a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx index ec4ed4c0c22..369ca4becdb 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx @@ -1,10 +1,10 @@ -import { isDeeplyEqual } from '@clerk/shared/react/index'; import type { SessionVerificationResource, SessionVerificationSecondFactor, SignInFactor } from '@clerk/types'; -import React, { useEffect } from 'react'; +import React, { useEffect, useMemo } from 'react'; import { LoadingCard, withCardStateProvider } from '../../elements'; import { useRouter } from '../../router'; import { determineStartingSignInSecondFactor } from '../SignIn/utils'; +import { secondFactorsAreEqual } from './useReverificationAlternativeStrategies'; import { UserVerificationFactorTwoTOTP } from './UserVerificationFactorTwoTOTP'; import { useUserVerificationSession, withUserVerificationSessionGuard } from './useUserVerificationSession'; import { UVFactorTwoAlternativeMethods } from './UVFactorTwoAlternativeMethods'; @@ -22,7 +22,7 @@ const factorKey = (factor: SignInFactor | null | undefined) => { return key; }; -export function _UserVerificationFactorTwo(): JSX.Element { +export function UserVerificationFactorTwoComponent(): JSX.Element { const { navigate } = useRouter(); const { data } = useUserVerificationSession(); const sessionVerification = data as SessionVerificationResource; @@ -45,13 +45,19 @@ export function _UserVerificationFactorTwo(): JSX.Element { toggleAllStrategies(); }; - const hasAlternativeStrategies = - (availableFactors && availableFactors.filter(factor => isDeeplyEqual(factor, currentFactor)).length > 0) || false; + const hasAlternativeStrategies = useMemo( + () => + (availableFactors && + availableFactors.filter(factor => secondFactorsAreEqual(factor, currentFactor)).length > 0) || + false, + [availableFactors, currentFactor], + ); useEffect(() => { if (sessionVerification.status === 'needs_first_factor') { void navigate('../'); } + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); if (!currentFactor) { @@ -97,5 +103,5 @@ export function _UserVerificationFactorTwo(): JSX.Element { } export const UserVerificationFactorTwo = withUserVerificationSessionGuard( - withCardStateProvider(_UserVerificationFactorTwo), + withCardStateProvider(UserVerificationFactorTwoComponent), ); diff --git a/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorOne.test.tsx b/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorOne.test.tsx index cd0d21559de..dffd8e052ca 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorOne.test.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorOne.test.tsx @@ -135,7 +135,81 @@ describe('UserVerificationFactorOne', () => { }); describe('Use another method', () => { - it.todo('should list enabled first factor methods without the current one'); + it('should list enabled first factor methods without the current one', async () => { + const { wrapper, fixtures } = await createFixtures(f => { + f.withUser({ username: 'clerkuser' }); + }); + fixtures.session?.startVerification.mockResolvedValue({ + status: 'needs_first_factor', + supportedFirstFactors: [ + { + strategy: 'email_code', + emailAddressId: 'email_1', + safeIdentifier: 'xxx@hello.com', + }, + { + strategy: 'email_code', + emailAddressId: 'email_2', + safeIdentifier: 'xxx+1@hello.com', + }, + ], + }); + fixtures.session?.prepareFirstFactorVerification.mockResolvedValue({}); + + const { getByText, getByRole } = render(, { wrapper }); + + await waitFor(() => { + getByText('Verification required'); + getByText('Use another method'); + }); + + await waitFor(() => { + getByText('Use another method').click(); + expect(getByRole('button')).toHaveTextContent('Email code to xxx+1@hello.com'); + expect(getByRole('button')).not.toHaveTextContent('Email code to xxx@hello.com'); + }); + }); + + it('can select another method', async () => { + const { wrapper, fixtures } = await createFixtures(f => { + f.withUser({ username: 'clerkuser' }); + }); + fixtures.session?.startVerification.mockResolvedValue({ + status: 'needs_first_factor', + supportedFirstFactors: [ + { + strategy: 'email_code', + emailAddressId: 'email_1', + safeIdentifier: 'xxx@hello.com', + }, + { + strategy: 'email_code', + emailAddressId: 'email_2', + safeIdentifier: 'xxx+1@hello.com', + }, + ], + }); + fixtures.session?.prepareFirstFactorVerification.mockResolvedValue({}); + + const { getByText, container } = render(, { wrapper }); + + await waitFor(() => { + getByText('Verification required'); + expect(container).toHaveTextContent('xxx@hello.com'); + expect(container).not.toHaveTextContent('xxx+1@hello.com'); + getByText('Use another method'); + }); + + await waitFor(() => { + getByText('Use another method').click(); + getByText('Email code to xxx+1@hello.com').click(); + }); + + await waitFor(() => { + getByText('Verification required'); + expect(container).toHaveTextContent('xxx+1@hello.com'); + }); + }); }); describe('Get Help', () => { diff --git a/packages/clerk-js/src/ui/components/UserVerification/useReverificationAlternativeStrategies.ts b/packages/clerk-js/src/ui/components/UserVerification/useReverificationAlternativeStrategies.ts index 16367dc9510..9184ae8e8fc 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/useReverificationAlternativeStrategies.ts +++ b/packages/clerk-js/src/ui/components/UserVerification/useReverificationAlternativeStrategies.ts @@ -1,10 +1,10 @@ -import type { SignInFactor, SignInFirstFactor } from '@clerk/types'; +import type { SignInFactor, SignInFirstFactor, SignInSecondFactor } from '@clerk/types'; import { useMemo } from 'react'; import { allStrategiesButtonsComparator } from '../../utils'; import { factorHasLocalStrategy, isResetPasswordStrategy } from '../SignIn/utils'; -const factorsAreEqual = (a: SignInFactor | null | undefined, b: SignInFactor | null | undefined) => { +const firstFactorsAreEqual = (a: SignInFactor | null | undefined, b: SignInFactor | null | undefined) => { if (!a || !b) { return false; } @@ -20,6 +20,21 @@ const factorsAreEqual = (a: SignInFactor | null | undefined, b: SignInFactor | n return a.strategy === b.strategy; }; +export const secondFactorsAreEqual = ( + a: SignInSecondFactor | null | undefined, + b: SignInSecondFactor | null | undefined, +) => { + if (!a || !b) { + return false; + } + + if (a.strategy === 'phone_code' && b.strategy === 'phone_code') { + return a.phoneNumberId === b.phoneNumberId; + } + + return a.strategy === b.strategy; +}; + export function useReverificationAlternativeStrategies({ filterOutFactor, supportedFirstFactors, @@ -38,7 +53,7 @@ export function useReverificationAlternativeStrategies({ ? (supportedFirstFactors .filter(f => !f.strategy.startsWith('oauth_')) .filter(factor => factorHasLocalStrategy(factor)) - .filter(factor => !factorsAreEqual(factor, filterOutFactor)) + .filter(factor => !firstFactorsAreEqual(factor, filterOutFactor)) // Only include passkey if the device supports it. // @ts-ignore Types are not public yet. .filter(factor => (factor.strategy === 'passkey' ? isWebAuthnSupported() : true)) From 37e71ddbfc8c0ed7d487e3bbd9344954a1fd4d20 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Wed, 26 Feb 2025 16:44:08 +0200 Subject: [PATCH 7/9] tests(clerk-js): Add tests for second factor alternative methods --- .../UserVerificationFactorTwo.tsx | 16 ++-- .../__tests__/UVFactorTwo.test.tsx | 78 ++++++++++++++++++- 2 files changed, 85 insertions(+), 9 deletions(-) diff --git a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx index 369ca4becdb..24c254dbae7 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx @@ -36,6 +36,11 @@ export function UserVerificationFactorTwoComponent(): JSX.Element { const [showAllStrategies, setShowAllStrategies] = React.useState(!currentFactor); const toggleAllStrategies = () => setShowAllStrategies(s => !s); + const secondFactorsExcludingCurrent = useMemo( + () => availableFactors?.filter(factor => secondFactorsAreEqual(factor, currentFactor)), + [availableFactors, currentFactor], + ); + const handleFactorPrepare = () => { lastPreparedFactorKeyRef.current = factorKey(currentFactor); }; @@ -46,11 +51,8 @@ export function UserVerificationFactorTwoComponent(): JSX.Element { }; const hasAlternativeStrategies = useMemo( - () => - (availableFactors && - availableFactors.filter(factor => secondFactorsAreEqual(factor, currentFactor)).length > 0) || - false, - [availableFactors, currentFactor], + () => (secondFactorsExcludingCurrent && secondFactorsExcludingCurrent.length > 0) || false, + [secondFactorsExcludingCurrent], ); useEffect(() => { @@ -64,10 +66,10 @@ export function UserVerificationFactorTwoComponent(): JSX.Element { return ; } - if (showAllStrategies) { + if (showAllStrategies && hasAlternativeStrategies) { return ( diff --git a/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorTwo.test.tsx b/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorTwo.test.tsx index 4ced1804580..1662697b9db 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorTwo.test.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorTwo.test.tsx @@ -134,8 +134,82 @@ describe('UserVerificationFactorTwo', () => { }); }); - describe('Use another method', () => { - it.todo('should list enabled second factor methods without the current one'); + describe('Use another second factor method', () => { + it('should list enabled second factor methods without the current one', async () => { + const { wrapper, fixtures } = await createFixtures(f => { + f.withUser({ username: 'clerkuser' }); + }); + fixtures.session?.startVerification.mockResolvedValue({ + status: 'needs_second_factor', + supportedSecondFactors: [ + { + strategy: 'phone_code', + phoneNumberId: 'phone_1', + safeIdentifier: '+3069XXXXXXX1', + }, + { + strategy: 'phone_code', + phoneNumberId: 'phone_2', + safeIdentifier: '+3069XXXXXXX2', + }, + ], + }); + fixtures.session?.prepareSecondFactorVerification.mockResolvedValue({}); + + const { getByText, getByRole } = render(, { wrapper }); + + await waitFor(() => { + getByText('Verification required'); + getByText('Use another method'); + }); + + await waitFor(() => { + getByText('Use another method').click(); + expect(getByRole('button')).toHaveTextContent('Send SMS code to +3069XXXXXXX1'); + expect(getByRole('button')).not.toHaveTextContent('Send SMS code to +3069XXXXXXX2'); + }); + }); + + it.skip('can select another method', async () => { + const { wrapper, fixtures } = await createFixtures(f => { + f.withUser({ username: 'clerkuser' }); + }); + fixtures.session?.startVerification.mockResolvedValue({ + status: 'needs_second_factor', + supportedSecondFactors: [ + { + strategy: 'phone_code', + phoneNumberId: 'phone_1', + safeIdentifier: '+3069XXXXXXX1', + }, + { + strategy: 'phone_code', + phoneNumberId: 'phone_2', + safeIdentifier: '+3069XXXXXXX2', + }, + ], + }); + fixtures.session?.prepareSecondFactorVerification.mockResolvedValue({}); + + const { getByText, container } = render(, { wrapper }); + + await waitFor(() => { + getByText('Verification required'); + expect(container).toHaveTextContent('+3069XXXXXXX1'); + expect(container).not.toHaveTextContent('+3069XXXXXXX2'); + getByText('Use another method'); + }); + + await waitFor(() => { + getByText('Use another method').click(); + getByText('Send SMS code to +3069XXXXXXX2').click(); + }); + + await waitFor(() => { + getByText('Verification required'); + expect(container).toHaveTextContent('+3069XXXXXXX2'); + }); + }); }); describe('Get Help', () => { From f6040d1573acb8ac2645006820ea9fa5d578a31e Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Wed, 26 Feb 2025 17:32:46 +0200 Subject: [PATCH 8/9] tests(clerk-js): Add test for second factor alternative methods --- .../UserVerification/UserVerificationFactorTwo.tsx | 2 +- .../UserVerification/__tests__/UVFactorTwo.test.tsx | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx index 24c254dbae7..13f897d67f6 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx @@ -37,7 +37,7 @@ export function UserVerificationFactorTwoComponent(): JSX.Element { const toggleAllStrategies = () => setShowAllStrategies(s => !s); const secondFactorsExcludingCurrent = useMemo( - () => availableFactors?.filter(factor => secondFactorsAreEqual(factor, currentFactor)), + () => availableFactors?.filter(factor => !secondFactorsAreEqual(factor, currentFactor)), [availableFactors, currentFactor], ); diff --git a/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorTwo.test.tsx b/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorTwo.test.tsx index 1662697b9db..fac8f08770c 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorTwo.test.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/__tests__/UVFactorTwo.test.tsx @@ -165,12 +165,12 @@ describe('UserVerificationFactorTwo', () => { await waitFor(() => { getByText('Use another method').click(); - expect(getByRole('button')).toHaveTextContent('Send SMS code to +3069XXXXXXX1'); - expect(getByRole('button')).not.toHaveTextContent('Send SMS code to +3069XXXXXXX2'); + expect(getByRole('button')).toHaveTextContent('Send SMS code to +3069XXXXXXX2'); + expect(getByRole('button')).not.toHaveTextContent('Send SMS code to +3069XXXXXXX1'); }); }); - it.skip('can select another method', async () => { + it('can select another method', async () => { const { wrapper, fixtures } = await createFixtures(f => { f.withUser({ username: 'clerkuser' }); }); @@ -208,6 +208,12 @@ describe('UserVerificationFactorTwo', () => { await waitFor(() => { getByText('Verification required'); expect(container).toHaveTextContent('+3069XXXXXXX2'); + getByText('Use another method'); + }); + + await waitFor(() => { + getByText('Use another method').click(); + expect(container).toHaveTextContent('+3069XXXXXXX1'); }); }); }); From 11c231ecf06f1702f23980304ed94cf12a225fae Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Thu, 6 Mar 2025 20:42:41 +0200 Subject: [PATCH 9/9] fix(clerk-js): Fix types --- .../components/UserVerification/UserVerificationFactorTwo.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx index 13f897d67f6..815626bdd64 100644 --- a/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx +++ b/packages/clerk-js/src/ui/components/UserVerification/UserVerificationFactorTwo.tsx @@ -69,7 +69,7 @@ export function UserVerificationFactorTwoComponent(): JSX.Element { if (showAllStrategies && hasAlternativeStrategies) { return (