Merge pull request #4454 from logto-io/gao-check-personal-info

refactor(core): check personal info when setting password

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

@@ -72,13 +72,9 @@ export default function interactionRoutes<T extends AnonymousRouter>(
       status: [204, 400, 401, 403, 422],
     }),
     koaInteractionSie(queries),
-    async ({ guard: { body }, passwordPolicyChecker }, next) => {
-      await validatePassword(body.profile?.password, passwordPolicyChecker);
-      return next();
-    },
     async (ctx, next) => {
       const { event, identifier, profile } = ctx.guard.body;
-      const { signInExperience, createLog } = ctx;
+      const { signInExperience, createLog, passwordPolicyChecker } = ctx;
 
       const eventLog = createLog(`Interaction.${event}.Update`);
       eventLog.append({ event });
@@ -101,6 +97,11 @@ export default function interactionRoutes<T extends AnonymousRouter>(
 
       eventLog.append({ profile, verifiedIdentifiers });
 
+      await validatePassword(tenant, profile?.password, passwordPolicyChecker, {
+        identifiers: verifiedIdentifier,
+        profile,
+      });
+
       await storeInteractionResult(
         { event, identifiers: verifiedIdentifiers, profile },
         ctx,
@@ -210,17 +211,13 @@ export default function interactionRoutes<T extends AnonymousRouter>(
       status: [204, 400, 404],
     }),
     koaInteractionSie(queries),
-    async ({ guard: { body }, passwordPolicyChecker }, next) => {
-      await validatePassword(body.password, passwordPolicyChecker);
-      return next();
-    },
     async (ctx, next) => {
       const profilePayload = ctx.guard.body;
-      const { signInExperience, interactionDetails, createLog } = ctx;
+      const { signInExperience, interactionDetails, createLog, passwordPolicyChecker } = ctx;
 
       // Check interaction exists
       const interactionStorage = getInteractionStorage(interactionDetails.result);
-      const { event } = interactionStorage;
+      const { event, identifiers } = interactionStorage;
 
       const profileLog = createLog(`Interaction.${event}.Profile.Create`);
       profileLog.append({ profile: profilePayload, interactionStorage });
@@ -229,6 +226,11 @@ export default function interactionRoutes<T extends AnonymousRouter>(
         verifyProfileSettings(profilePayload, signInExperience);
       }
 
+      await validatePassword(tenant, profilePayload.password, passwordPolicyChecker, {
+        identifiers,
+        profile: profilePayload,
+      });
+
       await storeInteractionResult(
         {
           profile: profilePayload,
@@ -252,30 +254,31 @@ export default function interactionRoutes<T extends AnonymousRouter>(
       status: [204, 400, 404],
     }),
     koaInteractionSie(queries),
-    async ({ guard: { body }, passwordPolicyChecker }, next) => {
-      await validatePassword(body.password, passwordPolicyChecker);
-      return next();
-    },
     async (ctx, next) => {
       const profilePayload = ctx.guard.body;
-      const { signInExperience, interactionDetails, createLog } = ctx;
+      const { signInExperience, interactionDetails, createLog, passwordPolicyChecker } = ctx;
 
       const interactionStorage = getInteractionStorage(interactionDetails.result);
+      const { event, identifiers, profile } = interactionStorage;
+      const mergedProfile = { ...profile, ...profilePayload };
 
-      const profileLog = createLog(`Interaction.${interactionStorage.event}.Profile.Update`);
+      const profileLog = createLog(`Interaction.${event}.Profile.Update`);
 
       profileLog.append({ profile: profilePayload, interactionStorage, method: 'PATCH' });
 
-      if (interactionStorage.event !== InteractionEvent.ForgotPassword) {
+      if (event !== InteractionEvent.ForgotPassword) {
         verifyProfileSettings(profilePayload, signInExperience);
       }
 
+      await validatePassword(tenant, profilePayload.password, passwordPolicyChecker, {
+        identifiers,
+        // Merge with previous to provide a complete profile for validation
+        profile: mergedProfile,
+      });
+
       await storeInteractionResult(
         {
-          profile: {
+          profile: mergedProfile,
-            ...interactionStorage.profile,
-            ...profilePayload,
-          },
         },
         ctx,
         provider,
@@ -386,9 +389,8 @@ export default function interactionRoutes<T extends AnonymousRouter>(
       // Check interaction exists
       const { event } = getInteractionStorage(interactionDetails.result);
 
-      // This file needs refactor
-      // eslint-disable-next-line max-lines
       await sendVerificationCodeToIdentifier(
+        // eslint-disable-next-line max-lines -- TODO: refactor @simeng
         { event, ...guard.body },
         interactionDetails.jti,
         createLog,

packages/core/src/routes/interaction/utils/find-user-by-identifier.ts

@@ -2,6 +2,16 @@ import type TenantContext from '#src/tenants/TenantContext.js';
 
 import type { UserIdentity } from '../types/index.js';
 
+/**
+ * Find user by the given identifier in the following order:
+ *
+ * 1. Find user by username
+ * 2. Find user by email
+ * 3. Find user by phone
+ * 4. Find user by social identity
+ *
+ * @returns The found user or `null` if no user is found.
+ */
 export default async function findUserByIdentifier(
   { queries, connectors }: TenantContext,
   identity: UserIdentity

packages/core/src/routes/interaction/utils/validate-password.ts

@@ -1,21 +1,78 @@
-import { type PasswordPolicyChecker } from '@logto/core-kit';
+import { type PasswordPolicyChecker, type UserInfo } from '@logto/core-kit';
 import { type Optional } from '@silverhand/essentials';
 
 import RequestError from '#src/errors/RequestError/index.js';
+import type TenantContext from '#src/tenants/TenantContext.js';
+
+import { type AnonymousInteractionResult } from '../types/index.js';
+
+import findUserByIdentifier from './find-user-by-identifier.js';
+
+/**
+ * Fetch current user information from the interaction storage by checking `identifiers`
+ * and `profile` properties.
+ *
+ * Data in `profile` will override data in `identifiers`, if any. Because `profile` will
+ * overwrite the data in `identifiers` if this interaction is validated.
+ */
+const fetchUserInfo = async (
+  tenant: TenantContext,
+  { identifiers, profile }: Pick<AnonymousInteractionResult, 'identifiers' | 'profile'>
+): Promise<UserInfo> => {
+  const users = await Promise.all(
+    identifiers?.map(async (identifier) => {
+      switch (identifier.key) {
+        case 'emailVerified': {
+          return findUserByIdentifier(tenant, { email: identifier.value });
+        }
+        case 'phoneVerified': {
+          return findUserByIdentifier(tenant, { phone: identifier.value });
+        }
+        case 'social': {
+          return findUserByIdentifier(tenant, identifier);
+        }
+        case 'accountId': {
+          return tenant.queries.users.findUserById(identifier.value);
+        }
+        default: {
+          return null;
+        }
+      }
+    }) ?? []
+  );
+
+  // Use the first non-null user as the current user. Users with different account IDs
+  // should not be mixed in the same interaction, and it should be validated in some other
+  // places.
+  const user = users.find((user) => user !== null);
+
+  return {
+    username: user?.username ?? profile?.username,
+    email: user?.primaryEmail ?? profile?.email,
+    phoneNumber: user?.primaryPhone ?? profile?.phone,
+    name: user?.name ?? undefined,
+  };
+};
 
 /**
  * Validate password against the given password policy if the password is not undefined,
  * throw a {@link RequestError} if the password is invalid; otherwise, do nothing.
  */
 export const validatePassword = async (
+  tenant: TenantContext,
   password: Optional<string>,
-  checker: PasswordPolicyChecker
+  checker: PasswordPolicyChecker,
+  { identifiers, profile }: Pick<AnonymousInteractionResult, 'identifiers' | 'profile'>
 ) => {
   if (password === undefined) {
     return;
   }
 
-  const issues = await checker.check(password, {});
+  const issues = await checker.check(
+    password,
+    checker.policy.rejects.userInfo ? await fetchUserInfo(tenant, { identifiers, profile }) : {}
+  );
+
   if (issues.length > 0) {
     throw new RequestError('password.rejected', issues);
   }