From 2a5e0841ecb024517c804741a0af7aca18abde50 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Thu, 29 Aug 2019 16:55:59 -0500 Subject: [PATCH] Change ability to check capabilities, not policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API change in #6017 returns JSON that contains the expanded policy, including all capabilities implied by a policy. So even if you set `policy="write"`, you get back a list of capabilities that includes `submit-job`. There’s therefore no reason to examine the returned policy. --- ui/app/abilities/job.js | 7 +------ ui/tests/acceptance/jobs-list-test.js | 8 ++++---- ui/tests/unit/abilities/job-test.js | 26 +++++++++++++------------- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/ui/app/abilities/job.js b/ui/app/abilities/job.js index db10e5c2e1e..7406141dd80 100644 --- a/ui/app/abilities/job.js +++ b/ui/app/abilities/job.js @@ -40,13 +40,8 @@ export default Ability.extend({ 'rulesForActiveNamespace.@each.capabilities', function() { return this.rulesForActiveNamespace.some(rules => { - // TODO given that the API returns a fully-expanded set of rules, - // where just a policy word turns into an array of capabilities, - // maybe checking capabilities is the only necessity? - const policy = rules.Policy; const capabilities = getWithDefault(rules, 'Capabilities', []); - - return policy == 'write' || capabilities.includes('submit-job'); + return capabilities.includes('submit-job'); }); } ), diff --git a/ui/tests/acceptance/jobs-list-test.js b/ui/tests/acceptance/jobs-list-test.js index b043fb79403..80f810108ae 100644 --- a/ui/tests/acceptance/jobs-list-test.js +++ b/ui/tests/acceptance/jobs-list-test.js @@ -96,7 +96,7 @@ module('Acceptance | jobs list', function(hooks) { name: 'something', rules: ` namespace "${job1.namespaceId}" { - policy = "write" + capabilities = ["list-jobs", "submit-job"] } namespace "${job2.namespaceId}" { @@ -111,7 +111,7 @@ module('Acceptance | jobs list', function(hooks) { Namespaces: [ { Name: job1.namespaceId, - Policy: 'write', + Capabilities: ['list-jobs', 'submit-job'], }, { Name: job2.namespaceId, @@ -141,7 +141,7 @@ module('Acceptance | jobs list', function(hooks) { name: 'anonymous', rules: ` namespace "default" { - policy = "write" + capabilities = ["list-jobs", "submit-job"] } node { @@ -151,7 +151,7 @@ module('Acceptance | jobs list', function(hooks) { Namespaces: [ { Name: 'default', - Policy: 'write', + Capabilities: ['list-jobs', 'submit-job'], }, ], }, diff --git a/ui/tests/unit/abilities/job-test.js b/ui/tests/unit/abilities/job-test.js index 061c04f7dbb..8e45fe0ecbc 100644 --- a/ui/tests/unit/abilities/job-test.js +++ b/ui/tests/unit/abilities/job-test.js @@ -16,7 +16,7 @@ module('Unit | Ability | job run FIXME just for ease of filtering', function(hoo assert.ok(jobAbility.canRun); }); - test('it permits job run for client tokens with a policy that has namespace write', function(assert) { + test('it permits job run for client tokens with a policy that has namespace submit-job', function(assert) { const mockSystem = Service.extend({ activeNamespace: { name: 'aNamespace', @@ -31,7 +31,7 @@ module('Unit | Ability | job run FIXME just for ease of filtering', function(hoo Namespaces: [ { Name: 'aNamespace', - Policy: 'write', + Capabilities: ['submit-job'], }, ], }, @@ -46,7 +46,7 @@ module('Unit | Ability | job run FIXME just for ease of filtering', function(hoo assert.ok(jobAbility.canRun); }); - test('it permits job run for client tokens with a policy that has default namespace write and no policy for active namespace', function(assert) { + test('it permits job run for client tokens with a policy that has default namespace submit-job and no capabilities for active namespace', function(assert) { const mockSystem = Service.extend({ activeNamespace: { name: 'anotherNamespace', @@ -61,11 +61,11 @@ module('Unit | Ability | job run FIXME just for ease of filtering', function(hoo Namespaces: [ { Name: 'aNamespace', - Policy: 'read', + Capabilities: [], }, { Name: 'default', - Policy: 'write', + Capabilities: ['submit-job'], }, ], }, @@ -80,7 +80,7 @@ module('Unit | Ability | job run FIXME just for ease of filtering', function(hoo assert.ok(jobAbility.canRun); }); - test('it blocks job run for client tokens with a policy that has namespace read', function(assert) { + test('it blocks job run for client tokens with a policy that has no submit-job capability', function(assert) { const mockSystem = Service.extend({ activeNamespace: { name: 'aNamespace', @@ -95,7 +95,7 @@ module('Unit | Ability | job run FIXME just for ease of filtering', function(hoo Namespaces: [ { Name: 'aNamespace', - Policy: 'read', + Capabilities: ['list-jobs'], }, ], }, @@ -125,27 +125,27 @@ module('Unit | Ability | job run FIXME just for ease of filtering', function(hoo Namespaces: [ { Name: 'production-*', - Policy: 'write', + Capabilities: ['submit-job'], }, { Name: 'production-api', - Policy: 'write', + Capabilities: ['submit-job'], }, { Name: 'production-web', - Policy: 'deny', + Capabilities: [], }, { Name: '*-suffixed', - Policy: 'write', + Capabilities: ['submit-job'], }, { Name: '*-more-suffixed', - Policy: 'deny', + Capabilities: [], }, { Name: '*-abc-*', - Policy: 'write', + Capabilities: ['submit-job'], }, ], },