From 15ab4d587e1bd4c8b4e85846041950983fc9837d Mon Sep 17 00:00:00 2001
From: wangsijie <wangsijie@silverhand.io>
Date: Tue, 17 Oct 2023 14:18:53 +0800
Subject: [PATCH] fix(core): validate mandatory mfa (#4639)

---
 .../verifications/mfa-verification.test.ts       | 16 +++++++++++-----
 .../verifications/mfa-verification.ts            | 14 +++++++-------
 .../hooks/use-mfa-verification-error-handler.ts  | 12 ++++++------
 packages/experience/src/types/guard.ts           |  2 +-
 4 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/packages/core/src/routes/interaction/verifications/mfa-verification.test.ts b/packages/core/src/routes/interaction/verifications/mfa-verification.test.ts
index eb9ae4e22..a7a430392 100644
--- a/packages/core/src/routes/interaction/verifications/mfa-verification.test.ts
+++ b/packages/core/src/routes/interaction/verifications/mfa-verification.test.ts
@@ -51,7 +51,7 @@ const mfaRequiredCtx = {
   signInExperience: {
     ...mockSignInExperience,
     mfa: {
-      factors: [MfaFactor.TOTP],
+      factors: [MfaFactor.TOTP, MfaFactor.WebAuthn],
       policy: MfaPolicy.Mandatory,
     },
   },
@@ -79,8 +79,11 @@ describe('validateMandatoryBindMfa', () => {
         validateMandatoryBindMfa(tenantContext, mfaRequiredCtx, interaction)
       ).rejects.toMatchError(
         new RequestError(
-          { code: 'user.missing_mfa', status: 422 },
-          { missingFactors: [MfaFactor.TOTP] }
+          {
+            code: 'user.missing_mfa',
+            status: 422,
+          },
+          { availableFactors: [MfaFactor.TOTP, MfaFactor.WebAuthn] }
         )
       );
     });
@@ -111,8 +114,11 @@ describe('validateMandatoryBindMfa', () => {
         validateMandatoryBindMfa(tenantContext, mfaRequiredCtx, signInInteraction)
       ).rejects.toMatchError(
         new RequestError(
-          { code: 'user.missing_mfa', status: 422 },
-          { missingFactors: [MfaFactor.TOTP] }
+          {
+            code: 'user.missing_mfa',
+            status: 422,
+          },
+          { availableFactors: [MfaFactor.TOTP, MfaFactor.WebAuthn] }
         )
       );
     });
diff --git a/packages/core/src/routes/interaction/verifications/mfa-verification.ts b/packages/core/src/routes/interaction/verifications/mfa-verification.ts
index d748705eb..aed81145e 100644
--- a/packages/core/src/routes/interaction/verifications/mfa-verification.ts
+++ b/packages/core/src/routes/interaction/verifications/mfa-verification.ts
@@ -83,15 +83,14 @@ export const validateMandatoryBindMfa = async (
   }
 
   if (event === InteractionEvent.Register) {
-    const missingFactors = factors.filter((factor) => factor !== bindMfa?.type);
     assertThat(
-      missingFactors.length === 0,
+      bindMfa && factors.includes(bindMfa.type),
       new RequestError(
         {
           code: 'user.missing_mfa',
           status: 422,
         },
-        { missingFactors }
+        { availableFactors: factors.map((factor) => factor) }
       )
     );
   }
@@ -99,17 +98,18 @@ export const validateMandatoryBindMfa = async (
   if (event === InteractionEvent.SignIn) {
     const { accountId } = interaction;
     const { mfaVerifications } = await tenant.queries.users.findUserById(accountId);
-    const missingFactors = factors.filter(
-      (factor) => factor !== bindMfa?.type && !mfaVerifications.some(({ type }) => type === factor)
+    const hasFactorInBind = Boolean(bindMfa && factors.includes(bindMfa.type));
+    const hasFactorInUser = factors.some((factor) =>
+      mfaVerifications.some(({ type }) => type === factor)
     );
     assertThat(
-      missingFactors.length === 0,
+      hasFactorInBind || hasFactorInUser,
       new RequestError(
         {
           code: 'user.missing_mfa',
           status: 422,
         },
-        { missingFactors }
+        { availableFactors: factors.map((factor) => factor) }
       )
     );
   }
diff --git a/packages/experience/src/hooks/use-mfa-verification-error-handler.ts b/packages/experience/src/hooks/use-mfa-verification-error-handler.ts
index 459a4af7e..fdb0186fb 100644
--- a/packages/experience/src/hooks/use-mfa-verification-error-handler.ts
+++ b/packages/experience/src/hooks/use-mfa-verification-error-handler.ts
@@ -27,23 +27,23 @@ const useMfaVerificationErrorHandler = ({ replace }: Options = {}) => {
     () => ({
       'user.missing_mfa': (error) => {
         const [_, data] = validate(error.data, missingMfaFactorsErrorDataGuard);
-        const missingFactors = data?.missingFactors ?? [];
+        const availableFactors = data?.availableFactors ?? [];
 
-        if (missingFactors.length === 0) {
+        if (availableFactors.length === 0) {
           setToast(error.message);
           return;
         }
 
-        if (missingFactors.length > 1) {
-          const state: MfaFactorsState = { availableFactors: missingFactors };
+        if (availableFactors.length > 1) {
+          const state: MfaFactorsState = { availableFactors };
           navigate({ pathname: `/${UserMfaFlow.MfaBinding}` }, { replace, state });
           return;
         }
 
-        const factor = missingFactors[0];
+        const factor = availableFactors[0];
 
         if (factor === MfaFactor.TOTP) {
-          void startTotpBinding(missingFactors);
+          void startTotpBinding(availableFactors);
         }
         // Todo: @xiaoyijun handle other factors
       },
diff --git a/packages/experience/src/types/guard.ts b/packages/experience/src/types/guard.ts
index 007fbc3ef..8e67c839e 100644
--- a/packages/experience/src/types/guard.ts
+++ b/packages/experience/src/types/guard.ts
@@ -72,7 +72,7 @@ const mfaFactorsGuard = s.array(
 );
 
 export const missingMfaFactorsErrorDataGuard = s.object({
-  missingFactors: mfaFactorsGuard,
+  availableFactors: mfaFactorsGuard,
 });
 
 export const requireMfaFactorsErrorDataGuard = s.object({