From d99742c6c5139feccae902c1da0a6f2afa70d7da Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Tue, 15 Jun 2021 09:21:54 -0600 Subject: [PATCH] Implement ember-cp-validations on KV secret engine (#11785) * initial setup * initial validation setup for empty path object. * removal console logs * validation on keyup for kv * in progress * making some progress * more progress * closer * done with create page now to fix edit page that I broke * fix secret edit display on create * test and final touches * cleanup mountbackendform * cleanup * add changelog * address pr comments * address styling pr comment --- changelog/11785.txt | 3 + ui/app/components/mount-backend-form.js | 12 +- ui/app/components/secret-edit.js | 50 ++++- ui/app/models/secret-engine.js | 10 +- ui/app/models/secret-v2.js | 18 +- ui/app/styles/core/forms.scss | 4 + ui/app/styles/core/message.scss | 4 + .../components/mount-backend-form.hbs | 5 +- .../components/secret-edit-display.hbs | 201 ++++++++++-------- ui/app/templates/components/secret-edit.hbs | 26 ++- ui/lib/core/addon/components/alert-inline.js | 4 +- ui/lib/core/addon/components/form-field.js | 8 + .../components/form-field-groups.hbs | 2 + .../addon/templates/components/form-field.hbs | 26 ++- ui/package.json | 1 + .../secrets/backend/kv/secret-test.js | 26 ++- ui/yarn.lock | 24 +++ 17 files changed, 310 insertions(+), 114 deletions(-) create mode 100644 changelog/11785.txt diff --git a/changelog/11785.txt b/changelog/11785.txt new file mode 100644 index 000000000..e8bf42907 --- /dev/null +++ b/changelog/11785.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: Add Validation to KV secret engine +``` \ No newline at end of file diff --git a/ui/app/components/mount-backend-form.js b/ui/app/components/mount-backend-form.js index 6e0237cef..a01e039eb 100644 --- a/ui/app/components/mount-backend-form.js +++ b/ui/app/components/mount-backend-form.js @@ -1,5 +1,5 @@ import { inject as service } from '@ember/service'; -import { computed } from '@ember/object'; +import { computed, set } from '@ember/object'; import Component from '@ember/component'; import { task } from 'ember-concurrency'; import { methods } from 'vault/helpers/mountable-auth-methods'; @@ -49,6 +49,10 @@ export default Component.extend({ const modelType = type === 'secret' ? 'secret-engine' : 'auth-method'; const model = this.store.createRecord(modelType); this.set('mountModel', model); + + this.set('validationMessages', { + path: '', + }); }, mountTypes: computed('engines', 'mountType', function() { @@ -99,6 +103,12 @@ export default Component.extend({ .withTestWaiter(), actions: { + onKeyUp(name, value) { + this.mountModel.set('path', value); + this.mountModel.validations.attrs.path.isValid + ? set(this.validationMessages, 'path', '') + : set(this.validationMessages, 'path', this.mountModel.validations.attrs.path.message); + }, onTypeChange(path, value) { if (path === 'type') { this.wizard.set('componentState', value); diff --git a/ui/app/components/secret-edit.js b/ui/app/components/secret-edit.js index bb171886a..514776a64 100644 --- a/ui/app/components/secret-edit.js +++ b/ui/app/components/secret-edit.js @@ -57,6 +57,10 @@ export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, { hasLintError: false, isV2: false, + // cp-validation related properties + validationMessages: null, + validationErrorCount: 0, + init() { this._super(...arguments); let secrets = this.model.secretData; @@ -79,9 +83,16 @@ export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, { if (this.mode === 'edit') { this.send('addRow'); } + + this.set('validationMessages', { + path: '', + key: '', + maxVersions: '', + }); }, - waitForKeyUp: task(function*() { + waitForKeyUp: task(function*(name, value) { + this.checkValidation(name, value); while (true) { let event = yield waitForEvent(document.body, 'keyup'); this.onEscape(event); @@ -171,6 +182,32 @@ export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, { return this.router.transitionTo(...arguments); }, + checkValidation(name, value) { + // because path and key are not on the model performing custom validations instead of cp-validations + if (name === 'path' || name === 'key') { + // no value indicates missing presence + !value + ? set(this.validationMessages, name, `${name} can't be blank`) + : set(this.validationMessages, name, ''); + } + if (name === 'maxVersions') { + // checking for value because value which is blank on first load. No keyup event has occurred and default is 10. + if (value) { + let number = Number(value); + this.model.set('maxVersions', number); + } + if (!this.model.validations.attrs.maxVersions.isValid) { + set(this.validationMessages, name, this.model.validations.attrs.maxVersions.message); + } else { + set(this.validationMessages, name, ''); + } + } + + let values = Object.values(this.validationMessages); + + this.set('validationErrorCount', values.filter(Boolean).length); + }, + onEscape(e) { if (e.keyCode !== keys.ESC || this.mode !== 'show') { return; @@ -312,17 +349,14 @@ export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, { createOrUpdateKey(type, event) { event.preventDefault(); - const MAXIMUM_VERSIONS = 9999999999999999; let model = this.modelForData; - let secret = this.model; - // prevent from submitting if there's no key + let arraySecretKeys = Object.keys(model.secretData); if (type === 'create' && isBlank(model.path || model.id)) { - this.flashMessages.danger('Please provide a path for the secret'); + this.checkValidation('path', ''); return; } - const maxVersions = secret.get('maxVersions'); - if (MAXIMUM_VERSIONS < maxVersions) { - this.flashMessages.danger('Max versions is too large'); + if (arraySecretKeys.includes('')) { + this.checkValidation('key', ''); return; } diff --git a/ui/app/models/secret-engine.js b/ui/app/models/secret-engine.js index f5a050924..21ded56d8 100644 --- a/ui/app/models/secret-engine.js +++ b/ui/app/models/secret-engine.js @@ -2,12 +2,20 @@ import Model, { attr } from '@ember-data/model'; import { computed } from '@ember/object'; import { fragment } from 'ember-data-model-fragments/attributes'; import fieldToAttrs, { expandAttributeMeta } from 'vault/utils/field-to-attrs'; +import { validator, buildValidations } from 'ember-cp-validations'; //identity will be managed separately and the inclusion //of the system backend is an implementation detail const LIST_EXCLUDED_BACKENDS = ['system', 'identity']; -export default Model.extend({ +const Validations = buildValidations({ + path: validator('presence', { + presence: true, + message: "Path can't be blank", + }), +}); + +export default Model.extend(Validations, { path: attr('string'), accessor: attr('string'), name: attr('string'), diff --git a/ui/app/models/secret-v2.js b/ui/app/models/secret-v2.js index 5a1b00881..1060a8580 100644 --- a/ui/app/models/secret-v2.js +++ b/ui/app/models/secret-v2.js @@ -4,8 +4,24 @@ import { alias } from '@ember/object/computed'; import { expandAttributeMeta } from 'vault/utils/field-to-attrs'; import KeyMixin from 'vault/mixins/key-mixin'; import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities'; +import { validator, buildValidations } from 'ember-cp-validations'; -export default Model.extend(KeyMixin, { +const Validations = buildValidations({ + maxVersions: [ + validator('number', { + allowString: false, + integer: true, + message: 'Maximum versions must be a number', + }), + validator('length', { + min: 1, + max: 16, + message: 'You cannot go over 16 characters', + }), + ], +}); + +export default Model.extend(KeyMixin, Validations, { failedServerRead: attr('boolean'), engine: belongsTo('secret-engine', { async: false }), engineId: attr('string'), diff --git a/ui/app/styles/core/forms.scss b/ui/app/styles/core/forms.scss index 7db94e48a..ceb3399d8 100644 --- a/ui/app/styles/core/forms.scss +++ b/ui/app/styles/core/forms.scss @@ -323,3 +323,7 @@ label { fieldset.form-fieldset { border: none; } + +.has-error-border { + border: 1px solid $red-500; +} diff --git a/ui/app/styles/core/message.scss b/ui/app/styles/core/message.scss index c6348b287..dd5e3202d 100644 --- a/ui/app/styles/core/message.scss +++ b/ui/app/styles/core/message.scss @@ -127,6 +127,10 @@ &.padding-top { padding-top: $size-8; } + + &.is-marginless { + margin-bottom: 0; + } } .has-text-highlight { diff --git a/ui/app/templates/components/mount-backend-form.hbs b/ui/app/templates/components/mount-backend-form.hbs index e2cde2f20..d19f49823 100644 --- a/ui/app/templates/components/mount-backend-form.hbs +++ b/ui/app/templates/components/mount-backend-form.hbs @@ -29,10 +29,9 @@ @mode="enable" @noun={{if (eq mountType "auth") "Auth Method" "Secret Engine"}} /> - {{#if showEnable}} - + {{else}} {{#each (array "generic" "cloud" "infra") as |category|}} @@ -69,7 +68,7 @@ type="submit" data-test-mount-submit="true" class="button is-primary {{if mountBackend.isRunning "loading"}}" - disabled={{mountBackend.isRunning}} + disabled={{or mountBackend.isRunning validationError}} > {{#if (eq mountType "auth")}} Enable Method diff --git a/ui/app/templates/components/secret-edit-display.hbs b/ui/app/templates/components/secret-edit-display.hbs index 902dd4798..05be4f615 100644 --- a/ui/app/templates/components/secret-edit-display.hbs +++ b/ui/app/templates/components/secret-edit-display.hbs @@ -1,92 +1,115 @@ - {{#if (and (or @model.isNew @canEditV2Secret) @isV2 (not @model.failedServerRead))}} -
- - {{#each @model.fields as |attr|}} - - {{/each}} -
- {{/if}} +{{#if (and (or @model.isNew @canEditV2Secret) @isV2 (not @model.failedServerRead))}} +
+ + {{#each @model.fields as |attr|}} + + {{/each}} +
+{{/if}} - {{#if @showWriteWithoutReadWarning}} - {{#if (and @isV2 @model.failedServerRead)}} - - {{else if @isV2}} - - {{else}} - + {{else if @isV2}} + + {{else}} + + {{/if}} +{{/if}} + +{{#if @showAdvancedMode}} +
+ + +
+{{else}} +
+ + {{#each @secretData as |secret index|}} +
+
+ +
+
+ +
+
+ {{#if (eq @secretData.length (inc index))}} + + {{else}} + + {{/if}} +
+
+ {{#if @validationMessages.key}} + {{/if}} - {{/if}} - - {{#if @showAdvancedMode}} -
- - -
- {{else}} -
- - {{#each @secretData as |secret index|}} -
-
- -
-
- -
-
- {{#if (eq @secretData.length (inc index))}} - - {{else}} - - {{/if}} -
-
- {{/each}} -
- {{/if}} + {{/each}} +
+{{/if}} diff --git a/ui/app/templates/components/secret-edit.hbs b/ui/app/templates/components/secret-edit.hbs index d8d748310..585485790 100644 --- a/ui/app/templates/components/secret-edit.hbs +++ b/ui/app/templates/components/secret-edit.hbs @@ -142,8 +142,24 @@

- +

+ {{#if (get validationMessages 'path')}} + + {{/if}} {{#if modelForData.isFolder}}

The secret path may not end in / @@ -165,12 +181,14 @@ deleteRow=(action "deleteRow") addRow=(action "addRow") }} + @onKeyUp={{perform waitForKeyUp}} + @validationMessages={{validationMessages}} />

@@ -222,7 +242,7 @@
{{else if (eq attr.type "boolean")}} diff --git a/ui/package.json b/ui/package.json index aab44fb98..9f334c0f2 100644 --- a/ui/package.json +++ b/ui/package.json @@ -93,6 +93,7 @@ "ember-concurrency": "^1.3.0", "ember-concurrency-test-waiter": "^0.3.2", "ember-copy": "^1.0.0", + "ember-cp-validations": "^4.0.0-beta.12", "ember-data": "~3.22.0", "ember-data-model-fragments": "5.0.0-beta.0", "ember-engines": "^0.8.3", diff --git a/ui/tests/acceptance/secrets/backend/kv/secret-test.js b/ui/tests/acceptance/secrets/backend/kv/secret-test.js index a923d8e9d..51782d165 100644 --- a/ui/tests/acceptance/secrets/backend/kv/secret-test.js +++ b/ui/tests/acceptance/secrets/backend/kv/secret-test.js @@ -1,4 +1,12 @@ -import { click, visit, settled, currentURL, currentRouteName } from '@ember/test-helpers'; +import { + click, + visit, + settled, + currentURL, + currentRouteName, + fillIn, + triggerKeyEvent, +} from '@ember/test-helpers'; import { create } from 'ember-cli-page-object'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; @@ -56,6 +64,22 @@ module('Acceptance | secrets/secret/create', function(hooks) { assert.ok(showPage.editIsPresent, 'shows the edit button'); }); + test('it disables save when validation errors occur', async function(assert) { + let enginePath = `kv-${new Date().getTime()}`; + await mountSecrets.visit(); + await mountSecrets.enable('kv', enginePath); + await click('[data-test-secret-create="true"]'); + await fillIn('[data-test-secret-path="true"]', 'abc'); + await fillIn('[data-test-input="maxVersions"]', 'abc'); + await triggerKeyEvent('[data-test-input="maxVersions"]', 'keyup', 65); + await settled(); + assert.dom('[data-test-secret-save="true"]').isDisabled('Save button is disabled'); + await fillIn('[data-test-input="maxVersions"]', 20); + await triggerKeyEvent('[data-test-input="maxVersions"]', 'keyup', 65); + await click('[data-test-secret-save="true"]'); + assert.equal(currentURL(), `/vault/secrets/${enginePath}/show/abc`, 'navigates to show secret'); + }); + test('version 1 performs the correct capabilities lookup', async function(assert) { let enginePath = `kv-${new Date().getTime()}`; let secretPath = 'foo/bar'; diff --git a/ui/yarn.lock b/ui/yarn.lock index ed934b96f..b75161606 100644 --- a/ui/yarn.lock +++ b/ui/yarn.lock @@ -8191,6 +8191,15 @@ ember-copy@1.0.0, ember-copy@^1.0.0: dependencies: ember-cli-babel "^6.6.0" +ember-cp-validations@^4.0.0-beta.12: + version "4.0.0-beta.12" + resolved "https://registry.yarnpkg.com/ember-cp-validations/-/ember-cp-validations-4.0.0-beta.12.tgz#27c7e79e36194b8bb55c5c97421b2671f0abf58c" + integrity sha512-GHOJm2pjan4gOOBFecs7PdEf86vnWgTPCtfqwyqf3wlN0CihYf+mHZhjnnN6R1fnPDn+qLwByl6gJq7il115dw== + dependencies: + ember-cli-babel "^7.1.2" + ember-require-module "^0.3.0" + ember-validators "^3.0.1" + ember-data-model-fragments@5.0.0-beta.0: version "5.0.0-beta.0" resolved "https://registry.yarnpkg.com/ember-data-model-fragments/-/ember-data-model-fragments-5.0.0-beta.0.tgz#da90799970317ca852f96b2ea1548ca70094a5bb" @@ -8399,6 +8408,13 @@ ember-radio-button@^2.0.1: ember-cli-babel "^6.9.2" ember-cli-htmlbars "^1.1.1" +ember-require-module@^0.3.0: + version "0.3.0" + resolved "https://registry.yarnpkg.com/ember-require-module/-/ember-require-module-0.3.0.tgz#65aff7908b5b846467e4526594d33cfe0c23456b" + integrity sha512-rYN4YoWbR9VlJISSmx0ZcYZOgMcXZLGR7kdvp3zDerjIvYmHm/3p+K56fEAYmJILA6W4F+cBe41Tq2HuQAZizA== + dependencies: + ember-cli-babel "^6.9.2" + ember-resolver@^8.0.2: version "8.0.2" resolved "https://registry.yarnpkg.com/ember-resolver/-/ember-resolver-8.0.2.tgz#8a45a744aaf5391eb52b4cb393b3b06d2db1975c" @@ -8604,6 +8620,14 @@ ember-truth-helpers@^2.1.0: dependencies: ember-cli-babel "^6.6.0" +ember-validators@^3.0.1: + version "3.0.1" + resolved "https://registry.yarnpkg.com/ember-validators/-/ember-validators-3.0.1.tgz#9e0f7ed4ce6817aa05f7d46e95a0267c03f1f043" + integrity sha512-GbvvECDG9N7U+4LXxPWNgiSnGbOzgvGBIxtS4kw2uyEIy7kymtgszhpSnm8lGMKYnhCKBqFingh8qnVKlCi0lg== + dependencies: + ember-cli-babel "^6.9.2" + ember-require-module "^0.3.0" + ember-wormhole@^0.5.5: version "0.5.5" resolved "https://registry.yarnpkg.com/ember-wormhole/-/ember-wormhole-0.5.5.tgz#db417ff748cb21e574cd5f233889897bc27096cb"