Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI/Wrong sentinel error message for auth methods #14551

Merged
merged 13 commits into from
Mar 18, 2022
37 changes: 24 additions & 13 deletions ui/app/components/mount-backend-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,18 +112,21 @@ export default Component.extend({
throw err;
}
}

// TODO CMB: this flash message seems to be an over-generalization, so added the 'if...path !==' check, but want to revisit
// TODO should we check instead if config capabilities.capabilites.includes('deny')
if (!capabilities.get('canUpdate')) {
// if there is no sys/mount issue then error is config endpoint.
this.flashMessages.warning(
'You do not have access to the config endpoint. The secret engine was mounted, but the configuration settings were not saved.'
);
// remove the config data from the model otherwise it will save it even if the network request failed.
[this.mountModel.maxVersions, this.mountModel.casRequired, this.mountModel.deleteVersionAfter] = [
0,
false,
0,
];
if (capabilities.get('path') !== 'userpass/config') {
// if there is no sys/mount issue then error is config endpoint.
this.flashMessages.warning(
'You do not have access to the config endpoint. The secret engine was mounted, but the configuration settings were not saved.'
);
// remove the config data from the model otherwise it will save it even if the network request failed.
[this.mountModel.maxVersions, this.mountModel.casRequired, this.mountModel.deleteVersionAfter] = [
0,
false,
0,
];
}
}
try {
yield mountModel.save();
Expand All @@ -135,9 +138,17 @@ export default Component.extend({
'You do not have access to the sys/mounts endpoint. The secret engine was not mounted.'
);
return;
} else if (err.errors) {
let errors = err.errors.map((e) => {
if (typeof e === 'object') return e.title || e.message || JSON.stringify(e);
return e;
});
this.set('errors', errors);
return;
} else {
this.set('errorMessage', err.message);
return;
}
this.set('errorMessage', 'This mount path already exist.');
return;
}
let mountType = this.mountType;
mountType = mountType === 'secret' ? `${mountType}s engine` : `${mountType} method`;
Expand Down
5 changes: 2 additions & 3 deletions ui/app/mixins/policy-edit-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ export default Mixin.create({
}
return this.transitionToRoute('vault.cluster.policy.show', m.get('policyType'), m.get('name'));
})
.catch(() => {
// do nothing here...
// message-error component will show errors
.catch((e) => {
model.set('errors', e.errors);
});
},

Expand Down
2 changes: 1 addition & 1 deletion ui/app/models/policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import { computed } from '@ember/object';
import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities';

export default Model.extend({
errors: attr('array'),
name: attr('string'),
policy: attr('string'),
policyType: computed('constructor.modelName', function () {
return this.constructor.modelName.split('/')[1];
}),

updatePath: lazyCapabilities(apiPath`sys/policies/${'policyType'}/${'id'}`, 'id', 'policyType'),
canDelete: alias('updatePath.canDelete'),
canEdit: alias('updatePath.canUpdate'),
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/components/mount-backend-form.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<form {{action (perform this.mountBackend) on="submit"}}>
<div class="box is-sideless is-fullwidth is-marginless">
<NamespaceReminder @mode="enable" @noun={{if (eq this.mountType "auth") "Auth Method" "Secret Engine"}} />
<MessageError @model={{this.model}} @errorMessage={{this.errorMessage}} />
<MessageError @model={{this.model}} @errorMessage={{this.errorMessage}} @errors={{this.errors}} />
{{#if this.showEnable}}
<FormFieldGroups
@model={{this.mountModel}}
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/vault/cluster/policies/create.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

<form {{action "savePolicy" this.model on="submit"}}>
<div class="box is-bottomless is-fullwidth is-marginless">
<MessageError @model={{this.model}} />
<MessageError @model={{@model}} @errors={{this.model.errors}} />
<NamespaceReminder @mode="create" @noun="policy" />
<div class="field">
<label for="policy-name" class="is-label">Name</label>
Expand Down
138 changes: 97 additions & 41 deletions ui/lib/core/addon/components/message-error.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { computed } from '@ember/object';
import Component from '@ember/component';
import Component from '@glimmer/component';
import layout from '../templates/components/message-error';
import { setComponentTemplate } from '@ember/component';

/**
* @module MessageError
Expand All @@ -11,48 +11,104 @@ import layout from '../templates/components/message-error';
* <MessageError @model={{model}} />
* ```
*
* @param model=null{DS.Model} - An Ember data model that will be used to bind error statest to the internal
* @param {object} [model=null] - An Ember data model that will be used to bind error states to the internal
* `errors` property.
* @param errors=null{Array} - An array of error strings to show.
* @param errorMessage=null{String} - An Error string to display.
* @param {array} [errors=null] - An array of error strings to show.
* @param {string} [errorMessage=null] - An Error string to display.
*/
export default Component.extend({
layout,
model: null,
errors: null,
errorMessage: null,

displayErrors: computed(
'errorMessage',
'model.{isError,adapterError.message,adapterError.errors.@each}',
'errors',
'errors.[]',
function () {
const errorMessage = this.errorMessage;
const errors = this.errors;
const modelIsError = this.model?.isError;
if (errorMessage) {
return [errorMessage];
}

if (errors && errors.length > 0) {
return errors;
}
class MessageError extends Component {
get errorMessage() {
return this.args.errorMessage;
}

if (modelIsError) {
if (!this.model.adapterError) {
return;
}
if (this.model.adapterError.errors.length > 0) {
return this.model.adapterError.errors.map((e) => {
if (typeof e === 'object') return e.title || e.message || JSON.stringify(e);
return e;
});
}
return [this.model.adapterError.message];
}
get errors() {
return this.args.errors;
}

get model() {
return this.args.model;
}

get displayErrors() {
if (this.errorMessage) {
return [this.errorMessage];
}

if (this.errors && this.errors.length > 0) {
return this.errors;
}

return 'no error';
if (this.model?.isError) {
let adapterError = this.model?.adapterError;
if (!adapterError) {
return null;
}
if (adapterError.errors.length > 0) {
return adapterError.errors.map((e) => {
if (typeof e === 'object') return e.title || e.message || JSON.stringify(e);
return e;
});
}
return [adapterError.message];
}
),
});
return null;
}
}
export default setComponentTemplate(layout, MessageError);

// import { computed } from '@ember/object';
// import Component from '@ember/component';
// import layout from '../templates/components/message-error';

// /**
// * @module MessageError
// @@ -11,48 +11,39 @@ import layout from '../templates/components/message-error';
// * <MessageError @model={{model}} />
// * ```
// *
// * @param model=null{DS.Model} - An Ember data model that will be used to bind error statest to the internal
// * `errors` property.
// * @param errors=null{Array} - An array of error strings to show.
// * @param errorMessage=null{String} - An Error string to display.
// */
// export default Component.extend({
// layout,
// model: null,
// errors: null,
// errorMessage: null,

// displayErrors: computed(
// 'errorMessage',
// 'model.{isError,adapterError.message,adapterError.errors.@each}',
// 'errors',
// 'errors.[]',
// function () {
// const errorMessage = this.errorMessage;
// const errors = this.errors;
// const modelIsError = this.model?.isError;
// if (errorMessage) {
// return [errorMessage];
// }

// if (errors && errors.length > 0) {
// return errors;
// }

// if (modelIsError) {
// if (!this.model.adapterError) {
// return;
// }
// if (this.model.adapterError.errors.length > 0) {
// return this.model.adapterError.errors.map((e) => {
// if (typeof e === 'object') return e.title || e.message || JSON.stringify(e);
// return e;
// });
// }
// return [this.model.adapterError.message];
// }

// return 'no error';
// }
// ),
// });
4 changes: 2 additions & 2 deletions ui/lib/core/addon/templates/components/message-error.hbs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{{#if this.displayErrors.length}}
{{#if this.displayErrors}}
{{#each this.displayErrors as |error|}}
<AlertBanner @type="danger" @message={{error}} data-test-error />
<AlertBanner @type="danger" @message={{error}} data-test-error ...attributes />
{{/each}}
{{/if}}
2 changes: 1 addition & 1 deletion ui/tests/integration/components/auth-form-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ module('Integration | Component | auth form', function (hooks) {
this.set('cluster', EmberObject.create({ standby: true }));
this.set('selectedAuth', 'token');
await render(hbs`{{auth-form cluster=cluster selectedAuth=selectedAuth}}`);
assert.equal(component.errorText, '');
assert.equal(component.errorMessagePresent, false);
component.login();
// because this is an ember-concurrency backed service,
// we have to manually force settling the run queue
Expand Down
3 changes: 2 additions & 1 deletion ui/tests/pages/components/auth-form.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { collection, clickable, fillable, text, value } from 'ember-cli-page-object';
import { collection, clickable, fillable, text, value, isPresent } from 'ember-cli-page-object';

export default {
tabs: collection('[data-test-auth-method]', {
Expand All @@ -11,6 +11,7 @@ export default {
tokenValue: value('[data-test-token]'),
password: fillable('[data-test-password]'),
errorText: text('[data-test-auth-error]'),
errorMessagePresent: isPresent('[data-test-auth-error]'),
descriptionText: text('[data-test-description]'),
login: clickable('[data-test-auth-submit]'),
};