feat: improve UX for logged users with mandatory 2FA roles

.changeset/ten-schools-collect.md

@@ -0,0 +1,6 @@
+---
+"@rocket.chat/meteor": patch
+"@rocket.chat/i18n": patch
+---
+
+Improves UX for users with mandatory 2FA roles by clarifying required actions

apps/meteor/client/components/TextCopy.tsx

@@ -31,7 +31,7 @@ const TextCopy = ({ text, wrapper = defaultWrapperRenderer, ...props }: TextCopy
 			justifyContent='stretch'
 			alignItems='flex-start'
 			flexGrow={1}
-			padding={16}
+			pb={16}
 			backgroundColor='surface'
 			width='full'
 			{...props}

apps/meteor/client/views/account/security/AccountSecurityPage.tsx

@@ -1,4 +1,4 @@
-import { Box, Accordion, AccordionItem, ButtonGroup, Button } from '@rocket.chat/fuselage';
+import { Box, Accordion, AccordionItem, ButtonGroup, Button, Callout } from '@rocket.chat/fuselage';
 import { useSetting, useTranslation, useUser } from '@rocket.chat/ui-contexts';
 import { useId } from 'react';
 import type { ReactElement } from 'react';
@@ -9,6 +9,7 @@ import EndToEnd from './EndToEnd';
 import TwoFactorEmail from './TwoFactorEmail';
 import TwoFactorTOTP from './TwoFactorTOTP';
 import { Page, PageHeader, PageScrollableContentWithShadow, PageFooter } from '../../../components/Page';
+import { useRequire2faSetup } from '../../hooks/useRequire2faSetup';
 
 const passwordDefaultValues = { password: '', confirmationPassword: '' };
 
@@ -38,6 +39,8 @@ const AccountSecurityPage = (): ReactElement => {
 
 	const passwordFormId = useId();
 
+	const require2faSetup = useRequire2faSetup();
+
 	return (
 		<Page>
 			<PageHeader title={t('Security')} />
@@ -46,15 +49,20 @@ const AccountSecurityPage = (): ReactElement => {
 					{allowPasswordChange && (
 						<FormProvider {...methods}>
 							<Accordion>
-								<AccordionItem title={t('Password')} defaultExpanded>
+								<AccordionItem title={t('Password')} expanded={!require2faSetup}>
 									<ChangePassword id={passwordFormId} />
 								</AccordionItem>
 							</Accordion>
 						</FormProvider>
 					)}
 					<Accordion>
 						{(twoFactorTOTP || showEmailTwoFactor) && twoFactorEnabled && (
-							<AccordionItem title={t('Two Factor Authentication')}>
+							<AccordionItem expanded={require2faSetup} title={t('Two Factor Authentication')}>
+								{require2faSetup && (
+									<Callout type='warning' title={t('Enable_two-factor_authentication')} mbe='24px'>
+										{t('Enable_two-factor_authentication_callout_description')}
+									</Callout>
+								)}
 								{twoFactorTOTP && <TwoFactorTOTP />}
 								{showEmailTwoFactor && <TwoFactorEmail />}
 							</AccordionItem>

apps/meteor/client/views/account/security/TwoFactorEmail.tsx

@@ -1,14 +1,15 @@
-import { Box, Button, Margins } from '@rocket.chat/fuselage';
+import { Box, Field, FieldLabel, FieldRow, Margins, ToggleSwitch } from '@rocket.chat/fuselage';
 import { useUser } from '@rocket.chat/ui-contexts';
-import type { ComponentProps } from 'react';
-import { useCallback } from 'react';
+import type { ComponentProps, FormEvent } from 'react';
+import { useCallback, useId } from 'react';
 import { useTranslation } from 'react-i18next';
 
 import { useEndpointAction } from '../../../hooks/useEndpointAction';
 
 const TwoFactorEmail = (props: ComponentProps<typeof Box>) => {
 	const { t } = useTranslation();
 	const user = useUser();
+	const emailId = useId();
 
 	const isEnabled = user?.services?.email2fa?.enabled;
 
@@ -19,30 +20,26 @@ const TwoFactorEmail = (props: ComponentProps<typeof Box>) => {
 		successMessage: t('Two-factor_authentication_disabled'),
 	});
 
-	const handleEnable = useCallback(async () => {
-		await enable2faAction();
-	}, [enable2faAction]);
-	const handleDisable = useCallback(async () => {
-		await disable2faAction();
-	}, [disable2faAction]);
+	const handleEnable = useCallback(
+		async (e: FormEvent<HTMLInputElement>) => {
+			if (e.currentTarget.checked) {
+				await enable2faAction();
+			} else {
+				await disable2faAction();
+			}
+		},
+		[disable2faAction, enable2faAction],
+	);
 
 	return (
 		<Box display='flex' flexDirection='column' alignItems='flex-start' mbs={16} {...props}>
 			<Margins blockEnd={8}>
-				<Box fontScale='h4'>{t('Two-factor_authentication_email')}</Box>
-				{isEnabled && (
-					<Button danger onClick={handleDisable}>
-						{t('Disable_two-factor_authentication_email')}
-					</Button>
-				)}
-				{!isEnabled && (
-					<>
-						<Box>{t('Two-factor_authentication_email_is_currently_disabled')}</Box>
-						<Button primary onClick={handleEnable}>
-							{t('Enable_two-factor_authentication_email')}
-						</Button>
-					</>
-				)}
+				<Field>
+					<FieldRow>
+						<FieldLabel htmlFor={emailId}>{t('Two-factor_authentication_email')}</FieldLabel>
+						<ToggleSwitch id={emailId} checked={isEnabled} onChange={handleEnable} />
+					</FieldRow>
+				</Field>
 			</Margins>
 		</Box>
 	);

apps/meteor/client/views/account/security/TwoFactorTOTP.tsx

@@ -1,8 +1,8 @@
-import { Box, Button, TextInput, Margins } from '@rocket.chat/fuselage';
-import { useSafely } from '@rocket.chat/fuselage-hooks';
+import { Box, Button, TextInput, Margins, Field, FieldRow, FieldLabel, ToggleSwitch } from '@rocket.chat/fuselage';
+import { useEffectEvent, useSafely } from '@rocket.chat/fuselage-hooks';
 import { useSetModal, useToastMessageDispatch, useUser, useMethod } from '@rocket.chat/ui-contexts';
-import type { ReactElement, ComponentPropsWithoutRef } from 'react';
-import { useState, useCallback, useEffect } from 'react';
+import type { ReactElement, ComponentPropsWithoutRef, FormEvent } from 'react';
+import { useState, useCallback, useEffect, useId } from 'react';
 import { useForm } from 'react-hook-form';
 import { useTranslation } from 'react-i18next';
 import qrcode from 'yaqrcode';
@@ -51,7 +51,7 @@ const TwoFactorTOTP = (props: TwoFactorTOTPProps): ReactElement => {
 		updateCodesRemaining();
 	}, [checkCodesRemainingFn, setCodesRemaining, totpEnabled]);
 
-	const handleEnableTotp = useCallback(async () => {
+	const enableTotp = useEffectEvent(async () => {
 		try {
 			const result = await enableTotpFn();
 
@@ -62,26 +62,46 @@ const TwoFactorTOTP = (props: TwoFactorTOTPProps): ReactElement => {
 		} catch (error) {
 			dispatchToastMessage({ type: 'error', message: error });
 		}
-	}, [dispatchToastMessage, enableTotpFn, setQrCode, setRegisteringTotp, setTotpSecret]);
+	});
+
+	const disableTotp = useEffectEvent(async () => {
+		if (!totpEnabled) {
+			setRegisteringTotp(false);
+
+			return;
+		}
 
-	const handleDisableTotp = useCallback(async () => {
 		const onDisable = async (authCode: string): Promise<void> => {
 			try {
 				const result = await disableTotpFn(authCode);
 
 				if (!result) {
-					return dispatchToastMessage({ type: 'error', message: t('Invalid_two_factor_code') });
+					dispatchToastMessage({ type: 'error', message: t('Invalid_two_factor_code') });
+
+					return;
 				}
 
 				dispatchToastMessage({ type: 'success', message: t('Two-factor_authentication_disabled') });
 			} catch (error) {
 				dispatchToastMessage({ type: 'error', message: error });
 			}
+
 			closeModal();
 		};
 
 		setModal(<TwoFactorTotpModal onConfirm={onDisable} onClose={closeModal} />);
-	}, [closeModal, disableTotpFn, dispatchToastMessage, setModal, t]);
+	});
+
+	const handleToggleTotp = useEffectEvent(async (e: FormEvent<HTMLInputElement>) => {
+		if (e.currentTarget?.checked) {
+			void enableTotp();
+		} else {
+			void disableTotp();
+		}
+	});
+
+	const totpId = useId();
+	const totpCodeId = useId();
 
 	const handleVerifyCode = useCallback(
 		async ({ authCode }: TwoFactorTOTPFormData) => {
@@ -94,6 +114,8 @@ const TwoFactorTOTP = (props: TwoFactorTOTPProps): ReactElement => {
 
 				setRegisteringTotp(false);
 				setModal(<BackupCodesModal codes={result.codes} onClose={closeModal} />);
+
+				dispatchToastMessage({ type: 'success', message: t('Two-factor_authentication_enabled') });
 			} catch (error) {
 				dispatchToastMessage({ type: 'error', message: error });
 			}
@@ -121,38 +143,35 @@ const TwoFactorTOTP = (props: TwoFactorTOTPProps): ReactElement => {
 	return (
 		<Box display='flex' flexDirection='column' alignItems='flex-start' {...props}>
 			<Margins blockEnd={8}>
-				<Box fontScale='h4'>{t('Two-factor_authentication_via_TOTP')}</Box>
-				{!totpEnabled && !registeringTotp && (
-					<>
-						<Box>{t('Two-factor_authentication_is_currently_disabled')}</Box>
-						<Button primary onClick={handleEnableTotp}>
-							{t('Enable_two-factor_authentication')}
-						</Button>
-					</>
-				)}
+				<Field>
+					<FieldRow>
+						<FieldLabel htmlFor={totpId}>{t('Two-factor_authentication_via_TOTP')}</FieldLabel>
+						<ToggleSwitch id={totpId} checked={registeringTotp || totpEnabled} onChange={handleToggleTotp} />
+					</FieldRow>
+				</Field>
 				{!totpEnabled && registeringTotp && (
 					<>
 						<Box>{t('Scan_QR_code')}</Box>
 						<Box>{t('Scan_QR_code_alternative_s')}</Box>
 						<TextCopy text={totpSecret || ''} />
-						<Box is='img' size='x200' src={qrCode} aria-hidden='true' />
-						<Box display='flex' flexDirection='row' w='full'>
-							<TextInput placeholder={t('Enter_authentication_code')} {...register('authCode')} />
-							<Button primary onClick={handleSubmit(handleVerifyCode)}>
-								{t('Verify')}
-							</Button>
-						</Box>
+						<Box mis='-16px' mb='-16px' is='img' size='x200' src={qrCode} aria-hidden='true' />
+						<Field>
+							<FieldLabel htmlFor={totpCodeId}>{t('Enter_code_provided_by_authentication_app')}</FieldLabel>
+							<FieldRow>
+								<TextInput id={totpCodeId} mie='8px' {...register('authCode')} />
+								<Button primary onClick={handleSubmit(handleVerifyCode)}>
+									{t('Verify')}
+								</Button>
+							</FieldRow>
+						</Field>
 					</>
 				)}
 				{totpEnabled && (
 					<>
-						<Button danger onClick={handleDisableTotp}>
-							{t('Disable_two-factor_authentication')}
-						</Button>
 						<Box fontScale='p2m' mbs={8}>
 							{t('Backup_codes')}
 						</Box>
-						<Box>{t('You_have_n_codes_remaining', { number: codesRemaining })}</Box>
+						<Box color='font-secondary-info'>{t('You_have_n_codes_remaining', { number: codesRemaining })}</Box>
 						<Button onClick={handleRegenerateCodes}>{t('Regenerate_codes')}</Button>
 					</>
 				)}

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

@@ -0,0 +1,22 @@
+import { useSetting, useUser } from '@rocket.chat/ui-contexts';
+import { useCallback } from 'react';
+
+import { Roles } from '../../../app/models/client';
+import { useReactiveValue } from '../../hooks/useReactiveValue';
+
+export const useRequire2faSetup = () => {
+	const user = useUser();
+	const tfaEnabled = useSetting('Accounts_TwoFactorAuthentication_Enabled');
+
+	return useReactiveValue(
+		useCallback(() => {
+			// User is already using 2fa
+			if (!user || user?.services?.totp?.enabled || user?.services?.email2fa?.enabled) {
+				return false;
+			}
+
+			const mandatoryRole = Roles.findOne({ _id: { $in: user.roles ?? [] }, mandatory2fa: true });
+			return !!(mandatoryRole !== undefined && tfaEnabled);
+		}, [tfaEnabled, user]),
+	);
+};

apps/meteor/client/views/root/MainLayout/TwoFactorAuthSetupCheck.tsx

@@ -1,30 +1,25 @@
 import { FeaturePreview, FeaturePreviewOff, FeaturePreviewOn } from '@rocket.chat/ui-client';
-import { useLayout, useUser, useSetting } from '@rocket.chat/ui-contexts';
+import { useLayout, useSetModal } from '@rocket.chat/ui-contexts';
 import type { ReactElement, ReactNode } from 'react';
-import { lazy, useCallback } from 'react';
+import { lazy, useLayoutEffect } from 'react';
 
 import LayoutWithSidebar from './LayoutWithSidebar';
 import LayoutWithSidebarV2 from './LayoutWithSidebarV2';
-import { Roles } from '../../../../app/models/client';
-import { useReactiveValue } from '../../../hooks/useReactiveValue';
+import TwoFactorRequiredModal from './TwoFactorRequiredModal';
+import { useRequire2faSetup } from '../../hooks/useRequire2faSetup';
 
 const AccountSecurityPage = lazy(() => import('../../account/security/AccountSecurityPage'));
 
 const TwoFactorAuthSetupCheck = ({ children }: { children: ReactNode }): ReactElement => {
 	const { isEmbedded: embeddedLayout } = useLayout();
-	const user = useUser();
-	const tfaEnabled = useSetting('Accounts_TwoFactorAuthentication_Enabled');
-	const require2faSetup = useReactiveValue(
-		useCallback(() => {
-			// User is already using 2fa
-			if (!user || user?.services?.totp?.enabled || user?.services?.email2fa?.enabled) {
-				return false;
-			}
+	const require2faSetup = useRequire2faSetup();
+	const setModal = useSetModal();
 
-			const mandatoryRole = Roles.findOne({ _id: { $in: user.roles ?? [] }, mandatory2fa: true });
-			return mandatoryRole !== undefined && tfaEnabled;
-		}, [tfaEnabled, user]),
-	);
+	useLayoutEffect(() => {
+		if (require2faSetup) {
+			setModal(<TwoFactorRequiredModal />);
+		}
+	}, [setModal, require2faSetup]);
 
 	if (require2faSetup) {
 		return (

apps/meteor/client/views/root/MainLayout/TwoFactorRequiredModal.tsx

@@ -0,0 +1,31 @@
+import { Button, Modal, ModalContent, ModalFooter, ModalFooterControllers, ModalHeader, ModalTitle } from '@rocket.chat/fuselage';
+import { useSetModal } from '@rocket.chat/ui-contexts';
+import { useCallback } from 'react';
+import { useTranslation } from 'react-i18next';
+
+const TwoFactorRequiredModal = () => {
+	const { t } = useTranslation();
+	const setModal = useSetModal();
+
+	const closeModal = useCallback(() => {
+		setModal(null);
+	}, [setModal]);
+
+	return (
+		<Modal>
+			<ModalHeader>
+				<ModalTitle>{t('Two-factor_authentication_required')}</ModalTitle>
+			</ModalHeader>
+			<ModalContent>{t('Enable_two-factor_authentication_callout_description')}</ModalContent>
+			<ModalFooter>
+				<ModalFooterControllers>
+					<Button primary onClick={closeModal}>
+						{t('Set_up_2FA')}
+					</Button>
+				</ModalFooterControllers>
+			</ModalFooter>
+		</Modal>
+	);
+};
+
+export default TwoFactorRequiredModal;

apps/meteor/tests/e2e/account-profile.spec.ts

@@ -81,22 +81,15 @@ test.describe.serial('settings-account-profile', () => {
 			expect(results.violations).toEqual([]);
 		});
 
-		test('expect to disable email 2FA', async () => {
+		test('should disable and enable email 2FA', async () => {
 			await poAccountProfile.security2FASection.click();
-			await expect(poAccountProfile.disableEmail2FAButton).toBeVisible();
-			await poAccountProfile.disableEmail2FAButton.click();
-
+			await expect(poAccountProfile.email2FASwitch).toBeVisible();
+			await poAccountProfile.email2FASwitch.click();
 			await expect(poHomeChannel.toastSuccess).toBeVisible();
-			await expect(poAccountProfile.enableEmail2FAButton).toBeVisible();
-		});
-
-		test('expect to enable email 2FA', async () => {
-			await poAccountProfile.security2FASection.click();
-			await expect(poAccountProfile.enableEmail2FAButton).toBeVisible();
-			await poAccountProfile.enableEmail2FAButton.click();
+			await poHomeChannel.dismissToast();
 
+			await poAccountProfile.email2FASwitch.click();
 			await expect(poHomeChannel.toastSuccess).toBeVisible();
-			await expect(poAccountProfile.disableEmail2FAButton).toBeVisible();
 		});
 	});