feat(core): add organizationRequiredMfaPolicy guard (#7108)

add organizationRequiredMfaPolicy guard in experience API

Author: simeng-li

packages/core/src/routes/experience/classes/mfa.ts

@@ -13,6 +13,8 @@ import {
   MfaPolicy,
   type User,
   VerificationType,
+  type Mfa as MfaSettings,
+  OrganizationRequiredMfaPolicy,
 } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
 import { deduplicate } from '@silverhand/essentials';
@@ -24,6 +26,7 @@ import type Libraries from '#src/tenants/Libraries.js';
 import type Queries from '#src/tenants/Queries.js';
 import assertThat from '#src/utils/assert-that.js';
 
+import { EnvSet } from '../../../env-set/index.js';
 import { type InteractionContext } from '../types.js';
 
 import { SignInExperienceValidator } from './libraries/sign-in-experience-validator.js';
@@ -150,10 +153,13 @@ export class Mfa {
    * @throws {RequestError} with status 422 if the MFA policy is not user controlled
    */
   async skip() {
-    const { policy } = await this.signInExperienceValidator.getMfaSettings();
+    const mfaSettings = await this.signInExperienceValidator.getMfaSettings();
+    const { policy } = mfaSettings;
+    const user = await this.interactionContext.getIdentifiedUser();
 
     assertThat(
-      policy !== MfaPolicy.Mandatory,
+      policy !== MfaPolicy.Mandatory &&
+        !(await this.isMfaRequiredByUserOrganizations(mfaSettings, user.id)),
       new RequestError({
         code: 'session.mfa.mfa_policy_not_user_controlled',
         status: 422,
@@ -269,23 +275,39 @@ export class Mfa {
    * @throws {RequestError} with status 422 if the user has not bound the backup code but enabled in the sign-in experience
    * @throws {RequestError} with status 422 if the user existing backup codes is empty, new backup codes is required
    */
+  // eslint-disable-next-line complexity
   async assertUserMandatoryMfaFulfilled() {
-    const { factors, policy } = await this.signInExperienceValidator.getMfaSettings();
+    const mfaSettings = await this.signInExperienceValidator.getMfaSettings();
+    const { policy, factors } = mfaSettings;
 
     // If there are no factors, then there is nothing to check
     if (factors.length === 0) {
       return;
     }
 
-    const { mfaVerifications, logtoConfig } = await this.interactionContext.getIdentifiedUser();
+    const {
+      mfaVerifications,
+      logtoConfig,
+      id: userId,
+    } = await this.interactionContext.getIdentifiedUser();
 
-    // If the policy is no prompt, then there is nothing to check
-    if (policy === MfaPolicy.NoPrompt) {
+    const isMfaRequiredByUserOrganizations = await this.isMfaRequiredByUserOrganizations(
+      mfaSettings,
+      userId
+    );
+
+    // If the policy is no prompt, and mfa is not required by the user organizations, then there is nothing to check
+    if (policy === MfaPolicy.NoPrompt && !isMfaRequiredByUserOrganizations) {
       return;
     }
 
-    // If the policy is not mandatory and the user has skipped MFA, then there is nothing to check
-    if (policy !== MfaPolicy.Mandatory && (this.#mfaSkipped ?? isMfaSkipped(logtoConfig))) {
+    // If the policy is not mandatory and the user has skipped MFA,
+    // and MFA is not required by the user organizations, then there is nothing to check
+    if (
+      policy !== MfaPolicy.Mandatory &&
+      (this.#mfaSkipped ?? isMfaSkipped(logtoConfig)) &&
+      !isMfaRequiredByUserOrganizations
+    ) {
       return;
     }
 
@@ -308,7 +330,7 @@ export class Mfa {
       availableFactors.some((factor) => linkedFactors.includes(factor)),
       new RequestError(
         { code: 'user.missing_mfa', status: 422 },
-        policy === MfaPolicy.Mandatory
+        policy === MfaPolicy.Mandatory || isMfaRequiredByUserOrganizations
           ? { availableFactors }
           : { availableFactors, skippable: true }
       )
@@ -340,4 +362,21 @@ export class Mfa {
 
     assertThat(isFactorsEnabled, new RequestError({ code: 'session.mfa.mfa_factor_not_enabled' }));
   }
+
+  private async isMfaRequiredByUserOrganizations(mfaSettings: MfaSettings, userId: string) {
+    // TODO: Remove this check when the feature is enabled for production
+    if (!EnvSet.values.isDevFeaturesEnabled) {
+      return false;
+    }
+
+    if (mfaSettings.organizationRequiredMfaPolicy !== OrganizationRequiredMfaPolicy.Mandatory) {
+      return false;
+    }
+
+    const organizations = await this.queries.organizations.relations.users.getOrganizationsByUserId(
+      userId
+    );
+
+    return organizations.some(({ isMfaRequired }) => isMfaRequired);
+  }
 }

packages/core/src/routes/experience/profile-routes.ts

@@ -19,6 +19,8 @@ import { experienceRoutes } from './const.js';
 import { type ExperienceInteractionRouterContext } from './types.js';
 
 /**
+ * This middleware is guards the current interaction status is MFA verified (if MFA is enabled)
+ *
  * @throws {RequestError} with status 400 if current interaction is ForgotPassword
  * @throws {RequestError} with status 404 if current interaction is not identified
  * @throws {RequestError} with status 403 if MFA verification status is not verified
@@ -40,7 +42,6 @@ function verifiedInteractionGuard<
       })
     );
 
-    // Guard MFA verification status
     await experienceInteraction.guardMfaVerificationStatus();
 
     return next();
@@ -73,7 +74,7 @@ export default function interactionProfileRoutes<T extends ExperienceInteraction
         })
       );
 
-      // Guard MFA verification status for SignIn interaction only
+      //  User profile updates require MFA verification (if MFA is enabled) during the sign-in event.
       if (interactionEvent === InteractionEvent.SignIn) {
         await experienceInteraction.guardMfaVerificationStatus();
       }

packages/integration-tests/src/helpers/index.ts

@@ -86,6 +86,7 @@ type ExpectedErrorInfo = {
   code: string;
   status: number;
   messageIncludes?: string;
+  unexpectedProperties?: string[];
 };
 
 export const expectRejects = async <T = void>(
@@ -102,7 +103,7 @@ export const expectRejects = async <T = void>(
 };
 
 const expectRequestError = async <T = void>(error: unknown, expected: ExpectedErrorInfo) => {
-  const { code, status, messageIncludes } = expected;
+  const { code, status, messageIncludes, unexpectedProperties = [] } = expected;
 
   if (!(error instanceof HTTPError)) {
     fail('Error should be an instance of RequestError');
@@ -124,6 +125,10 @@ const expectRequestError = async <T = void>(error: unknown, expected: ExpectedEr
     expect(body.message.includes(messageIncludes)).toBeTruthy();
   }
 
+  for (const property of unexpectedProperties) {
+    expect(body.data).not.toHaveProperty(property);
+  }
+
   return body.data;
 };