From 5768ae4f9b450227deba07b756aaaf5446dfe380 Mon Sep 17 00:00:00 2001 From: claire bontempo <68122737+hellobontempo@users.noreply.github.com> Date: Wed, 3 May 2023 09:48:08 -0700 Subject: [PATCH] UI: add enterprise only pki/config/crl parameters to edit configuration form (#20479) * update version service * render enterprise groups * render enterprise params * move group headers to within loop * cleanup template * update form tests * change version service references to hasFeature to hasControlGroups getter * add params to details view * update version service test --- ui/app/models/pki/crl.js | 44 +++++++-- .../cluster/access/control-group-accessor.js | 4 +- .../access/control-groups-configure.js | 2 +- .../vault/cluster/access/control-groups.js | 2 +- ui/app/services/version.js | 96 +++++++++---------- .../page/pki-configuration-details.hbs | 9 ++ .../page/pki-configuration-details.ts | 7 +- .../page/pki-configuration-edit.hbs | 58 ++++++----- .../components/page/pki-configuration-edit.ts | 6 +- .../pki/page/pki-configuration-edit.js | 2 + .../page/pki-configuration-details-test.js | 30 ++++++ .../pki/page/pki-configuration-edit-test.js | 90 +++++++++++++++++ ui/tests/unit/services/version-test.js | 8 +- 13 files changed, 260 insertions(+), 98 deletions(-) diff --git a/ui/app/models/pki/crl.js b/ui/app/models/pki/crl.js index 1d844763f..55030c3d5 100644 --- a/ui/app/models/pki/crl.js +++ b/ui/app/models/pki/crl.js @@ -7,7 +7,16 @@ import Model, { attr } from '@ember-data/model'; import { withFormFields } from 'vault/decorators/model-form-fields'; import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities'; -@withFormFields(['expiry', 'autoRebuildGracePeriod', 'deltaRebuildInterval', 'ocspExpiry']) +const formFieldGroups = [ + { + 'Certificate Revocation List (CRL)': ['expiry', 'autoRebuildGracePeriod', 'deltaRebuildInterval'], + }, + { + 'Online Certificate Status Protocol (OCSP)': ['ocspExpiry'], + }, + { 'Unified Revocation': ['crossClusterRevocation', 'unifiedCrl', 'unifiedCrlOnExistingPaths'] }, +]; +@withFormFields(null, formFieldGroups) export default class PkiCrlModel extends Model { // This model uses the backend value as the model ID @@ -17,6 +26,7 @@ export default class PkiCrlModel extends Model { labelDisabled: 'Auto-rebuild off', mapToBoolean: 'autoRebuild', isOppositeValue: false, + editType: 'ttl', helperTextEnabled: 'Vault will rebuild the CRL in the below grace period before expiration', helperTextDisabled: 'Vault will not automatically rebuild the CRL', }) @@ -28,6 +38,7 @@ export default class PkiCrlModel extends Model { labelDisabled: 'Delta CRL building off', mapToBoolean: 'enableDelta', isOppositeValue: false, + editType: 'ttl', helperTextEnabled: 'Vault will rebuild the delta CRL at the interval below:', helperTextDisabled: 'Vault will not rebuild the delta CRL at an interval', }) @@ -39,6 +50,7 @@ export default class PkiCrlModel extends Model { labelDisabled: 'No expiry', mapToBoolean: 'disable', isOppositeValue: true, + editType: 'ttl', helperTextDisabled: 'The CRL will not be built.', helperTextEnabled: 'The CRL will expire after:', }) @@ -50,21 +62,37 @@ export default class PkiCrlModel extends Model { labelDisabled: 'OCSP responder APIs disabled', mapToBoolean: 'ocspDisable', isOppositeValue: true, + editType: 'ttl', helperTextEnabled: "Requests about a certificate's status will be valid for:", helperTextDisabled: 'Requests cannot be made to check if an individual certificate is valid.', }) ocspExpiry; - // TODO follow-on ticket to add enterprise only attributes: - /* - @attr('boolean') crossClusterRevocation; - @attr('boolean') unifiedCrl; - @attr('boolean') unifiedCrlOnExistingPaths; - */ + // enterprise only params + @attr('boolean', { + label: 'Cross-cluster revocation', + helpText: + 'Enables cross-cluster revocation request queues. When a serial not issued on this local cluster is passed to the /revoke endpoint, it is replicated across clusters and revoked by the issuing cluster if it is online.', + }) + crossClusterRevocation; + + @attr('boolean', { + label: 'Unified CRL', + helpText: + 'Enables unified CRL and OCSP building. This synchronizes all revocations between clusters; a single, unified CRL will be built on the active node of the primary performance replication (PR) cluster.', + }) + unifiedCrl; + + @attr('boolean', { + label: 'Unified CRL on existing paths', + helpText: + 'If enabled, existing CRL and OCSP paths will return the unified CRL instead of a response based on cluster-local data.', + }) + unifiedCrlOnExistingPaths; @lazyCapabilities(apiPath`${'id'}/config/crl`, 'id') crlPath; get canSet() { - return this.crlPath.get('canCreate') !== false; + return this.crlPath.get('canUpdate') !== false; } } diff --git a/ui/app/routes/vault/cluster/access/control-group-accessor.js b/ui/app/routes/vault/cluster/access/control-group-accessor.js index fffa2819b..a4c28edd1 100644 --- a/ui/app/routes/vault/cluster/access/control-group-accessor.js +++ b/ui/app/routes/vault/cluster/access/control-group-accessor.js @@ -18,9 +18,7 @@ export default Route.extend(UnloadModel, { }, model(params) { - return this.version.hasFeature('Control Groups') - ? this.store.findRecord('control-group', params.accessor) - : null; + return this.version.hasControlGroups ? this.store.findRecord('control-group', params.accessor) : null; }, actions: { diff --git a/ui/app/routes/vault/cluster/access/control-groups-configure.js b/ui/app/routes/vault/cluster/access/control-groups-configure.js index 9b6bd6f81..177f22326 100644 --- a/ui/app/routes/vault/cluster/access/control-groups-configure.js +++ b/ui/app/routes/vault/cluster/access/control-groups-configure.js @@ -19,7 +19,7 @@ export default Route.extend(UnloadModel, { model() { const type = 'control-group-config'; - return this.version.hasFeature('Control Groups') + return this.version.hasControlGroups ? this.store.findRecord(type, 'config').catch((e) => { // if you haven't saved a config, the API 404s, so create one here to edit and return it if (e.httpStatus === 404) { diff --git a/ui/app/routes/vault/cluster/access/control-groups.js b/ui/app/routes/vault/cluster/access/control-groups.js index bf6b119e4..c2f438038 100644 --- a/ui/app/routes/vault/cluster/access/control-groups.js +++ b/ui/app/routes/vault/cluster/access/control-groups.js @@ -18,6 +18,6 @@ export default Route.extend(UnloadModel, { }, model() { - return this.version.hasFeature('Control Groups') ? this.store.createRecord('control-group') : null; + return this.version.hasControlGroups ? this.store.createRecord('control-group') : null; }, }); diff --git a/ui/app/services/version.js b/ui/app/services/version.js index 035e15700..cd3be849d 100644 --- a/ui/app/services/version.js +++ b/ui/app/services/version.js @@ -3,81 +3,71 @@ * SPDX-License-Identifier: MPL-2.0 */ -import { readOnly, match, not } from '@ember/object/computed'; import Service, { inject as service } from '@ember/service'; -import { computed } from '@ember/object'; -import { task } from 'ember-concurrency'; +import { keepLatestTask, task } from 'ember-concurrency'; +import { tracked } from '@glimmer/tracking'; -const hasFeatureMethod = (context, featureKey) => { - const features = context.get('features'); - if (!features) { - return false; +export default class VersionService extends Service { + @service store; + @tracked features = []; + @tracked version = null; + + get hasPerfReplication() { + return this.features.includes('Performance Replication'); } - return features.includes(featureKey); -}; -const hasFeature = (featureKey) => { - return computed('features', 'features.[]', function () { - return hasFeatureMethod(this, featureKey); - }); -}; -export default Service.extend({ - _features: null, - features: readOnly('_features'), - version: null, - store: service(), - hasPerfReplication: hasFeature('Performance Replication'), + get hasDRReplication() { + return this.features.includes('DR Replication'); + } - hasDRReplication: hasFeature('DR Replication'), + get hasSentinel() { + return this.features.includes('Sentinel'); + } - hasSentinel: hasFeature('Sentinel'), - hasNamespaces: hasFeature('Namespaces'), + get hasNamespaces() { + return this.features.includes('Namespaces'); + } - isEnterprise: match('version', /\+.+$/), + get hasControlGroups() { + return this.features.includes('Control Groups'); + } - isOSS: not('isEnterprise'), + get isEnterprise() { + if (!this.version) return false; + return this.version.includes('+'); + } - setVersion(resp) { - this.set('version', resp.version); - }, + get isOSS() { + return !this.isEnterprise; + } - hasFeature(feature) { - return hasFeatureMethod(this, feature); - }, - - setFeatures(resp) { - if (!resp.features) { - return; - } - this.set('_features', resp.features); - }, - - getVersion: task(function* () { - if (this.version) { - return; - } + @task + *getVersion() { + if (this.version) return; const response = yield this.store.adapterFor('cluster').health(); - this.setVersion(response); + this.version = response.version; return; - }), + } - getFeatures: task(function* () { + @keepLatestTask + *getFeatures() { if (this.features?.length || this.isOSS) { return; } try { const response = yield this.store.adapterFor('cluster').features(); - this.setFeatures(response); + this.features = response.features; return; } catch (err) { // if we fail here, we're likely in DR Secondary mode and don't need to worry about it } - }).keepLatest(), + } - fetchVersion: function () { + fetchVersion() { return this.getVersion.perform(); - }, - fetchFeatures: function () { + } + + fetchFeatures() { return this.getFeatures.perform(); - }, -}); + } +} diff --git a/ui/lib/pki/addon/components/page/pki-configuration-details.hbs b/ui/lib/pki/addon/components/page/pki-configuration-details.hbs index a720312ec..aa047999e 100644 --- a/ui/lib/pki/addon/components/page/pki-configuration-details.hbs +++ b/ui/lib/pki/addon/components/page/pki-configuration-details.hbs @@ -69,6 +69,15 @@ {{#unless @crl.ocspDisable}} {{/unless}} + + {{#if this.isEnterprise}} +

+ Unified Revocation +

+ + + + {{/if}} {{/if}} {{else}} diff --git a/ui/lib/pki/addon/components/page/pki-configuration-details.ts b/ui/lib/pki/addon/components/page/pki-configuration-details.ts index ced46d1de..031bd1e08 100644 --- a/ui/lib/pki/addon/components/page/pki-configuration-details.ts +++ b/ui/lib/pki/addon/components/page/pki-configuration-details.ts @@ -13,6 +13,7 @@ import { tracked } from '@glimmer/tracking'; import RouterService from '@ember/routing/router-service'; import FlashMessageService from 'vault/services/flash-messages'; import Store from '@ember-data/store'; +import VersionService from 'vault/services/version'; interface Args { currentPath: string; @@ -22,9 +23,13 @@ export default class PkiConfigurationDetails extends Component { @service declare readonly store: Store; @service declare readonly router: RouterService; @service declare readonly flashMessages: FlashMessageService; - + @service declare readonly version: VersionService; @tracked showDeleteAllIssuers = false; + get isEnterprise() { + return this.version.isEnterprise; + } + @action async deleteAllIssuers() { try { diff --git a/ui/lib/pki/addon/components/page/pki-configuration-edit.hbs b/ui/lib/pki/addon/components/page/pki-configuration-edit.hbs index 69c8987c9..2e211d6c5 100644 --- a/ui/lib/pki/addon/components/page/pki-configuration-edit.hbs +++ b/ui/lib/pki/addon/components/page/pki-configuration-edit.hbs @@ -23,32 +23,40 @@
-

- Certificate Revocation List (CRL) -

{{#if @crl.canSet}} - {{#each @crl.formFields as |attr|}} - {{#if (eq attr.name "ocspExpiry")}} -

- Online Certificate Status Protocol (OCSP) -

- {{/if}} - {{#if (or (includes attr.name this.alwaysRender) (not @crl.disable))}} - {{#let (get @crl attr.options.mapToBoolean) as |booleanValue|}} -
- -
- {{/let}} - {{/if}} + {{#each @crl.formFieldGroups as |fieldGroup|}} + {{#each-in fieldGroup as |group fields|}} + {{#if (or (not-eq group "Unified Revocation") this.isEnterprise)}} +

+ {{group}} +

+ {{/if}} + {{#each fields as |attr|}} + {{#if (eq attr.options.editType "ttl")}} + {{#if (or (includes attr.name (array "expiry" "ocspExpiry")) (not @crl.disable))}} + {{#let (get @crl attr.options.mapToBoolean) as |enabled|}} + {{! 'enabled' is the pki/crl model's boolean attr that corresponds to the duration set by the ttl }} +
+ +
+ {{/let}} + {{/if}} + {{else}} + {{#if this.isEnterprise}} + + {{/if}} + {{/if}} + {{/each}} + {{/each-in}} {{/each}} {{else}} { @service declare readonly router: RouterService; @service declare readonly flashMessages: FlashMessageService; + @service declare readonly version: VersionService; @tracked invalidFormAlert = ''; @tracked errorBanner = ''; - get alwaysRender() { - return ['expiry', 'ocspExpiry']; + get isEnterprise() { + return this.version.isEnterprise; } @task diff --git a/ui/tests/helpers/pki/page/pki-configuration-edit.js b/ui/tests/helpers/pki/page/pki-configuration-edit.js index 27b45ae9f..1085ce6dd 100644 --- a/ui/tests/helpers/pki/page/pki-configuration-edit.js +++ b/ui/tests/helpers/pki/page/pki-configuration-edit.js @@ -16,4 +16,6 @@ export const SELECTORS = { cancelButton: '[data-test-configuration-edit-cancel]', validationAlert: '[data-test-configuration-edit-validation-alert]', deleteButton: (attr) => `[data-test-input="${attr}"] [data-test-string-list-button="delete"]`, + groupHeader: (group) => `[data-test-crl-header="${group}"]`, + checkboxInput: (attr) => `[data-test-input="${attr}"]`, }; diff --git a/ui/tests/integration/components/pki/page/pki-configuration-details-test.js b/ui/tests/integration/components/pki/page/pki-configuration-details-test.js index e283ab166..c9419e85f 100644 --- a/ui/tests/integration/components/pki/page/pki-configuration-details-test.js +++ b/ui/tests/integration/components/pki/page/pki-configuration-details-test.js @@ -35,6 +35,9 @@ module('Integration | Component | Page::PkiConfigurationDetails', function (hook deltaRebuildInterval: '15m', ocspExpiry: '77h', ocspDisable: false, + crossClusterRevocation: true, + unifiedCrl: true, + unifiedCrlOnExistingPaths: true, }); this.mountConfig = { id: 'pki-test', @@ -142,6 +145,33 @@ module('Integration | Component | Page::PkiConfigurationDetails', function (hook assert.dom(SELECTORS.rowValue('Delta rebuild interval')).doesNotExist(); }); + test('it renders enterprise params in crl section', async function (assert) { + this.version = this.owner.lookup('service:version'); + this.version.version = '1.13.1+ent'; + await render( + hbs`,`, + { owner: this.engine } + ); + assert.dom(SELECTORS.rowValue('Cross-cluster revocation')).hasText('Yes'); + assert.dom(SELECTORS.rowIcon('Cross-cluster revocation', 'check-circle')); + assert.dom(SELECTORS.rowValue('Unified CRL')).hasText('Yes'); + assert.dom(SELECTORS.rowIcon('Unified CRL', 'check-circle')); + assert.dom(SELECTORS.rowValue('Unified CRL on existing paths')).hasText('Yes'); + assert.dom(SELECTORS.rowIcon('Unified CRL on existing paths', 'check-circle')); + }); + + test('it does not render enterprise params in crl section', async function (assert) { + this.version = this.owner.lookup('service:version'); + this.version.version = '1.13.1'; + await render( + hbs`,`, + { owner: this.engine } + ); + assert.dom(SELECTORS.rowValue('Cross-cluster revocation')).doesNotExist(); + assert.dom(SELECTORS.rowValue('Unified CRL')).doesNotExist(); + assert.dom(SELECTORS.rowValue('Unified CRL on existing paths')).doesNotExist(); + }); + test('shows the correct information on mount configuration section', async function (assert) { await render( hbs`,`, diff --git a/ui/tests/integration/components/pki/page/pki-configuration-edit-test.js b/ui/tests/integration/components/pki/page/pki-configuration-edit-test.js index 5964f7d07..1879faa8b 100644 --- a/ui/tests/integration/components/pki/page/pki-configuration-edit-test.js +++ b/ui/tests/integration/components/pki/page/pki-configuration-edit-test.js @@ -208,4 +208,94 @@ module('Integration | Component | page/pki-configuration-edit', function (hooks) await click(SELECTORS.saveButton); }); + + test('it renders enterprise only params', async function (assert) { + assert.expect(6); + this.version = this.owner.lookup('service:version'); + this.version.version = '1.13.1+ent'; + this.server.post(`/${this.backend}/config/crl`, (schema, req) => { + assert.ok(true, 'request made to save crl config'); + assert.propEqual( + JSON.parse(req.requestBody), + { + auto_rebuild: false, + auto_rebuild_grace_period: '12h', + delta_rebuild_interval: '15m', + disable: false, + enable_delta: false, + expiry: '72h', + ocsp_disable: false, + ocsp_expiry: '12h', + cross_cluster_revocation: true, + unified_crl: true, + unified_crl_on_existing_paths: true, + }, + 'crl payload includes enterprise params' + ); + }); + this.server.post(`/${this.backend}/config/urls`, () => { + assert.ok(true, 'request made to save urls config'); + }); + await render( + hbs` + + `, + this.context + ); + assert.dom(SELECTORS.groupHeader('Certificate Revocation List (CRL)')).exists(); + assert.dom(SELECTORS.groupHeader('Online Certificate Status Protocol (OCSP)')).exists(); + assert.dom(SELECTORS.groupHeader('Unified Revocation')).exists(); + await click(SELECTORS.checkboxInput('crossClusterRevocation')); + await click(SELECTORS.checkboxInput('unifiedCrl')); + await click(SELECTORS.checkboxInput('unifiedCrlOnExistingPaths')); + await click(SELECTORS.saveButton); + }); + + test('it renders does not render enterprise only params for OSS', async function (assert) { + assert.expect(9); + this.version = this.owner.lookup('service:version'); + this.version.version = '1.13.1'; + this.server.post(`/${this.backend}/config/crl`, (schema, req) => { + assert.ok(true, 'request made to save crl config'); + assert.propEqual( + JSON.parse(req.requestBody), + { + auto_rebuild: false, + auto_rebuild_grace_period: '12h', + delta_rebuild_interval: '15m', + disable: false, + enable_delta: false, + expiry: '72h', + ocsp_disable: false, + ocsp_expiry: '12h', + }, + 'crl payload does not include enterprise params' + ); + }); + this.server.post(`/${this.backend}/config/urls`, () => { + assert.ok(true, 'request made to save urls config'); + }); + await render( + hbs` + + `, + this.context + ); + + assert.dom(SELECTORS.checkboxInput('crossClusterRevocation')).doesNotExist(); + assert.dom(SELECTORS.checkboxInput('unifiedCrl')).doesNotExist(); + assert.dom(SELECTORS.checkboxInput('unifiedCrlOnExistingPaths')).doesNotExist(); + assert.dom(SELECTORS.groupHeader('Certificate Revocation List (CRL)')).exists(); + assert.dom(SELECTORS.groupHeader('Online Certificate Status Protocol (OCSP)')).exists(); + assert.dom(SELECTORS.groupHeader('Unified Revocation')).doesNotExist(); + await click(SELECTORS.saveButton); + }); }); diff --git a/ui/tests/unit/services/version-test.js b/ui/tests/unit/services/version-test.js index e9e7d8fed..5d10b25e2 100644 --- a/ui/tests/unit/services/version-test.js +++ b/ui/tests/unit/services/version-test.js @@ -18,14 +18,14 @@ module('Unit | Service | version', function (hooks) { test('setting version computes isEnterprise properly', function (assert) { const service = this.owner.lookup('service:version'); - service.set('version', '0.9.5+prem'); + service.set('version', '0.9.5+ent'); assert.false(service.get('isOSS')); assert.true(service.get('isEnterprise')); }); test('setting version with hsm ending computes isEnterprise properly', function (assert) { const service = this.owner.lookup('service:version'); - service.set('version', '0.9.5+prem.hsm'); + service.set('version', '0.9.5+ent.hsm'); assert.false(service.get('isOSS')); assert.true(service.get('isEnterprise')); }); @@ -33,14 +33,14 @@ module('Unit | Service | version', function (hooks) { test('hasPerfReplication', function (assert) { const service = this.owner.lookup('service:version'); assert.false(service.get('hasPerfReplication')); - service.set('_features', ['Performance Replication']); + service.set('features', ['Performance Replication']); assert.true(service.get('hasPerfReplication')); }); test('hasDRReplication', function (assert) { const service = this.owner.lookup('service:version'); assert.false(service.get('hasDRReplication')); - service.set('_features', ['DR Replication']); + service.set('features', ['DR Replication']); assert.true(service.get('hasDRReplication')); }); });