diff --git a/packages/core/src/routes/interaction/actions/submit-interaction.test.ts b/packages/core/src/routes/interaction/actions/submit-interaction.test.ts index eae657ef1..0b7a06113 100644 --- a/packages/core/src/routes/interaction/actions/submit-interaction.test.ts +++ b/packages/core/src/routes/interaction/actions/submit-interaction.test.ts @@ -153,7 +153,7 @@ describe('submit action', () => { }); }); - it('sign-in', async () => { + it('sign-in with new profile', async () => { getLogtoConnectorById.mockResolvedValueOnce({ metadata: { target: 'logto' }, dbEntry: { syncProfile: false }, @@ -182,10 +182,35 @@ describe('submit action', () => { expect(assignInteractionResults).toBeCalledWith(ctx, provider, { login: { accountId: 'foo' } }); }); + it('sign-in and sync new Social', async () => { + getLogtoConnectorById.mockResolvedValueOnce({ + metadata: { target: 'logto' }, + dbEntry: { syncProfile: true }, + }); + + const interaction: VerifiedSignInInteractionResult = { + event: InteractionEvent.SignIn, + accountId: 'foo', + profile: { email: 'email' }, + identifiers, + }; + + await submitInteraction(interaction, ctx, provider); + expect(getLogtoConnectorById).toBeCalledWith('logto'); + expect(updateUserById).toBeCalledWith('foo', { + primaryEmail: 'email', + name: userInfo.name, + avatar: userInfo.avatar, + lastSignInAt: now, + }); + expect(assignInteractionResults).toBeCalledWith(ctx, provider, { login: { accountId: 'foo' } }); + }); + it('reset password', async () => { const interaction: VerifiedForgotPasswordInteractionResult = { event: InteractionEvent.ForgotPassword, accountId: 'foo', + identifiers: [{ key: 'accountId', value: 'foo' }], profile: { password: 'password' }, }; await submitInteraction(interaction, ctx, provider); diff --git a/packages/core/src/routes/interaction/actions/submit-interaction.ts b/packages/core/src/routes/interaction/actions/submit-interaction.ts index e299ea735..1b1a6478c 100644 --- a/packages/core/src/routes/interaction/actions/submit-interaction.ts +++ b/packages/core/src/routes/interaction/actions/submit-interaction.ts @@ -1,4 +1,4 @@ -import type { User } from '@logto/schemas'; +import type { User, Profile } from '@logto/schemas'; import { InteractionEvent, UserRole, adminConsoleApplicationId } from '@logto/schemas'; import { conditional } from '@silverhand/essentials'; import type { Provider } from 'oidc-provider'; @@ -16,23 +16,23 @@ import type { VerifiedSignInInteractionResult, VerifiedRegisterInteractionResult, } from '../types/index.js'; -import { clearInteractionStorage } from '../utils/interaction.js'; +import { clearInteractionStorage, categorizeIdentifiers } from '../utils/interaction.js'; -const getSocialUpdateProfile = async ({ +const filterSocialIdentifiers = (identifiers: Identifier[]): SocialIdentifier[] => + identifiers.filter((identifier): identifier is SocialIdentifier => identifier.key === 'social'); + +const getNewSocialProfile = async ({ user, connectorId, identifiers, }: { user?: User; connectorId: string; - identifiers?: Identifier[]; + identifiers: SocialIdentifier[]; }) => { // TODO: @simeng refactor me. This step should be verified by the previous profile verification cycle Already. // Should pickup the verified social user info result automatically - const socialIdentifier = identifiers?.find( - (identifier): identifier is SocialIdentifier => - identifier.key === 'social' && identifier.connectorId === connectorId - ); + const socialIdentifier = identifiers.find((identifier) => identifier.connectorId === connectorId); if (!socialIdentifier) { return; @@ -46,6 +46,7 @@ const getSocialUpdateProfile = async ({ const { userInfo } = socialIdentifier; const { name, avatar, id } = userInfo; + // Update the user name and avatar if the connector has syncProfile enabled or is new registered user const profileUpdate = conditional( (syncProfile || !user) && { ...conditional(name && { name }), @@ -59,19 +60,41 @@ const getSocialUpdateProfile = async ({ }; }; -const parseUserProfile = async ( - { profile, identifiers }: VerifiedSignInInteractionResult | VerifiedRegisterInteractionResult, +const getSyncedSocialUserProfile = async (socialIdentifier: SocialIdentifier) => { + const { + userInfo: { name, avatar }, + connectorId, + } = socialIdentifier; + + const { + dbEntry: { syncProfile }, + } = await getLogtoConnectorById(connectorId); + + return conditional( + syncProfile && { + ...conditional(name && { name }), + ...conditional(avatar && { avatar }), + } + ); +}; + +const parseNewUserProfile = async ( + profile: Profile, + profileIdentifiers: Identifier[], user?: User ) => { - if (!profile) { - return; - } - const { phone, username, email, connectorId, password } = profile; const [passwordProfile, socialProfile] = await Promise.all([ conditional(password && (await encryptUserPassword(password))), - conditional(connectorId && (await getSocialUpdateProfile({ connectorId, identifiers, user }))), + conditional( + connectorId && + (await getNewSocialProfile({ + connectorId, + identifiers: filterSocialIdentifiers(profileIdentifiers), + user, + })) + ), ]); return { @@ -80,6 +103,26 @@ const parseUserProfile = async ( ...conditional(email && { primaryEmail: email }), ...passwordProfile, ...socialProfile, + }; +}; + +const parseUserProfile = async ( + { profile, identifiers }: VerifiedSignInInteractionResult | VerifiedRegisterInteractionResult, + user?: User +) => { + const { authIdentifiers, profileIdentifiers } = categorizeIdentifiers(identifiers ?? [], profile); + + const newUserProfile = profile && (await parseNewUserProfile(profile, profileIdentifiers, user)); + + // Sync the last social profile + const socialIdentifier = filterSocialIdentifiers(authIdentifiers).slice(-1)[0]; + + const syncedSocialUserProfile = + socialIdentifier && (await getSyncedSocialUserProfile(socialIdentifier)); + + return { + ...syncedSocialUserProfile, + ...newUserProfile, lastSignInAt: Date.now(), }; }; @@ -118,9 +161,7 @@ export default async function submitInteraction( const user = await findUserById(accountId); const upsertProfile = await parseUserProfile(interaction, user); - if (upsertProfile) { - await updateUserById(accountId, upsertProfile); - } + await updateUserById(accountId, upsertProfile); await assignInteractionResults(ctx, provider, { login: { accountId } }); @@ -131,6 +172,7 @@ export default async function submitInteraction( const { passwordEncrypted, passwordEncryptionMethod } = await encryptUserPassword( profile.password ); + await updateUserById(accountId, { passwordEncrypted, passwordEncryptionMethod }); await clearInteractionStorage(ctx, provider); ctx.status = 204; diff --git a/packages/core/src/routes/interaction/types/guard.ts b/packages/core/src/routes/interaction/types/guard.ts index bf328af60..0cf7b44d2 100644 --- a/packages/core/src/routes/interaction/types/guard.ts +++ b/packages/core/src/routes/interaction/types/guard.ts @@ -67,7 +67,7 @@ export const verifiedSignInteractionResultGuard = z.object({ event: z.literal(InteractionEvent.SignIn), accountId: z.string(), profile: profileGuard.optional(), - identifiers: z.array(identifierGuard).optional(), + identifiers: z.array(identifierGuard), }); export const forgotPasswordProfileGuard = z.object({ @@ -77,5 +77,6 @@ export const forgotPasswordProfileGuard = z.object({ export const verifiedForgotPasswordInteractionResultGuard = z.object({ event: z.literal(InteractionEvent.ForgotPassword), accountId: z.string(), + identifiers: z.array(identifierGuard), profile: forgotPasswordProfileGuard, }); diff --git a/packages/core/src/routes/interaction/types/index.ts b/packages/core/src/routes/interaction/types/index.ts index ef4ff3e1c..393588102 100644 --- a/packages/core/src/routes/interaction/types/index.ts +++ b/packages/core/src/routes/interaction/types/index.ts @@ -68,11 +68,13 @@ export type ForgotPasswordInteractionResult = Omit & { + | (Omit & { accountId: string; + identifiers: Identifier[]; }) - | (Omit & { + | (Omit & { accountId: string; + identifiers: Identifier[]; }); export type IdentifierVerifiedInteractionResult = diff --git a/packages/core/src/routes/interaction/utils/interaction.test.ts b/packages/core/src/routes/interaction/utils/interaction.test.ts index c0971c42e..ebec31bc8 100644 --- a/packages/core/src/routes/interaction/utils/interaction.test.ts +++ b/packages/core/src/routes/interaction/utils/interaction.test.ts @@ -44,7 +44,7 @@ describe('interaction utils', () => { { email: 'foo@logto.io', connectorId: 'foo_connector' } ) ).toEqual({ - userAccountIdentifiers: [usernameIdentifier, phoneIdentifier], + authIdentifiers: [usernameIdentifier, phoneIdentifier], profileIdentifiers: [emailIdentifier, socialIdentifier], }); }); diff --git a/packages/core/src/routes/interaction/utils/interaction.ts b/packages/core/src/routes/interaction/utils/interaction.ts index c1e51d680..957764a0a 100644 --- a/packages/core/src/routes/interaction/utils/interaction.ts +++ b/packages/core/src/routes/interaction/utils/interaction.ts @@ -48,7 +48,7 @@ export const mergeIdentifiers = (newIdentifier: Identifier, oldIdentifiers?: Ide /** * 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[]} authIdentifiers - 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 @@ -59,10 +59,10 @@ export const categorizeIdentifiers = ( identifiers: Identifier[], profile?: Profile ): { - userAccountIdentifiers: Identifier[]; + authIdentifiers: Identifier[]; profileIdentifiers: Identifier[]; } => { - const userAccountIdentifiers = new Set(); + const authIdentifiers = new Set(); const profileIdentifiers = new Set(); for (const identifier of identifiers) { @@ -70,11 +70,11 @@ export const categorizeIdentifiers = ( profileIdentifiers.add(identifier); continue; } - userAccountIdentifiers.add(identifier); + authIdentifiers.add(identifier); } return { - userAccountIdentifiers: [...userAccountIdentifiers], + authIdentifiers: [...authIdentifiers], profileIdentifiers: [...profileIdentifiers], }; }; diff --git a/packages/core/src/routes/interaction/verifications/mandatory-user-profile-validation.test.ts b/packages/core/src/routes/interaction/verifications/mandatory-user-profile-validation.test.ts index a424f5e38..677d38687 100644 --- a/packages/core/src/routes/interaction/verifications/mandatory-user-profile-validation.test.ts +++ b/packages/core/src/routes/interaction/verifications/mandatory-user-profile-validation.test.ts @@ -32,6 +32,7 @@ describe('validateMandatoryUserProfile', () => { }; const interaction: IdentifierVerifiedInteractionResult = { event: InteractionEvent.SignIn, + identifiers: [{ key: 'accountId', value: 'foo' }], accountId: 'foo', }; diff --git a/packages/core/src/routes/interaction/verifications/profile-verification-forgot-password.test.ts b/packages/core/src/routes/interaction/verifications/profile-verification-forgot-password.test.ts index 11663e4be..784bc0d10 100644 --- a/packages/core/src/routes/interaction/verifications/profile-verification-forgot-password.test.ts +++ b/packages/core/src/routes/interaction/verifications/profile-verification-forgot-password.test.ts @@ -3,6 +3,8 @@ import { createMockUtils, pickDefault } from '@logto/shared/esm'; import RequestError from '#src/errors/RequestError/index.js'; +import type { Identifier } from '../types/index.js'; + const { jest } = import.meta; const { mockEsm, mockEsmWithActual } = createMockUtils(jest); @@ -19,6 +21,7 @@ const verifyProfile = await pickDefault(import('./profile-verification.js')); describe('forgot password interaction profile verification', () => { const baseInteraction = { event: InteractionEvent.ForgotPassword, + identifiers: [{ key: 'accountId', value: 'foo' }] as Identifier[], accountId: 'foo', }; diff --git a/packages/core/src/routes/interaction/verifications/profile-verification-protected-identifier.test.ts b/packages/core/src/routes/interaction/verifications/profile-verification-protected-identifier.test.ts index 6556057b8..e4e337f74 100644 --- a/packages/core/src/routes/interaction/verifications/profile-verification-protected-identifier.test.ts +++ b/packages/core/src/routes/interaction/verifications/profile-verification-protected-identifier.test.ts @@ -24,7 +24,11 @@ mockEsm('#src/connectors/index.js', () => ({ const verifyProfile = await pickDefault(import('./profile-verification.js')); describe('profile protected identifier verification', () => { - const baseInteraction = { event: InteractionEvent.SignIn, accountId: 'foo' }; + const baseInteraction = { + event: InteractionEvent.SignIn, + identifiers: [{ key: 'accountId', value: 'foo' }] as Identifier[], + accountId: 'foo', + }; afterEach(() => { jest.clearAllMocks(); 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 dbbb27041..025b087f9 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 @@ -23,18 +23,7 @@ describe('verifyUserAccount', () => { jest.clearAllMocks(); }); - it('empty identifiers with accountId', async () => { - const interaction: SignInInteractionResult = { - event: InteractionEvent.SignIn, - accountId: 'foo', - }; - - const result = await verifyUserAccount(interaction); - - expect(result).toEqual(result); - }); - - it('empty identifiers withOut accountId should throw', async () => { + it('empty identifiers should throw', async () => { const interaction: SignInInteractionResult = { event: InteractionEvent.SignIn, }; @@ -52,7 +41,7 @@ describe('verifyUserAccount', () => { const result = await verifyUserAccount(interaction); - expect(result).toEqual({ event: InteractionEvent.SignIn, accountId: 'foo', identifiers: [] }); + expect(result).toEqual({ ...interaction, accountId: 'foo' }); }); it('verify emailVerified identifier', async () => { @@ -66,7 +55,7 @@ describe('verifyUserAccount', () => { const result = await verifyUserAccount(interaction); expect(findUserByIdentifierMock).toBeCalledWith({ email: 'email' }); - expect(result).toEqual({ event: InteractionEvent.SignIn, accountId: 'foo', identifiers: [] }); + expect(result).toEqual({ ...interaction, accountId: 'foo' }); }); it('verify phoneVerified identifier', async () => { @@ -80,7 +69,7 @@ describe('verifyUserAccount', () => { const result = await verifyUserAccount(interaction); expect(findUserByIdentifierMock).toBeCalledWith({ phone: '123456' }); - expect(result).toEqual({ event: InteractionEvent.SignIn, accountId: 'foo', identifiers: [] }); + expect(result).toEqual({ ...interaction, accountId: 'foo' }); }); it('verify social identifier', async () => { @@ -97,7 +86,7 @@ describe('verifyUserAccount', () => { userInfo: { id: 'foo' }, }); - expect(result).toEqual({ event: InteractionEvent.SignIn, accountId: 'foo', identifiers: [] }); + expect(result).toEqual({ ...interaction, accountId: 'foo' }); }); it('verify social identifier user identity not exist', async () => { @@ -138,7 +127,7 @@ describe('verifyUserAccount', () => { const result = await verifyUserAccount(interaction); expect(findUserByIdentifierMock).toBeCalledWith({ email: 'email' }); - expect(result).toEqual({ event: InteractionEvent.SignIn, accountId: 'foo', identifiers: [] }); + expect(result).toEqual({ ...interaction, accountId: 'foo' }); }); it('verify accountId and emailVerified identifier with email user not exist', async () => { @@ -234,17 +223,6 @@ describe('verifyUserAccount', () => { const result = await verifyUserAccount(interaction); expect(findUserByIdentifierMock).toBeCalledWith({ email: 'email' }); - expect(result).toEqual({ - event: InteractionEvent.SignIn, - accountId: 'foo', - identifiers: [ - { key: 'social', connectorId: 'connectorId', userInfo: { id: 'foo' } }, - { key: 'phoneVerified', value: '123456' }, - ], - profile: { - phone: '123456', - connectorId: 'connectorId', - }, - }); + expect(result).toEqual({ ...interaction, accountId: 'foo' }); }); }); 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 f1c31a28b..fdb4aeb27 100644 --- a/packages/core/src/routes/interaction/verifications/user-identity-verification.ts +++ b/packages/core/src/routes/interaction/verifications/user-identity-verification.ts @@ -15,7 +15,7 @@ import type { Identifier, } from '../types/index.js'; import findUserByIdentifier from '../utils/find-user-by-identifier.js'; -import { isAccountVerifiedInteractionResult, categorizeIdentifiers } from '../utils/interaction.js'; +import { categorizeIdentifiers } from '../utils/interaction.js'; const identifyUserByVerifiedEmailOrPhone = async ( identifier: VerifiedEmailIdentifier | VerifiedPhoneIdentifier @@ -77,29 +77,21 @@ export default async function verifyUserAccount( ): Promise { const { identifiers = [], accountId, profile } = interaction; - const { userAccountIdentifiers, profileIdentifiers } = categorizeIdentifiers( - identifiers, - profile - ); + // Only verify authIdentifiers, should ignore those profile identifiers + const { authIdentifiers } = categorizeIdentifiers(identifiers, profile); - // 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 + // _authIdentifiers is required to identify a user account assertThat( - userAccountIdentifiers.length > 0, + authIdentifiers.length > 0, new RequestError({ code: 'session.identifier_not_found', status: 404, }) ); - // Verify userAccountIdentifiers + // Verify authIdentifiers const accountIds = await Promise.all( - userAccountIdentifiers.map(async (identifier) => identifyUser(identifier)) + authIdentifiers.map(async (identifier) => identifyUser(identifier)) ); const deduplicateAccountIds = deduplicate(accountIds); @@ -112,10 +104,9 @@ export default async function verifyUserAccount( new RequestError('session.verification_failed') ); - // Return the verified interaction and remove the consumed userAccountIdentifiers return { ...interaction, - identifiers: profileIdentifiers, + identifiers, accountId: deduplicateAccountIds[0], }; }