Swap route settings.configure-secret-backend for nested edit and index route under secret.configuration (#27918)

* router changes and appropriate file shuffling

* changelog

* fix test routes

* handle redirect... is this okay?

* test redirect coverage

* move configure-secret-backend test and cleanup

* coverage for non configurable secret engine:

* clean up

* remove redirect

changelog/27918.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+ui: Move secret-engine configuration create/edit from routing `vault/settings/secrets/configure/<backend>` to  `vault/secrets/<backend>/configuration/edit`
+```

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

@@ -37,7 +37,7 @@
       @icon="chevron-right"
       @iconPosition="trailing"
       @text="Configure {{this.typeDisplay}}"
-      @route="vault.cluster.settings.configure-secret-backend"
+      @route="vault.cluster.secrets.backend.configuration.edit"
       @model={{@model.id}}
     />
   </EmptyState>

ui/app/components/sidebar/nav/cluster.hbs

@@ -9,7 +9,7 @@
   <Nav.Link @route="vault.cluster.dashboard" @text="Dashboard" data-test-sidebar-nav-link="Dashboard" />
   <Nav.Link
     @route="vault.cluster.secrets"
-    @current-when="vault.cluster.secrets vault.cluster.settings.mount-secret-backend vault.cluster.settings.configure-secret-backend"
+    @current-when="vault.cluster.secrets vault.cluster.settings.mount-secret-backend vault.cluster.secrets.backend.configuration.edit"
     @text="Secrets Engines"
     data-test-sidebar-nav-link="Secrets Engines"
   />

ui/app/router.js

@@ -50,10 +50,6 @@ Router.map(function () {
           });
         });
         this.route('mount-secret-backend');
-        this.route('configure-secret-backend', { path: '/secrets/configure/:backend' }, function () {
-          this.route('index', { path: '/' });
-          this.route('section', { path: '/:section_name' });
-        });
       });
       this.route('unseal');
       this.route('tools', function () {
@@ -172,7 +168,10 @@ Router.map(function () {
           this.mount('ldap');
           this.mount('pki');
           this.route('index', { path: '/' });
-          this.route('configuration');
+          this.route('configuration', function () {
+            // only CONFIGURABLE_SECRET_ENGINES can be configured and access the edit route
+            this.route('edit');
+          });
           // because globs / params can't be empty,
           // we have to special-case ids of '' with their own routes
           this.route('list-root', { path: '/list/' });

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

@@ -13,7 +13,7 @@ export default Route.extend({
   store: service(),
 
   model() {
-    const { backend } = this.paramsFor(this.routeName);
+    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)) {

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

@@ -2,52 +2,5 @@
   Copyright (c) HashiCorp, Inc.
   SPDX-License-Identifier: BUSL-1.1
 ~}}
-
-<SecretListHeader
-  @model={{this.model}}
-  @backendCrumb={{hash
-    label=this.model.id
-    text=this.model.id
-    path="vault.cluster.secrets.backend.list-root"
-    model=this.model.id
-  }}
-  @isConfigure={{true}}
-/>
-
-{{#if this.isConfigurable}}
-  <Toolbar>
-    <ToolbarActions>
-      <ToolbarLink
-        @route="vault.cluster.settings.configure-secret-backend"
-        @model={{this.model.id}}
-        data-test-secret-backend-configure
-      >
-        Configure
-      </ToolbarLink>
-    </ToolbarActions>
-  </Toolbar>
-
-  <SecretEngine::ConfigurationDetails @model={{this.model}} />
-
-  <SecretsEngineMountConfig @model={{this.model}} class="has-top-margin-xl has-bottom-margin-xl" data-test-mount-config />
-
-{{else}}
-  <div class="box is-fullwidth is-sideless is-paddingless is-marginless">
-    {{#each this.model.attrs as |attr|}}
-      {{#if (eq attr.type "object")}}
-        <InfoTableRow
-          @alwaysRender={{not (is-empty-value (get this.model attr.name))}}
-          @label={{or attr.options.label (to-label attr.name)}}
-          @value={{stringify (get this.model (or attr.options.fieldValue attr.name))}}
-        />
-      {{else}}
-        <InfoTableRow
-          @alwaysRender={{and (not (is-empty-value (get this.model attr.name))) (not-eq attr.name "version")}}
-          @formatTtl={{eq attr.options.editType "ttl"}}
-          @label={{or attr.options.label (to-label attr.name)}}
-          @value={{get this.model (or attr.options.fieldValue attr.name)}}
-        />
-      {{/if}}
-    {{/each}}
-  </div>
-{{/if}}
+{{! use the index route to view configuration and sibling edit route to edit the configuration. }}
+{{outlet}}

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

@@ -0,0 +1,52 @@
+{{!
+  Copyright (c) HashiCorp, Inc.
+  SPDX-License-Identifier: BUSL-1.1
+~}}
+
+<SecretListHeader
+  @model={{this.model}}
+  @backendCrumb={{hash
+    label=this.model.id
+    text=this.model.id
+    path="vault.cluster.secrets.backend.list-root"
+    model=this.model.id
+  }}
+  @isConfigure={{true}}
+/>
+
+{{#if this.isConfigurable}}
+  <Toolbar>
+    <ToolbarActions>
+      <ToolbarLink
+        @route="vault.cluster.secrets.backend.configuration.edit"
+        @model={{this.model.id}}
+        data-test-secret-backend-configure
+      >
+        Configure
+      </ToolbarLink>
+    </ToolbarActions>
+  </Toolbar>
+
+  <SecretEngine::ConfigurationDetails @model={{this.model}} />
+  <SecretsEngineMountConfig @model={{this.model}} class="has-top-margin-xl has-bottom-margin-xl" data-test-mount-config />
+
+{{else}}
+  <div class="box is-fullwidth is-sideless is-paddingless is-marginless">
+    {{#each this.model.attrs as |attr|}}
+      {{#if (eq attr.type "object")}}
+        <InfoTableRow
+          @alwaysRender={{not (is-empty-value (get this.model attr.name))}}
+          @label={{or attr.options.label (to-label attr.name)}}
+          @value={{stringify (get this.model (or attr.options.fieldValue attr.name))}}
+        />
+      {{else}}
+        <InfoTableRow
+          @alwaysRender={{and (not (is-empty-value (get this.model attr.name))) (not-eq attr.name "version")}}
+          @formatTtl={{eq attr.options.editType "ttl"}}
+          @label={{or attr.options.label (to-label attr.name)}}
+          @value={{get this.model (or attr.options.fieldValue attr.name)}}
+        />
+      {{/if}}
+    {{/each}}
+  </div>
+{{/if}}

ui/app/templates/vault/cluster/secrets/backend/error.hbs

@@ -14,7 +14,7 @@
       </Hds::Breadcrumb>
     </p.top>
     <p.levelLeft>
-      <h1 class="title is-3 has-text-grey">
+      <h1 class="title is-3 has-text-grey" data-test-backend-error-title>
         {{#if (eq this.model.httpStatus 404)}}
           404 Not Found
         {{else if (eq this.model.httpStatus 403)}}

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

@@ -54,7 +54,7 @@ module('Acceptance | aws | configuration', function (hooks) {
     await enablePage.enable('aws', path);
     await click(SES.configTab);
     await click(SES.configure);
-    assert.strictEqual(currentURL(), `/vault/settings/secrets/configure/${path}`);
+    assert.strictEqual(currentURL(), `/vault/secrets/${path}/configuration/edit`);
     assert.dom(SES.configureTitle('aws')).hasText('Configure AWS');
     assert.dom(SES.aws.rootForm).exists('it lands on the root configuration form.');
     assert.dom(GENERAL.tab('access-to-aws')).exists('renders the root creds tab');
@@ -63,6 +63,17 @@ module('Acceptance | aws | configuration', function (hooks) {
     await runCmd(`delete sys/mounts/${path}`);
   });
 
+  test('it should show error if old url is entered', async function (assert) {
+    // we are intentionally not redirecting from the old url to the new one.
+    const path = `aws-${this.uid}`;
+    await enablePage.enable('aws', path);
+    await click(SES.configTab);
+    await visit(`/vault/settings/secrets/configure/${path}`);
+    assert.dom('[data-test-not-found]').exists('shows page-error');
+    // cleanup
+    await runCmd(`delete sys/mounts/${path}`);
+  });
+
   test('it should save root AWS configuration', async function (assert) {
     assert.expect(3);
     const path = `aws-${this.uid}`;

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

@@ -3,23 +3,19 @@
  * SPDX-License-Identifier: BUSL-1.1
  */
 
-import { click, settled } from '@ember/test-helpers';
+import { click } from '@ember/test-helpers';
 import { module, test } from 'qunit';
 import { setupApplicationTest } from 'ember-qunit';
 import { v4 as uuidv4 } from 'uuid';
-
-import { visit } from '@ember/test-helpers';
+import { SECRET_ENGINE_SELECTORS as SES } from 'vault/tests/helpers/secret-engine/secret-engine-selectors';
+import { runCmd } from 'vault/tests/helpers/commands';
 import authPage from 'vault/tests/pages/auth';
 import enablePage from 'vault/tests/pages/settings/mount-secret-backend';
 import { create } from 'ember-cli-page-object';
 import fm from 'vault/tests/pages/components/flash-message';
 const flashMessage = create(fm);
-const SELECTORS = {
-  generateSigningKey: '[data-test-ssh-input="generate-signing-key-checkbox"]',
-  saveConfig: '[data-test-ssh-input="configure-submit"]',
-  publicKey: '[data-test-ssh-input="public-key"]',
-};
-module('Acceptance | settings/configure/secrets/ssh', function (hooks) {
+
+module('Acceptance | secrets configuration | edit', function (hooks) {
   setupApplicationTest(hooks);
 
   hooks.beforeEach(function () {
@@ -30,19 +26,22 @@ module('Acceptance | settings/configure/secrets/ssh', function (hooks) {
   test('it configures ssh ca', async function (assert) {
     const path = `ssh-configure-${this.uid}`;
     await enablePage.enable('ssh', path);
-    await settled();
-    visit(`/vault/settings/secrets/configure/${path}`);
-    await settled();
-    assert.dom(SELECTORS.generateSigningKey).isChecked('generate_signing_key defaults to true');
-    await click(SELECTORS.generateSigningKey);
-    await click(SELECTORS.saveConfig);
+    await click(SES.configTab);
+    await click(SES.configure);
+    assert
+      .dom(SES.ssh.sshInput('generate-signing-key-checkbox'))
+      .isChecked('generate_signing_key defaults to true');
+    await click(SES.ssh.sshInput('generate-signing-key-checkbox'));
+    await click(SES.ssh.sshInput('configure-submit'));
     assert.strictEqual(
       flashMessage.latestMessage,
       'missing public_key',
       'renders warning flash message for failed save'
     );
-    await click(SELECTORS.generateSigningKey);
-    await click(SELECTORS.saveConfig);
-    assert.dom(SELECTORS.publicKey).exists('renders public key after saving config');
+    await click(SES.ssh.sshInput('generate-signing-key-checkbox'));
+    await click(SES.ssh.sshInput('configure-submit'));
+    assert.dom(SES.ssh.sshInput('public-key')).exists('renders public key after saving config');
+    // cleanup
+    await runCmd(`delete sys/mounts/${path}`);
   });
 });

ui/tests/acceptance/secrets/backend/cubbyhole/secret-test.js

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: BUSL-1.1
  */
 
-import { currentRouteName, settled } from '@ember/test-helpers';
+import { currentRouteName, settled, click, visit } from '@ember/test-helpers';
 import { module, test } from 'qunit';
 import { setupApplicationTest } from 'ember-qunit';
 import { v4 as uuidv4 } from 'uuid';
@@ -14,6 +14,7 @@ import showPage from 'vault/tests/pages/secrets/backend/kv/show';
 import listPage from 'vault/tests/pages/secrets/backend/list';
 import authPage from 'vault/tests/pages/auth';
 import { assertSecretWrap } from 'vault/tests/helpers/components/secret-edit-toolbar';
+import { SECRET_ENGINE_SELECTORS as SES } from 'vault/tests/helpers/secret-engine/secret-engine-selectors';
 
 module('Acceptance | secrets/cubbyhole/create', function (hooks) {
   setupApplicationTest(hooks);
@@ -53,4 +54,13 @@ module('Acceptance | secrets/cubbyhole/create', function (hooks) {
 
     await assertSecretWrap(assert, this.server, requestPath);
   });
+
+  test('it does not show the option to configure', async function (assert) {
+    await visit(`/vault/secrets/cubbyhole/list`);
+    await click(SES.configTab);
+    assert.dom(SES.configure).doesNotExist('does not show the configure button');
+    // try to force it by visiting the URL
+    await visit(`/vault/secrets/cubbyhole/configuration/edit`);
+    assert.dom('[data-test-backend-error-title]').hasText('404 Not Found', 'shows 404 error');
+  });
 });

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

@@ -36,6 +36,17 @@ module('Acceptance | ssh | configuration', function (hooks) {
     await runCmd(`delete sys/mounts/${sshPath}`);
   });
 
+  test('it should show error if old url is entered', async function (assert) {
+    // we are intentionally not redirecting from the old url to the new one
+    const sshPath = `ssh-${this.uid}`;
+    await enablePage.enable('ssh', sshPath);
+    await click(SES.configTab);
+    await visit(`/vault/settings/secrets/configure/${sshPath}`);
+    assert.dom('[data-test-not-found]').exists('shows page-error');
+    // cleanup
+    await runCmd(`delete sys/mounts/${sshPath}`);
+  });
+
   test('it should show a public key after saving default configuration', async function (assert) {
     const sshPath = `ssh-${this.uid}`;
     await enablePage.enable('ssh', sshPath);
@@ -43,7 +54,7 @@ module('Acceptance | ssh | configuration', function (hooks) {
     await click(SES.configure);
     assert.strictEqual(
       currentURL(),
-      `/vault/settings/secrets/configure/${sshPath}`,
+      `/vault/secrets/${sshPath}/configuration/edit`,
       'transitions to the configuration page'
     );
     assert.dom(SES.ssh.configureForm).exists('renders ssh configuration form');
@@ -52,7 +63,7 @@ module('Acceptance | ssh | configuration', function (hooks) {
     await click(SES.ssh.sshInput('configure-submit'));
     assert.strictEqual(
       currentURL(),
-      `/vault/settings/secrets/configure/${sshPath}`,
+      `/vault/secrets/${sshPath}/configuration/edit`,
       'stays on configuration form page.'
     );
 

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

@@ -104,7 +104,11 @@ module('Acceptance | ssh secret backend', function (hooks) {
 
     await click('[data-test-secret-backend-configure]');
 
-    assert.strictEqual(currentURL(), `/vault/settings/secrets/configure/${sshPath}`);
+    assert.strictEqual(
+      currentURL(),
+      `/vault/secrets/${sshPath}/configuration/edit`,
+      'transitions to the configuration page'
+    );
     assert.dom('[data-test-ssh-configure-form]').exists('renders the empty configuration form');
 
     // default has generate CA checked so we just submit the form