From 698610a12fba4a1f7fcc952c70b90de2ce338124 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Fri, 9 Dec 2022 12:12:32 +0800 Subject: [PATCH] refactor(core): refactor the user account verification (#2607) --- packages/core/src/routes/admin-user.test.ts | 4 +- packages/core/src/routes/admin-user.ts | 9 +- packages/core/src/routes/init.ts | 15 +- .../interaction/actions/submit-interaction.ts | 2 + .../core/src/routes/interaction/index.test.ts | 168 +++++++++++++++++- packages/core/src/routes/interaction/index.ts | 10 +- .../src/routes/interaction/utils/index.ts | 24 +-- .../interaction/utils/interaction.test.ts | 61 +++---- .../routes/interaction/utils/interaction.ts | 71 ++++++-- .../identifier-payload-verification.ts | 10 +- .../verifications/profile-verification.ts | 13 +- .../user-identity-verification.test.ts | 2 +- .../user-identity-verification.ts | 42 ++--- 13 files changed, 322 insertions(+), 109 deletions(-) diff --git a/packages/core/src/routes/admin-user.test.ts b/packages/core/src/routes/admin-user.test.ts index 601ecaef8..b0711b92e 100644 --- a/packages/core/src/routes/admin-user.test.ts +++ b/packages/core/src/routes/admin-user.test.ts @@ -129,11 +129,11 @@ describe('adminUserRoutes', () => { const username = 'MJAtLogto'; const password = 'PASSWORD'; const name = 'Michael'; - const primaryEmail = 'foo@logto.io'; + const { primaryEmail, primaryPhone } = mockUser; const response = await userRequest .post('/users') - .send({ primaryEmail, username, password, name }); + .send({ primaryEmail, primaryPhone, username, password, name }); expect(response.status).toEqual(200); expect(response.body).toEqual({ ...mockUserResponse, diff --git a/packages/core/src/routes/admin-user.ts b/packages/core/src/routes/admin-user.ts index 660cabcc3..3d967d66b 100644 --- a/packages/core/src/routes/admin-user.ts +++ b/packages/core/src/routes/admin-user.ts @@ -25,6 +25,7 @@ import { hasUser, updateUserById, hasUserWithEmail, + hasUserWithPhone, } from '#src/queries/user.js'; import assertThat from '#src/utils/assert-that.js'; @@ -127,6 +128,7 @@ export default function adminUserRoutes(router: T) { '/users', koaGuard({ body: object({ + primaryPhone: string().regex(phoneRegEx).optional(), primaryEmail: string().regex(emailRegEx).optional(), username: string().regex(usernameRegEx).optional(), password: string().regex(passwordRegEx), @@ -134,7 +136,7 @@ export default function adminUserRoutes(router: T) { }), }), async (ctx, next) => { - const { primaryEmail, username, password, name } = ctx.guard.body; + const { primaryEmail, primaryPhone, username, password, name } = ctx.guard.body; assertThat( !username || !(await hasUser(username)), @@ -150,6 +152,10 @@ export default function adminUserRoutes(router: T) { status: 422, }) ); + assertThat( + !primaryPhone || !(await hasUserWithPhone(primaryPhone)), + new RequestError({ code: 'user.phone_already_in_use' }) + ); const id = await generateUserId(); @@ -158,6 +164,7 @@ export default function adminUserRoutes(router: T) { const user = await insertUser({ id, primaryEmail, + primaryPhone, username, passwordEncrypted, passwordEncryptionMethod, diff --git a/packages/core/src/routes/init.ts b/packages/core/src/routes/init.ts index b0d093845..474a586a8 100644 --- a/packages/core/src/routes/init.ts +++ b/packages/core/src/routes/init.ts @@ -12,6 +12,7 @@ import authnRoutes from './authn.js'; import connectorRoutes from './connector.js'; import customPhraseRoutes from './custom-phrase.js'; import dashboardRoutes from './dashboard.js'; +import interactionRoutes from './interaction/index.js'; import logRoutes from './log.js'; import phraseRoutes from './phrase.js'; import profileRoutes from './profile.js'; @@ -30,6 +31,10 @@ const createRouters = (provider: Provider) => { sessionRouter.use(koaLogSession(provider)); sessionRoutes(sessionRouter, provider); + const interactionRouter: AnonymousRouter = new Router(); + interactionRouter.use(koaLogSession(provider)); + interactionRoutes(interactionRouter, provider); + const managementRouter: AuthedRouter = new Router(); managementRouter.use(koaAuth(UserRole.Admin)); applicationRoutes(managementRouter); @@ -52,9 +57,15 @@ const createRouters = (provider: Provider) => { statusRoutes(anonymousRouter); authnRoutes(anonymousRouter); // The swagger.json should contain all API routers. - swaggerRoutes(anonymousRouter, [sessionRouter, profileRouter, managementRouter, anonymousRouter]); + swaggerRoutes(anonymousRouter, [ + sessionRouter, + interactionRouter, + profileRouter, + managementRouter, + anonymousRouter, + ]); - return [sessionRouter, profileRouter, managementRouter, anonymousRouter]; + return [sessionRouter, interactionRouter, profileRouter, managementRouter, anonymousRouter]; }; export default function initRouter(app: Koa, provider: Provider) { diff --git a/packages/core/src/routes/interaction/actions/submit-interaction.ts b/packages/core/src/routes/interaction/actions/submit-interaction.ts index 41b75e480..47ae8fdb6 100644 --- a/packages/core/src/routes/interaction/actions/submit-interaction.ts +++ b/packages/core/src/routes/interaction/actions/submit-interaction.ts @@ -16,6 +16,7 @@ import type { VerifiedSignInInteractionResult, VerifiedRegisterInteractionResult, } from '../types/index.js'; +import { clearInteractionStorage } from '../utils/interaction.js'; const getSocialUpdateProfile = async ({ user, @@ -124,5 +125,6 @@ export default async function submitInteraction( profile.password ); await updateUserById(accountId, { passwordEncrypted, passwordEncryptionMethod }); + await clearInteractionStorage(ctx, provider); ctx.status = 204; } diff --git a/packages/core/src/routes/interaction/index.test.ts b/packages/core/src/routes/interaction/index.test.ts index 4b7a57f4e..af08a4710 100644 --- a/packages/core/src/routes/interaction/index.test.ts +++ b/packages/core/src/routes/interaction/index.test.ts @@ -2,17 +2,23 @@ import { ConnectorType } from '@logto/connector-kit'; import { Event } from '@logto/schemas'; import { Provider } from 'oidc-provider'; +import { mockSignInExperience } from '#src/__mocks__/sign-in-experience.js'; import RequestError from '#src/errors/RequestError/index.js'; import { createRequester } from '#src/utils/test-utils.js'; -import interactionRoutes, { verificationPrefix } from './index.js'; +import submitInteraction from './actions/submit-interaction.js'; +import interactionRoutes, { verificationPrefix, interactionPrefix } from './index.js'; +import type { InteractionContext } from './types/index.js'; +import { getInteractionStorage } from './utils/interaction.js'; import { sendPasscodeToIdentifier } from './utils/passcode-validation.js'; +import { + verifyIdentifier, + verifyProfile, + validateMandatoryUserProfile, +} from './verifications/index.js'; // FIXME @Darcy: no more `enabled` for `connectors` table const getLogtoConnectorByIdHelper = jest.fn(async (connectorId: string) => { - const database = { - enabled: connectorId === 'social_enabled', - }; const metadata = { id: connectorId === 'social_enabled' @@ -23,7 +29,7 @@ const getLogtoConnectorByIdHelper = jest.fn(async (connectorId: string) => { }; return { - dbEntry: database, + dbEntry: {}, metadata, type: connectorId.startsWith('social') ? ConnectorType.Social : ConnectorType.Sms, getAuthorizationUri: jest.fn(async () => ''), @@ -53,12 +59,45 @@ jest.mock('oidc-provider', () => ({ Provider: jest.fn(() => ({ interactionDetails: jest.fn().mockResolvedValue({ jti: 'jti', + result: {}, + params: { + client_id: 'demo_app', + }, }), })), })); +jest.mock('#src/lib/sign-in-experience/index.js', () => ({ + getSignInExperienceForApplication: jest.fn().mockResolvedValue(mockSignInExperience), +})); + +jest.mock('./verifications/index.js', () => ({ + verifyIdentifier: jest.fn(), + verifyProfile: jest.fn(), + validateMandatoryUserProfile: jest.fn(), +})); + +jest.mock('./actions/submit-interaction.js', () => + jest.fn((_interaction, ctx: InteractionContext) => { + ctx.body = { redirectUri: 'logto.io' }; + }) +); + +jest.mock('./utils/interaction.js', () => ({ + getInteractionStorage: jest.fn(), +})); + const log = jest.fn(); +const koaInteractionBodyGuardSpy = jest.spyOn( + jest.requireActual('./middleware/koa-interaction-body-guard.js'), + 'default' +); +const koaSessionSignInExperienceGuardSpy = jest.spyOn( + jest.requireActual('./middleware/koa-session-sign-in-experience-guard.js'), + 'default' +); + describe('session -> interactionRoutes', () => { const sessionRequest = createRequester({ anonymousRoutes: interactionRoutes, @@ -73,6 +112,125 @@ describe('session -> interactionRoutes', () => { ], }); + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('PUT /interaction', () => { + const path = interactionPrefix; + + it('sign-in event should call methods properly', async () => { + const body = { + event: Event.SignIn, + identifier: { + username: 'username', + password: 'password', + }, + }; + const response = await sessionRequest.put(path).send(body); + expect(koaInteractionBodyGuardSpy).toBeCalled(); + expect(koaSessionSignInExperienceGuardSpy).toBeCalled(); + expect(verifyIdentifier).toBeCalled(); + expect(verifyProfile).toBeCalled(); + expect(validateMandatoryUserProfile).toBeCalled(); + expect(submitInteraction).toBeCalled(); + expect(response.status).toEqual(200); + expect(response.body).toEqual({ redirectUri: 'logto.io' }); + }); + + it('forgot password event should not call UserProfile validation', async () => { + const body = { + event: Event.ForgotPassword, + identifier: { + email: 'email@logto.io', + passcode: 'passcode', + }, + profile: { + password: 'password', + }, + }; + + const response = await sessionRequest.put(path).send(body); + + expect(verifyIdentifier).toBeCalled(); + expect(verifyProfile).toBeCalled(); + expect(validateMandatoryUserProfile).not.toBeCalled(); + expect(submitInteraction).toBeCalled(); + expect(response.status).toEqual(200); + }); + }); + + describe('PATCH /interaction', () => { + const path = interactionPrefix; + const getInteractionStorageMock = getInteractionStorage as jest.Mock; + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('sign-in event with register event interaction session in record should call methods properly', async () => { + getInteractionStorageMock.mockResolvedValueOnce({ event: Event.Register }); + + const body = { + event: Event.SignIn, + }; + + const response = await sessionRequest.patch(path).send(body); + expect(verifyIdentifier).toBeCalled(); + expect(verifyProfile).toBeCalled(); + expect(validateMandatoryUserProfile).toBeCalled(); + expect(submitInteraction).toBeCalled(); + expect(response.status).toEqual(200); + expect(response.body).toEqual({ redirectUri: 'logto.io' }); + }); + + it('sign-in event with forgot password event interaction session in record should reject', async () => { + getInteractionStorageMock.mockResolvedValueOnce({ event: Event.ForgotPassword }); + + const body = { + event: Event.SignIn, + }; + + const response = await sessionRequest.patch(path).send(body); + expect(verifyIdentifier).not.toBeCalled(); + expect(verifyProfile).not.toBeCalled(); + expect(validateMandatoryUserProfile).not.toBeCalled(); + expect(submitInteraction).not.toBeCalled(); + expect(response.status).toEqual(404); + }); + + it('Forgot event with forgot password event interaction session in record should call methods properly', async () => { + getInteractionStorageMock.mockResolvedValueOnce({ event: Event.ForgotPassword }); + + const body = { + event: Event.ForgotPassword, + }; + + const response = await sessionRequest.patch(path).send(body); + expect(verifyIdentifier).toBeCalled(); + expect(verifyProfile).toBeCalled(); + expect(validateMandatoryUserProfile).not.toBeCalled(); + expect(submitInteraction).toBeCalled(); + expect(response.status).toEqual(200); + expect(response.body).toEqual({ redirectUri: 'logto.io' }); + }); + + it('Forgot event with sign-in event interaction session in record should call methods properly', async () => { + getInteractionStorageMock.mockResolvedValueOnce({ event: Event.SignIn }); + + const body = { + event: Event.ForgotPassword, + }; + + const response = await sessionRequest.patch(path).send(body); + expect(verifyIdentifier).not.toBeCalled(); + expect(verifyProfile).not.toBeCalled(); + expect(validateMandatoryUserProfile).not.toBeCalled(); + expect(submitInteraction).not.toBeCalled(); + expect(response.status).toEqual(404); + }); + }); + describe('POST /verification/passcode', () => { const path = `${verificationPrefix}/passcode`; it('should call send passcode properly', async () => { diff --git a/packages/core/src/routes/interaction/index.ts b/packages/core/src/routes/interaction/index.ts index f02c788c9..4bfbd051d 100644 --- a/packages/core/src/routes/interaction/index.ts +++ b/packages/core/src/routes/interaction/index.ts @@ -21,7 +21,7 @@ import { validateMandatoryUserProfile, } from './verifications/index.js'; -export const identifierPrefix = '/identifier'; +export const interactionPrefix = '/interaction'; export const verificationPrefix = '/verification'; export default function interactionRoutes( @@ -29,7 +29,7 @@ export default function interactionRoutes( provider: Provider ) { router.put( - identifierPrefix, + interactionPrefix, koaInteractionBodyGuard(), koaSessionSignInExperienceGuard(provider), async (ctx, next) => { @@ -53,7 +53,7 @@ export default function interactionRoutes( ); router.patch( - identifierPrefix, + interactionPrefix, koaInteractionBodyGuard(), koaSessionSignInExperienceGuard(provider), async (ctx, next) => { @@ -65,7 +65,7 @@ export default function interactionRoutes( event === Event.ForgotPassword ? interactionStorage.event === Event.ForgotPassword : interactionStorage.event !== Event.ForgotPassword, - new RequestError({ code: 'session.verification_session_not_found' }) + new RequestError({ code: 'session.verification_session_not_found', status: 404 }) ); const identifierVerifiedInteraction = await verifyIdentifier( @@ -86,7 +86,7 @@ export default function interactionRoutes( } ); - router.delete(identifierPrefix, async (ctx, next) => { + router.delete(interactionPrefix, async (ctx, next) => { await provider.interactionDetails(ctx.req, ctx.res); const error: LogtoErrorCode = 'oidc.aborted'; await assignInteractionResults(ctx, provider, { error }); diff --git a/packages/core/src/routes/interaction/utils/index.ts b/packages/core/src/routes/interaction/utils/index.ts index 5e4a285a8..efaa83e29 100644 --- a/packages/core/src/routes/interaction/utils/index.ts +++ b/packages/core/src/routes/interaction/utils/index.ts @@ -1,10 +1,6 @@ -import type { Profile, SocialConnectorPayload, User, IdentifierPayload } from '@logto/schemas'; +import type { SocialConnectorPayload, User, IdentifierPayload } from '@logto/schemas'; -import type { - PasscodeIdentifierPayload, - PasswordIdentifierPayload, - Identifier, -} from '../types/index.js'; +import type { PasscodeIdentifierPayload, PasswordIdentifierPayload } from '../types/index.js'; export const isPasswordIdentifier = ( identifier: IdentifierPayload @@ -19,22 +15,6 @@ export const isSocialIdentifier = ( ): identifier is SocialConnectorPayload => 'connectorId' in identifier && 'connectorData' in identifier; -export const isProfileIdentifier = (identifier: Identifier, profile?: Profile) => { - if (identifier.key === 'accountId') { - return false; - } - - if (identifier.key === 'emailVerified') { - return profile?.email === identifier.value; - } - - if (identifier.key === 'phoneVerified') { - return profile?.phone === identifier.value; - } - - return profile?.connectorId === identifier.connectorId; -}; - // Social identities can take place the role of password export const isUserPasswordSet = ({ passwordEncrypted, diff --git a/packages/core/src/routes/interaction/utils/interaction.test.ts b/packages/core/src/routes/interaction/utils/interaction.test.ts index 274f97d97..6d0a8858b 100644 --- a/packages/core/src/routes/interaction/utils/interaction.test.ts +++ b/packages/core/src/routes/interaction/utils/interaction.test.ts @@ -6,40 +6,37 @@ describe('interaction utils', () => { const emailIdentifier: Identifier = { key: 'emailVerified', value: 'foo@logto.io' }; const phoneIdentifier: Identifier = { key: 'phoneVerified', value: '12346' }; - it('mergeIdentifiers', () => { - expect(mergeIdentifiers({})).toEqual(undefined); - expect(mergeIdentifiers({ oldIdentifiers: [usernameIdentifier] })).toEqual([ - usernameIdentifier, - ]); - expect(mergeIdentifiers({ newIdentifiers: [usernameIdentifier] })).toEqual([ - usernameIdentifier, - ]); - expect( - mergeIdentifiers({ - oldIdentifiers: [usernameIdentifier], - newIdentifiers: [usernameIdentifier], - }) - ).toEqual([usernameIdentifier]); + describe('mergeIdentifiers', () => { + it('new identifiers only ', () => { + expect(mergeIdentifiers([usernameIdentifier])).toEqual([usernameIdentifier]); + }); - expect( - mergeIdentifiers({ - oldIdentifiers: [emailIdentifier], - newIdentifiers: [usernameIdentifier], - }) - ).toEqual([emailIdentifier, usernameIdentifier]); + it('same identifiers should replace', () => { + expect(mergeIdentifiers([usernameIdentifier], [{ key: 'accountId', value: 'foo2' }])).toEqual( + [usernameIdentifier] + ); + }); - expect( - mergeIdentifiers({ - oldIdentifiers: [emailIdentifier, phoneIdentifier], - newIdentifiers: [phoneIdentifier, usernameIdentifier], - }) - ).toEqual([emailIdentifier, phoneIdentifier, usernameIdentifier]); + it('different identifiers should merge', () => { + expect(mergeIdentifiers([emailIdentifier], [usernameIdentifier])).toEqual([ + usernameIdentifier, + emailIdentifier, + ]); - expect( - mergeIdentifiers({ - oldIdentifiers: [emailIdentifier, phoneIdentifier], - newIdentifiers: [usernameIdentifier], - }) - ).toEqual([emailIdentifier, phoneIdentifier, usernameIdentifier]); + expect(mergeIdentifiers([usernameIdentifier], [emailIdentifier, phoneIdentifier])).toEqual([ + emailIdentifier, + phoneIdentifier, + usernameIdentifier, + ]); + }); + + it('mixed identifiers should replace and merge', () => { + expect( + mergeIdentifiers( + [phoneIdentifier, usernameIdentifier], + [emailIdentifier, { key: 'phoneVerified', value: '465789' }] + ) + ).toEqual([emailIdentifier, phoneIdentifier, usernameIdentifier]); + }); }); }); diff --git a/packages/core/src/routes/interaction/utils/interaction.ts b/packages/core/src/routes/interaction/utils/interaction.ts index 49a169113..5237e6e41 100644 --- a/packages/core/src/routes/interaction/utils/interaction.ts +++ b/packages/core/src/routes/interaction/utils/interaction.ts @@ -1,4 +1,4 @@ -import type { Event } from '@logto/schemas'; +import type { Event, Profile } from '@logto/schemas'; import type { Context } from 'koa'; import type { Provider } from 'oidc-provider'; @@ -12,21 +12,29 @@ import type { AccountVerifiedInteractionResult, } from '../types/index.js'; -// Unique identifier type is required -export const mergeIdentifiers = (pairs: { - newIdentifiers?: Identifier[]; - oldIdentifiers?: Identifier[]; -}) => { - const { newIdentifiers, oldIdentifiers } = pairs; - - if (!newIdentifiers) { - return oldIdentifiers; +const isProfileIdentifier = (identifier: Identifier, profile?: Profile) => { + if (identifier.key === 'accountId') { + return false; } + if (identifier.key === 'emailVerified') { + return profile?.email === identifier.value; + } + + if (identifier.key === 'phoneVerified') { + return profile?.phone === identifier.value; + } + + return profile?.connectorId === identifier.connectorId; +}; + +// Unique identifier type is required +export const mergeIdentifiers = (newIdentifiers: Identifier[], oldIdentifiers?: Identifier[]) => { if (!oldIdentifiers) { return newIdentifiers; } + // Filter out identifiers with the same key in the oldIdentifiers and replaced with new ones const leftOvers = oldIdentifiers.filter((oldIdentifier) => { return !newIdentifiers.some((newIdentifier) => newIdentifier.key === oldIdentifier.key); }); @@ -34,6 +42,40 @@ export const mergeIdentifiers = (pairs: { return [...leftOvers, ...newIdentifiers]; }; +/** + * Categorize the identifiers based on their different use cases + * @typedef {Object} result + * @property {Identifier[]} userAccountIdentifiers - identifiers to verify a specific user account e.g. for sign-in and reset-password + * @property {Identifier[]} profileIdentifiers - identifiers to verify a new anonymous profile e.g. new email, new phone or new social identity + * + * @param {Identifier[]} identifiers + * @param {Profile} profile + * @returns + */ +export const categorizeIdentifiers = ( + identifiers: Identifier[], + profile?: Profile +): { + userAccountIdentifiers: Identifier[]; + profileIdentifiers: Identifier[]; +} => { + const userAccountIdentifiers = new Set(); + const profileIdentifiers = new Set(); + + for (const identifier of identifiers) { + if (isProfileIdentifier(identifier, profile)) { + profileIdentifiers.add(identifier); + continue; + } + userAccountIdentifiers.add(identifier); + } + + return { + userAccountIdentifiers: [...userAccountIdentifiers], + profileIdentifiers: [...profileIdentifiers], + }; +}; + export const isAccountVerifiedInteractionResult = ( interaction: AnonymousInteractionResult ): interaction is AccountVerifiedInteractionResult => Boolean(interaction.accountId); @@ -68,3 +110,12 @@ export const getInteractionStorage = async (ctx: Context, provider: Provider) => return parseResult.data; }; + +export const clearInteractionStorage = async (ctx: Context, provider: Provider) => { + const { result } = await provider.interactionDetails(ctx.req, ctx.res); + + if (result) { + const { event, profile, identifier, ...rest } = result; + await provider.interactionResult(ctx.req, ctx.res, { ...rest }); + } +}; diff --git a/packages/core/src/routes/interaction/verifications/identifier-payload-verification.ts b/packages/core/src/routes/interaction/verifications/identifier-payload-verification.ts index 3424c4232..325f6aec7 100644 --- a/packages/core/src/routes/interaction/verifications/identifier-payload-verification.ts +++ b/packages/core/src/routes/interaction/verifications/identifier-payload-verification.ts @@ -66,6 +66,7 @@ const verifySocialIdentityInInteractionRecord = async ( { connectorId, identityType }: SocialIdentityPayload, interactionRecord?: AnonymousInteractionResult ): Promise => { + // Sign-In with social verified email or phone requires a social identifier in the interaction result const socialIdentifierRecord = interactionRecord?.identifiers?.find( (entity): entity is SocialIdentifier => entity.key === 'social' && entity.connectorId === connectorId @@ -88,6 +89,7 @@ const verifyIdentifierPayload = async ( ): Promise => { const { identifier, event } = ctx.interactionPayload; + // No Identifier in payload if (!identifier) { return; } @@ -104,6 +106,7 @@ const verifyIdentifierPayload = async ( return verifySocialIdentifier(identifier, ctx); } + // Sign-In with social verified email or phone return verifySocialIdentityInInteractionRecord(identifier, interactionRecord); }; @@ -119,10 +122,9 @@ export default async function identifierPayloadVerification( const interaction: PayloadVerifiedInteractionResult = { ...interactionRecord, event, - identifiers: mergeIdentifiers({ - oldIdentifiers: interactionRecord?.identifiers, - newIdentifiers: identifier && [identifier], - }), + identifiers: identifier + ? mergeIdentifiers([identifier], interactionRecord?.identifiers) + : interactionRecord?.identifiers, }; await storeInteractionResult(interaction, ctx, provider); diff --git a/packages/core/src/routes/interaction/verifications/profile-verification.ts b/packages/core/src/routes/interaction/verifications/profile-verification.ts index dba032815..a986a6945 100644 --- a/packages/core/src/routes/interaction/verifications/profile-verification.ts +++ b/packages/core/src/routes/interaction/verifications/profile-verification.ts @@ -70,7 +70,7 @@ const verifyProfileIdentifiers = ( } }; -const verifyProfileNotRegistered = async ( +const verifyProfileNotRegisteredByOtherUserAccount = async ( { username, email, phone, connectorId }: Profile, identifiers: Identifier[] = [] ) => { @@ -128,7 +128,10 @@ const verifyProfileNotRegistered = async ( } }; -const verifyProfileNotExist = async ({ username, email, phone, password }: Profile, user: User) => { +const verifyProfileNotExistInCurrentUserAccount = async ( + { username, email, phone, password }: Profile, + user: User +) => { if (username) { assertThat( !user.username, @@ -183,7 +186,7 @@ export default async function verifyProfile( assertThat(isValidRegisterProfile(profile), new RequestError({ code: 'guard.invalid_input' })); verifyProfileIdentifiers(profile, identifiers); - await verifyProfileNotRegistered(profile, identifiers); + await verifyProfileNotRegisteredByOtherUserAccount(profile, identifiers); const interactionWithProfile: VerifiedRegisterInteractionResult = { ...interaction, profile }; await storeInteractionResult(interactionWithProfile, ctx, provider); @@ -195,8 +198,8 @@ export default async function verifyProfile( verifyProfileIdentifiers(profile, identifiers); // Find existing account const user = await findUserById(accountId); - await verifyProfileNotExist(profile, user); - await verifyProfileNotRegistered(profile, identifiers); + await verifyProfileNotExistInCurrentUserAccount(profile, user); + await verifyProfileNotRegisteredByOtherUserAccount(profile, identifiers); const interactionWithProfile: VerifiedSignInInteractionResult = { ...interaction, profile }; await storeInteractionResult(interactionWithProfile, ctx, provider); diff --git a/packages/core/src/routes/interaction/verifications/user-identity-verification.test.ts b/packages/core/src/routes/interaction/verifications/user-identity-verification.test.ts index 1678d90ea..8ac0bc348 100644 --- a/packages/core/src/routes/interaction/verifications/user-identity-verification.test.ts +++ b/packages/core/src/routes/interaction/verifications/user-identity-verification.test.ts @@ -58,7 +58,7 @@ describe('userAccountVerification', () => { }; await expect(userAccountVerification(interaction, ctx, provider)).rejects.toMatchError( - new RequestError({ code: 'session.unauthorized', status: 401 }) + new RequestError({ code: 'session.verification_session_not_found', status: 404 }) ); expect(storeInteractionResult).not.toBeCalled(); }); diff --git a/packages/core/src/routes/interaction/verifications/user-identity-verification.ts b/packages/core/src/routes/interaction/verifications/user-identity-verification.ts index d13ee0aad..1c8a319b0 100644 --- a/packages/core/src/routes/interaction/verifications/user-identity-verification.ts +++ b/packages/core/src/routes/interaction/verifications/user-identity-verification.ts @@ -16,10 +16,10 @@ import type { InteractionContext, } from '../types/index.js'; import findUserByIdentifier from '../utils/find-user-by-identifier.js'; -import { isProfileIdentifier } from '../utils/index.js'; import { storeInteractionResult, isAccountVerifiedInteractionResult, + categorizeIdentifiers, } from '../utils/interaction.js'; const identifyUserByVerifiedEmailOrPhone = async ( @@ -82,46 +82,48 @@ export default async function userAccountVerification( ctx: InteractionContext, provider: Provider ): Promise { - const { identifiers = [], accountId } = interaction; - // Need to merge the profile in payload - const profile = { ...interaction.profile, ...ctx.interactionPayload.profile }; + const { identifiers = [], accountId, profile } = interaction; - // Filter all non-profile identifiers - const userIdentifiers = identifiers.filter( - (identifier) => !isProfileIdentifier(identifier, profile) + const { userAccountIdentifiers, profileIdentifiers } = categorizeIdentifiers( + identifiers, + // Need to merge the profile in payload + { ...profile, ...ctx.interactionPayload.profile } ); - if (isAccountVerifiedInteractionResult(interaction) && userIdentifiers.length === 0) { + // Return the interaction directly if it is accountVerified and has no unverified userAccountIdentifiers + // e.g. profile fulfillment request with account already verified in the interaction result + if (isAccountVerifiedInteractionResult(interaction) && userAccountIdentifiers.length === 0) { return interaction; } + // _userAccountIdentifiers is required to identify a user account assertThat( - userIdentifiers.length > 0, + userAccountIdentifiers.length > 0, new RequestError({ - code: 'session.unauthorized', - status: 401, + code: 'session.verification_session_not_found', + status: 404, }) ); - // Verify All non-profile identifiers + // Verify userAccountIdentifiers const accountIds = await Promise.all( - userIdentifiers.map(async (identifier) => identifyUser(identifier)) + userAccountIdentifiers.map(async (identifier) => identifyUser(identifier)) ); - const deduplicateAccountIds = deduplicate(accountIds); - // Inconsistent identities + // Inconsistent account identifiers check + assertThat(deduplicateAccountIds.length === 1, new RequestError('session.verification_failed')); + + // Valid accountId verification. Should also equal to the accountId in record if exist. Else throw assertThat( - deduplicateAccountIds.length === 1 && - deduplicateAccountIds[0] && - (!accountId || accountId === deduplicateAccountIds[0]), + deduplicateAccountIds[0] && (!accountId || accountId === deduplicateAccountIds[0]), new RequestError('session.verification_failed') ); - // Assign verification result and filter out account verified identifiers + // Assign the verification result and store the profile identifiers left const verifiedInteraction: AccountVerifiedInteractionResult = { ...interaction, - identifiers: identifiers.filter((identifier) => isProfileIdentifier(identifier, profile)), + identifiers: profileIdentifiers, accountId: deduplicateAccountIds[0], };