diff --git a/lib/teleterm/daemon/daemon.go b/lib/teleterm/daemon/daemon.go index c826468ca22da..f0b44f7bcb14c 100644 --- a/lib/teleterm/daemon/daemon.go +++ b/lib/teleterm/daemon/daemon.go @@ -114,11 +114,6 @@ func (s *Service) relogin(ctx context.Context, req *api.ReloginRequest) error { } defer s.reloginMu.Unlock() - if err := s.importantModalSemaphore.Acquire(ctx); err != nil { - return trace.Wrap(err) - } - defer s.importantModalSemaphore.Release() - const reloginUserTimeout = time.Minute timeoutCtx, cancelTshdEventsCtx := context.WithTimeout(ctx, reloginUserTimeout) defer cancelTshdEventsCtx() @@ -892,7 +887,7 @@ func (s *Service) UpdateAndDialTshdEventsServerAddress(serverAddress string) err client := api.NewTshdEventsServiceClient(conn) s.tshdEventsClient = client - s.importantModalSemaphore = newWaitSemaphore(maxConcurrentImportantModals, imporantModalWaitDuraiton) + s.headlessAuthSemaphore = newWaitSemaphore(maxConcurrentImportantModals, imporantModalWaitDuraiton) return nil } @@ -1191,20 +1186,32 @@ type Service struct { gateways map[string]gateway.Gateway // tshdEventsClient is a client to send events to the Electron App. tshdEventsClient api.TshdEventsServiceClient - // The Electron App can only display one important Modal at a time. tshd events - // that trigger an important modal (relogin, headless login) should use this - // lock to ensure it doesn't overwrite existing tshd-initiated important modals. - // - // We use a semaphore instead of a mutex in order to cancel important modals that - // are no longer relevant before acquisition. + + // The Electron App can display multiple important modals by showing the latest one and hiding the others. + // However, we should be careful with it, and generally try to limit the number of prompts on the tshd side, + // to avoid flooding the app. + // Multiple prompts of the same type may also conflict with each other. This is currently possible with + // MFA prompts (see the mfaMu comment for details). + // Generally, only one prompt of each type (e.g., re-login, MFA) should be allowed at the same time. // - // We use a waitSemaphore in order to make sure there is a clear transition between modals. - importantModalSemaphore *waitSemaphore + // But why do we allow multiple important modals at all? It is necessary in specific scenarios where one + // modal action requires completing in another. Currently, there are two cases: + // 1. A hardware key prompt may appear during a re-login process. + // 2. An MFA prompt may appear during a re-login process. + + // We use a waitSemaphore to make sure there is a clear transition between modals. + // We allow a single headless auth prompt at a time. + headlessAuthSemaphore *waitSemaphore + // mfaMu prevents concurrent MFA prompts. These can happen when using VNet with per-session MFA. + // Issuing an MFA prompt starts the Webauthn goroutine which prompts for key touch on hardware level. + // Webauthn does not support concurrent prompts, so without this semaphore, one of the prompts would fail immediately. + mfaMu sync.Mutex + // reloginMu is used when a goroutine needs to request a relogin from the Electron app. + // We allow a single relogin prompt at a time. + reloginMu sync.Mutex + // usageReporter batches the events and sends them to prehog usageReporter *usagereporter.UsageReporter - // reloginMu is used when a goroutine needs to request a relogin from the Electron app. Since the - // app can show only one login modal at a time, we need to submit only one request at a time. - reloginMu sync.Mutex // headlessWatcherClosers holds a map of root cluster URIs to headless watchers. headlessWatcherClosers map[string]context.CancelFunc headlessWatcherClosersMu sync.Mutex diff --git a/lib/teleterm/daemon/daemon_headless.go b/lib/teleterm/daemon/daemon_headless.go index 3ea8b5755d678..2662791ca17bb 100644 --- a/lib/teleterm/daemon/daemon_headless.go +++ b/lib/teleterm/daemon/daemon_headless.go @@ -272,10 +272,10 @@ func (s *Service) sendPendingHeadlessAuthentication(ctx context.Context, ha *typ HeadlessAuthenticationClientIp: ha.ClientIpAddress, } - if err := s.importantModalSemaphore.Acquire(ctx); err != nil { + if err := s.headlessAuthSemaphore.Acquire(ctx); err != nil { return trace.Wrap(err) } - defer s.importantModalSemaphore.Release() + defer s.headlessAuthSemaphore.Release() _, err := s.tshdEventsClient.SendPendingHeadlessAuthentication(ctx, req) return trace.Wrap(err) diff --git a/lib/teleterm/daemon/daemon_test.go b/lib/teleterm/daemon/daemon_test.go index 00bf1176ce03f..9c46057f7ac30 100644 --- a/lib/teleterm/daemon/daemon_test.go +++ b/lib/teleterm/daemon/daemon_test.go @@ -489,7 +489,7 @@ func TestRetryWithRelogin(t *testing.T) { } } -func TestImportantModalSemaphore(t *testing.T) { +func TestConcurrentHeadlessAuthPrompts(t *testing.T) { t.Parallel() ctx := context.Background() @@ -522,54 +522,54 @@ func TestImportantModalSemaphore(t *testing.T) { // Claim the important modal semaphore. customWaitDuration := 10 * time.Millisecond - daemon.importantModalSemaphore.waitDuration = customWaitDuration - err = daemon.importantModalSemaphore.Acquire(ctx) + daemon.headlessAuthSemaphore.waitDuration = customWaitDuration + err = daemon.headlessAuthSemaphore.Acquire(ctx) require.NoError(t, err) - // relogin and sending pending headless authentications should be blocked. + // Pending headless authentications should be blocked. - reloginErrC := make(chan error) + headlessPromptErr1 := make(chan error) go func() { - reloginErrC <- daemon.relogin(ctx, &api.ReloginRequest{}) + headlessPromptErr1 <- daemon.sendPendingHeadlessAuthentication(ctx, &types.HeadlessAuthentication{}, "") }() - sphaErrC := make(chan error) + headlessPromptErr2 := make(chan error) go func() { - sphaErrC <- daemon.sendPendingHeadlessAuthentication(ctx, &types.HeadlessAuthentication{}, "") + headlessPromptErr2 <- daemon.sendPendingHeadlessAuthentication(ctx, &types.HeadlessAuthentication{}, "") }() select { - case <-reloginErrC: - t.Error("relogin completed successfully without acquiring the important modal semaphore") - case <-sphaErrC: - t.Error("sendPendingHeadlessAuthentication completed successfully without acquiring the important modal semaphore") + case <-headlessPromptErr1: + t.Error("sendPendingHeadlessAuthentication for the first prompt completed successfully without acquiring the semaphore") + case <-headlessPromptErr2: + t.Error("sendPendingHeadlessAuthentication for the second prompt completed successfully without acquiring the semaphore") case <-time.After(100 * time.Millisecond): } - // if the request's ctx is canceled, they will unblock and return an error instead. + // If the request's ctx is canceled, they will unblock and return an error instead. cancelCtx, cancel := context.WithCancel(ctx) cancel() - err = daemon.relogin(cancelCtx, &api.ReloginRequest{}) + err = daemon.sendPendingHeadlessAuthentication(cancelCtx, &types.HeadlessAuthentication{}, "") require.Error(t, err) err = daemon.sendPendingHeadlessAuthentication(cancelCtx, &types.HeadlessAuthentication{}, "") require.Error(t, err) - // Release the semaphore. relogin and sending pending headless authentication should + // Release the semaphore. Pending headless authentication should // complete successfully after a short delay between each semaphore release. releaseTime := time.Now() - daemon.importantModalSemaphore.Release() + daemon.headlessAuthSemaphore.Release() var otherC chan error select { - case err := <-reloginErrC: + case err := <-headlessPromptErr1: require.NoError(t, err) - otherC = sphaErrC - case err := <-sphaErrC: + otherC = headlessPromptErr2 + case err := <-headlessPromptErr2: require.NoError(t, err) - otherC = reloginErrC + otherC = headlessPromptErr1 case <-time.After(time.Second): t.Error("important modal operations failed to acquire unclaimed semaphore") } @@ -589,8 +589,7 @@ func TestImportantModalSemaphore(t *testing.T) { t.Error("important modal semaphore should not be acquired before waiting the specified duration") } - require.EqualValues(t, 1, service.reloginCount.Load(), "Unexpected number of calls to service.Relogin") - require.EqualValues(t, 1, service.sendPendingHeadlessAuthenticationCount.Load(), "Unexpected number of calls to service.SendPendingHeadlessAuthentication") + require.EqualValues(t, 2, service.sendPendingHeadlessAuthenticationCount.Load(), "Unexpected number of calls to service.SendPendingHeadlessAuthentication") } type mockTSHDEventsService struct { diff --git a/lib/teleterm/daemon/hardwarekeyprompt.go b/lib/teleterm/daemon/hardwarekeyprompt.go index 4c8b5838a3b01..ad75ccf6f699b 100644 --- a/lib/teleterm/daemon/hardwarekeyprompt.go +++ b/lib/teleterm/daemon/hardwarekeyprompt.go @@ -30,6 +30,22 @@ import ( // NewHardwareKeyPromptConstructor returns a new hardware key prompt constructor // for this service and the given root cluster URI. +// +// TODO(gzdunek): Improve multi-cluster and multi-hardware keys support. +// The code in yubikey.go doesn't really support using multiple hardware keys (like one per cluster): +// 1. We don't offer a choice which key should be used on the initial login. +// 2. Keys are cached per slot, not per physical key - it's not possible to use different keys with the same slot. +// +// Additionally, using the same hardware key for two clusters is not ideal too. +// Since we cache the keys per slot, if two clusters specify the same one, +// the user will always see the prompt for the same cluster URI. For example, if you are logged into both +// cluster-a and cluster-b, the prompt will always say "Unlock hardware key to access cluster-b." +// It seems that the better option would be to have a prompt per physical key, not per cluster. +// But I will leave that for the future, it's hard to say how common these scenarios will be in Connect. +// +// Because the code in yubikey.go assumes you use a single key, we don't have any mutex here. +// (unlike other modals triggered by tshd). +// We don't expect receiving prompts from different hardware keys. func (s *Service) NewHardwareKeyPromptConstructor(rootClusterURI uri.ResourceURI) keys.HardwareKeyPrompt { return &hardwareKeyPrompter{s: s, rootClusterURI: rootClusterURI} } @@ -41,10 +57,6 @@ type hardwareKeyPrompter struct { // Touch prompts the user to touch the hardware key. func (h *hardwareKeyPrompter) Touch(ctx context.Context) error { - if err := h.s.importantModalSemaphore.Acquire(ctx); err != nil { - return trace.Wrap(err) - } - defer h.s.importantModalSemaphore.Release() _, err := h.s.tshdEventsClient.PromptHardwareKeyTouch(ctx, &api.PromptHardwareKeyTouchRequest{ RootClusterUri: h.rootClusterURI.String(), }) @@ -56,10 +68,6 @@ func (h *hardwareKeyPrompter) Touch(ctx context.Context) error { // AskPIN prompts the user for a PIN. func (h *hardwareKeyPrompter) AskPIN(ctx context.Context, requirement keys.PINPromptRequirement) (string, error) { - if err := h.s.importantModalSemaphore.Acquire(ctx); err != nil { - return "", trace.Wrap(err) - } - defer h.s.importantModalSemaphore.Release() res, err := h.s.tshdEventsClient.PromptHardwareKeyPIN(ctx, &api.PromptHardwareKeyPINRequest{ RootClusterUri: h.rootClusterURI.String(), PinOptional: requirement == keys.PINOptional, @@ -74,10 +82,6 @@ func (h *hardwareKeyPrompter) AskPIN(ctx context.Context, requirement keys.PINPr // The Electron app prompt must handle default values for PIN and PUK, // preventing the user from submitting empty/default values. func (h *hardwareKeyPrompter) ChangePIN(ctx context.Context) (*keys.PINAndPUK, error) { - if err := h.s.importantModalSemaphore.Acquire(ctx); err != nil { - return nil, trace.Wrap(err) - } - defer h.s.importantModalSemaphore.Release() res, err := h.s.tshdEventsClient.PromptHardwareKeyPINChange(ctx, &api.PromptHardwareKeyPINChangeRequest{ RootClusterUri: h.rootClusterURI.String(), }) @@ -93,10 +97,6 @@ func (h *hardwareKeyPrompter) ChangePIN(ctx context.Context) (*keys.PINAndPUK, e // ConfirmSlotOverwrite asks the user if the slot's private key and certificate can be overridden. func (h *hardwareKeyPrompter) ConfirmSlotOverwrite(ctx context.Context, message string) (bool, error) { - if err := h.s.importantModalSemaphore.Acquire(ctx); err != nil { - return false, trace.Wrap(err) - } - defer h.s.importantModalSemaphore.Release() res, err := h.s.tshdEventsClient.ConfirmHardwareKeySlotOverwrite(ctx, &api.ConfirmHardwareKeySlotOverwriteRequest{ RootClusterUri: h.rootClusterURI.String(), Message: message, diff --git a/lib/teleterm/daemon/mfaprompt.go b/lib/teleterm/daemon/mfaprompt.go index abdcd3dc2089c..adeda73e1518b 100644 --- a/lib/teleterm/daemon/mfaprompt.go +++ b/lib/teleterm/daemon/mfaprompt.go @@ -63,10 +63,8 @@ func (s *Service) NewMFAPrompt(resourceURI uri.ResourceURI, cfg *libmfa.PromptCo } func (s *Service) promptAppMFA(ctx context.Context, in *api.PromptMFARequest) (*api.PromptMFAResponse, error) { - if err := s.importantModalSemaphore.Acquire(ctx); err != nil { - return nil, trace.Wrap(err) - } - defer s.importantModalSemaphore.Release() + s.mfaMu.Lock() + defer s.mfaMu.Unlock() return s.tshdEventsClient.PromptMFA(ctx, in) } diff --git a/web/packages/design/src/DialogConfirmation/DialogConfirmation.story.jsx b/web/packages/design/src/DialogConfirmation/DialogConfirmation.story.tsx similarity index 97% rename from web/packages/design/src/DialogConfirmation/DialogConfirmation.story.jsx rename to web/packages/design/src/DialogConfirmation/DialogConfirmation.story.tsx index 5c8ea7fc371df..51e5e25ed7b8a 100644 --- a/web/packages/design/src/DialogConfirmation/DialogConfirmation.story.jsx +++ b/web/packages/design/src/DialogConfirmation/DialogConfirmation.story.tsx @@ -16,8 +16,6 @@ * along with this program. If not, see . */ -import React from 'react'; - import { ButtonPrimary } from '../Button'; import DialogConfirmation, { diff --git a/web/packages/design/src/DialogConfirmation/DialogConfirmation.test.jsx b/web/packages/design/src/DialogConfirmation/DialogConfirmation.test.tsx similarity index 92% rename from web/packages/design/src/DialogConfirmation/DialogConfirmation.test.jsx rename to web/packages/design/src/DialogConfirmation/DialogConfirmation.test.tsx index a1974fe1fc247..4887037fd3974 100644 --- a/web/packages/design/src/DialogConfirmation/DialogConfirmation.test.jsx +++ b/web/packages/design/src/DialogConfirmation/DialogConfirmation.test.tsx @@ -16,11 +16,9 @@ * along with this program. If not, see . */ -import React from 'react'; - import { render, fireEvent } from 'design/utils/testing'; -import DialogConfirmation from './DialogConfirmation'; +import { DialogConfirmation } from './DialogConfirmation'; test('onClose is respected', () => { const onClose = jest.fn(); diff --git a/web/packages/design/src/DialogConfirmation/DialogConfirmation.jsx b/web/packages/design/src/DialogConfirmation/DialogConfirmation.tsx similarity index 52% rename from web/packages/design/src/DialogConfirmation/DialogConfirmation.jsx rename to web/packages/design/src/DialogConfirmation/DialogConfirmation.tsx index f15f1b5c8604c..80b0c33ead6d7 100644 --- a/web/packages/design/src/DialogConfirmation/DialogConfirmation.jsx +++ b/web/packages/design/src/DialogConfirmation/DialogConfirmation.tsx @@ -16,22 +16,36 @@ * along with this program. If not, see . */ -import React from 'react'; +import { ReactNode } from 'react'; +import { StyleFunction } from 'styled-components'; import Dialog from 'design/Dialog'; -function DialogConfirmation(props) { - const { children, open, onClose, dialogCss } = props; +export function DialogConfirmation(props: { + open: boolean; + /** + * Prevent unmounting the component and its children from the DOM when closed. + * Instead, hides it with CSS. + */ + keepInDOMAfterClose?: boolean; + /** @deprecated This props has no effect, it was never passed down to `Dialog`. */ + disableEscapeKeyDown?: boolean; + children?: ReactNode; + onClose?: ( + event: KeyboardEvent | React.MouseEvent, + reason: 'escapeKeyDown' | 'backdropClick' + ) => void; + dialogCss?: StyleFunction; +}) { return ( - {children} + {props.children} ); } - -export default DialogConfirmation; diff --git a/web/packages/design/src/DialogConfirmation/index.js b/web/packages/design/src/DialogConfirmation/index.ts similarity index 94% rename from web/packages/design/src/DialogConfirmation/index.js rename to web/packages/design/src/DialogConfirmation/index.ts index 4a2b0c46d6057..62a4d56409401 100644 --- a/web/packages/design/src/DialogConfirmation/index.js +++ b/web/packages/design/src/DialogConfirmation/index.ts @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import DialogConfirmation from './DialogConfirmation'; +import { DialogConfirmation } from './DialogConfirmation'; import { DialogTitle, DialogContent, diff --git a/web/packages/design/src/Modal/Modal.test.tsx b/web/packages/design/src/Modal/Modal.test.tsx index 0c0e33aa965db..09a63cdfc9df3 100644 --- a/web/packages/design/src/Modal/Modal.test.tsx +++ b/web/packages/design/src/Modal/Modal.test.tsx @@ -36,128 +36,144 @@ const escapeKey = { key: 'Escape', }; -describe('design/Modal', () => { - it('respects open prop set to false', () => { - render( - -
Hello
- - ); +test('respects open prop set to false', () => { + render( + +
Hello
+ + ); - expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); +}); + +test('respects onBackdropClick prop', () => { + const mockFn = jest.fn(); + + renderModal({ + onBackdropClick: mockFn, }); - it('respects onBackdropClick prop', () => { - const mockFn = jest.fn(); + // handlebackdropClick + fireEvent.click(screen.getByTestId('backdrop')); + expect(mockFn).toHaveBeenCalled(); +}); - renderModal({ - onBackdropClick: mockFn, - }); +test('respects onEscapeKeyDown props', () => { + const mockFn = jest.fn(); - // handlebackdropClick - fireEvent.click(screen.getByTestId('backdrop')); - expect(mockFn).toHaveBeenCalled(); + const { container } = renderModal({ + onEscapeKeyDown: mockFn, }); - it('respects onEscapeKeyDown props', () => { - const mockFn = jest.fn(); + // handleDocumentKeyDown + fireEvent.keyDown(container, escapeKey); + expect(mockFn).toHaveBeenCalled(); +}); - const { container } = renderModal({ - onEscapeKeyDown: mockFn, - }); +test('respects onClose prop', () => { + const mockFn = jest.fn(); - // handleDocumentKeyDown - fireEvent.keyDown(container, escapeKey); - expect(mockFn).toHaveBeenCalled(); + const { container } = renderModal({ + onClose: mockFn, }); - it('respects onClose prop', () => { - const mockFn = jest.fn(); - - const { container } = renderModal({ - onClose: mockFn, - }); + // handlebackdropClick + fireEvent.click(screen.getByTestId('backdrop')); + expect(mockFn).toHaveBeenCalled(); - // handlebackdropClick - fireEvent.click(screen.getByTestId('backdrop')); - expect(mockFn).toHaveBeenCalled(); + // handleDocumentKeyDown + fireEvent.keyDown(container, escapeKey); + expect(mockFn).toHaveBeenCalled(); +}); - // handleDocumentKeyDown - fireEvent.keyDown(container, escapeKey); - expect(mockFn).toHaveBeenCalled(); +test('respects hideBackDrop prop', () => { + renderModal({ + hideBackdrop: true, }); - it('respects hideBackDrop prop', () => { - renderModal({ - hideBackdrop: true, - }); + expect(screen.queryByTestId('backdrop')).not.toBeInTheDocument(); +}); - expect(screen.queryByTestId('backdrop')).not.toBeInTheDocument(); +test('respects disableBackdropClick prop', () => { + const mockFn = jest.fn(); + renderModal({ + disableBackdropClick: true, + onClose: mockFn, }); - it('respects disableBackdropClick prop', () => { - const mockFn = jest.fn(); - renderModal({ - disableBackdropClick: true, - onClose: mockFn, - }); + // handleBackdropClick + fireEvent.click(screen.getByTestId('backdrop')); + expect(mockFn).not.toHaveBeenCalled(); +}); - // handleBackdropClick - fireEvent.click(screen.getByTestId('backdrop')); - expect(mockFn).not.toHaveBeenCalled(); +test('respects disableEscapeKeyDown prop', () => { + const mockFn = jest.fn(); + const { container } = renderModal({ + disableEscapeKeyDown: true, + onClose: mockFn, }); - it('respects disableEscapeKeyDown prop', () => { - const mockFn = jest.fn(); - const { container } = renderModal({ - disableEscapeKeyDown: true, - onClose: mockFn, - }); + // handleDocumentKeyDown + fireEvent.keyDown(container, escapeKey); + expect(mockFn).not.toHaveBeenCalled(); +}); - // handleDocumentKeyDown - fireEvent.keyDown(container, escapeKey); - expect(mockFn).not.toHaveBeenCalled(); +test('unmount cleans up event listeners and closes modal', () => { + const mockFn = jest.fn(); + const { container, unmount } = renderModal({ + onEscapeKeyDown: mockFn, }); - test('unmount cleans up event listeners and closes modal', () => { - const mockFn = jest.fn(); - const { container, unmount } = renderModal({ - onEscapeKeyDown: mockFn, - }); + unmount(); - unmount(); + expect(screen.queryByTestId('Modal')).not.toBeInTheDocument(); - expect(screen.queryByTestId('Modal')).not.toBeInTheDocument(); + fireEvent.keyDown(container, escapeKey); + expect(mockFn).not.toHaveBeenCalled(); +}); - fireEvent.keyDown(container, escapeKey); - expect(mockFn).not.toHaveBeenCalled(); +test('respects backdropProps prop invisible', () => { + renderModal({ + BackdropProps: { invisible: true }, }); - test('respects backdropProps prop invisible', () => { - renderModal({ - BackdropProps: { invisible: true }, - }); - - expect(screen.getByTestId('backdrop')).toHaveStyle({ - 'background-color': 'transparent', - }); + expect(screen.getByTestId('backdrop')).toHaveStyle({ + 'background-color': 'transparent', }); +}); - it('restores focus on close', async () => { - const user = userEvent.setup(); - render(); - const toggleModalButton = screen.getByRole('button', { name: 'Toggle' }); +test('restores focus on close', async () => { + const user = userEvent.setup(); + render(); + const toggleModalButton = screen.getByRole('button', { name: 'Toggle' }); - await user.click(toggleModalButton); - // Type in the input inside the modal to shift focus into another element. - const input = screen.getByLabelText('Input in modal'); - await user.type(input, 'a'); + await user.click(toggleModalButton); + // Type in the input inside the modal to shift focus into another element. + const input = screen.getByLabelText('Input in modal'); + await user.type(input, 'a'); - const closeModal = screen.getByRole('button', { name: 'Close modal' }); - await user.click(closeModal); + const closeModal = screen.getByRole('button', { name: 'Close modal' }); + await user.click(closeModal); - expect(toggleModalButton).toHaveFocus(); - }); + expect(toggleModalButton).toHaveFocus(); +}); + +test('closing dialog when attachCustomKeyEventHandler is set only hides it with DOM', async () => { + const { rerender } = render( + +
Hello
+ + ); + + expect(screen.getByText('Hello')).toBeVisible(); + + rerender( + +
Hello
+ + ); + + expect(screen.getByText('Hello')).not.toBeVisible(); }); const ToggleableModal = () => { diff --git a/web/packages/design/src/Modal/Modal.tsx b/web/packages/design/src/Modal/Modal.tsx index e93085d426e7f..99eaf6f2b286d 100644 --- a/web/packages/design/src/Modal/Modal.tsx +++ b/web/packages/design/src/Modal/Modal.tsx @@ -25,6 +25,11 @@ export type ModalProps = { * If `true`, the modal is open. */ open: boolean; + /** + * Prevent unmounting the component and its children from the DOM when closed. + * Instead, hides it with CSS. + */ + keepInDOMAfterClose?: boolean; className?: string; @@ -200,15 +205,23 @@ export default class Modal extends React.Component { } render() { - const { BackdropProps, children, modalCss, hideBackdrop, open, className } = - this.props; - - if (!open) { + const { + BackdropProps, + children, + modalCss, + hideBackdrop, + open, + className, + keepInDOMAfterClose, + } = this.props; + + if (!open && !keepInDOMAfterClose) { return null; } return createPortal( ` const StyledModal = styled.div<{ modalCss?: StyleFunction; ref: React.RefObject; + hiddenInDom?: boolean; }>` position: fixed; z-index: 1200; @@ -268,5 +282,6 @@ const StyledModal = styled.div<{ bottom: 0; top: 0; left: 0; + ${props => props.hiddenInDom && `display: none;`}; ${props => props.modalCss?.(props)} `; diff --git a/web/packages/teleterm/src/ui/ClusterConnect/ClusterConnect.tsx b/web/packages/teleterm/src/ui/ClusterConnect/ClusterConnect.tsx index 5a88fd2f71a2c..9a3f5cac5c54b 100644 --- a/web/packages/teleterm/src/ui/ClusterConnect/ClusterConnect.tsx +++ b/web/packages/teleterm/src/ui/ClusterConnect/ClusterConnect.tsx @@ -28,7 +28,10 @@ import { ClusterAdd } from './ClusterAdd'; import { ClusterLogin } from './ClusterLogin'; import { dialogCss } from './spacing'; -export function ClusterConnect(props: { dialog: DialogClusterConnect }) { +export function ClusterConnect(props: { + dialog: DialogClusterConnect; + hidden?: boolean; +}) { const [createdClusterUri, setCreatedClusterUri] = useState< RootClusterUri | undefined >(); @@ -49,7 +52,8 @@ export function ClusterConnect(props: { dialog: DialogClusterConnect }) { dialogCss={dialogCss} disableEscapeKeyDown={false} onClose={props.dialog.onCancel} - open={true} + open={!props.hidden} + keepInDOMAfterClose > {!clusterUri ? ( ({ maxWidth: '400px', diff --git a/web/packages/teleterm/src/ui/DocumentsReopen/DocumentsReopen.tsx b/web/packages/teleterm/src/ui/DocumentsReopen/DocumentsReopen.tsx index 7e9e5d3603a57..33d17aeb959a3 100644 --- a/web/packages/teleterm/src/ui/DocumentsReopen/DocumentsReopen.tsx +++ b/web/packages/teleterm/src/ui/DocumentsReopen/DocumentsReopen.tsx @@ -31,14 +31,13 @@ import { P } from 'design/Text/Text'; import { RootClusterUri, routing } from 'teleterm/ui/uri'; import { useAppContext } from 'teleterm/ui/appContextProvider'; -interface DocumentsReopenProps { +export function DocumentsReopen(props: { rootClusterUri: RootClusterUri; numberOfDocuments: number; onCancel(): void; onConfirm(): void; -} - -export function DocumentsReopen(props: DocumentsReopenProps) { + hidden?: boolean; +}) { const { rootClusterUri } = props; const { clustersService } = useAppContext(); // TODO(ravicious): Use a profile name here from the URI and remove the dependency on @@ -49,7 +48,8 @@ export function DocumentsReopen(props: DocumentsReopenProps) { return ( ({ maxWidth: '400px', diff --git a/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx b/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx index d395dd8f9f03d..1cb1a0e5e20c4 100644 --- a/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx +++ b/web/packages/teleterm/src/ui/HeadlessAuthn/HeadlessAuthentication.tsx @@ -36,6 +36,7 @@ interface HeadlessAuthenticationProps { skipConfirm: boolean; onCancel(): void; onSuccess(): void; + hidden?: boolean; } export function HeadlessAuthentication(props: HeadlessAuthenticationProps) { @@ -81,6 +82,7 @@ export function HeadlessAuthentication(props: HeadlessAuthenticationProps) { return (