Refactor SSH Configuration workflow (#28122)

* initial copy from other #28004

* pr feedback

* grr

ui/app/adapters/secret-engine.js

@@ -87,13 +87,6 @@ export default ApplicationAdapter.extend({
     }
   },
 
-  findRecord(store, type, path, snapshot) {
-    if (snapshot.attr('type') === 'ssh') {
-      return this.ajax(`/v1/${encodePath(path)}/config/ca`, 'GET');
-    }
-    return { data: {} };
-  },
-
   queryRecord(store, type, query) {
     if (query.type === 'aws') {
       return this.ajax(`/v1/${encodePath(query.backend)}/config/lease`, 'GET').then((resp) => {

ui/app/adapters/ssh/ca-config.js

@@ -8,13 +8,39 @@ import { encodePath } from 'vault/utils/path-encoding-helpers';
 
 export default class SshCaConfig extends ApplicationAdapter {
   namespace = 'v1';
-  // For now this is only being used on the vault.cluster.secrets.backend.configuration route. This is a read-only route.
+
-  // Eventually, this will be used to create the ca config for the SSH secret backend, replacing the requests located on the secret-engine adapter.
   queryRecord(store, type, query) {
     const { backend } = query;
     return this.ajax(`${this.buildURL()}/${encodePath(backend)}/config/ca`, 'GET').then((resp) => {
       resp.id = backend;
+      resp.backend = backend;
       return resp;
     });
   }
+
+  createOrUpdate(store, type, snapshot) {
+    const serializer = store.serializerFor(type.modelName);
+    const data = serializer.serialize(snapshot);
+    const backend = snapshot.record.backend;
+    return this.ajax(`${this.buildURL()}/${backend}/config/ca`, 'POST', { data }).then((resp) => {
+      // ember data requires an id on the response
+      return {
+        ...resp,
+        id: backend,
+      };
+    });
+  }
+
+  createRecord() {
+    return this.createOrUpdate(...arguments);
+  }
+
+  updateRecord() {
+    return this.createOrUpdate(...arguments);
+  }
+
+  deleteRecord(store, type, snapshot) {
+    const backend = snapshot.record.backend;
+    return this.ajax(`${this.buildURL()}/${backend}/config/ca`, 'DELETE');
+  }
 }

ui/app/components/secret-engine/configuration-details.hbs

@@ -6,13 +6,14 @@
 {{#if @configModels.length}}
   {{#each @configModels as |configModel|}}
     {{#each configModel.attrs as |attr|}}
-      {{#if attr.options.sensitive}}
+      {{! public key while not sensitive when editing/creating, should be hidden by default on viewing }}
+      {{#if (or attr.options.sensitive (eq attr.name "publicKey"))}}
         <InfoTableRow
           alwaysRender={{not (is-empty-value (get configModel attr.name))}}
           @label={{or attr.options.label (to-label attr.name)}}
           @value={{get configModel (or attr.options.fieldValue attr.name)}}
         >
-          {{#if attr.options.sensitive}}
+          {{#if (or attr.options.sensitive (eq attr.name "publicKey"))}}
             <MaskedInput
               @value={{get configModel attr.name}}
               @name={{attr.name}}

ui/app/components/secret-engine/configure-ssh.hbs

@@ -3,7 +3,51 @@
   SPDX-License-Identifier: BUSL-1.1
 ~}}
 
-{{#if @configured}}
+<form {{on "submit" (perform this.save)}} aria-label="save ssh creds" data-test-configure-form>
+  <div class="box is-fullwidth is-shadowless is-marginless">
+    <NamespaceReminder @mode="save" @noun="configuration" />
+    <MessageError @errorMessage={{this.errorMessage}} />
+    {{#unless @model.isNew}}
+      <p class="has-text-grey-dark">
+        NOTE: You must delete your existing certificate and key before saving new values.
+      </p>
+    {{/unless}}
+  </div>
+  {{#if @model.isNew}}
+    <div class="box is-fullwidth is-sideless">
+      {{#each @model.formFields as |attr|}}
+        <FormField @attr={{attr}} @model={{@model}} @modelValidations={{this.modelValidations}} />
+      {{/each}}
+    </div>
+    <div class="box is-fullwidth is-bottomless">
+      <div class="control">
+        <Hds::Button
+          @text="Save"
+          @icon={{if this.save.isRunning "loading"}}
+          type="submit"
+          disabled={{this.save.isRunning}}
+          data-test-configure-save-button
+        />
+        <Hds::Button
+          @text="Cancel"
+          @color="secondary"
+          class="has-left-margin-s"
+          disabled={{this.save.isRunning}}
+          {{on "click" this.onCancel}}
+          data-test-cancel-button
+        />
+      </div>
+      {{#if this.invalidFormAlert}}
+        <AlertInline
+          data-test-invalid-form-alert
+          class="has-top-padding-s"
+          @type="danger"
+          @message={{this.invalidFormAlert}}
+        />
+      {{/if}}
+    </div>
+  {{else}}
+    {{! Model is not new and keys have already been created. Require user deletes the keys before creating new ones }}
     <div class="box is-fullwidth is-sideless is-marginless" data-test-edit-config-section>
       <div class="field">
         <label for="publicKey" class="is-label">
@@ -26,65 +70,17 @@
         <Hds::Copy::Button
           @text="Copy"
           @textToCopy={{@model.publicKey}}
-        @onError={{(fn (set-flash-message "Clipboard copy failed. The Clipboard API requires a secure context." "danger"))}}
+          @onError={{fn (set-flash-message "Clipboard copy failed. The Clipboard API requires a secure context." "danger")}}
           class="primary"
         />
         <ConfirmAction
           @buttonText="Delete"
           @buttonColor="secondary"
-        @confirmMessage="This will remove the CA certificate information."
+          @confirmMessage="Confirming will remove the CA certificate information."
-        @onConfirmAction={{this.delete}}
+          @onConfirmAction={{this.deleteCaConfig}}
           data-test-delete-public-key
         />
       </Hds::ButtonSet>
     </div>
-{{else}}
-  <form {{on "submit" this.saveConfig}} data-test-configure-form>
-    <div class="box is-fullwidth is-sideless is-marginless">
-      <NamespaceReminder @mode="save" @noun="configuration" />
-      <div class="field">
-        <label for="privateKey" class="is-label">
-          Private key
-        </label>
-        <div class="control">
-          <MaskedInput @name="privateKey" id="privateKey" @value={{@model.privateKey}} @onChange={{mut @model.privateKey}} />
-        </div>
-      </div>
-      <div class="field">
-        <label for="publicKey" class="is-label">
-          Public key
-        </label>
-        <div class="control">
-          <Textarea name="publicKey" id="publicKey" class="input" @value={{@model.publicKey}} data-test-input="publicKey" />
-        </div>
-      </div>
-      <div class="b-checkbox">
-        <Input
-          @type="checkbox"
-          id="generateSigningKey"
-          class="styled"
-          @checked={{@model.generateSigningKey}}
-          {{on "change" (fn (mut @model.generateSigningKey) (not @model.generateSigningKey))}}
-          data-test-input="generate-signing-key-checkbox"
-        />
-        <label for="generateSigningKey" class="is-label">
-          Generate signing key
-          <InfoTooltip>
-            Specifies if Vault should generate the signing key pair internally
-          </InfoTooltip>
-        </label>
-      </div>
-    </div>
-    <div class="field box is-fullwidth is-bottomless is-marginless">
-      <div class="control">
-        <Hds::Button
-          @text="Save"
-          @icon={{if @loading "loading"}}
-          type="submit"
-          disabled={{@loading}}
-          data-test-configure-save-button
-        />
-      </div>
-    </div>
-  </form>
   {{/if}}
+</form>

ui/app/components/secret-engine/configure-ssh.js

@@ -1,38 +0,0 @@
-/**
- * Copyright (c) HashiCorp, Inc.
- * SPDX-License-Identifier: BUSL-1.1
- */
-
-import Component from '@glimmer/component';
-import { action } from '@ember/object';
-
-/**
- * @module ConfigureSshSComponent
- *
- * @example
- * ```js
- * <SecretEngine::ConfigureSsh
- *    @model={{this.model}}
- *    @configured={{this.configured}}
- *    @saveConfig={{action "saveConfig"}}
- *    @loading={{this.loading}}
- *  />
- * ```
- *
- * @param {string} model - ssh secret engine model
- * @param {Function} saveConfig - parent action which updates the configuration
- * @param {boolean} loading - property in parent that updates depending on status of parent's action
- *
- */
-export default class ConfigureSshComponent extends Component {
-  @action
-  delete() {
-    this.args.saveConfig({ delete: true });
-  }
-
-  @action
-  saveConfig(event) {
-    event.preventDefault();
-    this.args.saveConfig({ delete: false });
-  }
-}

ui/app/components/secret-engine/configure-ssh.ts

@@ -0,0 +1,118 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: BUSL-1.1
+ */
+
+import Component from '@glimmer/component';
+import { action } from '@ember/object';
+import { task } from 'ember-concurrency';
+import { waitFor } from '@ember/test-waiters';
+import { service } from '@ember/service';
+import { tracked } from '@glimmer/tracking';
+import { ValidationMap } from 'vault/vault/app-types';
+import errorMessage from 'vault/utils/error-message';
+
+import type CaConfigModel from 'vault/models/ssh/ca-config';
+import type Router from '@ember/routing/router';
+import type Store from '@ember-data/store';
+import type FlashMessageService from 'vault/services/flash-messages';
+
+/**
+ * @module ConfigureSshComponent is used to configure the SSH secret engine.
+ *
+ * @example
+ * ```js
+ * <SecretEngine::ConfigureSsh
+ *    @model={{this.model.ssh-ca-config}}
+ *    @id={{this.model.id}}
+ *  />
+ * ```
+ *
+ * @param {string} model - SSH ca-config model
+ * @param {string} id - name of the SSH secret engine, ex: 'ssh-123'
+ */
+
+interface Args {
+  model: CaConfigModel;
+  id: string;
+}
+
+export default class ConfigureSshComponent extends Component<Args> {
+  @service declare readonly router: Router;
+  @service declare readonly store: Store;
+  @service declare readonly flashMessages: FlashMessageService;
+
+  @tracked errorMessage: string | null = null;
+  @tracked invalidFormAlert: string | null = null;
+  @tracked modelValidations: ValidationMap | null = null;
+
+  @task
+  @waitFor
+  *save(event: Event) {
+    event.preventDefault();
+    this.resetErrors();
+    const { id, model } = this.args;
+    const isValid = this.validate(model);
+
+    if (!isValid) return;
+    // Check if any of the model's attributes have changed.
+    // If no changes to the model, transition and notify user.
+    // Otherwise, save the model.
+    const attributesChanged = Object.keys(model.changedAttributes()).length > 0;
+    if (!attributesChanged) {
+      this.flashMessages.info('No changes detected.');
+      this.transition();
+    }
+
+    try {
+      yield model.save();
+      this.transition();
+      this.flashMessages.success(`Successfully saved ${id}'s root configuration.`);
+    } catch (error) {
+      this.errorMessage = errorMessage(error);
+      this.invalidFormAlert = 'There was an error submitting this form.';
+    }
+  }
+
+  validate(model: CaConfigModel) {
+    const { isValid, state, invalidFormMessage } = model.validate();
+    this.modelValidations = isValid ? null : state;
+    this.invalidFormAlert = isValid ? '' : invalidFormMessage;
+    return isValid;
+  }
+
+  resetErrors() {
+    this.flashMessages.clearMessages();
+    this.errorMessage = null;
+    this.invalidFormAlert = null;
+  }
+
+  transition(isDelete = false) {
+    // deleting a key is the only case in which we want to stay on the create/edit page.
+    if (isDelete) {
+      this.router.transitionTo('vault.cluster.secrets.backend.configuration.edit', this.args.id);
+    } else {
+      this.router.transitionTo('vault.cluster.secrets.backend.configuration', this.args.id);
+    }
+  }
+
+  @action
+  onCancel() {
+    // clear errors because they're canceling out of the workflow.
+    this.resetErrors();
+    this.transition();
+  }
+
+  @action
+  async deleteCaConfig() {
+    const { model } = this.args;
+    try {
+      await model.destroyRecord();
+      this.transition(true);
+      this.flashMessages.success('CA information deleted successfully.');
+    } catch (error) {
+      model.rollbackAttributes();
+      this.flashMessages.danger(errorMessage(error));
+    }
+  }
+}

ui/app/controllers/vault/cluster/secrets/backend/configuration/edit.js

@@ -29,31 +29,6 @@ export default Controller.extend(CONFIG_ATTRS, {
     this.setProperties(CONFIG_ATTRS);
   },
   actions: {
-    saveConfig(options = { delete: false }) {
-      const isDelete = options.delete;
-      if (this.model.type === 'ssh') {
-        this.set('loading', true);
-        this.model
-          .saveCA({ isDelete })
-          .then(() => {
-            this.send('refreshRoute');
-            this.set('configured', !isDelete);
-            if (isDelete) {
-              this.flashMessages.success('SSH Certificate Authority Configuration deleted!');
-            } else {
-              this.flashMessages.success('SSH Certificate Authority Configuration saved!');
-            }
-          })
-          .catch((error) => {
-            const errorMessage = error.errors ? error.errors.join('. ') : error;
-            this.flashMessages.danger(errorMessage);
-          })
-          .finally(() => {
-            this.set('loading', false);
-          });
-      }
-    },
-
     save(method, data) {
       this.set('loading', true);
       const hasData = Object.keys(data).some((key) => {

ui/app/models/secret-engine.js

@@ -68,14 +68,6 @@ export default class SecretEngineModel extends Model {
   })
   version;
 
-  // SSH specific attributes
-  @attr('string') privateKey;
-  @attr('string') publicKey;
-  @attr('boolean', {
-    defaultValue: true,
-  })
-  generateSigningKey;
-
   // AWS specific attributes
   @attr('string') lease;
   @attr('string') leaseMax;
@@ -257,24 +249,6 @@ export default class SecretEngineModel extends Model {
   }
 
   /* ACTIONS */
-  saveCA(options) {
-    if (this.type !== 'ssh') {
-      return;
-    }
-    if (options.isDelete) {
-      this.privateKey = null;
-      this.publicKey = null;
-      this.generateSigningKey = false;
-    }
-    return this.save({
-      adapterOptions: {
-        options: options,
-        apiPath: 'config/ca',
-        attrsToSend: ['privateKey', 'publicKey', 'generateSigningKey'],
-      },
-    });
-  }
-
   saveZeroAddressConfig() {
     return this.save({
       adapterOptions: {

ui/app/models/ssh/ca-config.js

@@ -5,16 +5,50 @@
 
 import Model, { attr } from '@ember-data/model';
 import { expandAttributeMeta } from 'vault/utils/field-to-attrs';
+import { withModelValidations } from 'vault/decorators/model-validations';
 
+const validations = {
+  generateSigningKey: [
+    {
+      validator(model) {
+        const { publicKey, privateKey, generateSigningKey } = model;
+        // if generateSigningKey is false, both public and private keys are required
+        if (!generateSigningKey && (!publicKey || !privateKey)) {
+          return false;
+        }
+        return true;
+      },
+      message: 'Provide a Public and Private key or set "Generate Signing Key" to true.',
+    },
+  ],
+  publicKey: [
+    {
+      validator(model) {
+        const { publicKey, privateKey } = model;
+        // regardless of generateSigningKey, if one key is set they both need to be set.
+        return publicKey || privateKey ? publicKey && privateKey : true;
+      },
+      message: 'You must provide a Public and Private keys or leave both unset.',
+    },
+  ],
+};
+// there are more options available on the API, but the UI does not support them yet.
+@withModelValidations(validations)
 export default class SshCaConfig extends Model {
   @attr('string') backend; // dynamic path of secret -- set on response from value passed to queryRecord
   @attr('string', { sensitive: true }) privateKey; // obfuscated, never returned by API
-  @attr('string', { sensitive: true }) publicKey;
+  @attr('string') publicKey;
   @attr('boolean', { defaultValue: true })
   generateSigningKey;
-  // there are more options available on the API, but the UI does not support them yet.
+
+  // do not return private key for configuration.index view
   get attrs() {
     const keys = ['publicKey', 'generateSigningKey'];
     return expandAttributeMeta(this, keys);
   }
+  // return private key for edit/create view
+  get formFields() {
+    const keys = ['privateKey', 'publicKey', 'generateSigningKey'];
+    return expandAttributeMeta(this, keys);
+  }
 }

ui/app/routes/vault/cluster/secrets/backend/configuration/edit.js

@@ -1,70 +0,0 @@
-/**
- * Copyright (c) HashiCorp, Inc.
- * SPDX-License-Identifier: BUSL-1.1
- */
-
-import AdapterError from '@ember-data/adapter/error';
-import { set } from '@ember/object';
-import Route from '@ember/routing/route';
-import { service } from '@ember/service';
-import { CONFIGURABLE_SECRET_ENGINES } from 'vault/helpers/mountable-secret-engines';
-
-export default Route.extend({
-  store: service(),
-
-  model() {
-    const { backend } = this.paramsFor('vault.cluster.secrets.backend');
-    return this.store.query('secret-engine', { path: backend }).then((modelList) => {
-      const model = modelList && modelList[0];
-      if (!model || !CONFIGURABLE_SECRET_ENGINES.includes(model.type)) {
-        const error = new AdapterError();
-        set(error, 'httpStatus', 404);
-        throw error;
-      }
-      return this.store.findRecord('secret-engine', backend).then(
-        () => {
-          return model;
-        },
-        () => {
-          return model;
-        }
-      );
-    });
-  },
-
-  afterModel(model) {
-    const type = model.type;
-
-    if (type === 'aws') {
-      return this.store
-        .queryRecord('secret-engine', {
-          backend: model.id,
-          type,
-        })
-        .then(
-          () => model,
-          () => model
-        );
-    }
-    return model;
-  },
-
-  setupController(controller, model) {
-    if (model.publicKey) {
-      controller.set('configured', true);
-    }
-    return this._super(...arguments);
-  },
-
-  resetController(controller, isExiting) {
-    if (isExiting) {
-      controller.reset();
-    }
-  },
-
-  actions: {
-    refreshRoute() {
-      this.refresh();
-    },
-  },
-});

ui/app/routes/vault/cluster/secrets/backend/configuration/edit.ts

@@ -0,0 +1,109 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: BUSL-1.1
+ */
+
+import AdapterError from '@ember-data/adapter/error';
+import { set } from '@ember/object';
+import Route from '@ember/routing/route';
+import { service } from '@ember/service';
+import { CONFIGURABLE_SECRET_ENGINES } from 'vault/helpers/mountable-secret-engines';
+import errorMessage from 'vault/utils/error-message';
+import { action } from '@ember/object';
+
+import type Store from '@ember-data/store';
+import type SecretEngineModel from 'vault/models/secret-engine';
+
+// This route file is reused for all configurable secret engines.
+// It generates config models based on the engine type.
+// Saving and updating of those models are done within the engine specific components.
+
+const CONFIG_ADAPTERS_PATHS: Record<string, string[]> = {
+  // aws: ['aws/lease-config', 'aws/root-config'], TODO will be uncommented when AWS refactor occurs
+  ssh: ['ssh/ca-config'],
+};
+
+export default class SecretsBackendConfigurationEdit extends Route {
+  @service declare readonly store: Store;
+
+  async model() {
+    const { backend } = this.paramsFor('vault.cluster.secrets.backend');
+    const secretEngineRecord = this.modelFor('vault.cluster.secrets.backend') as { type: SecretEngineModel };
+    const type = secretEngineRecord.type as string;
+
+    // if the engine type is not configurable, return a 404.
+    if (!secretEngineRecord || !CONFIGURABLE_SECRET_ENGINES.includes(type)) {
+      const error = new AdapterError();
+      set(error, 'httpStatus', 404);
+      throw error;
+    }
+    // TODO this conditional will be removed when we handle AWS
+    if (type !== 'aws') {
+      // generate the model based on the engine type.
+      // and pre-set with the type and backend (e.g. type: ssh, id: ssh-123)
+      const model: Record<string, unknown> = { type, id: backend };
+      for (const adapterPath of CONFIG_ADAPTERS_PATHS[type] as string[]) {
+        // convert the adapterPath with a name that can be passed to the components
+        // ex: adapterPath = ssh/ca-config, convert to: ssh-ca-config so that you can pass to component @model={{this.model.ssh-ca-config}}
+        const standardizedKey = adapterPath.replace(/\//g, '-');
+        try {
+          model[standardizedKey] = await this.store.queryRecord(adapterPath, {
+            backend,
+            type,
+          });
+        } catch (e: AdapterError) {
+          // For most models if the adapter returns a 404, we want to create a new record.
+          // The ssh secret engine however returns a 400 if the CA is not configured.
+          // For ssh's 400 error, we want to create the CA config model.
+          if (
+            e.httpStatus === 404 ||
+            (type === 'ssh' && e.httpStatus === 400 && errorMessage(e) === `keys haven't been configured yet`)
+          ) {
+            model[standardizedKey] = await this.store.createRecord(adapterPath, {
+              backend,
+              type,
+            });
+          } else {
+            throw e;
+          }
+        }
+      }
+      return model;
+    } else {
+      // TODO for now AWS configs rely on the secret-engine model and adapter. This will be refactored.
+      return await this.store.findRecord('secret-engine', backend);
+    }
+  }
+
+  // TODO everything below line will be removed once we handle AWS. This is the old code wrapped in AWS conditionals when appropriate.
+  afterModel(model: Record<string, unknown>) {
+    const type = model.type;
+
+    if (type === 'aws') {
+      return this.store
+        .queryRecord('secret-engine', {
+          backend: model.id,
+          type,
+        })
+        .then(
+          () => model,
+          () => model
+        );
+    }
+    return model;
+  }
+
+  resetController(controller, isExiting) {
+    if (controller.model.type === 'aws') {
+      if (isExiting) {
+        controller.reset();
+      }
+    }
+  }
+
+  @action
+  willTransition() {
+    // catch the transition and refresh model so the route shows the most recent model data.
+    this.refresh();
+  }
+}

ui/app/templates/vault/cluster/secrets/backend/configuration/edit.hbs

@@ -41,12 +41,5 @@
     @saveAWSLease={{action "save" "saveAWSLease"}}
   />
 {{else if (eq this.model.type "ssh")}}
-  <SecretEngine::ConfigureSsh
+  <SecretEngine::ConfigureSsh @model={{this.model.ssh-ca-config}} @id={{this.model.id}} />
-    @model={{this.model}}
-    @configured={{this.configured}}
-    @saveConfig={{action "saveConfig"}}
-    @loading={{this.loading}}
-  />
 {{/if}}
-
-{{outlet}}

ui/tests/acceptance/secrets/backend/ssh/configuration-test.js

@@ -3,11 +3,10 @@
  * SPDX-License-Identifier: BUSL-1.1
  */
 
-import { click, fillIn, currentURL, waitFor, visit } from '@ember/test-helpers';
+import { click, fillIn, currentURL, visit, waitFor } from '@ember/test-helpers';
 import { module, test } from 'qunit';
 import { setupApplicationTest } from 'ember-qunit';
 import { v4 as uuidv4 } from 'uuid';
-import { spy } from 'sinon';
 
 import authPage from 'vault/tests/pages/auth';
 import enablePage from 'vault/tests/pages/settings/mount-secret-backend';
@@ -23,10 +22,7 @@ module('Acceptance | ssh | configuration', function (hooks) {
   setupMirage(hooks);
 
   hooks.beforeEach(function () {
-    const flash = this.owner.lookup('service:flash-messages');
-    this.flashDangerSpy = spy(flash, 'danger');
     this.store = this.owner.lookup('service:store');
-
     this.uid = uuidv4();
     return authPage.login();
   });
@@ -70,18 +66,20 @@ module('Acceptance | ssh | configuration', function (hooks) {
     await click(SES.ssh.save);
     assert.strictEqual(
       currentURL(),
-      `/vault/secrets/${sshPath}/configuration/edit`,
+      `/vault/secrets/${sshPath}/configuration`,
-      'stays on configuration form page.'
+      'navigates to the details page.'
     );
     // There is a delay in the backend for the public key to be generated, wait for it to complete by checking that the public key is displayed
-    await waitFor(GENERAL.inputByAttr('public-key'));
+    await waitFor(GENERAL.infoRowLabel('Public key'));
-    assert.dom(GENERAL.inputByAttr('public-key')).hasText('***********', 'public key is masked');
+    assert.dom(GENERAL.infoRowLabel('Public key')).exists('public key shown on the details screen');
+
+    await click(SES.configure);
     assert
       .dom(SES.ssh.editConfigSection)
       .exists('renders the edit configuration section of the form and not the create part');
     // delete Public key
-    await click(SES.ssh.deletePublicKey);
+    await click(SES.ssh.delete);
-    assert.dom(GENERAL.confirmMessage).hasText('This will remove the CA certificate information.');
+    assert.dom(GENERAL.confirmMessage).hasText('Confirming will remove the CA certificate information.');
     await click(GENERAL.confirmButton);
     assert.strictEqual(
       currentURL(),
@@ -90,9 +88,7 @@ module('Acceptance | ssh | configuration', function (hooks) {
     );
     assert.dom(GENERAL.maskedInput('privateKey')).hasNoText('Private key is empty and reset');
     assert.dom(GENERAL.inputByAttr('publicKey')).hasNoText('Public key is empty and reset');
-    assert
+    assert.dom(GENERAL.inputByAttr('generateSigningKey')).isChecked('Generate signing key is checked');
-      .dom(GENERAL.inputByAttr('generate-signing-key-checkbox'))
-      .isNotChecked('Generate signing key is unchecked');
     await click(SES.viewBackend);
     await click(SES.configTab);
     assert
@@ -107,15 +103,15 @@ module('Acceptance | ssh | configuration', function (hooks) {
     await enablePage.enable('ssh', path);
     await click(SES.configTab);
     await click(SES.configure);
-    assert
+    assert.dom(GENERAL.inputByAttr('generateSigningKey')).isChecked('generate_signing_key defaults to true');
-      .dom(GENERAL.inputByAttr('generate-signing-key-checkbox'))
+    await click(GENERAL.inputByAttr('generateSigningKey'));
-      .isChecked('generate_signing_key defaults to true');
-    await click(GENERAL.inputByAttr('generate-signing-key-checkbox'));
     await click(SES.ssh.save);
-    assert.true(this.flashDangerSpy.calledWith('missing public_key'), 'Danger flash message is displayed');
+    assert
+      .dom(GENERAL.inlineError)
+      .hasText('Provide a Public and Private key or set "Generate Signing Key" to true.');
     // visit the details page and confirm the public key is not shown
     await visit(`/vault/secrets/${path}/configuration`);
-    assert.dom(GENERAL.infoRowLabel('Public key')).doesNotExist('Public Key label does not exist');
+    assert.dom(GENERAL.infoRowLabel('Public key')).doesNotExist('Public key label does not exist');
     assert.dom(GENERAL.emptyStateTitle).hasText('SSH not configured', 'SSH not configured');
     // cleanup
     await runCmd(`delete sys/mounts/${path}`);

ui/tests/acceptance/secrets/backend/ssh/roles-test.js

@@ -3,7 +3,16 @@
  * SPDX-License-Identifier: BUSL-1.1
  */
 
-import { click, fillIn, currentURL, find, settled, waitUntil, currentRouteName } from '@ember/test-helpers';
+import {
+  click,
+  fillIn,
+  currentURL,
+  find,
+  settled,
+  waitUntil,
+  currentRouteName,
+  waitFor,
+} from '@ember/test-helpers';
 import { module, test } from 'qunit';
 import { setupApplicationTest } from 'ember-qunit';
 import { v4 as uuidv4 } from 'uuid';
@@ -102,8 +111,9 @@ module('Acceptance | ssh | roles', function (hooks) {
     await click(SES.configure);
     // default has generate CA checked so we just submit the form
     await click(SES.ssh.save);
-    await click(SES.viewBackend);
+    // There is a delay in the backend for the public key to be generated, wait for it to complete by checking that the public key is displayed
-
+    await waitFor(GENERAL.infoRowLabel('Public key'));
+    await click(GENERAL.tab(sshPath));
     for (const role of ROLES) {
       // create a role
       await click(SES.createSecret);

ui/tests/helpers/secret-engine/secret-engine-helpers.js

@@ -48,11 +48,13 @@ const createAwsLeaseConfig = (store, backend) => {
 };
 
 const createSshCaConfig = (store, backend) => {
+  // consider this model a placeholder for the actual ssh/ca-config model that has been generated with data. isNew is false.
   store.pushPayload('ssh/ca-config', {
     id: backend,
     modelName: 'ssh/ca-config',
     data: {
       backend,
+      public_key: '123456',
       generate_signing_key: true,
     },
   });

ui/tests/helpers/secret-engine/secret-engine-selectors.ts

@@ -32,8 +32,9 @@ export const SECRET_ENGINE_SELECTORS = {
   ssh: {
     configureForm: '[data-test-configure-form]',
     editConfigSection: '[data-test-edit-config-section]',
-    deletePublicKey: '[data-test-delete-public-key]',
     save: '[data-test-configure-save-button]',
+    cancel: '[data-test-cancel-button]',
+    delete: '[data-test-delete-public-key]',
     createRole: '[data-test-role-ssh-create]',
     deleteRole: '[data-test-ssh-role-delete]',
   },

ui/tests/integration/components/secret-engine/configuration-details-test.js

@@ -33,7 +33,7 @@ module('Integration | Component | SecretEngine/configuration-details', function
   });
 
   test('it shows config details if configModel(s) are passed in', async function (assert) {
-    assert.expect(14);
+    assert.expect(21);
     for (const type of CONFIGURABLE_SECRET_ENGINES) {
       const backend = `test-${type}`;
       this.configModels = createConfig(this.store, backend, type);
@@ -45,6 +45,12 @@ module('Integration | Component | SecretEngine/configuration-details', function
         assert
           .dom(GENERAL.infoRowValue(key))
           .hasText(responseKeyAndValue, `${key} value for the ${type} config details exists.`);
+        // make sure the ones that should be masked are masked, and others are not.
+        if (key === 'private_key' || key === 'public_key') {
+          assert.dom(GENERAL.infoRowValue(key)).hasClass('masked-input', `${key} is masked`);
+        } else {
+          assert.dom(GENERAL.infoRowValue(key)).doesNotHaveClass('masked-input', `${key} is not masked`);
+        }
       }
     }
   });

ui/tests/integration/components/secret-engine/configure-ssh-test.js

@@ -5,66 +5,114 @@
 
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'vault/tests/helpers';
-import { render, click } from '@ember/test-helpers';
+import { render, click, fillIn } from '@ember/test-helpers';
 import { hbs } from 'ember-cli-htmlbars';
 import { GENERAL } from 'vault/tests/helpers/general-selectors';
 import { SECRET_ENGINE_SELECTORS as SES } from 'vault/tests/helpers/secret-engine/secret-engine-selectors';
 import { createConfig } from 'vault/tests/helpers/secret-engine/secret-engine-helpers';
+import { setupMirage } from 'ember-cli-mirage/test-support';
 import sinon from 'sinon';
 
 module('Integration | Component | SecretEngine/configure-ssh', function (hooks) {
   setupRenderingTest(hooks);
+  setupMirage(hooks);
 
   hooks.beforeEach(function () {
     this.store = this.owner.lookup('service:store');
-    this.model = createConfig(this.store, 'ssh-test', 'ssh');
+    const router = this.owner.lookup('service:router');
-    this.saveConfig = sinon.stub();
+    this.id = 'ssh-test';
+    this.model = this.store.createRecord('ssh/ca-config', { backend: this.id });
+    this.transitionStub = sinon.stub(router, 'transitionTo');
+    this.refreshStub = sinon.stub(router, 'refresh');
   });
 
   test('it shows create fields if not configured', async function (assert) {
     await render(hbs`
       <SecretEngine::ConfigureSsh
     @model={{this.model}}
-    @configured={{false}}
+    @id={{this.id}}
-    @saveConfig={{this.saveConfig}}
-    @loading={{false}}
   />
     `);
     assert.dom(GENERAL.maskedInput('privateKey')).hasNoText('Private key is empty and reset');
     assert.dom(GENERAL.inputByAttr('publicKey')).hasNoText('Public key is empty and reset');
     assert
-      .dom(GENERAL.inputByAttr('generate-signing-key-checkbox'))
+      .dom(GENERAL.inputByAttr('generateSigningKey'))
       .isChecked('Generate signing key is checked by default');
   });
 
-  test('it calls save with correct arg', async function (assert) {
+  test('it should go back to parent route on cancel', async function (assert) {
     await render(hbs`
       <SecretEngine::ConfigureSsh
     @model={{this.model}}
-    @configured={{false}}
+    @id={{this.id}}
-    @saveConfig={{this.saveConfig}}
-    @loading={{false}}
   />
     `);
-    await click(SES.ssh.save);
+
-    assert.ok(
+    await click(SES.ssh.cancel);
-      this.saveConfig.withArgs({ delete: false }).calledOnce,
+
-      'calls the saveConfig action with args delete:false'
+    assert.true(
+      this.transitionStub.calledWith('vault.cluster.secrets.backend.configuration', 'ssh-test'),
+      'On cancel the router transitions to the parent configuration index route.'
     );
   });
 
-  test('it shows masked key if model is not new', async function (assert) {
+  test('it should validate form fields', async function (assert) {
-    // replace model with model that has public_key
-    this.model = {
-      publicKey:
-        'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC3lCZ7W2eJZ9W9qzv7K9GJ5qJYQ2cY6C+5Kv8Jtjz8h6wqZJ9U9K1lJ9Z6zq4sX0f7Q5X2l8L4gTt2+2ZKpVv6g1KQ6JG5H4QbVrQq2r4FzZQ2B0Y8q5c7q3Y5X6q4Q6',
-    };
     await render(hbs`
       <SecretEngine::ConfigureSsh
     @model={{this.model}}
-    @configured={{true}}
+    @id={{this.id}}
-    @saveConfig={{this.saveConfig}}
+  />
-    @loading={{false}}
+    `);
+    await fillIn(GENERAL.inputByAttr('publicKey'), 'hello');
+    await click(SES.ssh.save);
+    assert
+      .dom(GENERAL.inlineError)
+      .hasText(
+        'You must provide a Public and Private keys or leave both unset.',
+        'Public key validation error renders.'
+      );
+
+    await click(GENERAL.inputByAttr('generateSigningKey'));
+    await click(SES.ssh.save);
+    assert
+      .dom(GENERAL.inlineError)
+      .hasText(
+        'You must provide a Public and Private keys or leave both unset.',
+        'Generate signing key validation message shows.'
+      );
+  });
+
+  test('it should generate signing key', async function (assert) {
+    assert.expect(2);
+    this.server.post('/ssh-test/config/ca', (schema, req) => {
+      const data = JSON.parse(req.requestBody);
+      const expected = {
+        backend: this.id,
+        generate_signing_key: true,
+      };
+      assert.deepEqual(expected, data, 'POST request made to save ca-config with correct properties');
+    });
+    await render(hbs`
+      <SecretEngine::ConfigureSsh
+    @model={{this.model}}
+    @id={{this.id}}
+  />
+    `);
+
+    await click(SES.ssh.save);
+    assert.dom(SES.ssh.editConfigSection).exists('renders the edit configuration section of the form');
+  });
+
+  module('editing', function (hooks) {
+    hooks.beforeEach(function () {
+      this.editId = 'ssh-edit-me';
+      this.editModel = createConfig(this.store, 'ssh-edit-me', 'ssh');
+    });
+    test('it populates fields when editing', async function (assert) {
+      await render(hbs`
+      <SecretEngine::ConfigureSsh
+    @model={{this.editModel}}
+    @id={{this.editId}}
   />
     `);
       assert
@@ -74,25 +122,28 @@ module('Integration | Component | SecretEngine/configure-ssh', function (hooks)
       await click('[data-test-button="toggle-masked"]');
       assert
         .dom(GENERAL.inputByAttr('public-key'))
-      .hasText(this.model.publicKey, 'public key is unmasked and shows the actual value');
+        .hasText(this.editModel.publicKey, 'public key is unmasked and shows the actual value');
     });
 
-  test('it calls delete correctly', async function (assert) {
+    test('it allows you to delete a public key', async function (assert) {
+      assert.expect(3);
+      this.server.delete('/ssh-edit-me/config/ca', () => {
+        assert.true(true, 'DELETE request made to ca-config with correct properties');
+      });
       await render(hbs`
       <SecretEngine::ConfigureSsh
-    @model={{this.model}}
+    @model={{this.editModel}}
-    @configured={{true}}
+    @id={{this.editId}}
-    @saveConfig={{this.saveConfig}}
-    @loading={{false}}
   />
     `);
       // delete Public key
-    await click(SES.ssh.deletePublicKey);
+      await click(SES.ssh.delete);
-    assert.dom(GENERAL.confirmMessage).hasText('This will remove the CA certificate information.');
+      assert.dom(GENERAL.confirmMessage).hasText('Confirming will remove the CA certificate information.');
       await click(GENERAL.confirmButton);
-    assert.ok(
+      assert.true(
-      this.saveConfig.withArgs({ delete: true }).calledOnce,
+        this.transitionStub.calledWith('vault.cluster.secrets.backend.configuration.edit', 'ssh-edit-me'),
-      'calls the saveConfig action with args delete:true'
+        'On delete the router transitions to the current route.'
       );
     });
   });
+});

ui/tests/unit/adapters/secret-engine-test.js

@@ -4,62 +4,146 @@
  */
 
 import { module, test } from 'qunit';
-import { setupTest } from 'ember-qunit';
+import { setupRenderingTest } from 'vault/tests/helpers';
+import { render, click, fillIn } from '@ember/test-helpers';
+import { hbs } from 'ember-cli-htmlbars';
+import { GENERAL } from 'vault/tests/helpers/general-selectors';
+import { SECRET_ENGINE_SELECTORS as SES } from 'vault/tests/helpers/secret-engine/secret-engine-selectors';
+import { createConfig } from 'vault/tests/helpers/secret-engine/secret-engine-helpers';
 import { setupMirage } from 'ember-cli-mirage/test-support';
+import sinon from 'sinon';
 
-module('Unit | Adapter | secret engine', function (hooks) {
+module('Integration | Component | SecretEngine/configure-ssh', function (hooks) {
-  setupTest(hooks);
+  setupRenderingTest(hooks);
   setupMirage(hooks);
 
-  const storeStub = {
+  hooks.beforeEach(function () {
-    serializerFor() {
+    this.store = this.owner.lookup('service:store');
-      return {
+    const router = this.owner.lookup('service:router');
-        serializeIntoHash() {},
+    this.id = 'ssh-test';
-      };
+    this.model = this.store.createRecord('ssh/ca-config', { backend: this.id });
-    },
+    this.transitionStub = sinon.stub(router, 'transitionTo');
-  };
+    this.refreshStub = sinon.stub(router, 'refresh');
-  const type = {
-    modelName: 'secret-engine',
-  };
-
-  test('Empty query', function (assert) {
-    assert.expect(1);
-    this.server.get('/sys/internal/ui/mounts', () => {
-      assert.ok('query calls the correct url');
-      return {};
-    });
-    const adapter = this.owner.lookup('adapter:secret-engine');
-    adapter['query'](storeStub, type, {});
-  });
-  test('Query with a path', function (assert) {
-    assert.expect(1);
-    this.server.get('/sys/internal/ui/mounts/foo', () => {
-      assert.ok('query calls the correct url');
-      return {};
-    });
-    const adapter = this.owner.lookup('adapter:secret-engine');
-    adapter['query'](storeStub, type, { path: 'foo' });
   });
 
-  test('Query with nested path', function (assert) {
+  test('it shows create fields if not configured', async function (assert) {
-    assert.expect(1);
+    await render(hbs`
-    this.server.get('/sys/internal/ui/mounts/foo/bar/baz', () => {
+      <SecretEngine::ConfigureSsh
-      assert.ok('query calls the correct url');
+    @model={{this.model}}
-      return {};
+    @id={{this.id}}
-    });
+  />
-    const adapter = this.owner.lookup('adapter:secret-engine');
+    `);
-    adapter['query'](storeStub, type, { path: 'foo/bar/baz' });
+    assert.dom(GENERAL.maskedInput('privateKey')).hasNoText('Private key is empty and reset');
+    assert.dom(GENERAL.inputByAttr('publicKey')).hasNoText('Public key is empty and reset');
+    assert
+      .dom(GENERAL.inputByAttr('generateSigningKey'))
+      .isChecked('Generate signing key is checked by default');
   });
 
-  test('Fails gracefully finding records for non ssh engines', function (assert) {
+  test('it should go back to parent route on cancel', async function (assert) {
-    assert.expect(1);
+    await render(hbs`
-    const snapshot = {
+      <SecretEngine::ConfigureSsh
-      attr() {
+    @model={{this.model}}
-        return { type: 'aws', path: 'aws/' };
+    @id={{this.id}}
-      },
+  />
+    `);
+
+    await click(SES.ssh.cancel);
+
+    assert.true(
+      this.transitionStub.calledWith('vault.cluster.secrets.backend.configuration', 'ssh-test'),
+      'On cancel the router transitions to the parent configuration index route.'
+    );
+  });
+
+  test('it should validate form fields', async function (assert) {
+    await render(hbs`
+      <SecretEngine::ConfigureSsh
+    @model={{this.model}}
+    @id={{this.id}}
+  />
+    `);
+    await fillIn(GENERAL.inputByAttr('publicKey'), 'hello');
+    await click(SES.ssh.save);
+    assert
+      .dom(GENERAL.inlineError)
+      .hasText(
+        'You must provide a Public and Private keys or leave both unset.',
+        'Public key validation error renders.'
+      );
+
+    await click(GENERAL.inputByAttr('generateSigningKey'));
+    await click(SES.ssh.save);
+    assert
+      .dom(GENERAL.inlineError)
+      .hasText(
+        'You must provide a Public and Private keys or leave both unset.',
+        'Generate signing key validation message shows.'
+      );
+  });
+
+  test('it should generate signing key', async function (assert) {
+    assert.expect(2);
+    this.server.post('/ssh-test/config/ca', (schema, req) => {
+      const data = JSON.parse(req.requestBody);
+      const expected = {
+        backend: this.id,
+        generate_signing_key: true,
       };
-    const adapter = this.owner.lookup('adapter:secret-engine');
+      assert.deepEqual(expected, data, 'POST request made to save ca-config with correct properties');
-    const response = adapter.findRecord(storeStub, 'aws', { path: 'aws' }, snapshot);
+    });
-    assert.propEqual(response, { data: {} }, 'returns empty data object');
+    await render(hbs`
+      <SecretEngine::ConfigureSsh
+    @model={{this.model}}
+    @id={{this.id}}
+  />
+    `);
+
+    await click(SES.ssh.save);
+    assert.dom(SES.ssh.editConfigSection).exists('renders the edit configuration section of the form');
+  });
+
+  module('editing', function (hooks) {
+    hooks.beforeEach(function () {
+      this.editId = 'ssh-edit-me';
+      this.editModel = createConfig(this.store, 'ssh-edit-me', 'ssh');
+    });
+    test('it populates fields when editing', async function (assert) {
+      await render(hbs`
+      <SecretEngine::ConfigureSsh
+    @model={{this.editModel}}
+    @id={{this.editId}}
+  />
+    `);
+      assert
+        .dom(SES.ssh.editConfigSection)
+        .exists('renders the edit configuration section of the form and not the create part');
+      assert.dom(GENERAL.inputByAttr('public-key')).hasText('***********', 'public key is masked');
+      await click('[data-test-button="toggle-masked"]');
+      assert
+        .dom(GENERAL.inputByAttr('public-key'))
+        .hasText(this.editModel.publicKey, 'public key is unmasked and shows the actual value');
+    });
+
+    test('it allows you to delete a public key', async function (assert) {
+      assert.expect(3);
+      this.server.delete('/ssh-edit-me/config/ca', () => {
+        assert.true(true, 'DELETE request made to ca-config with correct properties');
+      });
+      await render(hbs`
+      <SecretEngine::ConfigureSsh
+    @model={{this.editModel}}
+    @id={{this.editId}}
+  />
+    `);
+      // delete Public key
+      await click(SES.ssh.delete);
+      assert.dom(GENERAL.confirmMessage).hasText('Confirming will remove the CA certificate information.');
+      await click(GENERAL.confirmButton);
+      assert.true(
+        this.transitionStub.calledWith('vault.cluster.secrets.backend.configuration.edit', 'ssh-edit-me'),
+        'On delete the router transitions to the current route.'
+      );
+    });
   });
 });

ui/tests/unit/models/secret-engine-test.js

@@ -346,80 +346,6 @@ module('Unit | Model | secret-engine', function (hooks) {
     });
   });
 
-  module('saveCA', function () {
-    test('does not call endpoint if type != ssh', async function (assert) {
-      assert.expect(1);
-      const model = this.store.createRecord('secret-engine', {
-        type: 'not-ssh',
-      });
-      const saveSpy = sinon.spy(model, 'save');
-      await model.saveCA({});
-      assert.ok(saveSpy.notCalled, 'save not called');
-    });
-    test('calls save with correct params', async function (assert) {
-      assert.expect(4);
-      const model = this.store.createRecord('secret-engine', {
-        type: 'ssh',
-        privateKey: 'private-key',
-        publicKey: 'public-key',
-        generateSigningKey: true,
-      });
-      const saveStub = sinon.stub(model, 'save').callsFake((params) => {
-        assert.deepEqual(
-          params,
-          {
-            adapterOptions: {
-              options: {},
-              apiPath: 'config/ca',
-              attrsToSend: ['privateKey', 'publicKey', 'generateSigningKey'],
-            },
-          },
-          'send correct params to save'
-        );
-        return;
-      });
-
-      await model.saveCA({});
-      assert.strictEqual(model.privateKey, 'private-key', 'value exists before save');
-      assert.strictEqual(model.publicKey, 'public-key', 'value exists before save');
-      assert.true(model.generateSigningKey, 'value true before save');
-
-      saveStub.restore();
-    });
-    test('sets properties when isDelete', async function (assert) {
-      assert.expect(7);
-      const model = this.store.createRecord('secret-engine', {
-        type: 'ssh',
-        privateKey: 'private-key',
-        publicKey: 'public-key',
-        generateSigningKey: true,
-      });
-      const saveStub = sinon.stub(model, 'save').callsFake((params) => {
-        assert.deepEqual(
-          params,
-          {
-            adapterOptions: {
-              options: { isDelete: true },
-              apiPath: 'config/ca',
-              attrsToSend: ['privateKey', 'publicKey', 'generateSigningKey'],
-            },
-          },
-          'send correct params to save'
-        );
-        return;
-      });
-      assert.strictEqual(model.privateKey, 'private-key', 'value exists before save');
-      assert.strictEqual(model.publicKey, 'public-key', 'value exists before save');
-      assert.true(model.generateSigningKey, 'value true before save');
-
-      await model.saveCA({ isDelete: true });
-      assert.strictEqual(model.privateKey, null, 'value null after save');
-      assert.strictEqual(model.publicKey, null, 'value null after save');
-      assert.false(model.generateSigningKey, 'value false after save');
-      saveStub.restore();
-    });
-  });
-
   module('saveZeroAddressConfig', function () {
     test('calls save with correct params', async function (assert) {
       assert.expect(1);