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
This commit is contained in:
Angel Garbarino 2021-06-15 09:21:54 -06:00 committed by GitHub
parent c70b2dd5eb
commit d99742c6c5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 310 additions and 114 deletions

3
changelog/11785.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
ui: Add Validation to KV secret engine
```

View File

@ -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);

View File

@ -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;
}

View File

@ -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'),

View File

@ -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'),

View File

@ -323,3 +323,7 @@ label {
fieldset.form-fieldset {
border: none;
}
.has-error-border {
border: 1px solid $red-500;
}

View File

@ -127,6 +127,10 @@
&.padding-top {
padding-top: $size-8;
}
&.is-marginless {
margin-bottom: 0;
}
}
.has-text-highlight {

View File

@ -29,10 +29,9 @@
@mode="enable"
@noun={{if (eq mountType "auth") "Auth Method" "Secret Engine"}}
/>
<MessageError @model={{mountModel}} />
{{#if showEnable}}
<FormFieldGroups @model={{mountModel}} @onChange={{action "onTypeChange"}} @renderGroup="default" />
<FormFieldGroups @model={{mountModel}} @onChange={{action "onTypeChange"}} @renderGroup="default" @validationMessages={{validationMessages}} @onKeyUp={{action "onKeyUp"}} />
<FormFieldGroups @model={{mountModel}} @onChange={{action "onTypeChange"}} @renderGroup="Method Options" />
{{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

View File

@ -1,92 +1,115 @@
{{#if (and (or @model.isNew @canEditV2Secret) @isV2 (not @model.failedServerRead))}}
<div data-test-metadata-fields class="form-section box is-shadowless is-fullwidth">
<label class="title is-5">
Secret metadata
</label>
{{#each @model.fields as |attr|}}
<FormField data-test-field @attr={{attr}} @model={{@model}} />
{{/each}}
</div>
{{/if}}
{{#if (and (or @model.isNew @canEditV2Secret) @isV2 (not @model.failedServerRead))}}
<div data-test-metadata-fields class="form-section box is-shadowless is-fullwidth">
<label class="title is-5">
Secret metadata
</label>
{{#each @model.fields as |attr|}}
<FormField
data-test-field
@attr={{attr}}
@model={{@model}}
@onKeyUp={{@onKeyUp}}
@validationMessages={{@validationMessages}}
/>
{{/each}}
</div>
{{/if}}
{{#if @showWriteWithoutReadWarning}}
{{#if (and @isV2 @model.failedServerRead)}}
<AlertBanner
@type="warning"
@message="Your policies prevent you from reading metadata for this secret and the current version's data. Creating a new version of the secret with this form will not be able to use the check-and-set mechanism. If this is required on the secret, then you will need access to read the secret's metadata."
@class="is-marginless"
data-test-v2-no-cas-warning
/>
{{else if @isV2}}
<AlertBanner
@type="warning"
@message="Your policies prevent you from reading the current secret version. Saving this form will create a new version of the secret and will utilize the available check-and-set mechanism."
@class="is-marginless"
data-test-v2-write-without-read
/>
{{else}}
<AlertBanner
@type="warning"
@message="Your policies prevent you from reading the current secret data. Saving using this form will overwrite the existing values."
@class="is-marginless"
data-test-v1-write-without-read
{{#if @showWriteWithoutReadWarning}}
{{#if (and @isV2 @model.failedServerRead)}}
<AlertBanner
@type="warning"
@message="Your policies prevent you from reading metadata for this secret and the current version's data. Creating a new version of the secret with this form will not be able to use the check-and-set mechanism. If this is required on the secret, then you will need access to read the secret's metadata."
@class="is-marginless"
data-test-v2-no-cas-warning
/>
{{else if @isV2}}
<AlertBanner
@type="warning"
@message="Your policies prevent you from reading the current secret version. Saving this form will create a new version of the secret and will utilize the available check-and-set mechanism."
@class="is-marginless"
data-test-v2-write-without-read
/>
{{else}}
<AlertBanner
@type="warning"
@message="Your policies prevent you from reading the current secret data. Saving using this form will overwrite the existing values."
@class="is-marginless"
data-test-v1-write-without-read
/>
{{/if}}
{{/if}}
{{#if @showAdvancedMode}}
<div class="form-section">
<JsonEditor
@title={{if isV2 "Version Data" "Secret Data"}}
@value={{@codemirrorString}}
@valueUpdated={{action @editActions.codemirrorUpdated}}
@onFocusOut={{action @editActions.formatJSON}}>
</JsonEditor>
</div>
{{else}}
<div class="form-section">
<label class="title is-5">
{{#if isV2}}
Version data
{{else}}
Secret data
{{/if}}
</label>
{{#each @secretData as |secret index|}}
<div class="info-table-row">
<div class="column is-one-quarter info-table-row-edit">
<Input
data-test-secret-key={{true}}
@value={{secret.name}}
placeholder="key"
@change={{action @editActions.handleChange}}
class="input {{if @validationMessages.key "has-error-border"}}"
@autocomplete="off"
@spellcheck="false"
{{on "keyup" (fn @onKeyUp "key" secret.name)}}
/>
</div>
<div class="column info-table-row-edit">
<MaskedInput
@name={{secret.name}}
@onKeyDown={{@editActions.handleKeyDown}}
@onChange={{@editActions.handleChange}}
@value={{secret.value}}
data-test-secret-value="true"
/>
</div>
<div class="column is-narrow info-table-row-edit">
{{#if (eq @secretData.length (inc index))}}
<button type="button" {{action @editActions.addRow}} class="button is-outlined is-primary" data-test-secret-add-row="true">
Add
</button>
{{else}}
<button
class="button has-text-grey is-expanded is-icon"
type="button"
{{action @editActions.deleteRow secret.name}}
aria-label="Delete row"
>
<Icon
@glyph="trash"
@size="l"
class="has-text-grey-light"
/>
</button>
{{/if}}
</div>
</div>
{{#if @validationMessages.key}}
<AlertInline
@type="danger"
@message={{@validationMessages.key}}
@paddingTop=true
@isMarginless=true
/>
{{/if}}
{{/if}}
{{#if @showAdvancedMode}}
<div class="form-section">
<JsonEditor
@title={{if isV2 "Version Data" "Secret Data"}}
@value={{@codemirrorString}}
@valueUpdated={{action @editActions.codemirrorUpdated}}
@onFocusOut={{action @editActions.formatJSON}}>
</JsonEditor>
</div>
{{else}}
<div class="form-section">
<label class="title is-5">
{{#if isV2}}
Version data
{{else}}
Secret data
{{/if}}
</label>
{{#each @secretData as |secret index|}}
<div class="info-table-row">
<div class="column is-one-quarter info-table-row-edit">
<Input data-test-secret-key={{true}} @value={{secret.name}} placeholder="key" @change={{action @editActions.handleChange}} class="input" @autocomplete="off" @spellcheck="false" />
</div>
<div class="column info-table-row-edit">
<MaskedInput
@name={{secret.name}}
@onKeyDown={{@editActions.handleKeyDown}}
@onChange={{@editActions.handleChange}}
@value={{secret.value}}
data-test-secret-value="true"
/>
</div>
<div class="column is-narrow info-table-row-edit">
{{#if (eq @secretData.length (inc index))}}
<button type="button" {{action @editActions.addRow}} class="button is-outlined is-primary" data-test-secret-add-row="true">
Add
</button>
{{else}}
<button
class="button has-text-grey is-expanded is-icon"
type="button"
{{action @editActions.deleteRow secret.name}}
aria-label="Delete row"
>
<Icon
@glyph="trash"
@size="l"
class="has-text-grey-light"
/>
</button>
{{/if}}
</div>
</div>
{{/each}}
</div>
{{/if}}
{{/each}}
</div>
{{/if}}

View File

@ -142,8 +142,24 @@
<MessageError @model={{modelForData}} @errorMessage={{error}} />
<label class="is-label" for="kv-key">Path for this secret</label>
<p class="control is-expanded">
<Input @autocomplete="off" @spellcheck="false" data-test-secret-path="true" @id="kv-key" class="input" @value={{get modelForData modelForData.pathAttr}} />
<Input
@autocomplete="off"
@spellcheck="false"
data-test-secret-path="true"
@id="kv-key"
class="input {{if (get validationMessages 'path') "has-error-border"}}"
@value={{get modelForData modelForData.pathAttr}}
onkeyup={{perform waitForKeyUp "path" value="target.value"}}
/>
</p>
{{#if (get validationMessages 'path')}}
<AlertInline
@type="danger"
@message={{get validationMessages 'path'}}
@paddingTop=true
@isMarginless=true
/>
{{/if}}
{{#if modelForData.isFolder}}
<p class="help is-danger">
The secret path may not end in <code>/</code>
@ -165,12 +181,14 @@
deleteRow=(action "deleteRow")
addRow=(action "addRow")
}}
@onKeyUp={{perform waitForKeyUp}}
@validationMessages={{validationMessages}}
/>
<div class="field is-grouped box is-fullwidth is-bottomless">
<div class="control">
<button
type="submit"
disabled={{buttonDisabled}}
disabled={{or buttonDisabled validationErrorCount}}
class="button is-primary"
data-test-secret-save=true
>
@ -214,6 +232,8 @@
deleteRow=(action "deleteRow")
addRow=(action "addRow")
}}
@onKeyUp={{perform waitForKeyUp}}
@validationMessages={{validationMessages}}
/>
</div>
<div class="field is-grouped is-grouped-split is-fullwidth box is-bottomless">
@ -222,7 +242,7 @@
<button
data-test-secret-save
type="submit"
disabled={{buttonDisabled}}
disabled={{or buttonDisabled validationErrorCount}}
class="button is-primary"
>
Save

View File

@ -16,7 +16,7 @@ import layout from '../templates/components/alert-inline';
* @param [message=null]{String} - The message to display within the alert.
* @param [sizeSmall=false]{Boolean} - Whether or not to display a small font with padding below of alert message.
* @param [paddingTop=false]{Boolean} - Whether or not to add padding above component.
*
* @param [isMarginless=false]{Boolean} - Whether or not to remove margin bottom below component.
*/
export default Component.extend({
@ -26,7 +26,7 @@ export default Component.extend({
sizeSmall: false,
paddingTop: false,
classNames: ['message-inline'],
classNameBindings: ['sizeSmall:size-small', 'paddingTop:padding-top'],
classNameBindings: ['sizeSmall:size-small', 'paddingTop:padding-top', 'isMarginless:is-marginless'],
textClass: computed('type', function() {
if (this.type == 'danger') {

View File

@ -18,11 +18,13 @@ import layout from '../templates/components/form-field';
* ```
*
* @param [onChange=null] {Func} - Called whenever a value on the model changes via the component.
* @param [onKeyUp=null] {Func} - Called whenever cp-validations is being used and you need to validation on keyup. Send name of field and value of input.
* @param attr=null {Object} - This is usually derived from ember model `attributes` lookup, and all members of `attr.options` are optional.
* @param model=null {DS.Model} - The Ember Data model that `attr` is defined on
* @param [disabled=false] {Boolean} - whether the field is disabled
* @param [showHelpText=true] {Boolean} - whether to show the tooltip with help text from OpenAPI
* @param [subText] {String} - Text to be displayed below the label
* @param [validationMessages] {Object} - Object of errors. If attr.name is in object and has error message display in AlertInline.
*
*/
@ -147,5 +149,11 @@ export default Component.extend({
this.send('setAndBroadcast', path, null);
}
},
handleKeyUp(name, value) {
if (!this.onKeyUp) {
return;
}
this.onKeyUp(name, value);
},
},
});

