From 61c80518de89e8ebb37b15a91dd1606bedf38f00 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Tue, 27 Jul 2021 14:08:16 -0400 Subject: [PATCH 1/6] feat: Add credential store attributes to it's form --- addons/core/translations/resources/en-us.yaml | 25 +++ .../form/credential-store/index.hbs | 152 ++++++++++++++++++ 2 files changed, 177 insertions(+) diff --git a/addons/core/translations/resources/en-us.yaml b/addons/core/translations/resources/en-us.yaml index a448c0011..b9eb433c3 100644 --- a/addons/core/translations/resources/en-us.yaml +++ b/addons/core/translations/resources/en-us.yaml @@ -290,6 +290,31 @@ credential-store: delete: Delete Credential Store types: vault: Vault + form: + address: + label: Address + help: The address of the Vault server. This should be a complete URL such as https://127.0.0.1:8200. + namespace: + label: Namespace + help: A Vault namespace. Requires Vault Enterprise. + ca_cert: + label: CA Certificate + help: A PEM-encoded CA certificate to verify the Vault server's TLS certificate. + tls_server_name: + label: TLS Server Name + help: Name to use as the SNI host when connecting to Vault via TLS. + tls_skip_verify: + label: TLS Skip Verify + help: Disable verification of TLS certificates. Using this option is highly discouraged as it decreases the security of data transmissions to and from the Vault server. + token: + label: Token + help: A token used for accessing Vault. + client_certificate: + label: Client Certificate + help: A PEM-encoded client certificate to use for TLS authentication to the Vault server. + client_certificate_key: + label: Client Certificate Key + help: A PEM-encoded private key matching the client certificate from Client Certificate. credential-library: title: Credential Library title_plural: Credential Libraries diff --git a/ui/admin/app/components/form/credential-store/index.hbs b/ui/admin/app/components/form/credential-store/index.hbs index b70183ed3..cd50e3aae 100644 --- a/ui/admin/app/components/form/credential-store/index.hbs +++ b/ui/admin/app/components/form/credential-store/index.hbs @@ -52,6 +52,158 @@ {{/if}} + + {{#if @model.errors.attributes_address}} + + {{#each @model.errors.attributes_address as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + + + + {{#if @model.errors.attributes_token}} + + {{#each @model.errors.attributes_token as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + + + + {{#if @model.errors.attributes_namespace}} + + {{#each @model.errors.attributes_namespace as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + + + + {{#if @model.errors.attributes_tls_server_name}} + + {{#each @model.errors.attributes_tls_server_name as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + + + + + + {{#if @model.errors.attributes_client_certificate}} + + {{#each @model.errors.attributes_client_certificate as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + + + + {{#if @model.errors.attributes_client_certificate_key}} + + {{#each @model.errors.attributes_client_certificate_key as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + + + + {{#if @model.errors.attributes_ca_cert}} + + {{#each @model.errors.attributes_ca_cert as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + + Date: Tue, 27 Jul 2021 14:15:59 -0700 Subject: [PATCH 2/6] =?UTF-8?q?test:=20=F0=9F=92=8D=20Add=20creation=20err?= =?UTF-8?q?or=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add credential library creation error tests --- .../credential-library-test.js | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/ui/admin/tests/acceptance/credential-library/credential-library-test.js b/ui/admin/tests/acceptance/credential-library/credential-library-test.js index a23fac3f2..77dbc0912 100644 --- a/ui/admin/tests/acceptance/credential-library/credential-library-test.js +++ b/ui/admin/tests/acceptance/credential-library/credential-library-test.js @@ -137,6 +137,112 @@ module('Acceptance | credential-library', function (hooks) { ); }); + test.skip('can save changes to existing credential library', async function (assert) { + assert.expect(3); + assert.notEqual(instances.credentialLibrary.name, 'random string'); + await visit(urls.credentialLibrary); + await click('form [type="button"]', 'Activate edit mode'); + await fillIn('[name="name"]', 'random string'); + await click('.rose-form-actions [type="submit"]'); + assert.equal(currentURL(), urls.credentialLibrary); + assert.equal( + this.server.schema.credentialLibraries.all().models[0].name, + 'random string' + ); + }); + + test('can cancel changes to existing credential library', async function (assert) { + assert.expect(2); + await visit(urls.credentialLibrary); + await click('form [type="button"]', 'Activate edit mode'); + await fillIn('[name="name"]', 'random string'); + await click('.rose-form-actions [type="button"]'); + assert.notEqual(instances.credentialStore.name, 'random string'); + assert.equal(find('[name="name"]').value, instances.credentialLibrary.name); + }); + + test('saving an existing credential library with invalid fields displays error messages', async function (assert) { + assert.expect(2); + this.server.patch('/credential-libraries/:id', () => { + return new Response( + 400, + {}, + { + status: 400, + code: 'invalid_argument', + message: 'The request was invalid.', + details: { + request_fields: [ + { + name: 'name', + description: 'Name is required', + }, + ], + }, + } + ); + }); + await visit(urls.credentialLibrary); + await click('form [type="button"]', 'Activate edit mode'); + await fillIn('[name="name"]', 'random string'); + await click('[type="submit"]'); + assert.ok( + find('[role="alert"]').textContent.trim(), + 'The request was invalid.' + ); + assert.ok( + find('.rose-form-error-message').textContent.trim(), + 'Name is required.' + ); + }); + + test('can discard unsaved credential library changes via dialog', async function (assert) { + assert.expect(5); + const confirmService = this.owner.lookup('service:confirm'); + confirmService.enabled = true; + assert.notEqual(instances.credentialLibrary.name, 'random string'); + + await visit(urls.credentialLibrary); + await click('form [type="button"]', 'Activate edit mode'); + await fillIn('[name="name"]', 'random string'); + assert.equal(currentURL(), urls.credentialLibrary); + + try { + await visit(urls.credentialLibraries); + } catch (e) { + assert.ok(find('.rose-dialog')); + await click('.rose-dialog-footer button:first-child', 'Click Discard'); + assert.equal(currentURL(), urls.credentialLibraries); + assert.notEqual( + this.server.schema.credentialLibraries.all().models[0].name, + 'random string' + ); + } + }); + + test('can cancel discard unsaved credential library via dialog', async function (assert) { + assert.expect(5); + const confirmService = this.owner.lookup('service:confirm'); + confirmService.enabled = true; + assert.notEqual(instances.credentialLibrary.name, 'random string'); + await visit(urls.credentialLibrary); + await click('form [type="button"]', 'Activate edit mode'); + await fillIn('[name="name"]', 'random string'); + assert.equal(currentURL(), urls.credentialLibrary); + + try { + await visit(urls.credentialLibraries); + } catch (e) { + assert.ok(find('.rose-dialog')); + await click('.rose-dialog-footer button:last-child'); + assert.equal(currentURL(), urls.credentialLibrary); + assert.notEqual( + this.server.schema.credentialLibraries.all().models[0].name, + 'random string' + ); + } + }); + test('can delete credential library', async function (assert) { assert.expect(1); const count = getCredentialLibraryCount(); From 8cceb4090fcc6ba59da7021fe146aeb68a80c818 Mon Sep 17 00:00:00 2001 From: Carlos Alcaide Date: Tue, 27 Jul 2021 14:27:13 -0700 Subject: [PATCH 3/6] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20Change=20save=20?= =?UTF-8?q?crdential=20library=20behaviour?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change save credential library behaviour to not redirect after saving a change --- .../credential-stores/credential-store/credential-libraries.js | 3 ++- .../acceptance/credential-library/credential-library-test.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ui/admin/app/routes/scopes/scope/credential-stores/credential-store/credential-libraries.js b/ui/admin/app/routes/scopes/scope/credential-stores/credential-store/credential-libraries.js index d9602dba4..84684ecfd 100644 --- a/ui/admin/app/routes/scopes/scope/credential-stores/credential-store/credential-libraries.js +++ b/ui/admin/app/routes/scopes/scope/credential-stores/credential-store/credential-libraries.js @@ -47,7 +47,8 @@ export default class ScopesScopeCredentialStoresCredentialStoreCredentialLibrari async save(credentialLibrary) { await credentialLibrary.save(); await this.transitionTo( - 'scopes.scope.credential-stores.credential-store.credential-libraries' + 'scopes.scope.credential-stores.credential-store.credential-libraries.credential-library', + credentialLibrary ); this.refresh(); } diff --git a/ui/admin/tests/acceptance/credential-library/credential-library-test.js b/ui/admin/tests/acceptance/credential-library/credential-library-test.js index 77dbc0912..245fc7157 100644 --- a/ui/admin/tests/acceptance/credential-library/credential-library-test.js +++ b/ui/admin/tests/acceptance/credential-library/credential-library-test.js @@ -137,7 +137,7 @@ module('Acceptance | credential-library', function (hooks) { ); }); - test.skip('can save changes to existing credential library', async function (assert) { + test('can save changes to existing credential library', async function (assert) { assert.expect(3); assert.notEqual(instances.credentialLibrary.name, 'random string'); await visit(urls.credentialLibrary); From e40ff1f63ce02846c75678d8c09da8e09ff15113 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 28 Jul 2021 13:22:47 -0400 Subject: [PATCH 4/6] fix: Protect credentialstore attrs hmac values in serializati --- .../fragment-credential-store-attributes.js | 10 ++ ...agment-credential-store-attributes-test.js | 114 +++++++++++++++--- addons/core/translations/resources/en-us.yaml | 6 +- .../form/credential-store/index.hbs | 32 ++++- 4 files changed, 146 insertions(+), 16 deletions(-) diff --git a/addons/api/addon/serializers/fragment-credential-store-attributes.js b/addons/api/addon/serializers/fragment-credential-store-attributes.js index 547c6a4f7..883a823aa 100644 --- a/addons/api/addon/serializers/fragment-credential-store-attributes.js +++ b/addons/api/addon/serializers/fragment-credential-store-attributes.js @@ -19,6 +19,16 @@ export default class FragmentCredentialStoreAttributesSerializer extends JSONSer if (type === 'string' && json[key] === '') json[key] = null; // Do not serialize read-only attributes. if (options.readOnly) delete json[key]; + // Token cannot be unset. If it's falsy, it must be removed. + if (key === 'token' && !json[key]) delete json[key]; + // Client certificate key cannot be unset when certificate is set. + // If it's falsy, it must be removed. + if ( + key === 'client_certificate_key' && + !json[key] && + json['client_certificate'] + ) + delete json[key]; return value; } } diff --git a/addons/api/tests/unit/serializers/fragment-credential-store-attributes-test.js b/addons/api/tests/unit/serializers/fragment-credential-store-attributes-test.js index 7fbbd0744..cfa00734b 100644 --- a/addons/api/tests/unit/serializers/fragment-credential-store-attributes-test.js +++ b/addons/api/tests/unit/serializers/fragment-credential-store-attributes-test.js @@ -6,26 +6,112 @@ module( function (hooks) { setupTest(hooks); - // Replace this with your real tests. - test('it exists', function (assert) { - let store = this.owner.lookup('service:store'); - let serializer = store.serializerFor( - 'fragment-credential-store-attributes' + test('it serializes empty strings to null', function (assert) { + assert.expect(1); + const store = this.owner.lookup('service:store'); + const record = store.createRecord( + 'fragment-credential-store-attributes', + { + address: '', + ca_cert: '', + client_certificate: '', + client_certificate_key: '', + namespace: '', + tls_server_name: '', + tls_skip_verify: false, + } ); - - assert.ok(serializer); + let serializedRecord = record.serialize(); + assert.deepEqual(serializedRecord, { + address: null, + ca_cert: null, + client_certificate: null, + client_certificate_key: null, + namespace: null, + tls_server_name: null, + tls_skip_verify: false, + }); }); - test('it serializes records', function (assert) { - let store = this.owner.lookup('service:store'); - let record = store.createRecord( + test('it does not serialize token if present but falsy', function (assert) { + assert.expect(1); + const store = this.owner.lookup('service:store'); + const record = store.createRecord( 'fragment-credential-store-attributes', - {} + { + token: null, + address: 'http://localhost:5000', + ca_cert: 'ca-cert-123', + client_certificate: 'client-cert-123', + client_certificate_key: 'client-cert-key-456', + namespace: 'foobar', + tls_server_name: 'yes', + tls_skip_verify: false, + } + ); + let serializedRecord = record.serialize(); + assert.deepEqual( + serializedRecord, + { + address: 'http://localhost:5000', + ca_cert: 'ca-cert-123', + client_certificate: 'client-cert-123', + client_certificate_key: 'client-cert-key-456', + namespace: 'foobar', + tls_server_name: 'yes', + tls_skip_verify: false, + }, + 'token attribute is not expected' + ); + }); + + test('it does not serialize client certificate key if present but falsy when client certificate is present', function (assert) { + assert.expect(2); + const store = this.owner.lookup('service:store'); + const record = store.createRecord( + 'fragment-credential-store-attributes', + { + client_certificate: 'client-cert-123', + client_certificate_key: '', + address: 'http://localhost:5000', + token: 'foo', + ca_cert: 'ca-cert-123', + namespace: 'foobar', + tls_server_name: 'yes', + tls_skip_verify: false, + } + ); + let serializedRecord = record.serialize(); + assert.deepEqual( + serializedRecord, + { + client_certificate: 'client-cert-123', + address: 'http://localhost:5000', + token: 'foo', + ca_cert: 'ca-cert-123', + namespace: 'foobar', + tls_server_name: 'yes', + tls_skip_verify: false, + }, + 'client certificate key attribute is not expected' ); - let serializedRecord = record.serialize(); - - assert.ok(serializedRecord); + record.client_certificate_key = 'client-cert-key-456'; + serializedRecord = record.serialize(); + assert.deepEqual( + serializedRecord, + { + client_certificate: 'client-cert-123', + client_certificate_key: 'client-cert-key-456', + address: 'http://localhost:5000', + token: 'foo', + ca_cert: 'ca-cert-123', + namespace: 'foobar', + tls_server_name: 'yes', + tls_skip_verify: false, + }, + 'client certificate key attribute is expected' + ); }); } ); diff --git a/addons/core/translations/resources/en-us.yaml b/addons/core/translations/resources/en-us.yaml index b9eb433c3..9d001037f 100644 --- a/addons/core/translations/resources/en-us.yaml +++ b/addons/core/translations/resources/en-us.yaml @@ -308,13 +308,17 @@ credential-store: help: Disable verification of TLS certificates. Using this option is highly discouraged as it decreases the security of data transmissions to and from the Vault server. token: label: Token - help: A token used for accessing Vault. + help: A token used for accessing Vault. + token_hmac: + label: Token HMAC client_certificate: label: Client Certificate help: A PEM-encoded client certificate to use for TLS authentication to the Vault server. client_certificate_key: label: Client Certificate Key help: A PEM-encoded private key matching the client certificate from Client Certificate. + client_certificate_key_hmac: + label: Client Certificate Key HMAC credential-library: title: Credential Library title_plural: Credential Libraries diff --git a/ui/admin/app/components/form/credential-store/index.hbs b/ui/admin/app/components/form/credential-store/index.hbs index cd50e3aae..c77bfb410 100644 --- a/ui/admin/app/components/form/credential-store/index.hbs +++ b/ui/admin/app/components/form/credential-store/index.hbs @@ -92,6 +92,15 @@ {{/if}} + {{#unless @model.isNew}} + + {{/unless}} + + @error={{@model.errors.attributes_tls_skip_verify}} + as |field| + > + {{#if @model.errors.attributes_tls_skip_verify}} + + {{#each @model.errors.attributes_tls_skip_verify as |error|}} + {{error.message}} + {{/each}} + + {{/if}} + + {{#unless @model.isNew}} + + {{/unless}} + Date: Wed, 28 Jul 2021 10:51:57 -0700 Subject: [PATCH 5/6] =?UTF-8?q?test:=20=F0=9F=92=8D=20Change=20assertion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change assertion to avoid false positive --- .../acceptance/credential-library/credential-library-test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/admin/tests/acceptance/credential-library/credential-library-test.js b/ui/admin/tests/acceptance/credential-library/credential-library-test.js index ce2265856..c2b71f8f1 100644 --- a/ui/admin/tests/acceptance/credential-library/credential-library-test.js +++ b/ui/admin/tests/acceptance/credential-library/credential-library-test.js @@ -157,7 +157,10 @@ module('Acceptance | credential-library', function (hooks) { await click('form [type="button"]', 'Activate edit mode'); await fillIn('[name="name"]', 'random string'); await click('.rose-form-actions [type="button"]'); - assert.notEqual(instances.credentialStore.name, 'random string'); + assert.notEqual( + this.server.schema.credentialLibraries.all().models[0].name, + 'random string' + ); assert.equal(find('[name="name"]').value, instances.credentialLibrary.name); }); From 296b5fbecdce27e6cf33a588ae3447644d9aa001 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Wed, 28 Jul 2021 16:27:40 -0400 Subject: [PATCH 6/6] =?UTF-8?q?docs:=20=E2=9C=8F=EF=B8=8F=20add=20doc=20li?= =?UTF-8?q?nks=20for=20cred=20stores=20and=20sources?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../credential-libraries/credential-library/-header.hbs | 5 ++++- .../app/templates/scopes/scope/credential-stores/index.hbs | 2 +- ui/admin/config/environment.js | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ui/admin/app/templates/scopes/scope/credential-stores/credential-store/credential-libraries/credential-library/-header.hbs b/ui/admin/app/templates/scopes/scope/credential-stores/credential-store/credential-libraries/credential-library/-header.hbs index 3c2de9eef..ce2cbce51 100644 --- a/ui/admin/app/templates/scopes/scope/credential-stores/credential-store/credential-libraries/credential-library/-header.hbs +++ b/ui/admin/app/templates/scopes/scope/credential-stores/credential-store/credential-libraries/credential-library/-header.hbs @@ -1,4 +1,7 @@ -

{{t 'resources.credential-library.title'}}

+

+ {{t 'resources.credential-library.title'}} + +

{{t 'resources.credential-library.description'}}

{{t 'resources.credential-store.title_plural'}} - +

{{#if @model}}

{{t 'resources.credential-store.description'}}

diff --git a/ui/admin/config/environment.js b/ui/admin/config/environment.js index fca0dab92..9f13c5e50 100644 --- a/ui/admin/config/environment.js +++ b/ui/admin/config/environment.js @@ -55,7 +55,8 @@ module.exports = function (environment) { 'host-set': '/host-sets', 'host-set.new': '/host-sets/new', 'host-set.add-hosts': '/host-sets/add-hosts', - 'credential-stores': '/credential-stores', + 'credential-store': '/credential-stores', + 'credential-source': '/credential-sources', host: '/hosts', 'host.new': '/hosts/new', role: '/roles',