refactor(console,core): refactor sign-up verify and sign-in syncing (#7191)

* refactor(console,core): refactor sign-up settings syncing logic

refactor the sign-up settings syncing logic

* fix(test): fix tests

fix tests

* fix(console): fix sign-up identifiers listener

fix sign-up identifiers listener

packages/console/src/ds-components/Checkbox/Checkbox/index.tsx

@@ -2,7 +2,9 @@ import classNames from 'classnames';
 import type { ReactNode } from 'react';
 import { useLayoutEffect, useState } from 'react';
 
-import { Tooltip } from '@/ds-components/Tip';
+import Tip from '@/assets/icons/tip.svg?react';
+import IconButton from '@/ds-components/IconButton';
+import { ToggleTip, Tooltip } from '@/ds-components/Tip';
 import { onKeyDownHandler } from '@/utils/a11y';
 
 import styles from './index.module.scss';
@@ -17,6 +19,7 @@ type Props = {
   readonly label?: ReactNode;
   readonly className?: string;
   readonly tooltip?: ReactNode;
+  readonly suffixTooltip?: ReactNode;
 };
 
 function Checkbox({
@@ -27,6 +30,7 @@ function Checkbox({
   label,
   className,
   tooltip,
+  suffixTooltip,
 }: Props) {
   const [isIndeterminate, setIsIndeterminate] = useState(indeterminate);
 
@@ -104,6 +108,17 @@ function Checkbox({
         </Tooltip>
         {label && <span className={styles.label}>{label}</span>}
       </div>
+      {suffixTooltip && (
+        <ToggleTip
+          anchorClassName={styles.toggleTipButton}
+          content={suffixTooltip}
+          horizontalAlign="start"
+        >
+          <IconButton size="small">
+            <Tip />
+          </IconButton>
+        </ToggleTip>
+      )}
     </div>
   );
 }

packages/console/src/pages/SignInExperience/PageContent/SignUpAndSignIn/SignInForm/SignInMethodEditBox/SignInMethodItem.tsx

@@ -7,6 +7,7 @@ import { useTranslation } from 'react-i18next';
 import Draggable from '@/assets/icons/draggable.svg?react';
 import Minus from '@/assets/icons/minus.svg?react';
 import SwitchArrowIcon from '@/assets/icons/switch-arrow.svg?react';
+import { isDevFeaturesEnabled } from '@/consts/env';
 import Checkbox from '@/ds-components/Checkbox';
 import IconButton from '@/ds-components/IconButton';
 import { Tooltip } from '@/ds-components/Tip';
@@ -66,7 +67,10 @@ function SignInMethodItem({
               checked={password}
               disabled={!isPasswordCheckable}
               tooltip={conditional(
-                !isPasswordCheckable && t('sign_in_exp.sign_up_and_sign_in.tip.password_auth')
+                // TODO:remove the password tool tip
+                !isDevFeaturesEnabled &&
+                  !isPasswordCheckable &&
+                  t('sign_in_exp.sign_up_and_sign_in.tip.password_auth')
               )}
               onChange={(checked) => {
                 onVerificationStateChange('password', checked);

packages/console/src/pages/SignInExperience/PageContent/SignUpAndSignIn/SignInForm/SignInMethodEditBox/index.tsx

@@ -80,7 +80,9 @@ function SignInMethodEditBox() {
         return !isSignUpVerificationRequired;
       }
 
-      // If the sign-in identifier is also enabled for sign-up.
+      // If the email or phone sign-in method is enabled as one of the sign-up identifiers
+      // and password is not required for sign-up, then verification code is required and uncheckable.
+      // This is to ensure new users can sign in without password.
       const signUpVerificationRequired = signUpIdentifiers.some(
         (signUpIdentifier) =>
           signUpIdentifier === identifier ||

packages/console/src/pages/SignInExperience/PageContent/SignUpAndSignIn/SignUpForm/SignUpForm.tsx

@@ -1,5 +1,4 @@
-import { AlternativeSignUpIdentifier, SignInIdentifier } from '@logto/schemas';
-import { useCallback, useMemo } from 'react';
+import { useMemo } from 'react';
 import { Controller, useFormContext, useWatch } from 'react-hook-form';
 import { useTranslation } from 'react-i18next';
 
@@ -10,90 +9,32 @@ import FormField from '@/ds-components/FormField';
 import type { SignInExperienceForm } from '../../../types';
 import FormFieldDescription from '../../components/FormFieldDescription';
 import FormSectionTitle from '../../components/FormSectionTitle';
-import { createSignInMethod } from '../utils';
 
 import SignUpIdentifiersEditBox from './SignUpIdentifiersEditBox';
 import styles from './index.module.scss';
+import useSignUpPasswordListeners from './use-sign-up-password-listeners';
 
 function SignUpForm() {
   const { t } = useTranslation(undefined, { keyPrefix: 'admin_console' });
-  const {
-    control,
-    setValue,
-    getValues,
-    trigger,
-    formState: { submitCount },
-  } = useFormContext<SignInExperienceForm>();
+
+  const { control } = useFormContext<SignInExperienceForm>();
 
   const signUpIdentifiers = useWatch({
     control,
     name: 'signUp.identifiers',
   });
 
-  const { shouldShowAuthenticationFields, shouldShowVerificationField } = useMemo(() => {
-    return {
-      shouldShowAuthenticationFields: signUpIdentifiers.length > 0,
-      shouldShowVerificationField: signUpIdentifiers[0]?.identifier !== SignInIdentifier.Username,
-    };
-  }, [signUpIdentifiers]);
+  const signUpVerify = useWatch({
+    control,
+    name: 'signUp.verify',
+  });
 
-  // Sync sign-in methods when sign-up methods change
-  const syncSignInMethods = useCallback(() => {
-    const signInMethods = getValues('signIn.methods');
-    const signUp = getValues('signUp');
+  const showAuthenticationFields = useMemo(
+    () => signUpIdentifiers.length > 0,
+    [signUpIdentifiers.length]
+  );
 
-    const { password, identifiers } = signUp;
-
-    const enabledSignUpIdentifiers = identifiers.reduce<SignInIdentifier[]>(
-      (identifiers, { identifier: signUpIdentifier }) => {
-        if (signUpIdentifier === AlternativeSignUpIdentifier.EmailOrPhone) {
-          return [...identifiers, SignInIdentifier.Email, SignInIdentifier.Phone];
-        }
-
-        return [...identifiers, signUpIdentifier];
-      },
-      []
-    );
-
-    // Note: Auto append newly assigned sign-up identifiers to the sign-in methods list if they don't already exist
-    // User may remove them manually if they don't want to use it for sign-in.
-    const mergedSignInMethods = enabledSignUpIdentifiers.reduce((methods, signUpIdentifier) => {
-      if (signInMethods.some(({ identifier }) => identifier === signUpIdentifier)) {
-        return methods;
-      }
-
-      return [...methods, createSignInMethod(signUpIdentifier)];
-    }, signInMethods);
-
-    setValue(
-      'signIn.methods',
-      mergedSignInMethods.map((method) => {
-        const { identifier } = method;
-
-        if (identifier === SignInIdentifier.Username) {
-          return method;
-        }
-
-        return {
-          ...method,
-          // Auto enabled password for email and phone sign-in methods if password is required for sign-up.
-          // User may disable it manually if they don't want to use password for email or phone sign-in.
-          password: method.password || password,
-          // Note: if password is not set for sign-up,
-          // then auto enable verification code for email and phone sign-in methods.
-          verificationCode: password ? method.verificationCode : true,
-        };
-      })
-    );
-
-    // Note: we need to revalidate the sign-in methods after the signIn form data has been updated
-    if (submitCount) {
-      // Wait for the form re-render before validating the new data.
-      setTimeout(() => {
-        void trigger('signIn.methods');
-      }, 0);
-    }
-  }, [getValues, setValue, submitCount, trigger]);
+  useSignUpPasswordListeners();
 
   return (
     <Card>
@@ -102,9 +43,9 @@ function SignUpForm() {
         <FormFieldDescription>
           {t('sign_in_exp.sign_up_and_sign_in.sign_up.identifier_description')}
         </FormFieldDescription>
-        <SignUpIdentifiersEditBox syncSignInMethods={syncSignInMethods} />
+        <SignUpIdentifiersEditBox />
       </FormField>
-      {shouldShowAuthenticationFields && (
+      {showAuthenticationFields && (
         <FormField title="sign_in_exp.sign_up_and_sign_in.sign_up.sign_up_authentication">
           <FormFieldDescription>
             {t('sign_in_exp.sign_up_and_sign_in.sign_up.authentication_description')}
@@ -117,14 +58,11 @@ function SignUpForm() {
                 <Checkbox
                   label={t('sign_in_exp.sign_up_and_sign_in.sign_up.set_a_password_option')}
                   checked={value}
-                  onChange={(value) => {
-                    onChange(value);
-                    syncSignInMethods();
-                  }}
+                  onChange={onChange}
                 />
               )}
             />
-            {shouldShowVerificationField && (
+            {signUpVerify && (
               <Controller
                 name="signUp.verify"
                 control={control}
@@ -133,7 +71,7 @@ function SignUpForm() {
                     disabled
                     label={t('sign_in_exp.sign_up_and_sign_in.sign_up.verify_at_sign_up_option')}
                     checked={value}
-                    tooltip={t('sign_in_exp.sign_up_and_sign_in.tip.verify_at_sign_up')}
+                    suffixTooltip={t('sign_in_exp.sign_up_and_sign_in.sign_up.verification_tip')}
                     onChange={onChange}
                   />
                 )}

packages/console/src/pages/SignInExperience/PageContent/SignUpAndSignIn/SignUpForm/SignUpIdentifiersEditBox/index.tsx

@@ -4,7 +4,7 @@ import {
   type SignUpIdentifier,
 } from '@logto/schemas';
 import { t } from 'i18next';
-import { useCallback, useMemo } from 'react';
+import { useCallback, useEffect, useMemo } from 'react';
 import { Controller, useFieldArray, useFormContext, useWatch } from 'react-hook-form';
 
 import { DragDropProvider, DraggableItem } from '@/ds-components/DragDrop';
@@ -12,7 +12,7 @@ import useEnabledConnectorTypes from '@/hooks/use-enabled-connector-types';
 import { type SignInExperienceForm } from '@/pages/SignInExperience/types';
 
 import IdentifiersAddButton from '../../components/IdentifiersAddButton';
-import { getSignUpIdentifiersRequiredConnectors } from '../../utils';
+import { createSignInMethod, getSignUpIdentifiersRequiredConnectors } from '../../utils';
 
 import SignUpIdentifierItem from './SignUpIdentifierItem';
 import styles from './index.module.scss';
@@ -29,15 +29,14 @@ const emailOrPhoneOption = {
 
 const signUpIdentifierOptions = [...signInIdentifierOptions, emailOrPhoneOption];
 
-type Props = {
-  /**
-   * Sync the sign-in methods when the sign-up settings change.
-   */
-  readonly syncSignInMethods: () => void;
-};
-
-function SignUpIdentifiersEditBox({ syncSignInMethods }: Props) {
-  const { control, getValues, setValue } = useFormContext<SignInExperienceForm>();
+function SignUpIdentifiersEditBox() {
+  const {
+    control,
+    getValues,
+    setValue,
+    trigger,
+    formState: { submitCount },
+  } = useFormContext<SignInExperienceForm>();
 
   const signUpIdentifiers = useWatch({ control, name: 'signUp.identifiers' });
 
@@ -48,37 +47,70 @@ function SignUpIdentifiersEditBox({ syncSignInMethods }: Props) {
     name: 'signUp.identifiers',
   });
 
-  // Revalidate the primary identifier authentication fields when the identifiers change
-  const onSignUpIdentifiersChange = useCallback(() => {
-    const identifiers = getValues('signUp.identifiers').map(({ identifier }) => identifier);
-    setValue('signUp.verify', identifiers[0] !== SignInIdentifier.Username);
-    syncSignInMethods();
-  }, [getValues, setValue, syncSignInMethods]);
+  /**
+   * Append the sign-in methods based on the selected sign-up identifier.
+   */
+  const appendSignInMethods = useCallback(
+    (identifier: SignUpIdentifier) => {
+      const signInMethods = getValues('signIn.methods');
+      const signInMethodsSet = new Set(signInMethods.map(({ identifier }) => identifier));
 
-  const onDeleteSignUpIdentifier = useCallback(() => {
-    const identifiers = getValues('signUp.identifiers').map(({ identifier }) => identifier);
+      const newSignUpIdentifiers =
+        identifier === AlternativeSignUpIdentifier.EmailOrPhone
+          ? [SignInIdentifier.Email, SignInIdentifier.Phone]
+          : [identifier];
 
-    if (identifiers.length === 0) {
-      setValue('signUp.password', false);
-      setValue('signUp.verify', false);
-      // Password changed need to sync sign-in methods
-      syncSignInMethods();
-      return;
-    }
+      const newSignInMethods = newSignUpIdentifiers.filter(
+        (identifier) => !signInMethodsSet.has(identifier)
+      );
 
-    onSignUpIdentifiersChange();
-  }, [getValues, onSignUpIdentifiersChange, setValue, syncSignInMethods]);
+      if (newSignInMethods.length === 0) {
+        return;
+      }
+
+      setValue(
+        'signIn.methods',
+        signInMethods.concat(newSignInMethods.map((identifier) => createSignInMethod(identifier))),
+        {
+          shouldDirty: true,
+        }
+      );
+
+      if (submitCount) {
+        // Wait for the form re-render before validating the new data.
+        setTimeout(() => {
+          void trigger('signIn.methods');
+        }, 0);
+      }
+    },
+    [getValues, setValue, submitCount, trigger]
+  );
 
   const onAppendSignUpIdentifier = useCallback(
     (identifier: SignUpIdentifier) => {
+      appendSignInMethods(identifier);
+
       if (identifier === SignInIdentifier.Username) {
-        setValue('signUp.password', true);
+        setValue('signUp.password', true, {
+          // Make sure to trigger the on password change hook
+          shouldDirty: true,
+        });
       }
-      onSignUpIdentifiersChange();
     },
-    [onSignUpIdentifiersChange, setValue]
+    [appendSignInMethods, setValue]
   );
 
+  useEffect(() => {
+    if (signUpIdentifiers.length === 0) {
+      setValue('signUp.password', false, { shouldDirty: true });
+    }
+
+    const isSignUpVerify = signUpIdentifiers.some(
+      ({ identifier }) => identifier !== SignInIdentifier.Username
+    );
+    setValue('signUp.verify', isSignUpVerify, { shouldDirty: true });
+  }, [setValue, signUpIdentifiers]);
+
   const options = useMemo<
     Array<{
       value: SignUpIdentifier;
@@ -131,10 +163,7 @@ function SignUpIdentifiersEditBox({ syncSignInMethods }: Props) {
               key={id}
               id={id}
               sortIndex={index}
-              moveItem={(dragIndex, hoverIndex) => {
-                swap(dragIndex, hoverIndex);
-                onSignUpIdentifiersChange();
-              }}
+              moveItem={swap}
               className={styles.draggleItemContainer}
             >
               <Controller
@@ -166,7 +195,6 @@ function SignUpIdentifiersEditBox({ syncSignInMethods }: Props) {
                     errorMessage={error?.message}
                     onDelete={() => {
                       remove(index);
-                      onDeleteSignUpIdentifier();
                     }}
                   />
                 )}

packages/console/src/pages/SignInExperience/PageContent/SignUpAndSignIn/SignUpForm/use-sign-up-password-listeners.ts

@@ -0,0 +1,69 @@
+import { SignInIdentifier } from '@logto/schemas';
+import { useEffect, useRef } from 'react';
+import { useFormContext, useWatch } from 'react-hook-form';
+
+import { type SignInExperienceForm } from '@/pages/SignInExperience/types';
+
+/**
+ * This hook listens to the password field changes in the sign-up form,
+ * and updates the sign-in methods accordingly.
+ *
+ * - if the password is enabled for sign-up, then it will be enabled for all sign-in methods.
+ * - if the password is not required for sign-up, then verification code authentication method is required for email and phone sign-in methods.
+ */
+const useSignUpPasswordListeners = () => {
+  const {
+    control,
+    getValues,
+    setValue,
+    trigger,
+    formState: { submitCount },
+  } = useFormContext<SignInExperienceForm>();
+
+  const isFirstMount = useRef(true);
+
+  const signUpPassword = useWatch({ control, name: 'signUp.password' });
+
+  useEffect(() => {
+    // Only sync the password settings on updates (skip the first mount)
+    if (isFirstMount.current) {
+      // eslint-disable-next-line @silverhand/fp/no-mutation
+      isFirstMount.current = false;
+      return;
+    }
+
+    const signInMethods = getValues('signIn.methods');
+
+    setValue(
+      'signIn.methods',
+      signInMethods.map((method) => {
+        if (method.identifier === SignInIdentifier.Username) {
+          // No need to mutate the username method
+          return method;
+        }
+
+        return {
+          ...method,
+          // Auto enabled password for all sign-in methods,
+          // if the password is enabled for sign-up
+          password: method.password || signUpPassword,
+          // If password is not required for sign-up,
+          // then verification code authentication method is required for email and phone sign-in methods
+          verificationCode: signUpPassword ? method.verificationCode : true,
+        };
+      }),
+      {
+        shouldDirty: true,
+      }
+    );
+
+    if (submitCount) {
+      // Wait for the form re-render before validating the new data.
+      setTimeout(() => {
+        void trigger('signIn.methods');
+      }, 0);
+    }
+  }, [getValues, setValue, signUpPassword, submitCount, trigger]);
+};
+
+export default useSignUpPasswordListeners;

packages/core/src/libraries/sign-in-experience/sign-up.test.ts

@@ -125,6 +125,7 @@ describe('validate sign-up', () => {
         validateSignUp(
           {
             ...mockSignUp,
+            verify: true,
             secondaryIdentifiers: [{ identifier: SignInIdentifier.Email, verify: true }],
           },
           []
@@ -142,6 +143,7 @@ describe('validate sign-up', () => {
         validateSignUp(
           {
             ...mockSignUp,
+            verify: true,
             secondaryIdentifiers: [{ identifier: SignInIdentifier.Phone, verify: true }],
           },
           []
@@ -159,6 +161,7 @@ describe('validate sign-up', () => {
         validateSignUp(
           {
             ...mockSignUp,
+            verify: true,
             secondaryIdentifiers: [
               { identifier: AlternativeSignUpIdentifier.EmailOrPhone, verify: true },
             ],
@@ -228,12 +231,45 @@ describe('validate sign-up', () => {
       {
         identifier: AlternativeSignUpIdentifier.EmailOrPhone,
       },
-    ])('should throw when identifier is %p and verify is not true', async (identifier) => {
+    ])(
+      'should throw when identifier is %p and verify is not true for each identifier',
+      async (identifier) => {
+        expect(() => {
+          validateSignUp(
+            {
+              ...mockSignUp,
+              secondaryIdentifiers: [identifier],
+            },
+            enabledConnectors
+          );
+        }).toMatchError(
+          new RequestError({
+            code: 'sign_in_experiences.passwordless_requires_verify',
+          })
+        );
+      }
+    );
+
+    test.each([
+      {
+        identifier: SignInIdentifier.Email,
+        verify: true,
+      },
+      {
+        identifier: SignInIdentifier.Phone,
+        verify: true,
+      },
+      {
+        identifier: AlternativeSignUpIdentifier.EmailOrPhone,
+        verify: true,
+      },
+    ])('should throw when identifier is %p and signUp.verify is not true', async (identifier) => {
       expect(() => {
         validateSignUp(
           {
             ...mockSignUp,
             secondaryIdentifiers: [identifier],
+            verify: false,
           },
           enabledConnectors
         );

packages/core/src/libraries/sign-in-experience/sign-up.ts

@@ -51,13 +51,11 @@ const validatePasswordlessIdentifiers = (
   { identifiers, secondaryIdentifiers = [], verify }: SignUp,
   enabledConnectors: LogtoConnector[]
 ) => {
-  const primaryIdentifiers = new Set(identifiers);
-
   if (
-    primaryIdentifiers.has(SignInIdentifier.Email) ||
-    primaryIdentifiers.has(SignInIdentifier.Phone)
+    identifiers.some((identifier) => identifier !== SignInIdentifier.Username) ||
+    secondaryIdentifiers.some(({ identifier }) => identifier !== SignInIdentifier.Username)
   ) {
-    // Primary passwordless identifiers must have verify enabled.
+    // Passwordless identifiers must have verify enabled.
     assertThat(
       verify,
       new RequestError({
@@ -76,6 +74,7 @@ const validatePasswordlessIdentifiers = (
     );
   }
 
+  const primaryIdentifiers = new Set(identifiers);
   const secondaryIdentifiersSet = new Set(secondaryIdentifiers.map((item) => item.identifier));
 
   // Assert email connector is enabled

packages/core/src/routes/experience/classes/libraries/sign-in-experience-validator.test.ts

@@ -405,7 +405,7 @@ describe('SignInExperienceValidator', () => {
         {
           identifiers: [SignInIdentifier.Phone, SignInIdentifier.Email],
           password: true,
-          verify: false,
+          verify: true,
         },
         new Set([MissingProfile.password, MissingProfile.emailOrPhone]),
       ],
@@ -432,7 +432,7 @@ describe('SignInExperienceValidator', () => {
         signUp: {
           identifiers: [SignInIdentifier.Email, SignInIdentifier.Username],
           password: true,
-          verify: false,
+          verify: true,
         },
       });
       const signInExperienceValidator = new SignInExperienceValidator(
@@ -465,7 +465,7 @@ describe('SignInExperienceValidator', () => {
           {
             identifiers: [SignInIdentifier.Username],
             password: true,
-            verify: false,
+            verify: true,
             secondaryIdentifiers: [
               { identifier: SignInIdentifier.Email },
               { identifier: SignInIdentifier.Phone },

packages/integration-tests/src/tests/api/experience-api/register-interaction/username-password.test.ts

@@ -102,7 +102,7 @@ devFeatureTest.describe(
           signUp: {
             identifiers: [SignInIdentifier.Username],
             password: true,
-            verify: false,
+            verify: true,
             secondaryIdentifiers: [
               {
                 identifier: secondaryIdentifier,

packages/phrases/src/locales/en/translation/admin-console/sign-in-exp/sign-up-and-sign-in.ts

@@ -11,6 +11,8 @@ const sign_up_and_sign_in = {
     sign_up_identifier: 'Sign-up identifiers',
     identifier_description: 'The sign-up identifier is required for account creation.',
     sign_up_authentication: 'Authentication setting for sign-up',
+    verification_tip:
+      'Users must verify the email or phone number you’ve configured by entering a verification code during sign-up.',
     authentication_description:
       'All selected actions will be obligatory for users to complete the flow.',
     set_a_password_option: 'Create your password',
@@ -47,7 +49,7 @@ const sign_up_and_sign_in = {
   tip: {
     set_a_password: 'A unique set of a password to your username is a must.',
     verify_at_sign_up:
-      'We currently only support verified email. Your user base may contain a large number of poor-quality email addresses if no validation.',
+      'We currently only support verified email and phone. Your user base may contain a large number of poor-quality email addresses or phone numbers if no validation.',
     password_auth:
       'This is essential as you have enabled the option to set a password during the sign-up process.',
     verification_code_auth: