From 1f6ce06744c20a12dd8898eeed62c92765bc6055 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 12 Oct 2017 16:56:20 -0700 Subject: [PATCH 1/3] Specialized error for 403s that links to the ACLs page --- ui/app/controllers/application.js | 4 +++ ui/app/routes/application.js | 5 ++- ui/app/routes/jobs/job.js | 2 +- ui/app/templates/application.hbs | 7 +++++ .../acceptance/application-errors-test.js | 31 ++++++++++++++++++- 5 files changed, 46 insertions(+), 3 deletions(-) diff --git a/ui/app/controllers/application.js b/ui/app/controllers/application.js index 17629c90f..de9b23684 100644 --- a/ui/app/controllers/application.js +++ b/ui/app/controllers/application.js @@ -27,6 +27,10 @@ export default Controller.extend({ .map(code => '' + code); }), + is403: computed('errorCodes.[]', function() { + return this.get('errorCodes').includes('403'); + }), + is404: computed('errorCodes.[]', function() { return this.get('errorCodes').includes('404'); }), diff --git a/ui/app/routes/application.js b/ui/app/routes/application.js index f84961641..7728d25f2 100644 --- a/ui/app/routes/application.js +++ b/ui/app/routes/application.js @@ -11,10 +11,13 @@ export default Route.extend({ actions: { didTransition() { - this.controllerFor('application').set('error', null); window.scrollTo(0, 0); }, + willTransition() { + this.controllerFor('application').set('error', null); + }, + error(error) { this.controllerFor('application').set('error', error); }, diff --git a/ui/app/routes/jobs/job.js b/ui/app/routes/jobs/job.js index 5ae53a122..076e3471d 100644 --- a/ui/app/routes/jobs/job.js +++ b/ui/app/routes/jobs/job.js @@ -8,7 +8,7 @@ export default Route.extend({ model({ job_id }) { return this.get('store') - .find('job', job_id) + .findRecord('job', job_id, { reload: true }) .then(job => { return job.get('allocations').then(() => job); }) diff --git a/ui/app/templates/application.hbs b/ui/app/templates/application.hbs index 1299680b6..2ee39614a 100644 --- a/ui/app/templates/application.hbs +++ b/ui/app/templates/application.hbs @@ -10,6 +10,13 @@ {{else if is404}}

Not Found

What you're looking for couldn't be found. It either doesn't exist or you are not authorized to see it.

+ {{else if is403}} +

Not Authorized

+ {{#if token.secret}} +

Your {{#link-to "settings.tokens"}}ACL token{{/link-to}} does not provide the required permissions. Contact your administrator if this is an error.

+ {{else}} +

Provide an {{#link-to "settings.tokens"}}ACL token{{/link-to}} with requisite permissions to view this.

+ {{/if}} {{else}}

Error

Something went wrong.

diff --git a/ui/tests/acceptance/application-errors-test.js b/ui/tests/acceptance/application-errors-test.js index 385961325..78373bd7e 100644 --- a/ui/tests/acceptance/application-errors-test.js +++ b/ui/tests/acceptance/application-errors-test.js @@ -11,7 +11,7 @@ moduleForAcceptance('Acceptance | application errors ', { }); test('transitioning away from an error page resets the global error', function(assert) { - server.pretender.get('/v1/nodes', () => [403, {}, null]); + server.pretender.get('/v1/nodes', () => [500, {}, null]); visit('/nodes'); @@ -25,3 +25,32 @@ test('transitioning away from an error page resets the global error', function(a assert.notOk(find('.error-message'), 'Application is no longer in an error state'); }); }); + +test('the 403 error page links to the ACL tokens page', function(assert) { + const job = server.db.jobs[0]; + + server.pretender.get(`/v1/job/${job.id}`, () => [403, {}, null]); + + visit(`/jobs/${job.id}`); + + andThen(() => { + assert.ok(find('.error-message'), 'Error message is shown'); + assert.equal( + find('.error-message .title').textContent, + 'Not Authorized', + 'Error message is for 403' + ); + }); + + andThen(() => { + click('.error-message a'); + }); + + andThen(() => { + assert.equal( + currentURL(), + '/settings/tokens', + 'Error message contains a link to the tokens page' + ); + }); +}); From f6f024235e3de773a8d4da50441af16e9958fb0f Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 12 Oct 2017 17:40:04 -0700 Subject: [PATCH 2/3] Handle the case where hash.Members is undefined --- ui/app/serializers/agent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/serializers/agent.js b/ui/app/serializers/agent.js index f54db7276..f99134f15 100644 --- a/ui/app/serializers/agent.js +++ b/ui/app/serializers/agent.js @@ -28,7 +28,7 @@ export default ApplicationSerializer.extend({ }, normalizeResponse(store, typeClass, hash, ...args) { - return this._super(store, typeClass, hash.Members, ...args); + return this._super(store, typeClass, hash.Members || [], ...args); }, normalizeSingleResponse(store, typeClass, hash, id, ...args) { From 4a35f3c5a5cc4e355d71d32380cb4ef12c41a011 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 12 Oct 2017 17:40:49 -0700 Subject: [PATCH 3/3] Handle 403s gracefully - When a list 403s, treat it as if it were empty - When a single resource 403s, redirect to an application error page that has a backdoor link to the tokens page --- ui/app/adapters/application.js | 9 +++++++-- ui/app/controllers/application.js | 15 ++------------- ui/app/utils/codes-for-error.js | 15 +++++++++++++++ 3 files changed, 24 insertions(+), 15 deletions(-) create mode 100644 ui/app/utils/codes-for-error.js diff --git a/ui/app/adapters/application.js b/ui/app/adapters/application.js index ee7dd063a..96837c027 100644 --- a/ui/app/adapters/application.js +++ b/ui/app/adapters/application.js @@ -1,5 +1,6 @@ import Ember from 'ember'; import RESTAdapter from 'ember-data/adapters/rest'; +import codesForError from '../utils/codes-for-error'; const { get, computed, inject } = Ember; @@ -21,8 +22,12 @@ export default RESTAdapter.extend({ findAll() { return this._super(...arguments).catch(error => { - if (error.code === '501' || (error.errors && error.errors.findBy('status', '501'))) { - // Feature is not implemented in this version of Nomad + const errorCodes = codesForError(error); + + const isNotAuthorized = errorCodes.includes('403'); + const isNotImplemented = errorCodes.includes('501'); + + if (isNotAuthorized || isNotImplemented) { return []; } diff --git a/ui/app/controllers/application.js b/ui/app/controllers/application.js index de9b23684..f577ec54e 100644 --- a/ui/app/controllers/application.js +++ b/ui/app/controllers/application.js @@ -1,4 +1,5 @@ import Ember from 'ember'; +import codesForError from '../utils/codes-for-error'; const { Controller, computed, inject, run, observer } = Ember; @@ -12,19 +13,7 @@ export default Controller.extend({ }), errorCodes: computed('error', function() { - const error = this.get('error'); - const codes = [error.code]; - - if (error.errors) { - error.errors.forEach(err => { - codes.push(err.status); - }); - } - - return codes - .compact() - .uniq() - .map(code => '' + code); + return codesForError(this.get('error')); }), is403: computed('errorCodes.[]', function() { diff --git a/ui/app/utils/codes-for-error.js b/ui/app/utils/codes-for-error.js new file mode 100644 index 000000000..199290fa6 --- /dev/null +++ b/ui/app/utils/codes-for-error.js @@ -0,0 +1,15 @@ +// Returns an array of error codes as strings for an Ember error object +export default function codesForError(error) { + const codes = [error.code]; + + if (error.errors) { + error.errors.forEach(err => { + codes.push(err.status); + }); + } + + return codes + .compact() + .uniq() + .map(code => '' + code); +}