clean up

ui/app/adapters/aws/root-config.js

@@ -8,10 +8,12 @@ import { encodePath } from 'vault/utils/path-encoding-helpers';
 
 export default class AwsRootConfig 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 root config for the AWS secret backend, replacing the requests located on the secret-engine adapter.
   queryRecord(store, type, query) {
-    return this.ajax(`/v1/${encodePath(query.backend)}/config/root`, 'GET').then((resp) => {
-      resp.id = query.backend;
+    const { backend } = query;
+    return this.ajax(`/v1/${encodePath(backend)}/config/root`, 'GET').then((resp) => {
+      resp.id = backend;
       return resp;
     });
   }

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

@@ -8,10 +8,12 @@ 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) {
-    return this.ajax(`/v1/${encodePath(query.backend)}/config/ca`, 'GET').then((resp) => {
-      resp.id = query.backend;
+    const { backend } = query;
+    return this.ajax(`/v1/${encodePath(backend)}/config/ca`, 'GET').then((resp) => {
+      resp.id = backend;
       return resp;
     });
   }

ui/app/components/configure-aws-secret.hbs

@@ -122,14 +122,12 @@
         </p>
       </div>
       <TtlPicker
-        id="ttl-lease"
         @label="Lease"
         @initialValue={{@model.lease}}
         @initialEnabled={{@model.lease}}
         @onChange={{fn this.handleTtlChange "lease"}}
       />
       <TtlPicker
-        id="ttl-lease-max"
         @label="Maximum Lease"
         @initialValue={{@model.leaseMax}}
         @initialEnabled={{@model.leaseMax}}

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

@@ -4,6 +4,7 @@
 ~}}
 
 {{#if this.configError}}
+  {{! Surface API errors not associated with empty configuration details }}
   <EmptyState data-test-config-error={{@model.id}} @title="Something went wrong" @message={{this.configError}} />
 {{else if this.configModel}}
   {{#each this.configModel.attrs as |attr|}}
@@ -24,6 +25,7 @@
     {{/if}}
   {{/each}}
 {{else}}
+  {{! Prompt for a user to configure the secret engine }}
   <EmptyState
     data-test-config-cta
     @title="{{this.typeDisplay}} not configured"

ui/app/components/secret-engine/configurable-secret-engine-details.ts

@@ -11,17 +11,18 @@ import { tracked } from '@glimmer/tracking';
 import type Store from '@ember-data/store';
 import type SecretEngineModel from 'vault/models/secret-engine';
 import type AdapterError from '@ember-data/adapter';
+import type Model from '@ember-data/model';
 
 /**
  * @module ConfigurableSecretEngineDetails
- * The `ConfigurableSecretEngineDetails` is used by configurable secret engines to show either a prompt, error
- * or configuration details depending on the response from the engines specific config endpoint (ex: aws -> aws/root-config vs ssh: ssh/ca-config).
+ * `ConfigurableSecretEngineDetails` is used by configurable secret engines (AWS, SSH) to show either an API error, configuration details, or a prompt to configure the engine. Which of these is shown is determined by the engine type and whether the user has configured the engine yet.
  *
- * @example ```js
- *   <ConfigurableSecretEngineDetails @model={{this.model}} />```
+ * @example
+ * ```js
+ * <ConfigurableSecretEngineDetails @model={{this.model}} />
+ * ```
  *
  * @param {object} model - The secret-engine model to be configured.
- *
  */
 
 interface Args {
@@ -30,31 +31,36 @@ interface Args {
 
 export default class ConfigurableSecretEngineDetails extends Component<Args> {
   @service declare readonly store: Store;
-  @tracked configModel = null;
   @tracked configError: string | null = null;
+  @tracked configModel: Model | null = null;
 
   constructor(owner: unknown, args: Args) {
     super(owner, args);
     const { model } = this.args;
+    const { id, type } = model;
+    // Should not be able to get here without a model, but in case an upstream change allows it, catching the failure.
     if (!model) {
       this.configError =
         'We are unable to access the mount information for this engine. Ask you administrator if you think you should have access to this secret engine.';
       return;
     }
-    // Fetch the config for the engine. Will eventually include GCP and Azure.
-    if (model.type === 'aws') {
-      this.fetchAwsRootConfig(model.id);
-    }
-    if (model.type === 'ssh') {
-      this.fetchSshCaConfig(model.id);
+    // Fetch the config for the engine type.
+    switch (type) {
+      case 'aws':
+        this.fetchAwsRootConfig(id);
+        break;
+      case 'ssh':
+        this.fetchSshCaConfig(id);
+        break;
     }
   }
 
   async fetchAwsRootConfig(backend: string) {
     try {
       this.configModel = await this.store.queryRecord('aws/root-config', { backend });
     } catch (e: AdapterError) {
-      // If it's a 404 then the user has not configured the backend yet and we want to show the prompt instead
+      // If the error is something other than 404 "not found" then an API error has come back and this will be displayed to the user as an error.
+      // If it's 404 then configError is not set nor is the configModel and a prompt to configure will be shown.
       if (e.httpStatus !== 404) {
         this.configError = errorMessage(e);
       }
@@ -66,9 +72,9 @@ export default class ConfigurableSecretEngineDetails extends Component<Args> {
     try {
       this.configModel = await this.store.queryRecord('ssh/ca-config', { backend });
     } catch (e: AdapterError) {
-      // The SSH Api does not return a 404 not found but a 400 error after first mounting the engine with the
+      // The SSH api does not return a 404 not found but a 400 error after first mounting the engine with the
       // message that keys have not been configured yet.
-      // To show a prompt instead of an error when first configuring the backend, we need to catch that specific 400 error and continue to set a prompt message instead.
+      // We need to check the message of the 400 error and if it's the keys message, return a prompt instead of a configError.
       if (e.httpStatus !== 404 && errorMessage(e) !== `keys haven't been configured yet`) {
         this.configError = errorMessage(e);
       }
@@ -77,8 +83,8 @@ export default class ConfigurableSecretEngineDetails extends Component<Args> {
   }
 
   get typeDisplay() {
-    // Will eventually handle GCP and Azure.
-    // Did not use capitalize helper because some are all caps and some only title case.
+    // Will eventually include GCP and Azure.
+    // Did not use capitalize helper because some are all caps and some only title case (ex: Azure vs AWS).
     const { type } = this.args.model;
     switch (type) {
       case 'aws':

ui/app/models/aws/root-config.js

@@ -24,7 +24,7 @@ export default class AwsRootConfig extends Model {
     subText: 'Number of max retries the client should use for recoverable errors. Default is -1.',
   })
   maxRetries;
-  // TODO: there are more options available on the API, but the UI does not support them yet.
+  // there are more options available on the API, but the UI does not support them yet.
   get attrs() {
     const keys = ['accessKey', 'region', 'iamEndpoint', 'stsEndpoint', 'maxRetries'];
     return expandAttributeMeta(this, keys);

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

@@ -8,15 +8,14 @@ import { expandAttributeMeta } from 'vault/utils/field-to-attrs';
 
 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;
+  @attr('string', { sensitive: true }) privateKey; // obfuscated, never returned by API
   @attr('string', { sensitive: true }) publicKey;
   @attr('boolean', {
     defaultValue: true,
   })
   generateSigningKey;
-  // TODO: there are more options available on the API, but the UI does not support them yet.
+  // there are more options available on the API, but the UI does not support them yet.
   get attrs() {
-    // do not show private key, not returned by the API
     const keys = ['publicKey', 'generateSigningKey'];
     return expandAttributeMeta(this, keys);
   }

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

@@ -12,7 +12,9 @@ export default class SecretsBackendConfigurationRoute extends Route {
   async model() {
     const backend = this.modelFor('vault.cluster.secrets.backend');
     if (backend.isV2KV) {
-      const canRead = this.store.findRecord('capabilities', `${backend.id}/config`).canRead;
+      const canRead = await this.store
+        .findRecord('capabilities', `${backend.id}/config`)
+        .then((response) => response.canRead);
       // only set these config params if they can read the config endpoint.
       if (canRead) {
         // design wants specific default to show that can't be set in the model

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

@@ -21,7 +21,6 @@ import {
   expectedValueOfConfigKeys,
 } from 'vault/tests/helpers/secret-engine/secret-engine-helpers';
 
-
 module('Acceptance | aws | configuration', function (hooks) {
   setupApplicationTest(hooks);
   setupMirage(hooks);
@@ -127,13 +126,13 @@ module('Acceptance | aws | configuration', function (hooks) {
     createConfig(this.store, path, type); // create the aws root config in the store
     await click(SES.configTab);
     for (const key of expectedConfigKeys(type)) {
-      assert.dom(GENERAL.infoRowLabel(key)).exists(`key for ${key} on the ${type} config details exists.`);
+      assert.dom(GENERAL.infoRowLabel(key)).exists(`${key} on the ${type} config details exists.`);
       const responseKeyAndValue = expectedValueOfConfigKeys(type, key);
       assert
         .dom(GENERAL.infoRowValue(key))
         .hasText(responseKeyAndValue, `value for ${key} on the ${type} config details exists.`);
     }
-    // check mount configuration details is present and accurate.
+    // check mount configuration details are present and accurate.
     await click(SES.configurationToggle);
     assert
       .dom(GENERAL.infoRowValue('Path'))
@@ -169,5 +168,6 @@ module('Acceptance | aws | configuration', function (hooks) {
     assert.dom(GENERAL.infoRowValue('Access key')).hasText('hello', 'Access key has been updated to hello');
     assert.dom(GENERAL.infoRowValue('Region')).hasText('ca-central-1', 'Region has been added');
     // cleanup
+    await runCmd(`delete sys/mounts/${path}`);
   });
 });

ui/tests/helpers/general-selectors.ts

@@ -80,7 +80,6 @@ export const GENERAL = {
   },
   navLink: (label: string) => `[data-test-sidebar-nav-link="${label}"]`,
   cancelButton: '[data-test-cancel]',
-  saveButtonId: (id: string) => `[data-test-save=${id}]`, // there are many uses of save button, but very few with an id. Instead of making all instances of saveButton a function with an empty string, we can just use this selector. TODO: should be removed after refactor of AWS.
   saveButton: '[data-test-save]',
   saveButtonId: (id: string) => `[data-test-save="${id}"]`, // there are many uses of save button, but very few with an id. Instead of making all instances of saveButton a function with an empty string, we can just use this selector. TODO: should be removed after refactor of AWS.
   maskedInput: (name: string) => `[data-test-textarea="${name}"]`,

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

@@ -82,11 +82,11 @@ module('Integration | Component | SecretEngine::configurable-secret-engine-detai
 
       await render(hbs`<SecretEngine::ConfigurableSecretEngineDetails @model={{this.model}}/>`);
       for (const key of expectedConfigKeys(type)) {
-        assert.dom(GENERAL.infoRowLabel(key)).exists(`key for ${key} on the ${type} config details exists.`);
+        assert.dom(GENERAL.infoRowLabel(key)).exists(`${key} on the ${type} config details exists.`);
         const responseKeyAndValue = expectedValueOfConfigKeys(type, key);
         assert
           .dom(GENERAL.infoRowValue(key))
-          .hasText(responseKeyAndValue, `value for ${key} on the ${type} config details exists.`);
+          .hasText(responseKeyAndValue, `${key} value for the ${type} config details exists.`);
       }
     }
   });