From 7556dfd1581050efd6bc2da83ece592410223ac0 Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Thu, 7 Sep 2023 15:09:33 -0600 Subject: [PATCH] Fix KV bug on deletion_time (#22842) * the fix * working, but work in progress maybe change to another helper, put draft up? * some fixes and restructing * change names * test assert wrong locally, lets see about gh --- ui/app/models/kv/data.js | 8 +++++++- ui/app/models/kv/metadata.js | 11 +++++++++-- ui/lib/kv/addon/components/kv-version-dropdown.hbs | 2 +- ui/lib/kv/addon/components/page/secret/details.hbs | 6 +++--- ui/lib/kv/addon/components/page/secret/details.js | 9 +++++---- .../page/secret/metadata/version-history.hbs | 4 ++-- ui/lib/kv/addon/utils/kv-deleted.js | 14 ++++++++++++++ ui/lib/kv/app/utils/kv-deleted.js | 1 + .../backend/kv/kv-v2-workflow-edge-cases-test.js | 2 +- .../kv/page/kv-page-version-history-test.js | 2 +- ui/tests/unit/adapters/kv/metadata-test.js | 6 ++++-- 11 files changed, 48 insertions(+), 17 deletions(-) create mode 100644 ui/lib/kv/addon/utils/kv-deleted.js create mode 100644 ui/lib/kv/app/utils/kv-deleted.js diff --git a/ui/app/models/kv/data.js b/ui/app/models/kv/data.js index 0d076f3be7..6f4bcaf689 100644 --- a/ui/app/models/kv/data.js +++ b/ui/app/models/kv/data.js @@ -7,6 +7,7 @@ import Model, { attr } from '@ember-data/model'; import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities'; import { withModelValidations } from 'vault/decorators/model-validations'; import { withFormFields } from 'vault/decorators/model-form-fields'; +import { isDeleted } from 'kv/utils/kv-deleted'; /* sample response { @@ -71,11 +72,16 @@ export default class KvSecretDataModel extends Model { get state() { if (this.destroyed) return 'destroyed'; - if (this.deletionTime) return 'deleted'; + if (this.isSecretDeleted) return 'deleted'; if (this.createdTime) return 'created'; return ''; } + // cannot use isDeleted as model property name because of an ember property conflict + get isSecretDeleted() { + return isDeleted(this.deletionTime); + } + // Permissions @lazyCapabilities(apiPath`${'backend'}/data/${'path'}`, 'backend', 'path') dataPath; @lazyCapabilities(apiPath`${'backend'}/metadata/${'path'}`, 'backend', 'path') metadataPath; diff --git a/ui/app/models/kv/metadata.js b/ui/app/models/kv/metadata.js index 9850e29927..1f998f488a 100644 --- a/ui/app/models/kv/metadata.js +++ b/ui/app/models/kv/metadata.js @@ -8,6 +8,7 @@ import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities'; import { withModelValidations } from 'vault/decorators/model-validations'; import { withFormFields } from 'vault/decorators/model-form-fields'; import { keyIsFolder } from 'core/utils/key-utils'; +import { isDeleted } from 'kv/utils/kv-deleted'; const validations = { maxVersions: [ @@ -44,7 +45,7 @@ export default class KvSecretMetadataModel extends Model { editType: 'ttl', label: 'Automate secret deletion', helperTextDisabled: `A secret's version must be manually deleted.`, - helperTextEnabled: 'Delete all new versions of this secret after.', + helperTextEnabled: 'Delete all new versions of this secret after:', }) deleteVersionAfter; @@ -67,10 +68,16 @@ export default class KvSecretMetadataModel extends Model { return keyIsFolder(this.path); } + // cannot use isDeleted due to ember property conflict + get isSecretDeleted() { + return isDeleted(this.deletionTime); + } + // turns version object into an array for version dropdown menu get sortedVersions() { const array = []; for (const key in this.versions) { + this.versions[key].isSecretDeleted = isDeleted(this.versions[key].deletion_time); array.push({ version: key, ...this.versions[key] }); } // version keys are in order created with 1 being the oldest, we want newest first @@ -81,7 +88,7 @@ export default class KvSecretMetadataModel extends Model { get currentSecret() { if (!this.versions || !this.currentVersion) return false; const data = this.versions[this.currentVersion]; - const state = data.destroyed ? 'destroyed' : data.deletion_time ? 'deleted' : 'created'; + const state = data.destroyed ? 'destroyed' : isDeleted(data.deletion_time) ? 'deleted' : 'created'; return { state, isDeactivated: state !== 'created', diff --git a/ui/lib/kv/addon/components/kv-version-dropdown.hbs b/ui/lib/kv/addon/components/kv-version-dropdown.hbs index ef703fb952..8f006da972 100644 --- a/ui/lib/kv/addon/components/kv-version-dropdown.hbs +++ b/ui/lib/kv/addon/components/kv-version-dropdown.hbs @@ -14,7 +14,7 @@ {{versionData.version}} {{#if versionData.destroyed}} - {{else if versionData.deletion_time}} + {{else if versionData.isSecretDeleted}} {{else if (loose-equal versionData.version @metadata.currentVersion)}} diff --git a/ui/lib/kv/addon/components/page/secret/details.hbs b/ui/lib/kv/addon/components/page/secret/details.hbs index 898904a450..82dd6b656f 100644 --- a/ui/lib/kv/addon/components/page/secret/details.hbs +++ b/ui/lib/kv/addon/components/page/secret/details.hbs @@ -64,7 +64,7 @@ -{{#if (or @secret.deletionTime (not this.emptyState))}} +{{#if (or @secret.isSecretDeleted (not this.emptyState))}}
{{#unless this.hideHeaders}} @@ -76,10 +76,10 @@
{{/unless}}
- {{#if (or @secret.deletionTime @secret.createdTime)}} + {{#if (or @secret.isSecretDeleted @secret.createdTime)}} {{/if}}
diff --git a/ui/lib/kv/addon/components/page/secret/details.js b/ui/lib/kv/addon/components/page/secret/details.js index 94226209ee..844706f8db 100644 --- a/ui/lib/kv/addon/components/page/secret/details.js +++ b/ui/lib/kv/addon/components/page/secret/details.js @@ -10,6 +10,7 @@ import { next } from '@ember/runloop'; import { inject as service } from '@ember/service'; import { task } from 'ember-concurrency'; import { waitFor } from '@ember/test-waiters'; +import { isDeleted } from 'kv/utils/kv-deleted'; /** * @module KvSecretDetails renders the key/value data of a KV secret. @@ -73,7 +74,7 @@ export default class KvSecretDetails extends Component { this.refreshRoute(); } catch (err) { this.flashMessages.danger( - `There was a problem undeleting ${secret.path}. Error: ${err.errors.join(' ')}.` + `There was a problem undeleting ${secret.path}. Error: ${err.errors?.join(' ')}.` ); } } @@ -129,7 +130,7 @@ export default class KvSecretDetails extends Component { if (meta?.destroyed) { return 'destroyed'; } - if (meta?.deletion_time) { + if (isDeleted(meta?.deletion_time)) { return 'deleted'; } if (meta?.created_time) { @@ -172,7 +173,7 @@ export default class KvSecretDetails extends Component { }; } // only destructure if we can read secret data - const { version, destroyed, deletionTime } = this.args.secret; + const { version, destroyed, isSecretDeleted } = this.args.secret; if (destroyed) { return { title: `Version ${version} of this secret has been permanently destroyed`, @@ -184,7 +185,7 @@ export default class KvSecretDetails extends Component { link: '/vault/docs/secrets/kv/kv-v2', }; } - if (deletionTime) { + if (isSecretDeleted) { return { title: `Version ${version} of this secret has been deleted`, message: `This version has been deleted but can be undeleted. ${ diff --git a/ui/lib/kv/addon/components/page/secret/metadata/version-history.hbs b/ui/lib/kv/addon/components/page/secret/metadata/version-history.hbs index 03912fefa8..a8e50999db 100644 --- a/ui/lib/kv/addon/components/page/secret/metadata/version-history.hbs +++ b/ui/lib/kv/addon/components/page/secret/metadata/version-history.hbs @@ -37,7 +37,7 @@ Destroyed
- {{else if versionData.deletion_time}} + {{else if versionData.isSecretDeleted}}
@@ -75,7 +75,7 @@ @route="secret.details.edit" @query={{hash version=versionData.version}} data-test-create-new-version-from={{versionData.version}} - @disabled={{or versionData.destroyed versionData.deletion_time}} + @disabled={{or versionData.destroyed versionData.isSecretDeleted}} > Create new version from {{versionData.version}} diff --git a/ui/lib/kv/addon/utils/kv-deleted.js b/ui/lib/kv/addon/utils/kv-deleted.js new file mode 100644 index 0000000000..5ce0e3757f --- /dev/null +++ b/ui/lib/kv/addon/utils/kv-deleted.js @@ -0,0 +1,14 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: MPL-2.0 + */ +import timestamp from 'core/utils/timestamp'; + +export function isDeleted(date) { + // on the kv/data model, deletion_time does not always mean the secret has been deleted. + // if the delete_version_after is set then the deletion_time will be UTC of that time, even if it's a future time from now. + // to determine if the secret is deleted we check if deletion_time <= time right now. + const deletionTime = new Date(date); + const now = timestamp.now(); + return deletionTime <= now; +} diff --git a/ui/lib/kv/app/utils/kv-deleted.js b/ui/lib/kv/app/utils/kv-deleted.js new file mode 100644 index 0000000000..4220c305f9 --- /dev/null +++ b/ui/lib/kv/app/utils/kv-deleted.js @@ -0,0 +1 @@ +export { default } from 'kv/utils/kv-deleted'; diff --git a/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js b/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js index bb7d9c2c2d..f44e27b5fe 100644 --- a/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js +++ b/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js @@ -366,7 +366,7 @@ module('Acceptance | Enterprise | kv-v2 workflow | edge cases', function (hooks) }); test('namespace: it manages state throughout delete, destroy and undelete operations', async function (assert) { - assert.expect(32); + assert.expect(34); const backend = this.backend; const ns = this.namespace; const secret = 'my-delete-secret'; diff --git a/ui/tests/integration/components/kv/page/kv-page-version-history-test.js b/ui/tests/integration/components/kv/page/kv-page-version-history-test.js index d11dc61e02..76baf7d1d5 100644 --- a/ui/tests/integration/components/kv/page/kv-page-version-history-test.js +++ b/ui/tests/integration/components/kv/page/kv-page-version-history-test.js @@ -66,7 +66,7 @@ module('Integration | Component | kv | Page::Secret::Metadata::Version-History', .dom(`${PAGE.versions.icon(version)} [data-test-icon="x-square-fill"]`) .hasStyle({ color: 'rgb(199, 52, 69)' }); } - if (data.deletion_time) { + if (data.isSecretDeleted) { assert .dom(`${PAGE.versions.icon(version)} [data-test-icon="x-square-fill"]`) .hasStyle({ color: 'rgb(111, 118, 130)' }); diff --git a/ui/tests/unit/adapters/kv/metadata-test.js b/ui/tests/unit/adapters/kv/metadata-test.js index 075d99cccb..b2bf994805 100644 --- a/ui/tests/unit/adapters/kv/metadata-test.js +++ b/ui/tests/unit/adapters/kv/metadata-test.js @@ -9,6 +9,8 @@ import { setupMirage } from 'ember-cli-mirage/test-support'; import { kvMetadataPath } from 'vault/utils/kv-path'; import { Response } from 'miragejs'; +const UTC_DATE = '1994-11-05T00:00:00.000Z'; + const EXAMPLE_KV_METADATA_GET_RESPONSE = { request_id: 'foobar', data: { @@ -23,7 +25,7 @@ const EXAMPLE_KV_METADATA_GET_RESPONSE = { versions: { 1: { created_time: 'created-time', - deletion_time: 'deletion-time', + deletion_time: UTC_DATE, destroyed: false, }, 2: { created_time: 'created-time', deletion_time: '', destroyed: false }, @@ -75,7 +77,7 @@ module('Unit | Adapter | kv/metadata', function (hooks) { versions: { 1: { created_time: 'created-time', - deletion_time: 'deletion-time', + deletion_time: UTC_DATE, destroyed: false, }, 2: {