feat: 🎸 update connect logic for targets with static addresses (#1874)

* feat: 🎸 update connect logic for targets with static addresses

✅ Closes: ICU-10430

* refactor: 💡 remove console logs

* refactor: 💡 add missing awaits

ui/desktop/app/controllers/scopes/scope/projects/targets/target.js

@@ -0,0 +1,14 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: MPL-2.0
+ */
+
+import Controller from '@ember/controller';
+
+export default class ScopesScopeProjectsTargetsTargetController extends Controller {
+  // =attributes
+
+  queryParams = [{ isConnecting: { type: 'boolean' } }];
+
+  isConnecting = false;
+}

ui/desktop/app/routes/scopes/scope/projects/targets.js

@@ -7,15 +7,12 @@ import Route from '@ember/routing/route';
 import { inject as service } from '@ember/service';
 import { action } from '@ember/object';
 import { resourceFilter } from 'core/decorators/resource-filter';
-import { loading } from 'ember-loading';
 
 export default class ScopesScopeProjectsTargetsRoute extends Route {
   // =services
 
   @service can;
   @service clusterUrl;
-  @service confirm;
-  @service ipc;
   @service resourceFilterStore;
   @service router;
   @service session;
@@ -74,58 +71,6 @@ export default class ScopesScopeProjectsTargetsRoute extends Route {
     session.acknowledged = true;
   }
 
-  /**
-   * Establish a session to current target.
-   * @param {TargetModel} model
-   * @param {HostModel} host
-   */
-  @action
-  @loading
-  async connect(model, host) {
-    // TODO: Connect: move this logic into the target model
-    try {
-      // Check for CLI
-      const cliExists = await this.ipc.invoke('cliExists');
-      if (!cliExists) throw new Error('Cannot find Boundary CLI.');
-
-      const options = {
-        target_id: model.id,
-        token: this.session.data.authenticated.token,
-      };
-
-      if (host) options.host_id = host.id;
-
-      // Create target session
-      const connectionDetails = await this.ipc.invoke('connect', options);
-
-      // Associate the connection details with the session
-      const { session_id, address, port, credentials } = connectionDetails;
-      const session = await this.store.findRecord('session', session_id);
-
-      // Flag the session has been open in the desktop client
-      session.started_desktop_client = true;
-      // Update the session record with proxy information from the CLI
-      // In the future, it may make sense to push this off to the API so that
-      // we don't have to manually persist the proxy details.
-      session.proxy_address = address;
-      session.proxy_port = port;
-      if (credentials) {
-        credentials.forEach((cred) => session.addCredential(cred));
-      }
-
-      await this.router.transitionTo(
-        'scopes.scope.projects.sessions.session',
-        session
-      );
-    } catch (e) {
-      this.confirm
-        .confirm(e.message, { isConnectError: true })
-        // Retry
-        .then(() => this.connect(model))
-        .catch(() => null /* no op */);
-    }
-  }
-
   /**
    * Sets the specified resource filter field to the specified value.
    * @param {string} field

ui/desktop/app/routes/scopes/scope/projects/targets/target.js

@@ -5,11 +5,17 @@
 
 import Route from '@ember/routing/route';
 import { inject as service } from '@ember/service';
+import { action } from '@ember/object';
+import { loading } from 'ember-loading';
 
 export default class ScopesScopeProjectsTargetsTargetRoute extends Route {
   // =services
 
   @service store;
+  @service confirm;
+  @service ipc;
+  @service router;
+  @service session;
 
   // =methods
 
@@ -17,9 +23,82 @@ export default class ScopesScopeProjectsTargetsTargetRoute extends Route {
    * Load a target
    * @param {object} params
    * @param {string} params.target_id
+   * @param {bool} params.isConnecting
    * @return {TargetModel}
    */
-  model({ target_id }) {
-    return this.store.findRecord('target', target_id, { reload: true });
+  async model({ target_id, isConnecting }) {
+    const target = await this.store.findRecord('target', target_id, {
+      reload: true,
+    });
+
+    if (isConnecting) {
+      await this.preConnect(target);
+    }
+
+    return target;
+  }
+
+  /**
+   * Determine if we show host modal or quick connect based on target attributes.
+   * @param {TargetModel} model
+   */
+  @action
+  @loading
+  async preConnect(target) {
+    if (target.address) {
+      await this.connect(target);
+    }
+  }
+
+  /**
+   * Establish a session to current target.
+   * @param {TargetModel} model
+   * @param {HostModel} host
+   */
+  @action
+  @loading
+  async connect(model, host) {
+    // TODO: Connect: move this logic into the target model
+    try {
+      // Check for CLI
+      const cliExists = await this.ipc.invoke('cliExists');
+      if (!cliExists) throw new Error('Cannot find Boundary CLI.');
+
+      const options = {
+        target_id: model.id,
+        token: this.session.data.authenticated.token,
+      };
+
+      if (host) options.host_id = host.id;
+
+      // Create target session
+      const connectionDetails = await this.ipc.invoke('connect', options);
+
+      // Associate the connection details with the session
+      const { session_id, address, port, credentials } = connectionDetails;
+      const session = await this.store.findRecord('session', session_id);
+
+      // Flag the session has been open in the desktop client
+      session.started_desktop_client = true;
+      // Update the session record with proxy information from the CLI
+      // In the future, it may make sense to push this off to the API so that
+      // we don't have to manually persist the proxy details.
+      session.proxy_address = address;
+      session.proxy_port = port;
+      if (credentials) {
+        credentials.forEach((cred) => session.addCredential(cred));
+      }
+
+      await this.router.transitionTo(
+        'scopes.scope.projects.sessions.session',
+        session
+      );
+    } catch (e) {
+      this.confirm
+        .confirm(e.message, { isConnectError: true })
+        // Retry
+        .then(() => this.connect(model))
+        .catch(() => null /* no op */);
+    }
   }
 }

ui/desktop/app/templates/scopes/scope/projects/targets/index.hbs

@@ -115,8 +115,10 @@
               <span class='hds-font-weight-semibold'>
                 {{#if (can 'read model' B.data)}}
                   <LinkTo
+                    data-test-visit-target
                     @route='scopes.scope.projects.targets.target'
                     @model={{B.data.id}}
+                    @query={{hash isConnecting=false}}
                   >
                     {{B.data.displayName}}
                   </LinkTo>
@@ -159,9 +161,12 @@
             <B.Td>
               {{#if (can 'connect target' B.data)}}
                 <Hds::Button
+                  data-test-targets-connect-button
                   @text={{t 'resources.session.actions.connect'}}
                   @color='secondary'
-                  {{on 'click' (route-action 'connect' B.data)}}
+                  @route='scopes.scope.projects.targets.target'
+                  @model={{B.data.id}}
+                  @query={{hash isConnecting=true}}
                 />
               {{/if}}
             </B.Td>

ui/desktop/app/templates/scopes/scope/projects/targets/target.hbs

@@ -20,7 +20,7 @@
       @text={{t 'resources.session.actions.connect'}}
       @icon='entry-point'
       @iconPosition='trailing'
-      {{on 'click' (route-action 'connect' @model)}}
+      {{on 'click' (route-action 'preConnect' @model)}}
     />
   </page.actions>
 

ui/desktop/tests/acceptance/projects/sessions/session-test.js

@@ -91,7 +91,7 @@ module('Acceptance | projects | sessions | session', function (hooks) {
     });
     instances.target = this.server.create(
       'target',
-      { scope: instances.scopes.project },
+      { scope: instances.scopes.project, address: 'localhost' },
       'withAssociations'
     );
     instances.session = this.server.create(

ui/desktop/tests/acceptance/projects/targets/index-test.js

@@ -0,0 +1,200 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: MPL-2.0
+ */
+
+import { module, test } from 'qunit';
+import { visit, currentURL, click, find, findAll } from '@ember/test-helpers';
+import { setupApplicationTest } from 'ember-qunit';
+import setupMirage from 'ember-cli-mirage/test-support/setup-mirage';
+import { Response } from 'miragejs';
+import a11yAudit from 'ember-a11y-testing/test-support/audit';
+import sinon from 'sinon';
+import {
+  currentSession,
+  authenticateSession,
+  invalidateSession,
+} from 'ember-simple-auth/test-support';
+
+module('Acceptance | projects | targets', function (hooks) {
+  setupApplicationTest(hooks);
+  setupMirage(hooks);
+
+  let mockIPC;
+  let messageHandler;
+
+  const instances = {
+    scopes: {
+      global: null,
+      org: null,
+      project: null,
+    },
+    authMethods: {
+      global: null,
+    },
+    target: null,
+  };
+
+  const stubs = {
+    global: null,
+    org: null,
+    ipsService: null,
+  };
+
+  const urls = {
+    index: '/',
+    clusterUrl: '/cluster-url',
+    scopes: {
+      global: null,
+      org: null,
+    },
+    authenticate: {
+      global: null,
+      methods: {
+        global: null,
+      },
+    },
+    projects: null,
+    targets: null,
+    target: null,
+    sessions: null,
+  };
+
+  const setDefaultClusterUrl = (test) => {
+    const windowOrigin = window.location.origin;
+    const clusterUrl = test.owner.lookup('service:clusterUrl');
+    clusterUrl.rendererClusterUrl = windowOrigin;
+  };
+
+  hooks.beforeEach(function () {
+    authenticateSession();
+
+    // create scopes
+    instances.scopes.global = this.server.create('scope', { id: 'global' });
+    stubs.global = { id: 'global', type: 'global' };
+    instances.scopes.org = this.server.create('scope', {
+      type: 'org',
+      scope: stubs.global,
+    });
+    stubs.org = { id: instances.scopes.org.id, type: 'org' };
+    instances.scopes.project = this.server.create('scope', {
+      type: 'project',
+      scope: stubs.org,
+    });
+    stubs.project = { id: instances.scopes.project.id, type: 'project' };
+
+    instances.authMethods.global = this.server.create('auth-method', {
+      scope: instances.scopes.global,
+    });
+
+    instances.hostCatalog = this.server.create(
+      'host-catalog',
+      { scope: instances.scopes.project },
+      'withChildren'
+    );
+    instances.target = this.server.create(
+      'target',
+      {
+        scope: instances.scopes.project,
+      },
+      'withAssociations'
+    );
+
+    instances.session = this.server.create(
+      'session',
+      {
+        scope: instances.scopes.project,
+        status: 'active',
+      },
+      'withAssociations'
+    );
+
+    urls.scopes.global = `/scopes/${instances.scopes.global.id}`;
+    urls.scopes.org = `/scopes/${instances.scopes.org.id}`;
+    urls.authenticate.global = `${urls.scopes.global}/authenticate`;
+    urls.authenticate.methods.global = `${urls.authenticate.global}/${instances.authMethods.global.id}`;
+    urls.projects = `${urls.scopes.org}/projects`;
+    urls.targets = `${urls.projects}/targets`;
+    urls.target = `${urls.targets}/${instances.target.id}`;
+
+    class MockIPC {
+      clusterUrl = null;
+
+      invoke(method, payload) {
+        return this[method](payload);
+      }
+
+      getClusterUrl() {
+        return this.clusterUrl;
+      }
+
+      setClusterUrl(clusterUrl) {
+        this.clusterUrl = clusterUrl;
+        return this.clusterUrl;
+      }
+    }
+
+    mockIPC = new MockIPC();
+    messageHandler = async function (event) {
+      if (event.origin !== window.location.origin) return;
+      const { method, payload } = event.data;
+      if (method) {
+        const response = await mockIPC.invoke(method, payload);
+        event.ports[0].postMessage(response);
+      }
+    };
+
+    window.addEventListener('message', messageHandler);
+    setDefaultClusterUrl(this);
+
+    const ipcService = this.owner.lookup('service:ipc');
+    stubs.ipcService = sinon.stub(ipcService, 'invoke');
+  });
+
+  hooks.afterEach(function () {
+    window.removeEventListener('message', messageHandler);
+  });
+
+  test('visiting index while unauthenticated redirects to global authenticate method', async function (assert) {
+    invalidateSession();
+    assert.expect(2);
+    await visit(urls.targets);
+    await a11yAudit();
+    assert.notOk(currentSession().isAuthenticated);
+    assert.strictEqual(currentURL(), urls.authenticate.methods.global);
+  });
+
+  test('visiting index', async function (assert) {
+    assert.expect(2);
+    const targetsCount = this.server.schema.targets.all().models.length;
+    await visit(urls.targets);
+    await a11yAudit();
+    assert.strictEqual(currentURL(), urls.targets);
+    assert.strictEqual(findAll('tbody tr').length, targetsCount);
+  });
+
+  test('visiting a target', async function (assert) {
+    assert.expect(1);
+    await visit(urls.targets);
+    await click('tbody tr td span a');
+    assert.strictEqual(currentURL(), urls.target);
+  });
+
+  test('visiting empty targets', async function (assert) {
+    assert.expect(1);
+    this.server.get('/targets', () => new Response(200));
+    await visit(urls.targets);
+    assert.strictEqual(
+      find('.rose-message-title').textContent.trim(),
+      'No Targets Available'
+    );
+  });
+
+  test('cannot navigate to a target without proper authorization', async function (assert) {
+    assert.expect(1);
+    instances.target.authorized_actions =
+      instances.target.authorized_actions.filter((item) => item !== 'read');
+    await visit(urls.targets);
+    assert.notOk(find('main tbody .rose-table-header-cell:nth-child(1) a'));
+  });
+});

ui/desktop/tests/acceptance/projects/targets/target-test.js

@@ -4,20 +4,14 @@
  */
 
 import { module, test } from 'qunit';
-import { visit, currentURL, click, find, findAll } from '@ember/test-helpers';
+import { visit, click, find, findAll } from '@ember/test-helpers';
 import { setupApplicationTest } from 'ember-qunit';
 import setupMirage from 'ember-cli-mirage/test-support/setup-mirage';
-import { Response } from 'miragejs';
-import a11yAudit from 'ember-a11y-testing/test-support/audit';
 import sinon from 'sinon';
-import {
-  currentSession,
-  authenticateSession,
-  invalidateSession,
-} from 'ember-simple-auth/test-support';
+import { authenticateSession } from 'ember-simple-auth/test-support';
 import { TYPE_TARGET_SSH } from 'api/models/target';
 
-module('Acceptance | projects | targets', function (hooks) {
+module('Acceptance | projects | targets | target', function (hooks) {
   setupApplicationTest(hooks);
   setupMirage(hooks);
 
@@ -97,6 +91,7 @@ module('Acceptance | projects | targets', function (hooks) {
       'target',
       {
         scope: instances.scopes.project,
+        address: 'localhost',
       },
       'withAssociations'
     );
@@ -156,49 +151,6 @@ module('Acceptance | projects | targets', function (hooks) {
     window.removeEventListener('message', messageHandler);
   });
 
-  test('visiting index while unauthenticated redirects to global authenticate method', async function (assert) {
-    invalidateSession();
-    assert.expect(2);
-    await visit(urls.targets);
-    await a11yAudit();
-    assert.notOk(currentSession().isAuthenticated);
-    assert.strictEqual(currentURL(), urls.authenticate.methods.global);
-  });
-
-  test('visiting index', async function (assert) {
-    assert.expect(2);
-    const targetsCount = this.server.schema.targets.all().models.length;
-    await visit(urls.targets);
-    await a11yAudit();
-    assert.strictEqual(currentURL(), urls.targets);
-    assert.strictEqual(findAll('tbody tr').length, targetsCount);
-  });
-
-  test('visiting a target', async function (assert) {
-    assert.expect(1);
-    await visit(urls.targets);
-    await click('tbody tr td span a');
-    assert.strictEqual(currentURL(), urls.target);
-  });
-
-  test('visiting empty targets', async function (assert) {
-    assert.expect(1);
-    this.server.get('/targets', () => new Response(200));
-    await visit(urls.targets);
-    assert.strictEqual(
-      find('.rose-message-title').textContent.trim(),
-      'No Targets Available'
-    );
-  });
-
-  test('cannot navigate to a target without proper authorization', async function (assert) {
-    assert.expect(1);
-    instances.target.authorized_actions =
-      instances.target.authorized_actions.filter((item) => item !== 'read');
-    await visit(urls.targets);
-    assert.notOk(find('main tbody .rose-table-header-cell:nth-child(1) a'));
-  });
-
   test.skip('connecting to a target', async function (assert) {
     assert.expect(4);
     stubs.ipcService.withArgs('cliExists').returns(true);
@@ -271,35 +223,15 @@ module('Acceptance | projects | targets', function (hooks) {
     assert.ok(find('.rose-dialog-body h3').textContent.trim().includes('SSH'));
   });
 
-  // Skipping because this test doesn't make sense.  Users will never even see
-  // targets for which they do not have authorize-session.  The only reason this
-  // test ever passed was because filtering in target mocks wasn't previously
-  // implemented.
-  test.skip('cannot connect to a target without proper authorization', async function (assert) {
-    assert.expect(3);
-    instances.target.update({
-      authorized_actions: instances.target.authorized_actions.filter(
-        (action) => action != 'authorize-session'
-      ),
-    });
-    assert.notOk(
-      instances.target.authorized_actions.includes('authorize-session')
-    );
-    await visit(urls.targets);
-    assert.ok(find('tbody tr:first-child'));
-    assert.notOk(find('tbody tr:first-child td:last-child button'));
-  });
-
   test('handles cli error on connect', async function (assert) {
     assert.expect(4);
     stubs.ipcService.withArgs('cliExists').returns(true);
     const confirmService = this.owner.lookup('service:confirm');
     confirmService.enabled = true;
-    await visit(urls.targets);
-    await click(
-      'tbody tr:first-child td:last-child button',
-      'Activate connect mode'
-    );
+
+    await visit(urls.target);
+    await click('[data-test-target-detail-connect-button]');
+
     assert.ok(find('.rose-dialog-error'), 'Error dialog');
     const dialogButtons = findAll('.rose-dialog-footer button');
     assert.strictEqual(dialogButtons.length, 2);
@@ -321,11 +253,10 @@ module('Acceptance | projects | targets', function (hooks) {
     stubs.ipcService.withArgs('connect').rejects();
     const confirmService = this.owner.lookup('service:confirm');
     confirmService.enabled = true;
-    await visit(urls.targets);
-    await click(
-      'tbody tr:first-child td:last-child button',
-      'Activate connect mode'
-    );
+
+    await visit(urls.target);
+    await click('[data-test-target-detail-connect-button]');
+
     assert.ok(find('.rose-dialog-error'), 'Error dialog');
     const dialogButtons = findAll('.rose-dialog-footer button');
     assert.strictEqual(dialogButtons.length, 2);
@@ -346,14 +277,13 @@ module('Acceptance | projects | targets', function (hooks) {
     stubs.ipcService.withArgs('cliExists').rejects();
     const confirmService = this.owner.lookup('service:confirm');
     confirmService.enabled = true;
-    await visit(urls.targets);
-    await click(
-      'tbody tr:first-child td:last-child button',
-      'Activate connect mode'
-    );
+
+    await visit(urls.target);
+    await click('[data-test-target-detail-connect-button]');
     const firstErrorDialog = find('.rose-dialog');
     await click('.rose-dialog footer .rose-button-primary', 'Retry');
     const secondErrorDialog = find('.rose-dialog');
+
     assert.notEqual(secondErrorDialog.id, firstErrorDialog.id);
   });
 });

ui/desktop/tests/acceptance/scopes-test.js

@@ -103,7 +103,7 @@ module('Acceptance | scopes', function (hooks) {
     );
     instances.target = this.server.create(
       'target',
-      { scope: instances.scopes.project },
+      { scope: instances.scopes.project, address: 'localhost' },
       'withAssociations'
     );
 
@@ -172,14 +172,17 @@ module('Acceptance | scopes', function (hooks) {
     const targetsCount = this.server.schema.targets.all().models.length;
 
     await visit(urls.targets);
+
     assert.strictEqual(currentURL(), urls.targets);
     assert.strictEqual(findAll('tbody tr').length, targetsCount);
   });
 
   test('visiting global scope', async function (assert) {
     assert.expect(1);
+
     await visit(urls.scopes.global);
     await a11yAudit();
+
     assert.strictEqual(currentURL(), urls.globalTargets);
   });
 
@@ -194,15 +197,19 @@ module('Acceptance | scopes', function (hooks) {
       const response = id === 'global' ? new Response(404) : scope;
       return response;
     });
+
     await visit(urls.scopes.global);
     await a11yAudit();
+
     assert.strictEqual(currentURL(), urls.globalTargets);
   });
 
   test('visiting org scope', async function (assert) {
     assert.expect(1);
+
     await visit(urls.scopes.org);
     await a11yAudit();
+
     assert.strictEqual(currentURL(), urls.targets);
   });
 
@@ -212,8 +219,10 @@ module('Acceptance | scopes', function (hooks) {
 
     await click('.rose-header-nav .rose-dropdown a:nth-of-type(2)');
     assert.strictEqual(currentURL(), urls.targets);
+
     await click('.rose-header-nav .rose-dropdown a:nth-of-type(3)');
     assert.strictEqual(currentURL(), urls.org2Targets);
+
     await click('.rose-header-nav .rose-dropdown a:nth-of-type(1)');
     assert.strictEqual(currentURL(), urls.globalTargets);
   });
@@ -221,23 +230,29 @@ module('Acceptance | scopes', function (hooks) {
   test('visiting index while unauthenticated redirects to global authenticate method', async function (assert) {
     invalidateSession();
     assert.expect(2);
+
     await visit(urls.targets);
     await a11yAudit();
+
     assert.notOk(currentSession().isAuthenticated);
     assert.strictEqual(currentURL(), urls.authenticate.methods.global);
   });
 
   test('visiting a target', async function (assert) {
     assert.expect(1);
+
     await visit(urls.targets);
-    await click('tbody tr td span a');
+    await click('[data-test-visit-target]');
+
     assert.strictEqual(currentURL(), urls.target);
   });
 
   test('visiting empty targets', async function (assert) {
     assert.expect(1);
     this.server.get('/targets', () => new Response(200));
+
     await visit(urls.targets);
+
     assert.ok(
       find('.rose-message-title').textContent.trim(),
       'No Targets Available'
@@ -257,10 +272,8 @@ module('Acceptance | scopes', function (hooks) {
     confirmService.enabled = true;
 
     await visit(urls.targets);
-    await click(
-      'tbody tr:first-child td:last-child button',
-      'Activate connect mode'
-    );
+    await click('[data-test-targets-connect-button]');
+
     assert.ok(find('.dialog-detail'), 'Success dialog');
     assert.strictEqual(findAll('.rose-dialog-footer button').length, 1);
     assert.strictEqual(
@@ -277,10 +290,8 @@ module('Acceptance | scopes', function (hooks) {
     confirmService.enabled = true;
 
     await visit(urls.scopes.global);
-    await click(
-      'tbody tr:first-child td:last-child button',
-      'Activate connect mode'
-    );
+    await click('[data-test-targets-connect-button]');
+
     assert.ok(find('.rose-dialog-error'), 'Error dialog');
     const dialogButtons = findAll('.rose-dialog-footer button');
     assert.strictEqual(dialogButtons.length, 2);
@@ -304,10 +315,8 @@ module('Acceptance | scopes', function (hooks) {
     confirmService.enabled = true;
 
     await visit(urls.targets);
-    await click(
-      'tbody tr:first-child td:last-child button',
-      'Activate connect mode'
-    );
+    await click('[data-test-targets-connect-button]');
+
     assert.ok(find('.rose-dialog-error'), 'Error dialog');
     const dialogButtons = findAll('.rose-dialog-footer button');
     assert.strictEqual(dialogButtons.length, 2);

ui/desktop/tests/unit/controllers/scopes/scope/projects/targets/target-test.js

@@ -0,0 +1,21 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: MPL-2.0
+ */
+
+import { module, test } from 'qunit';
+import { setupTest } from 'desktop/tests/helpers';
+
+module(
+  'Unit | Controller | scopes/scope/projects/targets/target',
+  function (hooks) {
+    setupTest(hooks);
+
+    test('it exists', function (assert) {
+      let controller = this.owner.lookup(
+        'controller:scopes/scope/projects/targets/target'
+      );
+      assert.ok(controller);
+    });
+  }
+);