AP-9274 # Fixed catering for users with no preferred mfa method

src/apps/mfa-service.ts

@@ -1,6 +1,9 @@
+import { MiscTypes } from '@oneblink/types'
+import { getMfaSettings } from './services/cognito'
+import { userMeetsMfaRequirement } from '../utils/mfa-requirement'
+
+export { getMfaSettings }
 export {
-  checkIsMfaEnabled,
-  getMfaSettings,
   updateUserPhoneNumber,
   removeUserPhoneNumber,
   verifyUserPhoneNumber,
@@ -11,11 +14,7 @@ export {
   generateMfaAuthenticatorAppQrCodeUrl,
   DEFAULT_MFA_SETTINGS,
 } from './services/cognito'
-export type {
-  MfaMethod,
-  MfaRequirementCheckResult,
-  MfaSettings,
-} from './services/cognito'
+export type { MfaMethod, MfaSettings } from './services/cognito'
 export {
   isMfaRequired,
   mfaRequirementToSelectedMethods,
@@ -27,3 +26,39 @@ export {
   formatMfaMethodNotAcceptedMessage,
 } from '../utils/mfa-requirement'
 export type { MfaRequirementMethod } from '../utils/mfa-requirement'
+
+export type MfaRequirementCheckResult = {
+  mfaSettings: Awaited<ReturnType<typeof getMfaSettings>>
+  userMeetsMfaRequirement: boolean
+}
+
+/**
+ * Check if the current user meets an MFA requirement.
+ *
+ * #### Example
+ *
+ * ```js
+ * const { mfaSettings, userMeetsMfaRequirement } =
+ *   await mfaService.checkIsMfaEnabled('any')
+ * if (userMeetsMfaRequirement) {
+ *   // User has met the MFA requirement
+ * } else {
+ *   // Prompt user to set up MFA
+ * }
+ * ```
+ *
+ * @returns
+ */
+export async function checkIsMfaEnabled(
+  mfaRequirement: MiscTypes.MfaRequirement | undefined,
+): Promise<MfaRequirementCheckResult> {
+  const mfaSettings = await getMfaSettings()
+
+  return {
+    mfaSettings,
+    userMeetsMfaRequirement: userMeetsMfaRequirement(
+      mfaRequirement,
+      mfaSettings,
+    ),
+  }
+}

src/apps/services/AWSCognitoClient.ts

@@ -17,7 +17,6 @@ import {
 } from '@aws-sdk/client-cognito-identity-provider'
 import Sentry from '../Sentry'
 import { OneBlinkAppsError } from '..'
-import { MiscTypes } from '@oneblink/types'
 
 export type MfaMethod = 'authenticator' | 'sms'
 
@@ -44,41 +43,38 @@ export const DEFAULT_MFA_SETTINGS: MfaSettings = {
   },
 }
 
-export type MfaRequirementCheckResult = {
-  mfaSettings: MfaSettings
-  userMeetsMfaRequirement: boolean
-}
-
-const MFA_REQUIREMENT_METHOD_CHECKS = {
-  sms: (mfaSettings: MfaSettings) => mfaSettings.sms.enabled,
-  authenticatorApp: (mfaSettings: MfaSettings) =>
-    mfaSettings.authenticator.enabled,
-} satisfies Record<
-  keyof MiscTypes.MfaRequirement,
-  (mfaSettings: MfaSettings) => boolean
->
+export function resolveMfaPreferredFlags({
+  authenticatorEnabled,
+  smsEnabled,
+  preferredMfaSetting,
+}: {
+  authenticatorEnabled: boolean
+  smsEnabled: boolean
+  preferredMfaSetting: string | undefined
+}): {
+  authenticatorPreferred: boolean
+  smsPreferred: boolean
+} {
+  const cognitoAuthenticatorPreferred =
+    preferredMfaSetting === 'SOFTWARE_TOKEN_MFA'
+  const cognitoSmsPreferred = preferredMfaSetting === 'SMS_MFA'
 
-function checkUserMeetsMfaRequirement(
-  mfaRequirement: MiscTypes.MfaRequirement | undefined,
-  mfaSettings: MfaSettings,
-): boolean {
-  if (!mfaRequirement) {
-    return true
+  if (cognitoAuthenticatorPreferred && authenticatorEnabled) {
+    return { authenticatorPreferred: true, smsPreferred: false }
   }
 
-  const requiredMethods = (
-    Object.keys(MFA_REQUIREMENT_METHOD_CHECKS) as Array<
-      keyof MiscTypes.MfaRequirement
-    >
-  ).filter((method) => mfaRequirement[method])
+  if (cognitoSmsPreferred && smsEnabled) {
+    return { authenticatorPreferred: false, smsPreferred: true }
+  }
 
-  if (requiredMethods.length === 0) {
-    return true
+  if (authenticatorEnabled && smsEnabled) {
+    return { authenticatorPreferred: true, smsPreferred: false }
   }
 
-  return requiredMethods.some((method) =>
-    MFA_REQUIREMENT_METHOD_CHECKS[method](mfaSettings),
-  )
+  return {
+    authenticatorPreferred: authenticatorEnabled,
+    smsPreferred: smsEnabled,
+  }
 }
 
 export type LoginAttemptResponse = {
@@ -592,34 +588,28 @@ export default class AWSCognitoClient {
         (attribute) => attribute.Name === 'phone_number_verified',
       )?.Value === 'true'
 
+    const authenticatorEnabled = mfaList.includes('SOFTWARE_TOKEN_MFA')
+    const smsEnabled = mfaList.includes('SMS_MFA')
+    const { authenticatorPreferred, smsPreferred } = resolveMfaPreferredFlags({
+      authenticatorEnabled,
+      smsEnabled,
+      preferredMfaSetting,
+    })
+
     return {
       authenticator: {
-        enabled: mfaList.includes('SOFTWARE_TOKEN_MFA'),
-        preferred: preferredMfaSetting === 'SOFTWARE_TOKEN_MFA',
+        enabled: authenticatorEnabled,
+        preferred: authenticatorPreferred,
       },
       sms: {
-        enabled: mfaList.includes('SMS_MFA'),
-        preferred: preferredMfaSetting === 'SMS_MFA',
+        enabled: smsEnabled,
+        preferred: smsPreferred,
         phoneNumber,
         isPhoneNumberVerified,
       },
     }
   }
 
-  async checkIsMfaEnabled(
-    mfaRequirement: MiscTypes.MfaRequirement | undefined,
-  ): Promise<MfaRequirementCheckResult> {
-    const mfaSettings = await this.getMfaSettings()
-
-    return {
-      mfaSettings,
-      userMeetsMfaRequirement: checkUserMeetsMfaRequirement(
-        mfaRequirement,
-        mfaSettings,
-      ),
-    }
-  }
-
   async updateUserPhoneNumber(
     phoneNumber: string,
   ): Promise<{ isPhoneNumberVerified: boolean }> {

src/apps/services/cognito.ts

@@ -4,7 +4,6 @@ import AWSCognitoClient, {
   DEFAULT_MFA_SETTINGS,
   LoginAttemptResponse,
   MfaMethod,
-  MfaRequirementCheckResult,
   MfaSettings,
 } from './AWSCognitoClient'
 
@@ -449,35 +448,6 @@ function generateMfaAuthenticatorAppQrCodeUrl(
   return `otpauth://totp/${tenants.current.productShortName}:${profile.email}?secret=${mfaAuthenticatorAppSetup.secretCode}&issuer=${tenants.current.productShortName}`
 }
 
-/**
- * Check if the current user meets an MFA requirement.
- *
- * #### Example
- *
- * ```js
- * const { mfaSettings, userMeetsMfaRequirement } =
- *   await mfaService.checkIsMfaEnabled('any')
- * if (userMeetsMfaRequirement) {
- *   // User has met the MFA requirement
- * } else {
- *   // Prompt user to set up MFA
- * }
- * ```
- *
- * @returns
- */
-async function checkIsMfaEnabled(
-  mfaRequirement: MiscTypes.MfaRequirement | undefined,
-): Promise<MfaRequirementCheckResult> {
-  if (!awsCognitoClient) {
-    throw new Error(
-      '"authService" has not been initiated. You must call the init() function before checking if the current user has MFA enabled.',
-    )
-  }
-
-  return await awsCognitoClient.checkIsMfaEnabled(mfaRequirement)
-}
-
 async function getMfaSettings(abortSignal?: AbortSignal) {
   if (!awsCognitoClient) {
     throw new Error(
@@ -591,7 +561,6 @@ export {
   getCognitoIdToken,
   getUserProfile,
   getUserFriendlyName,
-  checkIsMfaEnabled,
   getMfaSettings,
   updateUserPhoneNumber,
   removeUserPhoneNumber,
@@ -603,9 +572,4 @@ export {
   generateMfaAuthenticatorAppQrCodeUrl,
   DEFAULT_MFA_SETTINGS,
 }
-export type {
-  LoginAttemptResponse,
-  MfaMethod,
-  MfaRequirementCheckResult,
-  MfaSettings,
-}
+export type { LoginAttemptResponse, MfaMethod, MfaSettings }

src/utils/mfa-requirement.ts

@@ -84,17 +84,6 @@ export function formatMfaRequirementMethodLabel(
   }
 }
 
-function isMfaRequirementMethodEnabled(
-  method: MfaRequirementMethod,
-  mfaSettings: MfaSettings,
-): boolean {
-  if (method === 'sms') {
-    return mfaSettings.sms.preferred
-  }
-
-  return mfaSettings.authenticator.preferred
-}
-
 export function userMeetsMfaRequirement(
   mfaRequirement: MiscTypes.MfaRequirement | undefined,
   mfaSettings: MfaSettings,
@@ -103,9 +92,23 @@ export function userMeetsMfaRequirement(
     return true
   }
 
-  return mfaRequirementToSelectedMethods(mfaRequirement).some((method) =>
-    isMfaRequirementMethodEnabled(method, mfaSettings),
-  )
+  if (
+    mfaRequirement?.sms &&
+    mfaSettings.sms.enabled &&
+    mfaSettings.sms.preferred
+  ) {
+    return true
+  }
+
+  if (
+    mfaRequirement?.authenticatorApp &&
+    mfaSettings.authenticator.enabled &&
+    mfaSettings.authenticator.preferred
+  ) {
+    return true
+  }
+
+  return false
 }
 
 export function formatMfaMethodNotAcceptedMessage(

tests/apps/services/AWSCognitoClient.test.ts

@@ -0,0 +1,67 @@
+import { describe, expect, test } from 'vitest'
+import { resolveMfaPreferredFlags } from '../../../src/apps/services/AWSCognitoClient'
+
+describe('resolveMfaPreferredFlags', () => {
+  test('returns no preferred methods when none are enabled', () => {
+    expect(
+      resolveMfaPreferredFlags({
+        authenticatorEnabled: false,
+        smsEnabled: false,
+        preferredMfaSetting: undefined,
+      }),
+    ).toEqual({
+      authenticatorPreferred: false,
+      smsPreferred: false,
+    })
+  })
+
+  test('uses Cognito preferred setting when it matches an enabled method', () => {
+    expect(
+      resolveMfaPreferredFlags({
+        authenticatorEnabled: true,
+        smsEnabled: true,
+        preferredMfaSetting: 'SMS_MFA',
+      }),
+    ).toEqual({
+      authenticatorPreferred: false,
+      smsPreferred: true,
+    })
+  })
+
+  test('prefers the only enabled method when Cognito has no preferred setting', () => {
+    expect(
+      resolveMfaPreferredFlags({
+        authenticatorEnabled: false,
+        smsEnabled: true,
+        preferredMfaSetting: undefined,
+      }),
+    ).toEqual({
+      authenticatorPreferred: false,
+      smsPreferred: true,
+    })
+
+    expect(
+      resolveMfaPreferredFlags({
+        authenticatorEnabled: true,
+        smsEnabled: false,
+        preferredMfaSetting: undefined,
+      }),
+    ).toEqual({
+      authenticatorPreferred: true,
+      smsPreferred: false,
+    })
+  })
+
+  test('prefers authenticator when multiple methods are enabled without a preferred setting', () => {
+    expect(
+      resolveMfaPreferredFlags({
+        authenticatorEnabled: true,
+        smsEnabled: true,
+        preferredMfaSetting: undefined,
+      }),
+    ).toEqual({
+      authenticatorPreferred: true,
+      smsPreferred: false,
+    })
+  })
+})