From cd30860cb68c698f87b320a9a7bc901357d99cb3 Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Thu, 12 Jan 2023 16:33:14 -0700 Subject: [PATCH] PKI Issuer Edit (#18687) * adds pki issuer edit view * updates pki issuer details test and fixes styling issue in issuer edit form * addresses feedback --- ui/app/adapters/pki/issuer.js | 7 + ui/app/models/pki/issuer.js | 93 ++++++++--- ui/app/serializers/pki/issuer.js | 19 +++ .../addon/components/page/pki-issuer-edit.hbs | 77 ++++++++++ .../addon/components/page/pki-issuer-edit.ts | 57 +++++++ ui/lib/pki/addon/routes/index.js | 10 ++ .../pki/addon/routes/issuers/issuer/edit.js | 4 +- .../addon/templates/issuers/issuer/edit.hbs | 13 +- ui/package.json | 1 + .../pki/pki-engine-workflow-test.js | 2 +- .../pki/page/pki-issuer-edit-test.js | 145 ++++++++++++++++++ ui/types/vault/app-types.ts | 4 + ui/types/vault/models/pki/issuer.d.ts | 29 ++++ 13 files changed, 439 insertions(+), 22 deletions(-) create mode 100644 ui/lib/pki/addon/components/page/pki-issuer-edit.hbs create mode 100644 ui/lib/pki/addon/components/page/pki-issuer-edit.ts create mode 100644 ui/lib/pki/addon/routes/index.js create mode 100644 ui/tests/integration/components/pki/page/pki-issuer-edit-test.js create mode 100644 ui/types/vault/models/pki/issuer.d.ts diff --git a/ui/app/adapters/pki/issuer.js b/ui/app/adapters/pki/issuer.js index bef1bfe89..1cf5780fa 100644 --- a/ui/app/adapters/pki/issuer.js +++ b/ui/app/adapters/pki/issuer.js @@ -37,6 +37,13 @@ export default class PkiIssuerAdapter extends ApplicationAdapter { }); } + updateRecord(store, type, snapshot) { + const { backend, issuerId } = snapshot.record; + const data = this.serialize(snapshot); + const url = this.urlForQuery(backend, issuerId); + return this.ajax(url, 'POST', { data }); + } + query(store, type, query) { return this.ajax(this.urlForQuery(query.backend), 'GET', this.optionsForQuery()); } diff --git a/ui/app/models/pki/issuer.js b/ui/app/models/pki/issuer.js index 9e94d6a25..3dccc5d1f 100644 --- a/ui/app/models/pki/issuer.js +++ b/ui/app/models/pki/issuer.js @@ -3,25 +3,31 @@ import { attr } from '@ember-data/model'; import { withFormFields } from 'vault/decorators/model-form-fields'; import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities'; -@withFormFields(null, [ - { - default: [ - 'certificate', - 'caChain', - 'commonName', - 'issuerName', - 'notValidBefore', - 'serialNumber', - 'keyId', - 'uriSans', - 'notValidAfter', - ], - }, - { 'Issuer URLs': ['issuingCertificates', 'crlDistributionPoints', 'ocspServers', 'deltaCrlUrls'] }, -]) +const issuerUrls = ['issuingCertificates', 'crlDistributionPoints', 'ocspServers']; +@withFormFields( + ['issuerName', 'leafNotAfterBehavior', 'usage', 'manualChain', ...issuerUrls], + [ + { + default: [ + 'certificate', + 'caChain', + 'commonName', + 'issuerName', + 'notValidBefore', + 'serialNumber', + 'keyId', + 'uriSans', + 'notValidAfter', + ], + }, + { 'Issuer URLs': issuerUrls }, + ] +) export default class PkiIssuerModel extends PkiCertificateBaseModel { - getHelpUrl(backend) { - return `/v1/${backend}/issuer/example?help=1`; + // there are too many differences between what openAPI returns and the designs for the update form + // manually defining the attrs instead with the correct meta data + get useOpenAPI() { + return false; } @attr('string') issuerId; @@ -42,6 +48,57 @@ export default class PkiIssuerModel extends PkiCertificateBaseModel { }) uriSans; + @attr('string') issuerName; + + @attr({ + label: 'Leaf notAfter behavior', + subText: + 'What happens when a leaf certificate is issued, but its NotAfter field (and therefore its expiry date) exceeds that of this issuer.', + docLink: '/vault/api-docs/secret/pki#update-issuer', + editType: 'yield', + valueOptions: ['err', 'truncate', 'permit'], + }) + leafNotAfterBehavior; + + @attr({ + label: 'Usage', + subText: 'Allowed usages for this issuer. It can always be read', + editType: 'yield', + valueOptions: [ + { label: 'Issuing certificates', value: 'issuing-certificates' }, + { label: 'Signing CRLs', value: 'crl-signing' }, + { label: 'Signing OCSPs', value: 'ocsp-signing' }, + ], + }) + usage; + + @attr('string', { + label: 'Manual chain', + subText: + "An advanced field useful when automatic chain building isn't desired. The first element must be the present issuer's reference.", + }) + manualChain; + + @attr('string', { + label: 'Issuing certificates', + subText: + 'The URL values for the Issuing Certificate field. These are different URLs for the same resource, and should be added individually, not in a comma-separated list.', + editType: 'stringArray', + }) + issuingCertificates; + + @attr('string', { + label: 'CRL distribution points', + subText: 'Specifies the URL values for the CRL Distribution Points field.', + }) + crlDistributionPoints; + + @attr('string', { + label: 'OCSP servers', + subText: 'Specifies the URL values for the OCSP Servers field.', + }) + ocspServers; + @lazyCapabilities(apiPath`${'backend'}/issuer/${'issuerId'}`) issuerPath; @lazyCapabilities(apiPath`${'backend'}/root/rotate/exported`) rotateExported; @lazyCapabilities(apiPath`${'backend'}/root/rotate/internal`) rotateInternal; diff --git a/ui/app/serializers/pki/issuer.js b/ui/app/serializers/pki/issuer.js index 6457e0368..b8e0bff99 100644 --- a/ui/app/serializers/pki/issuer.js +++ b/ui/app/serializers/pki/issuer.js @@ -4,6 +4,25 @@ import ApplicationSerializer from '../application'; export default class PkiIssuerSerializer extends ApplicationSerializer { primaryKey = 'issuer_id'; + constructor() { + super(...arguments); + // remove following attrs from serialization + const attrs = [ + 'caChain', + 'certificate', + 'commonName', + 'issuerId', + 'keyId', + 'notValidAfter', + 'notValidBefore', + 'serialNumber', + ]; + this.attrs = attrs.reduce((attrObj, attr) => { + attrObj[attr] = { serialize: false }; + return attrObj; + }, {}); + } + normalizeResponse(store, primaryModelClass, payload, id, requestType) { if (payload.data.certificate) { // Parse certificate back from the API and add to payload diff --git a/ui/lib/pki/addon/components/page/pki-issuer-edit.hbs b/ui/lib/pki/addon/components/page/pki-issuer-edit.hbs new file mode 100644 index 000000000..37031a41f --- /dev/null +++ b/ui/lib/pki/addon/components/page/pki-issuer-edit.hbs @@ -0,0 +1,77 @@ +
+
+ + {{#each @model.formFields as |field|}} + {{#if (eq field.name "issuingCertificates")}} +
+

+ Issuer URLs +

+
+ {{/if}} + + {{#if (eq field.name "usage")}} + {{#each field.options.valueOptions as |option|}} +
+ + +
+ {{/each}} + {{else if (eq field.name "leafNotAfterBehavior")}} +
+
+ +
+
+ {{/if}} + + {{/each}} +
+
+ + + {{#if this.error}} +
+ +
+ {{/if}} +
+
\ No newline at end of file diff --git a/ui/lib/pki/addon/components/page/pki-issuer-edit.ts b/ui/lib/pki/addon/components/page/pki-issuer-edit.ts new file mode 100644 index 000000000..76e9daeaa --- /dev/null +++ b/ui/lib/pki/addon/components/page/pki-issuer-edit.ts @@ -0,0 +1,57 @@ +import Component from '@glimmer/component'; +import { tracked } from '@glimmer/tracking'; +import { inject as service } from '@ember/service'; +import { action } from '@ember/object'; +import { task } from 'ember-concurrency'; +import { waitFor } from '@ember/test-waiters'; +import errorMessage from 'vault/utils/error-message'; +import RouterService from '@ember/routing/router-service'; +import FlashMessageService from 'vault/services/flash-messages'; +import PkiIssuerModel from 'vault/models/pki/issuer'; + +interface Args { + model: PkiIssuerModel; +} + +export default class PkiIssuerEditComponent extends Component { + @service declare readonly router: RouterService; + @service declare readonly flashMessages: FlashMessageService; + + @tracked usageValues: Array = []; + @tracked error = null; + + constructor(owner: unknown, args: Args) { + super(owner, args); + this.usageValues = (this.args.model.usage || '').split(','); + } + + toDetails() { + this.router.transitionTo('vault.cluster.secrets.backend.pki.issuers.issuer.details'); + } + + @action + setUsage(value: string) { + const method = this.usageValues.includes(value) ? 'removeObject' : 'addObject'; + this.usageValues[method](value); + this.args.model.usage = this.usageValues.join(','); + } + + @task + @waitFor + *save(event: Event) { + event.preventDefault(); + try { + yield this.args.model.save(); + this.flashMessages.success('Successfully updated issuer'); + this.toDetails(); + } catch (error) { + this.error = errorMessage(error); + } + } + + @action + cancel() { + this.args.model.rollbackAttributes(); + this.toDetails(); + } +} diff --git a/ui/lib/pki/addon/routes/index.js b/ui/lib/pki/addon/routes/index.js new file mode 100644 index 000000000..c928aa93f --- /dev/null +++ b/ui/lib/pki/addon/routes/index.js @@ -0,0 +1,10 @@ +import Route from '@ember/routing/route'; +import { inject as service } from '@ember/service'; + +export default class PkiRoute extends Route { + @service router; + + redirect() { + this.router.transitionTo('vault.cluster.secrets.backend.pki.overview'); + } +} diff --git a/ui/lib/pki/addon/routes/issuers/issuer/edit.js b/ui/lib/pki/addon/routes/issuers/issuer/edit.js index 31614efcf..9819512d7 100644 --- a/ui/lib/pki/addon/routes/issuers/issuer/edit.js +++ b/ui/lib/pki/addon/routes/issuers/issuer/edit.js @@ -1,3 +1,3 @@ -import Route from '@ember/routing/route'; +import PkiIssuerDetailsRoute from './details'; -export default class PkiIssuerEditRoute extends Route {} +export default class PkiIssuerEditRoute extends PkiIssuerDetailsRoute {} diff --git a/ui/lib/pki/addon/templates/issuers/issuer/edit.hbs b/ui/lib/pki/addon/templates/issuers/issuer/edit.hbs index 62fec9342..f7e2eb423 100644 --- a/ui/lib/pki/addon/templates/issuers/issuer/edit.hbs +++ b/ui/lib/pki/addon/templates/issuers/issuer/edit.hbs @@ -1 +1,12 @@ -route: issuers.issuer.edit \ No newline at end of file + + + + + +

+ Update issuer +

+ + + + \ No newline at end of file diff --git a/ui/package.json b/ui/package.json index 252e017b3..dcf512378 100644 --- a/ui/package.json +++ b/ui/package.json @@ -30,6 +30,7 @@ "test:oss": "yarn run test -f='!enterprise'", "test:quick": "node scripts/start-vault.js", "test:quick-oss": "yarn test:quick -f='!enterprise'", + "types:declare": "declare () { yarn tsc $1 --declaration --allowJs --emitDeclarationOnly --experimentalDecorators --outDir $2; }; declare", "vault": "VAULT_REDIRECT_ADDR=http://127.0.0.1:8200 vault server -log-level=error -dev -dev-root-token-id=root -dev-ha -dev-transactional", "vault:cluster": "VAULT_REDIRECT_ADDR=http://127.0.0.1:8202 vault server -log-level=error -dev -dev-root-token-id=root -dev-listen-address=127.0.0.1:8202 -dev-ha -dev-transactional" }, diff --git a/ui/tests/acceptance/pki/pki-engine-workflow-test.js b/ui/tests/acceptance/pki/pki-engine-workflow-test.js index 035a554d3..6ae9d11bc 100644 --- a/ui/tests/acceptance/pki/pki-engine-workflow-test.js +++ b/ui/tests/acceptance/pki/pki-engine-workflow-test.js @@ -383,7 +383,7 @@ module('Acceptance | pki workflow', function (hooks) { .exists({ count: 9 }, 'Renders 9 info table items under default group'); assert .dom(`${SELECTORS.issuerDetails.urlsGroup} ${SELECTORS.issuerDetails.row}`) - .exists({ count: 4 }, 'Renders 4 info table items under URLs group'); + .exists({ count: 3 }, 'Renders 4 info table items under URLs group'); assert.dom(SELECTORS.issuerDetails.groupTitle).exists({ count: 1 }, 'only 1 group title rendered'); }); }); diff --git a/ui/tests/integration/components/pki/page/pki-issuer-edit-test.js b/ui/tests/integration/components/pki/page/pki-issuer-edit-test.js new file mode 100644 index 000000000..a1a7c46e3 --- /dev/null +++ b/ui/tests/integration/components/pki/page/pki-issuer-edit-test.js @@ -0,0 +1,145 @@ +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import { click, fillIn, render } from '@ember/test-helpers'; +import { hbs } from 'ember-cli-htmlbars'; +import { setupEngine } from 'ember-engines/test-support'; +import { setupMirage } from 'ember-cli-mirage/test-support'; +import { Response } from 'miragejs'; +import sinon from 'sinon'; + +const selectors = { + name: '[data-test-input="issuerName"]', + leaf: '[data-test-field="leafNotAfterBehavior"] select', + leafOption: '[data-test-field="leafNotAfterBehavior"] option', + usageCert: '[data-test-usage="Issuing certificates"]', + usageCrl: '[data-test-usage="Signing CRLs"]', + usageOcsp: '[data-test-usage="Signing OCSPs"]', + manualChain: '[data-test-input="manualChain"]', + certUrls: '[data-test-string-list-input]', + certUrl1: '[data-test-string-list-input="0"]', + certUrl2: '[data-test-string-list-input="1"]', + certUrlAdd: '[data-test-string-list-button="add"]', + certUrlRemove: '[data-test-string-list-button="delete"]', + crlDist: '[data-test-input="crlDistributionPoints"]', + ocspServers: '[data-test-input="ocspServers"]', + save: '[data-test-save]', + cancel: '[data-test-cancel]', + error: '[data-test-error] p', + alert: '[data-test-inline-error-message]', +}; + +module('Integration | Component | pki | Page::PkiIssuerEditPage::PkiIssuerEdit', function (hooks) { + setupRenderingTest(hooks); + setupEngine(hooks, 'pki'); + setupMirage(hooks); + + hooks.beforeEach(function () { + const router = this.owner.lookup('service:router'); + const transitionSpy = sinon.stub(router, 'transitionTo'); + this.transitionCalled = () => + transitionSpy.calledWith('vault.cluster.secrets.backend.pki.issuers.issuer.details'); + + const store = this.owner.lookup('service:store'); + store.pushPayload('pki/issuer', { + modelName: 'pki/issuer', + data: { + issuer_id: 'test', + issuer_name: 'foo-bar', + leaf_not_after_behavior: 'err', + usage: 'read-only,issuing-certificates,ocsp-signing', + manual_chain: 'issuer_ref', + issuing_certificates: ['http://localhost', 'http://localhost:8200'], + crl_distribution_points: 'http://localhost', + ocsp_servers: 'http://localhost', + }, + }); + this.model = store.peekRecord('pki/issuer', 'test'); + // backend value on model pulled from secretMountPath service + this.owner.lookup('service:secretMountPath').update('pki'); + + this.update = async () => { + await fillIn(selectors.name, 'bar-baz'); + await click(selectors.usageCrl); + await click(selectors.certUrlRemove); + }; + }); + + test('it should populate fields with model values', async function (assert) { + await render(hbs``, { owner: this.engine }); + + assert.dom(selectors.name).hasValue(this.model.issuerName, 'Issuer name field populates'); + assert + .dom(selectors.leaf) + .hasValue(this.model.leafNotAfterBehavior, 'Leaf not after behavior option selected'); + assert + .dom(selectors.leafOption) + .hasText( + 'Error if the computed NotAfter exceeds that of this issuer', + 'Correct text renders for leaf option' + ); + assert.dom(selectors.usageCert).isChecked('Usage issuing certificates is checked'); + assert.dom(selectors.usageCrl).isNotChecked('Usage signing crls is not checked'); + assert.dom(selectors.usageOcsp).isChecked('Usage signing ocsps is checked'); + assert.dom(selectors.manualChain).hasValue(this.model.manualChain, 'Manual chain field populates'); + const certUrls = this.model.issuingCertificates.split(','); + assert.dom(selectors.certUrl1).hasValue(certUrls[0], 'Issuing certificate populates'); + assert.dom(selectors.certUrl2).hasValue(certUrls[1], 'Issuing certificate populates'); + assert + .dom(selectors.crlDist) + .hasValue(this.model.crlDistributionPoints, 'Crl distribution points populate'); + assert.dom(selectors.ocspServers).hasValue(this.model.ocspServers, 'Ocsp servers populate'); + }); + + test('it should rollback model attributes on cancel', async function (assert) { + await render(hbs``, { owner: this.engine }); + + await this.update(); + await click(selectors.cancel); + + assert.ok(this.transitionCalled(), 'Transitions to details route on cancel'); + assert.strictEqual(this.model.issuerName, 'foo-bar', 'Issuer name rolled back'); + assert.strictEqual(this.model.usage, 'read-only,issuing-certificates,ocsp-signing', 'Usage rolled back'); + assert.strictEqual( + this.model.issuingCertificates, + 'http://localhost,http://localhost:8200', + 'Issuing certificates rolled back' + ); + }); + + test('it should update issuer', async function (assert) { + assert.expect(4); + + this.server.post('/pki/issuer/test', (schema, req) => { + const data = JSON.parse(req.requestBody); + assert.strictEqual(data.issuer_name, 'bar-baz', 'Updated issuer name sent in POST request'); + assert.strictEqual( + data.usage, + 'read-only,issuing-certificates,ocsp-signing,crl-signing', + 'Updated usage sent in POST request' + ); + assert.strictEqual( + data.issuing_certificates, + 'http://localhost:8200', + 'Updated issuing certificates sent in POST request' + ); + return { data }; + }); + await render(hbs``, { owner: this.engine }); + + await this.update(); + await click(selectors.save); + assert.ok(this.transitionCalled(), 'Transitions to details route on save success'); + }); + + test('it should show error messages', async function (assert) { + this.server.post('/pki/issuer/test', () => new Response(404, {}, { errors: ['Some error occurred'] })); + + await render(hbs``, { owner: this.engine }); + await click(selectors.save); + + assert + .dom(selectors.alert) + .hasText('There was an error submitting this form.', 'Inline error alert renders'); + assert.dom(selectors.error).hasText('Some error occurred', 'Error message renders'); + }); +}); diff --git a/ui/types/vault/app-types.ts b/ui/types/vault/app-types.ts index 5369c73e5..f221d14e1 100644 --- a/ui/types/vault/app-types.ts +++ b/ui/types/vault/app-types.ts @@ -4,3 +4,7 @@ export interface FormField { type: string; options: unknown; } + +export interface FormFieldGroups { + [key: string]: Array; +} diff --git a/ui/types/vault/models/pki/issuer.d.ts b/ui/types/vault/models/pki/issuer.d.ts new file mode 100644 index 000000000..b5a59e7de --- /dev/null +++ b/ui/types/vault/models/pki/issuer.d.ts @@ -0,0 +1,29 @@ +import PkiCertificateBaseModel from './certificate/base'; +import { FormField, FormFieldGroups } from 'vault/app-types'; + +export default class PkiIssuerModel extends PkiCertificateBaseModel { + useOpenAPI(): boolean; + issuerId: string; + keyId: string; + uriSans: string; + issuerName: string; + leafNotAfterBehavior: string; + usage: string; + manualChain: string; + issuingCertificates: string; + crlDistributionPoints: string; + ocspServers: string; + /** these are all instances of the capabilities model which should be converted to native class and typed + rotateExported: any; + rotateInternal: any; + rotateExisting: any; + crossSignPath: any; + signIntermediate: any; + -------------------- **/ + formFields: Array; + formFieldGroups: FormFieldGroups; + get canRotateIssuer(): boolean; + get canCrossSign(): boolean; + get canSignIntermediate(): boolean; + get canConfigure(): boolean; +}