[ui] Modify variable access permissions for UI users with write in only certain namespaces (#24073)

* Modify variable access permissions for UI users with write in only certain namespaces

* Addressing some PR comments

* Variables index namespaces on * and ability checks are now namespaced

* Mistook Delete for Destroy, and update unit tests for mult-return allPaths

.changelog/24073.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fixes an issue where variables paths would not let namespaced users write variables unless they also had wildcard namespace variable write permissions
+```

ui/app/abilities/variable.js

@@ -60,12 +60,68 @@ export default class Variable extends AbstractAbility {
     );
   }
 
-  @computed('path', 'allPaths')
+  /**
+   * Check if the user has read access to a specific path in a specific namespace.
+   * @returns {boolean}
+   */
+  @computed(
+    'allVariablePathRules',
+    'namespace',
+    'path',
+    'token.selfTokenPolicies'
+  )
   get policiesSupportVariableRead() {
     const matchingPath = this._nearestMatchingPath(this.path);
-    return this.allPaths
-      .find((path) => path.name === matchingPath)
-      ?.capabilities?.includes('read');
+    if (this.namespace === WILDCARD_GLOB) {
+      return this.policyNamespacesIncludeVariablesCapabilities(
+        this.token.selfTokenPolicies,
+        ['read'],
+        matchingPath
+      );
+    } else {
+      return this.allVariablePathRules.some((rule) => {
+        const ruleMatchingPath = this._nearestMatchingPath(rule.name);
+        return (
+          (rule.namespace === WILDCARD_GLOB ||
+            rule.namespace === this.namespace) &&
+          (ruleMatchingPath === WILDCARD_GLOB ||
+            ruleMatchingPath === matchingPath) &&
+          rule.capabilities.includes('read')
+        );
+      });
+    }
+  }
+
+  /**
+   * Check if the user has destroy access to a specific path in a specific namespace.
+   * @returns {boolean}
+   */
+  @computed(
+    'allVariablePathRules',
+    'namespace',
+    'path',
+    'token.selfTokenPolicies'
+  )
+  get policiesSupportVariableDestroy() {
+    const matchingPath = this._nearestMatchingPath(this.path);
+    if (this.namespace === WILDCARD_GLOB) {
+      return this.policyNamespacesIncludeVariablesCapabilities(
+        this.token.selfTokenPolicies,
+        ['destroy'],
+        matchingPath
+      );
+    } else {
+      return this.allVariablePathRules.some((rule) => {
+        const ruleMatchingPath = this._nearestMatchingPath(rule.name);
+        return (
+          (rule.namespace === WILDCARD_GLOB ||
+            rule.namespace === this.namespace) &&
+          (ruleMatchingPath === WILDCARD_GLOB ||
+            ruleMatchingPath === matchingPath) &&
+          rule.capabilities.includes('destroy')
+        );
+      });
+    }
   }
 
   /**
@@ -85,7 +141,7 @@ export default class Variable extends AbstractAbility {
     capabilities = [],
     path
   ) {
-    const namespacesWithVariableCapabilities = policies
+    const variableCapabilitiesAmongNamespaces = policies
       .toArray()
       .filter((policy) => get(policy, 'rulesJSON.Namespaces'))
       .map((policy) => get(policy, 'rulesJSON.Namespaces'))
@@ -109,66 +165,75 @@ export default class Variable extends AbstractAbility {
       .compact();
 
     // Check for requested permissions
-    return namespacesWithVariableCapabilities.some((abilityList) => {
-      return capabilities.includes(abilityList);
+    return variableCapabilitiesAmongNamespaces.some((abilityList) => {
+      ['write', 'read', 'destroy'];
+      return capabilities.includes(abilityList); // at least one of the capabilities is included in the list
     });
   }
 
-  @computed('allPaths', 'namespace', 'path', 'token.selfTokenPolicies')
+  /**
+   * Check if the user has write access to a specific path in a specific namespace.
+   * @returns {boolean}
+   */
+  @computed(
+    'allVariablePathRules',
+    'namespace',
+    'path',
+    'token.selfTokenPolicies'
+  )
   get policiesSupportVariableWriting() {
-    if (this.namespace === WILDCARD_GLOB && this.path === WILDCARD_GLOB) {
-      // If you're checking if you can write from root, and you don't specify a namespace,
-      // Then if you can write in ANY path in ANY namespace, you can get to /new.
+    const matchingPath = this._nearestMatchingPath(this.path);
+    if (this.namespace === WILDCARD_GLOB) {
+      // Check policyNamespacesIncludeVariablesCapabilities, which is namespace-agnostic.
       return this.policyNamespacesIncludeVariablesCapabilities(
         this.token.selfTokenPolicies,
         ['write'],
-        this._nearestMatchingPath(this.path)
+        matchingPath
       );
     } else {
-      // Checking a specific path in a specific namespace.
-      // TODO: This doesn't cover the case when you're checking for the * namespace at a specific path.
-      // Right now we require you to specify yournamespace to enable the button.
-      const matchingPath = this._nearestMatchingPath(this.path);
-      return this.allPaths
-        .find((path) => path.name === matchingPath)
-        ?.capabilities?.includes('write');
+      // If the namespace is not wildcarded, then we dig into rules by namespace.
+      return this.allVariablePathRules.some((rule) => {
+        const ruleMatchingPath = this._nearestMatchingPath(rule.name);
+        return (
+          (rule.namespace === WILDCARD_GLOB ||
+            rule.namespace === this.namespace) &&
+          (ruleMatchingPath === WILDCARD_GLOB ||
+            ruleMatchingPath === matchingPath) &&
+          rule.capabilities.includes('write')
+        );
+      });
     }
   }
 
-  @computed('path', 'allPaths')
-  get policiesSupportVariableDestroy() {
-    const matchingPath = this._nearestMatchingPath(this.path);
-    return this.allPaths
-      .find((path) => path.name === matchingPath)
-      ?.capabilities?.includes('destroy');
-  }
-
+  /**
+   * Generate a list of all the path rules for all the policies
+   * that the user has access to.
+   * {
+   *   namespace: string,
+   *   name: string,
+   *   capabilities: string[],
+   * }
+   * @returns {Array}
+   */
   @computed('token.selfTokenPolicies.[]', 'namespace')
-  get allPaths() {
+  get allVariablePathRules() {
     return (get(this, 'token.selfTokenPolicies') || [])
       .toArray()
-      .reduce((paths, policy) => {
-        const namespaces = get(policy, 'rulesJSON.Namespaces');
-        const matchingNamespace = this._nearestMatchingNamespace(
-          namespaces,
-          this.namespace
-        );
+      .flatMap((policy) => {
+        const namespaces = get(policy, 'rulesJSON.Namespaces') || [];
 
-        const variables = (namespaces || []).find(
-          (namespace) => namespace.Name === matchingNamespace
-        )?.Variables;
-
-        const pathNames = variables?.Paths?.map((path) => ({
+        return namespaces.flatMap((namespace) => {
+          const variables = namespace.Variables;
+          const pathNames =
+            variables?.Paths?.map((path) => ({
+              namespace: namespace.Name,
               name: path.PathSpec,
               capabilities: path.Capabilities,
-        }));
+            })) || [];
 
-        if (pathNames) {
-          paths = [...paths, ...pathNames];
-        }
-
-        return paths;
-      }, []);
+          return pathNames;
+        });
+      });
   }
 
   _nearestMatchingNamespace(policyNamespaces, namespace) {
@@ -201,7 +266,7 @@ export default class Variable extends AbstractAbility {
   }
 
   _nearestMatchingPath(path) {
-    const pathNames = this.allPaths.map((path) => path.name);
+    const pathNames = this.allVariablePathRules.map((path) => path.name);
     if (pathNames.includes(path)) {
       return path;
     }

ui/app/templates/variables/index.hbs

@@ -23,7 +23,7 @@
           </Hds::Dropdown>
         {{/if}}
 
-        {{#if (can "write variable" path="*" namespace=this.namespaceSelection)}}
+        {{#if (can "write variable" path="*" namespace="*")}}
           <div
             {{keyboard-shortcut
               pattern=(array "n" "v")

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,7 +860,7 @@ 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,
@@ -862,9 +868,15 @@ module('Unit | Ability | variable', function (hooks) {
           {
             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,7 +937,7 @@ 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,
@@ -933,6 +945,22 @@ module('Unit | Ability | variable', function (hooks) {
           {
             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.'