fix: Blocked login when dismissed 2FA modal (#32482)

Co-authored-by: Tasso Evangelista <2263066+tassoevan@users.noreply.github.com>

.changeset/soft-donkeys-thank.md

@@ -0,0 +1,8 @@
+---
+"@rocket.chat/meteor": patch
+"@rocket.chat/mock-providers": patch
+"@rocket.chat/ui-contexts": patch
+"@rocket.chat/web-ui-registration": patch
+---
+
+Fixed an issue with blocked login when dismissed 2FA modal by clicking outside of it or pressing the escape key

apps/meteor/client/components/GenericModal/GenericModal.spec.tsx

@@ -0,0 +1,87 @@
+import { useSetModal } from '@rocket.chat/ui-contexts';
+import { act, screen } from '@testing-library/react';
+import { renderHook } from '@testing-library/react-hooks';
+import userEvent from '@testing-library/user-event';
+import type { ReactElement } from 'react';
+import React, { Suspense } from 'react';
+
+import ModalProviderWithRegion from '../../providers/ModalProvider/ModalProviderWithRegion';
+import GenericModal from './GenericModal';
+
+import '@testing-library/jest-dom';
+
+const renderModal = (modalElement: ReactElement) => {
+	const {
+		result: { current: setModal },
+	} = renderHook(() => useSetModal(), {
+		wrapper: ({ children }) => (
+			<Suspense fallback={null}>
+				<ModalProviderWithRegion>{children}</ModalProviderWithRegion>
+			</Suspense>
+		),
+	});
+
+	act(() => {
+		setModal(modalElement);
+	});
+
+	return { setModal };
+};
+
+describe('callbacks', () => {
+	it('should call onClose callback when dismissed', async () => {
+		const handleClose = jest.fn();
+
+		renderModal(<GenericModal title='Modal' onClose={handleClose} />);
+
+		expect(await screen.findByRole('heading', { name: 'Modal', exact: true })).toBeInTheDocument();
+
+		userEvent.keyboard('{Escape}');
+
+		expect(screen.queryByRole('heading', { name: 'Modal', exact: true })).not.toBeInTheDocument();
+
+		expect(handleClose).toHaveBeenCalled();
+	});
+
+	it('should NOT call onClose callback when confirmed', async () => {
+		const handleConfirm = jest.fn();
+		const handleClose = jest.fn();
+
+		const { setModal } = renderModal(<GenericModal title='Modal' onConfirm={handleConfirm} onClose={handleClose} />);
+
+		expect(await screen.findByRole('heading', { name: 'Modal', exact: true })).toBeInTheDocument();
+
+		userEvent.click(screen.getByRole('button', { name: 'Ok', exact: true }));
+
+		expect(handleConfirm).toHaveBeenCalled();
+
+		act(() => {
+			setModal(null);
+		});
+
+		expect(screen.queryByRole('heading', { name: 'Modal', exact: true })).not.toBeInTheDocument();
+
+		expect(handleClose).not.toHaveBeenCalled();
+	});
+
+	it('should NOT call onClose callback when cancelled', async () => {
+		const handleCancel = jest.fn();
+		const handleClose = jest.fn();
+
+		const { setModal } = renderModal(<GenericModal title='Modal' onCancel={handleCancel} onClose={handleClose} />);
+
+		expect(await screen.findByRole('heading', { name: 'Modal', exact: true })).toBeInTheDocument();
+
+		userEvent.click(screen.getByRole('button', { name: 'Cancel', exact: true }));
+
+		expect(handleCancel).toHaveBeenCalled();
+
+		act(() => {
+			setModal(null);
+		});
+
+		expect(screen.queryByRole('heading', { name: 'Modal', exact: true })).not.toBeInTheDocument();
+
+		expect(handleClose).not.toHaveBeenCalled();
+	});
+});

apps/meteor/client/components/GenericModal/GenericModal.tsx

@@ -1,9 +1,9 @@
 import { Button, Modal } from '@rocket.chat/fuselage';
-import { useUniqueId } from '@rocket.chat/fuselage-hooks';
+import { useEffectEvent, useUniqueId } from '@rocket.chat/fuselage-hooks';
 import type { Keys as IconName } from '@rocket.chat/icons';
 import { useTranslation } from '@rocket.chat/ui-contexts';
 import type { ComponentProps, ReactElement, ReactNode, ComponentPropsWithoutRef } from 'react';
-import React from 'react';
+import React, { useEffect, useRef } from 'react';
 
 import type { RequiredModalProps } from './withDoNotAskAgain';
 import { withDoNotAskAgain } from './withDoNotAskAgain';
@@ -78,6 +78,31 @@ const GenericModal = ({
 	const t = useTranslation();
 	const genericModalId = useUniqueId();
 
+	const dismissedRef = useRef(true);
+
+	const handleConfirm = useEffectEvent(() => {
+		dismissedRef.current = false;
+		onConfirm?.();
+	});
+
+	const handleCancel = useEffectEvent(() => {
+		dismissedRef.current = false;
+		onCancel?.();
+	});
+
+	const handleCloseButtonClick = useEffectEvent(() => {
+		dismissedRef.current = true;
+		onClose?.();
+	});
+
+	useEffect(
+		() => () => {
+			if (!dismissedRef.current) return;
+			onClose?.();
+		},
+		[onClose],
+	);
+
 	return (
 		<Modal aria-labelledby={`${genericModalId}-title`} wrapperFunction={wrapperFunction} {...props}>
 			<Modal.Header>
@@ -86,7 +111,7 @@ const GenericModal = ({
 					{tagline && <Modal.Tagline>{tagline}</Modal.Tagline>}
 					<Modal.Title id={`${genericModalId}-title`}>{title ?? t('Are_you_sure')}</Modal.Title>
 				</Modal.HeaderText>
-				<Modal.Close aria-label={t('Close')} onClick={onClose} />
+				<Modal.Close aria-label={t('Close')} onClick={handleCloseButtonClick} />
 			</Modal.Header>
 			<Modal.Content fontScale='p2'>{children}</Modal.Content>
 			<Modal.Footer justifyContent={dontAskAgain ? 'space-between' : 'end'}>
@@ -94,7 +119,7 @@ const GenericModal = ({
 				{annotation && !dontAskAgain && <Modal.FooterAnnotation>{annotation}</Modal.FooterAnnotation>}
 				<Modal.FooterControllers>
 					{onCancel && (
-						<Button secondary onClick={onCancel}>
+						<Button secondary onClick={handleCancel}>
 							{cancelText ?? t('Cancel')}
 						</Button>
 					)}
@@ -104,7 +129,7 @@ const GenericModal = ({
 						</Button>
 					)}
 					{!wrapperFunction && onConfirm && (
-						<Button {...getButtonProps(variant)} onClick={onConfirm} disabled={confirmDisabled}>
+						<Button {...getButtonProps(variant)} onClick={handleConfirm} disabled={confirmDisabled}>
 							{confirmText ?? t('Ok')}
 						</Button>
 					)}

apps/meteor/client/lib/imperativeModal.tsx

@@ -1,15 +1,15 @@
 import { Emitter } from '@rocket.chat/emitter';
 import React, { Suspense, createElement } from 'react';
-import type { ComponentProps, ElementType, ReactNode } from 'react';
+import type { ComponentProps, ComponentType, ReactNode } from 'react';
 
 import { modalStore } from '../providers/ModalProvider/ModalStore';
 
-type ReactModalDescriptor<TComponent extends ElementType> = {
+type ReactModalDescriptor<TComponent extends ComponentType<any> = ComponentType<any>> = {
 	component: TComponent;
 	props?: ComponentProps<TComponent>;
 };
 
-type ModalDescriptor = ReactModalDescriptor<ElementType> | null;
+type ModalDescriptor = ReactModalDescriptor | null;
 
 type ModalInstance = {
 	close: () => void;
@@ -41,11 +41,11 @@ class ImperativeModalEmmiter extends Emitter<{ update: ModalDescriptor }> {
 		this.store = store;
 	}
 
-	open = <TComponent extends ElementType>(descriptor: ReactModalDescriptor<TComponent>): ModalInstance => {
+	open = <TComponent extends ComponentType<any>>(descriptor: ReactModalDescriptor<TComponent>): ModalInstance => {
 		return this.store.open(mapCurrentModal(descriptor as ModalDescriptor));
 	};
 
-	push = <TComponent extends ElementType>(descriptor: ReactModalDescriptor<TComponent>): ModalInstance => {
+	push = <TComponent extends ComponentType<any>>(descriptor: ReactModalDescriptor<TComponent>): ModalInstance => {
 		return this.store.push(mapCurrentModal(descriptor as ModalDescriptor));
 	};
 

apps/meteor/client/portals/ModalPortal.tsx

@@ -1,18 +1,32 @@
-import type { ReactElement, ReactNode } from 'react';
-import React, { memo, useEffect, useState } from 'react';
+import type { ReactNode } from 'react';
+import { memo } from 'react';
 import { createPortal } from 'react-dom';
 
-import { createAnchor } from '../lib/utils/createAnchor';
-import { deleteAnchor } from '../lib/utils/deleteAnchor';
+const createModalRoot = (): HTMLElement => {
+	const id = 'modal-root';
+	const existing = document.getElementById(id);
+
+	if (existing) return existing;
+
+	const newOne = document.createElement('div');
+	newOne.id = id;
+	document.body.append(newOne);
+
+	return newOne;
+};
+
+let modalRoot: HTMLElement | null = null;
 
 type ModalPortalProps = {
 	children?: ReactNode;
 };
 
-const ModalPortal = ({ children }: ModalPortalProps): ReactElement => {
-	const [modalRoot] = useState(() => createAnchor('modal-root'));
-	useEffect(() => (): void => deleteAnchor(modalRoot), [modalRoot]);
-	return <>{createPortal(children, modalRoot)}</>;
+const ModalPortal = ({ children }: ModalPortalProps) => {
+	if (!modalRoot) {
+		modalRoot = createModalRoot();
+	}
+
+	return createPortal(children, modalRoot);
 };
 
 export default memo(ModalPortal);

apps/meteor/client/providers/ModalProvider/ModalProvider.spec.tsx

@@ -1,115 +1,138 @@
-// import type { IMessage } from '@rocket.chat/core-typings';
-import { Emitter } from '@rocket.chat/emitter';
 import { useSetModal } from '@rocket.chat/ui-contexts';
-import { render, screen } from '@testing-library/react';
-import { expect } from 'chai';
-import type { ReactNode } from 'react';
-import React, { Suspense, createContext, useContext, useEffect } from 'react';
+import { act, render, screen } from '@testing-library/react';
+import type { ForwardedRef, ReactElement } from 'react';
+import React, { Suspense, createContext, createRef, forwardRef, useContext, useImperativeHandle } from 'react';
 
 import GenericModal from '../../components/GenericModal';
 import { imperativeModal } from '../../lib/imperativeModal';
 import ModalRegion from '../../views/modal/ModalRegion';
 import ModalProvider from './ModalProvider';
 import ModalProviderWithRegion from './ModalProviderWithRegion';
+import '@testing-library/jest-dom';
 
-const TestContext = createContext({ title: 'default' });
-const emitter = new Emitter();
+const renderWithSuspense = (ui: ReactElement) =>
+	render(ui, {
+		wrapper: ({ children }) => <Suspense fallback={null}>{children}</Suspense>,
+	});
 
-const TestModal = ({ emitterEvent, modalFunc }: { emitterEvent: string; modalFunc?: () => ReactNode }) => {
-	const setModal = useSetModal();
-	const { title } = useContext(TestContext);
+describe('via useSetModal', () => {
+	const ModalTitleContext = createContext('default');
 
-	useEffect(() => {
-		emitter.on(emitterEvent, () => {
-			setModal(modalFunc || <GenericModal title={title} onClose={() => undefined}></GenericModal>);
-		});
-	}, [emitterEvent, setModal, title, modalFunc]);
+	type ModalOpenerAPI = { open: () => void };
 
-	return <></>;
-};
+	const ModalOpener = forwardRef((_: unknown, ref: ForwardedRef<ModalOpenerAPI>) => {
+		const setModal = useSetModal();
+		const title = useContext(ModalTitleContext);
+		useImperativeHandle(ref, () => ({
+			open: () => {
+				setModal(<GenericModal open title={title} />);
+			},
+		}));
+
+		return null;
+	});
 
-describe('Modal Provider', () => {
 	it('should render a modal', async () => {
-		render(
-			<Suspense fallback={null}>
+		const modalOpenerRef = createRef<ModalOpenerAPI>();
+
+		renderWithSuspense(
+			<ModalProviderWithRegion>
+				<ModalOpener ref={modalOpenerRef} />
+			</ModalProviderWithRegion>,
+		);
+
+		act(() => {
+			modalOpenerRef.current?.open();
+		});
+
+		expect(await screen.findByRole('dialog', { name: 'default' })).toBeInTheDocument();
+	});
+
+	it('should render a modal that consumes a context', async () => {
+		const modalOpenerRef = createRef<ModalOpenerAPI>();
+
+		renderWithSuspense(
+			<ModalTitleContext.Provider value='title from context'>
 				<ModalProviderWithRegion>
-					<TestModal emitterEvent='open' />
+					<ModalOpener ref={modalOpenerRef} />
 				</ModalProviderWithRegion>
-			</Suspense>,
+			</ModalTitleContext.Provider>,
 		);
-		emitter.emit('open');
-		expect(await screen.findByText('default')).to.exist;
+		act(() => {
+			modalOpenerRef.current?.open();
+		});
+
+		expect(await screen.findByRole('dialog', { name: 'title from context' })).toBeInTheDocument();
 	});
 
-	it('should render a modal that is passed as a function', async () => {
-		render(
-			<Suspense fallback={null}>
+	it('should render a modal in another region', async () => {
+		const modalOpener1Ref = createRef<ModalOpenerAPI>();
+		const modalOpener2Ref = createRef<ModalOpenerAPI>();
+
+		renderWithSuspense(
+			<ModalTitleContext.Provider value='modal1'>
 				<ModalProviderWithRegion>
-					<TestModal emitterEvent='open' modalFunc={() => <GenericModal title='function modal' onClose={() => undefined} />} />
+					<ModalOpener ref={modalOpener1Ref} />
 				</ModalProviderWithRegion>
-			</Suspense>,
+				<ModalTitleContext.Provider value='modal2'>
+					<ModalProviderWithRegion>
+						<ModalOpener ref={modalOpener2Ref} />
+					</ModalProviderWithRegion>
+				</ModalTitleContext.Provider>
+			</ModalTitleContext.Provider>,
 		);
-		emitter.emit('open');
-		expect(await screen.findByText('function modal')).to.exist;
+
+		act(() => {
+			modalOpener1Ref.current?.open();
+		});
+
+		expect(await screen.findByRole('dialog', { name: 'modal1' })).toBeInTheDocument();
+
+		act(() => {
+			modalOpener2Ref.current?.open();
+		});
+
+		expect(await screen.findByRole('dialog', { name: 'modal2' })).toBeInTheDocument();
 	});
+});
+
+describe('via imperativeModal', () => {
+	it('should render a modal through imperative modal', async () => {
+		renderWithSuspense(
+			<ModalProvider>
+				<ModalRegion />
+			</ModalProvider>,
+		);
 
-	it('should render a modal through imperative modal', () => {
-		async () => {
-			render(
-				<Suspense fallback={null}>
-					<ModalProvider>
-						<ModalRegion />
-					</ModalProvider>
-				</Suspense>,
-			);
-
-			const { close } = imperativeModal.open({
+		act(() => {
+			imperativeModal.open({
 				component: GenericModal,
-				props: { title: 'imperativeModal' },
+				props: { title: 'imperativeModal', open: true },
 			});
+		});
 
-			expect(await screen.findByText('imperativeModal')).to.exist;
+		expect(await screen.findByRole('dialog', { name: 'imperativeModal' })).toBeInTheDocument();
 
-			close();
+		act(() => {
+			imperativeModal.close();
+		});
 
-			expect(screen.queryByText('imperativeModal')).to.not.exist;
-		};
+		expect(screen.queryByText('imperativeModal')).not.toBeInTheDocument();
 	});
 
 	it('should not render a modal if no corresponding region exists', async () => {
 		// ModalProviderWithRegion will always have a region identifier set
 		// and imperativeModal will only render modals in the default region (e.g no region identifier)
-		render(
-			<Suspense fallback={null}>
-				<ModalProviderWithRegion />
-			</Suspense>,
-		);
-
-		imperativeModal.open({
-			component: GenericModal,
-			props: { title: 'imperativeModal' },
-		});
 
-		expect(screen.queryByText('imperativeModal')).to.not.exist;
-	});
+		renderWithSuspense(<ModalProviderWithRegion />);
 
-	it('should render a modal in another region', () => {
-		render(
-			<TestContext.Provider value={{ title: 'modal1' }}>
-				<ModalProviderWithRegion>
-					<TestModal emitterEvent='openModal1' />
-				</ModalProviderWithRegion>
-				<TestContext.Provider value={{ title: 'modal2' }}>
-					<ModalProviderWithRegion>
-						<TestModal emitterEvent='openModal2' />
-					</ModalProviderWithRegion>
-				</TestContext.Provider>
-			</TestContext.Provider>,
-		);
+		act(() => {
+			imperativeModal.open({
+				component: GenericModal,
+				props: { title: 'imperativeModal', open: true },
+			});
+		});
 
-		emitter.emit('openModal1');
-		expect(screen.getByText('modal1')).to.exist;
-		emitter.emit('openModal2');
-		expect(screen.getByText('modal2')).to.exist;
+		expect(screen.queryByRole('dialog', { name: 'imperativeModal' })).not.toBeInTheDocument();
 	});
 });

apps/meteor/client/providers/ModalProvider/ModalProvider.tsx

@@ -33,7 +33,7 @@ const ModalProvider = ({ children, region }: ModalProviderProps) => {
 			},
 			region,
 		}),
-		[currentModal, region, setModal],
+		[currentModal?.node, currentModal?.region, region, setModal],
 	);
 
 	return <ModalContext.Provider value={contextValue} children={children} />;

apps/meteor/client/views/modal/ModalRegion.tsx

@@ -1,6 +1,7 @@
-import { useModal, useCurrentModal } from '@rocket.chat/ui-contexts';
+import { useEffectEvent } from '@rocket.chat/fuselage-hooks';
+import { useCurrentModal, useModal } from '@rocket.chat/ui-contexts';
 import type { ReactElement } from 'react';
-import React, { lazy, useCallback } from 'react';
+import React, { lazy } from 'react';
 
 import ModalBackdrop from '../../components/ModalBackdrop';
 import ModalPortal from '../../portals/ModalPortal';
@@ -10,7 +11,9 @@ const FocusScope = lazy(() => import('react-aria').then((module) => ({ default:
 const ModalRegion = (): ReactElement | null => {
 	const currentModal = useCurrentModal();
 	const { setModal } = useModal();
-	const handleDismiss = useCallback(() => setModal(null), [setModal]);
+	const handleDismiss = useEffectEvent(() => {
+		setModal(null);
+	});
 
 	if (!currentModal) {
 		return null;

apps/meteor/client/views/room/contextualBar/VideoConference/hooks/useVideoConfOpenCall.spec.tsx

@@ -1,20 +1,18 @@
 import { faker } from '@faker-js/faker';
 import { ModalContext } from '@rocket.chat/ui-contexts';
-import type { WrapperComponent } from '@testing-library/react-hooks';
 import { renderHook } from '@testing-library/react-hooks';
+import type { ReactNode } from 'react';
 import React from 'react';
 
 import { useVideoConfOpenCall } from './useVideoConfOpenCall';
 
 describe('with window.RocketChatDesktop set', () => {
-	const wrapper: WrapperComponent<unknown> = ({ children }) => (
+	const wrapper = ({ children }: { children: ReactNode }) => (
 		<ModalContext.Provider
 			children={children}
 			value={{
 				modal: {
-					setModal: () => {
-						return null;
-					},
+					setModal: () => null,
 				},
 				currentModal: { component: null },
 			}}
@@ -54,7 +52,8 @@ describe('with window.RocketChatDesktop set', () => {
 
 describe('with window.RocketChatDesktop unset', () => {
 	const setModal = jest.fn();
-	const wrapper: WrapperComponent<unknown> = ({ children }) => (
+
+	const wrapper = ({ children }: { children: ReactNode }) => (
 		<ModalContext.Provider
 			children={children}
 			value={{

apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRedirectModerationConsole.ts

@@ -19,7 +19,7 @@ export const useRedirectModerationConsole = (uid: IUser['_id']): UserInfoAction
 
 	return {
 		content: t('Moderation_Action_View_reports'),
-		icon: 'warning' as const,
+		icon: 'warning',
 		onClick: redirectModerationConsoleAction,
 		type: 'privileges' as UserInfoActionType,
 	};

packages/mock-providers/src/MockedModalContext.tsx

@@ -1,9 +1,9 @@
 import { ModalContext } from '@rocket.chat/ui-contexts';
 import type { ReactNode } from 'react';
-import React from 'react';
+import React, { useState } from 'react';
 
 export const MockedModalContext = ({ children }: { children: React.ReactNode }) => {
-	const [currentModal, setCurrentModal] = React.useState<ReactNode>(null);
+	const [currentModal, setCurrentModal] = useState<ReactNode>(null);
 
 	return (
 		<ModalContext.Provider

packages/web-ui-registration/package.json

@@ -56,5 +56,8 @@
 		"react": "*",
 		"react-hook-form": "*",
 		"react-i18next": "*"
+	},
+	"volta": {
+		"extends": "../../package.json"
 	}
 }