View File

@ -9,6 +9,8 @@
@attr={{attr}}
@model={{model}}
@onChange={{onChange}}
@onKeyUp={{onKeyUp}}
@validationMessages={{validationMessages}}
/>
{{/unless}}
{{/each}}

View File

@ -248,12 +248,22 @@
{{/if}}
{{else}}
{{!-- Regular Text Input --}}
<input data-test-input={{attr.name}} id={{attr.name}} autocomplete="off" spellcheck="false"
value={{or (get model valuePath) attr.options.defaultValue}} onChange={{action
<input
data-test-input={{attr.name}}
id={{attr.name}}
autocomplete="off"
spellcheck="false"
value={{or (get model valuePath) attr.options.defaultValue}}
onChange={{action
(action "setAndBroadcast" valuePath)
value="target.value"
}} class="input" maxLength={{attr.options.characterLimit}} />
}}
onkeyup={{action
(action "handleKeyUp" attr.name)
value="target.value"
}}
class="input {{if (get validationMessages attr.name) "has-error-border"}}"
maxLength={{attr.options.characterLimit}} />
{{#if attr.options.validationAttr}}
{{#if
(and
@ -261,9 +271,15 @@
)
}}
<AlertInline @type="danger" @message={{attr.options.invalidMessage}} />
{{/if}}
{{/if}}
{{#if (get validationMessages attr.name)}}
<AlertInline
@type="danger"
@message={{get validationMessages attr.name}}
@paddingTop=true
/>
{{/if}}
{{/if}}
</div>
{{else if (eq attr.type "boolean")}}

View File

@ -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",

View File

@ -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';

View File

@ -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"