Skip to content

Commit

Permalink
refactor(core): extract password-validator (#6282)
Browse files Browse the repository at this point in the history
* refactor(core): extract password-validator

extract password validator

* fix(core): update comments and rename method name

update comment and rename method name
  • Loading branch information
simeng-li committed Jul 22, 2024
1 parent 3538b1e commit f56aeb8
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 1 addition & 15 deletions packages/core/src/routes/experience/classes/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
{
Expand Down Expand Up @@ -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',
});
});
});
18 changes: 0 additions & 18 deletions packages/core/src/routes/experience/classes/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { type UserInfo } from '@logto/core-kit';
import {
SignInIdentifier,
VerificationType,
Expand Down Expand Up @@ -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<InteractionProfile, 'name' | 'username' | 'primaryEmail' | 'primaryPhone'>
): 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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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<InteractionProfile, 'name' | 'username' | 'primaryEmail' | 'primaryPhone'>;
}): 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);
}
}
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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) {
Expand Down Expand Up @@ -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 });
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
import crypto from 'node:crypto';

import { PasswordPolicyChecker } from '@logto/core-kit';
import {
InteractionEvent,
type SignInExperience,
Expand Down Expand Up @@ -46,7 +43,6 @@ const getEmailIdentifierFromVerificationRecord = (verificationRecord: Verificati
*/
export class SignInExperienceValidator {
private signInExperienceDataCache?: SignInExperience;
#passwordPolicyChecker?: PasswordPolicyChecker;

constructor(
private readonly libraries: Libraries,
Expand Down Expand Up @@ -119,22 +115,19 @@ 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();

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
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { type ToZodObject } from '@logto/connector-kit';
import { type PasswordPolicyChecker } from '@logto/core-kit';
import {
type InteractionIdentifier,
interactionIdentifierGuard,
Expand All @@ -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';

Expand Down Expand Up @@ -68,6 +68,7 @@ export class NewPasswordIdentityVerification
private passwordEncryptionMethod?: UsersPasswordEncryptionMethod.Argon2i;

private readonly profileValidator: ProfileValidator;
private readonly signInExperienceValidator: SignInExperienceValidator;

constructor(
private readonly libraries: Libraries,
Expand All @@ -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() {
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,11 @@ export default function newPasswordIdentityVerificationRoutes<T extends WithLogC
identifier
);

const policyChecker =
await experienceInteraction.signInExperienceValidator.getPasswordPolicyChecker();
await newPasswordIdentityVerification.verify(password);

await newPasswordIdentityVerification.verify(password, policyChecker);
experienceInteraction.setVerificationRecord(newPasswordIdentityVerification);

ctx.experienceInteraction.setVerificationRecord(newPasswordIdentityVerification);

await ctx.experienceInteraction.save();
await experienceInteraction.save();

ctx.body = { verificationId: newPasswordIdentityVerification.id };

Expand Down

0 comments on commit f56aeb8

Please sign in to comment.