From f56aeb850bc5d6c057d2a5fc145a904c08770537 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Mon, 22 Jul 2024 14:06:24 +0800 Subject: [PATCH] refactor(core): extract password-validator (#6282) * refactor(core): extract password-validator extract password validator * fix(core): update comments and rename method name update comment and rename method name --- .../classes/experience-interaction.ts | 2 +- .../routes/experience/classes/utils.test.ts | 16 +----- .../src/routes/experience/classes/utils.ts | 18 ------ .../classes/validators/password-validator.ts | 57 +++++++++++++++++++ .../classes/validators/profile-validator.ts | 25 +------- .../sign-in-experience-validator.ts | 19 ++----- .../new-password-identity-verification.ts | 22 +++---- .../new-password-identity-verification.ts | 9 +-- 8 files changed, 80 insertions(+), 88 deletions(-) create mode 100644 packages/core/src/routes/experience/classes/validators/password-validator.ts diff --git a/packages/core/src/routes/experience/classes/experience-interaction.ts b/packages/core/src/routes/experience/classes/experience-interaction.ts index 16754d0f679..672d9bfc81a 100644 --- a/packages/core/src/routes/experience/classes/experience-interaction.ts +++ b/packages/core/src/routes/experience/classes/experience-interaction.ts @@ -98,7 +98,7 @@ export default class ExperienceInteraction { this.#profile = profile; // Profile validator requires the userId for existing user profile update validation - this.profileValidator = new ProfileValidator(libraries, queries, userId); + this.profileValidator = new ProfileValidator(libraries, queries); for (const record of verificationRecords) { const instance = buildVerificationRecord(libraries, queries, record); diff --git a/packages/core/src/routes/experience/classes/utils.test.ts b/packages/core/src/routes/experience/classes/utils.test.ts index dbe1f350754..02a146cbd01 100644 --- a/packages/core/src/routes/experience/classes/utils.test.ts +++ b/packages/core/src/routes/experience/classes/utils.test.ts @@ -2,7 +2,7 @@ import { type InteractionIdentifier, SignInIdentifier } from '@logto/schemas'; import { type InteractionProfile } from '../types.js'; -import { interactionIdentifierToUserProfile, profileToUserInfo } from './utils.js'; +import { interactionIdentifierToUserProfile } from './utils.js'; const identifierToProfileTestCase = [ { @@ -35,18 +35,4 @@ describe('experience utils tests', () => { expect(interactionIdentifierToUserProfile(identifier)).toEqual(expected); } ); - it('profileToUserInfo', () => { - expect( - profileToUserInfo({ - username: 'username', - primaryEmail: 'email', - primaryPhone: 'phone', - }) - ).toEqual({ - name: undefined, - username: 'username', - email: 'email', - phoneNumber: 'phone', - }); - }); }); diff --git a/packages/core/src/routes/experience/classes/utils.ts b/packages/core/src/routes/experience/classes/utils.ts index fe0011e52f4..e2949d8ec00 100644 --- a/packages/core/src/routes/experience/classes/utils.ts +++ b/packages/core/src/routes/experience/classes/utils.ts @@ -1,4 +1,3 @@ -import { type UserInfo } from '@logto/core-kit'; import { SignInIdentifier, VerificationType, @@ -158,23 +157,6 @@ export function interactionIdentifierToUserProfile( } } -/** - * This function is used to convert the interaction profile to the UserInfo format. - * It will be used by the PasswordPolicyChecker to check the password policy against the user profile. - */ -export function profileToUserInfo( - profile: Pick -): UserInfo { - const { name, username, primaryEmail, primaryPhone } = profile; - - return { - name: name ?? undefined, - username: username ?? undefined, - email: primaryEmail ?? undefined, - phoneNumber: primaryPhone ?? undefined, - }; -} - export const codeVerificationIdentifierRecordTypeMap = Object.freeze({ [SignInIdentifier.Email]: VerificationType.EmailVerificationCode, [SignInIdentifier.Phone]: VerificationType.PhoneVerificationCode, diff --git a/packages/core/src/routes/experience/classes/validators/password-validator.ts b/packages/core/src/routes/experience/classes/validators/password-validator.ts new file mode 100644 index 00000000000..edc0693200a --- /dev/null +++ b/packages/core/src/routes/experience/classes/validators/password-validator.ts @@ -0,0 +1,57 @@ +import { PasswordPolicyChecker, type UserInfo } from '@logto/core-kit'; +import { type SignInExperience, type User } from '@logto/schemas'; + +import RequestError from '#src/errors/RequestError/index.js'; +import { encryptUserPassword } from '#src/libraries/user.utils.js'; + +import { type InteractionProfile } from '../../types.js'; + +function getUserInfo({ + user, + profile, +}: { + user?: User; + profile?: Pick; +}): UserInfo { + return { + name: profile?.name ?? user?.name ?? undefined, + username: profile?.username ?? user?.username ?? undefined, + email: profile?.primaryEmail ?? user?.primaryEmail ?? undefined, + phoneNumber: profile?.primaryPhone ?? user?.primaryPhone ?? undefined, + }; +} + +export class PasswordValidator { + public readonly passwordPolicyChecker: PasswordPolicyChecker; + + constructor( + private readonly passwordPolicy: SignInExperience['passwordPolicy'], + private readonly user?: User + ) { + this.passwordPolicyChecker = new PasswordPolicyChecker(passwordPolicy); + } + + /** + * Validate password against the password policy + * @throws {RequestError} with status 422 if the password does not meet the policy + */ + public async validatePassword(password: string, profile: InteractionProfile) { + const userInfo = getUserInfo({ + user: this.user, + profile, + }); + + const issues = await this.passwordPolicyChecker.check( + password, + this.passwordPolicyChecker.policy.rejects.userInfo ? userInfo : {} + ); + + if (issues.length > 0) { + throw new RequestError({ code: 'password.rejected', status: 422 }, { issues }); + } + } + + public async createPasswordDigest(password: string) { + return encryptUserPassword(password); + } +} diff --git a/packages/core/src/routes/experience/classes/validators/profile-validator.ts b/packages/core/src/routes/experience/classes/validators/profile-validator.ts index dc6a5848981..f5eb01d1466 100644 --- a/packages/core/src/routes/experience/classes/validators/profile-validator.ts +++ b/packages/core/src/routes/experience/classes/validators/profile-validator.ts @@ -1,5 +1,3 @@ -import { type PasswordPolicyChecker, type UserInfo } from '@logto/core-kit'; - import RequestError from '#src/errors/RequestError/index.js'; import type Libraries from '#src/tenants/Libraries.js'; import type Queries from '#src/tenants/Queries.js'; @@ -10,9 +8,7 @@ import type { InteractionProfile } from '../../types.js'; export class ProfileValidator { constructor( private readonly libraries: Libraries, - private readonly queries: Queries, - /** UserId is required for existing user profile update validation */ - private readonly userId?: string + private readonly queries: Queries ) {} public async guardProfileUniquenessAcrossUsers(profile: InteractionProfile) { @@ -82,23 +78,4 @@ export class ProfileValidator { ); } } - - /** - * Validate password against the given password policy - * throw a {@link RequestError} -422 if the password is invalid; otherwise, do nothing. - */ - public async validatePassword( - password: string, - passwordPolicyChecker: PasswordPolicyChecker, - userInfo: UserInfo = {} - ) { - const issues = await passwordPolicyChecker.check( - password, - passwordPolicyChecker.policy.rejects.userInfo ? userInfo : {} - ); - - if (issues.length > 0) { - throw new RequestError({ code: 'password.rejected', status: 422 }, { issues }); - } - } } diff --git a/packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.ts b/packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.ts index 08b912ae294..201db65a9ad 100644 --- a/packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.ts +++ b/packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.ts @@ -1,6 +1,3 @@ -import crypto from 'node:crypto'; - -import { PasswordPolicyChecker } from '@logto/core-kit'; import { InteractionEvent, type SignInExperience, @@ -46,7 +43,6 @@ const getEmailIdentifierFromVerificationRecord = (verificationRecord: Verificati */ export class SignInExperienceValidator { private signInExperienceDataCache?: SignInExperience; - #passwordPolicyChecker?: PasswordPolicyChecker; constructor( private readonly libraries: Libraries, @@ -119,6 +115,12 @@ export class SignInExperienceValidator { return mfa; } + public async getPasswordPolicy() { + const { passwordPolicy } = await this.getSignInExperienceData(); + + return passwordPolicy; + } + public async getSignInExperienceData() { this.signInExperienceDataCache ||= await this.queries.signInExperiences.findDefaultSignInExperience(); @@ -126,15 +128,6 @@ export class SignInExperienceValidator { return this.signInExperienceDataCache; } - public async getPasswordPolicyChecker() { - if (!this.#passwordPolicyChecker) { - const { passwordPolicy } = await this.getSignInExperienceData(); - this.#passwordPolicyChecker = new PasswordPolicyChecker(passwordPolicy, crypto.subtle); - } - - return this.#passwordPolicyChecker; - } - /** * Guard the verification records contains email identifier with SSO enabled * diff --git a/packages/core/src/routes/experience/classes/verifications/new-password-identity-verification.ts b/packages/core/src/routes/experience/classes/verifications/new-password-identity-verification.ts index bb40c3b1284..8b51787f87d 100644 --- a/packages/core/src/routes/experience/classes/verifications/new-password-identity-verification.ts +++ b/packages/core/src/routes/experience/classes/verifications/new-password-identity-verification.ts @@ -1,5 +1,4 @@ import { type ToZodObject } from '@logto/connector-kit'; -import { type PasswordPolicyChecker } from '@logto/core-kit'; import { type InteractionIdentifier, interactionIdentifierGuard, @@ -10,13 +9,14 @@ import { generateStandardId } from '@logto/shared'; import { z } from 'zod'; import RequestError from '#src/errors/RequestError/index.js'; -import { encryptUserPassword } from '#src/libraries/user.utils.js'; import type Libraries from '#src/tenants/Libraries.js'; import type Queries from '#src/tenants/Queries.js'; import assertThat from '#src/utils/assert-that.js'; -import { interactionIdentifierToUserProfile, profileToUserInfo } from '../utils.js'; +import { interactionIdentifierToUserProfile } from '../utils.js'; +import { PasswordValidator } from '../validators/password-validator.js'; import { ProfileValidator } from '../validators/profile-validator.js'; +import { SignInExperienceValidator } from '../validators/sign-in-experience-validator.js'; import { type VerificationRecord } from './verification-record.js'; @@ -68,6 +68,7 @@ export class NewPasswordIdentityVerification private passwordEncryptionMethod?: UsersPasswordEncryptionMethod.Argon2i; private readonly profileValidator: ProfileValidator; + private readonly signInExperienceValidator: SignInExperienceValidator; constructor( private readonly libraries: Libraries, @@ -81,6 +82,7 @@ export class NewPasswordIdentityVerification this.passwordEncrypted = passwordEncrypted; this.passwordEncryptionMethod = passwordEncryptionMethod; this.profileValidator = new ProfileValidator(libraries, queries); + this.signInExperienceValidator = new SignInExperienceValidator(libraries, queries); } get isVerified() { @@ -93,19 +95,17 @@ export class NewPasswordIdentityVerification * - Check if the identifier is unique across users * - Validate the password against the password policy */ - async verify(password: string, passwordPolicyChecker: PasswordPolicyChecker) { + async verify(password: string) { const { identifier } = this; const identifierProfile = interactionIdentifierToUserProfile(identifier); - await this.profileValidator.guardProfileUniquenessAcrossUsers(identifierProfile); - await this.profileValidator.validatePassword( - password, - passwordPolicyChecker, - profileToUserInfo(identifierProfile) - ); + const passwordPolicy = await this.signInExperienceValidator.getPasswordPolicy(); + const passwordValidator = new PasswordValidator(passwordPolicy); + await passwordValidator.validatePassword(password, identifierProfile); - const { passwordEncrypted, passwordEncryptionMethod } = await encryptUserPassword(password); + const { passwordEncrypted, passwordEncryptionMethod } = + await passwordValidator.createPasswordDigest(password); this.passwordEncrypted = passwordEncrypted; this.passwordEncryptionMethod = passwordEncryptionMethod; diff --git a/packages/core/src/routes/experience/verification-routes/new-password-identity-verification.ts b/packages/core/src/routes/experience/verification-routes/new-password-identity-verification.ts index d6def4b28ab..2d41e754934 100644 --- a/packages/core/src/routes/experience/verification-routes/new-password-identity-verification.ts +++ b/packages/core/src/routes/experience/verification-routes/new-password-identity-verification.ts @@ -33,14 +33,11 @@ export default function newPasswordIdentityVerificationRoutes