From 05082b56a7999b991053e48e96f20b5d6aefc18d Mon Sep 17 00:00:00 2001 From: simeng-li Date: Fri, 26 Jul 2024 15:33:40 +0800 Subject: [PATCH] refactor(core): refactor backup code generates flow (#6339) refactor(core): refactor backup code generate flow refactor backup code generate flow --- .../core/src/routes/experience/classes/mfa.ts | 63 ++++------------- .../verifications/backup-code-verification.ts | 32 ++++++++- .../verifications/verification-record.ts | 6 +- .../src/routes/experience/profile-routes.ts | 67 ++----------------- .../backup-code-verification.ts | 35 ++++++++++ 5 files changed, 87 insertions(+), 116 deletions(-) diff --git a/packages/core/src/routes/experience/classes/mfa.ts b/packages/core/src/routes/experience/classes/mfa.ts index 15565bb18..0924590c4 100644 --- a/packages/core/src/routes/experience/classes/mfa.ts +++ b/packages/core/src/routes/experience/classes/mfa.ts @@ -18,7 +18,6 @@ import { deduplicate } from '@silverhand/essentials'; import { z } from 'zod'; import RequestError from '#src/errors/RequestError/index.js'; -import { generateBackupCodes } from '#src/routes/interaction/utils/backup-code-validation.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'; @@ -32,8 +31,6 @@ export type MfaData = { totp?: BindTotp; webAuthn?: BindWebAuthn[]; backupCode?: BindBackupCode; - /** The backup codes that have been generated but not yet added to the bindMfa queue */ - pendingBackupCodes?: string[]; }; export const mfaDataGuard = z.object({ @@ -41,7 +38,6 @@ export const mfaDataGuard = z.object({ totp: bindTotpGuard.optional(), webAuthn: z.array(bindWebAuthnGuard).optional(), backupCode: bindBackupCodeGuard.optional(), - pendingBackupCodes: z.array(z.string()).optional(), }) satisfies ToZodObject; export const userMfaDataKey = 'mfa'; @@ -81,15 +77,6 @@ export class Mfa { #totp?: BindTotp; #webAuthn?: BindWebAuthn[]; #backupCode?: BindBackupCode; - /** - * We split the backup codes binding flow into two steps: - * 1. Generate backup codes - * 2. Add backup codes - * - * This is to prevent the user may not receive the backup codes after generating them. - * User need to explicitly send the binding request to add the backup codes. - */ - #pendingBackupCodes?: string[]; constructor( private readonly libraries: Libraries, @@ -98,13 +85,12 @@ export class Mfa { private readonly interactionContext: InteractionContext ) { this.signInExperienceValidator = new SignInExperienceValidator(libraries, queries); - const { mfaSkipped, totp, webAuthn, backupCode, pendingBackupCodes } = data; + const { mfaSkipped, totp, webAuthn, backupCode } = data; this.#mfaSkipped = mfaSkipped; this.#totp = totp; this.#webAuthn = webAuthn; this.#backupCode = backupCode; - this.#pendingBackupCodes = pendingBackupCodes; } get mfaSkipped() { @@ -224,19 +210,25 @@ export class Mfa { } /** - * Generates new backup codes for the user. + * Add new backup codes to the user account. * + * - Any existing backup code factor will be replaced with the new one. + * + * @throws {RequestError} with status 404 if no pending backup codes are found * @throws {RequestError} with status 422 if Backup Code is not enabled in the sign-in experience * @throws {RequestError} with status 422 if the backup code is the only MFA factor - **/ - async generateBackupCodes() { + */ + async addBackupCodeByVerificationId(verificationId: string) { + const verificationRecord = this.interactionContext.getVerificationRecordByTypeAndId( + VerificationType.BackupCode, + verificationId + ); + await this.checkMfaFactorsEnabledInSignInExperience([MfaFactor.BackupCode]); const { mfaVerifications } = await this.interactionContext.getIdentifierUser(); - const userHasOtherMfa = mfaVerifications.some((mfa) => mfa.type !== MfaFactor.BackupCode); const hasOtherNewMfa = Boolean(this.#totp ?? this.#webAuthn?.length); - assertThat( userHasOtherMfa || hasOtherNewMfa, new RequestError({ @@ -245,35 +237,7 @@ export class Mfa { }) ); - const codes = generateBackupCodes(); - this.#pendingBackupCodes = codes; - - return this.#pendingBackupCodes; - } - - /** - * Add backup codes to the user account. - * - * - This is to ensure the user has received the backup codes before adding them to the account. - * - Any existing backup code factor will be replaced with the new one. - * - * @throws {RequestError} with status 404 if no pending backup codes are found - */ - async addBackupCodes() { - assertThat( - this.#pendingBackupCodes?.length, - new RequestError({ - code: 'session.mfa.pending_info_not_found', - status: 404, - }) - ); - - this.#backupCode = { - type: MfaFactor.BackupCode, - codes: this.#pendingBackupCodes, - }; - - this.#pendingBackupCodes = undefined; + this.#backupCode = verificationRecord.toBindMfa(); } /** @@ -338,7 +302,6 @@ export class Mfa { totp: this.#totp, webAuthn: this.#webAuthn, backupCode: this.#backupCode, - pendingBackupCodes: this.#pendingBackupCodes, }; } diff --git a/packages/core/src/routes/experience/classes/verifications/backup-code-verification.ts b/packages/core/src/routes/experience/classes/verifications/backup-code-verification.ts index 6afdafb7d..a717094d8 100644 --- a/packages/core/src/routes/experience/classes/verifications/backup-code-verification.ts +++ b/packages/core/src/routes/experience/classes/verifications/backup-code-verification.ts @@ -1,8 +1,14 @@ import { type ToZodObject } from '@logto/connector-kit'; -import { MfaFactor, VerificationType, type MfaVerificationBackupCode } from '@logto/schemas'; +import { + MfaFactor, + VerificationType, + type BindBackupCode, + type MfaVerificationBackupCode, +} from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; import { z } from 'zod'; +import { generateBackupCodes } from '#src/routes/interaction/utils/backup-code-validation.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'; @@ -15,6 +21,7 @@ export type BackupCodeVerificationRecordData = { /** UserId is required for backup code verification */ userId: string; code?: string; + backupCodes?: string[]; }; export const backupCodeVerificationRecordDataGuard = z.object({ @@ -22,6 +29,7 @@ export const backupCodeVerificationRecordDataGuard = z.object({ type: z.literal(VerificationType.BackupCode), userId: z.string(), code: z.string().optional(), + backupCodes: z.string().array().optional(), }) satisfies ToZodObject; export class BackupCodeVerification implements MfaVerificationRecord { @@ -43,17 +51,19 @@ export class BackupCodeVerification implements MfaVerificationRecord = { id: string; @@ -52,4 +52,8 @@ export abstract class MfaVerificationRecord< * A new bind MFA verification record can not be used for existing user's interaction verification. **/ abstract get isNewBindMfaVerification(): boolean; + /** + * Convert the verification record to a BindMfa data type. + */ + abstract toBindMfa(): BindMfa; } diff --git a/packages/core/src/routes/experience/profile-routes.ts b/packages/core/src/routes/experience/profile-routes.ts index 1358208fd..e41743889 100644 --- a/packages/core/src/routes/experience/profile-routes.ts +++ b/packages/core/src/routes/experience/profile-routes.ts @@ -142,7 +142,7 @@ export default function interactionProfileRoutes( `${experienceRoutes.mfa}`, koaGuard({ body: z.object({ - type: z.literal(MfaFactor.TOTP).or(z.literal(MfaFactor.WebAuthn)), + type: z.nativeEnum(MfaFactor), verificationId: z.string(), }), status: [204, 400, 403, 404, 422], @@ -172,6 +172,10 @@ export default function interactionProfileRoutes( await experienceInteraction.mfa.addWebAuthnByVerificationId(verificationId); break; } + case MfaFactor.BackupCode: { + await experienceInteraction.mfa.addBackupCodeByVerificationId(verificationId); + break; + } } await experienceInteraction.save(); @@ -181,65 +185,4 @@ export default function interactionProfileRoutes( return next(); } ); - - router.post( - `${experienceRoutes.mfa}/backup-codes/generate`, - koaGuard({ - status: [200, 400, 403, 404, 422], - response: z.object({ - codes: z.array(z.string()), - }), - }), - async (ctx, next) => { - const { experienceInteraction } = ctx; - - // Guard current interaction event is not ForgotPassword - assertThat( - experienceInteraction.interactionEvent !== InteractionEvent.ForgotPassword, - new RequestError({ - code: 'session.not_supported_for_forgot_password', - statue: 400, - }) - ); - - // Guard current interaction event is identified and MFA verified - await experienceInteraction.guardMfaVerificationStatus(); - - const backupCodes = await experienceInteraction.mfa.generateBackupCodes(); - - await experienceInteraction.save(); - - ctx.body = { codes: backupCodes }; - - return next(); - } - ); - - router.post( - `${experienceRoutes.mfa}/backup-codes`, - koaGuard({ - status: [204, 400, 403, 404, 422], - }), - async (ctx, next) => { - const { experienceInteraction } = ctx; - - // Guard current interaction event is not ForgotPassword - assertThat( - experienceInteraction.interactionEvent !== InteractionEvent.ForgotPassword, - new RequestError({ - code: 'session.not_supported_for_forgot_password', - statue: 400, - }) - ); - - // Guard current interaction event is identified and MFA verified - await experienceInteraction.guardMfaVerificationStatus(); - await experienceInteraction.mfa.addBackupCodes(); - await experienceInteraction.save(); - - ctx.status = 204; - - return next(); - } - ); } diff --git a/packages/core/src/routes/experience/verification-routes/backup-code-verification.ts b/packages/core/src/routes/experience/verification-routes/backup-code-verification.ts index 413681e2d..496995d08 100644 --- a/packages/core/src/routes/experience/verification-routes/backup-code-verification.ts +++ b/packages/core/src/routes/experience/verification-routes/backup-code-verification.ts @@ -17,6 +17,41 @@ export default function backupCodeVerificationRoutes( ) { const { libraries, queries } = tenantContext; + router.post( + `${experienceRoutes.verification}/backup-code/generate`, + koaGuard({ + status: [200, 400], + response: z.object({ + verificationId: z.string(), + codes: z.array(z.string()), + }), + }), + async (ctx, next) => { + const { experienceInteraction } = ctx; + + assertThat(experienceInteraction.identifiedUserId, 'session.identifier_not_found'); + + const backupCodeVerificationRecord = BackupCodeVerification.create( + libraries, + queries, + experienceInteraction.identifiedUserId + ); + + const codes = backupCodeVerificationRecord.generate(); + + ctx.experienceInteraction.setVerificationRecord(backupCodeVerificationRecord); + + await ctx.experienceInteraction.save(); + + ctx.body = { + verificationId: backupCodeVerificationRecord.id, + codes, + }; + + return next(); + } + ); + router.post( `${experienceRoutes.verification}/backup-code/verify`, koaGuard({