Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(core): extract password-validator #6282

Merged
merged 2 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
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 Expand Up @@ -255,7 +255,7 @@

const { socialIdentity, enterpriseSsoIdentity, ...rest } = this.#profile ?? {};

// TODO: profile updates validation

Check warning on line 258 in packages/core/src/routes/experience/classes/experience-interaction.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[no-warning-comments] Unexpected 'todo' comment: 'TODO: profile updates validation'.

// Update user profile
await userQueries.updateUserById(user.id, {
Expand All @@ -271,7 +271,7 @@
lastSignInAt: Date.now(),
});

// TODO: missing profile fields validation

Check warning on line 274 in packages/core/src/routes/experience/classes/experience-interaction.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[no-warning-comments] Unexpected 'todo' comment: 'TODO: missing profile fields validation'.

if (enterpriseSsoIdentity) {
await userSsoIdentitiesQueries.insert({
Expand Down Expand Up @@ -396,7 +396,7 @@

await this.provisionLibrary.newUserJtiOrganizationProvision(user.id, newProfile);

// TODO: new user hooks

Check warning on line 399 in packages/core/src/routes/experience/classes/experience-interaction.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[no-warning-comments] Unexpected 'todo' comment: 'TODO: new user hooks'.

this.userId = user.id;
}
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,
};
}

Check warning on line 22 in packages/core/src/routes/experience/classes/validators/password-validator.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/validators/password-validator.ts#L9-L22

Added lines #L9 - L22 were not covered by tests

export class PasswordValidator {
public readonly passwordPolicyChecker: PasswordPolicyChecker;

constructor(
private readonly passwordPolicy: SignInExperience['passwordPolicy'],
private readonly user?: User
) {
this.passwordPolicyChecker = new PasswordPolicyChecker(passwordPolicy);
}

Check warning on line 32 in packages/core/src/routes/experience/classes/validators/password-validator.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/validators/password-validator.ts#L28-L32

Added lines #L28 - L32 were not covered by tests

/**
* 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 });
}
}

Check warning on line 52 in packages/core/src/routes/experience/classes/validators/password-validator.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/validators/password-validator.ts#L39-L52

Added lines #L39 - L52 were not covered by tests

public async createPasswordDigest(password: string) {
return encryptUserPassword(password);
}

Check warning on line 56 in packages/core/src/routes/experience/classes/validators/password-validator.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/validators/password-validator.ts#L55-L56

Added lines #L55 - L56 were not covered by tests
}
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 @@
*/
export class SignInExperienceValidator {
private signInExperienceDataCache?: SignInExperience;
#passwordPolicyChecker?: PasswordPolicyChecker;

constructor(
private readonly libraries: Libraries,
Expand Down Expand Up @@ -119,22 +115,19 @@
return mfa;
}

public async getPasswordPolicy() {
const { passwordPolicy } = await this.getSignInExperienceData();

return passwordPolicy;
}

Check warning on line 122 in packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.ts#L119-L122

Added lines #L119 - L122 were not covered by tests

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 { 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 @@
private passwordEncryptionMethod?: UsersPasswordEncryptionMethod.Argon2i;

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

constructor(
private readonly libraries: Libraries,
Expand All @@ -81,6 +82,7 @@
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 @@
* - 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);

Check warning on line 105 in packages/core/src/routes/experience/classes/verifications/new-password-identity-verification.ts

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L103 - L105 were not covered by tests

const { passwordEncrypted, passwordEncryptionMethod } = await encryptUserPassword(password);
const { passwordEncrypted, passwordEncryptionMethod } =
await passwordValidator.createPasswordDigest(password);

Check warning on line 108 in packages/core/src/routes/experience/classes/verifications/new-password-identity-verification.ts

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L107 - L108 were not covered by tests

this.passwordEncrypted = passwordEncrypted;
this.passwordEncryptionMethod = passwordEncryptionMethod;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,11 @@
identifier
);

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

Check warning on line 36 in packages/core/src/routes/experience/verification-routes/new-password-identity-verification.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L36 was not covered by tests

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

Check warning on line 38 in packages/core/src/routes/experience/verification-routes/new-password-identity-verification.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L38 was not covered by tests

ctx.experienceInteraction.setVerificationRecord(newPasswordIdentityVerification);

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

Check warning on line 40 in packages/core/src/routes/experience/verification-routes/new-password-identity-verification.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L40 was not covered by tests

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

Expand Down
Loading