Skip to content

Commit

Permalink
fix: Blocked login when dismissed 2FA modal (#32482)
Browse files Browse the repository at this point in the history
Co-authored-by: Tasso Evangelista <[email protected]>
  • Loading branch information
tiagoevanp and tassoevan committed Jul 19, 2024
1 parent ebc858f commit 4e8aa57
Show file tree
Hide file tree
Showing 12 changed files with 270 additions and 108 deletions.
8 changes: 8 additions & 0 deletions .changeset/soft-donkeys-thank.md
Original file line number Diff line number Diff line change
@@ -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
87 changes: 87 additions & 0 deletions apps/meteor/client/components/GenericModal/GenericModal.spec.tsx
Original file line number Diff line number Diff line change
@@ -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();
});
});
35 changes: 30 additions & 5 deletions apps/meteor/client/components/GenericModal/GenericModal.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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>
Expand All @@ -86,15 +111,15 @@ 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'}>
{dontAskAgain}
{annotation && !dontAskAgain && <Modal.FooterAnnotation>{annotation}</Modal.FooterAnnotation>}
<Modal.FooterControllers>
{onCancel && (
<Button secondary onClick={onCancel}>
<Button secondary onClick={handleCancel}>
{cancelText ?? t('Cancel')}
</Button>
)}
Expand All @@ -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>
)}
Expand Down
10 changes: 5 additions & 5 deletions apps/meteor/client/lib/imperativeModal.tsx
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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));
};

Expand Down
30 changes: 22 additions & 8 deletions apps/meteor/client/portals/ModalPortal.tsx
Original file line number Diff line number Diff line change
@@ -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);
Loading

0 comments on commit 4e8aa57

Please sign in to comment.