From 61d8dd8e5bbbcde9aa2cd273a871aed12bbfbaa7 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Mon, 11 Jul 2022 12:33:17 -0400 Subject: [PATCH] [bugfix, ui] Allow running jobs from a namespace-limited token (#13659) * Allow running jobs from a namespace-limited token * qpNamespace cleanup * Looks like parse can deal with a * namespace * A little diff cleanup * Defensive destructuring * Removing accidental friendly-fire on can-scale * Testfix: Job run buttons from jobs index * Testfix: activeRegion job adapter string * Testfix: unit tests for job abilities correctly reflect the any-namespace rule * Testfix: job editor test looks for requests with namespace applied on plan --- ui/app/abilities/job.js | 29 +++++++++++++++-- ui/app/adapters/job.js | 2 +- ui/app/components/job-editor.js | 4 +-- ui/tests/acceptance/jobs-list-test.js | 31 +++++++++++++++++++ .../integration/components/job-editor-test.js | 2 +- ui/tests/unit/abilities/job-test.js | 13 +++++--- ui/tests/unit/adapters/job-test.js | 2 +- 7 files changed, 70 insertions(+), 13 deletions(-) diff --git a/ui/app/abilities/job.js b/ui/app/abilities/job.js index 02e175cb42b9..1c0fb1bb3097 100644 --- a/ui/app/abilities/job.js +++ b/ui/app/abilities/job.js @@ -1,5 +1,5 @@ import AbstractAbility from './abstract'; -import { computed } from '@ember/object'; +import { computed, get } from '@ember/object'; import { or } from '@ember/object/computed'; export default class Job extends AbstractAbility { @@ -9,7 +9,7 @@ export default class Job extends AbstractAbility { @or( 'bypassAuthorization', 'selfTokenIsManagement', - 'policiesSupportRunning', + 'specificNamespaceSupportsRunning', 'policiesSupportScaling' ) canScale; @@ -23,8 +23,31 @@ export default class Job extends AbstractAbility { @or('bypassAuthorization', 'selfTokenIsManagement', 'policiesSupportDispatching') canDispatch; - @computed('rulesForNamespace.@each.capabilities') + policyNamespacesIncludePermissions(policies = [], permissions = []) { + // For each policy record, extract all policies of all namespaces + const allNamespacePolicies = policies + .toArray() + .map(policy => get(policy, 'rulesJSON.Namespaces')) + .flat() + .map((namespace = {}) => { + return namespace.Capabilities; + }) + .flat() + .compact(); + + // Check for requested permissions + return allNamespacePolicies.some(policy => { + return permissions.includes(policy); + }); + } + + @computed('token.selfTokenPolicies.[]') get policiesSupportRunning() { + return this.policyNamespacesIncludePermissions(this.token.selfTokenPolicies, ['submit-job']); + } + + @computed('rulesForNamespace.@each.capabilities') + get specificNamespaceSupportsRunning() { return this.namespaceIncludesCapability('submit-job'); } diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index 7ff6027ff8d5..f4b07f87e445 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -25,7 +25,7 @@ export default class JobAdapter extends WatchableNamespaceIDs { } parse(spec) { - const url = addToPath(this.urlForFindAll('job'), '/parse'); + const url = addToPath(this.urlForFindAll('job'), '/parse?namespace=*'); return this.ajax(url, 'POST', { data: { JobHCL: spec, diff --git a/ui/app/components/job-editor.js b/ui/app/components/job-editor.js index bfa8bba3ee29..b5384277ef15 100644 --- a/ui/app/components/job-editor.js +++ b/ui/app/components/job-editor.js @@ -51,7 +51,7 @@ export default class JobEditor extends Component { try { yield this.job.parse(); } catch (err) { - const error = messageFromAdapterError(err) || 'Could not parse input'; + const error = messageFromAdapterError(err, 'parse jobs') || 'Could not parse input'; this.set('parseError', error); this.scrollToError(); return; @@ -61,7 +61,7 @@ export default class JobEditor extends Component { const plan = yield this.job.plan(); this.set('planOutput', plan); } catch (err) { - const error = messageFromAdapterError(err) || 'Could not plan job'; + const error = messageFromAdapterError(err, 'plan jobs') || 'Could not plan job'; this.set('planError', error); this.scrollToError(); } diff --git a/ui/tests/acceptance/jobs-list-test.js b/ui/tests/acceptance/jobs-list-test.js index 5e5c2052e9c0..a909a23b5a74 100644 --- a/ui/tests/acceptance/jobs-list-test.js +++ b/ui/tests/acceptance/jobs-list-test.js @@ -388,6 +388,37 @@ module('Acceptance | jobs list', function(hooks) { await JobsList.visit({ namespace: READ_AND_WRITE_NAMESPACE }); assert.notOk(JobsList.runJobButton.isDisabled); + await JobsList.visit({ namespace: READ_ONLY_NAMESPACE }); + assert.notOk(JobsList.runJobButton.isDisabled); + }); + + test('when the user has no client tokens that allow them to run a job', async function(assert) { + const READ_AND_WRITE_NAMESPACE = 'read-and-write-namespace'; + const READ_ONLY_NAMESPACE = 'read-only-namespace'; + + server.create('namespace', { id: READ_ONLY_NAMESPACE }); + + const policy = server.create('policy', { + id: 'something', + name: 'something', + rulesJSON: { + Namespaces: [ + { + Name: READ_ONLY_NAMESPACE, + Capabilities: ['list-job'], + }, + ], + }, + }); + + clientToken.policyIds = [policy.id]; + clientToken.save(); + + window.localStorage.nomadTokenSecret = clientToken.secretId; + + await JobsList.visit({ namespace: READ_AND_WRITE_NAMESPACE }); + assert.ok(JobsList.runJobButton.isDisabled); + await JobsList.visit({ namespace: READ_ONLY_NAMESPACE }); assert.ok(JobsList.runJobButton.isDisabled); }); diff --git a/ui/tests/integration/components/job-editor-test.js b/ui/tests/integration/components/job-editor-test.js index d3818ca4b9ce..b1f2c44a67c3 100644 --- a/ui/tests/integration/components/job-editor-test.js +++ b/ui/tests/integration/components/job-editor-test.js @@ -154,7 +154,7 @@ module('Integration | Component | job-editor', function(hooks) { await renderNewJob(this, job); await planJob(spec); const requests = this.server.pretender.handledRequests.mapBy('url'); - assert.ok(requests.includes('/v1/jobs/parse'), 'HCL job spec is parsed first'); + assert.ok(requests.includes('/v1/jobs/parse?namespace=*'), 'HCL job spec is parsed first'); assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'HCL job spec is planned'); assert.ok( requests.indexOf('/v1/jobs/parse') < requests.indexOf(`/v1/job/${newJobName}/plan`), diff --git a/ui/tests/unit/abilities/job-test.js b/ui/tests/unit/abilities/job-test.js index 8cd2b0ac1e7f..8a9aafc7f7a6 100644 --- a/ui/tests/unit/abilities/job-test.js +++ b/ui/tests/unit/abilities/job-test.js @@ -236,14 +236,17 @@ module('Unit | Ability | job', function(hooks) { this.owner.register('service:system', mockSystem); this.owner.register('service:token', mockToken); - assert.ok(this.can.cannot('run job', null, { namespace: 'production-web' })); + assert.ok( + this.can.can( + 'run job', + null, + { namespace: 'production-web' }, + 'The existence of a single namespace where a job can be run means that can run is enabled' + ) + ); assert.ok(this.can.can('run job', null, { namespace: 'production-api' })); assert.ok(this.can.can('run job', null, { namespace: 'production-other' })); assert.ok(this.can.can('run job', null, { namespace: 'something-suffixed' })); - assert.ok( - this.can.cannot('run job', null, { namespace: 'something-more-suffixed' }), - 'expected the namespace with the greatest number of matched characters to be chosen' - ); assert.ok( this.can.can('run job', null, { namespace: '000-abc-999' }), 'expected to be able to match against more than one wildcard' diff --git a/ui/tests/unit/adapters/job-test.js b/ui/tests/unit/adapters/job-test.js index e6d283bb9c5b..3a2299a5a54a 100644 --- a/ui/tests/unit/adapters/job-test.js +++ b/ui/tests/unit/adapters/job-test.js @@ -513,7 +513,7 @@ module('Unit | Adapter | Job', function(hooks) { await this.subject().parse('job "name-goes-here" {'); const request = this.server.pretender.handledRequests[0]; - assert.equal(request.url, `/v1/jobs/parse?region=${region}`); + assert.equal(request.url, `/v1/jobs/parse?namespace=*®ion=${region}`); assert.equal(request.method, 'POST'); assert.deepEqual(JSON.parse(request.requestBody), { JobHCL: 'job "name-goes-here" {',