From f65f00167571f027518dac24105866e8acc823a1 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Fri, 2 Nov 2018 14:44:36 +0000 Subject: [PATCH] UI: Catch 500 error on token endpoint and revert to legacy tokens (#4874) In some circumstances a consul 1.4 client could be running in an un-upgraded 1.3 or lower cluster. Currently this gives a 500 error on the new ACL token endpoint. Here we catch this specific 500 error/message and set the users AccessorID to null. Elsewhere in the frontend we use this fact (AccessorID being null) to decide whether to present the legacy or the new ACL UI to the user. Also: - Re-adds in most of the old style ACL acceptance tests, now that we are keeping the old style UI - Restricts code editors to HCL only mode for all `Rules` editing (legacy/'half legacy'/new style) - Adds a [Stop using] button to the old style ACL rows so its possible to logout. - Updates copy and documentation links for the upgrade notices --- ui-v2/app/components/app-view.js | 7 +++ ui-v2/app/helpers/token/is-legacy.js | 12 ++++- ui-v2/app/mixins/acl/with-actions.js | 23 ++++++++- ui-v2/app/routes/application.js | 9 +++- ui-v2/app/routes/dc/acls.js | 12 +++-- ui-v2/app/routes/dc/acls/index.js | 15 +++++- ui-v2/app/routes/dc/acls/tokens/index.js | 13 +++++ ui-v2/app/services/repository/policy.js | 4 +- ui-v2/app/services/repository/token.js | 24 ++++++++-- ui-v2/app/templates/dc/acls/-form.hbs | 2 +- .../app/templates/dc/acls/-notifications.hbs | 6 +++ ui-v2/app/templates/dc/acls/index.hbs | 11 +++++ .../app/templates/dc/acls/policies/-view.hbs | 2 +- .../dc/acls/tokens/-fieldsets-legacy.hbs | 2 +- ui-v2/app/templates/dc/acls/tokens/edit.hbs | 2 +- ui-v2/app/templates/dc/acls/tokens/index.hbs | 4 +- ui-v2/app/utils/acls-status.js | 9 +++- .../utils/http/acl/is-valid-server-error.js | 12 +++++ .../acceptance/components/acl-filter.feature | 2 +- .../acceptance/dc/acls/list-order.feature | 2 +- .../acceptance/dc/acls/tokens/index.feature | 33 ++++++++++++- .../dc/acls/tokens/legacy/update.feature | 2 +- ui-v2/tests/acceptance/dc/acls/update.feature | 5 +- ui-v2/tests/acceptance/dc/acls/use.feature | 10 ++-- .../tests/acceptance/page-navigation.feature | 2 +- ui-v2/tests/acceptance/token-header.feature | 11 +++-- ui-v2/tests/pages.js | 12 ++++- ui-v2/tests/pages/dc/acls/tokens/index.js | 43 +++++++++++------ ui-v2/tests/steps.js | 3 ++ .../unit/helpers/token/is-legacy-test.js | 46 ++++++++++++++++++ .../unit/mixins/acl/with-actions-test.js | 3 +- ui-v2/tests/unit/routes/application-test.js | 2 +- .../http/acl/is-valid-server-error-test.js | 48 +++++++++++++++++++ 33 files changed, 335 insertions(+), 58 deletions(-) create mode 100644 ui-v2/app/utils/http/acl/is-valid-server-error.js create mode 100644 ui-v2/tests/unit/helpers/token/is-legacy-test.js create mode 100644 ui-v2/tests/unit/utils/http/acl/is-valid-server-error-test.js diff --git a/ui-v2/app/components/app-view.js b/ui-v2/app/components/app-view.js index 6966ba6559..e5254eacc9 100644 --- a/ui-v2/app/components/app-view.js +++ b/ui-v2/app/components/app-view.js @@ -18,6 +18,13 @@ export default Component.extend(SlotsMixin, { $html.classList.remove(...templatize(['loading'])); } if (cls) { + // its possible for 'layout' templates to change after insert + // check for these specific layouts and clear them out + [...$html.classList].forEach(function(item, i) { + if (templatize(['edit', 'show', 'list']).indexOf(item) !== -1) { + $html.classList.remove(item); + } + }); $html.classList.add(...templatize(cls.split(' '))); } }, diff --git a/ui-v2/app/helpers/token/is-legacy.js b/ui-v2/app/helpers/token/is-legacy.js index 215b380b2e..d21144a3e5 100644 --- a/ui-v2/app/helpers/token/is-legacy.js +++ b/ui-v2/app/helpers/token/is-legacy.js @@ -1,9 +1,19 @@ import { helper } from '@ember/component/helper'; import { get } from '@ember/object'; +const _isLegacy = function(token) { + const rules = get(token, 'Rules'); + return get(token, 'Legacy') || (rules != null && rules.trim() != ''); +}; export function isLegacy(params, hash) { const token = params[0]; - return get(token, 'Legacy') || typeof get(token, 'Rules') !== 'undefined'; + // is array like (RecordManager isn't an array) + if (typeof token.length !== 'undefined') { + return token.find(function(item) { + return _isLegacy(item); + }); + } + return _isLegacy(token); } export default helper(isLegacy); diff --git a/ui-v2/app/mixins/acl/with-actions.js b/ui-v2/app/mixins/acl/with-actions.js index 54a09c8ede..f622645050 100644 --- a/ui-v2/app/mixins/acl/with-actions.js +++ b/ui-v2/app/mixins/acl/with-actions.js @@ -8,13 +8,34 @@ export default Mixin.create(WithBlockingActions, { actions: { use: function(item) { return get(this, 'feedback').execute(() => { + // old style legacy ACLs don't have AccessorIDs + // therefore set it to null, this way the frontend knows + // to use legacy ACLs return get(this, 'settings') - .persist({ token: get(item, 'ID') }) + .persist({ + token: { + AccessorID: null, + SecretID: get(item, 'ID'), + }, + }) .then(() => { return this.transitionTo('dc.services'); }); }, 'use'); }, + // TODO: This is also used in tokens, probably an opportunity to dry this out + logout: function(item) { + return get(this, 'feedback').execute(() => { + return get(this, 'settings') + .delete('token') + .then(() => { + // in this case we don't do the same as delete as we want to go to the new + // dc.acls.tokens page. If we get there via the dc.acls redirect/rewrite + // then we lose the flash message + return this.transitionTo('dc.acls.tokens'); + }); + }, 'logout'); + }, clone: function(item) { return get(this, 'feedback').execute(() => { return get(this, 'repo') diff --git a/ui-v2/app/routes/application.js b/ui-v2/app/routes/application.js index 662ea9a669..9c3de6c88b 100644 --- a/ui-v2/app/routes/application.js +++ b/ui-v2/app/routes/application.js @@ -12,6 +12,7 @@ export default Route.extend({ this._super(...arguments); }, repo: service('repository/dc'), + settings: service('settings'), actions: { loading: function(transition, originRoute) { let dc = null; @@ -58,13 +59,17 @@ export default Route.extend({ // 403 page // To note: Consul only gives you back a 403 if a non-existent token has been sent in the header // if a token has not been sent at all, it just gives you a 200 with an empty dataset + const model = this.modelFor('dc'); if (error.status === '403') { - return this.transitionTo('dc.acls.tokens'); + return get(this, 'settings') + .delete('token') + .then(() => { + return this.transitionTo('dc.acls.tokens', model.dc.Name); + }); } if (error.status === '') { error.message = 'Error'; } - const model = this.modelFor('dc'); hash({ error: error, dc: diff --git a/ui-v2/app/routes/dc/acls.js b/ui-v2/app/routes/dc/acls.js index 345658d688..cd32fd9c3e 100644 --- a/ui-v2/app/routes/dc/acls.js +++ b/ui-v2/app/routes/dc/acls.js @@ -2,7 +2,6 @@ import Route from '@ember/routing/route'; import { get } from '@ember/object'; import { inject as service } from '@ember/service'; import WithBlockingActions from 'consul-ui/mixins/with-blocking-actions'; - export default Route.extend(WithBlockingActions, { settings: service('settings'), feedback: service('feedback'), @@ -21,8 +20,15 @@ export default Route.extend(WithBlockingActions, { SecretID: secret, }, }) - .then(() => { - this.refresh(); + .then(item => { + // a null AccessorID means we are in legacy mode + // take the user to the legacy acls + // otherwise just refresh the page + if (get(item, 'token.AccessorID') === null) { + return this.transitionTo('dc.acls'); + } else { + this.refresh(); + } }); }); }, 'authorize'); diff --git a/ui-v2/app/routes/dc/acls/index.js b/ui-v2/app/routes/dc/acls/index.js index a2fa9d3631..b36a9e3527 100644 --- a/ui-v2/app/routes/dc/acls/index.js +++ b/ui-v2/app/routes/dc/acls/index.js @@ -7,6 +7,7 @@ import WithAclActions from 'consul-ui/mixins/acl/with-actions'; export default Route.extend(WithAclActions, { repo: service('repository/acl'), + settings: service('settings'), queryParams: { s: { as: 'filter', @@ -14,12 +15,24 @@ export default Route.extend(WithAclActions, { }, }, beforeModel: function(transition) { - return this.replaceWith('dc.acls.tokens'); + return get(this, 'settings') + .findBySlug('token') + .then(token => { + // If you don't have a token set or you have a + // token set with AccessorID set to not null (new ACL mode) + // then rewrite to the new acls + if (!token || get(token, 'AccessorID') !== null) { + // If you return here, you get a TransitionAborted error in the tests only + // everything works fine either way checking things manually + this.replaceWith('dc.acls.tokens'); + } + }); }, model: function(params) { return hash({ isLoading: false, items: get(this, 'repo').findAllByDatacenter(this.modelFor('dc').dc.Name), + token: get(this, 'settings').findBySlug('token'), }); }, setupController: function(controller, model) { diff --git a/ui-v2/app/routes/dc/acls/tokens/index.js b/ui-v2/app/routes/dc/acls/tokens/index.js index 2220e16fa7..757871e013 100644 --- a/ui-v2/app/routes/dc/acls/tokens/index.js +++ b/ui-v2/app/routes/dc/acls/tokens/index.js @@ -12,6 +12,19 @@ export default Route.extend(WithTokenActions, { replace: true, }, }, + beforeModel: function(transition) { + return get(this, 'settings') + .findBySlug('token') + .then(token => { + // If you have a token set with AccessorID set to null (legacy mode) + // then rewrite to the old acls + if (token && get(token, 'AccessorID') === null) { + // If you return here, you get a TransitionAborted error in the tests only + // everything works fine either way checking things manually + this.replaceWith('dc.acls'); + } + }); + }, model: function(params) { const repo = get(this, 'repo'); return hash({ diff --git a/ui-v2/app/services/repository/policy.js b/ui-v2/app/services/repository/policy.js index 20f0749d97..76df14ca89 100644 --- a/ui-v2/app/services/repository/policy.js +++ b/ui-v2/app/services/repository/policy.js @@ -4,7 +4,9 @@ import { typeOf } from '@ember/utils'; import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/policy'; import { Promise } from 'rsvp'; import statusFactory from 'consul-ui/utils/acls-status'; -const status = statusFactory(Promise); +import isValidServerErrorFactory from 'consul-ui/utils/http/acl/is-valid-server-error'; +const isValidServerError = isValidServerErrorFactory(); +const status = statusFactory(isValidServerError, Promise); const MODEL_NAME = 'policy'; export default Service.extend({ getModelName: function() { diff --git a/ui-v2/app/services/repository/token.js b/ui-v2/app/services/repository/token.js index a48aeec6b1..4aabe95c97 100644 --- a/ui-v2/app/services/repository/token.js +++ b/ui-v2/app/services/repository/token.js @@ -4,7 +4,9 @@ import { typeOf } from '@ember/utils'; import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/token'; import { Promise } from 'rsvp'; import statusFactory from 'consul-ui/utils/acls-status'; -const status = statusFactory(Promise); +import isValidServerErrorFactory from 'consul-ui/utils/http/acl/is-valid-server-error'; +const isValidServerError = isValidServerErrorFactory(); +const status = statusFactory(isValidServerError, Promise); const MODEL_NAME = 'token'; export default Service.extend({ getModelName: function() { @@ -20,10 +22,22 @@ export default Service.extend({ return status(obj); }, self: function(secret, dc) { - return get(this, 'store').self(this.getModelName(), { - secret: secret, - dc: dc, - }); + return get(this, 'store') + .self(this.getModelName(), { + secret: secret, + dc: dc, + }) + .catch(e => { + // If we get this 500 RPC error, it means we are a legacy ACL cluster + // set AccessorID to null - which for the frontend means legacy mode + if (isValidServerError(e)) { + return { + AccessorID: null, + SecretID: secret, + }; + } + return Promise.reject(e); + }); }, clone: function(item) { return get(this, 'store').clone(this.getModelName(), get(item, PRIMARY_KEY)); diff --git a/ui-v2/app/templates/dc/acls/-form.hbs b/ui-v2/app/templates/dc/acls/-form.hbs index c59129cd0f..b0ec83363c 100644 --- a/ui-v2/app/templates/dc/acls/-form.hbs +++ b/ui-v2/app/templates/dc/acls/-form.hbs @@ -14,7 +14,7 @@ {{#if create }}