From 8f5d62139c7de5617c41c51f9c5eeb8583cfef12 Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Wed, 19 May 2021 10:43:55 -0600 Subject: [PATCH] KV 2 Toolbar delete redesign (#11530) * initial setup, modify toolbar header * footer buttons setup * setup first delete version delete method * clean up * handle destory all versions * handle undelete * conditional for modal and undelete * remove delete from version area * modelForData in permissions * setup for soft delete and modify adpater to allow DELETE in additon to POST * dropdown for soft delete * stuck * handle all soft deletes * conditional for destroy all versions * remove old functionality from secret-version-menu * glimmerize secret-version-menu * Updated secret version menu and version history * Updated icons and columns in version history * create new component * clean up * glimmerize secret delete menu * fix undelete * Fixed radio labels in version delete menu * handle v1 delete * refining * handle errors with flash messages * add changelog * fix test * add to test * amend test * address PR comments * whoopies * add urlEncoding Co-authored-by: Arnav Palnitkar --- changelog/11530.txt | 3 + ui/app/adapters/secret-v2-version.js | 29 +++- ui/app/components/secret-delete-menu.js | 148 ++++++++++++++++++ ui/app/components/secret-edit.js | 2 +- ui/app/components/secret-version-menu.js | 63 +------- ui/app/styles/components/masked-input.scss | 1 - ui/app/styles/components/modal.scss | 14 ++ .../components/secret-delete-menu.hbs | 131 ++++++++++++++++ ui/app/templates/components/secret-edit.hbs | 87 +++------- .../components/secret-version-menu.hbs | 93 ++++------- .../cluster/secrets/backend/versions.hbs | 110 ++++++++----- .../secrets/backend/kv/secret-test.js | 76 ++++++++- ui/tests/pages/secrets/backend/kv/show.js | 8 + 13 files changed, 520 insertions(+), 245 deletions(-) create mode 100644 changelog/11530.txt create mode 100644 ui/app/components/secret-delete-menu.js create mode 100644 ui/app/templates/components/secret-delete-menu.hbs diff --git a/changelog/11530.txt b/changelog/11530.txt new file mode 100644 index 000000000..95bb8be7c --- /dev/null +++ b/changelog/11530.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: Redesign of KV 2 Delete toolbar. +``` \ No newline at end of file diff --git a/ui/app/adapters/secret-v2-version.js b/ui/app/adapters/secret-v2-version.js index 5aac49d73..5bdd02129 100644 --- a/ui/app/adapters/secret-v2-version.js +++ b/ui/app/adapters/secret-v2-version.js @@ -70,14 +70,27 @@ export default ApplicationAdapter.extend({ v2DeleteOperation(store, id, deleteType = 'delete') { let [backend, path, version] = JSON.parse(id); - - // deleteType should be 'delete', 'destroy', 'undelete' - return this.ajax(this._url(backend, path, deleteType), 'POST', { data: { versions: [version] } }).then( - () => { - let model = store.peekRecord('secret-v2-version', id); - return model && model.rollbackAttributes() && model.reload(); - } - ); + // deleteType should be 'delete', 'destroy', 'undelete', 'delete-latest-version', 'destroy-version' + if ((!version && deleteType === 'delete') || deleteType === 'delete-latest-version') { + return this.ajax(this._url(backend, path, 'data'), 'DELETE') + .then(() => { + let model = store.peekRecord('secret-v2-version', id); + return model && model.rollbackAttributes() && model.reload(); + }) + .catch(e => { + return e; + }); + } else { + return this.ajax(this._url(backend, path, deleteType), 'POST', { data: { versions: [version] } }) + .then(() => { + let model = store.peekRecord('secret-v2-version', id); + // potential that model.reload() is never called. + return model && model.rollbackAttributes() && model.reload(); + }) + .catch(e => { + return e; + }); + } }, handleResponse(status, headers, payload, requestData) { diff --git a/ui/app/components/secret-delete-menu.js b/ui/app/components/secret-delete-menu.js new file mode 100644 index 000000000..e5feb32e7 --- /dev/null +++ b/ui/app/components/secret-delete-menu.js @@ -0,0 +1,148 @@ +import Ember from 'ember'; +import { inject as service } from '@ember/service'; +import Component from '@glimmer/component'; +import { tracked } from '@glimmer/tracking'; +import { action } from '@ember/object'; +import { alias } from '@ember/object/computed'; +import { maybeQueryRecord } from 'vault/macros/maybe-query-record'; + +const getErrorMessage = errors => { + let errorMessage = errors?.join('. ') || 'Something went wrong. Check the Vault logs for more information.'; + return errorMessage; +}; +export default class SecretDeleteMenu extends Component { + @service store; + @service router; + @service flashMessages; + + @tracked showDeleteModal = false; + + @maybeQueryRecord( + 'capabilities', + context => { + if (!context.args.model) { + return; + } + let backend = context.args.model.backend; + let id = context.args.model.id; + let path = context.args.isV2 + ? `${encodeURIComponent(backend)}/data/${encodeURIComponent(id)}` + : `${encodeURIComponent(backend)}/${encodeURIComponent(id)}`; + return { + id: path, + }; + }, + 'isV2', + 'model', + 'model.id', + 'mode' + ) + updatePath; + @alias('updatePath.canDelete') canDelete; + @alias('updatePath.canUpdate') canUpdate; + + @maybeQueryRecord( + 'capabilities', + context => { + if (!context.args || !context.args.modelForData || !context.args.modelForData.id) return; + let [backend, id] = JSON.parse(context.args.modelForData.id); + return { + id: `${encodeURIComponent(backend)}/delete/${encodeURIComponent(id)}`, + }; + }, + 'model.id' + ) + deleteVersionPath; + @alias('deleteVersionPath.canUpdate') canDeleteAnyVersion; + + @maybeQueryRecord( + 'capabilities', + context => { + if (!context.args || !context.args.modelForData || !context.args.modelForData.id) return; + let [backend, id] = JSON.parse(context.args.modelForData.id); + return { + id: `${encodeURIComponent(backend)}/undelete/${encodeURIComponent(id)}`, + }; + }, + 'model.id' + ) + undeleteVersionPath; + @alias('undeleteVersionPath.canUpdate') canUndeleteVersion; + + @maybeQueryRecord( + 'capabilities', + context => { + if (!context.args || !context.args.modelForData || !context.args.modelForData.id) return; + let [backend, id] = JSON.parse(context.args.modelForData.id); + return { + id: `${encodeURIComponent(backend)}/destroy/${encodeURIComponent(id)}`, + }; + }, + 'model.id' + ) + destroyVersionPath; + @alias('destroyVersionPath.canUpdate') canDestroyVersion; + + @maybeQueryRecord( + 'capabilities', + context => { + if (!context.args.model || !context.args.model.engine || !context.args.model.id) return; + let backend = context.args.model.engine.id; + let id = context.args.model.id; + return { + id: `${encodeURIComponent(backend)}/metadata/${encodeURIComponent(id)}`, + }; + }, + 'model', + 'model.id', + 'mode' + ) + v2UpdatePath; + @alias('v2UpdatePath.canDelete') canDestroyAllVersions; + + get isLatestVersion() { + let { model } = this.args; + if (!model) return false; + let latestVersion = model.currentVersion; + let selectedVersion = model.selectedVersion.version; + if (latestVersion !== selectedVersion) { + return false; + } + return true; + } + + @action + handleDelete(deleteType) { + // deleteType should be 'delete', 'destroy', 'undelete', 'delete-latest-version', 'destroy-all-versions' + if (!deleteType) { + return; + } + if (deleteType === 'destroy-all-versions' || deleteType === 'v1') { + let { id } = this.args.model; + this.args.model.destroyRecord().then(() => { + this.args.navToNearestAncestor.perform(id); + }); + } else { + return this.store + .adapterFor('secret-v2-version') + .v2DeleteOperation(this.store, this.args.modelForData.id, deleteType) + .then(resp => { + if (Ember.testing) { + return; + } + if (!resp) { + this.showDeleteModal = false; + this.args.refresh(); + return; + } + if (resp.isAdapterError) { + const errorMessage = getErrorMessage(resp.errors); + this.flashMessages.danger(errorMessage); + } else { + // not likely to ever get to this situation, but adding just in case. + location.reload(); + } + }); + } + } +} diff --git a/ui/app/components/secret-edit.js b/ui/app/components/secret-edit.js index 4adbc148b..3c049b610 100644 --- a/ui/app/components/secret-edit.js +++ b/ui/app/components/secret-edit.js @@ -112,7 +112,7 @@ export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, { 'model.id', 'mode' ), - canDelete: alias('model.canDelete'), + canDelete: alias('updatePath.canDelete'), canEdit: alias('updatePath.canUpdate'), v2UpdatePath: maybeQueryRecord( diff --git a/ui/app/components/secret-version-menu.js b/ui/app/components/secret-version-menu.js index f698870d0..b4aa0db98 100644 --- a/ui/app/components/secret-version-menu.js +++ b/ui/app/components/secret-version-menu.js @@ -1,60 +1,5 @@ -import { maybeQueryRecord } from 'vault/macros/maybe-query-record'; -import Component from '@ember/component'; -import { inject as service } from '@ember/service'; -import { alias, or } from '@ember/object/computed'; +import Component from '@glimmer/component'; -export default Component.extend({ - tagName: '', - store: service(), - version: null, - useDefaultTrigger: false, - onRefresh() {}, - - deleteVersionPath: maybeQueryRecord( - 'capabilities', - context => { - let [backend, id] = JSON.parse(context.version.id); - return { - id: `${backend}/delete/${id}`, - }; - }, - 'version.id' - ), - canDeleteVersion: alias('deleteVersionPath.canUpdate'), - destroyVersionPath: maybeQueryRecord( - 'capabilities', - context => { - let [backend, id] = JSON.parse(context.version.id); - return { - id: `${backend}/destroy/${id}`, - }; - }, - 'version.id' - ), - canDestroyVersion: alias('destroyVersionPath.canUpdate'), - undeleteVersionPath: maybeQueryRecord( - 'capabilities', - context => { - let [backend, id] = JSON.parse(context.version.id); - return { - id: `${backend}/undelete/${id}`, - }; - }, - 'version.id' - ), - canUndeleteVersion: alias('undeleteVersionPath.canUpdate'), - - isFetchingVersionCapabilities: or( - 'deleteVersionPath.isPending', - 'destroyVersionPath.isPending', - 'undeleteVersionPath.isPending' - ), - actions: { - deleteVersion(deleteType = 'destroy') { - return this.store - .adapterFor('secret-v2-version') - .v2DeleteOperation(this.store, this.version.id, deleteType) - .then(this.onRefresh); - }, - }, -}); +export default class SecretVersionMenu extends Component { + onRefresh() {} +} diff --git a/ui/app/styles/components/masked-input.scss b/ui/app/styles/components/masked-input.scss index 65f03e0cb..32f6c5aac 100644 --- a/ui/app/styles/components/masked-input.scss +++ b/ui/app/styles/components/masked-input.scss @@ -66,7 +66,6 @@ height: auto; line-height: 1rem; min-width: $spacing-l; - z-index: 100; border: 0; box-shadow: none; color: $grey-light; diff --git a/ui/app/styles/components/modal.scss b/ui/app/styles/components/modal.scss index fe6292657..9378ced2f 100644 --- a/ui/app/styles/components/modal.scss +++ b/ui/app/styles/components/modal.scss @@ -76,3 +76,17 @@ pre { background: #f7f8fa; border-top: 1px solid #bac1cc; } + +.modal-radio-button { + display: flex; + align-items: baseline; + margin-bottom: $spacing-xs; + + input { + top: 2px; + } + + .helper-text { + margin-left: 10px; + } +} diff --git a/ui/app/templates/components/secret-delete-menu.hbs b/ui/app/templates/components/secret-delete-menu.hbs new file mode 100644 index 000000000..614f50e2e --- /dev/null +++ b/ui/app/templates/components/secret-delete-menu.hbs @@ -0,0 +1,131 @@ +{{#unless @isV2}} + {{#if this.canDelete}} + + Delete + + {{/if}} +{{else}} + {{#if (and this.canUndeleteVersion @modelForData.deleted)}} + + {{/if}} + {{#if (and (not @modelForData.deleted) (not @modelForData.destroyed)) }} + {{#if (or this.canDestroyVersion this.canDestroyAllVersions)}} + +
+ {{else}} + {{#if (or this.canDeleteAnyVersion (and this.isLatestVersion this.canDelete))}} + + Delete + + {{/if}} + {{/if}} + {{/if}} + + +
+

There are three ways to delete or destroy the {{@model.id}} secret. Each is different, so be sure to read the below carefully.

+

How would you like to proceed?

+ {{#unless @modelForData.destroyed}} + {{#unless @modelForData.deleted}} + {{#if this.canDelete}} +
+ +
+ +

This deletes Version {{@modelForData.version}} of the secret. It can be un-deleted later.

+
+
+ {{/if}} + {{/unless}} + {{#if this.canDestroyVersion}} +
+ +
+ +

Version {{@modelForData.version}} is permanently destroyed and cannot be read or recovered later.

+
+
+ {{/if}} + {{/unless}} + {{#if this.canDestroyAllVersions}} +
+ +
+ +

All secret versions and metadata are permanently destroyed and cannot be read or recovered later.

+
+
+ {{/if}} +
+
+ + +
+ +{{/unless}} diff --git a/ui/app/templates/components/secret-edit.hbs b/ui/app/templates/components/secret-edit.hbs index cefe55f43..4be44a5e2 100644 --- a/ui/app/templates/components/secret-edit.hbs +++ b/ui/app/templates/components/secret-edit.hbs @@ -33,74 +33,15 @@ {{/unless}} - {{#if (and (eq @mode "show") this.isV2 (not @model.failedServerRead))}} - - - - History - - - - - -
{{/if}} - - {{#if (and (eq mode 'show') canDelete)}} - - Delete secret - - {{/if}} - {{#if (and (eq mode 'show') (or canEditV2Secret canEdit))}} {{#let (concat 'vault.cluster.secrets.backend.' (if (eq mode 'show') 'edit' 'show')) as |targetRoute|}} {{#unless (and isV2 (or isWriteWithoutRead modelForData.destroyed modelForData.deleted))}} @@ -116,7 +57,7 @@ @class={{concat "toolbar-link" (if D.isOpen " is-active")}} @tagName="button" > - Copy secret + Copy @@ -159,7 +100,19 @@ {{/unless}} + {{/let}} + {{/if}} + {{#if (and (eq @mode "show") this.isV2 (not @model.failedServerRead))}} + + {{/if}} + + {{#if (and (eq mode 'show') (or canEditV2Secret canEdit))}} + {{#let (concat 'vault.cluster.secrets.backend.' (if (eq mode 'show') 'edit' 'show')) as |targetRoute|}} {{#if isV2}} - {{#if useDefaultTrigger}} - - {{else}} - Version {{this.version.version}} - - {{/if}} + Version {{@version.version}} + diff --git a/ui/app/templates/vault/cluster/secrets/backend/versions.hbs b/ui/app/templates/vault/cluster/secrets/backend/versions.hbs index e245844a0..9281ce6ce 100644 --- a/ui/app/templates/vault/cluster/secrets/backend/versions.hbs +++ b/ui/app/templates/vault/cluster/secrets/backend/versions.hbs @@ -1,11 +1,16 @@ - + model=model.engineId }} + @showCurrent={{true}} + />

@@ -17,52 +22,75 @@
-
- Version {{list.item.version}} - {{#if (eq list.item.version model.currentVersion)}} +
+ + Version {{list.item.version}} +
+ {{#if (eq list.item.version model.currentVersion)}} +
- Current + Current - {{/if}} -
-
- {{#if list.item.deleted}} +
+ {{/if}} + {{#if list.item.deleted}} +
- Deleted + Deleted - {{/if}} - {{#if list.item.destroyed}} +
+ {{/if}} + {{#if list.item.destroyed}} +
- Destroyed - - {{/if}} -
+ Destroyed + +
+ {{/if}}
- -
  • - - View version {{list.item.version}} - -
    • -
  • - - Create new version from {{list.item.version}} - -
    • -     + + + + + + + +     diff --git a/ui/tests/acceptance/secrets/backend/kv/secret-test.js b/ui/tests/acceptance/secrets/backend/kv/secret-test.js index 9fdde5a01..a923d8e9d 100644 --- a/ui/tests/acceptance/secrets/backend/kv/secret-test.js +++ b/ui/tests/acceptance/secrets/backend/kv/secret-test.js @@ -144,7 +144,7 @@ module('Acceptance | secrets/secret/create', function(hooks) { .submit(); await listPage.create(); await editPage.createSecret(secretPath, 'foo', 'bar'); - await showPage.deleteSecret(); + await showPage.deleteSecretV1(); assert.equal( currentRouteName(), 'vault.cluster.secrets.backend.list-root', @@ -251,6 +251,78 @@ module('Acceptance | secrets/secret/create', function(hooks) { assert.ok(showPage.editIsPresent, 'shows the edit button'); }); + test('version 2 with policy with destroy capabilities shows modal', async function(assert) { + let backend = 'kv-v2'; + const V2_POLICY = ` + path "kv-v2/destroy/*" { + capabilities = ["update"] + } + path "kv-v2/metadata/*" { + capabilities = ["list", "update", "delete"] + } + path "kv-v2/data/secret" { + capabilities = ["create", "read", "update"] + } + `; + await consoleComponent.runCommands([ + `write sys/mounts/${backend} type=kv options=version=2`, + `write sys/policies/acl/kv-v2-degrade policy=${btoa(V2_POLICY)}`, + // delete any kv previously written here so that tests can be re-run + 'delete kv-v2/metadata/secret', + 'write -field=client_token auth/token/create policies=kv-v2-degrade', + ]); + + let userToken = consoleComponent.lastLogOutput; + await logout.visit(); + await authPage.login(userToken); + + await writeSecret(backend, 'secret', 'foo', 'bar'); + await click('[data-test-delete-open-modal]'); + await settled(); + assert.dom('[data-test-delete-modal="destroy-version"]').exists('destroy this version option shows'); + assert.dom('[data-test-delete-modal="destroy-all-versions"]').exists('destroy all versions option shows'); + assert.dom('[data-test-delete-modal="delete-version"]').doesNotExist('delete version does not show'); + }); + + test('version 2 with policy with only delete option does not show modal and undelete is an option', async function(assert) { + let backend = 'kv-v2'; + const V2_POLICY = ` + path "kv-v2/delete/*" { + capabilities = ["update"] + } + path "kv-v2/undelete/*" { + capabilities = ["update"] + } + path "kv-v2/metadata/*" { + capabilities = ["list","read","create","update"] + } + path "kv-v2/data/secret" { + capabilities = ["create", "read"] + } + `; + await consoleComponent.runCommands([ + `write sys/mounts/${backend} type=kv options=version=2`, + `write sys/policies/acl/kv-v2-degrade policy=${btoa(V2_POLICY)}`, + // delete any kv previously written here so that tests can be re-run + 'delete kv-v2/metadata/secret', + 'write -field=client_token auth/token/create policies=kv-v2-degrade', + ]); + + let userToken = consoleComponent.lastLogOutput; + await logout.visit(); + await authPage.login(userToken); + await writeSecret(backend, 'secret', 'foo', 'bar'); + assert.dom('[data-test-delete-open-modal]').doesNotExist('delete version does not show'); + assert.dom('[data-test-secret-v2-delete="true"]').exists('drop down delete shows'); + await showPage.deleteSecretV2(); + // unable to reload page in test scenario so going to list and back to secret to confirm deletion + let url = `/vault/secrets/${backend}/list`; + await visit(url); + await click('[data-test-secret-link="secret"]'); + assert.dom('[data-test-component="empty-state"]').exists('secret has been deleted'); + assert.dom('[data-test-secret-undelete]').exists('undelete button shows'); + }); + test('paths are properly encoded', async function(assert) { let backend = 'kv'; let paths = [ @@ -305,7 +377,7 @@ module('Acceptance | secrets/secret/create', function(hooks) { await listPage.create(); await editPage.createSecret(secretPath, 'foo', 'bar'); await settled(); - await click('[data-test-popup-menu-trigger="history"]'); + await click('[data-test-popup-menu-trigger="version"]'); await settled(); await click('[data-test-version-history]'); await settled(); diff --git a/ui/tests/pages/secrets/backend/kv/show.js b/ui/tests/pages/secrets/backend/kv/show.js index 7d1071a94..49caf8a50 100644 --- a/ui/tests/pages/secrets/backend/kv/show.js +++ b/ui/tests/pages/secrets/backend/kv/show.js @@ -8,6 +8,8 @@ export default create({ text: text(), }), deleteBtn: clickable('[data-test-secret-delete] button'), + deleteBtnV1: clickable('[data-test-secret-v1-delete="true"] button'), + deleteBtnV2: clickable('[data-test-secret-v2-delete="true"] button'), confirmBtn: clickable('[data-test-confirm-button]'), rows: collection('data-test-row-label'), toggleJSON: clickable('[data-test-secret-json-toggle]'), @@ -22,4 +24,10 @@ export default create({ deleteSecret() { return this.deleteBtn().confirmBtn(); }, + deleteSecretV1() { + return this.deleteBtnV1().confirmBtn(); + }, + deleteSecretV2() { + return this.deleteBtnV2().confirmBtn(); + }, });