Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent activation of previous workspace when launching Connect via deep link to a different cluster #50063

Merged
merged 23 commits into from
Dec 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
51c37e9
Refactor `setActiveWorkspace` to async/await
gzdunek Dec 10, 2024
f43b0ae
Keep all the logic that restores a single workspace state in `getWork…
gzdunek Dec 10, 2024
dc70bdf
Separate restoring state from setting active workspace
gzdunek Dec 10, 2024
a6bb953
Allow the dialog to reopen documents to be closed without any decision
gzdunek Dec 11, 2024
e941385
Add a test that verifies if the correct workspace is activated
gzdunek Dec 11, 2024
9fc2003
Docs improvements
gzdunek Dec 13, 2024
cc2644a
Return early if there's no restoredWorkspace
gzdunek Dec 13, 2024
3dde323
Fix logger name
gzdunek Dec 13, 2024
8c4dbec
Improve test name
gzdunek Dec 13, 2024
8776788
Make restored state immutable
gzdunek Dec 16, 2024
f5be332
Fix comment
gzdunek Dec 17, 2024
35194aa
Do not wait for functions to be called in tests
gzdunek Dec 17, 2024
706b0f0
Add tests for discarding documents reopen dialog
gzdunek Dec 17, 2024
4bb34e3
Move setting `isInitialized` to a separate method
gzdunek Dec 17, 2024
23f82b8
Remove restored workspace when logging out so that we won't try to re…
gzdunek Dec 17, 2024
26c1dac
Do not try to restore previous documents if the user already opened n…
gzdunek Dec 17, 2024
9e810c1
Do not close dialog in test
gzdunek Dec 20, 2024
0753223
Improve `isInitialized` comment
gzdunek Dec 20, 2024
e275bbb
Call `setActiveWorkspace` again when we fail to change the workspace
gzdunek Dec 20, 2024
e339d09
Fix the logic around restoring previous documents
gzdunek Dec 20, 2024
c20ec38
Try to reopen documents also after `cluster-connect` is canceled
gzdunek Dec 30, 2024
a9bec27
canRestoreDocuments -> hasDocumentsToReopen
gzdunek Dec 30, 2024
aef61d0
Disallow parallel `setActiveWorkspace` calls (#50384)
gzdunek Dec 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions web/packages/build/jest/jest-environment-patched-jsdom.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ export default class PatchedJSDOMEnvironment extends JSDOMEnvironment {
if (!global.TransformStream) {
global.TransformStream = TransformStream;
}
// TODO(gzdunek): JSDOM doesn't support AbortSignal.any().
// Overwriting only this function doesn't help much, something between
// AbortSignal and AbortController is missing.
if (!global.AbortSignal.any) {
global.AbortSignal = AbortSignal;
global.AbortController = AbortController;
}
}
}
export const TestEnvironment = PatchedJSDOMEnvironment;
45 changes: 45 additions & 0 deletions web/packages/teleterm/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,48 @@ resource availability as possible.
### PTY communication overview (Renderer Process <=> Shared Process)

![PTY communication](docs/ptyCommunication.png)

### Overview of a deep link launch process

The diagram below illustrates the process of launching a deep link,
depending on the state of the workspaces.
It assumes that the app is not running and that the deep link targets a workspace
different from the persisted one.

<details>
<summary>Diagram</summary>

```mermaid
flowchart TD
Start([Start]) --> IsPreviousWorkspaceConnected{Is the previously active workspace connected?}
IsPreviousWorkspaceConnected --> |Valid certificate| PreviousWorkspaceReopenDocuments{Has documents to reopen from the previous workspace?}
IsPreviousWorkspaceConnected --> |Expired certificate| CancelPreviousWorkspaceLogin[Cancel the login dialog]
IsPreviousWorkspaceConnected --> |No persisted workspace| SwitchWorkspace

PreviousWorkspaceReopenDocuments --> |Yes| CancelPreviousWorkspaceDocumentsReopen[Cancel the reopen dialog without discarding documents]
PreviousWorkspaceReopenDocuments --> |No| SwitchWorkspace[Switch to a deep link workspace]

CancelPreviousWorkspaceDocumentsReopen --> SwitchWorkspace
CancelPreviousWorkspaceLogin --> SwitchWorkspace

SwitchWorkspace --> IsDeepLinkWorkspaceConnected{Is the deep link workspace connected?}
IsDeepLinkWorkspaceConnected --> |Valid certificate| DeepLinkWorkspaceReopenDocuments{Has documents to reopen from the deep link workspace?}
IsDeepLinkWorkspaceConnected --> |Not added| AddDeepLinkCluster[Add new cluster]
IsDeepLinkWorkspaceConnected --> |Expired certificate| LogInToDeepLinkWorkspace[Log in to workspace]

AddDeepLinkCluster --> AddDeepLinkClusterSuccess{Was the cluster added successfully?}
AddDeepLinkClusterSuccess --> |Yes| LogInToDeepLinkWorkspace
AddDeepLinkClusterSuccess --> |No| ReturnToPreviousWorkspace[Return to the previously active workspace and try to reopen its documents again]

