Skip to content

Commit

Permalink
Swap route settings.configure-secret-backend for nested edit and inde…
Browse files Browse the repository at this point in the history
…x 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
  • Loading branch information
Monkeychip authored Aug 1, 2024
1 parent 68a5741 commit 01709e9
Show file tree
Hide file tree
Showing 17 changed files with 123 additions and 81 deletions.
3 changes: 3 additions & 0 deletions changelog/27918.txt
Original file line number Diff line number Diff line change
@@ -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`
```
2 changes: 1 addition & 1 deletion ui/app/components/secret-engine/configuration-details.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion ui/app/components/sidebar/nav/cluster.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
/>
Expand Down
9 changes: 4 additions & 5 deletions ui/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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/' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
51 changes: 2 additions & 49 deletions ui/app/templates/vault/cluster/secrets/backend/configuration.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Original file line number Diff line number Diff line change
@@ -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}}
2 changes: 1 addition & 1 deletion ui/app/templates/vault/cluster/secrets/backend/error.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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)}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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}`);
});
});
12 changes: 11 additions & 1 deletion ui/tests/acceptance/secrets/backend/cubbyhole/secret-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);
Expand Down Expand Up @@ -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');
});
});
15 changes: 13 additions & 2 deletions ui/tests/acceptance/secrets/backend/ssh/ssh-configuration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,25 @@ 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);
await click(SES.configTab);
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');
Expand All @@ -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.'
);

Expand Down
6 changes: 5 additions & 1 deletion ui/tests/acceptance/secrets/backend/ssh/ssh-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 01709e9

Please sign in to comment.