refactor(core): refactor interaction identifier verification flow (#2790)

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);

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;

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,
 });

packages/core/src/routes/interaction/types/index.ts

@@ -68,11 +68,13 @@ export type ForgotPasswordInteractionResult = Omit<AnonymousInteractionResult, '
 };
 
 export type AccountVerifiedInteractionResult =
-  | (Omit<SignInInteractionResult, 'accountId'> & {
+  | (Omit<SignInInteractionResult, 'accountId' | 'identifiers'> & {
       accountId: string;
+      identifiers: Identifier[];
     })
-  | (Omit<ForgotPasswordInteractionResult, 'accountId'> & {
+  | (Omit<ForgotPasswordInteractionResult, 'accountId' | 'identifiers'> & {
       accountId: string;
+      identifiers: Identifier[];
     });
 
 export type IdentifierVerifiedInteractionResult =

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],
       });
     });

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<Identifier>();
+  const authIdentifiers = new Set<Identifier>();
   const profileIdentifiers = new Set<Identifier>();
 
   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],
   };
 };

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',
   };
 

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',
   };
 

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();

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' });
   });
 });

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<AccountVerifiedInteractionResult> {
   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],
   };
 }