From 72891eca9f55949bc38be87fdba209a808e5a1b8 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Fri, 19 Jul 2024 11:36:41 +0800 Subject: [PATCH 1/3] refactor(core,schemas): refactor the CodeVerification class split the CodeVerification class into EmailCodeVerification and PhoneCodeVerification --- .../classes/experience-interaction.test.ts | 4 +- .../sign-in-experience-validator.test.ts | 6 +- .../verifications/code-verification.ts | 149 +++++++++++++----- .../experience/classes/verifications/index.ts | 19 ++- .../verification-routes/verification-code.ts | 7 +- packages/schemas/src/types/interactions.ts | 9 +- 6 files changed, 138 insertions(+), 56 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 561df47d981..866a6a779a3 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,7 +75,7 @@ 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, identifier: { 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 83adb699842..44b3839bc6f 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/verifications/code-verification.ts b/packages/core/src/routes/experience/classes/verifications/code-verification.ts index e9879c9726c..06effc2023a 100644 --- a/packages/core/src/routes/experience/classes/verifications/code-verification.ts +++ b/packages/core/src/routes/experience/classes/verifications/code-verification.ts @@ -1,10 +1,12 @@ import { TemplateType } from '@logto/connector-kit'; import { InteractionEvent, + SignInIdentifier, VerificationType, verificationCodeIdentifierGuard, type User, type VerificationCodeIdentifier, + type VerificationCodeSignInIdentifier, } from '@logto/schemas'; import { type ToZodObject } from '@logto/schemas/lib/utils/zod.js'; import { generateStandardId } from '@logto/shared'; @@ -35,15 +37,22 @@ const eventToTemplateTypeMap: Record = { const getTemplateTypeByEvent = (event: InteractionEvent): TemplateType => eventToTemplateTypeMap[event]; +/** 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 }; + /** The JSON data type for the CodeVerification record */ -export type CodeVerificationRecordData = { +export type CodeVerificationRecordData< + T extends VerificationCodeSignInIdentifier = VerificationCodeSignInIdentifier, +> = { id: string; type: VerificationType.VerificationCode; - identifier: VerificationCodeIdentifier; + identifier: VerificationCodeIdentifier; interactionEvent: InteractionEvent; verified: boolean; }; - export const codeVerificationRecordDataGuard = z.object({ id: z.string(), type: z.literal(VerificationType.VerificationCode), @@ -52,47 +61,17 @@ export const codeVerificationRecordDataGuard = z.object({ 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 }; - /** - * 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`. + * CodeVerification is a verification type that verifies a given identifier by sending a verification code. + * This is the parent class for `EmailCodeVerification` and `PhoneCodeVerification`. Not publicly exposed. */ -export class CodeVerification - implements IdentifierVerificationRecord +class CodeVerification< + T extends VerificationCodeSignInIdentifier = VerificationCodeSignInIdentifier, +> 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: VerificationCodeIdentifier; /** * The interaction event that triggered the verification. @@ -102,12 +81,12 @@ 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; + protected verified: boolean; constructor( private readonly libraries: Libraries, private readonly queries: Queries, - data: CodeVerificationRecordData + data: CodeVerificationRecordData ) { const { id, identifier, verified, interactionEvent } = data; @@ -198,7 +177,7 @@ export class CodeVerification return type === 'email' ? { primaryEmail: value } : { primaryPhone: value }; } - toJson(): CodeVerificationRecordData { + toJson(): CodeVerificationRecordData { const { id, type, identifier, interactionEvent, verified } = this; return { @@ -210,3 +189,89 @@ export class CodeVerification }; } } + +/** + * CodeVerification is a verification type that verifies a given identifier by sending a verification code. + * + * @remark The verification code is sent to the user's email 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. + */ +export class EmailCodeVerification extends CodeVerification { + override toUserProfile(): { primaryEmail: string } { + assertThat( + this.verified, + new RequestError({ + code: 'session.verification_failed', + state: 400, + }) + ); + + const { value } = this.identifier; + + return { primaryEmail: value }; + } +} + +/** + * CodeVerification is a verification type that verifies a given identifier by sending a verification code. + * + * @remark The verification code is sent to the user's phone 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. + */ +export class PhoneCodeVerification extends CodeVerification { + override toUserProfile(): { primaryPhone: string } { + assertThat( + this.verified, + new RequestError({ + code: 'session.verification_failed', + state: 400, + }) + ); + + const { value } = this.identifier; + + return { primaryPhone: value }; + } +} + +export const assertEmailCodeVerificationData = ( + data: CodeVerificationRecordData +): data is CodeVerificationRecordData => + data.identifier.type === SignInIdentifier.Email; + +export const assertPhoneCodeVerificationData = ( + data: CodeVerificationRecordData +): data is CodeVerificationRecordData => + data.identifier.type === SignInIdentifier.Phone; + +/** + * 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; + + if (type === SignInIdentifier.Email) { + return new EmailCodeVerification(libraries, queries, { + id: generateStandardId(), + type: VerificationType.VerificationCode, + identifier, + interactionEvent, + verified: false, + }); + } + + return new PhoneCodeVerification(libraries, queries, { + id: generateStandardId(), + type: VerificationType.VerificationCode, + 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 8522de9a9cd..c8876780e05 100644 --- a/packages/core/src/routes/experience/classes/verifications/index.ts +++ b/packages/core/src/routes/experience/classes/verifications/index.ts @@ -10,8 +10,11 @@ import { type BackupCodeVerificationRecordData, } from './backup-code-verification.js'; import { - CodeVerification, + assertEmailCodeVerificationData, + assertPhoneCodeVerificationData, codeVerificationRecordDataGuard, + EmailCodeVerification, + PhoneCodeVerification, type CodeVerificationRecordData, } from './code-verification.js'; import { @@ -59,7 +62,8 @@ export type VerificationRecordData = */ export type VerificationRecord = | PasswordVerification - | CodeVerification + | EmailCodeVerification + | PhoneCodeVerification | SocialVerification | EnterpriseSsoVerification | TotpVerification @@ -89,7 +93,16 @@ export const buildVerificationRecord = ( return new PasswordVerification(libraries, queries, data); } case VerificationType.VerificationCode: { - return new CodeVerification(libraries, queries, data); + // TS can't distribute the CodeVerificationRecordData type directly + // so we need to assert the data type here + if (assertEmailCodeVerificationData(data)) { + return new EmailCodeVerification(libraries, queries, data); + } + if (assertPhoneCodeVerificationData(data)) { + return new PhoneCodeVerification(libraries, queries, data); + } + // Make the type checker happy + throw new Error('Invalid code verification data'); } case VerificationType.Social: { return new SocialVerification(libraries, queries, data); 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 1e99894b51c..73df5c48185 100644 --- a/packages/core/src/routes/experience/verification-routes/verification-code.ts +++ b/packages/core/src/routes/experience/verification-routes/verification-code.ts @@ -12,7 +12,7 @@ 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 { createNewCodeVerificationRecord } from '../classes/verifications/code-verification.js'; import { experienceRoutes } from '../const.js'; import { type WithExperienceInteractionContext } from '../middleware/koa-experience-interaction.js'; @@ -36,7 +36,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 +80,8 @@ export default function verificationCodeRoutes( assertThat( codeVerificationRecord && // Make the Verification type checker happy - codeVerificationRecord.type === VerificationType.VerificationCode, + codeVerificationRecord.type === VerificationType.VerificationCode && + codeVerificationRecord.identifier.type === 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 e833e2b3fe7..86d2d3fff2e 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(), From 9d4e069621ed2ed6da3407794efeeb4ce89dd358 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Fri, 19 Jul 2024 14:01:45 +0800 Subject: [PATCH 2/3] refactor(core,schemas): split CodeVerification type split CodeVerification type --- .../classes/experience-interaction.test.ts | 2 +- .../src/routes/experience/classes/utils.ts | 12 +- .../sign-in-experience-validator.ts | 15 ++- .../verifications/code-verification.ts | 108 ++++++++++-------- .../experience/classes/verifications/index.ts | 27 ++--- .../verifications/verification-record.ts | 3 +- .../verification-routes/verification-code.ts | 10 +- packages/schemas/src/types/interactions.ts | 3 +- 8 files changed, 97 insertions(+), 83 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 866a6a779a3..43c2ce6073b 100644 --- a/packages/core/src/routes/experience/classes/experience-interaction.test.ts +++ b/packages/core/src/routes/experience/classes/experience-interaction.test.ts @@ -77,7 +77,7 @@ describe('ExperienceInteraction class', () => { 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 9aebaf7b767..fe0011e52f4 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.ts b/packages/core/src/routes/experience/classes/validators/sign-in-experience-validator.ts index 9a22b63c21f..08b912ae294 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 06effc2023a..73a70f1baed 100644 --- a/packages/core/src/routes/experience/classes/verifications/code-verification.ts +++ b/packages/core/src/routes/experience/classes/verifications/code-verification.ts @@ -1,14 +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, - type VerificationCodeSignInIdentifier, } from '@logto/schemas'; -import { type ToZodObject } from '@logto/schemas/lib/utils/zod.js'; import { generateStandardId } from '@logto/shared'; import { z } from 'zod'; @@ -43,35 +40,36 @@ const getPasscodeIdentifierPayload = ( ): Parameters['createPasscode']>[2] => identifier.type === 'email' ? { email: identifier.value } : { phone: identifier.value }; +type CodeVerificationType = + | VerificationType.EmailVerificationCode + | VerificationType.PhoneVerificationCode; + +type SinInIdentifierTypeOf = + T extends VerificationType.EmailVerificationCode + ? SignInIdentifier.Email + : SignInIdentifier.Phone; + +type VerificationCodeIdentifierOf = VerificationCodeIdentifier< + SinInIdentifierTypeOf +>; + /** The JSON data type for the CodeVerification record */ -export type CodeVerificationRecordData< - T extends VerificationCodeSignInIdentifier = VerificationCodeSignInIdentifier, -> = { +export type CodeVerificationRecordData = { id: string; - type: VerificationType.VerificationCode; - identifier: VerificationCodeIdentifier; + type: T; + identifier: VerificationCodeIdentifierOf; 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; /** - * CodeVerification is a verification type that verifies a given identifier by sending a verification code. * This is the parent class for `EmailCodeVerification` and `PhoneCodeVerification`. Not publicly exposed. */ -class CodeVerification< - T extends VerificationCodeSignInIdentifier = VerificationCodeSignInIdentifier, -> implements IdentifierVerificationRecord +abstract class CodeVerification + implements IdentifierVerificationRecord { public readonly id: string; - public readonly type = VerificationType.VerificationCode; - public readonly identifier: VerificationCodeIdentifier; + public readonly identifier: VerificationCodeIdentifierOf; /** * The interaction event that triggered the verification. @@ -81,6 +79,7 @@ class CodeVerification< * `InteractionEvent.ForgotPassword` triggered verification results can not used as a verification record for other events. */ public readonly interactionEvent: InteractionEvent; + public abstract readonly type: T; protected verified: boolean; constructor( @@ -166,17 +165,6 @@ 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 { const { id, type, identifier, interactionEvent, verified } = this; @@ -188,16 +176,28 @@ class CodeVerification< verified, }; } + + abstract toUserProfile(): T extends VerificationType.EmailVerificationCode + ? { primaryEmail: string } + : { primaryPhone: string }; } +const basicCodeVerificationRecordDataGuard = z.object({ + id: z.string(), + interactionEvent: z.nativeEnum(InteractionEvent), + verified: z.boolean(), +}); + /** - * CodeVerification is a verification type that verifies a given identifier by sending a verification code. + * EmailCodeVerification is a verification type that verifies a given email identifier by sending a verification code. * * @remark The verification code is sent to the user's email 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. */ -export class EmailCodeVerification extends CodeVerification { - override toUserProfile(): { primaryEmail: string } { +export class EmailCodeVerification extends CodeVerification { + public readonly type = VerificationType.EmailVerificationCode; + + toUserProfile(): { primaryEmail: string } { assertThat( this.verified, new RequestError({ @@ -212,14 +212,24 @@ export class EmailCodeVerification extends CodeVerification>; + /** - * CodeVerification is a verification type that verifies a given identifier by sending a verification code. + * PhoneCodeVerification is a verification type that verifies a given phone identifier by sending a verification code. * * @remark The verification code is sent to the user's phone 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. */ -export class PhoneCodeVerification extends CodeVerification { - override toUserProfile(): { primaryPhone: string } { +export class PhoneCodeVerification extends CodeVerification { + public readonly type = VerificationType.PhoneVerificationCode; + + toUserProfile(): { primaryPhone: string } { assertThat( this.verified, new RequestError({ @@ -234,15 +244,13 @@ export class PhoneCodeVerification extends CodeVerification => - data.identifier.type === SignInIdentifier.Email; - -export const assertPhoneCodeVerificationData = ( - data: CodeVerificationRecordData -): data is CodeVerificationRecordData => - data.identifier.type === SignInIdentifier.Phone; +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. @@ -260,7 +268,7 @@ export const createNewCodeVerificationRecord = ( if (type === SignInIdentifier.Email) { return new EmailCodeVerification(libraries, queries, { id: generateStandardId(), - type: VerificationType.VerificationCode, + type: VerificationType.EmailVerificationCode, identifier, interactionEvent, verified: false, @@ -269,7 +277,7 @@ export const createNewCodeVerificationRecord = ( return new PhoneCodeVerification(libraries, queries, { id: generateStandardId(), - type: VerificationType.VerificationCode, + 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 c8876780e05..bc1ac79bdb5 100644 --- a/packages/core/src/routes/experience/classes/verifications/index.ts +++ b/packages/core/src/routes/experience/classes/verifications/index.ts @@ -10,11 +10,10 @@ import { type BackupCodeVerificationRecordData, } from './backup-code-verification.js'; import { - assertEmailCodeVerificationData, - assertPhoneCodeVerificationData, - codeVerificationRecordDataGuard, EmailCodeVerification, + emailCodeVerificationRecordDataGuard, PhoneCodeVerification, + phoneCodeVerificationRecordDataGuard, type CodeVerificationRecordData, } from './code-verification.js'; import { @@ -45,7 +44,8 @@ import { export type VerificationRecordData = | PasswordVerificationRecordData - | CodeVerificationRecordData + | CodeVerificationRecordData + | CodeVerificationRecordData | SocialVerificationRecordData | EnterpriseSsoVerificationRecordData | TotpVerificationRecordData @@ -72,7 +72,8 @@ export type VerificationRecord = export const verificationRecordDataGuard = z.discriminatedUnion('type', [ passwordVerificationRecordDataGuard, - codeVerificationRecordDataGuard, + emailCodeVerificationRecordDataGuard, + phoneCodeVerificationRecordDataGuard, socialVerificationRecordDataGuard, enterPriseSsoVerificationRecordDataGuard, totpVerificationRecordDataGuard, @@ -92,17 +93,11 @@ export const buildVerificationRecord = ( case VerificationType.Password: { return new PasswordVerification(libraries, queries, data); } - case VerificationType.VerificationCode: { - // TS can't distribute the CodeVerificationRecordData type directly - // so we need to assert the data type here - if (assertEmailCodeVerificationData(data)) { - return new EmailCodeVerification(libraries, queries, data); - } - if (assertPhoneCodeVerificationData(data)) { - return new PhoneCodeVerification(libraries, queries, data); - } - // Make the type checker happy - throw new Error('Invalid code verification 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 6eff8327515..a9ff16fb854 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 73df5c48185..6c9654ba407 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,6 +8,7 @@ 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 { 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'; @@ -80,8 +77,7 @@ export default function verificationCodeRoutes( assertThat( codeVerificationRecord && // Make the Verification type checker happy - codeVerificationRecord.type === VerificationType.VerificationCode && - codeVerificationRecord.identifier.type === identifier.type, + 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 86d2d3fff2e..c58ab56f638 100644 --- a/packages/schemas/src/types/interactions.ts +++ b/packages/schemas/src/types/interactions.ts @@ -58,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', From f3b8cadbc79168b3cbf999842f1cb1a2ec3cd0dd Mon Sep 17 00:00:00 2001 From: simeng-li Date: Mon, 22 Jul 2024 10:19:36 +0800 Subject: [PATCH 3/3] fix(core): code review updates code review updates --- .../verifications/code-verification.ts | 75 +++++++++++-------- 1 file changed, 42 insertions(+), 33 deletions(-) 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 73a70f1baed..0ccbc2f8971 100644 --- a/packages/core/src/routes/experience/classes/verifications/code-verification.ts +++ b/packages/core/src/routes/experience/classes/verifications/code-verification.ts @@ -44,16 +44,21 @@ type CodeVerificationType = | VerificationType.EmailVerificationCode | VerificationType.PhoneVerificationCode; -type SinInIdentifierTypeOf = - T extends VerificationType.EmailVerificationCode - ? SignInIdentifier.Email - : SignInIdentifier.Phone; +type SinInIdentifierTypeOf = { + [VerificationType.EmailVerificationCode]: SignInIdentifier.Email; + [VerificationType.PhoneVerificationCode]: SignInIdentifier.Phone; +}; type VerificationCodeIdentifierOf = VerificationCodeIdentifier< - SinInIdentifierTypeOf + SinInIdentifierTypeOf[T] >; -/** The JSON data type for the CodeVerification record */ +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; @@ -122,7 +127,8 @@ abstract 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. @@ -177,9 +183,7 @@ abstract class CodeVerification }; } - abstract toUserProfile(): T extends VerificationType.EmailVerificationCode - ? { primaryEmail: string } - : { primaryPhone: string }; + abstract toUserProfile(): CodeVerificationIdentifierMap[T]; } const basicCodeVerificationRecordDataGuard = z.object({ @@ -189,10 +193,11 @@ const basicCodeVerificationRecordDataGuard = z.object({ }); /** - * EmailCodeVerification is a verification type that verifies a given email identifier by sending a verification code. + * A verification code class that verifies a given email identifier. * - * @remark The verification code is sent to the user's email 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. + * @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; @@ -221,10 +226,11 @@ export const emailCodeVerificationRecordDataGuard = basicCodeVerificationRecordD }) satisfies ToZodObject>; /** - * PhoneCodeVerification is a verification type that verifies a given phone identifier by sending a verification code. + * A verification code class that verifies a given phone identifier. * - * @remark The verification code is sent to the user's phone 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. + * @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; @@ -253,7 +259,7 @@ export const phoneCodeVerificationRecordDataGuard = basicCodeVerificationRecordD }) satisfies ToZodObject>; /** - * Factory method to create a new EmailCodeVerification/PhoneCodeVerification record using the given identifier. + * Factory method to create a new `EmailCodeVerification` / `PhoneCodeVerification` record using the given identifier. */ export const createNewCodeVerificationRecord = ( libraries: Libraries, @@ -265,21 +271,24 @@ export const createNewCodeVerificationRecord = ( ) => { const { type } = identifier; - if (type === SignInIdentifier.Email) { - return new EmailCodeVerification(libraries, queries, { - id: generateStandardId(), - type: VerificationType.EmailVerificationCode, - identifier, - interactionEvent, - verified: false, - }); + 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, + }); + } } - - return new PhoneCodeVerification(libraries, queries, { - id: generateStandardId(), - type: VerificationType.PhoneVerificationCode, - identifier, - interactionEvent, - verified: false, - }); };