Refactor account and user abilities (#1694)

* refactor: 💡 add abilities to user and remove from account

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;
+  }
 }

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) {
+  let canService;
-    assert.expect(2);
+  let store;
-    const service = this.owner.lookup('service:can');
+
-    const model = {
+  const instances = {
-      authorized_actions: ['add-accounts'],
+    account: null,
-    };
+    user: null,
-    assert.ok(service.can('addAccounts user', model));
+  };
-    model.authorized_actions = [];
+
-    assert.notOk(service.can('addAccounts user', model));
+  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) {
+  test('can add accounts to user when authorized and given a known account type', function (assert) {
-    assert.expect(2);
+    assert.expect(5);
-    const service = this.owner.lookup('service:can');
+    instances.user.authorized_actions = ['add-accounts'];
-    const model = {
+    assert.true(canService.can('addAccounts user', instances.user));
-      authorized_actions: ['remove-accounts'],
+    instances.account.type = TYPE_AUTH_METHOD_OIDC;
-    };
+    assert.true(
-    assert.ok(service.can('removeAccounts user', model));
+      canService.can('addAccount user', instances.user, {
-    model.authorized_actions = [];
+        account: instances.account,
-    assert.notOk(service.can('removeAccounts user', model));
+      })
+    );
+    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,
+      })
+    );
   });
 });

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;
   }
 }

ui/admin/tests/unit/abilities/account-test.js

@@ -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));
-  });
-});

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,
+      })
+    );
+  });
+});