From 047176ef68334b5337e50cf2c4440dfb0b2d0d85 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Fri, 15 May 2026 10:23:44 +0200 Subject: [PATCH] Connect: remember clusters after logout (#66318) * Separate logging out and forgetting profile * Store `proxyHost` in the workspace and decouple workspace existence from profile existence * Display workspaces instead of profiles in identity list * Only activate workspace on launch when cluster exists * Handle cluster disappearing when `ClusterLogin` is open * Fix e2e tests * Add comments * Clean up rendering workspaces * Make test name more obvious * Make workspace state the source of truth for activation * Adjust callsites to updated `addWorkspace` and `setActiveWorkspace` behavior * Add `makeWorkspace` and `makePersistedWorkspace` test helpers * Fix mistakenly passed `cluster.uri` instead of `cluster.proxyHost` * Read `activeClusterUri` from `useStoreSelector` * Update `proxyHost` in workspace whenever it changes in profile * Use `proxyHost` in notification * Introduce `rootClusterProxyHost` * Get rid of `getActiveRootCluster()` * Rename `identityItems` * Improve comment for `setActiveWorkspace` init action * Fix `Identity` story * Previously used -> Saved in Connect * Clear app updates managing cluster only when forgetting cluster * Wrap `hasAnyWorkspaces` in boolean * Expired session -> Session expired, Saved in Connect -> In history * Fix e2e test --- e2e/helpers/connect.ts | 15 +- e2e/tests/connect/auth.spec.ts | 7 +- e2e/tests/connect/stateRestoration.spec.ts | 15 +- .../clusterLifecycleManager.test.ts | 32 +- .../clusterLifecycleManager.ts | 117 +++++--- .../clusterStore/clusterStore.test.ts | 59 ++-- .../mainProcess/clusterStore/clusterStore.ts | 24 +- .../src/mainProcess/fixtures/mocks.ts | 1 + .../teleterm/src/mainProcess/mainProcess.ts | 8 +- .../src/mainProcess/mainProcessClient.ts | 3 + .../teleterm/src/mainProcess/types.ts | 2 + .../teleterm/src/services/tshd/testHelpers.ts | 3 +- .../useAccessRequestCheckout.test.tsx | 5 + web/packages/teleterm/src/ui/App.test.tsx | 13 +- .../src/ui/AppInitializer/AppInitializer.tsx | 9 +- .../ClusterLogin/useClusterLogin.ts | 2 +- .../src/ui/DeepLinks/launchDeepLink.test.ts | 3 + .../useDocumentTerminal.test.tsx | 1 + .../src/ui/Documents/DocumentsRenderer.tsx | 25 +- .../ClusterConnectPanel.tsx | 54 ++-- .../src/ui/TopBar/Identity/Identity.story.tsx | 8 +- .../src/ui/TopBar/Identity/Identity.test.tsx | 87 +++++- .../src/ui/TopBar/Identity/Identity.tsx | 38 ++- .../Identity/IdentityList/IdentityList.tsx | 50 ++-- .../IdentityList/IdentityListItem.tsx | 126 +++++--- .../IdentitySelector/IdentitySelector.tsx | 2 +- .../teleterm/src/ui/TopBar/Identity/index.ts | 2 + .../src/ui/TopBar/Identity/useIdentity.ts | 65 ++-- web/packages/teleterm/src/ui/appContext.ts | 20 +- .../teleterm/src/ui/commandLauncher.ts | 13 +- .../teleterm/src/ui/fixtures/mocks.ts | 2 +- .../ui/services/clusters/clustersService.ts | 4 + .../connectionTrackerService.legacy.test.ts | 5 +- .../connectionTrackerService.ts | 6 +- .../statePersistenceService.ts | 1 + .../documentsService/connectToApp.test.ts | 1 + .../documentsService/connectToApp.ts | 6 +- .../connectToDatabase.test.ts | 2 + .../documentsService/connectToDatabase.ts | 6 +- .../documentsService/connectToKube.ts | 6 +- .../documentsService/connectToServer.ts | 6 +- .../connectToWindowsDesktop.ts | 6 +- .../services/workspacesService/testHelpers.ts | 65 ++++ .../workspacesService.test.ts | 282 +++++++++++++----- .../workspacesService/workspacesService.ts | 209 +++++++++---- 45 files changed, 1053 insertions(+), 363 deletions(-) create mode 100644 web/packages/teleterm/src/ui/services/workspacesService/testHelpers.ts diff --git a/e2e/helpers/connect.ts b/e2e/helpers/connect.ts index d800649f217..b7f662447db 100644 --- a/e2e/helpers/connect.ts +++ b/e2e/helpers/connect.ts @@ -90,7 +90,20 @@ export async function login( if (!password) { throw new Error('login: password required when username is provided'); } - await page.getByRole('button', { name: 'Connect', exact: true }).click(); + const connectButton = page.getByRole('button', { + name: 'Connect', + exact: true, + }); + const addClusterItem = page + .getByRole('listitem') + .filter({ hasText: 'Add Cluster' }); + + // The 'Connect' button is visible only when no clusters are present. + if (await connectButton.isVisible()) { + await connectButton.click(); + } else { + await addClusterItem.click(); + } const clusterInput = page.getByPlaceholder('teleport.example.com'); await expect(clusterInput).toBeVisible(); diff --git a/e2e/tests/connect/auth.spec.ts b/e2e/tests/connect/auth.spec.ts index 7768ec5b8c6..3ac9e08456c 100644 --- a/e2e/tests/connect/auth.spec.ts +++ b/e2e/tests/connect/auth.spec.ts @@ -17,6 +17,7 @@ */ import { test, expect } from '@gravitational/e2e/helpers/connect'; +import { startUrl } from '@gravitational/e2e/helpers/env'; test.use({ autoLogin: true }); @@ -28,5 +29,9 @@ test('logging out', async ({ app }) => { page.getByText('Are you sure you want to log out?') ).toBeVisible(); await page.getByRole('button', { name: 'Log Out', exact: true }).click(); - await expect(page.getByText('Connect a Cluster')).toBeVisible(); + const proxyHostname = new URL(startUrl).hostname; + const previouslyUsedCluster = page + .getByRole('listitem') + .filter({ hasText: proxyHostname }); + await expect(previouslyUsedCluster.getByText('Not logged in')).toBeVisible(); }); diff --git a/e2e/tests/connect/stateRestoration.spec.ts b/e2e/tests/connect/stateRestoration.spec.ts index 3f5253843f7..caf5e0af448 100644 --- a/e2e/tests/connect/stateRestoration.spec.ts +++ b/e2e/tests/connect/stateRestoration.spec.ts @@ -28,6 +28,7 @@ import { test, withDefaultAppConfig, } from '@gravitational/e2e/helpers/connect'; +import { startUrl } from '@gravitational/e2e/helpers/env'; // These tests manage the app lifecycle manually (multiple launches/closes), so they do not use the // `app` fixture. @@ -138,12 +139,14 @@ test.describe('state restoration from disk', () => { await using app = await launchApp(temp.path); const { page } = app; - // With no tsh dir, no cluster can be connected. - await expect(page.getByText('Connect a Cluster')).toBeVisible(); - // app_state.json still references the old cluster, so setActiveWorkspace should show an error. await expect( - page.getByText('Could not set cluster as active') + page.getByText('Log in to a cluster to use Teleport Connect') ).toBeVisible(); + const proxyHostname = new URL(startUrl).hostname; + const inHistoryCluster = page + .getByRole('listitem') + .filter({ hasText: proxyHostname }); + await expect(inHistoryCluster.getByText('In history')).toBeVisible(); } }); @@ -166,7 +169,9 @@ test.describe('state restoration from disk', () => { page.getByText('Are you sure you want to log out?') ).toBeVisible(); await page.getByRole('button', { name: 'Log Out', exact: true }).click(); - await expect(page.getByText('Connect a Cluster')).toBeVisible(); + await expect( + page.getByText('Log in to a cluster to use Teleport Connect') + ).toBeVisible(); // Login to the same cluster again. await login(page); diff --git a/web/packages/teleterm/src/mainProcess/clusterLifecycleManager/clusterLifecycleManager.test.ts b/web/packages/teleterm/src/mainProcess/clusterLifecycleManager/clusterLifecycleManager.test.ts index d1acb2ef29d..5f7c8bef8d8 100644 --- a/web/packages/teleterm/src/mainProcess/clusterLifecycleManager/clusterLifecycleManager.test.ts +++ b/web/packages/teleterm/src/mainProcess/clusterLifecycleManager/clusterLifecycleManager.test.ts @@ -113,7 +113,7 @@ const tests: { removeProfile: true, }); expect(rendererHandler.send).toHaveBeenCalledWith({ - op: 'will-logout-and-remove', + op: 'will-logout', uri: cluster.uri, }); }, @@ -235,6 +235,36 @@ const tests: { expect(rendererHandler.send).not.toHaveBeenCalled(); }, }, + { + name: 'when cluster proxy host changes, it updates state and notifies renderer', + setup: async ({ tshdClient }) => { + const next = makeRootCluster({ + connected: false, + proxyHost: 'new.example.com:443', + }); + jest + .spyOn(tshdClient, 'listRootClusters') + .mockResolvedValue(new MockedUnaryCall({ clusters: [cluster] })); + jest.spyOn(tshdClient, 'clearStaleClusterClients'); + return { + profileWatcher: makeWatcher([ + { op: 'changed', next, previous: cluster }, + ]), + }; + }, + expect: ({ clusterStore, tshdClient, rendererHandler }) => { + expect(clusterStore.getState().get(cluster.uri).proxyHost).toBe( + 'new.example.com:443' + ); + expect(tshdClient.clearStaleClusterClients).toHaveBeenCalledWith({ + rootClusterUri: cluster.uri, + }); + expect(rendererHandler.send).toHaveBeenCalledWith({ + op: 'did-change-proxy-host', + uri: cluster.uri, + }); + }, + }, { name: 'when access of logged in user changes, it updates state, clears stale clients, and notifies renderer', setup: async ({ tshdClient }) => { diff --git a/web/packages/teleterm/src/mainProcess/clusterLifecycleManager/clusterLifecycleManager.ts b/web/packages/teleterm/src/mainProcess/clusterLifecycleManager/clusterLifecycleManager.ts index 9c681c4a9d2..29c39b4fd6b 100644 --- a/web/packages/teleterm/src/mainProcess/clusterLifecycleManager/clusterLifecycleManager.ts +++ b/web/packages/teleterm/src/mainProcess/clusterLifecycleManager/clusterLifecycleManager.ts @@ -53,15 +53,18 @@ export interface ClusterLifecycleEvent { * * did-add-cluster - A cluster has been successfully added. * * did-change-access - The logged-in user has changed, or their roles, * or access requests have been updated. - * * will-logout - The user is about to be logged out. - * * will-logout-and-remove - The user is about to be logged out - * and their profile (cluster) will be removed. + * * did-change-proxy-host - The proxy address saved in the profile has changed. + * * will-logout - The user is about to be logged out, but Connect should + * keep the workspace as remembered cluster state. + * * will-forget-cluster - The user is about to forget the cluster in + * Connect. Its workspace and profile will be removed. */ op: | 'did-add-cluster' | 'did-change-access' + | 'did-change-proxy-host' | 'will-logout' - | 'will-logout-and-remove'; + | 'will-forget-cluster'; } export interface ProfileWatcherError { @@ -142,15 +145,44 @@ export class ClusterLifecycleManager { } } - async logoutAndRemoveCluster(uri: RootClusterUri): Promise { - this.logger.info('Logging out and removing cluster', { uri }); + /** + * Logs out from the cluster without removing the tsh profile. + * Session credentials are cleared, but the cluster remains visible in both + * Connect and tsh. + */ + async logoutCluster(uri: RootClusterUri): Promise { + this.logger.info('Logging out cluster', { uri }); try { - await this.sendRendererEvent({ op: 'will-logout-and-remove', uri }); - this.onBeforeRemove(uri); - await this.clusterStore.logoutAndRemove(uri); - this.logger.info('Logged out and removed cluster', { uri }); + await this.sendRendererEvent({ op: 'will-logout', uri }); + await this.clusterStore.logout(uri, { removeProfile: false }); + this.logger.info('Logged out cluster', { uri }); } catch (error) { - this.logger.error('Failed to log out and remove cluster', { uri }, error); + this.logger.error('Failed to log out cluster', { uri }, error); + throw error; + } + } + + /** + * Forgets the cluster in Connect and removes its tsh profile. + * If this cluster was selected to manage app updates, clears that selection. + */ + async forgetCluster(uri: RootClusterUri): Promise { + this.logger.info('Forgetting cluster', { uri }); + try { + await this.sendRendererEvent({ op: 'will-forget-cluster', uri }); + // Clear the managing cluster only when the cluster is forgotten in Connect. + // If the stored managing cluster is missing, auto-update resolution ignores it. + // Once that cluster is added back, the stored selection applies again. + // + // Do not wait for this promise to finish as we don't want to block logout + // on checking app updates. + this.appUpdater.maybeRemoveManagingCluster(uri).catch(error => { + this.logger.error('Failed to remove managing cluster', { uri }, error); + }); + await this.clusterStore.logout(uri, { removeProfile: true }); + this.logger.info('Forgot cluster', { uri }); + } catch (error) { + this.logger.error('Failed to forget cluster', { uri }, error); throw error; } } @@ -159,17 +191,8 @@ export class ClusterLifecycleManager { this.logger.info('Syncing cluster', { uri }); try { const { previous, next } = await this.clusterStore.sync(uri); - const accessChanged = hasAccessChanged( - previous.loggedInUser, - next.loggedInUser - ); - if (accessChanged) { - await this.sendRendererEvent({ - op: 'did-change-access', - uri: next.uri, - }); - } - this.logger.info('Cluster synced', { uri: next.uri, accessChanged }); + await this.emitClusterChangedEvents(previous, next); + this.logger.info('Cluster synced', { uri: next.uri }); } catch (error) { this.logger.error('Failed to sync cluster', { uri }, error); throw error; @@ -192,14 +215,6 @@ export class ClusterLifecycleManager { } } - private onBeforeRemove(uri: RootClusterUri): void { - // Do not wait for this promise to finish as we don't want to block logout - // on checking app updates. - this.appUpdater.maybeRemoveManagingCluster(uri).catch(error => { - this.logger.error('Failed to remove managing cluster', { uri }, error); - }); - } - /** * If the cluster is connected, try to sync it to get the full profile with details. * Otherwise, update the cluster with the profile read from disk. @@ -310,26 +325,21 @@ export class ClusterLifecycleManager { // user logged in again via tsh and the auth server has disconnect_expired_cert enabled. await client.clearStaleClusterClients({ rootClusterUri: next.uri }); await this.syncOrUpdateCluster(next); - - if (!hasAccessChanged(previous.loggedInUser, next.loggedInUser)) { - this.logger.info('Cluster profile changed without access changes', { - uri: next.uri, - }); - return; - } - await this.sendRendererEvent({ - op: 'did-change-access', - uri: next.uri, - }); + await this.emitClusterChangedEvents(previous, next); } private async handleClusterRemoved(cluster: Cluster): Promise { + // When a profile disappears externally (for example by running tsh logout), + // keep the workspace as Connect's remembered cluster state. await this.sendRendererEvent({ - op: 'will-logout-and-remove', + op: 'will-logout', uri: cluster.uri, }); - this.onBeforeRemove(cluster.uri); - await this.clusterStore.logoutAndRemove(cluster.uri); + // Keep the cluster store in sync with the profile state on disk. + // Once the profile is gone, we must remove if from the store too; otherwise Connect + // could try to use it before recreating the profile and RPCs would fail with errors such + // as "~/.tsh/ no such file or directory". + await this.clusterStore.logout(cluster.uri, { removeProfile: true }); } private async handleClusterLogout(cluster: Cluster): Promise { @@ -342,6 +352,25 @@ export class ClusterLifecycleManager { await this.syncOrUpdateCluster(cluster); } + private async emitClusterChangedEvents( + previous: Cluster | undefined, + next: Cluster + ): Promise { + if (previous?.proxyHost !== next.proxyHost) { + await this.sendRendererEvent({ + op: 'did-change-proxy-host', + uri: next.uri, + }); + } + + if (hasAccessChanged(previous?.loggedInUser, next.loggedInUser)) { + await this.sendRendererEvent({ + op: 'did-change-access', + uri: next.uri, + }); + } + } + private handleWatcherError(watcherError: ProfileWatcherError): void { this.logger.info( 'Reporting profile watcher error to renderer', diff --git a/web/packages/teleterm/src/mainProcess/clusterStore/clusterStore.test.ts b/web/packages/teleterm/src/mainProcess/clusterStore/clusterStore.test.ts index 642a3dccf2b..f87b246bc7f 100644 --- a/web/packages/teleterm/src/mainProcess/clusterStore/clusterStore.test.ts +++ b/web/packages/teleterm/src/mainProcess/clusterStore/clusterStore.test.ts @@ -18,6 +18,8 @@ import { enableMapSet, enablePatches } from 'immer'; +import { LoggedInUser } from 'gen-proto-ts/teleport/lib/teleterm/v1/cluster_pb'; + import { MockedUnaryCall } from 'teleterm/services/tshd/cloneableClient'; import { MockTshClient } from 'teleterm/services/tshd/fixtures/mocks'; import { @@ -78,14 +80,7 @@ test('adding a cluster does not overwrite an existing one', async () => { }); test('syncs cluster', async () => { - const mockClient = new MockTshClient(); - mockClient.getCluster = () => new MockedUnaryCall(clusterWithDetails); - mockClient.listLeafClusters = () => - new MockedUnaryCall({ clusters: [leafCluster] }); - const clusterStore = new ClusterStore( - () => Promise.resolve(mockClient), - mockWindowsManager - ); + const { clusterStore } = getTestSetup(); await clusterStore.sync(cluster.uri); @@ -94,19 +89,32 @@ test('syncs cluster', async () => { expect(state.get(leafCluster.uri)).toStrictEqual(leafCluster); }); -test('logs out of cluster', async () => { - const mockClient = new MockTshClient(); - mockClient.getCluster = () => new MockedUnaryCall(clusterWithDetails); - mockClient.listLeafClusters = () => - new MockedUnaryCall({ clusters: [leafCluster] }); - const logoutMock = jest.spyOn(mockClient, 'logout'); - const clusterStore = new ClusterStore( - () => Promise.resolve(mockClient), - mockWindowsManager - ); +test('logs out of cluster and keeps profile', async () => { + const { clusterStore, logoutMock } = getTestSetup(); await clusterStore.sync(cluster.uri); - await clusterStore.logoutAndRemove(cluster.uri); + await clusterStore.logout(cluster.uri); + + expect(logoutMock).toHaveBeenCalledWith({ + clusterUri: cluster.uri, + removeProfile: false, + }); + const state = clusterStore.getState(); + expect(state.get(clusterWithDetails.uri)).toEqual( + expect.objectContaining({ + ...clusterWithDetails, + connected: false, + loggedInUser: LoggedInUser.create(), + }) + ); + expect(state.get(leafCluster.uri)).toBeUndefined(); +}); + +test('logs out of cluster and removes profile', async () => { + const { clusterStore, logoutMock } = getTestSetup(); + await clusterStore.sync(cluster.uri); + + await clusterStore.logout(cluster.uri, { removeProfile: true }); expect(logoutMock).toHaveBeenCalledWith({ clusterUri: cluster.uri, @@ -116,3 +124,16 @@ test('logs out of cluster', async () => { expect(state.get(clusterWithDetails.uri)).toBeUndefined(); expect(state.get(leafCluster.uri)).toBeUndefined(); }); + +function getTestSetup() { + const mockClient = new MockTshClient(); + mockClient.getCluster = () => new MockedUnaryCall(clusterWithDetails); + mockClient.listLeafClusters = () => + new MockedUnaryCall({ clusters: [leafCluster] }); + const logoutMock = jest.spyOn(mockClient, 'logout'); + const clusterStore = new ClusterStore( + () => Promise.resolve(mockClient), + mockWindowsManager + ); + return { clusterStore, logoutMock }; +} diff --git a/web/packages/teleterm/src/mainProcess/clusterStore/clusterStore.ts b/web/packages/teleterm/src/mainProcess/clusterStore/clusterStore.ts index f98445e9fe2..1d01f15e9f1 100644 --- a/web/packages/teleterm/src/mainProcess/clusterStore/clusterStore.ts +++ b/web/packages/teleterm/src/mainProcess/clusterStore/clusterStore.ts @@ -22,6 +22,7 @@ import { Cluster, ShowResources, } from 'gen-proto-ts/teleport/lib/teleterm/v1/cluster_pb'; +import { LoggedInUser } from 'gen-proto-ts/teleport/lib/teleterm/v1/cluster_pb'; import Logger from 'teleterm/logger'; import { TshdClient } from 'teleterm/services/tshd'; @@ -76,16 +77,31 @@ export class ClusterStore { } /** - * Logs out of the cluster and removes its profile. + * Logs out of the cluster and optionally removes its profile. * Should only be called via ClusterLifecycleManager. */ - async logoutAndRemove(uri: RootClusterUri): Promise { + async logout( + uri: RootClusterUri, + { removeProfile = false }: { removeProfile?: boolean } = {} + ): Promise { const client = await this.getTshdClient(); - await client.logout({ clusterUri: uri, removeProfile: true }); + await client.logout({ clusterUri: uri, removeProfile }); await this.update(draft => { for (let d of draft.values()) { - if (routing.belongsToProfile(uri, d.uri)) { + if (!routing.belongsToProfile(uri, d.uri)) { + continue; + } + + // Always remove leaf clusters. + if (removeProfile || d.leaf) { draft.delete(d.uri); + } else { + // TODO(gzdunek): Get this logged-out state from getCluster instead of manually + // mirroring the fields it would return. getCluster cannot be used here yet + // because it always makes remote calls, which fail without a valid cert. + // It should support returning profile-backed data only. + d.connected = false; + d.loggedInUser = LoggedInUser.create(); } } }); diff --git a/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts b/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts index 2e5290ea59b..ede85748739 100644 --- a/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts +++ b/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts @@ -204,6 +204,7 @@ export class MockMainProcessClient implements MainProcessClient { return { cleanup: () => undefined }; } async logout(): Promise {} + async forgetCluster(): Promise {} async syncCluster(): Promise {} async addCluster(): Promise { return makeRootCluster(); diff --git a/web/packages/teleterm/src/mainProcess/mainProcess.ts b/web/packages/teleterm/src/mainProcess/mainProcess.ts index 94d7b1ea4c6..879acd441d1 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcess.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcess.ts @@ -712,9 +712,11 @@ export default class MainProcess { ); ipcHandle(MainProcessIpc.Logout, async (_, args) => { - await this.clusterLifecycleManager.logoutAndRemoveCluster( - args.clusterUri - ); + await this.clusterLifecycleManager.logoutCluster(args.clusterUri); + }); + + ipcHandle(MainProcessIpc.ForgetCluster, async (_, args) => { + await this.clusterLifecycleManager.forgetCluster(args.clusterUri); }); ipcMain.on(MainProcessIpc.InitClusterStoreSubscription, ev => { diff --git a/web/packages/teleterm/src/mainProcess/mainProcessClient.ts b/web/packages/teleterm/src/mainProcess/mainProcessClient.ts index cbd74d89fee..b8ef1bc8a80 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcessClient.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcessClient.ts @@ -262,6 +262,9 @@ export default function createMainProcessClient(): MainProcessClient { logout: (clusterUri: RootClusterUri) => { return ipcInvoke(MainProcessIpc.Logout, { clusterUri }); }, + forgetCluster: (clusterUri: RootClusterUri) => { + return ipcInvoke(MainProcessIpc.ForgetCluster, { clusterUri }); + }, registerClusterLifecycleHandler(listener): { cleanup: () => void; } { diff --git a/web/packages/teleterm/src/mainProcess/types.ts b/web/packages/teleterm/src/mainProcess/types.ts index abddc9aad3e..76590837469 100644 --- a/web/packages/teleterm/src/mainProcess/types.ts +++ b/web/packages/teleterm/src/mainProcess/types.ts @@ -234,6 +234,7 @@ export type MainProcessClient = { syncCluster(clusterUri: RootClusterUri): Promise; syncRootClusters(): Promise; logout(clusterUri: RootClusterUri): Promise; + forgetCluster(clusterUri: RootClusterUri): Promise; subscribeToClusterStore(listener: (value: ClusterStoreUpdate) => void): { cleanup: () => void; }; @@ -377,6 +378,7 @@ export enum MainProcessIpc { AddCluster = 'main-process-add-cluster', SyncRootClusters = 'main-process-sync-root-clusters', Logout = 'main-process-logout', + ForgetCluster = 'main-process-forget-cluster', RegisterClusterLifecycleHandler = 'main-process-register-cluster-lifecycle-handler', } diff --git a/web/packages/teleterm/src/services/tshd/testHelpers.ts b/web/packages/teleterm/src/services/tshd/testHelpers.ts index f87ea1f4888..5724c8344ad 100644 --- a/web/packages/teleterm/src/services/tshd/testHelpers.ts +++ b/web/packages/teleterm/src/services/tshd/testHelpers.ts @@ -36,6 +36,7 @@ import * as tsh from './types'; export const rootClusterUri = '/clusters/teleport-local.com'; export const leafClusterUri = `${rootClusterUri}/leaves/leaf`; +export const rootClusterProxyHost = 'teleport-local.com:3080'; export const makeServer = (props: Partial = {}): tsh.Server => ({ uri: `${rootClusterUri}/servers/1234abcd-1234-abcd-1234-abcd1234abcd`, @@ -118,7 +119,7 @@ export const makeRootCluster = ( name: 'teleport-local', connected: true, leaf: false, - proxyHost: 'teleport-local.com:3080', + proxyHost: rootClusterProxyHost, authClusterId: 'fefe3434-fefe-3434-fefe-3434fefe3434', loggedInUser: makeLoggedInUser(), proxyVersion: '11.1.0', diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx index cb33fa4e690..e2b1bfe630c 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx @@ -39,6 +39,7 @@ test('fetching requestable roles for servers uses UUID, not hostname', async () appContext.clustersService.setState(draftState => { draftState.clusters.set(rootClusterUri, cluster); }); + appContext.workspacesService.addWorkspace(cluster); await appContext.workspacesService.setActiveWorkspace(rootClusterUri); await appContext.workspacesService .getWorkspaceAccessRequestsService(rootClusterUri) @@ -76,6 +77,7 @@ test('fetching requestable roles for a kube_cluster resource without specifying appContext.clustersService.setState(draftState => { draftState.clusters.set(rootClusterUri, cluster); }); + appContext.workspacesService.addWorkspace(cluster); await appContext.workspacesService.setActiveWorkspace(rootClusterUri); await appContext.workspacesService .getWorkspaceAccessRequestsService(rootClusterUri) @@ -120,6 +122,7 @@ test(`fetching requestable roles for a kube cluster's namespaces only creates re appContext.clustersService.setState(draftState => { draftState.clusters.set(rootClusterUri, cluster); }); + appContext.workspacesService.addWorkspace(cluster); await appContext.workspacesService.setActiveWorkspace(rootClusterUri); await appContext.workspacesService .getWorkspaceAccessRequestsService(rootClusterUri) @@ -200,6 +203,7 @@ test('after creating an access request, pending requests and specifiable fields appContext.clustersService.setState(draftState => { draftState.clusters.set(rootClusterUri, cluster); }); + appContext.workspacesService.addWorkspace(cluster); await appContext.workspacesService.setActiveWorkspace(rootClusterUri); await appContext.workspacesService .getWorkspaceAccessRequestsService(rootClusterUri) @@ -318,6 +322,7 @@ test('updating kube namespaces', async () => { appContext.clustersService.setState(draftState => { draftState.clusters.set(rootClusterUri, cluster); }); + appContext.workspacesService.addWorkspace(cluster); await appContext.workspacesService.setActiveWorkspace(rootClusterUri); await appContext.workspacesService .getWorkspaceAccessRequestsService(rootClusterUri) diff --git a/web/packages/teleterm/src/ui/App.test.tsx b/web/packages/teleterm/src/ui/App.test.tsx index de019c14176..152fe4cdadc 100644 --- a/web/packages/teleterm/src/ui/App.test.tsx +++ b/web/packages/teleterm/src/ui/App.test.tsx @@ -27,6 +27,7 @@ import Logger, { NullService } from 'teleterm/logger'; import { makeRootCluster } from 'teleterm/services/tshd/testHelpers'; import { App } from 'teleterm/ui/App'; import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; +import { makeWorkspace } from 'teleterm/ui/services/workspacesService/testHelpers'; import { IAppContext } from 'teleterm/ui/types'; mockIntersectionObserver(); @@ -73,16 +74,16 @@ test('activating a workspace via deep link overrides the previously active works .mockReturnValue({ rootClusterUri: previouslyActiveCluster.uri, workspaces: { - [previouslyActiveCluster.uri]: { + [previouslyActiveCluster.uri]: makeWorkspace({ localClusterUri: previouslyActiveCluster.uri, documents: [], location: undefined, - }, - [deepLinkCluster.uri]: { + }), + [deepLinkCluster.uri]: makeWorkspace({ localClusterUri: deepLinkCluster.uri, documents: [], location: undefined, - }, + }), }, }); appContext.mainProcessClient.configService.set( @@ -174,7 +175,7 @@ test.each<{ .mockReturnValue({ rootClusterUri: rootCluster.uri, workspaces: { - [rootCluster.uri]: { + [rootCluster.uri]: makeWorkspace({ localClusterUri: rootCluster.uri, documents: [ { @@ -187,7 +188,7 @@ test.each<{ }, ], location: undefined, - }, + }), }, }); appContext.mainProcessClient.configService.set( diff --git a/web/packages/teleterm/src/ui/AppInitializer/AppInitializer.tsx b/web/packages/teleterm/src/ui/AppInitializer/AppInitializer.tsx index a243f2f9a68..7e4143b1c83 100644 --- a/web/packages/teleterm/src/ui/AppInitializer/AppInitializer.tsx +++ b/web/packages/teleterm/src/ui/AppInitializer/AppInitializer.tsx @@ -43,7 +43,14 @@ export const AppInitializer = () => { // activate it. const rootClusterUri = appContext.workspacesService.getRestoredState()?.rootClusterUri; - if (rootClusterUri) { + if ( + rootClusterUri && + // If the previously active workspace no longer has a cluster, start without an + // active workspace so ClusterConnectPanel is shown. This can happen when the + // profile was removed outside Connect. Recreating the profile should be a + // user-initiated reconnect action, not an automatic startup side effect. + appContext.clustersService.findCluster(rootClusterUri) + ) { void appContext.workspacesService.setActiveWorkspace(rootClusterUri); } appContext.mainProcessClient.signalUserInterfaceReadiness({ diff --git a/web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/useClusterLogin.ts b/web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/useClusterLogin.ts index 1b7e00d73e0..9f441245f6f 100644 --- a/web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/useClusterLogin.ts +++ b/web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/useClusterLogin.ts @@ -58,7 +58,7 @@ export function useClusterLogin(props: Props) { const cluster = clustersService.findCluster(clusterUri); const refAbortCtrl = useRef(null); const loggedInUserName = - props.prefill.username || cluster.loggedInUser?.name || null; + props.prefill.username || cluster?.loggedInUser?.name || null; const [ssoPrompt, setSsoPrompt] = useState('no-prompt'); const [passwordlessLoginState, setPasswordlessLoginState] = useState(); diff --git a/web/packages/teleterm/src/ui/DeepLinks/launchDeepLink.test.ts b/web/packages/teleterm/src/ui/DeepLinks/launchDeepLink.test.ts index 5b4866f8027..fe9eed837d1 100644 --- a/web/packages/teleterm/src/ui/DeepLinks/launchDeepLink.test.ts +++ b/web/packages/teleterm/src/ui/DeepLinks/launchDeepLink.test.ts @@ -96,6 +96,7 @@ it('opens cluster connect dialog if the cluster is not added yet', async () => { clustersService.setState(draft => { draft.clusters.set(cluster.uri, { ...cluster, connected: true }); }); + workspacesService.addWorkspace(cluster); dialog.onSuccess(dialog.clusterUri); @@ -119,6 +120,7 @@ it('switches to the workspace if the cluster already exists', async () => { clustersService.setState(draft => { draft.clusters.set(cluster.uri, { ...cluster, connected: true }); }); + workspacesService.addWorkspace(cluster); await launchDeepLink(appCtx, auxCtx, successResult); @@ -136,6 +138,7 @@ it('does not switch workspaces if the user does not log in to the cluster when a clustersService.setState(draft => { draft.clusters.set(cluster.uri, { ...cluster }); }); + workspacesService.addWorkspace(cluster); jest.spyOn(modalsService, 'openRegularDialog').mockImplementation(dialog => { if (dialog.kind !== 'cluster-connect') { diff --git a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx index 315918687f3..f94e7cb5402 100644 --- a/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx +++ b/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.test.tsx @@ -254,6 +254,7 @@ const testSetup = ( draftState.clusters.set(rootClusterUri, cluster); draftState.clusters.set(leafCluster.uri, leafCluster); }); + appContext.workspacesService.addWorkspace(cluster); appContext.workspacesService.setActiveWorkspace(rootClusterUri); const documentsService = appContext.workspacesService.getWorkspaceDocumentService(rootClusterUri); diff --git a/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx b/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx index 7922af2efea..49b60cbde15 100644 --- a/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx +++ b/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx @@ -17,6 +17,7 @@ */ import { MutableRefObject, useMemo } from 'react'; +import { useCallback } from 'react'; import { createPortal } from 'react-dom'; import styled from 'styled-components'; @@ -42,6 +43,7 @@ import { DocumentGatewayApp } from 'teleterm/ui/DocumentGatewayApp'; import { DocumentGatewayCliClient } from 'teleterm/ui/DocumentGatewayCliClient'; import { DocumentGatewayKube } from 'teleterm/ui/DocumentGatewayKube'; import { DocumentTerminal } from 'teleterm/ui/DocumentTerminal'; +import { useStoreSelector } from 'teleterm/ui/hooks/useStoreSelector'; import * as types from 'teleterm/ui/services/workspacesService'; import { DocumentsService, @@ -60,6 +62,14 @@ export function DocumentsRenderer(props: { desktopSessionControlsRef: MutableRefObject; }) { const { workspacesService } = useAppContext(); + const clusters = useStoreSelector( + 'clustersService', + useCallback(state => state.clusters, []) + ); + const workspaces = useStoreSelector( + 'workspacesService', + useCallback(state => state.workspaces, []) + ); function renderDocuments(documentsService: DocumentsService) { return documentsService.getDocuments().map(doc => { @@ -75,24 +85,25 @@ export function DocumentsRenderer(props: { }); } - const workspaces = useMemo( + const workspacesWithClusters = useMemo( () => - Object.entries(workspacesService.getWorkspaces()).map( - ([clusterUri, workspace]: [RootClusterUri, Workspace]) => ({ + Object.entries(workspaces) + // Workspaces can outlive their clusters. Render only those that have an accompanying cluster available. + .filter(([clusterUri]) => clusters.has(clusterUri)) + .map(([clusterUri, workspace]: [RootClusterUri, Workspace]) => ({ rootClusterUri: clusterUri, localClusterUri: workspace.localClusterUri, documentsService: workspacesService.getWorkspaceDocumentService(clusterUri), accessRequestsService: workspacesService.getWorkspaceAccessRequestsService(clusterUri), - }) - ), - [workspacesService.getWorkspaces()] + })), + [workspaces, clusters, workspacesService] ); return ( <> - {workspaces.map(workspace => ( + {workspacesWithClusters.map(workspace => ( . */ -import { useCallback, useEffect, useRef } from 'react'; +import { useEffect, useRef } from 'react'; import styled from 'styled-components'; import { @@ -30,37 +30,33 @@ import { Text, } from 'design'; -import { useAppContext } from 'teleterm/ui/appContextProvider'; import { NullKeyboardArrowsNavigation } from 'teleterm/ui/components/KeyboardArrowsNavigation/KeyboardArrowsNavigation'; -import { useStoreSelector } from 'teleterm/ui/hooks/useStoreSelector'; -import { TshHomeMigrationBanner } from 'teleterm/ui/TopBar/Identity'; -import { ClusterList } from 'teleterm/ui/TopBar/Identity/IdentityList/IdentityList'; -import { RootClusterUri } from 'teleterm/ui/uri'; +import { + TshHomeMigrationBanner, + useIdentity, + IdentityList, +} from 'teleterm/ui/TopBar/Identity'; export function ClusterConnectPanel() { - const ctx = useAppContext(); - const clusters = useStoreSelector( - 'clustersService', - useCallback(state => state.clusters, []) - ); - const rootClusters = [...clusters.values()].filter(c => !c.leaf); - function add(): void { - ctx.commandLauncher.executeCommand('cluster-connect', {}); - } - - function connect(clusterUri: RootClusterUri): void { - ctx.workspacesService.setActiveWorkspace(clusterUri); - } + const { + // ClusterConnectPanel is rendered only when there is no active workspace, so + // the hook's "other workspaces" are all available workspaces. + otherWorkspaces: availableWorkspaces, + logout, + forget, + addCluster, + changeWorkspace, + } = useIdentity(); const containerRef = useRef(null); // Focus the first item. - const hasCluster = !!rootClusters.length; + const hasAnyWorkspaces = !!availableWorkspaces.length; useEffect(() => { - if (hasCluster) { + if (hasAnyWorkspaces) { containerRef.current.querySelector('li').focus(); } - }, [hasCluster]); + }, [hasAnyWorkspaces]); return ( @@ -72,7 +68,7 @@ export function ClusterConnectPanel() { alignItems="center" > - {hasCluster ? ( + {hasAnyWorkspaces ? (

Clusters

@@ -101,10 +97,12 @@ export function ClusterConnectPanel() { } `} > - @@ -116,7 +114,7 @@ export function ClusterConnectPanel() { Connect an existing Teleport cluster
to start using Teleport Connect. - + Connect diff --git a/web/packages/teleterm/src/ui/TopBar/Identity/Identity.story.tsx b/web/packages/teleterm/src/ui/TopBar/Identity/Identity.story.tsx index 45e6b8ee584..b1a42f554c2 100644 --- a/web/packages/teleterm/src/ui/TopBar/Identity/Identity.story.tsx +++ b/web/packages/teleterm/src/ui/TopBar/Identity/Identity.story.tsx @@ -88,6 +88,7 @@ const meta: Meta = { const hasClusterWithLoggedInUser = props.activeCluster && (hasOrange || hasViolet); if (hasClusterWithLoggedInUser) { + clusters[0].connected = true; clusters[0].loggedInUser = makeLoggedInUser({ ...clusters[0].loggedInUser, validUntil: Timestamp.fromDate( @@ -165,6 +166,7 @@ const clusterViolet = makeRootCluster({ const clusterGreen = makeRootCluster({ name: 'green', loggedInUser: undefined, + connected: false, uri: '/clusters/green', }); @@ -183,9 +185,9 @@ const OpenIdentityPopover = (props: { props.clusters.forEach(c => { ctx.addRootCluster(c); }); - ctx.workspacesService.addWorkspace(clusterGreen.uri); - ctx.workspacesService.addWorkspace(clusterViolet.uri); - ctx.workspacesService.addWorkspace(clusterOrange.uri); + ctx.workspacesService.addWorkspace(clusterGreen); + ctx.workspacesService.addWorkspace(clusterViolet); + ctx.workspacesService.addWorkspace(clusterOrange); ctx.workspacesService.setState(draftState => { draftState.rootClusterUri = props.activeClusterUri; draftState.workspaces[clusterGreen.uri].color = 'green'; diff --git a/web/packages/teleterm/src/ui/TopBar/Identity/Identity.test.tsx b/web/packages/teleterm/src/ui/TopBar/Identity/Identity.test.tsx index 06b6d44fad2..b61556a533a 100644 --- a/web/packages/teleterm/src/ui/TopBar/Identity/Identity.test.tsx +++ b/web/packages/teleterm/src/ui/TopBar/Identity/Identity.test.tsx @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import { screen } from '@testing-library/react'; +import { screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { render } from 'design/utils/testing'; @@ -121,6 +121,91 @@ test('roles list remembers it was expanded', async () => { expect(await screen.findByText('requests-reviewer')).toBeVisible(); }); +test('shows each identity row with the correct profile status and action', async () => { + const appContext = new MockAppContext(); + const activeCluster = makeRootCluster({ + uri: '/clusters/active', + loggedInUser: makeLoggedInUser({ name: 'active-user' }), + }); + const connectedCluster = makeRootCluster({ + uri: '/clusters/connected', + loggedInUser: makeLoggedInUser({ + name: 'alice', + }), + }); + const expiredCluster = makeRootCluster({ + uri: '/clusters/expired', + connected: false, + loggedInUser: makeLoggedInUser({ + name: 'expired-user', + }), + }); + const disconnectedCluster = makeRootCluster({ + uri: '/clusters/disconnected', + connected: false, + loggedInUser: undefined, + }); + const executeCommandSpy = jest.spyOn( + appContext.commandLauncher, + 'executeCommand' + ); + const forgetClusterSpy = jest.spyOn( + appContext.mockMainProcessClient, + 'forgetCluster' + ); + + appContext.addRootCluster(activeCluster); + appContext.addRootCluster(connectedCluster, { noActivate: true }); + appContext.addRootCluster(expiredCluster, { noActivate: true }); + appContext.addRootCluster(disconnectedCluster, { noActivate: true }); + appContext.workspacesService.addWorkspace({ + uri: '/clusters/orphan', + proxyHost: 'this-is-orphaned-cluster.com', + }); + + render( + + + + ); + + await toggleProfiles(); + + expect( + within(await screen.findByTitle('Switch to connected')).getByText('alice') + ).toBeVisible(); + expect( + within(await screen.findByTitle('Switch to expired')).getByText( + 'expired-user · Session expired' + ) + ).toBeVisible(); + expect( + within(await screen.findByTitle('Switch to disconnected')).getByText( + 'Not logged in' + ) + ).toBeVisible(); + expect( + within(await screen.findByTitle('Switch to orphan')).getByText('In history') + ).toBeVisible(); + + await userEvent.click(screen.getByTitle('Log out from connected')); + expect(executeCommandSpy).toHaveBeenCalledWith('cluster-logout', { + clusterUri: '/clusters/connected', + }); + + await toggleProfiles(); + await userEvent.click(screen.getByTitle('Forget disconnected')); + expect(forgetClusterSpy).toHaveBeenCalledWith('/clusters/disconnected'); + + await toggleProfiles(); + await userEvent.click(screen.getByTitle('Forget expired')); + expect(forgetClusterSpy).toHaveBeenCalledWith('/clusters/expired'); + + await toggleProfiles(); + await userEvent.click(screen.getByTitle('Forget orphan')); + expect(forgetClusterSpy).toHaveBeenCalledWith('/clusters/orphan'); +}); + async function toggleProfiles() { await userEvent.click(await screen.findByTitle(/Open Profiles/)); } diff --git a/web/packages/teleterm/src/ui/TopBar/Identity/Identity.tsx b/web/packages/teleterm/src/ui/TopBar/Identity/Identity.tsx index 800395ca333..9785a3b7034 100644 --- a/web/packages/teleterm/src/ui/TopBar/Identity/Identity.tsx +++ b/web/packages/teleterm/src/ui/TopBar/Identity/Identity.tsx @@ -33,31 +33,34 @@ import { useKeyboardShortcuts, } from 'teleterm/ui/services/keyboardShortcuts'; -import { ActiveCluster, ClusterList } from './IdentityList/IdentityList'; +import { ActiveCluster, IdentityList } from './IdentityList/IdentityList'; import { IdentitySelector } from './IdentitySelector/IdentitySelector'; import { useIdentity } from './useIdentity'; export function IdentityContainer() { const { - activeRootCluster, - rootClusters, - changeRootCluster, + activeWorkspaceCluster, + otherWorkspaces, + changeWorkspace, logout, addCluster, + forget, refreshCluster, changeColor, } = useIdentity(); const selectorRef = useRef(null); const [open, setOpen] = useState(false); const { getLabelWithAccelerator } = useKeyboardShortcutFormatters(); - const hasClusters = activeRootCluster || rootClusters.length; + const hasAnyWorkspaces = Boolean( + activeWorkspaceCluster || otherWorkspaces.length + ); const togglePopoverOrAddCluster = useCallback(() => { - if (hasClusters) { + if (hasAnyWorkspaces) { setOpen(o => !o); } else { addCluster(); } - }, [addCluster, hasClusters]); + }, [addCluster, hasAnyWorkspaces]); useKeyboardShortcuts( useMemo( @@ -84,7 +87,7 @@ export function IdentityContainer() { } const deviceTrustStatus = calculateDeviceTrustStatus( - activeRootCluster?.loggedInUser + activeWorkspaceCluster?.loggedInUser ); const activeColor = useStoreSelector( 'workspacesService', @@ -97,7 +100,7 @@ export function IdentityContainer() { ref={selectorRef} onClick={togglePopoverOrAddCluster} open={open} - activeCluster={activeRootCluster} + activeCluster={activeWorkspaceCluster} activeColor={activeColor} makeTitle={makeTitle} deviceTrustStatus={deviceTrustStatus} @@ -112,24 +115,27 @@ export function IdentityContainer() { updatePositionOnChildResize > - {activeRootCluster && ( + {activeWorkspaceCluster && ( logout(activeRootCluster.uri))} - onRefresh={withClose(() => refreshCluster(activeRootCluster.uri))} + onLogout={withClose(() => logout(activeWorkspaceCluster.uri))} + onRefresh={withClose(() => + refreshCluster(activeWorkspaceCluster.uri) + )} deviceTrustStatus={deviceTrustStatus} /> )} {focusGrabber} - diff --git a/web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityList.tsx b/web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityList.tsx index afd93da7e7b..b9ef2f8ffa6 100644 --- a/web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityList.tsx +++ b/web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityList.tsx @@ -35,9 +35,10 @@ import { Cluster } from 'gen-proto-ts/teleport/lib/teleterm/v1/cluster_pb'; import { ProfileStatusError } from 'teleterm/ui/components/ProfileStatusError'; import { usePersistedState } from 'teleterm/ui/hooks/usePersistedState'; import { WorkspaceColor } from 'teleterm/ui/services/workspacesService'; -import { DeviceTrustStatus } from 'teleterm/ui/TopBar/Identity/Identity'; import { RootClusterUri, routing } from 'teleterm/ui/uri'; +import { DeviceTrustStatus } from '../Identity'; +import { IdentityItem } from '../useIdentity'; import { ColorPicker } from './ColorPicker'; import { AddClusterItem, @@ -71,7 +72,7 @@ export function ActiveCluster(props: { @@ -149,26 +150,41 @@ export function ActiveCluster(props: { ); } -export function ClusterList(props: { - clusters: Cluster[]; +export function IdentityList(props: { + items: IdentityItem[]; onSelect(clusterUri: RootClusterUri): void; - onLogout?(clusterUri: RootClusterUri): void; + onLogout(clusterUri: RootClusterUri): void; + onForget(clusterUri: RootClusterUri): void; onAdd(): void; }) { return ( <> - {props.clusters.map((cluster, index) => ( - props.onSelect(cluster.uri)} - onLogout={ - props.onLogout ? () => props.onLogout(cluster.uri) : undefined - } - /> - ))} - + {props.items + .toSorted((a, b) => { + return ( + // Puts items with profile first, then sorts equal groups alphabetically. + Number(!!b.cluster) - Number(!!a.cluster) || + routing + .parseClusterName(a.uri) + .localeCompare(routing.parseClusterName(b.uri)) + ); + }) + .map((identityItem, index) => { + const { cluster } = identityItem; + return ( + props.onSelect(identityItem.uri)} + onLogout={() => props.onLogout(identityItem.uri)} + onForget={() => props.onForget(identityItem.uri)} + /> + ); + })} + ); } diff --git a/web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityListItem.tsx b/web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityListItem.tsx index 66a5b26800e..c2ebd25a160 100644 --- a/web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityListItem.tsx +++ b/web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityListItem.tsx @@ -15,42 +15,36 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . */ -import { useCallback } from 'react'; + import styled from 'styled-components'; import { ButtonText, Flex, P3, Text } from 'design'; -import { Logout } from 'design/Icon'; +import { type IconComponentType, Logout, Trash } from 'design/Icon'; import { Cluster } from 'gen-proto-ts/teleport/lib/teleterm/v1/cluster_pb'; import { useKeyboardArrowsNavigation } from 'teleterm/ui/components/KeyboardArrowsNavigation'; import { ListItem } from 'teleterm/ui/components/ListItem'; import { ProfileStatusError } from 'teleterm/ui/components/ProfileStatusError'; -import { useStoreSelector } from 'teleterm/ui/hooks/useStoreSelector'; import { WorkspaceColor } from 'teleterm/ui/services/workspacesService'; -import { routing } from 'teleterm/ui/uri'; +import { RootClusterUri, routing } from 'teleterm/ui/uri'; import { UserIcon } from '../IdentitySelector/UserIcon'; export function IdentityListItem(props: { index: number; - cluster: Cluster; + uri: RootClusterUri; + color: WorkspaceColor; + cluster: Cluster | undefined; onSelect(): void; - /** If defined, the logout button is rendered. */ - onLogout?(): void; + onLogout(): void; + onForget(): void; }) { const { isActive } = useKeyboardArrowsNavigation({ index: props.index, onRun: props.onSelect, }); - const workspaceColor = useStoreSelector( - 'workspacesService', - useCallback( - state => state.workspaces[props.cluster.uri]?.color, - [props.cluster.uri] - ) - ); - - const profileName = routing.parseClusterName(props.cluster.uri); + const profileName = routing.parseClusterName(props.uri); + const subtitle = getSubtitle(props.cluster); return ( - {props.onLogout && ( - { - e.stopPropagation(); - props.onLogout(); - }} - > - - - )} + + {props.cluster?.connected ? ( + + ) : ( + + )} + - {props.cluster.profileStatusError && ( + {props.cluster?.profileStatusError && ( void; +}) { + return ( + { + e.stopPropagation(); + props.onClick(); + }} + > + + + ); +} + const StyledListItem = styled(ListItem)` padding: ${props => props.theme.space[2]}px ${props => props.theme.space[3]}px; flex-direction: column; @@ -169,6 +185,32 @@ export function TitleAndSubtitle(props: { title: string; subtitle?: string }) { ); } -export function getProfileNameLetter(cluster: Cluster): string { - return routing.parseClusterName(cluster.uri).at(0); +export function getProfileNameLetter(uri: RootClusterUri): string { + return routing.parseClusterName(uri).at(0); +} + +/** + * Maps cluster/profile state to the subtitle shown in the identity list. + * + * | How this state happened | Internal state | Subtitle | + * | ----------------------------------------------------------------------------------------------------| -------------------------------------------------------- | -------------------------- | + * | `tsh logout` removed the tsh profile, but Connect still remembers the workspace. | `cluster` is undefined. | In history | + * | `tsh logout --proxy=... --user=...` or Connect logout removed the credentials but kept the profile. | `cluster` exists, but `loggedInUser.name` is empty. | Not logged in | + * | The user's credentials expired. | `cluster` has `loggedInUser.name`, but is not connected. | ` · Session expired` | + * | The user is currently logged in. | `cluster` has `loggedInUser.name` and is connected. | `` | + */ +function getSubtitle(cluster: Cluster | undefined): string { + if (!cluster) { + return 'In history'; + } + + if (!cluster.loggedInUser?.name) { + return 'Not logged in'; + } + + if (!cluster.connected) { + return `${cluster.loggedInUser.name} · Session expired`; + } + + return cluster.loggedInUser.name; } diff --git a/web/packages/teleterm/src/ui/TopBar/Identity/IdentitySelector/IdentitySelector.tsx b/web/packages/teleterm/src/ui/TopBar/Identity/IdentitySelector/IdentitySelector.tsx index ce5c167af94..827083d373c 100644 --- a/web/packages/teleterm/src/ui/TopBar/Identity/IdentitySelector/IdentitySelector.tsx +++ b/web/packages/teleterm/src/ui/TopBar/Identity/IdentitySelector/IdentitySelector.tsx @@ -64,7 +64,7 @@ export const IdentitySelector = forwardRef< {props.deviceTrustStatus === 'requires-enrollment' && ( state.workspaces, []) + ); + const clusters = useStoreSelector( + 'clustersService', + useCallback(state => state.clusters, []) + ); + const activeClusterUri = useStoreSelector( + 'workspacesService', + useCallback(state => state.rootClusterUri, []) + ); + const activeWorkspaceCluster = + activeClusterUri && clusters.get(activeClusterUri); - async function changeRootCluster(clusterUri: RootClusterUri): Promise { + async function changeWorkspace(clusterUri: RootClusterUri): Promise { await ctx.workspacesService.setActiveWorkspace(clusterUri); } @@ -48,9 +61,15 @@ export function useIdentity() { ctx.commandLauncher.executeCommand('cluster-logout', { clusterUri }); } - const activeClusterUri = ctx.workspacesService.getRootClusterUri(); - function getActiveRootCluster(): Cluster | undefined { - return ctx.clustersService.findCluster(activeClusterUri); + async function forget(clusterUri: RootClusterUri): Promise { + try { + await ctx.mainProcessClient.forgetCluster(clusterUri); + } catch (err) { + ctx.notificationsService.notifyError({ + title: 'Failed to forget cluster', + description: getErrorMessage(err), + }); + } } function changeColor(color: WorkspaceColor): undefined { @@ -61,18 +80,28 @@ export function useIdentity() { ctx.workspacesService.changeWorkspaceColor(clusterUri, color); } - const rootClusters = ctx.clustersService - .getClusters() - .filter(c => !c.leaf) - .filter(c => c.uri !== activeClusterUri); + const otherWorkspaces: IdentityItem[] = Object.entries(workspaces) + .filter(([uri]) => uri !== activeClusterUri) + .map(([uri, workspace]) => ({ + uri, + workspace: workspace, + cluster: clusters.get(uri), + })); return { - changeRootCluster, + changeWorkspace, addCluster, refreshCluster, logout, + forget, changeColor, - activeRootCluster: getActiveRootCluster(), - rootClusters, + activeWorkspaceCluster, + otherWorkspaces, }; } + +export interface IdentityItem { + uri: RootClusterUri; + workspace: Workspace; + cluster: Cluster | undefined; +} diff --git a/web/packages/teleterm/src/ui/appContext.ts b/web/packages/teleterm/src/ui/appContext.ts index 532c495ec13..0bf28f88b33 100644 --- a/web/packages/teleterm/src/ui/appContext.ts +++ b/web/packages/teleterm/src/ui/appContext.ts @@ -229,7 +229,23 @@ export default class AppContext implements IAppContext { const task = processingQueue.then(async () => { switch (op) { case 'did-add-cluster': - return this.workspacesService.addWorkspace(uri); + const cluster = this.clustersService.findCluster(uri); + if (!cluster) { + throw new Error(`Cluster ${uri} does not exist`); + } + return this.workspacesService.addWorkspace({ + uri, + proxyHost: cluster.proxyHost, + }); + case 'did-change-proxy-host': + const updatedCluster = this.clustersService.findCluster(uri); + if (!updatedCluster) { + throw new Error(`Cluster ${uri} does not exist`); + } + return this.workspacesService.updateWorkspaceProxyHost({ + uri, + proxyHost: updatedCluster.proxyHost, + }); case 'did-change-access': if (!this.clustersService.findCluster(uri)?.connected) { // Only refresh resources when the cluster is connected. @@ -238,7 +254,7 @@ export default class AppContext implements IAppContext { return this.resourceRefreshListener(uri); case 'will-logout': return cleanUpBeforeLogout(this, uri, { removeWorkspace: false }); - case 'will-logout-and-remove': + case 'will-forget-cluster': return cleanUpBeforeLogout(this, uri, { removeWorkspace: true }); default: op satisfies never; diff --git a/web/packages/teleterm/src/ui/commandLauncher.ts b/web/packages/teleterm/src/ui/commandLauncher.ts index dc9009de4b8..6a2a117cc5e 100644 --- a/web/packages/teleterm/src/ui/commandLauncher.ts +++ b/web/packages/teleterm/src/ui/commandLauncher.ts @@ -17,7 +17,7 @@ */ import { IAppContext } from 'teleterm/ui/types'; -import { ClusterUri, RootClusterUri } from 'teleterm/ui/uri'; +import { ClusterUri, RootClusterUri, routing } from 'teleterm/ui/uri'; const commands = { 'tsh-install': { @@ -102,11 +102,14 @@ const commands = { description: '', async run(ctx: IAppContext, args: { clusterUri: ClusterUri }) { const { clusterUri } = args; - const rootCluster = - ctx.clustersService.findRootClusterByResource(clusterUri); - await ctx.workspacesService.setActiveWorkspace(rootCluster.uri); + const rootClusterUri = routing.ensureRootClusterUri(clusterUri); + const { isAtDesiredWorkspace } = + await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + if (!isAtDesiredWorkspace) { + return; + } const documentsService = - ctx.workspacesService.getWorkspaceDocumentService(rootCluster.uri); + ctx.workspacesService.getWorkspaceDocumentService(rootClusterUri); const doc = documentsService.findClusterDocument(clusterUri); if (doc) { documentsService.open(doc.uri); diff --git a/web/packages/teleterm/src/ui/fixtures/mocks.ts b/web/packages/teleterm/src/ui/fixtures/mocks.ts index c7a7f69e7c1..dd80fe1f347 100644 --- a/web/packages/teleterm/src/ui/fixtures/mocks.ts +++ b/web/packages/teleterm/src/ui/fixtures/mocks.ts @@ -62,7 +62,7 @@ export class MockAppContext extends AppContext { draftState.clusters.set(cluster.uri, cluster); }); const docs = Array.isArray(doc) ? doc : [doc]; - this.workspacesService.addWorkspace(cluster.uri); + this.workspacesService.addWorkspace(cluster); this.workspacesService.setState(draftState => { if (!options?.noActivate) { draftState.rootClusterUri = cluster.uri; diff --git a/web/packages/teleterm/src/ui/services/clusters/clustersService.ts b/web/packages/teleterm/src/ui/services/clusters/clustersService.ts index a1b062c5cca..d53c2151cfe 100644 --- a/web/packages/teleterm/src/ui/services/clusters/clustersService.ts +++ b/web/packages/teleterm/src/ui/services/clusters/clustersService.ts @@ -362,6 +362,10 @@ export class ClustersService extends ImmutableStore { return gateway; } + async addCluster(proxy: string) { + return await this.mainProcessClient.addCluster(proxy); + } + findCluster(clusterUri: uri.ClusterUri) { return this.state.clusters.get(clusterUri); } diff --git a/web/packages/teleterm/src/ui/services/connectionTracker/connectionTrackerService.legacy.test.ts b/web/packages/teleterm/src/ui/services/connectionTracker/connectionTrackerService.legacy.test.ts index e395364d843..88c40dafae4 100644 --- a/web/packages/teleterm/src/ui/services/connectionTracker/connectionTrackerService.legacy.test.ts +++ b/web/packages/teleterm/src/ui/services/connectionTracker/connectionTrackerService.legacy.test.ts @@ -17,6 +17,7 @@ */ import Logger, { NullService } from 'teleterm/logger'; +import { makeWorkspace } from 'teleterm/ui/services/workspacesService/testHelpers'; import { ClustersService } from '../clusters'; import { StatePersistenceService } from '../statePersistence'; @@ -253,7 +254,7 @@ function getTestSetupWithMockedDocuments(documents: Document[]) { // Insert the documents. workspacesService.setState(draftState => { - draftState.workspaces['/clusters/localhost'] = { + draftState.workspaces['/clusters/localhost'] = makeWorkspace({ color: 'purple', accessRequests: { pending: getEmptyPendingAccessRequest(), @@ -262,7 +263,7 @@ function getTestSetupWithMockedDocuments(documents: Document[]) { localClusterUri: '/clusters/localhost', location: documents[0]?.uri, documents: documents, - }; + }); }); return { workspacesService, connectionTrackerService }; diff --git a/web/packages/teleterm/src/ui/services/connectionTracker/connectionTrackerService.ts b/web/packages/teleterm/src/ui/services/connectionTracker/connectionTrackerService.ts index 863b2142d12..76b96117e57 100644 --- a/web/packages/teleterm/src/ui/services/connectionTracker/connectionTrackerService.ts +++ b/web/packages/teleterm/src/ui/services/connectionTracker/connectionTrackerService.ts @@ -107,7 +107,11 @@ export class ConnectionTrackerService extends ImmutableStore { function setTestCluster(appContext: IAppContext): void { const testCluster = makeRootCluster(); + appContext.workspacesService.addWorkspace(testCluster); appContext.workspacesService.setState(d => { d.rootClusterUri = testCluster.uri; }); diff --git a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToApp.ts b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToApp.ts index 6ff67350749..3bc2f058eed 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToApp.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToApp.ts @@ -182,7 +182,11 @@ export async function setUpAppGateway( origin: options.telemetry.origin, }); } else { - await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + const { isAtDesiredWorkspace } = + await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + if (!isAtDesiredWorkspace) { + return; + } documentsService.add(doc); documentsService.open(doc.uri); } diff --git a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToDatabase.test.ts b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToDatabase.test.ts index 2d6db02913c..e72af694c9e 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToDatabase.test.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToDatabase.test.ts @@ -21,6 +21,7 @@ import { makeRootCluster, } from 'teleterm/services/tshd/testHelpers'; import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; +import { makeWorkspace } from 'teleterm/ui/services/workspacesService/testHelpers'; import { IAppContext } from 'teleterm/ui/types'; import { connectToDatabase } from './connectToDatabase'; @@ -97,6 +98,7 @@ function setTestCluster( ): void { appContext.workspacesService.setState(d => { d.rootClusterUri = cluster.uri; + d.workspaces = { [cluster.uri]: makeWorkspace() }; }); appContext.clustersService.setState(d => { d.clusters.set(cluster.uri, cluster); diff --git a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToDatabase.ts b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToDatabase.ts index 95b6bc853da..71a5357c293 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToDatabase.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToDatabase.ts @@ -63,7 +63,11 @@ export async function connectToDatabase( origin: telemetry.origin, }); } else { - await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + const { isAtDesiredWorkspace } = + await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + if (!isAtDesiredWorkspace) { + return; + } documentsService.add(doc); documentsService.open(doc.uri); } diff --git a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToKube.ts b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToKube.ts index 0fd33696535..ee2f67028e4 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToKube.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToKube.ts @@ -34,7 +34,11 @@ export async function connectToKube( origin: telemetry.origin, }); - await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + const { isAtDesiredWorkspace } = + await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + if (!isAtDesiredWorkspace) { + return; + } documentsService.add(doc); documentsService.open(doc.uri); } diff --git a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToServer.ts b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToServer.ts index 9f58de0ee3f..a315e81ec4e 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToServer.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToServer.ts @@ -42,7 +42,11 @@ export async function connectToServer( doc.title = `${target.login}@${target.hostname}`; doc.login = target.login; - await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + const { isAtDesiredWorkspace } = + await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + if (!isAtDesiredWorkspace) { + return; + } documentsService.add(doc); documentsService.open(doc.uri); } diff --git a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToWindowsDesktop.ts b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToWindowsDesktop.ts index 59df222324b..a3444c128f3 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToWindowsDesktop.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToWindowsDesktop.ts @@ -33,7 +33,11 @@ export async function connectToWindowsDesktop( } ): Promise { const rootClusterUri = routing.ensureRootClusterUri(target.uri); - await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + const { isAtDesiredWorkspace } = + await ctx.workspacesService.setActiveWorkspace(rootClusterUri); + if (!isAtDesiredWorkspace) { + return; + } ctx.workspacesService .getWorkspaceDocumentService(rootClusterUri) .openExistingOrAddNew( diff --git a/web/packages/teleterm/src/ui/services/workspacesService/testHelpers.ts b/web/packages/teleterm/src/ui/services/workspacesService/testHelpers.ts new file mode 100644 index 00000000000..99342ac79d8 --- /dev/null +++ b/web/packages/teleterm/src/ui/services/workspacesService/testHelpers.ts @@ -0,0 +1,65 @@ +/** + * Teleport + * Copyright (C) 2026 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 . + */ + +import { + rootClusterProxyHost, + rootClusterUri, +} from 'teleterm/services/tshd/testHelpers'; + +import { PersistedWorkspace } from '../statePersistence'; +import { getEmptyPendingAccessRequest } from './accessRequestsService'; +import { makeDocumentCluster } from './documentsService/testHelpers'; +import { + getDefaultUnifiedResourcePreferences, + Workspace, +} from './workspacesService'; + +export function makeWorkspace(props?: Partial): Workspace { + const clusterUri = props?.localClusterUri ?? rootClusterUri; + const defaultDocument = makeDocumentCluster({ clusterUri }); + + return { + accessRequests: { + isBarCollapsed: false, + pending: getEmptyPendingAccessRequest(), + }, + color: 'purple', + connectMyComputer: undefined, + documents: [defaultDocument], + hasDocumentsToReopen: false, + localClusterUri: clusterUri, + location: defaultDocument.uri, + proxyHost: rootClusterProxyHost, + unifiedResourcePreferences: getDefaultUnifiedResourcePreferences(), + ...props, + }; +} + +export function makePersistedWorkspace( + props?: Partial +): PersistedWorkspace { + const clusterUri = props?.localClusterUri ?? rootClusterUri; + + return { + documents: [], + localClusterUri: clusterUri, + location: undefined, + proxyHost: rootClusterProxyHost, + ...props, + }; +} diff --git a/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.test.ts b/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.test.ts index adde6e1ac4e..ab3a881bcb7 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.test.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.test.ts @@ -17,12 +17,6 @@ */ import { Timestamp } from 'gen-proto-ts/google/protobuf/timestamp_pb'; -import { - AvailableResourceMode, - DefaultTab, - LabelsViewMode, - ViewMode, -} from 'gen-proto-ts/teleport/userpreferences/v1/unified_resource_preferences_pb'; import Logger, { NullService } from 'teleterm/logger'; import { createMockFileStorage } from 'teleterm/services/fileStorage/fixtures/mocks'; @@ -37,16 +31,15 @@ import { ClustersService } from '../clusters'; import { ModalsService } from '../modals'; import { NotificationsService } from '../notifications'; import { - PersistedWorkspace, StatePersistenceService, WorkspacesPersistedState, } from '../statePersistence'; -import { getEmptyPendingAccessRequest } from './accessRequestsService'; import { DocumentCluster, DocumentsService, DocumentVnetDiagReport, } from './documentsService'; +import { makePersistedWorkspace, makeWorkspace } from './testHelpers'; import { WorkspacesService, WorkspacesState } from './workspacesService'; beforeAll(() => { @@ -60,7 +53,7 @@ beforeEach(() => { describe('restoring workspace', () => { it('restores the workspace if there is a persisted state for given clusterUri', () => { const cluster = makeRootCluster(); - const testWorkspace: PersistedWorkspace = { + const testWorkspace = makePersistedWorkspace({ localClusterUri: cluster.uri, documents: [ { @@ -70,7 +63,7 @@ describe('restoring workspace', () => { }, ], location: '/docs/some_uri', - }; + }); const persistedWorkspace = { [cluster.uri]: testWorkspace }; @@ -82,27 +75,13 @@ describe('restoring workspace', () => { workspacesService.restorePersistedState(); expect(workspacesService.getWorkspaces()).toStrictEqual({ - [cluster.uri]: { - accessRequests: { - pending: { - kind: 'resource', - resources: new Map(), - }, - isBarCollapsed: false, - }, - color: 'purple', + [cluster.uri]: makeWorkspace({ + proxyHost: cluster.proxyHost, localClusterUri: testWorkspace.localClusterUri, documents: [expect.objectContaining({ kind: 'doc.cluster' })], location: expect.any(String), hasDocumentsToReopen: true, - connectMyComputer: undefined, - unifiedResourcePreferences: { - defaultTab: DefaultTab.ALL, - viewMode: ViewMode.CARD, - labelsViewMode: LabelsViewMode.COLLAPSED, - availableResourceMode: AvailableResourceMode.NONE, - }, - }, + }), }); expect(workspacesService.getRestoredState().workspaces).toStrictEqual( persistedWorkspace @@ -119,46 +98,98 @@ describe('restoring workspace', () => { workspacesService.restorePersistedState(); expect(workspacesService.getWorkspaces()).toStrictEqual({ - [cluster.uri]: { - accessRequests: { - isBarCollapsed: false, - pending: { - kind: 'resource', - resources: new Map(), - }, - }, - color: 'purple', + [cluster.uri]: makeWorkspace({ + proxyHost: cluster.proxyHost, localClusterUri: cluster.uri, documents: [expect.objectContaining({ kind: 'doc.cluster' })], location: expect.any(String), hasDocumentsToReopen: false, - connectMyComputer: undefined, - unifiedResourcePreferences: { - defaultTab: DefaultTab.ALL, - viewMode: ViewMode.CARD, - labelsViewMode: LabelsViewMode.COLLAPSED, - availableResourceMode: AvailableResourceMode.NONE, - }, - }, + }), }); expect(workspacesService.getRestoredState().workspaces).toStrictEqual({}); }); + it('keeps restored workspaces even when no matching cluster exists', () => { + const cluster = makeRootCluster(); + const orphanClusterUri = '/clusters/orphan'; + const orphanWorkspace = makePersistedWorkspace({ + color: 'blue', + localClusterUri: orphanClusterUri, + documents: [ + { + kind: 'doc.terminal_shell', + uri: '/docs/some_orphan_doc', + title: '/Users/alice/Downloads', + }, + ], + location: '/docs/some_orphan_doc', + }); + + const { workspacesService } = getTestSetup({ + cluster, + persistedWorkspaces: { + [orphanClusterUri]: orphanWorkspace, + }, + }); + + workspacesService.restorePersistedState(); + + expect(workspacesService.getWorkspace(orphanClusterUri)).toBeDefined(); + }); + + it('skips workspaces without a matching cluster or saved proxy host', () => { + const orphanClusterUri = '/clusters/orphan'; + const orphanWorkspace = makePersistedWorkspace({ + // No stored proxy host. + proxyHost: undefined, + localClusterUri: orphanClusterUri, + }); + + const { workspacesService } = getTestSetup({ + // No matching cluster. + cluster: undefined, + persistedWorkspaces: { + [orphanClusterUri]: orphanWorkspace, + }, + }); + + workspacesService.restorePersistedState(); + + expect(workspacesService.getWorkspace(orphanClusterUri)).toBeUndefined(); + }); + + it('keeps proxy host for restored workspaces without matching clusters', () => { + const orphanClusterUri = '/clusters/orphan'; + const orphanWorkspace = makePersistedWorkspace({ + proxyHost: 'orphan.example.com:443', + localClusterUri: orphanClusterUri, + }); + + const { workspacesService } = getTestSetup({ + cluster: undefined, + persistedWorkspaces: { + [orphanClusterUri]: orphanWorkspace, + }, + }); + + workspacesService.restorePersistedState(); + + expect(workspacesService.getWorkspace(orphanClusterUri).proxyHost).toBe( + orphanWorkspace.proxyHost + ); + }); + it('restores workspace color from state or assigns if empty', async () => { const clusterFoo = makeRootCluster({ uri: '/clusters/foo' }); - const workspaceFoo: PersistedWorkspace = { + const workspaceFoo = makePersistedWorkspace({ color: 'blue', localClusterUri: clusterFoo.uri, - documents: [], - location: undefined, - }; + }); const clusterBar = makeRootCluster({ uri: '/clusters/bar' }); - const workspaceBar: PersistedWorkspace = { + const workspaceBar = makePersistedWorkspace({ color: 'purple', localClusterUri: clusterBar.uri, - documents: [], - location: undefined, - }; + }); const clusterBaz = makeRootCluster({ uri: '/clusters/baz' }); const clusterQux = makeRootCluster({ uri: '/clusters/qux' }); const clusterWaldo = makeRootCluster({ uri: '/clusters/waldo' }); @@ -209,12 +240,7 @@ describe('state persistence', () => { rootClusterUri: cluster.uri, isInitialized: true, workspaces: { - [cluster.uri]: { - accessRequests: { - isBarCollapsed: true, - pending: getEmptyPendingAccessRequest(), - }, - color: 'purple', + [cluster.uri]: makeWorkspace({ localClusterUri: cluster.uri, documents: [ { @@ -236,7 +262,7 @@ describe('state persistence', () => { }, ], location: '/docs/authorize_web_session', - }, + }), }, }; const { workspacesService, statePersistenceService } = getTestSetup({ @@ -273,14 +299,8 @@ describe('state persistence', () => { rootClusterUri: cluster.uri, isInitialized: true, workspaces: { - [cluster.uri]: { - color: 'purple', + [cluster.uri]: makeWorkspace({ localClusterUri: cluster.uri, - location: undefined, - accessRequests: { - isBarCollapsed: true, - pending: getEmptyPendingAccessRequest(), - }, documents: [ makeDocumentVnetDiagReport({ report: { @@ -289,7 +309,7 @@ describe('state persistence', () => { }, }), ], - }, + }), }, }; @@ -344,6 +364,7 @@ describe('setActiveWorkspace', () => { cluster, persistedWorkspaces: {}, }); + workspacesService.addWorkspace(cluster); // Resolve the modal immediately. jest @@ -381,6 +402,55 @@ describe('setActiveWorkspace', () => { expect(workspacesService.getRootClusterUri()).toBeUndefined(); }); + it('recreates a missing cluster from workspace proxy host before connecting', async () => { + const clusterUri = '/clusters/foo.example.com'; + const proxyHost = 'foo.example.com:443'; + const { workspacesService, modalsService, clustersService } = getTestSetup({ + cluster: undefined, + persistedWorkspaces: { + [clusterUri]: makePersistedWorkspace({ + proxyHost, + localClusterUri: clusterUri, + }), + }, + }); + workspacesService.restorePersistedState(); + expect(clustersService.findCluster(clusterUri)).toBeUndefined(); + + jest + .spyOn(modalsService, 'openRegularDialog') + .mockImplementation(dialog => { + if (dialog.kind === 'cluster-connect') { + dialog.onCancel(); + } else { + throw new Error(`Got unexpected dialog ${dialog.kind}`); + } + + return { + closeDialog: () => {}, + }; + }); + + const result = await workspacesService.setActiveWorkspace(clusterUri); + + expect(clustersService.addCluster).toHaveBeenCalledWith(proxyHost); + expect(clustersService.findCluster(clusterUri)).toEqual( + expect.objectContaining({ + uri: clusterUri, + proxyHost, + connected: false, + }) + ); + expect(modalsService.openRegularDialog).toHaveBeenCalledWith( + expect.objectContaining({ + kind: 'cluster-connect', + clusterUri, + }), + expect.any(AbortSignal) + ); + expect(result.isAtDesiredWorkspace).toBe(false); + }); + it('does not switch the workspace if the login modal gets closed', async () => { const cluster = makeRootCluster({ connected: false, @@ -423,6 +493,7 @@ describe('setActiveWorkspace', () => { cluster: rootCluster, persistedWorkspaces: {}, }); + workspacesService.addWorkspace(rootCluster); jest.spyOn(notificationsService, 'notifyError'); @@ -442,7 +513,7 @@ describe('setActiveWorkspace', () => { it('sets location to first document if location points to non-existing document when reopening documents', async () => { const cluster = makeRootCluster(); - const testWorkspace: PersistedWorkspace = { + const testWorkspace = makePersistedWorkspace({ localClusterUri: cluster.uri, documents: [ { @@ -457,7 +528,7 @@ describe('setActiveWorkspace', () => { }, ], location: '/docs/non-existing-doc', - }; + }); const { workspacesService, modalsService } = getTestSetup({ cluster, @@ -503,7 +574,7 @@ describe('setActiveWorkspace', () => { it('ongoing setActive call is canceled when the method is called again', async () => { const clusterFoo = makeRootCluster({ uri: '/clusters/foo' }); const clusterBar = makeRootCluster({ uri: '/clusters/bar' }); - const workspace1: PersistedWorkspace = { + const workspace1 = makePersistedWorkspace({ localClusterUri: clusterFoo.uri, documents: [ { @@ -513,7 +584,7 @@ describe('setActiveWorkspace', () => { }, ], location: '/docs/non-existing-doc', - }; + }); const { workspacesService } = getTestSetup({ cluster: [clusterFoo, clusterBar], @@ -534,7 +605,7 @@ describe('setActiveWorkspace', () => { it('opens the documents-reopen dialog in the same tick as setting rootClusterUri', async () => { const cluster = makeRootCluster(); - const testWorkspace: PersistedWorkspace = { + const testWorkspace = makePersistedWorkspace({ localClusterUri: cluster.uri, documents: [ { @@ -544,7 +615,7 @@ describe('setActiveWorkspace', () => { }, ], location: '/docs/terminal_shell_uri', - }; + }); const { workspacesService, modalsService } = getTestSetup({ cluster, @@ -585,6 +656,60 @@ describe('setActiveWorkspace', () => { }); }); +describe('clearWorkspace', () => { + it('preserves proxy host and color for later workspace reuse', () => { + const clusterFoo = makeRootCluster({ uri: '/clusters/foo' }); + const clusterBar = makeRootCluster({ uri: '/clusters/bar' }); + const { workspacesService } = getTestSetup({ + cluster: [clusterFoo, clusterBar], + persistedWorkspaces: {}, + }); + + workspacesService.restorePersistedState(); + workspacesService.changeWorkspaceColor(clusterFoo.uri, 'red'); + const previousProxyHost = workspacesService.getWorkspace( + clusterFoo.uri + ).proxyHost; + const previousLocation = workspacesService.getWorkspace( + clusterFoo.uri + ).location; + + workspacesService.clearWorkspace(clusterFoo.uri); + + const workspace = workspacesService.getWorkspace(clusterFoo.uri); + expect(workspace.color).toBe('red'); + expect(workspace.proxyHost).toBe(previousProxyHost); + expect(workspace.documents).toHaveLength(1); + expect(workspace.documents[0]).toEqual( + expect.objectContaining({ kind: 'doc.cluster' }) + ); + expect(workspace.location).not.toEqual(previousLocation); + }); +}); + +describe('updateWorkspaceProxyHost', () => { + it('updates the remembered proxy host for an existing workspace', () => { + const cluster = makeRootCluster({ + uri: '/clusters/foo.example.com', + proxyHost: 'foo.example.com:443', + }); + const { workspacesService } = getTestSetup({ + cluster, + persistedWorkspaces: {}, + }); + workspacesService.addWorkspace(cluster); + + workspacesService.updateWorkspaceProxyHost({ + uri: cluster.uri, + proxyHost: 'proxy.foo.example.com:443', + }); + + expect(workspacesService.getWorkspace(cluster.uri).proxyHost).toBe( + 'proxy.foo.example.com:443' + ); + }); +}); + function getTestSetup(options: { cluster: tshd.Cluster | tshd.Cluster[] | undefined; persistedWorkspaces: WorkspacesPersistedState['workspaces']; @@ -620,6 +745,16 @@ function getTestSetup(options: { ), getRootClusters: () => normalizedClusters, syncRootClustersAndCatchErrors: async () => {}, + addCluster: jest.fn(async proxy => { + const cluster = makeRootCluster({ + uri: `/clusters/${proxy.replace(/:\d+$/, '')}`, + proxyHost: proxy, + connected: false, + loggedInUser: undefined, + }); + normalizedClusters.push(cluster); + return cluster; + }), }; let clusterDocument: DocumentCluster; @@ -647,6 +782,7 @@ function getTestSetup(options: { return { workspacesService, modalsService, + clustersService, notificationsService, statePersistenceService, }; diff --git a/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.ts b/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.ts index 636dc7f69aa..e03e05366c8 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.ts @@ -27,6 +27,7 @@ import { UnifiedResourcePreferences, ViewMode, } from 'gen-proto-ts/teleport/userpreferences/v1/unified_resource_preferences_pb'; +import { getErrorMessage } from 'shared/utils/error'; import { arrayObjectIsEqual } from 'shared/utils/highbar'; import Logger from 'teleterm/logger'; @@ -112,6 +113,11 @@ export interface Workspace { * This field is not persisted to disk. */ hasDocumentsToReopen?: boolean; + /** + * The proxy address used to add this root cluster. It lets Connect reconnect + * a remembered workspace even if the tsh profile is no longer available. + */ + proxyHost: string; } export class WorkspacesService extends ImmutableStore { @@ -298,9 +304,9 @@ export class WorkspacesService extends ImmutableStore { /** * setActiveWorkspace changes the active workspace to that of the given root cluster. - * If the root cluster doesn't have a workspace yet, setActiveWorkspace creates a default - * workspace state for the cluster and then asks the user about restoring documents from the - * previous session if there are any. + * The workspace must already exist. If its cluster is missing from the cluster store, + * setActiveWorkspace tries to add it back using the proxy host saved in the workspace state. + * It then asks the user about restoring documents from the previous session if there are any. * Only one call can be executed at a time. Any ongoing call is canceled when a new one is initiated. * * setActiveWorkspace never returns a rejected promise on its own. @@ -318,8 +324,8 @@ export class WorkspacesService extends ImmutableStore { * workspace of the given cluster. * * setActiveWorkspace never rejects on its own. However, it may fail to switch to the workspace - * if the user closes the cluster connect dialog or if the cluster with the given clusterUri - * wasn't found. + * if the workspace or cluster cannot be found, the cluster cannot be added back, or the user + * closes the cluster connect dialog. * * Callsites which don't check this return value were most likely written before this field was * added. They operate with the assumption that by the time the program gets to the @@ -339,16 +345,32 @@ export class WorkspacesService extends ImmutableStore { return { isAtDesiredWorkspace: true }; } + const workspace = this.getWorkspace(clusterUri); + if (!workspace) { + this.logger.error( + `The workspace for cluster ${clusterUri} does not exist` + ); + this.notificationsService.notifyError({ + title: `Could not switch to cluster ${routing.parseClusterName(clusterUri)}`, + }); + return { isAtDesiredWorkspace: false }; + } + let cluster = this.clustersService.findCluster(clusterUri); if (!cluster) { - this.notificationsService.notifyError({ - title: 'Could not set cluster as active', - description: `Cluster with URI ${clusterUri} does not exist`, - }); - this.logger.warn( - `Could not find cluster with uri ${clusterUri} when changing active cluster` - ); - return { isAtDesiredWorkspace: false }; + const proxyHost = workspace.proxyHost; + try { + cluster = await this.clustersService.addCluster(proxyHost); + } catch (err) { + this.logger.error('Failed to re-add cluster', err); + + this.notificationsService.notifyError({ + title: `Could not reconnect to ${proxyHost}. Check that the proxy is reachable and try again.`, + description: getErrorMessage(err), + }); + + return { isAtDesiredWorkspace: false }; + } } if (cluster.profileStatusError) { @@ -392,20 +414,12 @@ export class WorkspacesService extends ImmutableStore { return { isAtDesiredWorkspace: false }; } } - // If we don't have a workspace for this cluster, add it. - // TODO(gzdunek): Creating a workspace here might not be necessary - // after we started calling workspacesService.addWorkspace in ClusterAdd. + this.setState(draftState => { - if (!draftState.workspaces[clusterUri]) { - draftState.workspaces[clusterUri] = getWorkspaceDefaultState( - clusterUri, - draftState.workspaces - ); - } draftState.rootClusterUri = clusterUri; }); - const { hasDocumentsToReopen } = this.getWorkspace(clusterUri); + const { hasDocumentsToReopen } = workspace; if (!hasDocumentsToReopen) { return { isAtDesiredWorkspace: true }; } @@ -445,18 +459,44 @@ export class WorkspacesService extends ImmutableStore { return { isAtDesiredWorkspace: true }; } - addWorkspace(clusterUri: RootClusterUri): void { - if (this.state.workspaces[clusterUri]) { + addWorkspace({ + uri, + proxyHost, + }: { + uri: RootClusterUri; + proxyHost: string; + }): void { + if (this.state.workspaces[uri]) { + this.updateWorkspaceProxyHost({ uri, proxyHost }); return; } + this.setState(draftState => { - draftState.workspaces[clusterUri] = getWorkspaceDefaultState( - clusterUri, - draftState.workspaces + draftState.workspaces[uri] = getWorkspaceDefaultState( + uri, + draftState.workspaces, + { proxyHost } ); }); } + updateWorkspaceProxyHost({ + uri, + proxyHost, + }: { + uri: RootClusterUri; + proxyHost: string; + }): void { + const workspace = this.state.workspaces[uri]; + if (!workspace || !proxyHost || workspace.proxyHost === proxyHost) { + return; + } + + this.setState(draftState => { + draftState.workspaces[uri].proxyHost = proxyHost; + }); + } + removeWorkspace(clusterUri: RootClusterUri): void { this.setState(draftState => { delete draftState.workspaces[clusterUri]; @@ -468,9 +508,11 @@ export class WorkspacesService extends ImmutableStore { clearWorkspace(clusterUri: RootClusterUri): void { this.setState(draftState => { - draftState.workspaces[clusterUri] = getWorkspaceDefaultState( + const currentWorkspace = draftState.workspaces[clusterUri]; + draftState.workspaces[clusterUri] = getClearedWorkspaceState( clusterUri, - draftState.workspaces + draftState.workspaces, + currentWorkspace ); }); this.restoredState = produce(this.restoredState, draftState => { @@ -524,22 +566,47 @@ export class WorkspacesService extends ImmutableStore { // Make the restored state immutable. this.restoredState = produce(restoredState, () => {}); - const restoredWorkspaces = this.clustersService - .getRootClusters() - // Start restoring clusters from the ones that already have a workspace. - // The algorithm that assigns a color in getWorkspaceDefaultState needs - // to know all used colors. + const restoredWorkspaceUris = Object.keys(this.restoredState.workspaces); + const workspaceUris = Array.from( + new Set([ + ...restoredWorkspaceUris, + ...this.clustersService.getRootClusters().map(cluster => cluster.uri), + ]) + ); + + const restoredWorkspaces = workspaceUris + // Restore persisted workspaces first, so their colors are reserved before + // assigning colors to newly discovered clusters. .toSorted((a, b) => { - const hasA = !!this.restoredState.workspaces[a.uri]; - const hasB = !!this.restoredState.workspaces[b.uri]; + const hasA = !!this.restoredState.workspaces[a]; + const hasB = !!this.restoredState.workspaces[b]; return hasB === hasA ? 0 : hasA ? -1 : 1; }) - .reduce((workspaces, cluster) => { - const restoredWorkspace = this.restoredState.workspaces[cluster.uri]; - workspaces[cluster.uri] = getWorkspaceDefaultState( - cluster.uri, + .reduce>((workspaces, rootClusterUri) => { + const restoredWorkspace = this.restoredState.workspaces[rootClusterUri]; + const cluster = this.clustersService.findCluster(rootClusterUri); + const proxyHost = cluster?.proxyHost ?? restoredWorkspace?.proxyHost; + // Skip workspaces with no known proxy address. If the cluster is missing and the workspace has no + // saved proxy host, Connect cannot add the cluster again. + if (!proxyHost) { + return workspaces; + } + + const overrides = { ...restoredWorkspace, proxyHost }; + + if (!cluster) { + workspaces[rootClusterUri] = getClearedWorkspaceState( + rootClusterUri, + workspaces, + overrides + ); + return workspaces; + } + + workspaces[rootClusterUri] = getWorkspaceDefaultState( + rootClusterUri, workspaces, - restoredWorkspace + overrides ); return workspaces; }, {}); @@ -646,6 +713,7 @@ export class WorkspacesService extends ImmutableStore { const documentsToPersist = getDocumentsToPersist(workspace.documents); stateToSave.workspaces[w] = { + proxyHost: workspace.proxyHost, localClusterUri: workspace.localClusterUri, location: workspace.location, color: workspace.color, @@ -745,10 +813,11 @@ function getLocationToRestore( function getWorkspaceDefaultState( rootClusterUri: RootClusterUri, workspaces: Record, - restoredWorkspace?: Immutable + overrides?: Partial> ): Workspace { const defaultDocument = createClusterDocument({ clusterUri: rootClusterUri }); const defaultWorkspace: Workspace = { + proxyHost: '', accessRequests: { pending: getEmptyPendingAccessRequest(), isBarCollapsed: false, @@ -761,27 +830,59 @@ function getWorkspaceDefaultState( unifiedResourcePreferences: parseUnifiedResourcePreferences(undefined), color: parseWorkspaceColor(undefined, workspaces), }; - if (!restoredWorkspace) { + if (!overrides) { return defaultWorkspace; } - defaultWorkspace.localClusterUri = restoredWorkspace.localClusterUri; - defaultWorkspace.unifiedResourcePreferences = parseUnifiedResourcePreferences( - restoredWorkspace.unifiedResourcePreferences - ); - defaultWorkspace.color = parseWorkspaceColor( - restoredWorkspace.color, - workspaces - ); - defaultWorkspace.connectMyComputer = restoredWorkspace.connectMyComputer; + if (overrides.localClusterUri) { + defaultWorkspace.localClusterUri = overrides.localClusterUri; + } + + if (overrides.unifiedResourcePreferences) { + defaultWorkspace.unifiedResourcePreferences = + parseUnifiedResourcePreferences(overrides.unifiedResourcePreferences); + } + + if (overrides.color) { + defaultWorkspace.color = parseWorkspaceColor(overrides.color, workspaces); + } + + if (overrides.proxyHost) { + defaultWorkspace.proxyHost = overrides.proxyHost; + } + + if (overrides.connectMyComputer) { + defaultWorkspace.connectMyComputer = overrides.connectMyComputer; + } + defaultWorkspace.hasDocumentsToReopen = hasDocumentsToReopen({ - previousDocuments: restoredWorkspace.documents, + previousDocuments: overrides.documents, currentDocuments: defaultWorkspace.documents, }); return defaultWorkspace; } +/** + * Returns a cleared workspace state while preserving stable workspace metadata. + * This is used after logout, so session-related state is discarded, but `color` + * and `proxyHost` are kept for the remembered cluster. + */ +function getClearedWorkspaceState( + rootClusterUri: RootClusterUri, + workspaces: Record, + currentWorkspaceState: Partial> | undefined +) { + return getWorkspaceDefaultState( + rootClusterUri, + workspaces, + currentWorkspaceState && { + proxyHost: currentWorkspaceState.proxyHost, + color: currentWorkspaceState.color, + } + ); +} + // TODO(gzdunek): Parse the entire workspace state read from disk like below. function parseUnifiedResourcePreferences( unifiedResourcePreferences: unknown