fix: Roles based mandatory two factor to check all cases (#37338)

Author: yash-rajpal

.changeset/tall-flies-jump.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Improves mandatory role-based two-factor authentication setup to always verify available 2FA methods before enforcement.

apps/meteor/client/views/hooks/useRequire2faSetup.ts

@@ -5,14 +5,26 @@ import { Roles } from '../../stores';
 export const useRequire2faSetup = () => {
 	const user = useUser();
 	const tfaEnabled = useSetting('Accounts_TwoFactorAuthentication_Enabled', false);
+	const email2faEnabled = useSetting('Accounts_TwoFactorAuthentication_By_Email_Enabled', false);
+	const totp2faEnabled = useSetting('Accounts_TwoFactorAuthentication_By_TOTP_Enabled', false);
+	const is2FAEnabled = tfaEnabled && (email2faEnabled || totp2faEnabled);
 
 	return Roles.use((state) => {
-		// User is already using 2fa
-		if (!user || user?.services?.totp?.enabled || user?.services?.email2fa?.enabled) {
+		if (!user || !is2FAEnabled) {
 			return false;
 		}
 
 		const mandatoryRole = state.find((role) => !!role.mandatory2fa && user.roles?.includes(role._id));
-		return mandatoryRole !== undefined && tfaEnabled;
+
+		if (mandatoryRole === undefined) {
+			return false;
+		}
+
+		const hasEmail2FA = !!user?.services?.email2fa?.enabled;
+		const hasTotp2FA = !!user?.services?.totp?.enabled;
+
+		const hasAnyEnabled2FA = (email2faEnabled && hasEmail2FA) || (totp2faEnabled && hasTotp2FA);
+
+		return !hasAnyEnabled2FA;
 	});
 };

apps/meteor/tests/e2e/enforce-2FA.spec.ts

@@ -2,6 +2,7 @@ import { IS_EE } from './config/constants';
 import { Users } from './fixtures/userStates';
 import { HomeChannel, AccountProfile } from './page-objects';
 import { createCustomRole, deleteCustomRole } from './utils/custom-role';
+import { setSettingValueById } from './utils/setSettingValueById';
 import { test, expect } from './utils/test';
 
 test.use({ storageState: Users.admin.state });
@@ -62,4 +63,44 @@ test.describe('enforce two factor authentication', () => {
 		await expect(poHomeChannel.sidenav.sidebarHomeAction).toBeVisible();
 		await expect(poAccountProfile.securityHeader).not.toBeVisible();
 	});
+
+	test.describe('should still redirect to 2FA setup page when email 2FA is disabled', () => {
+		test.beforeAll(async ({ api }) => {
+			await setSettingValueById(api, 'Accounts_TwoFactorAuthentication_By_Email_Enabled', false);
+		});
+
+		test.afterAll(async ({ api }) => {
+			await setSettingValueById(api, 'Accounts_TwoFactorAuthentication_By_Email_Enabled', true);
+		});
+
+		test('should redirect to 2FA setup page and show totp 2FA setup', async ({ page }) => {
+			await page.goto('/home');
+			await poAccountProfile.required2faModalSetUpButton.click();
+			await expect(poHomeChannel.sidenav.sidebarHomeAction).not.toBeVisible();
+
+			await expect(poAccountProfile.securityHeader).toBeVisible();
+
+			await expect(poAccountProfile.security2FASection).toHaveAttribute('aria-expanded', 'true');
+			await expect(poAccountProfile.totp2FASwitch).toBeVisible();
+			await expect(poAccountProfile.email2FASwitch).not.toBeVisible();
+		});
+	});
+
+	test.describe('should not redirect to 2FA setup page when both email and totp 2FA are disabled', () => {
+		test.beforeAll(async ({ api }) => {
+			await setSettingValueById(api, 'Accounts_TwoFactorAuthentication_By_Email_Enabled', false);
+			await setSettingValueById(api, 'Accounts_TwoFactorAuthentication_By_TOTP_Enabled', false);
+		});
+
+		test.afterAll(async ({ api }) => {
+			await setSettingValueById(api, 'Accounts_TwoFactorAuthentication_By_Email_Enabled', true);
+			await setSettingValueById(api, 'Accounts_TwoFactorAuthentication_By_TOTP_Enabled', true);
+		});
+
+		test('should not redirect to 2FA setup page', async ({ page }) => {
+			await page.goto('/home');
+			await expect(poHomeChannel.sidenav.sidebarHomeAction).toBeVisible();
+			await expect(poAccountProfile.securityHeader).not.toBeVisible();
+		});
+	});
 });