Mistook Delete for Destroy, and update unit tests for mult-return all…

…Paths
hashicorp · Oct 2, 2024 · 82f6a7e · 82f6a7e
1 parent ca70e85
commit 82f6a7e
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 15 deletions.
diff --git a/.changelog/24073.txt b/.changelog/24073.txt
@@ -1,3 +1,3 @@
 ```release-note:bug
-ui: Fixes an issue where variables paths would not let namespaced users write variables unless they also had * namespace variable write permissions
+ui: Fixes an issue where variables paths would not let namespaced users write variables unless they also had wildcard namespace variable write permissions
 ```
diff --git a/ui/app/abilities/variable.js b/ui/app/abilities/variable.js
@@ -82,16 +82,18 @@ export default class Variable extends AbstractAbility {
       return this.allVariablePathRules.some((rule) => {
         const ruleMatchingPath = this._nearestMatchingPath(rule.name);
         return (
-          rule.namespace === this.namespace &&
-          ruleMatchingPath === matchingPath &&
+          (rule.namespace === WILDCARD_GLOB ||
+            rule.namespace === this.namespace) &&
+          (ruleMatchingPath === WILDCARD_GLOB ||
+            ruleMatchingPath === matchingPath) &&
           rule.capabilities.includes('read')
         );
       });
     }
   }
 
   /**
-   * Check if the user has delete access to a specific path in a specific namespace.
+   * Check if the user has destroy access to a specific path in a specific namespace.
    * @returns {boolean}
    */
   @computed(
@@ -105,16 +107,18 @@ export default class Variable extends AbstractAbility {
     if (this.namespace === WILDCARD_GLOB) {
       return this.policyNamespacesIncludeVariablesCapabilities(
         this.token.selfTokenPolicies,
-        ['delete'],
+        ['destroy'],
         matchingPath
       );
     } else {
       return this.allVariablePathRules.some((rule) => {
         const ruleMatchingPath = this._nearestMatchingPath(rule.name);
         return (
-          rule.namespace === this.namespace &&
-          ruleMatchingPath === matchingPath &&
-          rule.capabilities.includes('delete')
+          (rule.namespace === WILDCARD_GLOB ||
+            rule.namespace === this.namespace) &&
+          (ruleMatchingPath === WILDCARD_GLOB ||
+            ruleMatchingPath === matchingPath) &&
+          rule.capabilities.includes('destroy')
         );
       });
     }
@@ -191,8 +195,10 @@ export default class Variable extends AbstractAbility {
       return this.allVariablePathRules.some((rule) => {
         const ruleMatchingPath = this._nearestMatchingPath(rule.name);
         return (
-          rule.namespace === this.namespace &&
-          ruleMatchingPath === matchingPath &&
+          (rule.namespace === WILDCARD_GLOB ||
+            rule.namespace === this.namespace) &&
+          (ruleMatchingPath === WILDCARD_GLOB ||
+            ruleMatchingPath === matchingPath) &&
           rule.capabilities.includes('write')
         );
       });

diff --git a/ui/tests/unit/abilities/variable-test.js b/ui/tests/unit/abilities/variable-test.js
@@ -807,21 +807,27 @@ module('Unit | Ability | variable', function (hooks) {
       this.owner.register('service:token', mockToken);
       this.ability.namespace = 'bar';
 
-      const allPaths = this.ability.allPaths;
+      const allPaths = this.ability.allVariablePathRules;
 
       assert.deepEqual(
         allPaths,
         [
+          {
+            capabilities: ['write'],
+            name: 'foo',
+            namespace: 'default',
+          },
           {
             capabilities: ['read', 'write'],
             name: 'foo',
+            namespace: 'bar',
           },
         ],
         'It should return the exact path match.'
       );
     });
 
-    test('it matches on default if no namespace is selected', function (assert) {
+    test('it matches if no namespace is selected', function (assert) {
       const mockToken = Service.extend({
         aclEnabled: true,
         selfToken: { type: 'client' },
@@ -854,17 +860,23 @@ module('Unit | Ability | variable', function (hooks) {
       this.owner.register('service:token', mockToken);
       this.ability.namespace = undefined;
 
-      const allPaths = this.ability.allPaths;
+      const allPaths = this.ability.allVariablePathRules;
 
       assert.deepEqual(
         allPaths,
         [
           {
             capabilities: ['write'],
             name: 'foo',
+            namespace: 'default',
+          },
+          {
+            capabilities: ['read', 'write'],
+            name: 'foo',
+            namespace: 'bar',
           },
         ],
-        'It should return the exact path match.'
+        'It should return both matches separated by namespace.'
       );
     });
 
@@ -925,14 +937,30 @@ module('Unit | Ability | variable', function (hooks) {
       this.owner.register('service:token', mockToken);
       this.ability.namespace = 'pablo';
 
-      const allPaths = this.ability.allPaths;
+      const allPaths = this.ability.allVariablePathRules;
 
       assert.deepEqual(
         allPaths,
         [
           {
             capabilities: ['list'],
             name: '*',
+            namespace: '*',
+          },
+          {
+            capabilities: ['list', 'read', 'destroy', 'create'],
+            name: '*',
+            namespace: 'namespace-1',
+          },
+          {
+            capabilities: ['list', 'read', 'destroy', 'create'],
+            name: 'blue/*',
+            namespace: 'namespace-2',
+          },
+          {
+            capabilities: ['list', 'read', 'create'],
+            name: 'nomad/jobs/*',
+            namespace: 'namespace-2',
           },
         ],
         'It should return the glob matching namespace match.'