From a095225ba7c90fecb6cc51c833c05cd7ec6d8de8 Mon Sep 17 00:00:00 2001
From: Jai <41024828+ChaiWithJai@users.noreply.github.com>
Date: Mon, 1 Aug 2022 15:33:47 -0400
Subject: [PATCH] add namespace to path matching algorithm (#13948)

* refact:  namespace should be bound property

* chore: pass bound namespace property in template

* chore: update tests to account for bound namespace refactoring

* test: add test coverage for factoring namespace in path matching algo
---
 ui/app/abilities/variable.js                  |  7 +-
 ui/app/templates/variables/path.hbs           |  2 +-
 ui/app/templates/variables/variable/index.hbs |  4 +-
 ui/tests/acceptance/secure-variables-test.js  |  7 ++
 ui/tests/unit/abilities/variable-test.js      | 96 +++++++++++++++++++
 5 files changed, 108 insertions(+), 8 deletions(-)

diff --git a/ui/app/abilities/variable.js b/ui/app/abilities/variable.js
index 7b22d5d40..caa0ae5ec 100644
--- a/ui/app/abilities/variable.js
+++ b/ui/app/abilities/variable.js
@@ -61,15 +61,12 @@ export default class Variable extends AbstractAbility {
       ?.capabilities?.includes('destroy');
   }
 
-  @computed('token.selfTokenPolicies.[]', '_namespace')
+  @computed('token.selfTokenPolicies.[]', 'namespace')
   get allPaths() {
     return (get(this, 'token.selfTokenPolicies') || [])
       .toArray()
       .reduce((paths, policy) => {
-        const matchingNamespace = this._findMatchingNamespace(
-          get(policy, 'rulesJSON.Namespaces') || [],
-          this._namespace
-        );
+        const matchingNamespace = this.namespace ?? 'default';
 
         const variables = (get(policy, 'rulesJSON.Namespaces') || []).find(
           (namespace) => namespace.Name === matchingNamespace
diff --git a/ui/app/templates/variables/path.hbs b/ui/app/templates/variables/path.hbs
index 4d02ee84c..1a890ba1f 100644
--- a/ui/app/templates/variables/path.hbs
+++ b/ui/app/templates/variables/path.hbs
@@ -15,7 +15,7 @@
           />
         {{/if}}
         <div class="button-bar">
-        {{#if (can "write variable" path=this.model.absolutePath)}}
+        {{#if (can "write variable" path=this.model.absolutePath namespace=this.model.namespace)}}
           <LinkTo
             @route="variables.new"
             @query={{hash path=(concat this.model.absolutePath "/")}}
diff --git a/ui/app/templates/variables/variable/index.hbs b/ui/app/templates/variables/variable/index.hbs
index 6b9df4f35..6b5f98bff 100644
--- a/ui/app/templates/variables/variable/index.hbs
+++ b/ui/app/templates/variables/variable/index.hbs
@@ -36,7 +36,7 @@
   </div>
   <div>
     {{#unless this.isDeleting}}
-      {{#if (can "write variable" path=this.model.path)}}
+      {{#if (can "write variable" path=this.model.path namespace=this.model.namespace)}}
       <div class="two-step-button">
         <LinkTo
           data-test-edit-button
@@ -50,7 +50,7 @@
       </div>
       {{/if}}
     {{/unless}}
-    {{#if (can "destroy variable" path=this.model.path)}}
+    {{#if (can "destroy variable" path=this.model.path namespace=this.model.namespace)}}
       <TwoStepButton
         data-test-delete-button
         @alignRight={{true}}
diff --git a/ui/tests/acceptance/secure-variables-test.js b/ui/tests/acceptance/secure-variables-test.js
index 86d1785fe..3309d5c5d 100644
--- a/ui/tests/acceptance/secure-variables-test.js
+++ b/ui/tests/acceptance/secure-variables-test.js
@@ -62,6 +62,11 @@ module('Acceptance | secure variables', function (hooks) {
     defaultScenario(server);
     const variablesToken = server.db.tokens.find(SECURE_TOKEN_ID);
     window.localStorage.nomadTokenSecret = variablesToken.secretId;
+    server.db.variables.update({ namespace: 'default' });
+    const policy = server.db.policies.find('Variable Maker');
+    policy.rulesJSON.Namespaces[0].SecureVariables.Paths.find(
+      (path) => path.PathSpec === '*'
+    ).Capabilities = ['list', 'read', 'destroy'];
 
     await Variables.visit();
     assert.equal(currentURL(), '/variables');
@@ -480,6 +485,7 @@ module('Acceptance | secure variables', function (hooks) {
       policy.rulesJSON.Namespaces[0].SecureVariables.Paths.find(
         (path) => path.PathSpec === '*'
       ).Capabilities = ['list', 'write'];
+      server.db.variables.update({ namespace: 'default' });
       await Variables.visit();
       await click('[data-test-file-row]');
       // End Test Set-up
@@ -559,6 +565,7 @@ module('Acceptance | secure variables', function (hooks) {
       policy.rulesJSON.Namespaces[0].SecureVariables.Paths.find(
         (path) => path.PathSpec === '*'
       ).Capabilities = ['list', 'destroy'];
+      server.db.variables.update({ namespace: 'default' });
       await Variables.visit();
       await click('[data-test-file-row]');
       // End Test Set-up
diff --git a/ui/tests/unit/abilities/variable-test.js b/ui/tests/unit/abilities/variable-test.js
index 9780e4a59..909d72365 100644
--- a/ui/tests/unit/abilities/variable-test.js
+++ b/ui/tests/unit/abilities/variable-test.js
@@ -859,4 +859,100 @@ module('Unit | Ability | variable', function (hooks) {
       );
     });
   });
+
+  module('#allPaths', function () {
+    test('it filters by namespace and shows all matching paths on the namespace', function (assert) {
+      const mockToken = Service.extend({
+        aclEnabled: true,
+        selfToken: { type: 'client' },
+        selfTokenPolicies: [
+          {
+            rulesJSON: {
+              Namespaces: [
+                {
+                  Name: 'default',
+                  Capabilities: [],
+                  SecureVariables: {
+                    Paths: [{ Capabilities: ['write'], PathSpec: 'foo' }],
+                  },
+                },
+                {
+                  Name: 'bar',
+                  Capabilities: [],
+                  SecureVariables: {
+                    Paths: [
+                      { Capabilities: ['read', 'write'], PathSpec: 'foo' },
+                    ],
+                  },
+                },
+              ],
+            },
+          },
+        ],
+      });
+
+      this.owner.register('service:token', mockToken);
+      this.ability.namespace = 'bar';
+
+      const allPaths = this.ability.allPaths;
+
+      assert.deepEqual(
+        allPaths,
+        [
+          {
+            capabilities: ['read', 'write'],
+            name: 'foo',
+          },
+        ],
+        'It should return the exact path match.'
+      );
+    });
+
+    test('it matches on default if no namespace is selected', function (assert) {
+      const mockToken = Service.extend({
+        aclEnabled: true,
+        selfToken: { type: 'client' },
+        selfTokenPolicies: [
+          {
+            rulesJSON: {
+              Namespaces: [
+                {
+                  Name: 'default',
+                  Capabilities: [],
+                  SecureVariables: {
+                    Paths: [{ Capabilities: ['write'], PathSpec: 'foo' }],
+                  },
+                },
+                {
+                  Name: 'bar',
+                  Capabilities: [],
+                  SecureVariables: {
+                    Paths: [
+                      { Capabilities: ['read', 'write'], PathSpec: 'foo' },
+                    ],
+                  },
+                },
+              ],
+            },
+          },
+        ],
+      });
+
+      this.owner.register('service:token', mockToken);
+      this.ability.namespace = undefined;
+
+      const allPaths = this.ability.allPaths;
+
+      assert.deepEqual(
+        allPaths,
+        [
+          {
+            capabilities: ['write'],
+            name: 'foo',
+          },
+        ],
+        'It should return the exact path match.'
+      );
+    });
+  });
 });