diff --git a/ui/app/components/secret-edit.js b/ui/app/components/secret-edit.js index fcb816f98553..f1f1452ec5bf 100644 --- a/ui/app/components/secret-edit.js +++ b/ui/app/components/secret-edit.js @@ -5,6 +5,7 @@ import { computed, set } from '@ember/object'; import { alias, or } from '@ember/object/computed'; import { task, waitForEvent } from 'ember-concurrency'; import FocusOnInsertMixin from 'vault/mixins/focus-on-insert'; +import WithNavToNearestAncestor from 'vault/mixins/with-nav-to-nearest-ancestor'; import keys from 'vault/lib/keycodes'; import KVObject from 'vault/lib/kv-object'; import { maybeQueryRecord } from 'vault/macros/maybe-query-record'; @@ -13,7 +14,7 @@ const LIST_ROUTE = 'vault.cluster.secrets.backend.list'; const LIST_ROOT_ROUTE = 'vault.cluster.secrets.backend.list-root'; const SHOW_ROUTE = 'vault.cluster.secrets.backend.show'; -export default Component.extend(FocusOnInsertMixin, { +export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, { wizard: service(), router: service(), store: service(), @@ -163,7 +164,7 @@ export default Component.extend(FocusOnInsertMixin, { }), transitionToRoute() { - this.router.transitionTo(...arguments); + return this.router.transitionTo(...arguments); }, onEscape(e) { @@ -304,8 +305,9 @@ export default Component.extend(FocusOnInsertMixin, { }, deleteKey() { + let { id } = this.model; this.model.destroyRecord().then(() => { - this.transitionToRoute(LIST_ROOT_ROUTE); + this.navToNearestAncestor.perform(id); }); }, diff --git a/ui/app/controllers/vault/cluster/secrets/backend/list.js b/ui/app/controllers/vault/cluster/secrets/backend/list.js index 67e80153d374..4faf8b1075ef 100644 --- a/ui/app/controllers/vault/cluster/secrets/backend/list.js +++ b/ui/app/controllers/vault/cluster/secrets/backend/list.js @@ -3,8 +3,9 @@ import { inject as service } from '@ember/service'; import Controller from '@ember/controller'; import utils from 'vault/lib/key-utils'; import BackendCrumbMixin from 'vault/mixins/backend-crumb'; +import WithNavToNearestAncestor from 'vault/mixins/with-nav-to-nearest-ancestor'; -export default Controller.extend(BackendCrumbMixin, { +export default Controller.extend(BackendCrumbMixin, WithNavToNearestAncestor, { flashMessages: service(), queryParams: ['page', 'pageFilter', 'tab'], @@ -71,7 +72,7 @@ export default Controller.extend(BackendCrumbMixin, { this.get('flashMessages').success(`${name} was successfully deleted.`); this.send('reload'); if (type === 'secret') { - return this.transitionToRoute('vault.cluster.secrets.backend.list-root'); + this.navToNearestAncestor.perform(name); } }); }, diff --git a/ui/app/mixins/key-mixin.js b/ui/app/mixins/key-mixin.js index ec0c1dc035ed..443b1b8d8241 100644 --- a/ui/app/mixins/key-mixin.js +++ b/ui/app/mixins/key-mixin.js @@ -1,6 +1,6 @@ import { computed } from '@ember/object'; import Mixin from '@ember/object/mixin'; -import utils from '../lib/key-utils'; +import utils from 'vault/lib/key-utils'; export default Mixin.create({ // what attribute has the path for the key diff --git a/ui/app/mixins/with-nav-to-nearest-ancestor.js b/ui/app/mixins/with-nav-to-nearest-ancestor.js new file mode 100644 index 000000000000..ce1a38c499ed --- /dev/null +++ b/ui/app/mixins/with-nav-to-nearest-ancestor.js @@ -0,0 +1,40 @@ +import Mixin from '@ember/object/mixin'; +import utils from 'vault/lib/key-utils'; +import { task } from 'ember-concurrency'; + +// This mixin is currently used in a controller and a component, but we +// don't see cancellation of the task as the while loop runs in either + +// Controller in Ember are singletons so there's no cancellation there +// during the loop. For components, it might be expected that the task would +// be cancelled when we transitioned to a new route and a rerender occured, but this is not +// the case since we are catching the error. Since Ember's route transitions are lazy +// and we're catching any 404s, the loop continues until the transtion succeeds, or exhausts +// the ancestors array and transitions to the root +export default Mixin.create({ + navToNearestAncestor: task(function*(key) { + let ancestors = utils.ancestorKeysForKey(key); + let errored = false; + let nearest = ancestors.pop(); + while (nearest) { + try { + let transition = this.transitionToRoute('vault.cluster.secrets.backend.list', nearest); + transition.data.isDeletion = true; + yield transition.promise; + } catch (e) { + // in the route error event handler, we're only throwing when it's a 404, + // other errors will be in the route and will not be caught, so the task will complete + errored = true; + nearest = ancestors.pop(); + } finally { + if (!errored) { + nearest = null; + // eslint-disable-next-line + return; + } + errored = false; + } + } + yield this.transitionToRoute('vault.cluster.secrets.backend.list-root'); + }), +}); diff --git a/ui/app/routes/vault/cluster/secrets/backend/create-root.js b/ui/app/routes/vault/cluster/secrets/backend/create-root.js index a8aa145f87ac..33d898051290 100644 --- a/ui/app/routes/vault/cluster/secrets/backend/create-root.js +++ b/ui/app/routes/vault/cluster/secrets/backend/create-root.js @@ -6,9 +6,9 @@ let secretModel = (store, backend, key) => { let backendModel = store.peekRecord('secret-engine', backend); let modelType = backendModel.get('modelTypeForKV'); if (modelType !== 'secret-v2') { - return store.createRecord(modelType, { - id: key, - }); + let model = store.createRecord(modelType); + model.set('id', key); + return model; } let secret = store.createRecord(modelType); secret.set('engine', backendModel); diff --git a/ui/app/routes/vault/cluster/secrets/backend/list.js b/ui/app/routes/vault/cluster/secrets/backend/list.js index 2e861d4ee59b..3f1a876fd368 100644 --- a/ui/app/routes/vault/cluster/secrets/backend/list.js +++ b/ui/app/routes/vault/cluster/secrets/backend/list.js @@ -148,15 +148,22 @@ export default Route.extend({ actions: { error(error, transition) { - const { secret } = this.paramsFor(this.routeName); - const { backend } = this.paramsFor('vault.cluster.secrets.backend'); + let { secret } = this.paramsFor(this.routeName); + let { backend } = this.paramsFor('vault.cluster.secrets.backend'); + let is404 = error.httpStatus === 404; + let hasModel = this.controllerFor(this.routeName).get('hasModel'); + // this will occur if we've deleted something, + // and navigate to its parent and the parent doesn't exist - + // this if often the case with nested keys in kv-like engines + if (transition.data.isDeletion && is404) { + throw error; + } set(error, 'secret', secret); set(error, 'isRoot', true); set(error, 'backend', backend); - const hasModel = this.controllerFor(this.routeName).get('hasModel'); // only swallow the error if we have a previous model - if (hasModel && error.httpStatus === 404) { + if (hasModel && is404) { this.set('has404', true); transition.abort(); return false; diff --git a/ui/tests/acceptance/secrets/backend/kv/secret-test.js b/ui/tests/acceptance/secrets/backend/kv/secret-test.js index 9cccac069f07..3373166552fc 100644 --- a/ui/tests/acceptance/secrets/backend/kv/secret-test.js +++ b/ui/tests/acceptance/secrets/backend/kv/secret-test.js @@ -1,4 +1,4 @@ -import { currentURL, currentRouteName } from '@ember/test-helpers'; +import { settled, currentURL, currentRouteName } from '@ember/test-helpers'; import { create } from 'ember-cli-page-object'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; @@ -79,6 +79,65 @@ module('Acceptance | secrets/secret/create', function(hooks) { assert.ok(showPage.editIsPresent, 'shows the edit button'); }); + // https://github.com/hashicorp/vault/issues/5960 + test('version 1: nested paths creation maintains ability to navigate the tree', async function(assert) { + let enginePath = `kv-${new Date().getTime()}`; + let secretPath = '1/2/3/4'; + // mount version 1 engine + await mountSecrets.visit(); + await mountSecrets.selectType('kv'); + await withFlash( + mountSecrets + .next() + .path(enginePath) + .version(1) + .submit() + ); + + await listPage.create(); + await editPage.createSecret(secretPath, 'foo', 'bar'); + + // setup an ancestor for when we delete + await listPage.visitRoot({ backend: enginePath }); + await listPage.secrets.filterBy('text', '1/')[0].click(); + await listPage.create(); + await editPage.createSecret('1/2', 'foo', 'bar'); + + // lol we have to do this because ember-cli-page-object doesn't like *'s in visitable + await listPage.visitRoot({ backend: enginePath }); + await listPage.secrets.filterBy('text', '1/')[0].click(); + await listPage.secrets.filterBy('text', '2/')[0].click(); + await listPage.secrets.filterBy('text', '3/')[0].click(); + await listPage.create(); + + await editPage.createSecret(secretPath + 'a', 'foo', 'bar'); + await listPage.visitRoot({ backend: enginePath }); + await listPage.secrets.filterBy('text', '1/')[0].click(); + await listPage.secrets.filterBy('text', '2/')[0].click(); + let secretLink = listPage.secrets.filterBy('text', '3/')[0]; + assert.ok(secretLink, 'link to the 3/ branch displays properly'); + + await listPage.secrets.filterBy('text', '3/')[0].click(); + await listPage.secrets[0].menuToggle(); + await settled(); + await listPage.delete(); + await listPage.confirmDelete(); + await settled(); + assert.equal(currentRouteName(), 'vault.cluster.secrets.backend.list'); + assert.equal(currentURL(), `/vault/secrets/${enginePath}/list/1/2/3/`, 'remains on the page'); + + await listPage.secrets[0].menuToggle(); + await listPage.delete(); + await listPage.confirmDelete(); + await settled(); + assert.equal(currentRouteName(), 'vault.cluster.secrets.backend.list'); + assert.equal( + currentURL(), + `/vault/secrets/${enginePath}/list/1/`, + 'navigates to the ancestor created earlier' + ); + }); + test('it redirects to the path ending in / for list pages', async function(assert) { const path = `foo/bar/kv-path-${new Date().getTime()}`; await listPage.visitRoot({ backend: 'secret' }); diff --git a/ui/tests/pages/secrets/backend/list.js b/ui/tests/pages/secrets/backend/list.js index a436c791bf54..231b954e7348 100644 --- a/ui/tests/pages/secrets/backend/list.js +++ b/ui/tests/pages/secrets/backend/list.js @@ -1,4 +1,4 @@ -import { create, collection, visitable, clickable, isPresent } from 'ember-cli-page-object'; +import { create, collection, text, visitable, clickable, isPresent } from 'ember-cli-page-object'; import { getter } from 'ember-cli-page-object/macros'; export default create({ @@ -8,15 +8,21 @@ export default create({ createIsPresent: isPresent('[data-test-secret-create]'), configure: clickable('[data-test-secret-backend-configure]'), configureIsPresent: isPresent('[data-test-secret-backend-configure]'), - tabs: collection('[data-test-tab]'), secrets: collection('[data-test-secret-link]', { menuToggle: clickable('[data-test-popup-menu-trigger]'), + id: text(), + click: clickable(), }), menuItems: collection('.ember-basic-dropdown-content li', { testContainer: '#ember-testing', }), - + delete: clickable('[data-test-confirm-action-trigger]', { + testContainer: '#ember-testing', + }), + confirmDelete: clickable('[data-test-confirm-button]', { + testContainer: '#ember-testing', + }), backendIsEmpty: getter(function() { return this.secrets.length === 0; }),