From 52c0dccbc7514cf3aabf98203218869786b14d7b Mon Sep 17 00:00:00 2001 From: simeng-li Date: Tue, 23 Jul 2024 19:43:15 +0800 Subject: [PATCH] refactor(core): implement verification records map (#6289) * refactor(core): implement verificaiton records map implement verification records map * fix(core): fix invalid verification type error fix invalid verificaiton type error * fix(core): update the verification record map update the verification record map * fix(core): update some comments update some comments * refactor(core): polish promise dependency polish promise dependency * fix(core): fix the social/sso syncing profile logic fix the social/sso syncing profile logic * refactor(core): optimize the verification records map optimize the verification records map * fix(core): fix set method of VerificationRecord map fix set method of VerificationRecord map --- .../classes/experience-interaction.ts | 32 ++++++++++++----- .../src/routes/experience/classes/helpers.ts | 16 ++++++--- .../classes/validators/provision-library.ts | 2 +- .../enterprise-sso-verification.ts | 2 +- .../experience/classes/verifications/index.ts | 35 ++++++++++++------- .../verifications/verification-records-map.ts | 34 ++++++++++++++++++ .../enterprise-sso-verification.ts | 9 ++--- .../social-verification.ts | 10 +++--- .../verification-routes/totp-verification.ts | 10 +++--- .../verification-routes/verification-code.ts | 13 ++----- 10 files changed, 111 insertions(+), 52 deletions(-) create mode 100644 packages/core/src/routes/experience/classes/verifications/verification-records-map.ts diff --git a/packages/core/src/routes/experience/classes/experience-interaction.ts b/packages/core/src/routes/experience/classes/experience-interaction.ts index 9dd36d12f..e69ad74e5 100644 --- a/packages/core/src/routes/experience/classes/experience-interaction.ts +++ b/packages/core/src/routes/experience/classes/experience-interaction.ts @@ -1,5 +1,5 @@ import { type ToZodObject } from '@logto/connector-kit'; -import { InteractionEvent, type User, type VerificationType } from '@logto/schemas'; +import { InteractionEvent, type User } from '@logto/schemas'; import { conditional } from '@silverhand/essentials'; import { z } from 'zod'; @@ -24,7 +24,9 @@ import { verificationRecordDataGuard, type VerificationRecord, type VerificationRecordData, + type VerificationRecordMap, } from './verifications/index.js'; +import { VerificationRecordsMap } from './verifications/verification-records-map.js'; type InteractionStorage = { interactionEvent?: InteractionEvent; @@ -52,7 +54,7 @@ export default class ExperienceInteraction { public readonly provisionLibrary: ProvisionLibrary; /** The user verification record list for the current interaction. */ - private readonly verificationRecords = new Map(); + private readonly verificationRecords = new VerificationRecordsMap(); /** The userId of the user for the current interaction. Only available once the user is identified. */ private userId?: string; private userCache?: User; @@ -101,7 +103,7 @@ export default class ExperienceInteraction { for (const record of verificationRecords) { const instance = buildVerificationRecord(libraries, queries, record); - this.verificationRecords.set(instance.type, instance); + this.verificationRecords.setValue(instance); } } @@ -182,13 +184,21 @@ export default class ExperienceInteraction { * If a record with the same type already exists, it will be replaced. */ public setVerificationRecord(record: VerificationRecord) { - const { type } = record; - - this.verificationRecords.set(type, record); + this.verificationRecords.setValue(record); } - public getVerificationRecordById(verificationId: string) { - return this.verificationRecordsArray.find((record) => record.id === verificationId); + public getVerificationRecordByTypeAndId( + type: K, + verificationId: string + ): VerificationRecordMap[K] { + const record = this.verificationRecords.get(type); + + assertThat( + record?.id === verificationId, + new RequestError({ code: 'session.verification_session_not_found', status: 404 }) + ); + + return record; } /** @@ -298,7 +308,7 @@ export default class ExperienceInteraction { } private get verificationRecordsArray() { - return [...this.verificationRecords.values()]; + return this.verificationRecords.array(); } /** @@ -389,4 +399,8 @@ export default class ExperienceInteraction { this.userCache = user; return this.userCache; } + + private getVerificationRecordById(verificationId: string) { + return this.verificationRecordsArray.find((record) => record.id === verificationId); + } } diff --git a/packages/core/src/routes/experience/classes/helpers.ts b/packages/core/src/routes/experience/classes/helpers.ts index c208f2be2..29fa65ad0 100644 --- a/packages/core/src/routes/experience/classes/helpers.ts +++ b/packages/core/src/routes/experience/classes/helpers.ts @@ -16,8 +16,8 @@ import type { InteractionProfile } from '../types.js'; import { type VerificationRecord } from './verifications/index.js'; /** - * @throws {RequestError} -400 if the verification record type is not supported for user creation. - * @throws {RequestError} -400 if the verification record is not verified. + * @throws {RequestError} with status 400 if the verification record type is not supported for user creation. + * @throws {RequestError} with status 400 if the verification record is not verified. */ export const getNewUserProfileFromVerificationRecord = async ( verificationRecord: VerificationRecord @@ -30,8 +30,13 @@ export const getNewUserProfileFromVerificationRecord = async ( } case VerificationType.EnterpriseSso: case VerificationType.Social: { - const identityProfile = await verificationRecord.toUserProfile(); - const syncedProfile = await verificationRecord.toSyncedProfile(true); + const [identityProfile, syncedProfile] = await Promise.all([ + verificationRecord.toUserProfile(), + // Set `isNewUser` to true to specify syncing profile from the + // social/enterprise SSO identity for a new user. + verificationRecord.toSyncedProfile(true), + ]); + return { ...identityProfile, ...syncedProfile }; } default: { @@ -95,8 +100,9 @@ export const identifyUserByVerificationRecord = async ( // Auto fallback to identify the related user if the user does not exist for enterprise SSO. if (error instanceof RequestError && error.code === 'user.identity_not_exist') { const user = await verificationRecord.identifyRelatedUser(); + const syncedProfile = { - ...(await verificationRecord.toUserProfile()), + ...verificationRecord.toUserProfile(), ...(await verificationRecord.toSyncedProfile()), }; return { user, syncedProfile }; diff --git a/packages/core/src/routes/experience/classes/validators/provision-library.ts b/packages/core/src/routes/experience/classes/validators/provision-library.ts index 8d9e502ac..a8610b7b6 100644 --- a/packages/core/src/routes/experience/classes/validators/provision-library.ts +++ b/packages/core/src/routes/experience/classes/validators/provision-library.ts @@ -43,7 +43,7 @@ export class ProvisionLibrary { /** * Insert a new user into the Logto database using the provided profile. * - * - provision the organization for the new user based on the profile + * - Provision the organization for the new user based on the profile * - OSS only, new user provisioning */ async provisionNewUser(profile: InteractionProfile) { diff --git a/packages/core/src/routes/experience/classes/verifications/enterprise-sso-verification.ts b/packages/core/src/routes/experience/classes/verifications/enterprise-sso-verification.ts index db710ca69..5bf28a730 100644 --- a/packages/core/src/routes/experience/classes/verifications/enterprise-sso-verification.ts +++ b/packages/core/src/routes/experience/classes/verifications/enterprise-sso-verification.ts @@ -173,7 +173,7 @@ export class EnterpriseSsoVerification /** * Returns the use SSO identity as a new user profile. */ - async toUserProfile(): Promise>> { + toUserProfile(): Required> { assertThat( this.enterpriseSsoUserInfo && this.issuer, new RequestError({ code: 'session.verification_failed', status: 400 }) diff --git a/packages/core/src/routes/experience/classes/verifications/index.ts b/packages/core/src/routes/experience/classes/verifications/index.ts index bc1ac79bd..db9f601d5 100644 --- a/packages/core/src/routes/experience/classes/verifications/index.ts +++ b/packages/core/src/routes/experience/classes/verifications/index.ts @@ -41,6 +41,7 @@ import { totpVerificationRecordDataGuard, type TotpVerificationRecordData, } from './totp-verification.js'; +import { type VerificationRecord as GenericVerificationRecord } from './verification-record.js'; export type VerificationRecordData = | PasswordVerificationRecordData @@ -52,23 +53,33 @@ export type VerificationRecordData = | BackupCodeVerificationRecordData | NewPasswordIdentityVerificationRecordData; +// This is to ensure the keys of the map are the same as the type of the verification record +type VerificationRecordInterfaceMap = { + [K in VerificationType]?: GenericVerificationRecord; +}; +type AssertVerificationMap = T; + +export type VerificationRecordMap = AssertVerificationMap<{ + [VerificationType.Password]: PasswordVerification; + [VerificationType.EmailVerificationCode]: EmailCodeVerification; + [VerificationType.PhoneVerificationCode]: PhoneCodeVerification; + [VerificationType.Social]: SocialVerification; + [VerificationType.EnterpriseSso]: EnterpriseSsoVerification; + [VerificationType.TOTP]: TotpVerification; + [VerificationType.BackupCode]: BackupCodeVerification; + [VerificationType.NewPasswordIdentity]: NewPasswordIdentityVerification; +}>; + +type ValueOf = T[keyof T]; /** * Union type for all verification record types * - * @remark This is a discriminated union type. - * The VerificationRecord generic class can not narrow down the type of a verification record instance by its type property. + * @remarks This is a discriminated union type. + * The `VerificationRecord` generic class can not narrow down the type of a verification record instance by its type property. * This union type is used to narrow down the type of the verification record. - * Used in the ExperienceInteraction class to store and manage all the verification records. With a given verification type, we can narrow down the type of the verification record. + * Used in the `ExperienceInteraction` class to store and manage all the verification records. With a given verification type, we can narrow down the type of the verification record. */ -export type VerificationRecord = - | PasswordVerification - | EmailCodeVerification - | PhoneCodeVerification - | SocialVerification - | EnterpriseSsoVerification - | TotpVerification - | BackupCodeVerification - | NewPasswordIdentityVerification; +export type VerificationRecord = ValueOf; export const verificationRecordDataGuard = z.discriminatedUnion('type', [ passwordVerificationRecordDataGuard, diff --git a/packages/core/src/routes/experience/classes/verifications/verification-records-map.ts b/packages/core/src/routes/experience/classes/verifications/verification-records-map.ts new file mode 100644 index 000000000..dc60760f3 --- /dev/null +++ b/packages/core/src/routes/experience/classes/verifications/verification-records-map.ts @@ -0,0 +1,34 @@ +/** + * @fileoverview + * + * Since `Map` in TS does not support key value type mapping, + * we have to manually define a `setValue` method to ensure correct key will be set + * This class is used to store and manage all the verification records. + * + * Extends the Map class and adds a `setValue` method to ensure the key value type mapping. + */ + +import { type VerificationType } from '@logto/schemas'; + +import { type VerificationRecord, type VerificationRecordMap } from './index.js'; + +export class VerificationRecordsMap extends Map { + setValue(value: VerificationRecord) { + return super.set(value.type, value); + } + + override get( + key: K + ): VerificationRecordMap[K] | undefined { + // eslint-disable-next-line no-restricted-syntax + return super.get(key) as VerificationRecordMap[K] | undefined; + } + + override set(): never { + throw new Error('Use `setValue` method to set the value'); + } + + array(): VerificationRecord[] { + return [...this.values()]; + } +} diff --git a/packages/core/src/routes/experience/verification-routes/enterprise-sso-verification.ts b/packages/core/src/routes/experience/verification-routes/enterprise-sso-verification.ts index bdb1d86e7..ce65b992f 100644 --- a/packages/core/src/routes/experience/verification-routes/enterprise-sso-verification.ts +++ b/packages/core/src/routes/experience/verification-routes/enterprise-sso-verification.ts @@ -80,12 +80,13 @@ export default function enterpriseSsoVerificationRoutes( const { connectorId } = ctx.params; const { connectorData, verificationId } = ctx.guard.body; - const socialVerificationRecord = - ctx.experienceInteraction.getVerificationRecordById(verificationId); + const socialVerificationRecord = ctx.experienceInteraction.getVerificationRecordByTypeAndId( + VerificationType.Social, + verificationId + ); assertThat( - socialVerificationRecord && - socialVerificationRecord.type === VerificationType.Social && - socialVerificationRecord.connectorId === connectorId, + socialVerificationRecord.connectorId === connectorId, new RequestError({ code: 'session.verification_session_not_found', status: 404 }) ); diff --git a/packages/core/src/routes/experience/verification-routes/totp-verification.ts b/packages/core/src/routes/experience/verification-routes/totp-verification.ts index ef6a4925b..ba875a88e 100644 --- a/packages/core/src/routes/experience/verification-routes/totp-verification.ts +++ b/packages/core/src/routes/experience/verification-routes/totp-verification.ts @@ -75,13 +75,13 @@ export default function totpVerificationRoutes( // Verify new generated secret if (verificationId) { - const totpVerificationRecord = - experienceInteraction.getVerificationRecordById(verificationId); + const totpVerificationRecord = experienceInteraction.getVerificationRecordByTypeAndId( + VerificationType.TOTP, + verificationId + ); assertThat( - totpVerificationRecord && - totpVerificationRecord.type === VerificationType.TOTP && - totpVerificationRecord.userId === experienceInteraction.identifiedUserId, + totpVerificationRecord.userId === experienceInteraction.identifiedUserId, new RequestError({ code: 'session.verification_session_not_found', status: 404, 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 6c9654ba4..711546d38 100644 --- a/packages/core/src/routes/experience/verification-routes/verification-code.ts +++ b/packages/core/src/routes/experience/verification-routes/verification-code.ts @@ -2,11 +2,9 @@ import { InteractionEvent, verificationCodeIdentifierGuard } from '@logto/schema import type Router from 'koa-router'; import { z } from 'zod'; -import RequestError from '#src/errors/RequestError/index.js'; import { type WithLogContext } from '#src/middleware/koa-audit-log.js'; 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'; @@ -71,14 +69,9 @@ export default function verificationCodeRoutes( async (ctx, next) => { const { verificationId, code, identifier } = ctx.guard.body; - const codeVerificationRecord = - ctx.experienceInteraction.getVerificationRecordById(verificationId); - - assertThat( - codeVerificationRecord && - // Make the Verification type checker happy - codeVerificationRecord.type === codeVerificationIdentifierRecordTypeMap[identifier.type], - new RequestError({ code: 'session.verification_session_not_found', status: 404 }) + const codeVerificationRecord = ctx.experienceInteraction.getVerificationRecordByTypeAndId( + codeVerificationIdentifierRecordTypeMap[identifier.type], + verificationId ); await codeVerificationRecord.verify(identifier, code);