[v14] Connect My Computer: Do not omit checking all permissions when …

…the agent is configured (#32034)

* Do not omit checking all permissions when the agent is configured

* Check if the feature flag is enabled and the agent is configured in `connectMyComputerContext.canUse`

* Add a comment explaining `isAgentConfigured`

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

@@ -229,28 +229,43 @@ test('starts the agent automatically if the workspace autoStart flag is true', a
 describe('canUse', () => {
   const cases = [
     {
-      name: 'should be true when the user has permissions',
+      name: 'should be true when the user has permissions and the feature flag is enabled',
       hasPermissions: true,
+      isFeatureFlagEnabled: true,
       isAgentConfigured: false,
       expected: true,
     },
     {
-      name: 'should be true when the user does not have permissions, but the agent has been configured',
+      name: 'should be true when the user does not have permissions, but the agent has been configured and the feature flag is enabled',
       hasPermissions: false,
+      isFeatureFlagEnabled: true,
       isAgentConfigured: true,
       expected: true,
     },
     {
-      name: 'should be false when the user does not have permissions and the agent has not been configured',
+      name: 'should be false when the user does not have permissions, the agent has not been configured and the feature flag is enabled',
       hasPermissions: false,
       isAgentConfigured: false,
+      isFeatureFlagEnabled: true,
+      expected: false,
+    },
+    {
+      name: 'should be false when the user has permissions and the agent is configured but the feature flag is disabled',
+      hasPermissions: true,
+      isAgentConfigured: true,
+      isFeatureFlagEnabled: false,
       expected: false,
     },
   ];
 
   test.each(cases)(
     '$name',
-    async ({ hasPermissions, isAgentConfigured, expected }) => {
+    async ({
+      hasPermissions,
+      isAgentConfigured,
+      isFeatureFlagEnabled,
+      expected,
+    }) => {
       const { appContext, rootCluster } =
         getMocksWithConnectMyComputerEnabled();
       // update Connect My Computer permissions
@@ -259,6 +274,9 @@ describe('canUse', () => {
           rootCluster.uri
         ).loggedInUser.acl.tokens.create = hasPermissions;
       });
+      appContext.configService = createMockConfigService({
+        'feature.connectMyComputer': isFeatureFlagEnabled,
+      });
       const isAgentConfigFileCreated = Promise.resolve(isAgentConfigured);
       jest
         .spyOn(appContext.connectMyComputerService, 'isAgentConfigFileCreated')

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

@@ -40,7 +40,7 @@ import { Server } from 'teleterm/services/tshd/types';
 
 import { assertUnreachable } from '../utils';
 
-import { canUseConnectMyComputer } from './permissions';
+import { hasConnectMyComputerPermissions } from './permissions';
 
 import type {
   AgentProcessState,
@@ -113,15 +113,19 @@ export const ConnectMyComputerContextProvider: FC<{
     isAgentConfiguredAttempt.data;
 
   const rootCluster = clustersService.findCluster(props.rootClusterUri);
-  const canUse = useMemo(
-    () =>
-      canUseConnectMyComputer(
-        rootCluster,
-        configService,
-        mainProcessClient.getRuntimeSettings()
-      ) || isAgentConfigured,
-    [configService, isAgentConfigured, mainProcessClient, rootCluster]
-  );
+  const canUse = useMemo(() => {
+    const isFeatureFlagEnabled = configService.get(
+      'feature.connectMyComputer'
+    ).value;
+    const hasPermissions = hasConnectMyComputerPermissions(
+      rootCluster,
+      mainProcessClient.getRuntimeSettings()
+    );
+
+    // We check `isAgentConfigured`, because the user should always have access to the agent after configuring it.
+    // https://github.com/gravitational/teleport/blob/master/rfd/0133-connect-my-computer.md#access-to-ui-and-autostart
+    return isFeatureFlagEnabled && (hasPermissions || isAgentConfigured);
+  }, [configService, isAgentConfigured, mainProcessClient, rootCluster]);
 
   const [currentActionKind, setCurrentActionKind] =
     useState<CurrentAction['kind']>('observe-process');

web/packages/teleterm/src/ui/ConnectMyComputer/permissions.test.ts

@@ -14,56 +14,43 @@
  * limitations under the License.
  */
 
-import { createMockConfigService } from 'teleterm/services/config/fixtures/mocks';
 import { makeRuntimeSettings } from 'teleterm/mainProcess/fixtures/mocks';
 import { Platform } from 'teleterm/mainProcess/types';
 import {
   makeRootCluster,
   makeLoggedInUser,
 } from 'teleterm/services/tshd/testHelpers';
 
-import { canUseConnectMyComputer } from './permissions';
+import { hasConnectMyComputerPermissions } from './permissions';
 
 const testCases: {
   name: string;
   platform: Platform;
   canCreateToken: boolean;
-  isFeatureFlagEnabled: boolean;
   expect: boolean;
 }[] = [
   {
-    name: 'darwin, can create token, feature flag enabled',
+    name: 'should be true when OS is darwin and can create token',
     platform: 'darwin',
     canCreateToken: true,
-    isFeatureFlagEnabled: true,
     expect: true,
   },
   {
-    name: 'linux, can create token, feature flag enabled',
+    name: 'should be true when OS is  linux and can create token',
     platform: 'linux',
     canCreateToken: true,
-    isFeatureFlagEnabled: true,
     expect: true,
   },
   {
-    name: 'windows, can create token, feature flag enabled',
+    name: 'should be false when OS is windows and can create token',
     platform: 'win32',
     canCreateToken: true,
-    isFeatureFlagEnabled: true,
     expect: false,
   },
   {
-    name: 'darwin, cannot create token, feature flag enabled',
+    name: 'should be false when OS is darwin and cannot create token',
     platform: 'darwin',
     canCreateToken: false,
-    isFeatureFlagEnabled: true,
-    expect: false,
-  },
-  {
-    name: 'darwin, can create token, feature flag not enabled',
-    platform: 'darwin',
-    canCreateToken: true,
-    isFeatureFlagEnabled: false,
     expect: false,
   },
 ];
@@ -83,15 +70,8 @@ test.each(testCases)('$name', testCase => {
       },
     }),
   });
-  const configService = createMockConfigService({
-    'feature.connectMyComputer': testCase.isFeatureFlagEnabled,
-  });
   const runtimeSettings = makeRuntimeSettings({ platform: testCase.platform });
 
-  const isPermitted = canUseConnectMyComputer(
-    cluster,
-    configService,
-    runtimeSettings
-  );
+  const isPermitted = hasConnectMyComputerPermissions(cluster, runtimeSettings);
   expect(isPermitted).toEqual(testCase.expect);
 });

web/packages/teleterm/src/ui/ConnectMyComputer/permissions.ts

@@ -15,7 +15,6 @@
  */
 
 import { Cluster } from 'teleterm/services/tshd/types';
-import { ConfigService } from 'teleterm/services/config';
 import { RuntimeSettings } from 'teleterm/mainProcess/types';
 
 import * as tsh from 'teleterm/services/tshd/types';
@@ -29,9 +28,8 @@ import * as tsh from 'teleterm/services/tshd/types';
  * This will make it easier to understand what the user can and cannot do without having to jump around the code base.
  * https://github.com/gravitational/teleport/pull/28346#discussion_r1246653846
  * */
-export function canUseConnectMyComputer(
+export function hasConnectMyComputerPermissions(
   rootCluster: Cluster,
-  configService: ConfigService,
   runtimeSettings: RuntimeSettings
 ): boolean {
   if (rootCluster.leaf) {
@@ -45,7 +43,6 @@ export function canUseConnectMyComputer(
   return (
     isUnix &&
     rootCluster.loggedInUser?.acl?.tokens.create &&
-    rootCluster.loggedInUser?.userType == tsh.UserType.USER_TYPE_LOCAL &&
-    configService.get('feature.connectMyComputer').value
+    rootCluster.loggedInUser?.userType == tsh.UserType.USER_TYPE_LOCAL
   );
 }