Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Mount PKI options + allowed_managed_keys #19791

Merged
merged 14 commits into from
Apr 7, 2023
3 changes: 3 additions & 0 deletions changelog/19791.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: add allowed_managed_keys field to secret engine mount options
```
12 changes: 12 additions & 0 deletions ui/app/components/mount-backend-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ export default class MountBackendForm extends Component {
}
}

typeChangeSideEffect(type) {
if (!this.args.mountType === 'secret') return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we checking to see if there's no mountType or this.args.mountType !== 'secret'? If we're checking if there's no mountType, it won't ever equal 'secret'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! Here I essentially want to continue with the side effects only if mountType is secret (auth is the other possibility). If it makes more sense to read as below, I'm happy to update it

if (this.args.mountType === 'secret') {
  // do stuff
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it - that makes sense to me! I was reading it like !this.args.mountType === 'secrets' but it's really just checking (!(this.args.mountType === 'secrets')). I think we can keep it as it is! Thanks for clarifying 🙃

if (type === 'pki') {
// If type PKI, set max lease to ~10years
this.args.mountModel.config.maxLeaseTtl = '3650d';
} else {
// otherwise reset
this.args.mountModel.config.maxLeaseTtl = 0;
}
}

checkModelValidity(model) {
const { isValid, state, invalidFormMessage } = model.validate();
this.modelValidations = state;
Expand Down Expand Up @@ -146,6 +157,7 @@ export default class MountBackendForm extends Component {
@action
setMountType(value) {
this.args.mountModel.type = value;
this.typeChangeSideEffect(value);
this.checkPathChange(value);
}
}
9 changes: 8 additions & 1 deletion ui/app/decorators/model-expanded-attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,14 @@ export function withExpandedAttributes() {
});
const expanded = expandAttributeMeta(rModel, rAttrNames);
expanded.forEach((attr) => {
byKey[`${name}.${attr.name}`] = attr;
byKey[`${name}.${attr.name}`] = {
...attr,
options: {
...attr.options,
// This ensures the correct path is updated in FormField
fieldValue: `${name}.${attr.fieldValue || attr.name}`,
},
};
});
}, this);
this._allByKey = byKey;
Expand Down
1 change: 1 addition & 0 deletions ui/app/helpers/mountable-secret-engines.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ const MOUNTABLE_SECRET_ENGINES = [
{
displayName: 'PKI Certificates',
type: 'pki',
// engineRoute: 'pki.overview', // TODO VAULT-13822
category: 'generic',
},
{
Expand Down
5 changes: 4 additions & 1 deletion ui/app/models/mount-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,8 @@ export default class MountConfigModel extends Model {
})
tokenType;

@attr() allowedManagedKeys;
@attr({
editType: 'stringArray',
})
allowedManagedKeys;
}
17 changes: 15 additions & 2 deletions ui/app/models/secret-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ export default class SecretEngineModel extends Model {
fields.push('config.defaultLeaseTtl', 'config.maxLeaseTtl');
}
fields.push(
'config.allowedManagedKeys',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
Expand Down Expand Up @@ -181,6 +182,7 @@ export default class SecretEngineModel extends Model {
...CORE_OPTIONS,
'config.defaultLeaseTtl',
'config.maxLeaseTtl',
'config.allowedManagedKeys',
...STANDARD_CONFIG,
];
break;
Expand All @@ -190,21 +192,32 @@ export default class SecretEngineModel extends Model {
...CORE_OPTIONS,
'config.defaultLeaseTtl',
'config.maxLeaseTtl',
'config.allowedManagedKeys',
...STANDARD_CONFIG,
];
break;
case 'database':
// Highlight TTLs in default
defaultFields = ['path', 'config.defaultLeaseTtl', 'config.maxLeaseTtl'];
optionFields = [...CORE_OPTIONS, 'config.allowedManagedKeys', ...STANDARD_CONFIG];
break;
case 'pki':
defaultFields = ['path', 'config.defaultLeaseTtl', 'config.maxLeaseTtl', 'config.allowedManagedKeys'];
optionFields = [...CORE_OPTIONS, ...STANDARD_CONFIG];
break;
case 'keymgmt':
// no ttl options for keymgmt
optionFields = [...CORE_OPTIONS, ...STANDARD_CONFIG];
optionFields = [...CORE_OPTIONS, 'config.allowedManagedKeys', ...STANDARD_CONFIG];
break;
default:
defaultFields = ['path'];
optionFields = [...CORE_OPTIONS, 'config.defaultLeaseTtl', 'config.maxLeaseTtl', ...STANDARD_CONFIG];
optionFields = [
...CORE_OPTIONS,
'config.defaultLeaseTtl',
'config.maxLeaseTtl',
'config.allowedManagedKeys',
...STANDARD_CONFIG,
];
break;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@
<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 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 attr.name}}
@value={{get this.model (or attr.options.fieldValue attr.name)}}
/>
{{/if}}
{{/each}}
Expand Down
36 changes: 29 additions & 7 deletions ui/tests/acceptance/settings/mount-secret-backend-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) {
const path = `mount-kv-${this.uid}`;
const defaultTTLHours = 100;
const maxTTLHours = 300;
const defaultTTLSeconds = (defaultTTLHours * 60 * 60).toString();
const maxTTLSeconds = (maxTTLHours * 60 * 60).toString();

await page.visit();

Expand All @@ -51,15 +49,14 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) {
.maxTTLVal(maxTTLHours)
.submit();
await configPage.visit({ backend: path });
assert.strictEqual(configPage.defaultTTL, `${defaultTTLSeconds}s`, 'shows the proper TTL');
assert.strictEqual(configPage.maxTTL, `${maxTTLSeconds}s`, 'shows the proper max TTL');
assert.strictEqual(configPage.defaultTTL, `${defaultTTLHours}h`, 'shows the proper TTL');
assert.strictEqual(configPage.maxTTL, `${maxTTLHours}h`, 'shows the proper max TTL');
});

test('it sets the ttl when enabled then disabled', async function (assert) {
// always force the new mount to the top of the list
const path = `mount-kv-${this.uid}`;
const maxTTLHours = 300;
const maxTTLSeconds = (maxTTLHours * 60 * 60).toString();

await page.visit();

Expand All @@ -76,8 +73,33 @@ module('Acceptance | settings/mount-secret-backend', function (hooks) {
.maxTTLVal(maxTTLHours)
.submit();
await configPage.visit({ backend: path });
assert.strictEqual(configPage.defaultTTL, '0', 'shows the proper TTL');
assert.strictEqual(configPage.maxTTL, `${maxTTLSeconds}s`, 'shows the proper max TTL');
assert.strictEqual(configPage.defaultTTL, '0s', 'shows the proper TTL');
assert.strictEqual(configPage.maxTTL, `${maxTTLHours}h`, 'shows the proper max TTL');
});

test('it sets the max ttl after pki chosen, resets after', async function (assert) {
await page.visit();
assert.strictEqual(currentRouteName(), 'vault.cluster.settings.mount-secret-backend');
await page.selectType('pki');
await page.next();
assert.dom('[data-test-input="maxLeaseTtl"]').exists();
assert
.dom('[data-test-input="maxLeaseTtl"] [data-test-ttl-toggle]')
.isChecked('Toggle is checked by default');
assert.dom('[data-test-input="maxLeaseTtl"] [data-test-ttl-value]').hasValue('3650');
assert.dom('[data-test-input="maxLeaseTtl"] [data-test-select="ttl-unit"]').hasValue('d');

// Go back and choose a different type
await page.back();
await page.selectType('database');
await page.next();
assert.dom('[data-test-input="maxLeaseTtl"]').exists('3650');
assert
.dom('[data-test-input="maxLeaseTtl"] [data-test-ttl-toggle]')
.isNotChecked('Toggle is unchecked by default');
await page.enableMaxTtl();
assert.dom('[data-test-input="maxLeaseTtl"] [data-test-ttl-value]').hasValue('');
assert.dom('[data-test-input="maxLeaseTtl"] [data-test-select="ttl-unit"]').hasValue('s');
});

test('it throws error if setting duplicate path name', async function (assert) {
Expand Down
4 changes: 2 additions & 2 deletions ui/tests/pages/secrets/backend/configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ import { create, visitable, text } from 'ember-cli-page-object';

export default create({
visit: visitable('/vault/secrets/:backend/configuration'),
defaultTTL: text('[data-test-row-value="Default Lease TTL"]'),
maxTTL: text('[data-test-row-value="Max Lease TTL"]'),
defaultTTL: text('[data-test-value-div="Default Lease TTL"]'),
maxTTL: text('[data-test-value-div="Max Lease TTL"]'),
});
32 changes: 32 additions & 0 deletions ui/tests/unit/models/secret-engine-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ module('Unit | Model | secret-engine', function (hooks) {
'sealWrap',
'config.defaultLeaseTtl',
'config.maxLeaseTtl',
'config.allowedManagedKeys',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
Expand All @@ -88,6 +89,7 @@ module('Unit | Model | secret-engine', function (hooks) {
'sealWrap',
'config.defaultLeaseTtl',
'config.maxLeaseTtl',
'config.allowedManagedKeys',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
Expand All @@ -112,6 +114,7 @@ module('Unit | Model | secret-engine', function (hooks) {
'sealWrap',
'config.defaultLeaseTtl',
'config.maxLeaseTtl',
'config.allowedManagedKeys',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
Expand All @@ -136,6 +139,7 @@ module('Unit | Model | secret-engine', function (hooks) {
'accessor',
'local',
'sealWrap',
'config.allowedManagedKeys',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
Expand All @@ -161,6 +165,7 @@ module('Unit | Model | secret-engine', function (hooks) {
'sealWrap',
'config.defaultLeaseTtl',
'config.maxLeaseTtl',
'config.allowedManagedKeys',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
Expand All @@ -186,6 +191,7 @@ module('Unit | Model | secret-engine', function (hooks) {
'sealWrap',
'config.defaultLeaseTtl',
'config.maxLeaseTtl',
'config.allowedManagedKeys',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
Expand All @@ -212,6 +218,7 @@ module('Unit | Model | secret-engine', function (hooks) {
'sealWrap',
'config.defaultLeaseTtl',
'config.maxLeaseTtl',
'config.allowedManagedKeys',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
Expand All @@ -229,6 +236,30 @@ module('Unit | Model | secret-engine', function (hooks) {

assert.deepEqual(model.get('formFieldGroups'), [
{ default: ['path', 'config.defaultLeaseTtl', 'config.maxLeaseTtl'] },
{
'Method Options': [
'description',
'config.listingVisibility',
'local',
'sealWrap',
'config.allowedManagedKeys',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
'config.allowedResponseHeaders',
],
},
]);
});

test('returns correct values for pki', function (assert) {
assert.expect(1);
const model = this.store.createRecord('secret-engine', {
type: 'pki',
});

assert.deepEqual(model.get('formFieldGroups'), [
{ default: ['path', 'config.defaultLeaseTtl', 'config.maxLeaseTtl', 'config.allowedManagedKeys'] },
{
'Method Options': [
'description',
Expand Down Expand Up @@ -258,6 +289,7 @@ module('Unit | Model | secret-engine', function (hooks) {
'config.listingVisibility',
'local',
'sealWrap',
'config.allowedManagedKeys',
'config.auditNonHmacRequestKeys',
'config.auditNonHmacResponseKeys',
'config.passthroughRequestHeaders',
Expand Down