feat(core,schemas): add mandatory password guard on register

add mandatory password guard on register

packages/core/src/routes/experience/classes/experience-interaction.ts

@@ -1,6 +1,12 @@
 /* eslint-disable max-lines */
 import { type ToZodObject } from '@logto/connector-kit';
-import { InteractionEvent, type User } from '@logto/schemas';
+import {
+  InteractionEvent,
+  SignInIdentifier,
+  VerificationType,
+  type InteractionIdentifier,
+  type User,
+} from '@logto/schemas';
 import { conditional } from '@silverhand/essentials';
 import { z } from 'zod';
 
@@ -28,6 +34,7 @@ import { SignInExperienceValidator } from './libraries/sign-in-experience-valida
 import { Mfa, mfaDataGuard, userMfaDataKey, type MfaData } from './mfa.js';
 import { Profile } from './profile.js';
 import { toUserSocialIdentityData } from './utils.js';
+import { identifierCodeVerificationTypeMap } from './verifications/code-verification.js';
 import {
   buildVerificationRecord,
   verificationRecordDataGuard,
@@ -456,13 +463,28 @@ export default class ExperienceInteraction {
   /**
    * Create a new user using the verification record.
    *
-   * @throws {RequestError} with 400 if the verification record is invalid for creating a new user or not verified
+   * @throws {RequestError} with 422 if a new password identity verification is provided, but identifier (email/phone) is not verified
+   * @throws {RequestError} with 400 if the verification record can not be used for creating a new user or not verified
    * @throws {RequestError} with 422 if the profile data is not unique across users
+   * @throws {RequestError} with 422 if the password is required for the sign-up settings but only email/phone verification record is provided
    */
   private async createNewUser(verificationRecord: VerificationRecord) {
+    if (verificationRecord.type === VerificationType.NewPasswordIdentity) {
+      const { identifier } = verificationRecord;
+      assertThat(
+        this.isIdentifierVerified(identifier),
+        new RequestError(
+          { code: 'session.identifier_not_verified', status: 422 },
+          { identifier: identifier.value }
+        )
+      );
+    }
+
     const newProfile = await getNewUserProfileFromVerificationRecord(verificationRecord);
     await this.profile.profileValidator.guardProfileUniquenessAcrossUsers(newProfile);
 
+    await this.signInExperienceValidator.guardMandatoryPasswordOnRegister(verificationRecord);
+
     const user = await this.provisionLibrary.createUser(newProfile);
 
     this.userId = user.id;
@@ -500,6 +522,20 @@ export default class ExperienceInteraction {
     return this.verificationRecordsArray.find((record) => record.id === verificationId);
   }
 
+  private isIdentifierVerified(identifier: InteractionIdentifier) {
+    const { type, value } = identifier;
+
+    if (type === SignInIdentifier.Username) {
+      return true;
+    }
+
+    const verificationRecord = this.verificationRecords.get(
+      identifierCodeVerificationTypeMap[type]
+    );
+
+    return verificationRecord?.identifier.value === value && verificationRecord.isVerified;
+  }
+
   /**
    * Clean up the interaction storage.
    */

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

@@ -42,6 +42,15 @@ const newPasswordIdentityVerificationRecord = NewPasswordIdentityVerification.cr
   }
 );
 
+const emailNewPasswordIdentityVerificationRecord = NewPasswordIdentityVerification.create(
+  mockTenant.libraries,
+  mockTenant.queries,
+  {
+    type: SignInIdentifier.Email,
+    value: `foo@${emailDomain}`,
+  }
+);
+
 const passwordVerificationRecords = Object.fromEntries(
   Object.values(SignInIdentifier).map((identifier) => [
     identifier,
@@ -509,5 +518,119 @@ describe('SignInExperienceValidator', () => {
       ).rejects.toMatchError(expectError);
     });
   });
+
+  describe('guardMandatoryPasswordOnRegister', () => {
+    const testCases: Record<
+      string,
+      {
+        signInExperience: SignInExperience;
+        cases: Array<{ verificationRecord: VerificationRecord; accepted: boolean }>;
+      }
+    > = Object.freeze({
+      'should throw error for CodeVerification Records if password is required': {
+        signInExperience: {
+          ...mockSignInExperience,
+          signUp: {
+            identifiers: [SignInIdentifier.Email],
+            password: true,
+            verify: true,
+          },
+        },
+        cases: [
+          {
+            verificationRecord: verificationCodeVerificationRecords[SignInIdentifier.Email],
+            accepted: false,
+          },
+          {
+            verificationRecord: verificationCodeVerificationRecords[SignInIdentifier.Phone],
+            accepted: false,
+          },
+        ],
+      },
+      'should not throw error for CodeVerification Records if password is not required': {
+        signInExperience: {
+          ...mockSignInExperience,
+          signUp: {
+            identifiers: [SignInIdentifier.Email],
+            password: false,
+            verify: true,
+          },
+        },
+        cases: [
+          {
+            verificationRecord: verificationCodeVerificationRecords[SignInIdentifier.Email],
+            accepted: true,
+          },
+          {
+            verificationRecord: verificationCodeVerificationRecords[SignInIdentifier.Phone],
+            accepted: true,
+          },
+        ],
+      },
+      'should not throw error for NewPasswordIdentity verification record': {
+        signInExperience: {
+          ...mockSignInExperience,
+          signUp: {
+            identifiers: [SignInIdentifier.Username],
+            password: true,
+            verify: true,
+          },
+        },
+        cases: [
+          {
+            verificationRecord: newPasswordIdentityVerificationRecord,
+            accepted: true,
+          },
+          {
+            verificationRecord: emailNewPasswordIdentityVerificationRecord,
+            accepted: true,
+          },
+        ],
+      },
+      'should not throw error for Social and SSO verification records': {
+        signInExperience: {
+          ...mockSignInExperience,
+          signUp: {
+            identifiers: [SignInIdentifier.Email],
+            password: true,
+            verify: true,
+          },
+        },
+        cases: [
+          {
+            verificationRecord: socialVerificationRecord,
+            accepted: true,
+          },
+          {
+            verificationRecord: enterpriseSsoVerificationRecords,
+            accepted: true,
+          },
+        ],
+      },
+    });
+
+    describe.each(Object.keys(testCases))(`%s`, (testCase) => {
+      const { signInExperience, cases } = testCases[testCase]!;
+
+      it.each(cases)('guard verification record %p', async ({ verificationRecord, accepted }) => {
+        signInExperiences.findDefaultSignInExperience.mockResolvedValueOnce(signInExperience);
+
+        const signInExperienceSettings = new SignInExperienceValidator(
+          mockTenant.libraries,
+          mockTenant.queries
+        );
+
+        await (accepted
+          ? expect(
+              signInExperienceSettings.guardMandatoryPasswordOnRegister(verificationRecord)
+            ).resolves.not.toThrow()
+          : expect(
+              signInExperienceSettings.guardMandatoryPasswordOnRegister(verificationRecord)
+            ).rejects.toMatchError(
+              new RequestError({ code: 'user.password_required_in_profile', status: 422 })
+            ));
+      });
+    });
+  });
 });
 /* eslint-enable max-lines */

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

