From de617353533ae447e1258bd8a81ba0d354a19186 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Mon, 22 Jul 2024 10:36:25 +0800 Subject: [PATCH] refactor(core,schemas): refactor `CodeVerification` (#6277) * refactor(core,schemas): refactor the CodeVerification class split the CodeVerification class into EmailCodeVerification and PhoneCodeVerification * refactor(core,schemas): split CodeVerification type split CodeVerification type * fix(core): code review updates code review updates --- .../classes/experience-interaction.test.ts | 6 +- .../src/routes/experience/classes/utils.ts | 12 +- .../sign-in-experience-validator.test.ts | 6 +- .../sign-in-experience-validator.ts | 15 +- .../verifications/code-verification.ts | 214 ++++++++++++------ .../experience/classes/verifications/index.ts | 22 +- .../verifications/verification-record.ts | 3 +- .../verification-routes/verification-code.ts | 13 +- packages/schemas/src/types/interactions.ts | 12 +- 9 files changed, 204 insertions(+), 99 deletions(-) diff --git a/packages/core/src/routes/experience/classes/experience-interaction.test.ts b/packages/core/src/routes/experience/classes/experience-interaction.test.ts index 561df47d9..43c2ce607 100644 --- a/packages/core/src/routes/experience/classes/experience-interaction.test.ts +++ b/packages/core/src/routes/experience/classes/experience-interaction.test.ts @@ -17,7 +17,7 @@ import { createMockProvider } from '#src/test-utils/oidc-provider.js'; import { MockTenant } from '#src/test-utils/tenant.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; -import { CodeVerification } from './verifications/code-verification.js'; +import { EmailCodeVerification } from './verifications/code-verification.js'; const { jest } = import.meta; const { mockEsm } = createMockUtils(jest); @@ -75,9 +75,9 @@ describe('ExperienceInteraction class', () => { }; const { libraries, queries } = tenant; - const emailVerificationRecord = new CodeVerification(libraries, queries, { + const emailVerificationRecord = new EmailCodeVerification(libraries, queries, { id: 'mock_email_verification_id', - type: VerificationType.VerificationCode, + type: VerificationType.EmailVerificationCode, identifier: { type: SignInIdentifier.Email, value: mockEmail, diff --git a/packages/core/src/routes/experience/classes/utils.ts b/packages/core/src/routes/experience/classes/utils.ts index 9aebaf7b7..fe0011e52 100644 --- a/packages/core/src/routes/experience/classes/utils.ts +++ b/packages/core/src/routes/experience/classes/utils.ts @@ -4,6 +4,7 @@ import { VerificationType, type InteractionIdentifier, type User, + type VerificationCodeSignInIdentifier, } from '@logto/schemas'; import { conditional } from '@silverhand/essentials'; @@ -41,7 +42,8 @@ export const getNewUserProfileFromVerificationRecord = async ( ): Promise => { switch (verificationRecord.type) { case VerificationType.NewPasswordIdentity: - case VerificationType.VerificationCode: { + case VerificationType.EmailVerificationCode: + case VerificationType.PhoneVerificationCode: { return verificationRecord.toUserProfile(); } case VerificationType.EnterpriseSso: @@ -86,7 +88,8 @@ export const identifyUserByVerificationRecord = async ( switch (verificationRecord.type) { case VerificationType.Password: - case VerificationType.VerificationCode: { + case VerificationType.EmailVerificationCode: + case VerificationType.PhoneVerificationCode: { return { user: await verificationRecord.identifyUser() }; } case VerificationType.Social: { @@ -171,3 +174,8 @@ export function profileToUserInfo( phoneNumber: primaryPhone ?? undefined, }; } + +export const codeVerificationIdentifierRecordTypeMap = Object.freeze({ + [SignInIdentifier.Email]: VerificationType.EmailVerificationCode, + [SignInIdentifier.Phone]: VerificationType.PhoneVerificationCode, +}) satisfies Record; diff --git a/packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.test.ts b/packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.test.ts index 83adb6998..44b3839bc 100644 --- a/packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.test.ts +++ b/packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.test.ts @@ -11,7 +11,7 @@ import { mockSignInExperience } from '#src/__mocks__/sign-in-experience.js'; import RequestError from '#src/errors/RequestError/index.js'; import { MockTenant } from '#src/test-utils/tenant.js'; -import { CodeVerification } from '../verifications/code-verification.js'; +import { createNewCodeVerificationRecord } from '../verifications/code-verification.js'; import { EnterpriseSsoVerification } from '../verifications/enterprise-sso-verification.js'; import { type VerificationRecord } from '../verifications/index.js'; import { NewPasswordIdentityVerification } from '../verifications/new-password-identity-verification.js'; @@ -53,7 +53,7 @@ const passwordVerificationRecords = Object.fromEntries( ) as Record; const verificationCodeVerificationRecords = Object.freeze({ - [SignInIdentifier.Email]: CodeVerification.create( + [SignInIdentifier.Email]: createNewCodeVerificationRecord( mockTenant.libraries, mockTenant.queries, { @@ -62,7 +62,7 @@ const verificationCodeVerificationRecords = Object.freeze({ }, InteractionEvent.SignIn ), - [SignInIdentifier.Phone]: CodeVerification.create( + [SignInIdentifier.Phone]: createNewCodeVerificationRecord( mockTenant.libraries, mockTenant.queries, { 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 9a22b63c2..08b912ae2 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 @@ -4,6 +4,7 @@ import { PasswordPolicyChecker } from '@logto/core-kit'; import { InteractionEvent, type SignInExperience, + SignInIdentifier, SignInMode, VerificationType, } from '@logto/schemas'; @@ -18,12 +19,13 @@ import { type VerificationRecord } from '../verifications/index.js'; const getEmailIdentifierFromVerificationRecord = (verificationRecord: VerificationRecord) => { switch (verificationRecord.type) { case VerificationType.Password: - case VerificationType.VerificationCode: { + case VerificationType.EmailVerificationCode: + case VerificationType.PhoneVerificationCode: { const { identifier: { type, value }, } = verificationRecord; - return type === 'email' ? value : undefined; + return type === SignInIdentifier.Email ? value : undefined; } case VerificationType.Social: { const { socialUserInfo } = verificationRecord; @@ -174,7 +176,8 @@ export class SignInExperienceValidator { switch (verificationRecord.type) { case VerificationType.Password: - case VerificationType.VerificationCode: { + case VerificationType.EmailVerificationCode: + case VerificationType.PhoneVerificationCode: { const { identifier: { type }, } = verificationRecord; @@ -224,7 +227,8 @@ export class SignInExperienceValidator { ); break; } - case VerificationType.VerificationCode: { + case VerificationType.EmailVerificationCode: + case VerificationType.PhoneVerificationCode: { const { identifier: { type }, } = verificationRecord; @@ -255,7 +259,8 @@ export class SignInExperienceValidator { /** Forgot password only supports verification code type verification record */ private guardForgotPasswordVerificationMethod(verificationRecord: VerificationRecord) { assertThat( - verificationRecord.type === VerificationType.VerificationCode, + verificationRecord.type === VerificationType.EmailVerificationCode || + verificationRecord.type === VerificationType.PhoneVerificationCode, new RequestError({ code: 'session.not_supported_for_forgot_password', status: 422 }) ); } diff --git a/packages/core/src/routes/experience/classes/verifications/code-verification.ts b/packages/core/src/routes/experience/classes/verifications/code-verification.ts index e9879c972..0ccbc2f89 100644 --- a/packages/core/src/routes/experience/classes/verifications/code-verification.ts +++ b/packages/core/src/routes/experience/classes/verifications/code-verification.ts @@ -1,12 +1,11 @@ -import { TemplateType } from '@logto/connector-kit'; +import { TemplateType, type ToZodObject } from '@logto/connector-kit'; import { InteractionEvent, + SignInIdentifier, VerificationType, - verificationCodeIdentifierGuard, type User, type VerificationCodeIdentifier, } from '@logto/schemas'; -import { type ToZodObject } from '@logto/schemas/lib/utils/zod.js'; import { generateStandardId } from '@logto/shared'; import { z } from 'zod'; @@ -35,64 +34,47 @@ const eventToTemplateTypeMap: Record = { const getTemplateTypeByEvent = (event: InteractionEvent): TemplateType => eventToTemplateTypeMap[event]; -/** The JSON data type for the CodeVerification record */ -export type CodeVerificationRecordData = { - id: string; - type: VerificationType.VerificationCode; - identifier: VerificationCodeIdentifier; - interactionEvent: InteractionEvent; - verified: boolean; -}; - -export const codeVerificationRecordDataGuard = z.object({ - id: z.string(), - type: z.literal(VerificationType.VerificationCode), - identifier: verificationCodeIdentifierGuard, - interactionEvent: z.nativeEnum(InteractionEvent), - verified: z.boolean(), -}) satisfies ToZodObject; - /** This util method convert the interaction identifier to passcode library payload format */ const getPasscodeIdentifierPayload = ( identifier: VerificationCodeIdentifier ): Parameters['createPasscode']>[2] => identifier.type === 'email' ? { email: identifier.value } : { phone: identifier.value }; +type CodeVerificationType = + | VerificationType.EmailVerificationCode + | VerificationType.PhoneVerificationCode; + +type SinInIdentifierTypeOf = { + [VerificationType.EmailVerificationCode]: SignInIdentifier.Email; + [VerificationType.PhoneVerificationCode]: SignInIdentifier.Phone; +}; + +type VerificationCodeIdentifierOf = VerificationCodeIdentifier< + SinInIdentifierTypeOf[T] +>; + +type CodeVerificationIdentifierMap = { + [VerificationType.EmailVerificationCode]: { primaryEmail: string }; + [VerificationType.PhoneVerificationCode]: { primaryPhone: string }; +}; + +/** The JSON data type for the `CodeVerification` record */ +export type CodeVerificationRecordData = { + id: string; + type: T; + identifier: VerificationCodeIdentifierOf; + interactionEvent: InteractionEvent; + verified: boolean; +}; + /** - * CodeVerification is a verification type that verifies a given identifier by sending a verification code - * to the user's email or phone. - * - * @remark The verification code is sent to the user's email or phone and the user is required to enter the code to verify. - * If the identifier is for a existing user, the userId will be set after the verification. - * - * To avoid the redundant naming, the `CodeVerification` is used instead of `VerificationCodeVerification`. + * This is the parent class for `EmailCodeVerification` and `PhoneCodeVerification`. Not publicly exposed. */ -export class CodeVerification - implements IdentifierVerificationRecord +abstract class CodeVerification + implements IdentifierVerificationRecord { - /** - * Factory method to create a new CodeVerification record using the given identifier. - */ - static create( - libraries: Libraries, - queries: Queries, - identifier: VerificationCodeIdentifier, - interactionEvent: InteractionEvent - ) { - const record = new CodeVerification(libraries, queries, { - id: generateStandardId(), - type: VerificationType.VerificationCode, - identifier, - interactionEvent, - verified: false, - }); - - return record; - } - public readonly id: string; - public readonly type = VerificationType.VerificationCode; - public readonly identifier: VerificationCodeIdentifier; + public readonly identifier: VerificationCodeIdentifierOf; /** * The interaction event that triggered the verification. @@ -102,12 +84,13 @@ export class CodeVerification * `InteractionEvent.ForgotPassword` triggered verification results can not used as a verification record for other events. */ public readonly interactionEvent: InteractionEvent; - private verified: boolean; + public abstract readonly type: T; + protected verified: boolean; constructor( private readonly libraries: Libraries, private readonly queries: Queries, - data: CodeVerificationRecordData + data: CodeVerificationRecordData ) { const { id, identifier, verified, interactionEvent } = data; @@ -144,7 +127,8 @@ export class CodeVerification /** * Verify the `identifier` with the given code * - * @remark The identifier and code will be verified in the passcode library. + * @remarks + * The identifier and code will be verified in the passcode library. * No need to verify the identifier before calling this method. * * - `isVerified` will be set to true if the code is verified successfully. @@ -187,18 +171,7 @@ export class CodeVerification return user; } - toUserProfile(): { primaryEmail: string } | { primaryPhone: string } { - assertThat( - this.verified, - new RequestError({ code: 'session.verification_failed', status: 400 }) - ); - - const { type, value } = this.identifier; - - return type === 'email' ? { primaryEmail: value } : { primaryPhone: value }; - } - - toJson(): CodeVerificationRecordData { + toJson(): CodeVerificationRecordData { const { id, type, identifier, interactionEvent, verified } = this; return { @@ -209,4 +182,113 @@ export class CodeVerification verified, }; } + + abstract toUserProfile(): CodeVerificationIdentifierMap[T]; } + +const basicCodeVerificationRecordDataGuard = z.object({ + id: z.string(), + interactionEvent: z.nativeEnum(InteractionEvent), + verified: z.boolean(), +}); + +/** + * A verification code class that verifies a given email identifier. + * + * @remarks + * The verification code is sent to the user's email and the user is required to enter the exact same code to + * complete the process. If the identifier is for an existing user, the `userId` will be set after the verification. + */ +export class EmailCodeVerification extends CodeVerification { + public readonly type = VerificationType.EmailVerificationCode; + + toUserProfile(): { primaryEmail: string } { + assertThat( + this.verified, + new RequestError({ + code: 'session.verification_failed', + state: 400, + }) + ); + + const { value } = this.identifier; + + return { primaryEmail: value }; + } +} + +export const emailCodeVerificationRecordDataGuard = basicCodeVerificationRecordDataGuard.extend({ + type: z.literal(VerificationType.EmailVerificationCode), + identifier: z.object({ + type: z.literal(SignInIdentifier.Email), + value: z.string(), + }), +}) satisfies ToZodObject>; + +/** + * A verification code class that verifies a given phone identifier. + * + * @remarks + * The verification code will be sent to the user's phone and the user is required to enter the exact same code to + * complete the process. If the identifier is for an existing user, the `userId` will be set after the verification. + */ +export class PhoneCodeVerification extends CodeVerification { + public readonly type = VerificationType.PhoneVerificationCode; + + toUserProfile(): { primaryPhone: string } { + assertThat( + this.verified, + new RequestError({ + code: 'session.verification_failed', + state: 400, + }) + ); + + const { value } = this.identifier; + + return { primaryPhone: value }; + } +} + +export const phoneCodeVerificationRecordDataGuard = basicCodeVerificationRecordDataGuard.extend({ + type: z.literal(VerificationType.PhoneVerificationCode), + identifier: z.object({ + type: z.literal(SignInIdentifier.Phone), + value: z.string(), + }), +}) satisfies ToZodObject>; + +/** + * Factory method to create a new `EmailCodeVerification` / `PhoneCodeVerification` record using the given identifier. + */ +export const createNewCodeVerificationRecord = ( + libraries: Libraries, + queries: Queries, + identifier: + | VerificationCodeIdentifier + | VerificationCodeIdentifier, + interactionEvent: InteractionEvent +) => { + const { type } = identifier; + + switch (type) { + case SignInIdentifier.Email: { + return new EmailCodeVerification(libraries, queries, { + id: generateStandardId(), + type: VerificationType.EmailVerificationCode, + identifier, + interactionEvent, + verified: false, + }); + } + case SignInIdentifier.Phone: { + return new PhoneCodeVerification(libraries, queries, { + id: generateStandardId(), + type: VerificationType.PhoneVerificationCode, + identifier, + interactionEvent, + verified: false, + }); + } + } +}; diff --git a/packages/core/src/routes/experience/classes/verifications/index.ts b/packages/core/src/routes/experience/classes/verifications/index.ts index 8522de9a9..bc1ac79bd 100644 --- a/packages/core/src/routes/experience/classes/verifications/index.ts +++ b/packages/core/src/routes/experience/classes/verifications/index.ts @@ -10,8 +10,10 @@ import { type BackupCodeVerificationRecordData, } from './backup-code-verification.js'; import { - CodeVerification, - codeVerificationRecordDataGuard, + EmailCodeVerification, + emailCodeVerificationRecordDataGuard, + PhoneCodeVerification, + phoneCodeVerificationRecordDataGuard, type CodeVerificationRecordData, } from './code-verification.js'; import { @@ -42,7 +44,8 @@ import { export type VerificationRecordData = | PasswordVerificationRecordData - | CodeVerificationRecordData + | CodeVerificationRecordData + | CodeVerificationRecordData | SocialVerificationRecordData | EnterpriseSsoVerificationRecordData | TotpVerificationRecordData @@ -59,7 +62,8 @@ export type VerificationRecordData = */ export type VerificationRecord = | PasswordVerification - | CodeVerification + | EmailCodeVerification + | PhoneCodeVerification | SocialVerification | EnterpriseSsoVerification | TotpVerification @@ -68,7 +72,8 @@ export type VerificationRecord = export const verificationRecordDataGuard = z.discriminatedUnion('type', [ passwordVerificationRecordDataGuard, - codeVerificationRecordDataGuard, + emailCodeVerificationRecordDataGuard, + phoneCodeVerificationRecordDataGuard, socialVerificationRecordDataGuard, enterPriseSsoVerificationRecordDataGuard, totpVerificationRecordDataGuard, @@ -88,8 +93,11 @@ export const buildVerificationRecord = ( case VerificationType.Password: { return new PasswordVerification(libraries, queries, data); } - case VerificationType.VerificationCode: { - return new CodeVerification(libraries, queries, data); + case VerificationType.EmailVerificationCode: { + return new EmailCodeVerification(libraries, queries, data); + } + case VerificationType.PhoneVerificationCode: { + return new PhoneCodeVerification(libraries, queries, data); } case VerificationType.Social: { return new SocialVerification(libraries, queries, data); diff --git a/packages/core/src/routes/experience/classes/verifications/verification-record.ts b/packages/core/src/routes/experience/classes/verifications/verification-record.ts index 6eff83275..a9ff16fb8 100644 --- a/packages/core/src/routes/experience/classes/verifications/verification-record.ts +++ b/packages/core/src/routes/experience/classes/verifications/verification-record.ts @@ -19,7 +19,8 @@ export abstract class VerificationRecord< } type IdentifierVerificationType = - | VerificationType.VerificationCode + | VerificationType.EmailVerificationCode + | VerificationType.PhoneVerificationCode | VerificationType.Password | VerificationType.Social | VerificationType.EnterpriseSso; diff --git a/packages/core/src/routes/experience/verification-routes/verification-code.ts b/packages/core/src/routes/experience/verification-routes/verification-code.ts index 1e99894b5..6c9654ba4 100644 --- a/packages/core/src/routes/experience/verification-routes/verification-code.ts +++ b/packages/core/src/routes/experience/verification-routes/verification-code.ts @@ -1,8 +1,4 @@ -import { - InteractionEvent, - VerificationType, - verificationCodeIdentifierGuard, -} from '@logto/schemas'; +import { InteractionEvent, verificationCodeIdentifierGuard } from '@logto/schemas'; import type Router from 'koa-router'; import { z } from 'zod'; @@ -12,7 +8,8 @@ import koaGuard from '#src/middleware/koa-guard.js'; import type TenantContext from '#src/tenants/TenantContext.js'; import assertThat from '#src/utils/assert-that.js'; -import { CodeVerification } from '../classes/verifications/code-verification.js'; +import { codeVerificationIdentifierRecordTypeMap } from '../classes/utils.js'; +import { createNewCodeVerificationRecord } from '../classes/verifications/code-verification.js'; import { experienceRoutes } from '../const.js'; import { type WithExperienceInteractionContext } from '../middleware/koa-experience-interaction.js'; @@ -36,7 +33,7 @@ export default function verificationCodeRoutes( async (ctx, next) => { const { identifier, interactionEvent } = ctx.guard.body; - const codeVerification = CodeVerification.create( + const codeVerification = createNewCodeVerificationRecord( libraries, queries, identifier, @@ -80,7 +77,7 @@ export default function verificationCodeRoutes( assertThat( codeVerificationRecord && // Make the Verification type checker happy - codeVerificationRecord.type === VerificationType.VerificationCode, + codeVerificationRecord.type === codeVerificationIdentifierRecordTypeMap[identifier.type], new RequestError({ code: 'session.verification_session_not_found', status: 404 }) ); diff --git a/packages/schemas/src/types/interactions.ts b/packages/schemas/src/types/interactions.ts index e833e2b3f..c58ab56f6 100644 --- a/packages/schemas/src/types/interactions.ts +++ b/packages/schemas/src/types/interactions.ts @@ -41,12 +41,15 @@ export const interactionIdentifierGuard = z.object({ value: z.string(), }) satisfies ToZodObject; +export type VerificationCodeSignInIdentifier = SignInIdentifier.Email | SignInIdentifier.Phone; + /** Currently only email and phone are supported for verification code validation. */ -export type VerificationCodeIdentifier = { - type: SignInIdentifier.Email | SignInIdentifier.Phone; +export type VerificationCodeIdentifier< + T extends VerificationCodeSignInIdentifier = VerificationCodeSignInIdentifier, +> = { + type: T; value: string; }; - export const verificationCodeIdentifierGuard = z.object({ type: z.enum([SignInIdentifier.Email, SignInIdentifier.Phone]), value: z.string(), @@ -55,7 +58,8 @@ export const verificationCodeIdentifierGuard = z.object({ /** Logto supported interaction verification types. */ export enum VerificationType { Password = 'Password', - VerificationCode = 'VerificationCode', + EmailVerificationCode = 'EmailVerificationCode', + PhoneVerificationCode = 'PhoneVerificationCode', Social = 'Social', EnterpriseSso = 'EnterpriseSso', TOTP = 'Totp',