Skip to content

Commit

Permalink
refactor(core): extract isNewMfaVerification property
Browse files Browse the repository at this point in the history
extract isNewMfaVerifrication property
  • Loading branch information
simeng-li committed Jul 26, 2024
1 parent ddae22a commit 1154f7a
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,6 @@ const isMfaVerificationRecord = (
return mfaVerificationTypes.includes(verification.type);

Check warning on line 36 in packages/core/src/routes/experience/classes/libraries/mfa-validator.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/libraries/mfa-validator.ts#L34-L36

Added lines #L34 - L36 were not covered by tests
};

/**
* Check if the MFA verification record is a new bind MFA verification.
* New bind MFA verification can only be used for binding new MFA factors.
*/
const isNewBindMfaVerification = (verification: MfaVerificationRecord) => {
switch (verification.type) {
case VerificationType.TOTP: {
return Boolean(verification.secret);
}
case VerificationType.WebAuthn: {
return Boolean(verification.registrationInfo);
}
case VerificationType.BackupCode: {
return false;
}
}
};

export class MfaValidator {
constructor(
private readonly mfaSettings: Mfa,
Expand Down Expand Up @@ -137,7 +119,7 @@ export class MfaValidator {
isMfaVerificationRecord(verification) &&
verification.isVerified &&
// New bind MFA verification can not be used for verification
!isNewBindMfaVerification(verification) &&
!verification.isNewBindMfaVerification &&

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

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/libraries/mfa-validator.ts#L117-L122

Added lines #L117 - L122 were not covered by tests
// Check if the verification type is enabled in the user's MFA settings
this.userEnabledMfaVerifications.some(
(factor) => factor.type === mfaVerificationTypeToMfaFactorMap[verification.type]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ 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 { type VerificationRecord } from './verification-record.js';
import { type MfaVerificationRecord } from './verification-record.js';

export type BackupCodeVerificationRecordData = {
id: string;
Expand All @@ -24,7 +24,7 @@ export const backupCodeVerificationRecordDataGuard = z.object({
code: z.string().optional(),
}) satisfies ToZodObject<BackupCodeVerificationRecordData>;

export class BackupCodeVerification implements VerificationRecord<VerificationType.BackupCode> {
export class BackupCodeVerification implements MfaVerificationRecord<VerificationType.BackupCode> {
/**
* Factory method to create a new BackupCodeVerification instance
*
Expand Down Expand Up @@ -60,6 +60,10 @@ export class BackupCodeVerification implements VerificationRecord<VerificationTy
return Boolean(this.code);
}

get isNewBindMfaVerification() {
return false;
}

Check warning on line 65 in packages/core/src/routes/experience/classes/verifications/backup-code-verification.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/verifications/backup-code-verification.ts#L64-L65

Added lines #L64 - L65 were not covered by tests

async verify(code: string) {
const {
users: { findUserById, updateUserById },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ 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 { type VerificationRecord } from './verification-record.js';
import { type MfaVerificationRecord } from './verification-record.js';

const defaultDisplayName = 'Unnamed User';

Expand All @@ -47,7 +47,7 @@ export const totpVerificationRecordDataGuard = z.object({
verified: z.boolean(),
}) satisfies ToZodObject<TotpVerificationRecordData>;

export class TotpVerification implements VerificationRecord<VerificationType.TOTP> {
export class TotpVerification implements MfaVerificationRecord<VerificationType.TOTP> {
/**
* Factory method to create a new TotpVerification instance
*
Expand Down Expand Up @@ -90,6 +90,10 @@ export class TotpVerification implements VerificationRecord<VerificationType.TOT
return this.#secret;
}

get isNewBindMfaVerification() {
return Boolean(this.#secret);
}

Check warning on line 96 in packages/core/src/routes/experience/classes/verifications/totp-verification.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/verifications/totp-verification.ts#L89-L96

Added lines #L89 - L96 were not covered by tests
/**
* Create a new TOTP secret and QR code for the user.
* The secret will be stored in the instance and can be used for verifying the TOTP.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,19 @@ export abstract class IdentifierVerificationRecord<
/** Identify the user associated with the verification record. */
abstract identifyUser(): Promise<User>;
}

type MfaVerificationType =
| VerificationType.TOTP
| VerificationType.BackupCode
| VerificationType.WebAuthn;

export abstract class MfaVerificationRecord<
T extends MfaVerificationType = MfaVerificationType,
Json extends Data<T> = Data<T>,
> extends VerificationRecord<T, Json> {
/**
* Indicates if the verification record is for a new bind MFA verification.
* A new bind MFA verification record can not be used for existing user's interaction verification.
**/
abstract get isNewBindMfaVerification(): boolean;
}

Check warning on line 55 in packages/core/src/routes/experience/classes/verifications/verification-record.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/verifications/verification-record.ts#L40-L55

Added lines #L40 - L55 were not covered by tests
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ 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 { type VerificationRecord } from './verification-record.js';
import { type MfaVerificationRecord } from './verification-record.js';

export type WebAuthnVerificationRecordData = {
id: string;
Expand All @@ -50,7 +50,7 @@ export const webAuthnVerificationRecordDataGuard = z.object({
registrationInfo: bindWebAuthnGuard.optional(),
}) satisfies ToZodObject<WebAuthnVerificationRecordData>;

export class WebAuthnVerification implements VerificationRecord<VerificationType.WebAuthn> {
export class WebAuthnVerification implements MfaVerificationRecord<VerificationType.WebAuthn> {
/**
* Factory method to create a new WebAuthnVerification instance
*
Expand Down Expand Up @@ -104,6 +104,10 @@ export class WebAuthnVerification implements VerificationRecord<VerificationType
return this.#registrationInfo;
}

get isNewBindMfaVerification() {
return Boolean(this.#registrationInfo ?? this.registrationChallenge);
}

Check warning on line 110 in packages/core/src/routes/experience/classes/verifications/web-authn-verification.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/experience/classes/verifications/web-authn-verification.ts#L103-L110

Added lines #L103 - L110 were not covered by tests
/**
* @remarks
* This method is used to generate the WebAuthn registration options for the user.
Expand Down

0 comments on commit 1154f7a

Please sign in to comment.