Session recording and storage bucket abilities cleanup (#256)

* refactor: 💡 cleanup session recording abilities

* refactor: 💡 use super call instead of duplicating check

* refactor: 💡 cleanup storage bucket abilities

addons/api/addon/abilities/session-recording.js

@@ -18,14 +18,7 @@ export default class SessionRecordingAbility extends ModelAbility {
    * @type {boolean}
    */
   get canRead() {
-    return !this.model.isUnknown && this.hasAuthorizedAction('read');
+    return !this.model.isUnknown && super.canRead;
-  }
-
-  get canList() {
-    return (
-      this.hasAuthorizedCollectionAction('list') &&
-      (this.model.isGlobal || this.model.isOrg)
-    );
   }
 
   get canDownload() {

addons/api/addon/abilities/storage-bucket.js

@@ -18,24 +18,22 @@ export default class StorageBucketAbility extends ModelAbility {
    * @type {boolean}
    */
   get canRead() {
-    return !this.model.isUnknown && this.hasAuthorizedAction('read');
+    return !this.model.isUnknown && super.canRead;
   }
 
   /**
-   * Storage buckets can only exist within global and org scopes.
+   * Only "known" storage bucket types may be updated.
    * @type {boolean}
    */
-  get canCreate() {
+  get canUpdate() {
-    return (
+    return !this.model.isUnknown && super.canUpdate;
-      this.hasAuthorizedCollectionAction('create') &&
-      (this.model.isGlobal || this.model.isOrg)
-    );
   }
 
-  get canList() {
+  /**
-    return (
+   * Only "known" storage bucket types may be deleted.
-      this.hasAuthorizedCollectionAction('list') &&
+   * @type {boolean}
-      (this.model.isGlobal || this.model.isOrg)
+   */
-    );
+  get canDelete() {
+    return !this.model.isUnknown && super.canDelete;
   }
 }

ui/admin/app/abilities/scope.js

@@ -0,0 +1,44 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: MPL-2.0
+ */
+
+import OverrideModelAbility from './model';
+import { inject as service } from '@ember/service';
+
+export default class OverrideScopeAbility extends OverrideModelAbility {
+  // =services
+
+  @service features;
+
+  // =attributes
+
+  /**
+   * Creating a resource is allowed only if a create grant is present
+   * and collection-specific criteria are met.
+   * @type {boolean}
+   */
+  get canCreate() {
+    switch (this.collection) {
+      case 'storage-buckets':
+        return this.features.isEnabled('session-recording') && super.canCreate;
+      default:
+        return super.canCreate;
+    }
+  }
+
+  /**
+   * Listing a resource is allowed only if a list grant is present
+   * and collection-specific criteria are met.
+   * @type {boolean}
+   */
+  get canList() {
+    switch (this.collection) {
+      case 'session-recordings':
+      case 'storage-buckets':
+        return this.features.isEnabled('session-recording') && super.canList;
+      default:
+        return super.canList;
+    }
+  }
+}

ui/admin/app/abilities/session-recording.js

@@ -17,18 +17,4 @@ export default class OverrideSessionRecordingAbility extends SessionRecordingAbi
   get canRead() {
     return this.features.isEnabled('session-recording') ? super.canRead : false;
   }
-
-  /**
-   * This override ensures that session recordings can only be presented in the global scope
-   * and if the session-recording feature flag is enabled.
-   */
-  get canList() {
-    return this.features.isEnabled('session-recording')
-      ? this.hasAuthorizedCollectionAction('list') && this.model.isGlobal
-      : false;
-  }
-
-  get canNavigate() {
-    return this.canList;
-  }
 }

ui/admin/app/abilities/storage-bucket.js

@@ -19,16 +19,22 @@ export default class OverrideStorageBucketAbility extends StorageBucketAbility {
   }
 
   /**
-   * This override ensures that storage buckets can only be presented in the global scope
+   * This override ensures that storage buckets may be updated only if the
-   * and if the session-recording feature flag is enabled.
+   * session-recording feature flag is enabled.
    */
-  get canList() {
+  get canUpdate() {
     return this.features.isEnabled('session-recording')
-      ? this.hasAuthorizedCollectionAction('list') && this.model.isGlobal
+      ? super.canUpdate
       : false;
   }
 
-  get canNavigate() {
+  /**
-    return this.canCreate || this.canList;
+   * This override ensures that storage buckets may be deleted only if the
+   * session-recording feature flag is enabled.
+   */
+  get canDelete() {
+    return this.features.isEnabled('session-recording')
+      ? super.canDelete
+      : false;
   }
 }

ui/admin/app/routes/scopes/scope/session-recordings.js

@@ -29,7 +29,7 @@ export default class ScopesScopeSessionRecordingsRoute extends Route {
     const { id: scope_id } = scope;
 
     if (
-      this.can.can('list session-recording', scope, {
+      this.can.can('list scope', scope, {
         collection: 'session-recordings',
       })
     ) {

ui/admin/app/routes/scopes/scope/storage-buckets.js

@@ -35,7 +35,7 @@ export default class ScopesScopeStorageBucketsRoute extends Route {
     const scope = this.modelFor('scopes.scope');
     const { id: scope_id } = scope;
     if (
-      this.can.can('list storage-bucket', scope, {
+      this.can.can('list scope', scope, {
         collection: 'storage-buckets',
       })
     ) {

ui/admin/app/templates/scopes/scope.hbs

@@ -28,22 +28,14 @@
           </nav.link>
           {{#if (feature-flag 'session-recording')}}
             {{#if
-              (can
+              (can 'navigate scope' @model collection='session-recordings')
-                'navigate session-recording'
-                @model
-                collection='session-recordings'
-              )
             }}
               <nav.link @route='scopes.scope.session-recordings'>
                 <Rose::Icon @name='flight-icons/svg/film-16' @size='medium' />
                 {{t 'resources.session-recording.title_plural'}}
               </nav.link>
             {{/if}}
-            {{#if
+            {{#if (can 'navigate scope' @model collection='storage-buckets')}}
-              (can
-                'navigate storage-bucket' @model collection='storage-buckets'
-              )
-            }}
               <nav.link @route='scopes.scope.storage-buckets'>
                 <Rose::Icon
                   @name='flight-icons/svg/cloud-upload-16'

ui/admin/app/templates/scopes/scope/storage-buckets/index.hbs

@@ -17,9 +17,7 @@
   </page.header>
 
   <page.actions>
-    {{#if
+    {{#if (can 'create scope' this.scope collection='storage-buckets')}}
-      (can 'create storage-bucket' this.scope collection='storage-buckets')
-    }}
       {{#if @model}}
         <Hds::Button
           @route='scopes.scope.storage-buckets.new'
@@ -89,28 +87,41 @@
               {{/if}}
             </B.Td>
             <B.Td>
-              {{!TODO: add dropdown for delete actions}}
+              {{#let
-              <Hds::Dropdown as |dd|>
+                (hash
-                <dd.ToggleIcon
+                  canRead=(can 'read storage-bucket' B.data)
-                  @icon='more-horizontal'
+                  canDelete=(can 'delete storage-bucket' B.data)
-                  @text='Overflow Options'
+                )
-                  @hasChevron={{false}}
+                as |perms|
-                  @size='small'
+              }}
-                />
+                {{#if (or perms.canRead perms.canDelete)}}
-                <dd.Interactive
+                  <Hds::Dropdown as |dd|>
-                  @route='scopes.scope.storage-buckets.storage-bucket'
+                    <dd.ToggleIcon
-                  @model={{B.data.id}}
+                      @icon='more-horizontal'
-                  @text={{t 'actions.edit'}}
+                      @text='Overflow Options'
-                />
+                      @hasChevron={{false}}
-                {{#if (can 'delete model' B.data)}}
+                      @size='small'
-                  <dd.Separator />
+                    />
-                  <dd.Interactive
+                    {{#if perms.canRead}}
-                    @text={{t 'resources.storage-bucket.actions.delete'}}
+                      <dd.Interactive
-                    @color='critical'
+                        @route='scopes.scope.storage-buckets.storage-bucket'
-                    {{on 'click' (route-action 'delete' B.data)}}
+                        @model={{B.data.id}}
-                  />
+                        @text={{t 'actions.edit'}}
+                      />
+                    {{/if}}
+                    {{#if (and perms.canRead perms.canDelete)}}
+                      <dd.Separator />
+                    {{/if}}
+                    {{#if perms.canDelete}}
+                      <dd.Interactive
+                        @text={{t 'resources.storage-bucket.actions.delete'}}
+                        @color='critical'
+                        {{on 'click' (route-action 'delete' B.data)}}
+                      />
+                    {{/if}}
+                  </Hds::Dropdown>
                 {{/if}}
-              </Hds::Dropdown>
+              {{/let}}
             </B.Td>
           </B.Tr>
         </:body>
@@ -122,14 +133,10 @@
           as |message|
         >
           <message.description>
-            {{#if
+            {{#if (can 'list scope' this.scope collection='storage-buckets')}}
-              (can
-                'list storage-bucket' this.scope collection='storage-buckets'
-              )
-            }}
               {{t 'resources.storage-bucket.messages.none.description'}}
             {{else if (can
-              'create storage-bucket' this.scope collection='storage-buckets'
+              'create scope' this.scope collection='storage-buckets'
             )}}
               {{t
                 'descriptions.create-but-not-list'
@@ -142,11 +149,7 @@
               }}
             {{/if}}
           </message.description>
-          {{#if
+          {{#if (can 'create scope' this.scope collection='storage-buckets')}}
-            (can
-              'create storage-bucket' this.scope collection='storage-buckets'
-            )
-          }}
             <Hds::Link::Standalone
               @icon='plus-circle'
               @iconPosition='leading'

ui/admin/tests/e2e/helpers/boundary-ui.js

@@ -256,7 +256,11 @@ exports.waitForSessionToBeVisible = async (page, targetName) => {
  * @param {string} targetName Name of the target associated with the session
  * @param {string} credentialName Name of the credentials to be added to the target
  */
-exports.addBrokeredCredentialsToTarget = async (page, targetName, credentialName) => {
+exports.addBrokeredCredentialsToTarget = async (
+  page,
+  targetName,
+  credentialName
+) => {
   await page
     .getByRole('navigation', { name: 'Resources' })
     .getByRole('link', { name: 'Targets' })
@@ -282,4 +286,4 @@ exports.addBrokeredCredentialsToTarget = async (page, targetName, credentialName
   ).toBeVisible();
   await page.getByRole('button', { name: 'Dismiss' }).click();
   await expect(page.getByRole('link', { name: credentialName })).toBeVisible();
-}
+};

ui/admin/tests/unit/abilities/session-recording-test.js

@@ -89,22 +89,18 @@ module('Unit | Abilities | session-recording', function (hooks) {
     });
 
     assert.true(
-      canService.can('list session-recording', scopeModelWithAuthorizedAction, {
+      canService.can('list scope', scopeModelWithAuthorizedAction, {
         collection: 'session-recordings',
       })
     );
     assert.false(
-      canService.can(
+      canService.can('list scope', scopeModelWithoutAuthorizedAction, {
-        'list session-recording',
+        collection: 'session-recordings',
-        scopeModelWithoutAuthorizedAction,
+      })
-        {
-          collection: 'session-recordings',
-        }
-      )
     );
   });
 
-  test('cannot list session recording when in org scope', function (assert) {
+  test('can list session recording when in org scope when authorized', function (assert) {
     assert.expect(2);
 
     const scopeModelWithAuthorizedAction = store.createRecord('scope', {
@@ -118,19 +114,15 @@ module('Unit | Abilities | session-recording', function (hooks) {
       id: 'second',
     });
 
-    assert.false(
+    assert.true(
-      canService.can('list session-recording', scopeModelWithAuthorizedAction, {
+      canService.can('list scope', scopeModelWithAuthorizedAction, {
         collection: 'session-recordings',
       })
     );
     assert.false(
-      canService.can(
+      canService.can('list scope', scopeModelWithoutAuthorizedAction, {
-        'list session-recording',
+        collection: 'session-recordings',
-        scopeModelWithoutAuthorizedAction,
+      })
-        {
-          collection: 'session-recordings',
-        }
-      )
     );
   });
 
@@ -149,22 +141,14 @@ module('Unit | Abilities | session-recording', function (hooks) {
     });
 
     assert.true(
-      canService.can(
+      canService.can('navigate scope', scopeModelWithAuthorizedAction, {
-        'navigate session-recording',
+        collection: 'session-recordings',
-        scopeModelWithAuthorizedAction,
+      })
-        {
-          collection: 'session-recordings',
-        }
-      )
     );
     assert.false(
-      canService.can(
+      canService.can('navigate scope', scopeModelWithoutAuthorizedAction, {
-        'navigate session-recording',
+        collection: 'session-recordings',
-        scopeModelWithoutAuthorizedAction,
+      })
-        {
-          collection: 'session-recordings',
-        }
-      )
     );
   });
 
@@ -184,22 +168,14 @@ module('Unit | Abilities | session-recording', function (hooks) {
     });
 
     assert.false(
-      canService.can(
+      canService.can('navigate scope', scopeModelWithAuthorizedAction, {
-        'navigate session-recording',
+        collection: 'session-recordings',
-        scopeModelWithAuthorizedAction,
+      })
-        {
-          collection: 'session-recordings',
-        }
-      )
     );
     assert.false(
-      canService.can(
+      canService.can('navigate scope', scopeModelWithoutAuthorizedAction, {
-        'navigate session-recording',
+        collection: 'session-recordings',
-        scopeModelWithoutAuthorizedAction,
+      })
-        {
-          collection: 'session-recordings',
-        }
-      )
     );
   });
 });

ui/admin/tests/unit/abilities/storage-bucket-test.js

@@ -18,10 +18,10 @@ module('Unit | Abilities | storage-bucket', function (hooks) {
   let store;
 
   hooks.beforeEach(function () {
+    store = this.owner.lookup('service:store');
+    canService = this.owner.lookup('service:can');
     features = this.owner.lookup('service:features');
     features.enable('session-recording');
-    canService = this.owner.lookup('service:can');
-    store = this.owner.lookup('service:store');
   });
 
   test('can read storage bucket when authorized and feature is enabled', function (assert) {
@@ -84,7 +84,7 @@ module('Unit | Abilities | storage-bucket', function (hooks) {
     });
 
     assert.true(
-      canService.can('list storage-bucket', scopeModel, {
+      canService.can('list scope', scopeModel, {
         collection: 'storage-buckets',
       })
     );
@@ -100,13 +100,13 @@ module('Unit | Abilities | storage-bucket', function (hooks) {
     });
 
     assert.false(
-      canService.can('list storage-bucket', scopeModel, {
+      canService.can('list scope', scopeModel, {
         collection: 'storage-buckets',
       })
     );
   });
 
-  test('cannot list storage bucket when authorized and in org scope', function (assert) {
+  test('can list storage bucket when authorized and in org scope', function (assert) {
     assert.expect(1);
 
     const scopeModel = store.createRecord('scope', {
@@ -115,8 +115,8 @@ module('Unit | Abilities | storage-bucket', function (hooks) {
       id: 'o_123',
     });
 
-    assert.false(
+    assert.true(
-      canService.can('list storage-bucket', scopeModel, {
+      canService.can('list scope', scopeModel, {
         collection: 'storage-buckets',
       })
     );
@@ -132,7 +132,7 @@ module('Unit | Abilities | storage-bucket', function (hooks) {
     });
 
     assert.true(
-      canService.can('create storage-bucket', scopeModel, {
+      canService.can('create scope', scopeModel, {
         collection: 'storage-buckets',
       })
     );
@@ -148,13 +148,13 @@ module('Unit | Abilities | storage-bucket', function (hooks) {
     });
 
     assert.true(
-      canService.can('create storage-bucket', scopeModel, {
+      canService.can('create scope', scopeModel, {
         collection: 'storage-buckets',
       })
     );
   });
 
-  test('cannot create storage bucket when authorized and in project scope', function (assert) {
+  test('can create storage bucket when authorized and in project scope', function (assert) {
     assert.expect(1);
 
     const scopeModel = store.createRecord('scope', {
@@ -163,8 +163,8 @@ module('Unit | Abilities | storage-bucket', function (hooks) {
       id: 'p_123',
     });
 
-    assert.false(
+    assert.true(
-      canService.can('create storage-bucket', scopeModel, {
+      canService.can('create scope', scopeModel, {
         collection: 'storage-buckets',
       })
     );
@@ -180,7 +180,7 @@ module('Unit | Abilities | storage-bucket', function (hooks) {
     });
 
     assert.false(
-      canService.can('create storage-bucket', scopeModel, {
+      canService.can('create scope', scopeModel, {
         collection: 'storage-buckets',
       })
     );
@@ -196,7 +196,7 @@ module('Unit | Abilities | storage-bucket', function (hooks) {
     });
 
     assert.true(
-      canService.can('navigate storage-bucket', scopeModel, {
+      canService.can('navigate scope', scopeModel, {
         collection: 'storage-buckets',
       })
     );
@@ -212,8 +212,8 @@ module('Unit | Abilities | storage-bucket', function (hooks) {
       id: 'global',
     });
 
-    assert.true(
+    assert.false(
-      canService.can('navigate storage-bucket', scopeModel, {
+      canService.can('navigate scope', scopeModel, {
         collection: 'storage-buckets',
       })
     );