LogInToDeepLinkWorkspace --> IsLoginToDeepLinkWorkspaceSuccess{Was login successful?}
IsLoginToDeepLinkWorkspaceSuccess --> |Yes| DeepLinkWorkspaceReopenDocuments
IsLoginToDeepLinkWorkspaceSuccess --> |No| ReturnToPreviousWorkspace

DeepLinkWorkspaceReopenDocuments --> |Yes| ReopenDeepLinkWorkspaceDocuments[Reopen documents]
DeepLinkWorkspaceReopenDocuments --> |No| End

ReopenDeepLinkWorkspaceDocuments --> End
ReturnToPreviousWorkspace --> End
```

</details>
246 changes: 246 additions & 0 deletions web/packages/teleterm/src/ui/AppInitializer/AppInitializer.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,246 @@
/**
* Teleport
* Copyright (C) 2024 Gravitational, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import 'jest-canvas-mock';
import { render } from 'design/utils/testing';
import { screen, act } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { mockIntersectionObserver } from 'jsdom-testing-mocks';

import { MockAppContext } from 'teleterm/ui/fixtures/mocks';
import { makeRootCluster } from 'teleterm/services/tshd/testHelpers';
import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider';
import { ConnectionsContextProvider } from 'teleterm/ui/TopBar/Connections/connectionsContext';
import { VnetContextProvider } from 'teleterm/ui/Vnet';
import Logger, { NullService } from 'teleterm/logger';
import { MockedUnaryCall } from 'teleterm/services/tshd/cloneableClient';
import { ResourcesContextProvider } from 'teleterm/ui/DocumentCluster/resourcesContext';
import { IAppContext } from 'teleterm/ui/types';

import { AppInitializer } from './AppInitializer';

mockIntersectionObserver();
beforeAll(() => {
Logger.init(new NullService());
});

jest.mock('teleterm/ui/ClusterConnect', () => ({
ClusterConnect: props => (
<div
data-testid="mocked-dialog"
data-dialog-kind="cluster-connect"
data-dialog-is-hidden={props.hidden}
>
Connect to {props.dialog.clusterUri}
<button onClick={props.dialog.onSuccess}>Connect to cluster</button>
</div>
),
}));

test('activating a workspace via deep link overrides the previously active workspace', async () => {
// Before closing the app, both clusters were present in the state, with previouslyActiveCluster being active.
// However, the user clicked a deep link pointing to deepLinkCluster.
// The app should prioritize the user's intent by activating the workspace for the deep link,
// rather than reactivating the previously active cluster.
const previouslyActiveCluster = makeRootCluster({
uri: '/clusters/teleport-previously-active',
proxyHost: 'teleport-previously-active:3080',
name: 'teleport-previously-active',
connected: false,
});
const deepLinkCluster = makeRootCluster({
uri: '/clusters/teleport-deep-link',
proxyHost: 'teleport-deep-link:3080',
name: 'teleport-deep-link',
connected: false,
});
const appContext = new MockAppContext();
jest
.spyOn(appContext.statePersistenceService, 'getWorkspacesState')
.mockReturnValue({
rootClusterUri: previouslyActiveCluster.uri,
workspaces: {
[previouslyActiveCluster.uri]: {
localClusterUri: previouslyActiveCluster.uri,
documents: [],
location: undefined,
},
[deepLinkCluster.uri]: {
localClusterUri: deepLinkCluster.uri,
documents: [],
location: undefined,
},
},
});
appContext.mainProcessClient.configService.set(
'usageReporting.enabled',
false
);
jest.spyOn(appContext.tshd, 'listRootClusters').mockReturnValue(
new MockedUnaryCall({
clusters: [deepLinkCluster, previouslyActiveCluster],
})
);

render(
<MockAppContextProvider appContext={appContext}>
<ConnectionsContextProvider>
<VnetContextProvider>
<ResourcesContextProvider>
<AppInitializer />
</ResourcesContextProvider>
</VnetContextProvider>
</ConnectionsContextProvider>
</MockAppContextProvider>
);

expect(
await screen.findByText(`Connect to ${previouslyActiveCluster.uri}`)
).toBeInTheDocument();

// Launch a deep link and do not wait for the result.
act(() => {
void appContext.deepLinksService.launchDeepLink({
status: 'success',
url: {
host: deepLinkCluster.proxyHost,
hostname: deepLinkCluster.name,
port: '1234',
pathname: '/authenticate_web_device',
username: deepLinkCluster.loggedInUser.name,
searchParams: {
id: '123',
redirect_uri: '',
token: 'abc',
},
},
});
});

// The previous dialog has been replaced without a user interaction.
// In the real app, this happens fast enough that the user doesn't see the previous dialog.
expect(
await screen.findByText(`Connect to ${deepLinkCluster.uri}`)
).toBeInTheDocument();

// We confirm the current cluster-connect dialog.
const dialogSuccessButton = await screen.findByRole('button', {
name: 'Connect to cluster',
});
await userEvent.click(dialogSuccessButton);

// Check if the first activated workspace is the one from the deep link.
expect(await screen.findByTitle(/Current cluster:/)).toBeVisible();
expect(
screen.queryByTitle(`Current cluster: ${deepLinkCluster.name}`)
).toBeVisible();
});

test.each<{
name: string;
action(appContext: IAppContext): Promise<void>;
expectHasDocumentsToReopen: boolean;
}>([
{
name: 'closing documents reopen dialog via close button discards previous documents',
action: async () => {
await userEvent.click(await screen.findByTitle('Close'));
},
expectHasDocumentsToReopen: false,
},
{
name: 'starting new session in document reopen dialog discards previous documents',
action: async () => {
await userEvent.click(
await screen.findByRole('button', { name: 'Start New Session' })
);
},
expectHasDocumentsToReopen: false,
},
{
name: 'overwriting document reopen dialog with another regular dialog does not discard documents',
action: async appContext => {
act(() => {
appContext.modalsService.openRegularDialog({
kind: 'change-access-request-kind',
onConfirm() {},
onCancel() {},
});
});
},
expectHasDocumentsToReopen: true,
},
])('$name', async testCase => {
const rootCluster = makeRootCluster();
const appContext = new MockAppContext();
jest
.spyOn(appContext.statePersistenceService, 'getWorkspacesState')
.mockReturnValue({
rootClusterUri: rootCluster.uri,
workspaces: {
[rootCluster.uri]: {
localClusterUri: rootCluster.uri,
documents: [
{
kind: 'doc.access_requests',
uri: '/docs/123',
state: 'browsing',
clusterUri: rootCluster.uri,
requestId: '',
title: 'Access Requests',
},
],
location: undefined,
},
},
});
appContext.mainProcessClient.configService.set(
'usageReporting.enabled',
false
);
jest.spyOn(appContext.tshd, 'listRootClusters').mockReturnValue(
new MockedUnaryCall({
clusters: [rootCluster],
})
);

render(
<MockAppContextProvider appContext={appContext}>
<ConnectionsContextProvider>
<VnetContextProvider>
<ResourcesContextProvider>
<AppInitializer />
</ResourcesContextProvider>
</VnetContextProvider>
</ConnectionsContextProvider>
</MockAppContextProvider>
);

expect(
await screen.findByText(
'Do you want to reopen tabs from the previous session?'
)
).toBeInTheDocument();

await testCase.action(appContext);

expect(
appContext.workspacesService.getWorkspace(rootCluster.uri)
.hasDocumentsToReopen
).toBe(testCase.expectHasDocumentsToReopen);
});
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ export const AppInitializer = () => {
await appContext.pullInitialState();
setShouldShowUi(true);
await showStartupModalsAndNotifications(appContext);
// If there's a workspace that was active before closing the app,
// activate it.
const rootClusterUri =
appContext.workspacesService.getRestoredState()?.rootClusterUri;
if (rootClusterUri) {
void appContext.workspacesService.setActiveWorkspace(rootClusterUri);
}
appContext.mainProcessClient.signalUserInterfaceReadiness({
success: true,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ export async function showStartupModalsAndNotifications(
// "User job role" dialog is shown on the second launch (only if user agreed to reporting earlier).
await setUpUsageReporting(configService, ctx.modalsService);

// If there's a workspace that was active before the user closed the app, restorePersistedState
// will block until the user interacts with the login modal (if the cert is not valid anymore) and
// the modal for restoring documents.
await ctx.workspacesService.restorePersistedState();

notifyAboutConfigErrors(configService, ctx.notificationsService);
notifyAboutDuplicatedShortcutsCombinations(
ctx.keyboardShortcutsService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const Story = () => {
rootClusterUri="/clusters/foo.cloud.gravitational.io"
numberOfDocuments={8}
onConfirm={() => {}}
onCancel={() => {}}
onDiscard={() => {}}
/>
</MockAppContextProvider>
);
Expand All @@ -46,7 +46,7 @@ export const OneTab = () => {
rootClusterUri="/clusters/foo.cloud.gravitational.io"
numberOfDocuments={1}
onConfirm={() => {}}
onCancel={() => {}}
onDiscard={() => {}}
/>
</MockAppContextProvider>
);
Expand All @@ -59,7 +59,7 @@ export const LongClusterName = () => {
rootClusterUri="/clusters/foo.bar.baz.quux.cloud.gravitational.io"
numberOfDocuments={42}
onConfirm={() => {}}
onCancel={() => {}}
onDiscard={() => {}}
/>
</MockAppContextProvider>
);
Expand All @@ -75,7 +75,7 @@ export const LongContinuousClusterName = () => {
.join('')}`}
numberOfDocuments={680}
onConfirm={() => {}}
onCancel={() => {}}
onDiscard={() => {}}
/>
</MockAppContextProvider>
);
Expand Down
Loading
Loading