@@ -162,6 +162,25 @@ export class SignInExperienceValidator {
     return mandatoryUserProfile;
   }
 
+  /**
+   * If password is enabled in the sign-up settings,
+   * guard the verification record contains password (NewPasswordIdentity).
+   *
+   * - Password is not required for social and SSO verification records.
+   */
+  public async guardMandatoryPasswordOnRegister({ type }: VerificationRecord) {
+    const { signUp } = await this.getSignInExperienceData();
+
+    if (
+      signUp.password &&
+      [VerificationType.EmailVerificationCode, VerificationType.PhoneVerificationCode].includes(
+        type
+      )
+    ) {
+      throw new RequestError({ code: 'user.password_required_in_profile', status: 422 });
+    }
+  }
+
   /**
    * Guard the verification records contains email identifier with SSO enabled
    *

packages/core/src/routes/experience/classes/verifications/new-password-identity-verification.ts

@@ -45,8 +45,7 @@ export const newPasswordIdentityVerificationRecordDataGuard = z.object({
  *
  * @remarks This verification record can only be used for new user registration.
  * By default this verification record allows all types of identifiers, username, email, and phone.
- * But in our current product design, only username + password registration is supported. The identifier type
- * will be guarded at the API level.
+ * For email and phone identifiers, a `CodeVerification` record is required.
  */
 export class NewPasswordIdentityVerification
   implements VerificationRecord<VerificationType.NewPasswordIdentity>

