From 379f317d8b03e573cc2e35ccde06b040d8dc1862 Mon Sep 17 00:00:00 2001
From: lisbet-alvarez <107949262+lisbet-alvarez@users.noreply.github.com>
Date: Mon, 22 May 2023 11:16:48 -0700
Subject: [PATCH] Refactor account and user abilities (#1694)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* refactor: 💡 add abilities to user and remove from account
---
 addons/api/addon/abilities/user.js            |  16 ++
 addons/api/tests/unit/abilities/user-test.js  | 154 ++++++++++++++++--
 .../app/abilities/{account.js => user.js}     |  12 +-
 ui/admin/tests/unit/abilities/account-test.js |  80 ---------
 ui/admin/tests/unit/abilities/user-test.js    | 109 +++++++++++++
 5 files changed, 269 insertions(+), 102 deletions(-)
 rename ui/admin/app/abilities/{account.js => user.js} (67%)
 delete mode 100644 ui/admin/tests/unit/abilities/account-test.js
 create mode 100644 ui/admin/tests/unit/abilities/user-test.js

diff --git a/addons/api/addon/abilities/user.js b/addons/api/addon/abilities/user.js
index bda0003cf..2e5d7a740 100644
--- a/addons/api/addon/abilities/user.js
+++ b/addons/api/addon/abilities/user.js
@@ -24,4 +24,20 @@ export default class UserAbility extends ModelAbility {
   get canRemoveAccounts() {
     return this.hasAuthorizedAction('remove-accounts');
   }
+
+  /**
+   * @type {boolean}
+   */
+  get canAddAccount() {
+    const { account, canAddAccounts } = this;
+    return canAddAccounts && !account.isUnknown;
+  }
+
+  /**
+   * @type {boolean}
+   */
+  get canRemoveAccount() {
+    const { account, canRemoveAccounts } = this;
+    return canRemoveAccounts && !account.isUnknown;
+  }
 }
diff --git a/addons/api/tests/unit/abilities/user-test.js b/addons/api/tests/unit/abilities/user-test.js
index 06c551fdf..e15f94994 100644
--- a/addons/api/tests/unit/abilities/user-test.js
+++ b/addons/api/tests/unit/abilities/user-test.js
@@ -5,29 +5,147 @@
 
 import { module, test } from 'qunit';
 import { setupTest } from 'ember-qunit';
+import {
+  TYPE_AUTH_METHOD_LDAP,
+  TYPE_AUTH_METHOD_OIDC,
+  TYPE_AUTH_METHOD_PASSWORD,
+} from 'api/models/auth-method';
 
 module('Unit | Abilities | User', function (hooks) {
   setupTest(hooks);
 
-  test('it reflects when a given user resource may add accounts based on authorized_actions', function (assert) {
-    assert.expect(2);
-    const service = this.owner.lookup('service:can');
-    const model = {
-      authorized_actions: ['add-accounts'],
-    };
-    assert.ok(service.can('addAccounts user', model));
-    model.authorized_actions = [];
-    assert.notOk(service.can('addAccounts user', model));
+  let canService;
+  let store;
+
+  const instances = {
+    account: null,
+    user: null,
+  };
+
+  hooks.beforeEach(function () {
+    canService = this.owner.lookup('service:can');
+    store = this.owner.lookup('service:store');
+    instances.user = store.createRecord('user');
+    instances.account = store.createRecord('account');
   });
 
-  test('it reflects when a given user resource may remove accounts based on authorized_actions', function (assert) {
-    assert.expect(2);
-    const service = this.owner.lookup('service:can');
-    const model = {
-      authorized_actions: ['remove-accounts'],
-    };
-    assert.ok(service.can('removeAccounts user', model));
-    model.authorized_actions = [];
-    assert.notOk(service.can('removeAccounts user', model));
+  test('can add accounts to user when authorized and given a known account type', function (assert) {
+    assert.expect(5);
+    instances.user.authorized_actions = ['add-accounts'];
+    assert.true(canService.can('addAccounts user', instances.user));
+    instances.account.type = TYPE_AUTH_METHOD_OIDC;
+    assert.true(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = TYPE_AUTH_METHOD_PASSWORD;
+    assert.true(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = TYPE_AUTH_METHOD_LDAP;
+    assert.true(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = 'no-such-type';
+    assert.false(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+  });
+
+  test('cannot add accounts to user when unauthorized', function (assert) {
+    assert.expect(5);
+    instances.user.authorized_actions = [];
+    assert.false(canService.can('addAccounts user', instances.user));
+    instances.account.type = TYPE_AUTH_METHOD_OIDC;
+    assert.false(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = TYPE_AUTH_METHOD_PASSWORD;
+    assert.false(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = TYPE_AUTH_METHOD_LDAP;
+    assert.false(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = 'no-such-type';
+    assert.false(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+  });
+
+  test('can remove accounts from user when authorized and given a known account type', function (assert) {
+    assert.expect(5);
+    instances.user.authorized_actions = ['remove-accounts'];
+    assert.true(canService.can('removeAccounts user', instances.user));
+    instances.account.type = TYPE_AUTH_METHOD_OIDC;
+    assert.true(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = TYPE_AUTH_METHOD_PASSWORD;
+    assert.true(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = TYPE_AUTH_METHOD_LDAP;
+    assert.true(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = 'no-such-type';
+    assert.false(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+  });
+
+  test('cannot remove accounts from user when unauthorized', function (assert) {
+    assert.expect(5);
+    instances.user.authorized_actions = [];
+    assert.false(canService.can('removeAccounts user', instances.user));
+    instances.account.type = TYPE_AUTH_METHOD_OIDC;
+    assert.false(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = TYPE_AUTH_METHOD_PASSWORD;
+    assert.false(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = TYPE_AUTH_METHOD_LDAP;
+    assert.false(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = 'no-such-type';
+    assert.false(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
   });
 });
diff --git a/ui/admin/app/abilities/account.js b/ui/admin/app/abilities/user.js
similarity index 67%
rename from ui/admin/app/abilities/account.js
rename to ui/admin/app/abilities/user.js
index 5f6fcec87..80e46d66d 100644
--- a/ui/admin/app/abilities/account.js
+++ b/ui/admin/app/abilities/user.js
@@ -3,10 +3,10 @@
  * SPDX-License-Identifier: MPL-2.0
  */
 
-import AccountAbility from 'api/abilities/account';
+import UserAbility from 'api/abilities/user';
 import { inject as service } from '@ember/service';
 
-export default class OverrideAccountAbility extends AccountAbility {
+export default class OverrideUserAbility extends UserAbility {
   // =service
   @service features;
 
@@ -16,10 +16,14 @@ export default class OverrideAccountAbility extends AccountAbility {
    */
 
   get canAddAccount() {
-    return this.features.isEnabled('ldap-auth-methods') || !this.model.isLDAP;
+    return this.features.isEnabled('ldap-auth-methods') || !this.account.isLDAP
+      ? super.canAddAccount
+      : false;
   }
 
   get canRemoveAccount() {
-    return this.features.isEnabled('ldap-auth-methods') || !this.model.isLDAP;
+    return this.features.isEnabled('ldap-auth-methods') || !this.account.isLDAP
+      ? super.canRemoveAccount
+      : false;
   }
 }
diff --git a/ui/admin/tests/unit/abilities/account-test.js b/ui/admin/tests/unit/abilities/account-test.js
deleted file mode 100644
index 914bacaaa..000000000
--- a/ui/admin/tests/unit/abilities/account-test.js
+++ /dev/null
@@ -1,80 +0,0 @@
-/**
- * Copyright (c) HashiCorp, Inc.
- * SPDX-License-Identifier: MPL-2.0
- */
-
-import { module, test } from 'qunit';
-import { setupTest } from 'ember-qunit';
-import {
-  TYPE_AUTH_METHOD_LDAP,
-  TYPE_AUTH_METHOD_OIDC,
-  TYPE_AUTH_METHOD_PASSWORD,
-} from 'api/models/auth-method';
-
-module('Unit | Abilities | account', function (hooks) {
-  setupTest(hooks);
-
-  let canService;
-  let store;
-  let features;
-
-  hooks.beforeEach(function () {
-    canService = this.owner.lookup('service:can');
-    store = this.owner.lookup('service:store');
-    features = this.owner.lookup('service:features');
-  });
-
-  test('can add ldap account to user when feature flag enabled', function (assert) {
-    assert.expect(1);
-    features.enable('ldap-auth-methods');
-    const account = store.createRecord('account', {
-      type: TYPE_AUTH_METHOD_LDAP,
-    });
-    assert.true(canService.can('addAccount account', account));
-  });
-
-  test('can add non-ldap account to user', function (assert) {
-    assert.expect(2);
-    const account = store.createRecord('account', {
-      type: TYPE_AUTH_METHOD_OIDC,
-    });
-    assert.true(canService.can('addAccount account', account));
-    account.type = TYPE_AUTH_METHOD_PASSWORD;
-    assert.true(canService.can('addAccount account', account));
-  });
-
-  test('cannot add ldap account to user when feature flag disabled', function (assert) {
-    assert.expect(1);
-    const account = store.createRecord('account', {
-      type: TYPE_AUTH_METHOD_LDAP,
-    });
-    assert.false(canService.can('addAccount account', account));
-  });
-
-  test('can remove ldap account from user when feature flag enabled', function (assert) {
-    assert.expect(1);
-    features.enable('ldap-auth-methods');
-    const account = store.createRecord('account', {
-      type: TYPE_AUTH_METHOD_LDAP,
-    });
-    assert.true(canService.can('removeAccount account', account));
-  });
-
-  test('cannot remove ldap account from user when feature flag disabled', function (assert) {
-    assert.expect(1);
-    const account = store.createRecord('account', {
-      type: TYPE_AUTH_METHOD_LDAP,
-    });
-    assert.false(canService.can('removeAccount account', account));
-  });
-
-  test('can remove non-ldap account from user', function (assert) {
-    assert.expect(2);
-    const account = store.createRecord('account', {
-      type: TYPE_AUTH_METHOD_OIDC,
-    });
-    assert.true(canService.can('removeAccount account', account));
-    (account.type = TYPE_AUTH_METHOD_PASSWORD),
-      assert.true(canService.can('removeAccount account', account));
-  });
-});
diff --git a/ui/admin/tests/unit/abilities/user-test.js b/ui/admin/tests/unit/abilities/user-test.js
new file mode 100644
index 000000000..04c09244c
--- /dev/null
+++ b/ui/admin/tests/unit/abilities/user-test.js
@@ -0,0 +1,109 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: MPL-2.0
+ */
+
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+import {
+  TYPE_AUTH_METHOD_LDAP,
+  TYPE_AUTH_METHOD_OIDC,
+  TYPE_AUTH_METHOD_PASSWORD,
+} from 'api/models/auth-method';
+
+module('Unit | Abilities | user', function (hooks) {
+  setupTest(hooks);
+
+  let canService;
+  let store;
+  let features;
+
+  const instances = {
+    account: null,
+    user: null,
+  };
+
+  hooks.beforeEach(function () {
+    canService = this.owner.lookup('service:can');
+    store = this.owner.lookup('service:store');
+    features = this.owner.lookup('service:features');
+    instances.user = store.createRecord('user', {
+      authorized_actions: ['add-accounts', 'remove-accounts'],
+    });
+    instances.account = store.createRecord('account');
+  });
+
+  test('can add ldap account to user when feature flag enabled', function (assert) {
+    assert.expect(1);
+    features.enable('ldap-auth-methods');
+    instances.account.type = TYPE_AUTH_METHOD_LDAP;
+    assert.true(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+  });
+
+  test('can add non-ldap account to user', function (assert) {
+    assert.expect(2);
+    instances.account.type = TYPE_AUTH_METHOD_OIDC;
+    assert.true(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = TYPE_AUTH_METHOD_PASSWORD;
+    assert.true(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+  });
+
+  test('cannot add ldap account to user when feature flag disabled', function (assert) {
+    assert.expect(1);
+    instances.account.type = TYPE_AUTH_METHOD_LDAP;
+    assert.false(
+      canService.can('addAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+  });
+
+  test('can remove ldap account from user when feature flag enabled', function (assert) {
+    assert.expect(1);
+    features.enable('ldap-auth-methods');
+    instances.account.type = TYPE_AUTH_METHOD_LDAP;
+    assert.true(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+  });
+
+  test('cannot remove ldap account from user when feature flag disabled', function (assert) {
+    assert.expect(1);
+    instances.account.type = TYPE_AUTH_METHOD_LDAP;
+    assert.false(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+  });
+
+  test('can remove non-ldap account from user', function (assert) {
+    assert.expect(2);
+    instances.account.type = TYPE_AUTH_METHOD_OIDC;
+    assert.true(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+    instances.account.type = TYPE_AUTH_METHOD_PASSWORD;
+    assert.true(
+      canService.can('removeAccount user', instances.user, {
+        account: instances.account,
+      })
+    );
+  });
+});