From 04ec524e677df62875e8ff90abf93cd2a4d6bc98 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Mon, 9 Oct 2023 11:15:02 +0800 Subject: [PATCH] fix(core): fix new linked social email conflict error (#4623) --- .../src/routes/interaction/actions/helpers.ts | 151 +++++++++++++++ .../actions/submit-interaction.test.ts | 37 +++- .../interaction/actions/submit-interaction.ts | 175 +----------------- 3 files changed, 195 insertions(+), 168 deletions(-) create mode 100644 packages/core/src/routes/interaction/actions/helpers.ts diff --git a/packages/core/src/routes/interaction/actions/helpers.ts b/packages/core/src/routes/interaction/actions/helpers.ts new file mode 100644 index 000000000..a2aea3043 --- /dev/null +++ b/packages/core/src/routes/interaction/actions/helpers.ts @@ -0,0 +1,151 @@ +import { defaults, parseAffiliateData } from '@logto/affiliate'; +import { consoleLog } from '@logto/cli/lib/utils.js'; +import { type CreateUser, type User, adminTenantId } from '@logto/schemas'; +import { type OmitAutoSetFields } from '@logto/shared'; +import { conditional, trySafe } from '@silverhand/essentials'; +import { type IRouterContext } from 'koa-router'; + +import { EnvSet } from '#src/env-set/index.js'; +import { type CloudConnectionLibrary } from '#src/libraries/cloud-connection.js'; +import { type ConnectorLibrary } from '#src/libraries/connector.js'; +import { encryptUserPassword } from '#src/libraries/user.js'; +import type Queries from '#src/tenants/Queries.js'; +import type TenantContext from '#src/tenants/TenantContext.js'; + +import { + type Identifier, + type SocialIdentifier, + type VerifiedSignInInteractionResult, + type VerifiedRegisterInteractionResult, +} from '../types/index.js'; +import { categorizeIdentifiers } from '../utils/interaction.js'; + +const filterSocialIdentifiers = (identifiers: Identifier[]): SocialIdentifier[] => + identifiers.filter((identifier): identifier is SocialIdentifier => identifier.key === 'social'); + +/* Sync avatar and name from the latest social identity for existing users */ +const getSocialSyncProfile = async ( + { getLogtoConnectorById }: ConnectorLibrary, + authIdentifiers: Identifier[] +) => { + const socialIdentifier = filterSocialIdentifiers(authIdentifiers).at(-1); + + if (!socialIdentifier) { + return; + } + + const { + userInfo: { name, avatar }, + connectorId, + } = socialIdentifier; + + const { + dbEntry: { syncProfile }, + } = await getLogtoConnectorById(connectorId); + + return conditional( + syncProfile && { + ...conditional(name && { name }), + ...conditional(avatar && { avatar }), + } + ); +}; + +/* Parse the user profile from the new linked Social identity */ +const parseNewSocialProfile = async ( + { users: { hasUserWithEmail, hasUserWithPhone } }: Queries, + { getLogtoConnectorById }: ConnectorLibrary, + socialIdentifier: SocialIdentifier, + user?: User +) => { + const { connectorId, userInfo } = socialIdentifier; + const { name, avatar, id, email, phone } = userInfo; + + const { + metadata: { target }, + dbEntry: { syncProfile }, + } = await getLogtoConnectorById(connectorId); + + // Sync the social identity, merge the new social identity with the existing one + const identities = { ...user?.identities, [target]: { userId: id, details: userInfo } }; + + // Parse the profile for new user (register) + if (!user) { + return { + identities, + ...conditional(name && { name }), + ...conditional(avatar && { avatar }), + // Sync the email only if the email is not used by other users + ...conditional(email && !(await hasUserWithEmail(email)) && { primaryEmail: email }), + // Sync the phone only if the phone is not used by other users + ...conditional(phone && !(await hasUserWithPhone(phone)) && { primaryPhone: phone }), + }; + } + + // Sync the user name and avatar to the existing user if the connector has syncProfile enabled (sign-in) + return { + identities, + ...conditional( + syncProfile && { + ...conditional(name && { name }), + ...conditional(avatar && { avatar }), + } + ), + }; +}; + +/* Parse the user profile from the interaction result. */ +export const parseUserProfile = async ( + { connectors, queries }: TenantContext, + { profile, identifiers }: VerifiedSignInInteractionResult | VerifiedRegisterInteractionResult, + user?: User +): Promise, 'id'>> => { + const { authIdentifiers, profileIdentifiers } = categorizeIdentifiers(identifiers ?? [], profile); + const { phone, username, email, connectorId, password } = profile ?? {}; + + // Parse the new social profiles + const socialProfileIdentifier = connectorId + ? profileIdentifiers.find( + (identifier): identifier is SocialIdentifier => + identifier.key === 'social' && identifier.connectorId === connectorId + ) + : undefined; + + const newSocialProfile = + socialProfileIdentifier && + (await parseNewSocialProfile(queries, connectors, socialProfileIdentifier, user)); + + return { + ...newSocialProfile, + ...conditional(password && (await encryptUserPassword(password))), + ...conditional(phone && { primaryPhone: phone }), + ...conditional(email && { primaryEmail: email }), + ...conditional(username && { username }), + ...conditional(user && (await getSocialSyncProfile(connectors, authIdentifiers))), + lastSignInAt: Date.now(), + }; +}; + +/* Post affiliate data to the cloud service. */ +export const postAffiliateLogs = async ( + ctx: IRouterContext, + cloudConnection: CloudConnectionLibrary, + userId: string, + tenantId: string +) => { + if (!EnvSet.values.isCloud || tenantId !== adminTenantId) { + return; + } + + const affiliateData = trySafe(() => + parseAffiliateData(JSON.parse(decodeURIComponent(ctx.cookies.get(defaults.cookieName) ?? ''))) + ); + + if (affiliateData) { + const client = await cloudConnection.getClient(); + await client.post('/api/affiliate-logs', { + body: { userId, ...affiliateData }, + }); + consoleLog.info('Affiliate logs posted', userId); + } +}; 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 a82e41821..d5bdd66ae 100644 --- a/packages/core/src/routes/interaction/actions/submit-interaction.test.ts +++ b/packages/core/src/routes/interaction/actions/submit-interaction.test.ts @@ -51,8 +51,11 @@ const userQueries = { }), updateUserById: jest.fn(), hasActiveUsers: jest.fn().mockResolvedValue(true), + hasUserWithEmail: jest.fn().mockResolvedValue(false), + hasUserWithPhone: jest.fn().mockResolvedValue(false), }; -const { hasActiveUsers, updateUserById } = userQueries; + +const { hasActiveUsers, updateUserById, hasUserWithEmail, hasUserWithPhone } = userQueries; const userLibraries = { generateUserId: jest.fn().mockResolvedValue('uid'), insertUser: jest.fn() }; const { generateUserId, insertUser } = userLibraries; @@ -175,6 +178,38 @@ describe('submit action', () => { ); }); + it('register new social user should not sync email and phone if already exists', async () => { + hasUserWithEmail.mockResolvedValueOnce(true); + hasUserWithPhone.mockResolvedValueOnce(true); + + const interaction: VerifiedRegisterInteractionResult = { + event: InteractionEvent.Register, + profile: { connectorId: 'logto', username: 'username' }, + identifiers, + }; + + await submitInteraction(interaction, ctx, tenant); + + expect(generateUserId).toBeCalled(); + expect(hasActiveUsers).not.toBeCalled(); + expect(encryptUserPassword).not.toBeCalled(); + expect(getLogtoConnectorById).toBeCalledWith('logto'); + + expect(insertUser).toBeCalledWith( + { + id: 'uid', + username: 'username', + identities: { + logto: { userId: userInfo.id, details: userInfo }, + }, + name: userInfo.name, + avatar: userInfo.avatar, + lastSignInAt: now, + }, + ['user'] + ); + }); + it('register with bindMfa', async () => { const interaction: VerifiedRegisterInteractionResult = { event: InteractionEvent.Register, diff --git a/packages/core/src/routes/interaction/actions/submit-interaction.ts b/packages/core/src/routes/interaction/actions/submit-interaction.ts index 9aebb5a41..9b5b92a22 100644 --- a/packages/core/src/routes/interaction/actions/submit-interaction.ts +++ b/packages/core/src/routes/interaction/actions/submit-interaction.ts @@ -1,7 +1,6 @@ -import { defaults, parseAffiliateData } from '@logto/affiliate'; import { Component, CoreEvent, getEventName } from '@logto/app-insights/custom-event'; import { appInsights } from '@logto/app-insights/node'; -import type { User, Profile, CreateUser } from '@logto/schemas'; +import type { User } from '@logto/schemas'; import { AdminTenantRole, SignInMode, @@ -11,13 +10,10 @@ import { InteractionEvent, adminConsoleApplicationId, } from '@logto/schemas'; -import { generateStandardId, type OmitAutoSetFields } from '@logto/shared'; +import { generateStandardId } from '@logto/shared'; import { conditional, conditionalArray, trySafe } from '@silverhand/essentials'; -import { type IRouterContext } from 'koa-router'; import { EnvSet } from '#src/env-set/index.js'; -import { type CloudConnectionLibrary } from '#src/libraries/cloud-connection.js'; -import type { ConnectorLibrary } from '#src/libraries/connector.js'; import { assignInteractionResults } from '#src/libraries/session.js'; import { encryptUserPassword } from '#src/libraries/user.js'; import type { LogEntry, WithLogContext } from '#src/middleware/koa-audit-log.js'; @@ -28,145 +24,13 @@ import { getTenantId } from '#src/utils/tenant.js'; import type { WithInteractionDetailsContext } from '../middleware/koa-interaction-details.js'; import { type WithInteractionHooksContext } from '../middleware/koa-interaction-hooks.js'; import type { - Identifier, VerifiedInteractionResult, - SocialIdentifier, VerifiedSignInInteractionResult, VerifiedRegisterInteractionResult, } from '../types/index.js'; -import { clearInteractionStorage, categorizeIdentifiers } from '../utils/interaction.js'; +import { clearInteractionStorage } from '../utils/interaction.js'; -const filterSocialIdentifiers = (identifiers: Identifier[]): SocialIdentifier[] => - identifiers.filter((identifier): identifier is SocialIdentifier => identifier.key === 'social'); - -const getNewSocialProfile = async ( - { getLogtoConnectorById }: ConnectorLibrary, - { - user, - connectorId, - identifiers, - }: { - user?: User; - connectorId: string; - identifiers: SocialIdentifier[]; - } -) => { - // TODO: @simeng refactor me. This step should be verified by the previous profile verification cycle Already. - const socialIdentifier = identifiers.find((identifier) => identifier.connectorId === connectorId); - - if (!socialIdentifier) { - return; - } - - const { - metadata: { target }, - dbEntry: { syncProfile }, - } = await getLogtoConnectorById(connectorId); - - const { userInfo } = socialIdentifier; - const { name, avatar, id, email, phone } = userInfo; - - const identities = { ...user?.identities, [target]: { userId: id, details: userInfo } }; - - // Sync the name, avatar, email and phone for new user - if (!user) { - return { - identities, - ...conditional(name && { name }), - ...conditional(avatar && { avatar }), - ...conditional(email && { primaryEmail: email }), - ...conditional(phone && { primaryPhone: phone }), - }; - } - - // Sync the user name and avatar if the connector has syncProfile enabled - return { - identities, - ...conditional( - syncProfile && { - ...conditional(name && { name }), - ...conditional(avatar && { avatar }), - } - ), - }; -}; - -const getLatestUserProfileFromSocial = async ( - { getLogtoConnectorById }: ConnectorLibrary, - authIdentifiers: Identifier[] -) => { - const socialIdentifier = filterSocialIdentifiers(authIdentifiers).at(-1); - - if (!socialIdentifier) { - return; - } - - const { - userInfo: { name, avatar }, - connectorId, - } = socialIdentifier; - - const { - dbEntry: { syncProfile }, - } = await getLogtoConnectorById(connectorId); - - return conditional( - syncProfile && { - ...conditional(name && { name }), - ...conditional(avatar && { avatar }), - } - ); -}; - -const parseNewUserProfile = async ( - connectorLibrary: ConnectorLibrary, - profile: Profile, - profileIdentifiers: Identifier[], - user?: User -) => { - const { phone, username, email, connectorId, password } = profile; - - const [passwordProfile, socialProfile] = await Promise.all([ - conditional(password && (await encryptUserPassword(password))), - conditional( - connectorId && - (await getNewSocialProfile(connectorLibrary, { - connectorId, - identifiers: filterSocialIdentifiers(profileIdentifiers), - user, - })) - ), - ]); - - return { - ...socialProfile, // SocialProfile should be applied first - ...passwordProfile, - ...conditional(phone && { primaryPhone: phone }), - ...conditional(username && { username }), - ...conditional(email && { primaryEmail: email }), - }; -}; - -const parseUserProfile = async ( - connectorLibrary: ConnectorLibrary, - { profile, identifiers }: VerifiedSignInInteractionResult | VerifiedRegisterInteractionResult, - user?: User -): Promise, 'id'>> => { - const { authIdentifiers, profileIdentifiers } = categorizeIdentifiers(identifiers ?? [], profile); - - const newUserProfile = - profile && (await parseNewUserProfile(connectorLibrary, profile, profileIdentifiers, user)); - - // Sync from the latest social identity profile for existing users - const syncedSocialUserProfile = - user && (await getLatestUserProfileFromSocial(connectorLibrary, authIdentifiers)); - - return { - ...syncedSocialUserProfile, - ...newUserProfile, - lastSignInAt: Date.now(), - }; -}; +import { postAffiliateLogs, parseUserProfile } from './helpers.js'; const parseBindMfa = ({ bindMfa, @@ -196,36 +60,13 @@ const getInitialUserRoles = ( isCreatingFirstAdminUser && isCloud && getManagementApiAdminName(adminTenantId) ); -/** Post affiliate data to the cloud service. */ -const postAffiliateLogs = async ( - ctx: IRouterContext, - cloudConnection: CloudConnectionLibrary, - userId: string, - tenantId: string -) => { - if (!EnvSet.values.isCloud || tenantId !== adminTenantId) { - return; - } - - const affiliateData = trySafe(() => - parseAffiliateData(JSON.parse(decodeURIComponent(ctx.cookies.get(defaults.cookieName) ?? ''))) - ); - - if (affiliateData) { - const client = await cloudConnection.getClient(); - await client.post('/api/affiliate-logs', { - body: { userId, ...affiliateData }, - }); - consoleLog.info('Affiliate logs posted', userId); - } -}; - export default async function submitInteraction( interaction: VerifiedInteractionResult, ctx: WithLogContext & WithInteractionDetailsContext & WithInteractionHooksContext, - { provider, libraries, connectors, queries, cloudConnection, id: tenantId }: TenantContext, + tenantContext: TenantContext, log?: LogEntry ) { + const { provider, libraries, queries, cloudConnection, id: tenantId } = tenantContext; const { hasActiveUsers, findUserById, updateUserById } = queries.users; const { updateDefaultSignInExperience } = queries.signInExperiences; @@ -236,7 +77,7 @@ export default async function submitInteraction( if (event === InteractionEvent.Register) { const id = await generateUserId(); - const userProfile = await parseUserProfile(connectors, interaction); + const userProfile = await parseUserProfile(tenantContext, interaction); const mfaVerification = parseBindMfa(interaction); const { client_id } = ctx.interactionDetails.params; @@ -283,7 +124,7 @@ export default async function submitInteraction( if (event === InteractionEvent.SignIn) { const user = await findUserById(accountId); - const updateUserProfile = await parseUserProfile(connectors, interaction, user); + const updateUserProfile = await parseUserProfile(tenantContext, interaction, user); const mfaVerification = parseBindMfa(interaction); await updateUserById(accountId, {