Skip to content

Commit

Permalink
UI - don't coerce JSON input to an Object (#5271)
Browse files Browse the repository at this point in the history
* have fromJSON throw if trying to convert non-object to a KVObject

* catch the fromJSON error in secret-edit, display an error, and disabled the submit button
  • Loading branch information
meirish authored Sep 5, 2018
1 parent 51eea74 commit 1137763
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 7 deletions.
11 changes: 9 additions & 2 deletions ui/app/components/secret-edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export default Ember.Component.extend(FocusOnInsertMixin, {
// use a named action here so we don't have to pass one in
// this will bubble to the route
toggleAdvancedEdit: 'toggleAdvancedEdit',
error: null,

codemirrorString: null,

Expand Down Expand Up @@ -79,7 +80,8 @@ export default Ember.Component.extend(FocusOnInsertMixin, {
'key.isFolder',
'key.isError',
'key.flagsIsInvalid',
'hasLintError'
'hasLintError',
'error'
),

basicModeDisabled: computed('secretDataIsAdvanced', 'showAdvancedMode', function() {
Expand Down Expand Up @@ -242,10 +244,15 @@ export default Ember.Component.extend(FocusOnInsertMixin, {
},

codemirrorUpdated(val, codemirror) {
this.set('error', null);
codemirror.performLint();
const noErrors = codemirror.state.lint.marked.length === 0;
if (noErrors) {
this.get('secretData').fromJSONString(val);
try {
this.get('secretData').fromJSONString(val);
} catch (e) {
this.set('error', e.message);
}
}
this.set('hasLintError', !noErrors);
this.set('codemirrorString', val);
Expand Down
9 changes: 7 additions & 2 deletions ui/app/lib/kv-object.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import Ember from 'ember';

const { typeOf, guidFor } = Ember;

export default Ember.ArrayProxy.extend({
fromJSON(json) {
const contents = Object.keys(json || []).map(key => {
if (json && typeOf(json) !== 'object') {
throw new Error('Vault expects data to be formatted as an JSON object.');
}
let contents = Object.keys(json || []).map(key => {
let obj = {
name: key,
value: json[key],
};
Ember.guidFor(obj);
guidFor(obj);
return obj;
});
this.setObjects(
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/partials/secret-form-create.hbs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<form class="{{if showAdvancedMode 'advanced-edit' 'simple-edit'}}" onsubmit={{action "createOrUpdateKey" "create"}} onchange={{action "handleChange"}}>
<div class="field box is-fullwidth is-sideless is-marginless">
<NamespaceReminder @mode="create" @noun="secret" />
{{message-error model=key}}
{{message-error model=key errorMessage=error}}
<label class="label is-font-weight-normal" for="kv-key">Path for this secret</label>
<div class="field has-addons">
{{#if (not-eq key.initialParentKey '') }}
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/partials/secret-form-edit.hbs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<form onsubmit={{action "createOrUpdateKey" "update"}} onchange={{action "handleChange"}}>
<div class="box is-sideless is-fullwidth is-marginless">
{{message-error model=key}}
{{message-error model=key errorMessage=error}}
<NamespaceReminder @mode="edit" @noun="secret" />
{{#unless showAdvancedMode}}
<div class="table info-table-row-header">
Expand Down
40 changes: 39 additions & 1 deletion ui/tests/integration/components/secret-edit-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import hbs from 'htmlbars-inline-precompile';

moduleForComponent('secret-edit', 'Integration | Component | secret edit', {
integration: true,
beforeEach() {
this.inject.service('code-mirror', { as: 'codeMirror' });
},
});

test('it disables JSON toggle in show mode when is an advanced format', function(assert) {
Expand All @@ -19,7 +22,7 @@ test('it disables JSON toggle in show mode when is an advanced format', function
assert.dom('[data-test-secret-json-toggle]').isDisabled();
});

test('it does JSON toggle in show mode when is', function(assert) {
test('it does JSON toggle in show mode when showing string data', function(assert) {
this.set('mode', 'show');
this.set('key', {
secretData: {
Expand All @@ -32,3 +35,38 @@ test('it does JSON toggle in show mode when is', function(assert) {
this.render(hbs`{{secret-edit mode=mode key=key }}`);
assert.dom('[data-test-secret-json-toggle]').isNotDisabled();
});

test('it shows an error when creating and data is not an object', function(assert) {
this.set('mode', 'create');
this.set('key', {
secretData: {
int: '2',
null: 'null',
float: '1.234',
},
});

this.render(hbs`{{secret-edit mode=mode key=key preferAdvancedEdit=true }}`);
let instance = this.codeMirror.instanceFor(this.$('[data-test-component=json-editor]').attr('id'));
instance.setValue(JSON.stringify([{ foo: 'bar' }]));
assert.dom('[data-test-error]').includesText('Vault expects data to be formatted as an JSON object');
});

test('it shows an error when editing and the data is not an object', function(assert) {
this.set('mode', 'edit');
this.set('capabilities', {
canUpdate: true,
});
this.set('key', {
secretData: {
int: '2',
null: 'null',
float: '1.234',
},
});

this.render(hbs`{{secret-edit capabilities=capabilities mode=mode key=key preferAdvancedEdit=true }}`);
let instance = this.codeMirror.instanceFor(this.$('[data-test-component=json-editor]').attr('id'));
instance.setValue(JSON.stringify([{ foo: 'bar' }]));
assert.dom('[data-test-error]').includesText('Vault expects data to be formatted as an JSON object');
});
11 changes: 11 additions & 0 deletions ui/tests/unit/lib/kv-object-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@ fromJSONTests.forEach(function([name, input, content]) {
});
});

test(`fromJSON: non-object input`, function(assert) {
let input = [{ foo: 'bar' }];
assert.throws(
() => {
KVObject.create({ content: [] }).fromJSON(input);
},
/Vault expects data to be formatted as an JSON object/,
'throws when non-object input is used to construct the KVObject'
);
});

fromJSONTests.forEach(function([name, input, content]) {
test(`fromJSONString: ${name}`, function(assert) {
let inputString = JSON.stringify(input, null, 2);
Expand Down

0 comments on commit 1137763

Please sign in to comment.