Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Update behavior when deleting nested secret from list #26845

Merged
merged 7 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/26845.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:change
ui: deleting a nested secret will no longer redirect you to the nearest path segment
```
8 changes: 2 additions & 6 deletions ui/app/controllers/vault/cluster/secrets/backend/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ import { computed } from '@ember/object';
import { service } from '@ember/service';
import Controller from '@ember/controller';
import BackendCrumbMixin from 'vault/mixins/backend-crumb';
import WithNavToNearestAncestor from 'vault/mixins/with-nav-to-nearest-ancestor';
import ListController from 'core/mixins/list-controller';
import { keyIsFolder } from 'core/utils/key-utils';

export default Controller.extend(ListController, BackendCrumbMixin, WithNavToNearestAncestor, {
export default Controller.extend(ListController, BackendCrumbMixin, {
flashMessages: service(),
queryParams: ['page', 'pageFilter', 'tab'],

Expand Down Expand Up @@ -52,16 +51,13 @@ export default Controller.extend(ListController, BackendCrumbMixin, WithNavToNea
});
},

delete(item, type) {
delete(item) {
const name = item.id;
item
.destroyRecord()
.then(() => {
this.flashMessages.success(`${name} was successfully deleted.`);
this.send('reload');
if (type === 'secret') {
this.navToNearestAncestor.perform(name);
}
})
.catch((e) => {
const error = e.errors ? e.errors.join('. ') : e.message;
Expand Down
45 changes: 0 additions & 45 deletions ui/app/mixins/with-nav-to-nearest-ancestor.js

This file was deleted.

25 changes: 16 additions & 9 deletions ui/app/routes/vault/cluster/secrets/backend/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@ import { pathIsDirectory } from 'kv/utils/kv-breadcrumbs';

const SUPPORTED_BACKENDS = supportedSecretBackends();

function getValidPage(pageParam) {
if (typeof pageParam === 'number') {
return pageParam;
}
if (typeof pageParam === 'string') {
try {
return parseInt(pageParam, 10) || 1;
} catch (e) {
return 1;
}
}
return 1;
}

export default Route.extend({
store: service(),
templateName: 'vault/cluster/secrets/backend/list',
Expand Down Expand Up @@ -119,16 +133,15 @@ export default Route.extend({
id: secret,
backend,
responsePath: 'data.keys',
page: params.page || 1,
page: getValidPage(params.page),
pageFilter: params.pageFilter,
})
.then((model) => {
this.set('has404', false);
return model;
})
.catch((err) => {
// if we're at the root we don't want to throw
if (backendModel && err.httpStatus === 404 && secret === '') {
if (backendModel && err.httpStatus === 404) {
return [];
} else {
// else we're throwing and dealing with this in the error action
Expand Down Expand Up @@ -189,12 +202,6 @@ export default Route.extend({
/* eslint-disable-next-line ember/no-controller-access-in-routes */
const hasModel = this.controllerFor(this.routeName).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);
Expand Down
119 changes: 61 additions & 58 deletions ui/app/templates/vault/cluster/secrets/backend/list.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -13,39 +13,37 @@
{{#let (options-for-backend this.backendType this.tab) as |options|}}
{{#if (or this.model.meta.total (not this.isConfigurableTab))}}
<Toolbar>
{{#if this.model.meta.total}}
<ToolbarFilters>
<NavigateInput
@enterpriseProduct="vault"
@filterFocusDidChange={{action "setFilterFocus"}}
@filterDidChange={{action "setFilter"}}
@filter={{this.filter}}
@filterMatchesKey={{this.filterMatchesKey}}
@firstPartialMatch={{this.firstPartialMatch}}
@baseKey={{get this.baseKey "id"}}
@shouldNavigateTree={{options.navigateTree}}
@placeholder={{options.searchPlaceholder}}
@mode={{if (eq this.tab "cert") "secrets-cert" "secrets"}}
/>
{{#if this.filterFocused}}
{{#if this.filterMatchesKey}}
{{#unless this.filterIsFolder}}
<p class="input-hint">
<kbd>Enter</kbd>
to view
{{this.filter}}
</p>
{{/unless}}
{{/if}}
{{#if this.firstPartialMatch}}
<ToolbarFilters>
<NavigateInput
@enterpriseProduct="vault"
@filterFocusDidChange={{action "setFilterFocus"}}
@filterDidChange={{action "setFilter"}}
@filter={{this.filter}}
@filterMatchesKey={{this.filterMatchesKey}}
@firstPartialMatch={{this.firstPartialMatch}}
@baseKey={{get this.baseKey "id"}}
@shouldNavigateTree={{options.navigateTree}}
@placeholder={{options.searchPlaceholder}}
@mode={{if (eq this.tab "cert") "secrets-cert" "secrets"}}
/>
{{#if this.filterFocused}}
{{#if this.filterMatchesKey}}
{{#unless this.filterIsFolder}}
<p class="input-hint">
<kbd>Tab</kbd>
to autocomplete
<kbd>Enter</kbd>
to view
{{this.filter}}
</p>
{{/if}}
{{/unless}}
{{/if}}
</ToolbarFilters>
{{/if}}
{{#if this.firstPartialMatch}}
<p class="input-hint">
<kbd>Tab</kbd>
to autocomplete
</p>
{{/if}}
{{/if}}
</ToolbarFilters>

<ToolbarActions>
<ToolbarSecretLink
Expand Down Expand Up @@ -80,33 +78,28 @@
/>
{{/let}}
{{else}}
<div class="box is-sideless">
{{#if this.filterFocused}}
There are no
{{pluralize options.item}}
matching
<code>{{this.filter}}</code>, press
<kbd>ENTER</kbd>
to add one.
{{else}}
There are no
{{pluralize options.item}}
matching
<code>{{this.filter}}</code>.
{{/if}}
</div>
{{/each}}
{{! Empty state here means that a filter was applied on top of the results list }}
<EmptyState
@title='There are no {{pluralize options.item}} matching "{{this.filter}}". {{if
this.filterFocused
'Press ENTER to add one.'
}}'
/>

<Hds::Pagination::Numbered
@currentPage={{this.model.meta.currentPage}}
@currentPageSize={{this.model.meta.pageSize}}
@route={{concat "vault.cluster.secrets.backend.list" (unless this.baseKey.id "-root")}}
@model={{this.backendModel.id}}
@showSizeSelector={{false}}
@totalItems={{this.model.meta.total}}
@queryFunction={{this.paginationQueryParams}}
/>
{{/each}}

{{#if this.model.length}}
{{! Only show pagination when there are results on the page }}
<Hds::Pagination::Numbered
@currentPage={{this.model.meta.currentPage}}
@currentPageSize={{this.model.meta.pageSize}}
@route={{concat "vault.cluster.secrets.backend.list" (unless this.baseKey.id "-root")}}
@model={{this.backendModel.id}}
@showSizeSelector={{false}}
@totalItems={{this.model.meta.total}}
@queryFunction={{this.paginationQueryParams}}
/>
{{/if}}
{{else}}
{{#if (eq this.baseKey.id "")}}
{{#if (and options.firstStep (not this.tab))}}
Expand All @@ -124,10 +117,20 @@
<EmptyState
@title={{if
(eq this.filter this.baseKey.id)
(concat "No " (pluralize options.item) " under &quot;" this.filter "&quot;")
(concat "No folders matching &quot;" this.filter "&quot;")
(concat "No " (pluralize options.item) ' under "' this.filter '".')
(concat 'No folders matching "' this.filter '".')
}}
/>
>
<LinkTo @route="vault.cluster.secrets.backend.list-root" @model={{this.backend}} data-test-list-root-link>
Back to root
</LinkTo>
</EmptyState>
{{else}}
<EmptyState @title={{(concat "No " (pluralize options.item) ' matching "' this.filter '".')}}>
<LinkTo @route="vault.cluster.secrets.backend.list-root" @model={{this.backend}} data-test-list-root-link>
Back to root
</LinkTo>
</EmptyState>
{{/if}}
{{/if}}
{{/if}}
Expand Down
61 changes: 31 additions & 30 deletions ui/tests/acceptance/secrets/backend/kv/secret-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { writeSecret, writeVersionedSecret } from 'vault/tests/helpers/kv/kv-run
import { runCmd } from 'vault/tests/helpers/commands';
import { PAGE } from 'vault/tests/helpers/kv/kv-selectors';
import codemirror from 'vault/tests/helpers/codemirror';
import { GENERAL } from 'vault/tests/helpers/general-selectors';

const deleteEngine = async function (enginePath, assert) {
await logout.visit();
Expand Down Expand Up @@ -157,31 +158,28 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
// https://github.com/hashicorp/vault/issues/5960
test('version 1: nested paths creation maintains ability to navigate the tree', async function (assert) {
const enginePath = this.backend;
const secretPath = '1/2/3/4';
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();
const secretLink = listPage.secrets.filterBy('text', '3/')[0];
assert.ok(secretLink, 'link to the 3/ branch displays properly');
await runCmd([
`write ${enginePath}/1/2 foo=bar`,
`write ${enginePath}/1/2/3/4 foo=bar`,
`write ${enginePath}/1/2/3/4a foo=bar`,
'refresh',
]);
await settled();
// navigate to farthest leaf
await visit(`/vault/secrets/${enginePath}/list`);
assert.dom('[data-test-component="navigate-input"]').hasNoValue();
assert.dom('[data-test-secret-link]').exists({ count: 1 });
await click('[data-test-secret-link="1/"]');
assert.dom('[data-test-component="navigate-input"]').hasValue('1/');
assert.dom('[data-test-secret-link]').exists({ count: 2 });
await click('[data-test-secret-link="1/2/"]');
assert.dom('[data-test-component="navigate-input"]').hasValue('1/2/');
assert.dom('[data-test-secret-link]').exists({ count: 1 });
await click('[data-test-secret-link="1/2/3/"]');
assert.dom('[data-test-component="navigate-input"]').hasValue('1/2/3/');
assert.dom('[data-test-secret-link]').exists({ count: 2 });

await listPage.secrets.filterBy('text', '3/')[0].click();
// delete the items
await listPage.secrets.objectAt(0).menuToggle();
await settled();
await listPage.delete();
Expand All @@ -190,17 +188,20 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
assert.strictEqual(currentRouteName(), 'vault.cluster.secrets.backend.list');
assert.strictEqual(currentURL(), `/vault/secrets/${enginePath}/list/1/2/3/`, 'remains on the page');

assert.dom('[data-test-secret-link]').exists({ count: 1 });
await listPage.secrets.objectAt(0).menuToggle();
await listPage.delete();
await listPage.confirmDelete();
await settled();
assert.strictEqual(currentRouteName(), 'vault.cluster.secrets.backend.list');
assert.strictEqual(
currentURL(),
`/vault/secrets/${enginePath}/list/1/`,
'navigates to the ancestor created earlier'
);
assert.strictEqual(currentURL(), `/vault/secrets/${enginePath}/list/1/2/3/`, 'remains on the page');
assert.dom(GENERAL.emptyStateTitle).hasText('No secrets under "1/2/3/".');
await fillIn('[data-test-component="navigate-input"]', '1/2/');
assert.dom(GENERAL.emptyStateTitle).hasText('No secrets under "1/2/".');
await click('[data-test-list-root-link]');
assert.strictEqual(currentURL(), `/vault/secrets/${enginePath}/list`);
assert.dom('[data-test-secret-link]').exists({ count: 1 });
});

test('first level secrets redirect properly upon deletion', async function (assert) {
const secretPath = 'test';
await listPage.create();
Expand Down
Loading