ui: update path matching logic to account for schema change (#13605)

* refact: update path matching logic * refact: update tests to reflect rulesJSON change
hashicorp · Jul 6, 2022 · 1029bd5 · 1029bd5
1 parent ebdc450
commit 1029bd5
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 74 deletions.
diff --git a/ui/app/abilities/variable.js b/ui/app/abilities/variable.js
@@ -6,7 +6,6 @@ const WILDCARD_GLOB = '*';
 const WILDCARD_PATTERN = '/';
 const GLOBAL_FLAG = 'g';
 const WILDCARD_PATTERN_REGEX = new RegExp(WILDCARD_PATTERN, GLOBAL_FLAG);
-const PATH_PATTERN_REGEX = new RegExp(/Path(.*)/);
 
 export default class Variable extends AbstractAbility {
   // Pass in a namespace to `can` or `cannot` calls to override
@@ -46,24 +45,20 @@ export default class Variable extends AbstractAbility {
     });
   }
 
-  @computed('rulesForNamespace.@each.capabilities', 'path')
+  @computed('path', 'allPaths')
   get policiesSupportVariableWriting() {
     const matchingPath = this._nearestMatchingPath(this.path);
-    return this.rulesForNamespace.some((rules) => {
-      const keyName = `SecureVariables.Path "${matchingPath}".Capabilities`;
-      const capabilities = get(rules, keyName) || [];
-      return capabilities.includes('write');
-    });
+    return this.allPaths
+      .find((path) => path.name === matchingPath)
+      ?.capabilities?.includes('write');
   }
 
-  @computed('rulesForNamespace.@each.capabilities', 'path')
+  @computed('path', 'allPaths')
   get policiesSupportVariableDestroy() {
     const matchingPath = this._nearestMatchingPath(this.path);
-    return this.rulesForNamespace.some((rules) => {
-      const keyName = `SecureVariables.Path "${matchingPath}".Capabilities`;
-      const capabilities = get(rules, keyName) || [];
-      return capabilities.includes('destroy');
-    });
+    return this.allPaths
+      .find((path) => path.name === matchingPath)
+      ?.capabilities?.includes('destroy');
   }
 
   @computed('token.selfTokenPolicies.[]', '_namespace')
@@ -75,12 +70,22 @@ export default class Variable extends AbstractAbility {
           get(policy, 'rulesJSON.Namespaces') || [],
           this._namespace
         );
+
         const variables = (get(policy, 'rulesJSON.Namespaces') || []).find(
           (namespace) => namespace.Name === matchingNamespace
         )?.SecureVariables;
-        paths = { ...paths, ...variables };
+
+        const pathNames = variables?.Paths?.map((path) => ({
+          name: path.PathSpec,
+          capabilities: path.Capabilities,
+        }));
+
+        if (pathNames) {
+          paths = [...paths, ...pathNames];
+        }
+
         return paths;
