UI/Wrong sentinel error message for auth methods (#14551)

* priortize adapter error over model error

* glimmerize message-error component

* message error tweaks

* fix glimmerize

* fix some tests

* change error handling for mount backend form

* throw API error for secret engine not mounting

* fix tests"

* fix tests

* cleanup error handling for secret engine mounts

* fix test selector

* add changelog

* STOP BEING FLAKY

changelog/14551.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fix issue where UI incorrectly handled API errors when mounting backends
+```

ui/app/adapters/secret-engine.js

@@ -61,15 +61,8 @@ export default ApplicationAdapter.extend({
         data.id = path;
       }
       // first create the engine
-      try {
+      await this.ajax(this.url(path), 'POST', { data });
-        await this.ajax(this.url(path), 'POST', { data });
+
-      } catch (e) {
-        // if error determine if path duplicate or permissions
-        if (e.httpStatus === 400) {
-          throw new Error('samePath');
-        }
-        throw new Error('mountIssue');
-      }
       // second post to config
       try {
         await this.ajax(this.urlForConfig(path), 'POST', { data: configData });

ui/app/components/config-pki-ca.js

@@ -7,6 +7,7 @@ export default Component.extend({
   classNames: 'config-pki-ca',
   store: service('store'),
   flashMessages: service(),
+  errors: null,
 
   /*
    * @param boolean
@@ -150,8 +151,8 @@ export default Component.extend({
             );
           }
         })
-        .catch(() => {
+        .catch((e) => {
-          // handle promise rejection - error will be shown by message-error component
+          this.set('errors', e.errors);
         })
         .finally(() => {
           this.set('loading', false);

ui/app/components/config-pki.js

@@ -5,7 +5,7 @@ import { get } from '@ember/object';
 export default Component.extend({
   classNames: 'config-pki',
   flashMessages: service(),
-
+  errors: null,
   /*
    *
    * @param String
@@ -55,8 +55,8 @@ export default Component.extend({
           }
           this.send('refresh');
         })
-        .catch(() => {
+        .catch((e) => {
-          // handle promise rejection - error will be shown by message-error component
+          this.set('errors', e.errors);
         })
         .finally(() => {
           this.set('loading', false);

ui/app/components/mount-backend-form.js

@@ -113,8 +113,39 @@ export default Component.extend({
         }
       }
 
-      if (!capabilities.get('canUpdate')) {
+      let changedAttrKeys = Object.keys(mountModel.changedAttributes());
-        // if there is no sys/mount issue then error is config endpoint.
+      const updatesConfig =
+        mountModel.isV2KV &&
+        (changedAttrKeys.includes('casRequired') ||
+          changedAttrKeys.includes('deleteVersionAfter') ||
+          changedAttrKeys.includes('maxVersions'));
+
+      try {
+        yield mountModel.save();
+      } catch (err) {
+        if (err.httpStatus === 403) {
+          this.mountIssue = true;
+          this.set('isFormInvalid', this.mountIssue);
+          this.flashMessages.danger(
+            'You do not have access to the sys/mounts endpoint. The secret engine was not mounted.'
+          );
+          return;
+        }
+        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);
+        } else if (err.message) {
+          this.set('errorMessage', err.message);
+        } else {
+          this.set('errorMessage', 'An error occurred, check the vault logs.');
+        }
+        return;
+      }
+      if (updatesConfig && !capabilities.get('canUpdate')) {
+        // config error is not thrown from secret-engine adapter, so handling here
         this.flashMessages.warning(
           'You do not have access to the config endpoint. The secret engine was mounted, but the configuration settings were not saved.'
         );
@@ -125,20 +156,6 @@ export default Component.extend({
           0,
         ];
       }
-      try {
-        yield mountModel.save();
-      } catch (err) {
-        if (err.message === 'mountIssue') {
-          this.mountIssue = true;
-          this.set('isFormInvalid', this.mountIssue);
-          this.flashMessages.danger(
-            'You do not have access to the sys/mounts endpoint. The secret engine was not mounted.'
-          );
-          return;
-        }
-        this.set('errorMessage', 'This mount path already exist.');
-        return;
-      }
       let mountType = this.mountType;
       mountType = mountType === 'secret' ? `${mountType}s engine` : `${mountType} method`;
       this.flashMessages.success(`Successfully mounted the ${type} ${mountType} at ${path}.`);

ui/app/mixins/policy-edit-controller.js

@@ -36,9 +36,8 @@ export default Mixin.create({
           }
           return this.transitionToRoute('vault.cluster.policy.show', m.get('policyType'), m.get('name'));
         })
-        .catch(() => {
+        .catch((e) => {
-          // do nothing here...
+          model.set('errors', e.errors);
-          // message-error component will show errors
         });
     },
 

ui/app/models/policy.js

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

ui/app/templates/components/config-pki-ca.hbs

@@ -150,7 +150,7 @@
   {{else}}
     <h2 data-test-title class="title is-3">Sign intermediate</h2>
     <NamespaceReminder @mode="save" @noun="PKI change" />
-    <MessageError @model={{this.model}} />
+    <MessageError @model={{this.model}} @errors={{this.errors}} />
     <form {{action "saveCA" on="submit"}} data-test-sign-intermediate-form="true">
       <FormFieldGroupsLoop @model={{this.model}} @mode={{this.mode}} />
       <div class="field is-grouped box is-fullwidth is-bottomless">

ui/app/templates/components/config-pki.hbs

@@ -14,7 +14,7 @@
   </p>
 {{/if}}
 
-<MessageError @model={{this.config}} />
+<MessageError @model={{this.config}} @errors={{this.errors}} />
 <form {{action "save" this.section on="submit"}} class="box is-shadowless is-marginless is-fullwidth has-slim-padding">
   {{#each (get this.config (concat this.section "Attrs")) as |attr|}}
     <FormField @attr={{attr}} @model={{this.config}} @data-test-field={{attr.name}} />

ui/app/templates/components/mount-backend-form.hbs

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

ui/app/templates/vault/cluster/policies/create.hbs

@@ -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={{this.model}} @errors={{this.model.errors}} />
     <NamespaceReminder @mode="create" @noun="policy" />
     <div class="field">
       <label for="policy-name" class="is-label">Name</label>

ui/lib/core/addon/components/message-error.js

@@ -1,6 +1,6 @@
-import { computed } from '@ember/object';
+import Component from '@glimmer/component';
-import Component from '@ember/component';
 import layout from '../templates/components/message-error';
+import { setComponentTemplate } from '@ember/component';
 
 /**
  * @module MessageError
@@ -11,48 +11,37 @@ 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 {array} [errors=null] - An array of error strings to show.
- * @param errorMessage=null{String} - An Error string to display.
+ * @param {string} [errorMessage=null] - An Error string to display.
  */
-export default Component.extend({
-  layout,
-  model: null,
-  errors: null,
-  errorMessage: null,
 
-  displayErrors: computed(
+class MessageError extends Component {
-    'errorMessage',
+  get displayErrors() {
-    'model.{isError,adapterError.message,adapterError.errors.@each}',
+    let { errorMessage, errors, model } = this.args;
-    'errors',
+    if (errorMessage) {
-    'errors.[]',
+      return [errorMessage];
-    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';
     }
-  ),
+
-});
+    if (errors && errors.length > 0) {
+      return errors;
+    }
+
+    if (model?.isError) {
+      let adapterError = 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);

ui/lib/core/addon/templates/components/message-error.hbs

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

ui/tests/acceptance/client-current-test.js

@@ -112,9 +112,8 @@ module('Acceptance | clients current', function (hooks) {
     // FILTER BY NAMESPACE
     await clickTrigger();
     await searchSelect.options.objectAt(0).click();
-    await waitUntil(() => {
+    await settled();
-      return find('[data-test-horizontal-bar-chart]');
+    await waitUntil(() => find('[data-test-horizontal-bar-chart]'));
-    });
     assert.dom('[data-test-stat-text="total-clients"] .stat-value').hasText('15');
     assert.dom('[data-test-stat-text="entity-clients"] .stat-value').hasText('5');
     assert.dom('[data-test-stat-text="non-entity-clients"] .stat-value').hasText('10');
@@ -145,18 +144,14 @@ module('Acceptance | clients current', function (hooks) {
     // FILTER BY AUTH METHOD
     await clickTrigger();
     await searchSelect.options.objectAt(0).click();
-    await waitUntil(() => {
+    await waitUntil(() => find('#auth-method-search-select'));
-      return find('#auth-method-search-select');
-    });
     assert.dom('[data-test-stat-text="total-clients"] .stat-value').hasText('5');
     assert.dom('[data-test-stat-text="entity-clients"] .stat-value').hasText('3');
     assert.dom('[data-test-stat-text="non-entity-clients"] .stat-value').hasText('2');
     assert.dom(SELECTORS.attributionBlock).doesNotExist('Does not show attribution block');
     // Delete auth filter goes back to filtered only on namespace
     await click('#auth-method-search-select [data-test-selected-list-button="delete"]');
-    await waitUntil(() => {
+    await waitUntil(() => find('[data-test-horizontal-bar-chart]'));
-      return find('[data-test-horizontal-bar-chart]');
-    });
     assert.dom('[data-test-stat-text="total-clients"] .stat-value').hasText('15');
     assert.dom('[data-test-stat-text="entity-clients"] .stat-value').hasText('5');
     assert.dom('[data-test-stat-text="non-entity-clients"] .stat-value').hasText('10');

ui/tests/acceptance/settings/configure-secret-backends/pki/section-urls-test.js

@@ -1,4 +1,4 @@
-import { currentRouteName, settled } from '@ember/test-helpers';
+import { currentRouteName, settled, find, waitUntil } from '@ember/test-helpers';
 import { module, test } from 'qunit';
 import { setupApplicationTest } from 'ember-qunit';
 import page from 'vault/tests/pages/settings/configure-secret-backends/pki/section';
@@ -22,8 +22,7 @@ module('Acceptance | settings/configure/secrets/pki/urls', function (hooks) {
 
     await page.form.fields.objectAt(0).textarea('foo').change();
     await page.form.submit();
-    await settled();
+    await waitUntil(() => find('[data-test-error]'));
-
     assert.ok(page.form.hasError, 'shows error on invalid input');
 
     await page.form.fields.objectAt(0).textarea('foo.example.com').change();

ui/tests/acceptance/settings/mount-secret-backend-test.js

@@ -89,7 +89,7 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) {
     await page.enableEngine();
     await page.selectType('kv');
     await page.next().path(path).submit();
-    assert.dom('.alert-banner-message-body').hasText('This mount path already exist.');
+    assert.dom('.alert-banner-message-body').containsText(`path is already in use at ${path}`);
     assert.equal(currentRouteName(), 'vault.cluster.settings.mount-secret-backend');
 
     await page.secretList();

ui/tests/integration/components/auth-form-test.js

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

ui/tests/integration/components/ttl-picker-test.js

@@ -17,11 +17,10 @@ module('Integration | Component | ttl picker', function (hooks) {
 
     let callCount = this.changeSpy.callCount;
     await fillIn('[data-test-ttl-value]', 'foo');
-    assert.equal(this.changeSpy.callCount, callCount, "it did't call onChange again");
+    assert.equal(this.changeSpy.callCount, callCount, "it didn't call onChange again");
     assert.dom('[data-test-ttl-error]').includesText('Error', 'renders the error box');
-
     await fillIn('[data-test-ttl-value]', '33');
-    assert.dom('[data-test-ttl-error]').doesNotIncludeText('Error', 'removes the error box');
+    assert.dom('[data-test-ttl-error]').doesNotExist('removes the error box');
   });
 
   test('it shows 30s for invalid duration initialValue input', async function (assert) {

ui/tests/pages/components/auth-form.js

@@ -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]', {
@@ -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]'),
 };