Fix ACL requirements for job details UI (#11672)

.changelog/11672.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fix the ACL requirements for displaying the job details page
+```

ui/app/abilities/client.js

@@ -7,18 +7,30 @@ import classic from 'ember-classic-decorator';
 export default class Client extends AbstractAbility {
   // Map abilities to policy options (which are coarse for nodes)
   // instead of specific behaviors.
+  @or('bypassAuthorization', 'selfTokenIsManagement', 'policiesIncludeNodeRead')
+  canRead;
+
   @or('bypassAuthorization', 'selfTokenIsManagement', 'policiesIncludeNodeWrite')
   canWrite;
 
   @computed('token.selfTokenPolicies.[]')
-  get policiesIncludeNodeWrite() {
-    // For each policy record, extract the Node policy
-    const policies = (this.get('token.selfTokenPolicies') || [])
-      .toArray()
-      .map(policy => get(policy, 'rulesJSON.Node.Policy'))
-      .compact();
+  get policiesIncludeNodeRead() {
+    return policiesIncludePermissions(this.get('token.selfTokenPolicies'), ['read', 'write']);
+  }
 
-    // Node write is allowed if any policy allows it
-    return policies.some(policy => policy === 'write');
+  @computed('token.selfTokenPolicies.[]')
+  get policiesIncludeNodeWrite() {
+    return policiesIncludePermissions(this.get('token.selfTokenPolicies'), ['write']);
   }
 }
+
+function policiesIncludePermissions(policies = [], permissions = []) {
+  // For each policy record, extract the Node policy
+  const nodePolicies = policies
+    .toArray()
+    .map(policy => get(policy, 'rulesJSON.Node.Policy'))
+    .compact();
+
+  // Check for requested permissions
+  return nodePolicies.some(policy => permissions.includes(policy));
+}

ui/app/components/job-page/abstract.js

@@ -5,6 +5,7 @@ import classic from 'ember-classic-decorator';
 
 @classic
 export default class Abstract extends Component {
+  @service can;
   @service system;
 
   job = null;
@@ -20,6 +21,10 @@ export default class Abstract extends Component {
   // Set to a { title, description } to surface an error
   errorMessage = null;
 
+  get shouldDisplayClientInformation() {
+    return this.can.can('read client') && this.job.hasClientStatus;
+  }
+
   @action
   clearErrorMessage() {
     this.set('errorMessage', null);

ui/app/components/job-page/parameterized-child.js

@@ -1,14 +1,11 @@
 import { computed } from '@ember/object';
 import { alias } from '@ember/object/computed';
-import { inject as service } from '@ember/service';
 import PeriodicChildJobPage from './periodic-child';
 import classic from 'ember-classic-decorator';
-import jobClientStatus from 'nomad-ui/utils/properties/job-client-status';
 
 @classic
 export default class ParameterizedChild extends PeriodicChildJobPage {
   @alias('job.decodedPayload') payload;
-  @service store;
 
   @computed('payload')
   get payloadJSON() {
@@ -20,10 +17,4 @@ export default class ParameterizedChild extends PeriodicChildJobPage {
     }
     return json;
   }
-
-  @jobClientStatus('nodes', 'job') jobClientStatus;
-
-  get nodes() {
-    return this.store.peekAll('node');
-  }
 }

ui/app/components/job-page/parts/job-client-status-summary.js

@@ -2,20 +2,26 @@ import Component from '@ember/component';
 import { action, computed } from '@ember/object';
 import { classNames } from '@ember-decorators/component';
 import classic from 'ember-classic-decorator';
+import jobClientStatus from 'nomad-ui/utils/properties/job-client-status';
 
 @classic
 @classNames('boxed-section')
 export default class JobClientStatusSummary extends Component {
   job = null;
-  jobClientStatus = null;
+  nodes = null;
+  forceCollapsed = false;
   gotoClients() {}
 
-  @computed
+  @computed('forceCollapsed')
   get isExpanded() {
+    if (this.forceCollapsed) return false;
+
     const storageValue = window.localStorage.nomadExpandJobClientStatusSummary;
     return storageValue != null ? JSON.parse(storageValue) : true;
   }
 
+  @jobClientStatus('nodes', 'job') jobClientStatus;
+
   @action
   onSliceClick(ev, slice) {
     this.gotoClients([slice.className.camelize()]);

ui/app/components/job-page/periodic-child.js

@@ -1,13 +1,9 @@
 import AbstractJobPage from './abstract';
 import { computed } from '@ember/object';
-import { inject as service } from '@ember/service';
 import classic from 'ember-classic-decorator';
-import jobClientStatus from 'nomad-ui/utils/properties/job-client-status';
 
 @classic
 export default class PeriodicChild extends AbstractJobPage {
-  @service store;
-
   @computed('job.{name,id}', 'job.parent.{name,id}')
   get breadcrumbs() {
     const job = this.job;
@@ -25,10 +21,4 @@ export default class PeriodicChild extends AbstractJobPage {
       },
     ];
   }
-
-  @jobClientStatus('nodes', 'job') jobClientStatus;
-
-  get nodes() {
-    return this.store.peekAll('node');
-  }
 }

ui/app/components/job-page/sysbatch.js

@@ -1,15 +1,5 @@
 import AbstractJobPage from './abstract';
 import classic from 'ember-classic-decorator';
-import { inject as service } from '@ember/service';
-import jobClientStatus from 'nomad-ui/utils/properties/job-client-status';
 
 @classic
-export default class Sysbatch extends AbstractJobPage {
-  @service store;
-
-  @jobClientStatus('nodes', 'job') jobClientStatus;
-
-  get nodes() {
-    return this.store.peekAll('node');
-  }
-}
+export default class Sysbatch extends AbstractJobPage {}

ui/app/components/job-page/system.js

@@ -1,15 +1,5 @@
 import AbstractJobPage from './abstract';
 import classic from 'ember-classic-decorator';
-import { inject as service } from '@ember/service';
-import jobClientStatus from 'nomad-ui/utils/properties/job-client-status';
 
 @classic
-export default class System extends AbstractJobPage {
-  @service store;
-
-  @jobClientStatus('nodes', 'job') jobClientStatus;
-
-  get nodes() {
-    return this.store.peekAll('node');
-  }
-}
+export default class System extends AbstractJobPage {}

ui/app/components/list-accordion/accordion-head.js

@@ -9,8 +9,10 @@ export default class AccordionHead extends Component {
   'data-test-accordion-head' = true;
 
   buttonLabel = 'toggle';
+  tooltip = '';
   isOpen = false;
   isExpandable = true;
+  isDisabled = false;
   item = null;
 
   onClose() {}

ui/app/controllers/jobs/job/index.js

@@ -1,5 +1,5 @@
 import { inject as service } from '@ember/service';
-import { alias } from '@ember/object/computed';
+import { computed } from '@ember/object';
 import Controller from '@ember/controller';
 import WithNamespaceResetting from 'nomad-ui/mixins/with-namespace-resetting';
 import { action } from '@ember/object';
@@ -23,7 +23,15 @@ export default class IndexController extends Controller.extend(WithNamespaceRese
 
   currentPage = 1;
 
-  @alias('model') job;
+  @computed('model.job')
+  get job() {
+    return this.model.job;
+  }
+
+  @computed('model.nodes.[]')
+  get nodes() {
+    return this.model.nodes;
+  }
 
   sortProperty = 'name';
   sortDescending = false;

ui/app/models/allocation.js

@@ -30,6 +30,16 @@ export default class Allocation extends Model {
   @fragment('resources') allocatedResources;
   @attr('number') jobVersion;
 
+  // Store basic node information returned by the API to avoid the need for
+  // node:read ACL permission.
+  @attr('string') nodeName;
+  @computed
+  get shortNodeId() {
+    return this.belongsTo('node')
+      .id()
+      .split('-')[0];
+  }
+
   @attr('number') modifyIndex;
   @attr('date') modifyTime;
 

ui/app/routes/jobs/job/index.js

@@ -1,4 +1,5 @@
 import Route from '@ember/routing/route';
+import { inject as service } from '@ember/service';
 import { collect } from '@ember/object/computed';
 import {
   watchRecord,
@@ -9,34 +10,49 @@ import {
 import WithWatchers from 'nomad-ui/mixins/with-watchers';
 
 export default class IndexRoute extends Route.extend(WithWatchers) {
+  @service can;
+  @service store;
+
   async model() {
-    // Optimizing future node look ups by preemptively loading everything
-    await this.store.findAll('node');
-    return this.modelFor('jobs.job');
+    const job = this.modelFor('jobs.job');
+    if (!job) {
+      return { job, nodes: [] };
+    }
+
+    // Optimizing future node look ups by preemptively loading all nodes if
+    // necessary and allowed.
+    if (this.can.can('read client') && job.get('hasClientStatus')) {
+      await this.store.findAll('node');
+    }
+    const nodes = this.store.peekAll('node');
+    return { job, nodes };
   }
 
   startWatchers(controller, model) {
-    if (!model) {
+    if (!model.job) {
       return;
     }
     controller.set('watchers', {
-      model: this.watch.perform(model),
-      summary: this.watchSummary.perform(model.get('summary')),
-      allocations: this.watchAllocations.perform(model),
-      evaluations: this.watchEvaluations.perform(model),
+      model: this.watch.perform(model.job),
+      summary: this.watchSummary.perform(model.job.get('summary')),
+      allocations: this.watchAllocations.perform(model.job),
+      evaluations: this.watchEvaluations.perform(model.job),
       latestDeployment:
-        model.get('supportsDeployments') && this.watchLatestDeployment.perform(model),
+        model.job.get('supportsDeployments') && this.watchLatestDeployment.perform(model.job),
       list:
-        model.get('hasChildren') &&
-        this.watchAllJobs.perform({ namespace: model.namespace.get('name') }),
-      nodes: model.get('hasClientStatus') && this.watchNodes.perform(),
+        model.job.get('hasChildren') &&
+        this.watchAllJobs.perform({ namespace: model.job.namespace.get('name') }),
+      nodes:
+        this.can.can('read client') &&
+        model.job.get('hasClientStatus') &&
+        this.watchNodes.perform(),
     });
   }
 
   setupController(controller, model) {
     // Parameterized and periodic detail pages, which list children jobs,
     // should sort by submit time.
-    if (model && ['periodic', 'parameterized'].includes(model.templateType)) {
+    if (model.job && ['periodic', 'parameterized'].includes(model.job.templateType)) {
       controller.setProperties({
         sortProperty: 'submitTime',
         sortDescending: true,

ui/app/templates/components/allocation-row.hbs

@@ -1,8 +1,10 @@
 <td data-test-indicators class="is-narrow">
-  {{#if this.allocation.unhealthyDrivers.length}}
-    <span data-test-icon="unhealthy-driver" class="tooltip text-center" role="tooltip" aria-label="Allocation depends on unhealthy drivers">
-      {{x-icon "alert-triangle" class="is-warning"}}
-    </span>
+  {{#if (can "read client")}}
+    {{#if this.allocation.unhealthyDrivers.length}}
+      <span data-test-icon="unhealthy-driver" class="tooltip text-center" role="tooltip" aria-label="Allocation depends on unhealthy drivers">
+        {{x-icon "alert-triangle" class="is-warning"}}
+      </span>
+    {{/if}}
   {{/if}}
   {{#if this.allocation.nextAllocation}}
     <span data-test-icon="reschedule" class="tooltip text-center" role="tooltip" aria-label="Allocation was rescheduled">
@@ -38,20 +40,28 @@
 </td>
 {{#if (eq this.context "volume")}}
   <td data-test-client>
-    <Tooltip @text={{this.allocation.node.name}}>
-      <LinkTo @route="clients.client" @model={{this.allocation.node}}>
-        {{this.allocation.node.shortId}}
-      </LinkTo>
+    <Tooltip @text={{this.allocation.nodeName}}>
+      {{#if (can "read client")}}
+        <LinkTo @route="clients.client" @model={{this.allocation.node}}>
+          {{this.allocation.shortNodeId}}
+        </LinkTo>
+      {{else}}
+        {{this.allocation.shortNodeId}}
+      {{/if}}
     </Tooltip>
   </td>
 {{/if}}
 {{#if (or (eq this.context "taskGroup") (eq this.context "job"))}}
   <td data-test-job-version>{{this.allocation.jobVersion}}</td>
   <td data-test-client>
-    <Tooltip @text={{this.allocation.node.name}}>
-      <LinkTo @route="clients.client" @model={{this.allocation.node}}>
-        {{this.allocation.node.shortId}}
-      </LinkTo>
+    <Tooltip @text={{this.allocation.nodeName}}>
+      {{#if (can "read client")}}
+        <LinkTo @route="clients.client" @model={{this.allocation.node}}>
+          {{this.allocation.shortNodeId}}
+        </LinkTo>
+      {{else}}
+        {{this.allocation.shortNodeId}}
+      {{/if}}
     </Tooltip>
   </td>
 {{else if (or (eq this.context "node") (eq this.context "volume"))}}

ui/app/templates/components/job-page/parameterized-child.hbs

@@ -16,13 +16,13 @@
 
   {{#if this.job.hasClientStatus}}
     <JobPage::Parts::JobClientStatusSummary
-      @gotoClients={{this.gotoClients}}
       @job={{this.job}}
-      @jobClientStatus={{this.jobClientStatus}}
-    />
+      @nodes={{this.nodes}}
+      @forceCollapsed={{not this.shouldDisplayClientInformation}}
+      @gotoClients={{this.gotoClients}} />
   {{/if}}
 
-  <JobPage::Parts::Summary @job={{this.job}} @forceCollapsed={{this.job.hasClientStatus}} />
+  <JobPage::Parts::Summary @job={{this.job}} @forceCollapsed={{this.shouldDisplayClientInformation}} />
 
   <JobPage::Parts::PlacementFailures @job={{this.job}} />