fix(core): should not be able to skip mandatory MFA (#6892)

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

@@ -285,7 +285,7 @@ export class Mfa {
     }
 
     // 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 (policy !== MfaPolicy.Mandatory && (this.#mfaSkipped ?? isMfaSkipped(logtoConfig))) {
       return;
     }
 

packages/core/src/routes/interaction/verifications/mfa-verification.ts

@@ -153,7 +153,7 @@ export const validateMandatoryBindMfa = async (
     return interaction;
   }
 
-  // If the policy is not mandatory and the user has skipped MFA, skip check
+  // If the policy is not mandatory and the user has skipped MFA (in the current interaction), skip check
   const { mfaSkipped } = interaction;
   if (policy !== MfaPolicy.Mandatory && mfaSkipped) {
     return interaction;
@@ -177,7 +177,8 @@ export const validateMandatoryBindMfa = async (
     const { accountId } = interaction;
     const { mfaVerifications, logtoConfig } = await tenant.queries.users.findUserById(accountId);
 
-    if (isMfaSkipped(logtoConfig)) {
+    // If the policy is not mandatory and the user has skipped MFA (not in the current interaction), skip check
+    if (policy !== MfaPolicy.Mandatory && isMfaSkipped(logtoConfig)) {
       return interaction;
     }
 

packages/integration-tests/src/tests/api/experience-api/bind-mfa/happpy-path.test.ts

@@ -312,6 +312,35 @@ describe('Bind MFA APIs happy path', () => {
       await deleteUser(userId);
     });
 
+    it('should reject if user has not binded MFA even if it is skipped', async () => {
+      const { username, password } = generateNewUserProfile({ username: true, password: true });
+      await userApi.create({ username, password });
+
+      // Set to skipable policy first to update the user's mfa skipped state
+      await enableUserControlledMfaWithTotp();
+      const client = await initExperienceClient();
+      await identifyUserWithUsernamePassword(client, username, password);
+      await expectRejects(client.submitInteraction(), {
+        code: 'user.missing_mfa',
+        status: 422,
+      });
+      await client.skipMfaBinding();
+      const { redirectTo } = await client.submitInteraction();
+      const userId = await processSession(client, redirectTo);
+      await logoutClient(client);
+
+      // Set to mandatory policy to check the user's mfa skipped state
+      await enableMandatoryMfaWithTotpAndBackupCode();
+      const client2 = await initExperienceClient();
+      await identifyUserWithUsernamePassword(client2, username, password);
+      await expectRejects(client2.submitInteraction(), {
+        code: 'user.missing_mfa',
+        status: 422,
+      });
+
+      await deleteUser(userId);
+    });
+
     it('should bind backup codes on sign-in', async () => {
       const { username, password } = generateNewUserProfile({ username: true, password: true });
       const user = await userApi.create({ username, password });

packages/integration-tests/src/tests/api/interaction/mfa/totp.test.ts

@@ -157,6 +157,43 @@ describe('sign in and fulfill mfa (mandatory TOTP)', () => {
     await deleteUser(user.id);
   });
 
+  it('should fail with missing_mfa error even if mfa is skipped', async () => {
+    // Set to skipable policy first to update the user's mfa skipped state
+    await enableUserControlledMfaWithTotp();
+    const { userProfile, user } = await generateNewUser({ username: true, password: true });
+    const client = await initClient();
+    await client.successSend(putInteraction, {
+      event: InteractionEvent.SignIn,
+      identifier: {
+        username: userProfile.username,
+        password: userProfile.password,
+      },
+    });
+    await expectRejects(client.submitInteraction(), {
+      code: 'user.missing_mfa',
+      status: 422,
+    });
+    await client.successSend(skipMfaBinding);
+    await client.submitInteraction();
+
+    // Set to mandatory policy to check the user's mfa skipped state
+    await enableMandatoryMfaWithTotp();
+    const client2 = await initClient();
+    await client2.successSend(putInteraction, {
+      event: InteractionEvent.SignIn,
+      identifier: {
+        username: userProfile.username,
+        password: userProfile.password,
+      },
+    });
+    await expectRejects(client2.submitInteraction(), {
+      code: 'user.missing_mfa',
+      status: 422,
+    });
+
+    await deleteUser(user.id);
+  });
+
   it('should sign in and fulfill totp', async () => {
     const { id } = await registerWithMfa();
     await deleteUser(id);