From dc20fc88b9f54ca3960656413fca28ab7182173b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 18 Oct 2017 16:42:08 -0700 Subject: [PATCH] Unload all records when a token is set or cleared This guarantees no privileged data is shown to unprivileged credentials --- ui/app/controllers/settings/tokens.js | 18 ++++++++++++++- ui/app/mixins/sortable.js | 4 +++- ui/app/routes/jobs/job.js | 3 --- ui/tests/acceptance/token-test.js | 32 ++++++++++++++++++++++++++- 4 files changed, 51 insertions(+), 6 deletions(-) diff --git a/ui/app/controllers/settings/tokens.js b/ui/app/controllers/settings/tokens.js index ea9db8b04..5ea1f2a50 100644 --- a/ui/app/controllers/settings/tokens.js +++ b/ui/app/controllers/settings/tokens.js @@ -4,6 +4,7 @@ const { Controller, inject, computed, getOwner } = Ember; export default Controller.extend({ token: inject.service(), + store: inject.service(), secret: computed.reads('token.secret'), @@ -11,6 +12,10 @@ export default Controller.extend({ tokenIsInvalid: false, tokenRecord: null, + resetStore() { + this.get('store').unloadAll(); + }, + actions: { clearTokenProperties() { this.get('token').setProperties({ @@ -22,6 +27,7 @@ export default Controller.extend({ tokenIsInvalid: false, tokenRecord: null, }); + this.resetStore(); }, verifyToken() { @@ -32,10 +38,20 @@ export default Controller.extend({ TokenAdapter.findSelf().then( token => { + // Capture the token ID before clearing the store + const tokenId = token.get('id'); + + // Clear out all data to ensure only data the new token is privileged to + // see is shown + this.resetStore(); + + // Immediately refetch the token now that the store is empty + const newToken = this.get('store').findRecord('token', tokenId); + this.setProperties({ tokenIsValid: true, tokenIsInvalid: false, - tokenRecord: token, + tokenRecord: newToken, }); }, () => { diff --git a/ui/app/mixins/sortable.js b/ui/app/mixins/sortable.js index 9c01096d9..269075cd8 100644 --- a/ui/app/mixins/sortable.js +++ b/ui/app/mixins/sortable.js @@ -22,7 +22,9 @@ export default Mixin.create({ listToSort: computed(() => []), listSorted: computed('listToSort.[]', 'sortProperty', 'sortDescending', function() { - const sorted = this.get('listToSort').sortBy(this.get('sortProperty')); + const sorted = this.get('listToSort') + .compact() + .sortBy(this.get('sortProperty')); if (this.get('sortDescending')) { return sorted.reverse(); } diff --git a/ui/app/routes/jobs/job.js b/ui/app/routes/jobs/job.js index 076e3471d..b6387a2ec 100644 --- a/ui/app/routes/jobs/job.js +++ b/ui/app/routes/jobs/job.js @@ -9,9 +9,6 @@ export default Route.extend({ model({ job_id }) { return this.get('store') .findRecord('job', job_id, { reload: true }) - .then(job => { - return job.get('allocations').then(() => job); - }) .catch(notifyError(this)); }, }); diff --git a/ui/tests/acceptance/token-test.js b/ui/tests/acceptance/token-test.js index c0ee83c0f..5f0e35d60 100644 --- a/ui/tests/acceptance/token-test.js +++ b/ui/tests/acceptance/token-test.js @@ -67,7 +67,8 @@ test('the X-Nomad-Token header gets sent with requests once it is set', function const newRequests = server.pretender.handledRequests.slice(requestPosition); assert.ok(newRequests.length > 1, 'New requests have been made'); - newRequests.forEach(req => { + // Cross-origin requests can't have a token + newRequests.filter(req => !req.url.startsWith('//')).forEach(req => { assert.equal(req.requestHeaders['X-Nomad-Token'], secretId, `Token set for ${req.url}`); }); }); @@ -169,3 +170,32 @@ test('a success message and associated policies are shown when authenticating su }); }); }); + +test('setting a token clears the store', function(assert) { + const { secretId } = clientToken; + + visit('/jobs'); + + andThen(() => { + assert.ok(find('.job-row'), 'Jobs found'); + }); + + visit('/settings/tokens'); + + andThen(() => { + fillIn('.token-secret', secretId); + click('.token-submit'); + }); + + // Don't return jobs from the API the second time around + andThen(() => { + server.pretender.get('/v1/jobs', function() { + return [200, {}, '[]']; + }); + }); + + visit('/jobs'); + + // If jobs are lingering in the store, they would show up + assert.notOk(find('.job-row'), 'No jobs found'); +});