From 01709e992af3c7e51a77bdb9e7663aa5d53ca534 Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Thu, 1 Aug 2024 16:06:04 -0600 Subject: [PATCH] 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 | 3 ++ .../secret-engine/configuration-details.hbs | 2 +- ui/app/components/sidebar/nav/cluster.hbs | 2 +- .../backend/configuration/edit.js} | 0 .../index.js} | 0 ui/app/router.js | 9 ++-- .../backend/configuration/edit.js} | 2 +- .../index.js} | 0 .../cluster/secrets/backend/configuration.hbs | 51 +----------------- .../backend/configuration/edit.hbs} | 0 .../secrets/backend/configuration/index.hbs | 52 +++++++++++++++++++ .../vault/cluster/secrets/backend/error.hbs | 2 +- .../backend/aws/aws-configuration-test.js | 13 ++++- .../backend/configuration/edit-test.js} | 35 ++++++------- .../secrets/backend/cubbyhole/secret-test.js | 12 ++++- .../backend/ssh/ssh-configuration-test.js | 15 +++++- .../secrets/backend/ssh/ssh-test.js | 6 ++- 17 files changed, 123 insertions(+), 81 deletions(-) create mode 100644 changelog/27918.txt rename ui/app/controllers/vault/cluster/{settings/configure-secret-backend.js => secrets/backend/configuration/edit.js} (100%) rename ui/app/controllers/vault/cluster/secrets/backend/{configuration.js => configuration/index.js} (100%) rename ui/app/routes/vault/cluster/{settings/configure-secret-backend.js => secrets/backend/configuration/edit.js} (95%) rename ui/app/routes/vault/cluster/secrets/backend/{configuration.js => configuration/index.js} (100%) rename ui/app/templates/vault/cluster/{settings/configure-secret-backend.hbs => secrets/backend/configuration/edit.hbs} (100%) create mode 100644 ui/app/templates/vault/cluster/secrets/backend/configuration/index.hbs rename ui/tests/acceptance/{settings/configure-secret-backends/configure-ssh-secret-test.js => secrets/backend/configuration/edit-test.js} (50%) diff --git a/changelog/27918.txt b/changelog/27918.txt new file mode 100644 index 000000000000..bdf34609efe3 --- /dev/null +++ b/changelog/27918.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: Move secret-engine configuration create/edit from routing `vault/settings/secrets/configure/` to `vault/secrets//configuration/edit` +``` \ No newline at end of file diff --git a/ui/app/components/secret-engine/configuration-details.hbs b/ui/app/components/secret-engine/configuration-details.hbs index 49654f9f2307..4f8ff07eda24 100644 --- a/ui/app/components/secret-engine/configuration-details.hbs +++ b/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}} /> diff --git a/ui/app/components/sidebar/nav/cluster.hbs b/ui/app/components/sidebar/nav/cluster.hbs index b966cee19871..76012058daa7 100644 --- a/ui/app/components/sidebar/nav/cluster.hbs +++ b/ui/app/components/sidebar/nav/cluster.hbs @@ -9,7 +9,7 @@ diff --git a/ui/app/controllers/vault/cluster/settings/configure-secret-backend.js b/ui/app/controllers/vault/cluster/secrets/backend/configuration/edit.js similarity index 100% rename from ui/app/controllers/vault/cluster/settings/configure-secret-backend.js rename to ui/app/controllers/vault/cluster/secrets/backend/configuration/edit.js diff --git a/ui/app/controllers/vault/cluster/secrets/backend/configuration.js b/ui/app/controllers/vault/cluster/secrets/backend/configuration/index.js similarity index 100% rename from ui/app/controllers/vault/cluster/secrets/backend/configuration.js rename to ui/app/controllers/vault/cluster/secrets/backend/configuration/index.js diff --git a/ui/app/router.js b/ui/app/router.js index 00fd70ba5553..0c597e1192e7 100644 --- a/ui/app/router.js +++ b/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/' }); diff --git a/ui/app/routes/vault/cluster/settings/configure-secret-backend.js b/ui/app/routes/vault/cluster/secrets/backend/configuration/edit.js similarity index 95% rename from ui/app/routes/vault/cluster/settings/configure-secret-backend.js rename to ui/app/routes/vault/cluster/secrets/backend/configuration/edit.js index e3a711bc71da..877e1b6e2651 100644 --- a/ui/app/routes/vault/cluster/settings/configure-secret-backend.js +++ b/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)) { diff --git a/ui/app/routes/vault/cluster/secrets/backend/configuration.js b/ui/app/routes/vault/cluster/secrets/backend/configuration/index.js similarity index 100% rename from ui/app/routes/vault/cluster/secrets/backend/configuration.js rename to ui/app/routes/vault/cluster/secrets/backend/configuration/index.js diff --git a/ui/app/templates/vault/cluster/secrets/backend/configuration.hbs b/ui/app/templates/vault/cluster/secrets/backend/configuration.hbs index 2d441ec9344a..2f4d44870835 100644 --- a/ui/app/templates/vault/cluster/secrets/backend/configuration.hbs +++ b/ui/app/templates/vault/cluster/secrets/backend/configuration.hbs @@ -2,52 +2,5 @@ Copyright (c) HashiCorp, Inc. SPDX-License-Identifier: BUSL-1.1 ~}} - - - -{{#if this.isConfigurable}} - - - - Configure - - - - - - - - -{{else}} -
- {{#each this.model.attrs as |attr|}} - {{#if (eq attr.type "object")}} - - {{else}} - - {{/if}} - {{/each}} -
-{{/if}} \ No newline at end of file +{{! use the index route to view configuration and sibling edit route to edit the configuration. }} +{{outlet}} \ No newline at end of file diff --git a/ui/app/templates/vault/cluster/settings/configure-secret-backend.hbs b/ui/app/templates/vault/cluster/secrets/backend/configuration/edit.hbs similarity index 100% rename from ui/app/templates/vault/cluster/settings/configure-secret-backend.hbs rename to ui/app/templates/vault/cluster/secrets/backend/configuration/edit.hbs diff --git a/ui/app/templates/vault/cluster/secrets/backend/configuration/index.hbs b/ui/app/templates/vault/cluster/secrets/backend/configuration/index.hbs new file mode 100644 index 000000000000..00d23733bcec --- /dev/null +++ b/ui/app/templates/vault/cluster/secrets/backend/configuration/index.hbs @@ -0,0 +1,52 @@ +{{! + Copyright (c) HashiCorp, Inc. + SPDX-License-Identifier: BUSL-1.1 +~}} + + + +{{#if this.isConfigurable}} + + + + Configure + + + + + + + +{{else}} +
+ {{#each this.model.attrs as |attr|}} + {{#if (eq attr.type "object")}} + + {{else}} + + {{/if}} + {{/each}} +
+{{/if}} \ No newline at end of file diff --git a/ui/app/templates/vault/cluster/secrets/backend/error.hbs b/ui/app/templates/vault/cluster/secrets/backend/error.hbs index b82bbe477293..762e880a6f59 100644 --- a/ui/app/templates/vault/cluster/secrets/backend/error.hbs +++ b/ui/app/templates/vault/cluster/secrets/backend/error.hbs @@ -14,7 +14,7 @@ -

+

{{#if (eq this.model.httpStatus 404)}} 404 Not Found {{else if (eq this.model.httpStatus 403)}} diff --git a/ui/tests/acceptance/secrets/backend/aws/aws-configuration-test.js b/ui/tests/acceptance/secrets/backend/aws/aws-configuration-test.js index 0ce067a43e23..e96b76928b13 100644 --- a/ui/tests/acceptance/secrets/backend/aws/aws-configuration-test.js +++ b/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}`; diff --git a/ui/tests/acceptance/settings/configure-secret-backends/configure-ssh-secret-test.js b/ui/tests/acceptance/secrets/backend/configuration/edit-test.js similarity index 50% rename from ui/tests/acceptance/settings/configure-secret-backends/configure-ssh-secret-test.js rename to ui/tests/acceptance/secrets/backend/configuration/edit-test.js index 80ffda260916..f19287f6b0cb 100644 --- a/ui/tests/acceptance/settings/configure-secret-backends/configure-ssh-secret-test.js +++ b/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}`); }); }); diff --git a/ui/tests/acceptance/secrets/backend/cubbyhole/secret-test.js b/ui/tests/acceptance/secrets/backend/cubbyhole/secret-test.js index 7e91fe30c770..d17d579e6914 100644 --- a/ui/tests/acceptance/secrets/backend/cubbyhole/secret-test.js +++ b/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'); + }); }); diff --git a/ui/tests/acceptance/secrets/backend/ssh/ssh-configuration-test.js b/ui/tests/acceptance/secrets/backend/ssh/ssh-configuration-test.js index 4adb8413a043..46a8def541b1 100644 --- a/ui/tests/acceptance/secrets/backend/ssh/ssh-configuration-test.js +++ b/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.' ); diff --git a/ui/tests/acceptance/secrets/backend/ssh/ssh-test.js b/ui/tests/acceptance/secrets/backend/ssh/ssh-test.js index 1a7f9dc76ad5..c53132c6e516 100644 --- a/ui/tests/acceptance/secrets/backend/ssh/ssh-test.js +++ b/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