-      }, {});
+      }, []);
   }
 
   _formatMatchingPathRegEx(path, wildCardPlacement = 'end') {
@@ -100,21 +105,16 @@ export default class Variable extends AbstractAbility {
     const matches = [];
 
     for (const pathName of pathNames) {
-      const pathSubString = pathName.match(PATH_PATTERN_REGEX)[1];
-      const sanitizedPath = JSON.parse(pathSubString);
-
-      if (this._doesMatchPattern(sanitizedPath, path))
-        matches.push(sanitizedPath);
+      if (this._doesMatchPattern(pathName, path)) matches.push(pathName);
     }
 
     return matches;
   }
 
   _nearestMatchingPath(path) {
-    const formattedPathKey = `Path "${path}"`;
-    const pathNames = Object.keys(this.allPaths);
+    const pathNames = this.allPaths.map((path) => path.name);
 
-    if (pathNames.includes(formattedPathKey)) {
+    if (pathNames.includes(path)) {
       return path;
     }
 

diff --git a/ui/tests/acceptance/secure-variables-test.js b/ui/tests/acceptance/secure-variables-test.js
@@ -309,7 +309,7 @@ module('Acceptance | secure variables', function (hooks) {
       assert
         .dom('[data-test-delete-button]')
         .exists('The delete button is enabled in the view.');
-      await click('[data-test-delete-button]');
+      await click('[data-test-idle-button]');
 
       assert
         .dom('[data-test-confirmation-message]')
@@ -319,7 +319,7 @@ module('Acceptance | secure variables', function (hooks) {
 
       assert.equal(
         currentRouteName(),
-        'variables',
+        'variables.index',
         'Navigates user back to variables list page after destroying a variable.'
       );
 

diff --git a/ui/tests/unit/abilities/variable-test.js b/ui/tests/unit/abilities/variable-test.js
@@ -146,9 +146,7 @@ module('Unit | Ability | variable', function (hooks) {
                   Name: 'default',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "*"': {
-                      Capabilities: ['write'],
-                    },
+                    Paths: [{ Capabilities: ['write'], PathSpec: '*' }],
                   },
                 },
               ],
@@ -174,18 +172,14 @@ module('Unit | Ability | variable', function (hooks) {
                   Name: 'default',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "foo/bar"': {
-                      Capabilities: ['list'],
-                    },
+                    Paths: [{ Capabilities: ['list'], PathSpec: 'foo/bar' }],
                   },
                 },
                 {
                   Name: 'pablo',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "foo/bar"': {
-                      Capabilities: ['write'],
-                    },
+                    Paths: [{ Capabilities: ['write'], PathSpec: 'foo/bar' }],
                   },
                 },
               ],
@@ -210,7 +204,7 @@ module('Unit | Ability | variable', function (hooks) {
 
       this.owner.register('service:token', mockToken);
 
-      assert.notOk(this.ability.canWrite);
+      assert.notOk(this.ability.canDestroy);
     });
 
     test('it permits destroying variables when token type is management', function (assert) {
@@ -221,18 +215,18 @@ module('Unit | Ability | variable', function (hooks) {
 
       this.owner.register('service:token', mockToken);
 
-      assert.ok(this.ability.canWrite);
+      assert.ok(this.ability.canDestroy);
     });
 
-    test('it permits creating variables when acl is disabled', function (assert) {
+    test('it permits destroying variables when acl is disabled', function (assert) {
       const mockToken = Service.extend({
         aclEnabled: false,
         selfToken: { type: 'client' },
       });
 
       this.owner.register('service:token', mockToken);
 
-      assert.ok(this.ability.canWrite);
+      assert.ok(this.ability.canDestroy);
     });
 
     test('it permits destroying variables when token has SecureVariables with write capabilities in its rules', function (assert) {
@@ -247,9 +241,7 @@ module('Unit | Ability | variable', function (hooks) {
                   Name: 'default',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "*"': {
-                      Capabilities: ['destroy'],
-                    },
+                    Paths: [{ Capabilities: ['destroy'], PathSpec: '*' }],
                   },
                 },
               ],
@@ -275,18 +267,14 @@ module('Unit | Ability | variable', function (hooks) {
                   Name: 'default',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "foo/bar"': {
-                      Capabilities: ['list'],
-                    },
+                    Paths: [{ Capabilities: ['list'], PathSpec: 'foo/bar' }],
                   },
                 },
                 {
                   Name: 'pablo',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "foo/bar"': {
-                      Capabilities: ['destroy'],
-                    },
+                    Paths: [{ Capabilities: ['destroy'], PathSpec: 'foo/bar' }],
                   },
                 },
               ],
@@ -316,9 +304,7 @@ module('Unit | Ability | variable', function (hooks) {
                   Name: 'default',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "foo"': {
-                      Capabilities: ['write'],
-                    },
+                    Paths: [{ Capabilities: ['write'], PathSpec: 'foo' }],
                   },
                 },
               ],
@@ -339,7 +325,7 @@ module('Unit | Ability | variable', function (hooks) {
       );
     });
 
-    test('returns capabilities for the nearest ancestor if no exact match', function (assert) {
+    test('returns capabilities for the nearest fuzzy match if no exact match', function (assert) {
       const mockToken = Service.extend({
         aclEnabled: true,
         selfToken: { type: 'client' },
@@ -351,12 +337,10 @@ module('Unit | Ability | variable', function (hooks) {
                   Name: 'default',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "foo/*"': {
-                      Capabilities: ['write'],
-                    },
-                    'Path "foo/bar/*"': {
-                      Capabilities: ['write'],
-                    },
+                    Paths: [
+                      { Capabilities: ['write'], PathSpec: 'foo/*' },
+                      { Capabilities: ['write'], PathSpec: 'foo/bar/*' },
+                    ],
                   },
                 },
               ],
@@ -373,7 +357,7 @@ module('Unit | Ability | variable', function (hooks) {
       assert.equal(
         nearestMatchingPath,
         'foo/bar/*',
-        'It should return the nearest ancestor matching path.'
+        'It should return the nearest fuzzy matching path.'
       );
     });
 
@@ -389,9 +373,7 @@ module('Unit | Ability | variable', function (hooks) {
                   Name: 'default',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "foo/*"': {
-                      Capabilities: ['write'],
-                    },
+                    Paths: [{ Capabilities: ['write'], PathSpec: 'foo/*' }],
                   },
                 },
               ],
@@ -408,7 +390,7 @@ module('Unit | Ability | variable', function (hooks) {
       assert.equal(
         nearestMatchingPath,
         'foo/*',
-        'It should handle wildcard glob prefixes.'
+        'It should handle wildcard glob.'
       );
     });
 
@@ -424,12 +406,10 @@ module('Unit | Ability | variable', function (hooks) {
                   Name: 'default',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "*/bar"': {
-                      Capabilities: ['write'],
-                    },
-                    'Path "*/bar/baz"': {
-                      Capabilities: ['write'],
-                    },
+                    Paths: [
+                      { Capabilities: ['write'], PathSpec: '*/bar' },
+                      { Capabilities: ['write'], PathSpec: '*/bar/baz' },
+                    ],
                   },
                 },
               ],
@@ -462,12 +442,10 @@ module('Unit | Ability | variable', function (hooks) {
                   Name: 'default',
                   Capabilities: [],
                   SecureVariables: {
-                    'Path "*/bar"': {
-                      Capabilities: ['write'],
-                    },
-                    'Path "foo/*"': {
-                      Capabilities: ['write'],
-                    },
+                    Paths: [
+                      { Capabilities: ['write'], PathSpec: '*/bar' },
+                      { Capabilities: ['write'], PathSpec: 'foo/*' },
+                    ],
                   },
                 },
               ],