From a4fc962853474ee9082da39681f0461639ad7dab Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Tue, 4 Jun 2024 11:27:24 -0600 Subject: [PATCH 1/9] wip not working on edit view --- ui/app/models/sync/destinations/gh.js | 2 ++ ui/lib/core/addon/components/form-field.hbs | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/app/models/sync/destinations/gh.js b/ui/app/models/sync/destinations/gh.js index a8db62a9a804..8cd6e7bd147f 100644 --- a/ui/app/models/sync/destinations/gh.js +++ b/ui/app/models/sync/destinations/gh.js @@ -27,6 +27,8 @@ export default class SyncDestinationsGithubModel extends SyncDestinationModel { @attr('string', { subText: 'Personal access token to authenticate to the GitHub repository. If empty, Vault will use the GITHUB_ACCESS_TOKEN environment variable if configured.', + sensitive: true, + noCopy: true, }) accessToken; // obfuscated, never returned by API diff --git a/ui/lib/core/addon/components/form-field.hbs b/ui/lib/core/addon/components/form-field.hbs index 2b9b3103ea91..daab1680a36c 100644 --- a/ui/lib/core/addon/components/form-field.hbs +++ b/ui/lib/core/addon/components/form-field.hbs @@ -247,7 +247,8 @@ From 87b9b4b1dac0e5e392c0ff4bfa454c54a5b25cdb Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Mon, 10 Jun 2024 10:48:16 -0600 Subject: [PATCH 2/9] changelog --- changelog/27348.txt | 3 +++ ui/lib/core/addon/components/form-field.hbs | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 changelog/27348.txt diff --git a/changelog/27348.txt b/changelog/27348.txt new file mode 100644 index 000000000000..7ec32b29afa6 --- /dev/null +++ b/changelog/27348.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: Mask accessToken field when creating a Secrets sync Github destination. +``` diff --git a/ui/lib/core/addon/components/form-field.hbs b/ui/lib/core/addon/components/form-field.hbs index daab1680a36c..84991d19e757 100644 --- a/ui/lib/core/addon/components/form-field.hbs +++ b/ui/lib/core/addon/components/form-field.hbs @@ -247,7 +247,6 @@ Date: Mon, 10 Jun 2024 12:02:08 -0600 Subject: [PATCH 3/9] vercel and fix tests --- ui/app/models/sync/destinations/vercel-project.js | 2 ++ ui/lib/core/addon/components/masked-input.hbs | 2 +- ui/tests/acceptance/sync/secrets/destination-test.js | 2 +- ui/tests/helpers/general-selectors.ts | 1 + 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ui/app/models/sync/destinations/vercel-project.js b/ui/app/models/sync/destinations/vercel-project.js index aaa88f671c63..b788a780c41f 100644 --- a/ui/app/models/sync/destinations/vercel-project.js +++ b/ui/app/models/sync/destinations/vercel-project.js @@ -42,6 +42,8 @@ const formFieldGroups = [ export default class SyncDestinationsVercelProjectModel extends SyncDestinationModel { @attr('string', { subText: 'Vercel API access token with the permissions to manage environment variables.', + sensitive: true, + noCopy: true, }) accessToken; // obfuscated, never returned by API diff --git a/ui/lib/core/addon/components/masked-input.hbs b/ui/lib/core/addon/components/masked-input.hbs index eebc6d46f8c8..a2509a648271 100644 --- a/ui/lib/core/addon/components/masked-input.hbs +++ b/ui/lib/core/addon/components/masked-input.hbs @@ -32,7 +32,7 @@ aria-label={{or @name "masked input"}} {{on "change" this.onChange}} {{on "keyup" (fn this.handleKeyUp @name)}} - data-test-textarea + data-test-textarea={{@name}} /> {{/if}} {{#if @allowCopy}} diff --git a/ui/tests/acceptance/sync/secrets/destination-test.js b/ui/tests/acceptance/sync/secrets/destination-test.js index 452e3b457891..99f04dc9d69c 100644 --- a/ui/tests/acceptance/sync/secrets/destination-test.js +++ b/ui/tests/acceptance/sync/secrets/destination-test.js @@ -86,7 +86,7 @@ module('Acceptance | sync | destination', function (hooks) { await visit('vault/sync/secrets/destinations/vercel-project/destination-vercel/edit'); await click(ts.enableField('accessToken')); - await fillIn(ts.inputByAttr('accessToken'), 'foobar'); + await fillIn(ts.maskedInput('accessToken'), 'foobar'); await click(ts.saveButton); await click(ts.toolbar('Edit destination')); await click(ts.saveButton); diff --git a/ui/tests/helpers/general-selectors.ts b/ui/tests/helpers/general-selectors.ts index 674a5bb51a6d..9a0421492722 100644 --- a/ui/tests/helpers/general-selectors.ts +++ b/ui/tests/helpers/general-selectors.ts @@ -79,4 +79,5 @@ export const GENERAL = { navLink: (label: string) => `[data-test-sidebar-nav-link="${label}"]`, cancelButton: '[data-test-cancel]', saveButton: '[data-test-save]', + maskedInput: (name: string) => `[data-test-textarea="${name}"]`, }; From 9a1b54591b7b1bc197ff275ee499301b74722522 Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Mon, 10 Jun 2024 14:16:16 -0600 Subject: [PATCH 4/9] need conditional to not break all the things: --- ui/lib/core/addon/components/masked-input.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/lib/core/addon/components/masked-input.hbs b/ui/lib/core/addon/components/masked-input.hbs index a2509a648271..833b940a259d 100644 --- a/ui/lib/core/addon/components/masked-input.hbs +++ b/ui/lib/core/addon/components/masked-input.hbs @@ -32,7 +32,7 @@ aria-label={{or @name "masked input"}} {{on "change" this.onChange}} {{on "keyup" (fn this.handleKeyUp @name)}} - data-test-textarea={{@name}} + data-test-textarea={{or @name ""}} /> {{/if}} {{#if @allowCopy}} From 98b04db6fcab392eb2b13e81a041e8156a23e9b8 Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Wed, 12 Jun 2024 10:58:51 -0600 Subject: [PATCH 5/9] create test coverage and add for other obfustcaed fonts, still missing one. --- ui/app/models/sync/destinations/aws-sm.js | 4 ++++ ui/app/models/sync/destinations/azure-kv.js | 2 ++ ui/app/models/sync/destinations/gcp-sm.js | 1 + .../page/destinations/create-and-edit-test.js | 20 ++++++++++++++++++- 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/ui/app/models/sync/destinations/aws-sm.js b/ui/app/models/sync/destinations/aws-sm.js index 4b38875a2616..5574d19a2e53 100644 --- a/ui/app/models/sync/destinations/aws-sm.js +++ b/ui/app/models/sync/destinations/aws-sm.js @@ -32,6 +32,8 @@ export default class SyncDestinationsAwsSecretsManagerModel extends SyncDestinat label: 'Access key ID', subText: 'Access key ID to authenticate against the secrets manager. If empty, Vault will use the AWS_ACCESS_KEY_ID environment variable if configured.', + sensitive: true, + noCopy: true, }) accessKeyId; // obfuscated, never returned by API @@ -39,6 +41,8 @@ export default class SyncDestinationsAwsSecretsManagerModel extends SyncDestinat label: 'Secret access key', subText: 'Secret access key to authenticate against the secrets manager. If empty, Vault will use the AWS_SECRET_ACCESS_KEY environment variable if configured.', + sensitive: true, + noCopy: true, }) secretAccessKey; // obfuscated, never returned by API diff --git a/ui/app/models/sync/destinations/azure-kv.js b/ui/app/models/sync/destinations/azure-kv.js index 42de9a173006..fa363d695909 100644 --- a/ui/app/models/sync/destinations/azure-kv.js +++ b/ui/app/models/sync/destinations/azure-kv.js @@ -55,6 +55,8 @@ export default class SyncDestinationsAzureKeyVaultModel extends SyncDestinationM @attr('string', { subText: 'Client secret of an Azure app registration. If empty, Vault will use the AZURE_CLIENT_SECRET environment variable if configured.', + sensitive: true, + noCopy: true, }) clientSecret; // obfuscated, never returned by API diff --git a/ui/app/models/sync/destinations/gcp-sm.js b/ui/app/models/sync/destinations/gcp-sm.js index 03f70ac62254..6578ea26db1a 100644 --- a/ui/app/models/sync/destinations/gcp-sm.js +++ b/ui/app/models/sync/destinations/gcp-sm.js @@ -38,6 +38,7 @@ export default class SyncDestinationsGoogleCloudSecretManagerModel extends SyncD docLink: '/vault/docs/secrets/gcp#authentication', }) credentials; // obfuscated, never returned by API + // ARG TODO DOUBLE CHECK @attr('object', { subText: diff --git a/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js b/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js index c5d271163fb0..dd39285ddbec 100644 --- a/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js +++ b/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js @@ -10,7 +10,7 @@ import { setupMirage } from 'ember-cli-mirage/test-support'; import hbs from 'htmlbars-inline-precompile'; import sinon from 'sinon'; import { Response } from 'miragejs'; -import { click, render, typeIn } from '@ember/test-helpers'; +import { click, fillIn, render, typeIn } from '@ember/test-helpers'; import { PAGE } from 'vault/tests/helpers/sync/sync-selectors'; import { syncDestinations } from 'vault/helpers/sync-destinations'; import { decamelize, underscore } from '@ember/string'; @@ -192,6 +192,7 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE // CREATE FORM ASSERTIONS FOR EACH DESTINATION TYPE for (const destination of SYNC_DESTINATIONS) { const { name, type } = destination; + const obfuscatedFields = ['clientSecret', 'secretAccessKey', 'accessKeyId']; module(`create destination: ${type}`, function (hooks) { hooks.beforeEach(function () { @@ -209,6 +210,23 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE } }); + test('it masks obfuscated fields meep', async function (assert) { + const filteredObfuscatedFields = this.model.formFields.filter((field) => + obfuscatedFields.includes(field.name) + ); + assert.expect(filteredObfuscatedFields.length); + await this.renderFormComponent(); + // iterate over the form fields and filter for those that are obfuscated + // fill those in and assert that they are masked + filteredObfuscatedFields.forEach(async (field) => { + await fillIn(PAGE.maskedInput(field.name), 'blah'); + + assert + .dom(PAGE.maskedInput(field.name)) + .hasClass('masked-font', `it renders ${field.name} for ${destination} with masked font`); + }); + }); + test('it saves destination and transitions to details', async function (assert) { assert.expect(5); const name = 'my-name'; From d0912a69d7b226dc1b7b1bd4b824d799b7ff7e5c Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Wed, 12 Jun 2024 11:00:27 -0600 Subject: [PATCH 6/9] Update 27348.txt --- changelog/27348.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/27348.txt b/changelog/27348.txt index 7ec32b29afa6..ec7ece0b851a 100644 --- a/changelog/27348.txt +++ b/changelog/27348.txt @@ -1,3 +1,3 @@ ```release-note:improvement -ui: Mask accessToken field when creating a Secrets sync Github destination. +ui: Mask obfuscated fields when creating/editing a Secrets sync destination. ``` From 7de21b29c6dcfbcf03c1c0c9af4938d976a6e46c Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Wed, 12 Jun 2024 13:17:25 -0600 Subject: [PATCH 7/9] remove meep --- .../sync/secrets/page/destinations/create-and-edit-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js b/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js index dd39285ddbec..4f54f813ee8e 100644 --- a/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js +++ b/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js @@ -210,7 +210,7 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE } }); - test('it masks obfuscated fields meep', async function (assert) { + test('it masks obfuscated fields', async function (assert) { const filteredObfuscatedFields = this.model.formFields.filter((field) => obfuscatedFields.includes(field.name) ); From 455098fd12908344a6b5b5595163bff52e467106 Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Tue, 18 Jun 2024 09:22:48 -0600 Subject: [PATCH 8/9] comment --- ui/app/models/sync/destinations/gcp-sm.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/app/models/sync/destinations/gcp-sm.js b/ui/app/models/sync/destinations/gcp-sm.js index 6578ea26db1a..6de8200eeb5d 100644 --- a/ui/app/models/sync/destinations/gcp-sm.js +++ b/ui/app/models/sync/destinations/gcp-sm.js @@ -37,8 +37,7 @@ export default class SyncDestinationsGoogleCloudSecretManagerModel extends SyncD editType: 'file', docLink: '/vault/docs/secrets/gcp#authentication', }) - credentials; // obfuscated, never returned by API - // ARG TODO DOUBLE CHECK + credentials; // obfuscated, never returned by API. Masking handled by EnableInput component @attr('object', { subText: From 00944a979978f7c326372e4637e33eca7c177d30 Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Tue, 18 Jun 2024 10:19:58 -0600 Subject: [PATCH 9/9] test coverage --- ui/tests/helpers/sync/sync-selectors.js | 5 ++++ .../page/destinations/create-and-edit-test.js | 24 ++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/ui/tests/helpers/sync/sync-selectors.js b/ui/tests/helpers/sync/sync-selectors.js index 842ca92e7bdc..db3658f9a3ee 100644 --- a/ui/tests/helpers/sync/sync-selectors.js +++ b/ui/tests/helpers/sync/sync-selectors.js @@ -97,6 +97,11 @@ export const PAGE = { case 'customTags': await fillIn('[data-test-kv-key="0"]', 'foo'); return fillIn('[data-test-kv-value="0"]', value); + case 'accessKeyId': + case 'secretAccessKey': + case 'clientSecret': + case 'accessToken': + return fillIn(GENERAL.maskedInput(attr), value); case 'deploymentEnvironments': await click('[data-test-input="deploymentEnvironments"] input#development'); await click('[data-test-input="deploymentEnvironments"] input#preview'); diff --git a/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js b/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js index 4f54f813ee8e..46c86a0398cc 100644 --- a/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js +++ b/ui/tests/integration/components/sync/secrets/page/destinations/create-and-edit-test.js @@ -131,6 +131,28 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE await click(PAGE.saveButton); }); + test('edit: payload only contains masked inputs when they have changed', async function (assert) { + assert.expect(1); + this.model = this.generateModel(); + + this.server.patch(`sys/sync/destinations/${this.model.type}/${this.model.name}`, (schema, req) => { + const payload = JSON.parse(req.requestBody); + assert.propEqual( + payload, + { secret_access_key: 'new-secret' }, + 'payload contains the changed obfuscated field' + ); + return { payload }; + }); + + await this.renderFormComponent(); + await click(PAGE.enableField('accessKeyId')); + await click(PAGE.maskedInput('accessKeyId')); // click on input but do not change value + await click(PAGE.enableField('secretAccessKey')); + await fillIn(PAGE.maskedInput('secretAccessKey'), 'new-secret'); + await click(PAGE.saveButton); + }); + test('it renders API errors', async function (assert) { assert.expect(1); const name = 'my-failed-dest'; @@ -192,7 +214,7 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE // CREATE FORM ASSERTIONS FOR EACH DESTINATION TYPE for (const destination of SYNC_DESTINATIONS) { const { name, type } = destination; - const obfuscatedFields = ['clientSecret', 'secretAccessKey', 'accessKeyId']; + const obfuscatedFields = ['accessToken', 'clientSecret', 'secretAccessKey', 'accessKeyId']; module(`create destination: ${type}`, function (hooks) { hooks.beforeEach(function () {