Connect My Computer: Improve copy and UI consistency (#32841)

* CMC Status: Use namespace import for icons

* CMC Status: Use vertical three dots icon for the menu

This makes it consistent with the vertical three dots icon we use for
additional actions in the top bar.

* Change the name of agent cleanup daemon logfile

"cleanup" alone is too vague.

* Pass --insecure to agent if Connect runs in insecure mode

* Update copy related to security

* Sprinkle some whitespace in connectMyComputer context

* Explain why we can't remove connections before killing the agent

* Mention file URL in download error message

Without this, the UI would just show the previous message, but the user
would have no idea what exactly was being downloaded. This was especially
the case when updating the agent after it was already set up.

web/packages/teleterm/src/agentCleanupDaemon/agentCleanupDaemon.js

@@ -83,7 +83,7 @@ const logger = createLogger({
       maxsize: 4194304, // 4 MB - max size of a single file
       maxFiles: 5,
       dirname: logsDir,
-      filename: 'cleanup.log',
+      filename: 'agent-cleanup.log',
     }),
   ],
 }).child({

web/packages/teleterm/src/mainProcess/agentDownloader/fileDownloader.test.ts

@@ -120,7 +120,7 @@ test('rejects a promise when an unexpected error occurs', async () => {
   const result = downloader.run(URL, DOWNLOAD_DIR);
 
   expect(browserWindow.webContents.downloadURL).toHaveBeenCalledWith(URL);
-  await expect(result).rejects.toThrow(`Download failed.`);
+  await expect(result).rejects.toThrow(`Failed to download ${URL}`);
   expect(browserWindow.setProgressBar).toHaveBeenCalledWith(-1, {
     mode: 'error',
   });

web/packages/teleterm/src/mainProcess/agentDownloader/fileDownloader.ts

@@ -99,11 +99,7 @@ export class FileDownloader implements IFileDownloader {
             // TODO(gzdunek): electron doesn't expose much information about why the download failed.
             // Fortunately, there is a PR in works that will add more info https://github.com/electron/electron/pull/38859.
             // Use DownloadItem.getLastReason() when it gets merged.
-            onDownloadError(
-              new Error(
-                `Download failed. Requested file may not exist or is temporarily unavailable.`
-              )
-            );
+            onDownloadError(new Error(`Failed to download ${item.getURL()}`));
             break;
           case 'cancelled':
             onDownloadError(new Error('Download was cancelled.'));

web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts

@@ -88,6 +88,7 @@ test('agent process and cleanup daemon start with correct arguments', async () =
       agentBinaryPath,
       'start',
       `--config=${userDataDir}/agents/cluster.local/config.yaml`,
+      '--insecure',
     ]);
     expect(cleanupDaemon.spawnargs).toEqual([
       process.argv[0], // path to Node.js bin

web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts

@@ -71,6 +71,7 @@ export class AgentRunner {
       'start',
       `--config=${configFile}`,
       this.settings.isLocalBuild && '--skip-version-check',
+      this.settings.tshd.insecure && '--insecure',
     ].filter(Boolean);
 
     this.logger.info(

web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx

@@ -89,6 +89,13 @@ function Information(props: { onSetUpAgentClick(): void }) {
         access your computer as <strong>{systemUsername}</strong>.
         <br />
         <br />
+        Note that users with administrator privileges can assign that role to
+        themselves or craft another role which grants access to the node. We
+        recommend using Connect My Computer only in scenarios where no other
+        user could plausibly gain access to the node, such as when exploring a
+        Teleport cluster as its only user or in a home lab.
+        <br />
+        <br />
         Your computer will be shared while Teleport Connect is open. To stop
         sharing, close Teleport Connect or stop the agent through the Connect My
         Computer tab. Sharing will resume on app restart, unless you stop the

web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.tsx

@@ -31,7 +31,7 @@ import { Transition } from 'react-transition-group';
 
 import { makeLabelTag } from 'teleport/components/formatters';
 import { MenuIcon } from 'shared/components/MenuAction';
-import { CircleCheck, Laptop, Moon, Warning } from 'design/Icon';
+import * as icons from 'design/Icon';
 import Indicator from 'design/Indicator';
 
 import {
@@ -192,11 +192,12 @@ export function DocumentConnectMyComputerStatus(
                 display: flex;
               `}
             >
-              <Laptop mr={2} />
+              <icons.Laptop mr={2} />
               {/** The node name can be changed, so it might be different from the system hostname. */}
               {agentNode?.hostname || hostname}
             </Text>
             <MenuIcon
+              Icon={icons.MoreVert}
               buttonIconProps={{
                 css: css`
                   border-radius: ${props => props.theme.space[1]}px;
@@ -284,14 +285,16 @@ export function DocumentConnectMyComputerStatus(
             <>
               {isRunning ? (
                 <Text>
-                  Any cluster user with the role <strong>{roleName}</strong> can
-                  now access your computer as <strong>{systemUsername}</strong>.
+                  Cluster users with the role <strong>{roleName}</strong> and
+                  users with administrator privileges can now access your
+                  computer as <strong>{systemUsername}</strong>.
                 </Text>
               ) : (
                 <Text>
-                  Connecting your computer will allow any cluster user with the
-                  role <strong>{roleName}</strong> to access it as an SSH
-                  resource with the user <strong>{systemUsername}</strong>.
+                  Starting the agent will allow clusters users with the role{' '}
+                  <strong>{roleName}</strong> and users with administrator
+                  privileges to access it as an SSH resource as the user{' '}
+                  <strong>{systemUsername}</strong>.
                 </Text>
               )}
               {showConnectAndStopAgentButtons ? (
@@ -431,13 +434,13 @@ function prettifyCurrentAction(currentAction: CurrentAction): {
       switch (currentAction.agentProcessState.status) {
         case 'not-started': {
           return {
-            Icon: Moon,
+            Icon: icons.Moon,
             title: 'Agent not running',
           };
         }
         case 'running': {
           return {
-            Icon: props => <CircleCheck {...props} color="success" />,
+            Icon: props => <icons.CircleCheck {...props} color="success" />,
             title: 'Agent running',
           };
         }
@@ -447,7 +450,7 @@ function prettifyCurrentAction(currentAction: CurrentAction): {
 
           if (exitedSuccessfully) {
             return {
-              Icon: Moon,
+              Icon: icons.Moon,
               title: 'Agent not running',
             };
           } else {
@@ -515,7 +518,7 @@ function prettifyCurrentAction(currentAction: CurrentAction): {
         }
         case 'success': {
           return {
-            Icon: CircleCheck,
+            Icon: icons.CircleCheck,
             title: 'Agent removed',
             error: currentAction.attempt.statusText,
           };
@@ -528,7 +531,7 @@ function prettifyCurrentAction(currentAction: CurrentAction): {
   }
 }
 
-const StyledWarning = styled(Warning).attrs({
+const StyledWarning = styled(icons.Warning).attrs({
   color: 'error.main',
 })``;
 

web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx

@@ -232,7 +232,9 @@ export const ConnectMyComputerContextProvider: FC<{
   const [killAgentAttempt, killAgent] = useAsync(
     useCallback(async () => {
       setCurrentActionKind('kill');
+
       await connectMyComputerService.killAgent(rootClusterUri);
+
       setCurrentActionKind('observe-process');
       workspacesService.setConnectMyComputerAutoStart(rootClusterUri, false);
     }, [connectMyComputerService, rootClusterUri, workspacesService])
@@ -241,6 +243,7 @@ export const ConnectMyComputerContextProvider: FC<{
   const markAgentAsConfigured = useCallback(() => {
     setAgentConfiguredAttempt(makeSuccessAttempt(true));
   }, [setAgentConfiguredAttempt]);
+
   const markAgentAsNotConfigured = useCallback(() => {
     setDownloadAgentAttempt(makeEmptyAttempt());
     setAgentConfiguredAttempt(makeSuccessAttempt(false));
@@ -267,10 +270,12 @@ export const ConnectMyComputerContextProvider: FC<{
 
   const [removeAgentAttempt, removeAgent] = useAsync(
     useCallback(async () => {
+      // killAgent sets the current action to 'kill'.
       const [, error] = await killAgent();
       if (error) {
         throw error;
       }
+
       setCurrentActionKind('remove');
 
       let hasAccessDeniedError = false;
@@ -299,6 +304,18 @@ export const ConnectMyComputerContextProvider: FC<{
 
       // We have to remove connections before removing the agent directory, because
       // we get the node UUID from the that directory.
+      //
+      // Theoretically, removing connections only at this stage means that if there are active
+      // connections from the app at the time of killing the agent above, the shutdown of the agent
+      // will take a couple of extra seconds while the agent waits for the connections to close.
+      // However, we'd have to remove the connections before calling `killAgent` above and this
+      // messes up error handling somewhat. `removeConnections` would have to be executed after the
+      // current action is set to 'kill' so that any errors thrown by `removeConnections` are
+      // correctly reported in the UI.
+      //
+      // Otherwise, if `removeConnections` was called before `killAgent` and the function threw an
+      // error, it'd simply be swallowed. It'd be shown once the current action is set to 'remove',
+      // but this would never happen because of the error.
       await removeConnections();
       ctx.workspacesService.removeConnectMyComputerState(rootClusterUri);
       await ctx.connectMyComputerService.removeAgentDirectory(rootClusterUri);