diff --git a/src/apps/mfa-service.ts b/src/apps/mfa-service.ts index ad611358..3bdef2de 100644 --- a/src/apps/mfa-service.ts +++ b/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> + 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 { + const mfaSettings = await getMfaSettings() + + return { + mfaSettings, + userMeetsMfaRequirement: userMeetsMfaRequirement( + mfaRequirement, + mfaSettings, + ), + } +} diff --git a/src/apps/services/AWSCognitoClient.ts b/src/apps/services/AWSCognitoClient.ts index 5ff2dc52..0f3c3468 100644 --- a/src/apps/services/AWSCognitoClient.ts +++ b/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 { - const mfaSettings = await this.getMfaSettings() - - return { - mfaSettings, - userMeetsMfaRequirement: checkUserMeetsMfaRequirement( - mfaRequirement, - mfaSettings, - ), - } - } - async updateUserPhoneNumber( phoneNumber: string, ): Promise<{ isPhoneNumberVerified: boolean }> { diff --git a/src/apps/services/cognito.ts b/src/apps/services/cognito.ts index 92811da0..4d443836 100644 --- a/src/apps/services/cognito.ts +++ b/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 { - 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 } diff --git a/src/utils/mfa-requirement.ts b/src/utils/mfa-requirement.ts index 2b15fe7e..25b5a1c7 100644 --- a/src/utils/mfa-requirement.ts +++ b/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( diff --git a/tests/apps/services/AWSCognitoClient.test.ts b/tests/apps/services/AWSCognitoClient.test.ts new file mode 100644 index 00000000..13800bc3 --- /dev/null +++ b/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, + }) + }) +})