From 53b4920bbf96bf12644183eb22e107b4cb046bfe Mon Sep 17 00:00:00 2001
From: Noelle Daley <adriannenoelle@gmail.com>
Date: Tue, 29 Oct 2019 15:32:48 -0700
Subject: [PATCH] move capabilities check out of helper and into permissions
 service

---
 ui/app/helpers/has-permission.js           |  4 ++--
 ui/app/services/permissions.js             |  9 ++++++---
 ui/app/templates/vault/cluster/access.hbs  |  4 ++--
 ui/tests/unit/services/permissions-test.js | 23 +++++-----------------
 4 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/ui/app/helpers/has-permission.js b/ui/app/helpers/has-permission.js
index 55731c134a2d..0e5ae43c654c 100644
--- a/ui/app/helpers/has-permission.js
+++ b/ui/app/helpers/has-permission.js
@@ -15,9 +15,9 @@ export default Helper.extend({
   ),
 
   compute([route], params) {
-    let { routeParams, capabilities } = params;
+    let { routeParams } = params;
     let permissions = this.permissions;
 
-    return permissions.hasNavPermission(route, routeParams, capabilities);
+    return permissions.hasNavPermission(route, routeParams);
   },
 });
diff --git a/ui/app/services/permissions.js b/ui/app/services/permissions.js
index e3f8642c92a3..0590025b5b39 100644
--- a/ui/app/services/permissions.js
+++ b/ui/app/services/permissions.js
@@ -87,11 +87,14 @@ export default Service.extend({
     this.set('canViewAll', null);
   },
 
-  hasNavPermission(navItem, routeParams, capabilities = [null]) {
+  hasNavPermission(navItem, routeParams) {
     if (routeParams) {
-      return this.hasPermission(API_PATHS[navItem][routeParams], capabilities);
+      // viewing the entity and groups pages require the list capability, while the others require the default, which is anything other than deny
+      let capability = routeParams === 'entities' || routeParams === 'groups' ? ['list'] : [null];
+
+      return this.hasPermission(API_PATHS[navItem][routeParams], capability);
     }
-    return Object.values(API_PATHS[navItem]).some(path => this.hasPermission(path, capabilities));
+    return Object.values(API_PATHS[navItem]).some(path => this.hasPermission(path));
   },
 
   navPathParams(navItem) {
diff --git a/ui/app/templates/vault/cluster/access.hbs b/ui/app/templates/vault/cluster/access.hbs
index ecb4ee31b312..0aed44a583dc 100644
--- a/ui/app/templates/vault/cluster/access.hbs
+++ b/ui/app/templates/vault/cluster/access.hbs
@@ -7,14 +7,14 @@
         {{/link-to}}
       </li>
     {{/if}}
-    {{#if (has-permission "access" routeParams="entities" capabilities=(array "list"))}}
+    {{#if (has-permission "access" routeParams="entities")}}
       <li>
         {{#link-to "vault.cluster.access.identity" "entities" data-test-link=true }}
           Entities
         {{/link-to}}
       </li>
     {{/if}}
-    {{#if (has-permission "access" routeParams="groups" capabilities=(array "list"))}}
+    {{#if (has-permission "access" routeParams="groups")}}
       <li>
         {{#link-to "vault.cluster.access.identity" "groups" data-test-link=true }}
           Groups
diff --git a/ui/tests/unit/services/permissions-test.js b/ui/tests/unit/services/permissions-test.js
index 466e5fa0ea58..aef4a1533e8b 100644
--- a/ui/tests/unit/services/permissions-test.js
+++ b/ui/tests/unit/services/permissions-test.js
@@ -168,23 +168,10 @@ module('Unit | Service | permissions', function(hooks) {
       },
     };
     service.set('exactPaths', accessPaths);
-    assert.equal(
-      service.hasNavPermission(
-        'access',
-        'groups',
-        ['list', 'read'],
-        'checks permission when multiple capabilities are specified'
-      ),
-      true
-    );
-    assert.equal(
-      service.hasNavPermission('access', 'groups'),
-      true,
-      'checks permission when capabilities are not specified'
-    );
-  });
-
-  test('hasNavPermission returns false if a policy does not include access to any paths', function(assert) {
+    assert.equal(service.hasNavPermission('access', 'groups'), true);
+  });
+
+  test('hasNavPermission returns false if a policy does not include the required capabilities for at least one path', function(assert) {
     let service = this.owner.lookup('service:permissions');
     const accessPaths = {
       'sys/auth': {
@@ -195,7 +182,7 @@ module('Unit | Service | permissions', function(hooks) {
       },
     };
     service.set('exactPaths', accessPaths);
-    assert.equal(service.hasNavPermission('access', 'groups', ['list', 'read']), false);
+    assert.equal(service.hasNavPermission('access', 'groups'), false);
   });
 
   test('appends the namespace to the path if there is one', function(assert) {