packages/core/src/routes/experience/verification-routes/new-password-identity-verification.ts

@@ -1,4 +1,4 @@
-import { newPasswordIdentityVerificationPayloadGuard, VerificationType } from '@logto/schemas';
+import { passwordVerificationPayloadGuard, VerificationType } from '@logto/schemas';
 import { Action } from '@logto/schemas/lib/types/log/interaction.js';
 import type Router from 'koa-router';
 import { z } from 'zod';
@@ -17,7 +17,7 @@ export default function newPasswordIdentityVerificationRoutes<
   router.post(
     `${experienceRoutes.verification}/new-password-identity`,
     koaGuard({
-      body: newPasswordIdentityVerificationPayloadGuard,
+      body: passwordVerificationPayloadGuard,
       status: [200, 400, 422],
       response: z.object({
         verificationId: z.string(),

packages/integration-tests/src/client/experience/index.ts

@@ -3,7 +3,6 @@ import {
   type IdentificationApiPayload,
   type InteractionEvent,
   type MfaFactor,
-  type NewPasswordIdentityVerificationPayload,
   type PasswordVerificationPayload,
   type UpdateProfileApiPayload,
   type VerificationCodeIdentifier,
@@ -196,9 +195,7 @@ export class ExperienceClient extends MockClient {
       .json<{ verificationId: string }>();
   }
 
-  public async createNewPasswordIdentityVerification(
-    payload: NewPasswordIdentityVerificationPayload
-  ) {
+  public async createNewPasswordIdentityVerification(payload: PasswordVerificationPayload) {
     return api
       .post(`${experienceRoutes.verification}/new-password-identity`, {
         headers: { cookie: this.interactionCookie },

packages/integration-tests/src/helpers/experience/index.ts

@@ -109,28 +109,29 @@ export const registerNewUserWithVerificationCode = async (
     interactionEvent: InteractionEvent.Register,
   });
 
-  const verifiedVerificationId = await successfullyVerifyVerificationCode(client, {
+  await successfullyVerifyVerificationCode(client, {
     identifier,
     verificationId,
     code,
   });
 
-  await client.identifyUser({
-    verificationId: verifiedVerificationId,
-  });
-
   if (options?.fulfillPassword) {
-    await expectRejects(client.submitInteraction(), {
-      code: 'user.missing_profile',
+    await expectRejects(client.identifyUser({ verificationId }), {
+      code: 'user.password_required_in_profile',
       status: 422,
     });
 
     const password = generatePassword();
 
-    await client.updateProfile({
-      type: 'password',
-      value: password,
-    });
+    const { verificationId: newPasswordIdentityVerificationId } =
+      await client.createNewPasswordIdentityVerification({
+        identifier,
+        password,
+      });
+
+    await client.identifyUser({ verificationId: newPasswordIdentityVerificationId });
+  } else {
+    await client.identifyUser({ verificationId });
   }
 
   const { redirectTo } = await client.submitInteraction();