fix(core): prompt to bind mfa for register (#4783)

packages/core/src/routes/interaction/actions/submit-interaction.ts

@@ -30,6 +30,7 @@ import type {
   VerifiedRegisterInteractionResult,
 } from '../types/index.js';
 import { clearInteractionStorage } from '../utils/interaction.js';
+import { userMfaDataKey } from '../verifications/mfa-verification.js';
 
 import { postAffiliateLogs, parseUserProfile } from './helpers.js';
 
@@ -96,7 +97,7 @@ export default async function submitInteraction(
   const { event, profile } = interaction;
 
   if (event === InteractionEvent.Register) {
-    const { pendingAccountId } = interaction;
+    const { pendingAccountId, mfaSkipped } = interaction;
     const id = pendingAccountId ?? (await generateUserId());
     const userProfile = await parseUserProfile(tenantContext, interaction);
     const mfaVerifications = parseBindMfas(interaction);
@@ -119,6 +120,13 @@ export default async function submitInteraction(
             mfaVerifications,
           }
         ),
+        ...conditional(
+          mfaSkipped && {
+            [userMfaDataKey]: {
+              skipped: true,
+            },
+          }
+        ),
       },
       getInitialUserRoles(isInAdminTenant, isCreatingFirstAdminUser, isCloud)
     );

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

@@ -59,6 +59,8 @@ export const anonymousInteractionResultGuard = z.object({
   // The user id to be used for register, if not provided, a new one will be generated
   // WebAuthn requires a user id to be provided, so we have to generate and know it before submit interaction
   pendingAccountId: z.string().optional(),
+  // The marks that the user has skip binding new MFA (for new users, they don't have database records yet)
+  mfaSkipped: z.boolean().optional(),
 });
 
 export const forgotPasswordProfileGuard = z.object({

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

@@ -80,6 +80,7 @@ export type VerifiedRegisterInteractionResult = {
   identifiers?: Identifier[];
   bindMfas?: BindMfa[];
   pendingAccountId?: string;
+  mfaSkipped?: boolean;
 };
 
 export type VerifiedSignInInteractionResult = {

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

@@ -12,7 +12,6 @@ import {
   mockUserWithMfaVerifications,
 } from '#src/__mocks__/user.js';
 import RequestError from '#src/errors/RequestError/index.js';
-import { createMockProvider } from '#src/test-utils/oidc-provider.js';
 import { MockTenant } from '#src/test-utils/tenant.js';
 import { createContextWithRouteParameters } from '#src/utils/test-utils.js';
 
@@ -38,6 +37,9 @@ const mockBackupCodes = ['foo'];
 await mockEsmWithActual('../utils/backup-code-validation.js', () => ({
   generateBackupCodes: jest.fn().mockReturnValue(mockBackupCodes),
 }));
+const { storeInteractionResult } = await mockEsmWithActual('../utils/interaction.js', () => ({
+  storeInteractionResult: jest.fn(),
+}));
 
 const { validateMandatoryBindMfa, verifyBindMfa, verifyMfa } = await import(
   './mfa-verification.js'
@@ -104,8 +106,6 @@ const signInInteraction: AccountVerifiedInteractionResult = {
   accountId: 'foo',
 };
 
-const provider = createMockProvider();
-
 describe('validateMandatoryBindMfa', () => {
   afterEach(() => {
     findUserById.mockReset();
@@ -140,9 +140,32 @@ describe('validateMandatoryBindMfa', () => {
       ).resolves.not.toThrow();
     });
 
-    it('bindMfa missing and not required should pass', async () => {
+    it('bindMfa missing and not required should throw (for skip)', async () => {
       await expect(
         validateMandatoryBindMfa(tenantContext, baseCtx, interaction)
+      ).rejects.toMatchError(
+        new RequestError(
+          {
+            code: 'user.missing_mfa',
+            status: 422,
+          },
+          { availableFactors: [MfaFactor.TOTP], skippable: true }
+        )
+      );
+      expect(storeInteractionResult).toHaveBeenCalledWith(
+        { mfaSkipped: true },
+        expect.anything(),
+        expect.anything(),
+        expect.anything()
+      );
+    });
+
+    it('bindMfa missing and not required, marked as skipped should pass', async () => {
+      await expect(
+        validateMandatoryBindMfa(tenantContext, baseCtx, {
+          ...interaction,
+          mfaSkipped: true,
+        })
       ).resolves.not.toThrow();
     });
   });

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

@@ -82,7 +82,7 @@ export const verifyMfa = async (
   return interaction;
 };
 
-const userMfaDataKey = 'mfa';
+export const userMfaDataKey = 'mfa';
 /**
  * Check if the user has skipped MFA binding
  */
@@ -170,7 +170,7 @@ const validateMandatoryBindMfaForSignIn = async (
 
 export const validateMandatoryBindMfa = async (
   tenant: TenantContext,
-  ctx: WithInteractionSieContext & WithInteractionDetailsContext,
+  ctx: Context & WithInteractionSieContext & WithInteractionDetailsContext,
   interaction: VerifiedSignInInteractionResult | VerifiedRegisterInteractionResult
 ): Promise<VerifiedInteractionResult> => {
   const {
@@ -190,27 +190,45 @@ export const validateMandatoryBindMfa = async (
   );
 
   if (event === InteractionEvent.Register) {
-    if (policy !== MfaPolicy.Mandatory) {
+    if (hasFactorInBind) {
       return interaction;
     }
 
-    assertThat(
-      hasFactorInBind,
-      new RequestError(
+    if (policy === MfaPolicy.Mandatory) {
+      throw new RequestError(
         {
           code: 'user.missing_mfa',
           status: 422,
         },
         { availableFactors }
-      )
+      );
+    }
+
+    const { mfaSkipped } = interaction;
+    if (mfaSkipped) {
+      return interaction;
+    }
+
+    // Auto mark MFA skipped for new users, will change to manual mark in the future
+    await storeInteractionResult(
+      {
+        mfaSkipped: true,
+      },
+      ctx,
+      tenant.provider,
+      true
+    );
+
+    throw new RequestError(
+      {
+        code: 'user.missing_mfa',
+        status: 422,
+      },
+      { availableFactors, skippable: true }
     );
   }
 
-  if (event === InteractionEvent.SignIn) {
-    return validateMandatoryBindMfaForSignIn(tenant, ctx, interaction);
-  }
-
-  return interaction;
+  return validateMandatoryBindMfaForSignIn(tenant, ctx, interaction);
 };
 
 /**