From f589a07b3dd5fbfa0dfee881ca129929f0c6787c Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Tue, 21 Feb 2023 16:54:09 -0800 Subject: [PATCH 1/7] move validations to base certificate --- ui/app/models/pki/certificate/base.js | 5 +++++ ui/app/models/pki/certificate/generate.js | 6 ------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/ui/app/models/pki/certificate/base.js b/ui/app/models/pki/certificate/base.js index c1f9346050ea..a5799f45a643 100644 --- a/ui/app/models/pki/certificate/base.js +++ b/ui/app/models/pki/certificate/base.js @@ -3,6 +3,7 @@ import { assert } from '@ember/debug'; import { service } from '@ember/service'; import { withFormFields } from 'vault/decorators/model-form-fields'; import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities'; +import { withModelValidations } from 'vault/decorators/model-validations'; /** * There are many ways to generate a cert, but we want to display them in a consistent way. @@ -19,7 +20,11 @@ const certDisplayFields = [ 'notValidBefore', 'notValidAfter', ]; +const validations = { + commonName: [{ type: 'presence', message: 'Common name is required.' }], +}; +@withModelValidations(validations) @withFormFields(certDisplayFields) export default class PkiCertificateBaseModel extends Model { @service secretMountPath; diff --git a/ui/app/models/pki/certificate/generate.js b/ui/app/models/pki/certificate/generate.js index 4d8a50f7a585..10c633e678b9 100644 --- a/ui/app/models/pki/certificate/generate.js +++ b/ui/app/models/pki/certificate/generate.js @@ -1,6 +1,5 @@ import { attr } from '@ember-data/model'; import { withFormFields } from 'vault/decorators/model-form-fields'; -import { withModelValidations } from 'vault/decorators/model-validations'; import PkiCertificateBaseModel from './base'; const generateFromRole = [ @@ -20,11 +19,6 @@ const generateFromRole = [ 'More Options': ['format', 'privateKeyFormat'], }, ]; -const validations = { - commonName: [{ type: 'presence', message: 'Common name is required.' }], -}; - -@withModelValidations(validations) @withFormFields(null, generateFromRole) export default class PkiCertificateGenerateModel extends PkiCertificateBaseModel { getHelpUrl(backend) { From bbb35bb0a07e268c7ec9382b2915d4b963f13370 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Tue, 21 Feb 2023 17:05:58 -0800 Subject: [PATCH 2/7] add test --- .../components/pki/pki-role-generate-test.js | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/ui/tests/integration/components/pki/pki-role-generate-test.js b/ui/tests/integration/components/pki/pki-role-generate-test.js index 35b90507a8c2..7a1995ca02ea 100644 --- a/ui/tests/integration/components/pki/pki-role-generate-test.js +++ b/ui/tests/integration/components/pki/pki-role-generate-test.js @@ -18,7 +18,7 @@ module('Integration | Component | pki-role-generate', function (hooks) { this.store = this.owner.lookup('service:store'); this.secretMountPath = this.owner.lookup('service:secret-mount-path'); this.secretMountPath.currentPath = 'pki-test'; - this.model = this.store.createRecord('pki/certificate/generate', { + this.modelGenerate = this.store.createRecord('pki/certificate/generate', { role: 'my-role', }); this.onSuccess = Sinon.spy(); @@ -30,7 +30,7 @@ module('Integration | Component | pki-role-generate', function (hooks) { hbs`
@@ -41,7 +41,7 @@ module('Integration | Component | pki-role-generate', function (hooks) { assert.dom(SELECTORS.commonNameField).exists('shows the common name field'); assert.dom(SELECTORS.optionsToggle).exists('toggle exists'); await fillIn(SELECTORS.commonNameField, 'example.com'); - assert.strictEqual(this.model.commonName, 'example.com', 'Filling in the form updates the model'); + assert.strictEqual(this.modelGenerate.commonName, 'example.com', 'Filling in the form updates the model'); }); test('it should render the component displaying the cert', async function (assert) { @@ -70,14 +70,39 @@ module('Integration | Component | pki-role-generate', function (hooks) { assert.dom(SELECTORS.serialNumber).hasText('abcd-efgh-ijkl', 'shows serial number info row'); }); - test('it should display validation errors', async function (assert) { + test('it should display validation errors for cert generation', async function (assert) { assert.expect(3); await render( hbs`
+
+ `, + { owner: this.engine } + ); + await click(SELECTORS.generateButton); + + assert + .dom(SELECTORS.commonNameInlineError) + .hasText('Common name is required.', 'Common name validation error renders'); + assert.dom(SELECTORS.inlineAlert).hasText('There is an error with this form.', 'Alert renders'); + assert.dom(SELECTORS.commonNameErrorBorder).hasClass('has-error-border'); + }); + + test('it should display validation errors for cert signing', async function (assert) { + assert.expect(3); + this.modelSign = this.store.createRecord('pki/certificate/sign', { + role: 'my-role', + }); + await render( + hbs` +
+
From 58d5565e381d10f303211cfe97fd1fcb1a6fb6d3 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Tue, 21 Feb 2023 18:33:11 -0800 Subject: [PATCH 3/7] add typescript declarations --- ui/lib/pki/addon/components/pki-role-generate.ts | 3 ++- ui/types/vault/models/pki/certificate/base.d.ts | 1 + ui/types/vault/models/pki/certificate/generate.d.ts | 8 ++------ ui/types/vault/models/pki/certificate/sign.d.ts | 3 +++ 4 files changed, 8 insertions(+), 7 deletions(-) create mode 100644 ui/types/vault/models/pki/certificate/sign.d.ts diff --git a/ui/lib/pki/addon/components/pki-role-generate.ts b/ui/lib/pki/addon/components/pki-role-generate.ts index d8928bdc4c70..bfb7fd04f3aa 100644 --- a/ui/lib/pki/addon/components/pki-role-generate.ts +++ b/ui/lib/pki/addon/components/pki-role-generate.ts @@ -9,10 +9,11 @@ import errorMessage from 'vault/utils/error-message'; import FlashMessageService from 'vault/services/flash-messages'; import DownloadService from 'vault/services/download'; import PkiCertificateGenerateModel from 'vault/models/pki/certificate/generate'; +import PkiCertificateSignModel from 'vault/models/pki/certificate/sign'; interface Args { onSuccess: CallableFunction; - model: PkiCertificateGenerateModel; + model: PkiCertificateGenerateModel | PkiCertificateSignModel; type: string; } diff --git a/ui/types/vault/models/pki/certificate/base.d.ts b/ui/types/vault/models/pki/certificate/base.d.ts index f57e00d062b9..6e891dc016f2 100644 --- a/ui/types/vault/models/pki/certificate/base.d.ts +++ b/ui/types/vault/models/pki/certificate/base.d.ts @@ -19,4 +19,5 @@ export default class PkiCertificateBaseModel extends Model { importedKeys: string[]; revokePath: string; get canRevoke(): boolean; + validate(): ModelValidations; } diff --git a/ui/types/vault/models/pki/certificate/generate.d.ts b/ui/types/vault/models/pki/certificate/generate.d.ts index 089261a09a82..ab7c9d983ad4 100644 --- a/ui/types/vault/models/pki/certificate/generate.d.ts +++ b/ui/types/vault/models/pki/certificate/generate.d.ts @@ -1,14 +1,10 @@ -import Model from '@ember-data/model'; import { FormField } from 'vault/app-types'; +import PkiCertificateBaseModel from './base'; -export default class PkiCertificateGenerateModel extends Model { +export default class PkiCertificateGenerateModel extends PkiCertificateBaseModel { name: string; - backend: string; - serialNumber: string; - certificate: string; formFields: FormField[]; formFieldsGroup: { [k: string]: FormField[]; }[]; - validate(): ModelValidations; } diff --git a/ui/types/vault/models/pki/certificate/sign.d.ts b/ui/types/vault/models/pki/certificate/sign.d.ts new file mode 100644 index 000000000000..27d4b166d4a8 --- /dev/null +++ b/ui/types/vault/models/pki/certificate/sign.d.ts @@ -0,0 +1,3 @@ +import PkiCertificateBaseModel from './base'; + +export default class PkiCertificateGenerateModel extends PkiCertificateBaseModel {} From a2fecfac4209ed243a02c59894d42b55a3c1c956 Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Tue, 21 Feb 2023 18:44:14 -0800 Subject: [PATCH 4/7] remove validations all together --- ui/app/models/pki/certificate/base.js | 5 ----- ui/lib/pki/addon/components/pki-role-generate.hbs | 5 ----- ui/lib/pki/addon/components/pki-role-generate.ts | 9 --------- 3 files changed, 19 deletions(-) diff --git a/ui/app/models/pki/certificate/base.js b/ui/app/models/pki/certificate/base.js index a5799f45a643..c1f9346050ea 100644 --- a/ui/app/models/pki/certificate/base.js +++ b/ui/app/models/pki/certificate/base.js @@ -3,7 +3,6 @@ import { assert } from '@ember/debug'; import { service } from '@ember/service'; import { withFormFields } from 'vault/decorators/model-form-fields'; import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities'; -import { withModelValidations } from 'vault/decorators/model-validations'; /** * There are many ways to generate a cert, but we want to display them in a consistent way. @@ -20,11 +19,7 @@ const certDisplayFields = [ 'notValidBefore', 'notValidAfter', ]; -const validations = { - commonName: [{ type: 'presence', message: 'Common name is required.' }], -}; -@withModelValidations(validations) @withFormFields(certDisplayFields) export default class PkiCertificateBaseModel extends Model { @service secretMountPath; diff --git a/ui/lib/pki/addon/components/pki-role-generate.hbs b/ui/lib/pki/addon/components/pki-role-generate.hbs index 37e4f249ee59..87f3e7d17a4e 100644 --- a/ui/lib/pki/addon/components/pki-role-generate.hbs +++ b/ui/lib/pki/addon/components/pki-role-generate.hbs @@ -40,11 +40,6 @@ Cancel - {{#if this.invalidFormAlert}} -
- -
- {{/if}} {{/if}} \ No newline at end of file diff --git a/ui/lib/pki/addon/components/pki-role-generate.ts b/ui/lib/pki/addon/components/pki-role-generate.ts index bfb7fd04f3aa..2f33710426ed 100644 --- a/ui/lib/pki/addon/components/pki-role-generate.ts +++ b/ui/lib/pki/addon/components/pki-role-generate.ts @@ -24,8 +24,6 @@ export default class PkiRoleGenerate extends Component { @service declare readonly download: DownloadService; @tracked errorBanner = ''; - @tracked invalidFormAlert = ''; - @tracked modelValidations = null; get verb() { return this.args.type === 'sign' ? 'sign' : 'generate'; @@ -36,13 +34,6 @@ export default class PkiRoleGenerate extends Component { evt.preventDefault(); this.errorBanner = ''; const { model, onSuccess } = this.args; - const { isValid, state, invalidFormMessage } = model.validate(); - - this.modelValidations = isValid ? null : state; - this.invalidFormAlert = invalidFormMessage; - - if (!isValid) return; - try { yield model.save(); onSuccess(); From 7c3db184c02c9b8f14ac3b81d1b79f825ceeff7c Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Tue, 21 Feb 2023 18:45:07 -0800 Subject: [PATCH 5/7] remove validations from tests --- .../components/pki/pki-role-generate-test.js | 56 ++----------------- 1 file changed, 4 insertions(+), 52 deletions(-) diff --git a/ui/tests/integration/components/pki/pki-role-generate-test.js b/ui/tests/integration/components/pki/pki-role-generate-test.js index 7a1995ca02ea..216a0b275efe 100644 --- a/ui/tests/integration/components/pki/pki-role-generate-test.js +++ b/ui/tests/integration/components/pki/pki-role-generate-test.js @@ -1,6 +1,6 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { render, fillIn, click } from '@ember/test-helpers'; +import { render, fillIn } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; import { setupMirage } from 'ember-cli-mirage/test-support'; import { setupEngine } from 'ember-engines/test-support'; @@ -18,7 +18,7 @@ module('Integration | Component | pki-role-generate', function (hooks) { this.store = this.owner.lookup('service:store'); this.secretMountPath = this.owner.lookup('service:secret-mount-path'); this.secretMountPath.currentPath = 'pki-test'; - this.modelGenerate = this.store.createRecord('pki/certificate/generate', { + this.model = this.store.createRecord('pki/certificate/generate', { role: 'my-role', }); this.onSuccess = Sinon.spy(); @@ -30,7 +30,7 @@ module('Integration | Component | pki-role-generate', function (hooks) { hbs`
@@ -41,7 +41,7 @@ module('Integration | Component | pki-role-generate', function (hooks) { assert.dom(SELECTORS.commonNameField).exists('shows the common name field'); assert.dom(SELECTORS.optionsToggle).exists('toggle exists'); await fillIn(SELECTORS.commonNameField, 'example.com'); - assert.strictEqual(this.modelGenerate.commonName, 'example.com', 'Filling in the form updates the model'); + assert.strictEqual(this.model.commonName, 'example.com', 'Filling in the form updates the model'); }); test('it should render the component displaying the cert', async function (assert) { @@ -69,52 +69,4 @@ module('Integration | Component | pki-role-generate', function (hooks) { assert.dom(SELECTORS.certificate).exists({ count: 1 }, 'shows certificate info row'); assert.dom(SELECTORS.serialNumber).hasText('abcd-efgh-ijkl', 'shows serial number info row'); }); - - test('it should display validation errors for cert generation', async function (assert) { - assert.expect(3); - - await render( - hbs` -
- -
- `, - { owner: this.engine } - ); - await click(SELECTORS.generateButton); - - assert - .dom(SELECTORS.commonNameInlineError) - .hasText('Common name is required.', 'Common name validation error renders'); - assert.dom(SELECTORS.inlineAlert).hasText('There is an error with this form.', 'Alert renders'); - assert.dom(SELECTORS.commonNameErrorBorder).hasClass('has-error-border'); - }); - - test('it should display validation errors for cert signing', async function (assert) { - assert.expect(3); - this.modelSign = this.store.createRecord('pki/certificate/sign', { - role: 'my-role', - }); - await render( - hbs` -
- -
- `, - { owner: this.engine } - ); - await click(SELECTORS.generateButton); - - assert - .dom(SELECTORS.commonNameInlineError) - .hasText('Common name is required.', 'Common name validation error renders'); - assert.dom(SELECTORS.inlineAlert).hasText('There is an error with this form.', 'Alert renders'); - assert.dom(SELECTORS.commonNameErrorBorder).hasClass('has-error-border'); - }); }); From c3b4dc7dac0ac4f667a4d57a83522e25df2a3eac Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Tue, 21 Feb 2023 18:51:28 -0800 Subject: [PATCH 6/7] keep invalid form alert --- ui/lib/pki/addon/components/pki-role-generate.hbs | 5 +++++ ui/lib/pki/addon/components/pki-role-generate.ts | 2 ++ 2 files changed, 7 insertions(+) diff --git a/ui/lib/pki/addon/components/pki-role-generate.hbs b/ui/lib/pki/addon/components/pki-role-generate.hbs index 87f3e7d17a4e..37e4f249ee59 100644 --- a/ui/lib/pki/addon/components/pki-role-generate.hbs +++ b/ui/lib/pki/addon/components/pki-role-generate.hbs @@ -40,6 +40,11 @@ Cancel + {{#if this.invalidFormAlert}} +
+ +
+ {{/if}} {{/if}} \ No newline at end of file diff --git a/ui/lib/pki/addon/components/pki-role-generate.ts b/ui/lib/pki/addon/components/pki-role-generate.ts index 2f33710426ed..99cc3e1649f3 100644 --- a/ui/lib/pki/addon/components/pki-role-generate.ts +++ b/ui/lib/pki/addon/components/pki-role-generate.ts @@ -24,6 +24,7 @@ export default class PkiRoleGenerate extends Component { @service declare readonly download: DownloadService; @tracked errorBanner = ''; + @tracked invalidFormAlert = ''; get verb() { return this.args.type === 'sign' ? 'sign' : 'generate'; @@ -39,6 +40,7 @@ export default class PkiRoleGenerate extends Component { onSuccess(); } catch (err) { this.errorBanner = errorMessage(err, `Could not ${this.verb} certificate. See Vault logs for details.`); + this.invalidFormAlert = 'There was an error submitting this form.'; } } From 44fca725d8e478349598a5d0d37db03107ffb8ab Mon Sep 17 00:00:00 2001 From: "clairebontempo@gmail.com" Date: Tue, 21 Feb 2023 18:58:56 -0800 Subject: [PATCH 7/7] remove validate from declaration --- ui/types/vault/models/pki/certificate/base.d.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/types/vault/models/pki/certificate/base.d.ts b/ui/types/vault/models/pki/certificate/base.d.ts index 6e891dc016f2..f57e00d062b9 100644 --- a/ui/types/vault/models/pki/certificate/base.d.ts +++ b/ui/types/vault/models/pki/certificate/base.d.ts @@ -19,5 +19,4 @@ export default class PkiCertificateBaseModel extends Model { importedKeys: string[]; revokePath: string; get canRevoke(): boolean; - validate(): ModelValidations; }