From 66e78db4259500810c551cb09db3d1aa252f8cb0 Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Tue, 18 Jun 2024 14:20:22 -0600 Subject: [PATCH] Mask obfuscated Secret sync create/edit fields (#27348) * wip not working on edit view * changelog * vercel and fix tests * need conditional to not break all the things: * create test coverage and add for other obfustcaed fonts, still missing one. * Update 27348.txt * remove meep * comment * test coverage --- changelog/27348.txt | 3 ++ 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 | 2 +- ui/app/models/sync/destinations/gh.js | 2 + .../sync/destinations/vercel-project.js | 2 + ui/lib/core/addon/components/form-field.hbs | 2 +- ui/lib/core/addon/components/masked-input.hbs | 2 +- .../sync/secrets/destination-test.js | 2 +- ui/tests/helpers/general-selectors.ts | 1 + ui/tests/helpers/sync/sync-selectors.js | 5 +++ .../page/destinations/create-and-edit-test.js | 42 ++++++++++++++++++- 12 files changed, 64 insertions(+), 5 deletions(-) create mode 100644 changelog/27348.txt diff --git a/changelog/27348.txt b/changelog/27348.txt new file mode 100644 index 0000000000..ec7ece0b85 --- /dev/null +++ b/changelog/27348.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: Mask obfuscated fields when creating/editing a Secrets sync destination. +``` diff --git a/ui/app/models/sync/destinations/aws-sm.js b/ui/app/models/sync/destinations/aws-sm.js index 4b38875a26..5574d19a2e 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 42de9a1730..fa363d6959 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 03f70ac622..6de8200eeb 100644 --- a/ui/app/models/sync/destinations/gcp-sm.js +++ b/ui/app/models/sync/destinations/gcp-sm.js @@ -37,7 +37,7 @@ export default class SyncDestinationsGoogleCloudSecretManagerModel extends SyncD editType: 'file', docLink: '/vault/docs/secrets/gcp#authentication', }) - credentials; // obfuscated, never returned by API + credentials; // obfuscated, never returned by API. Masking handled by EnableInput component @attr('object', { subText: diff --git a/ui/app/models/sync/destinations/gh.js b/ui/app/models/sync/destinations/gh.js index a8db62a9a8..8cd6e7bd14 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/app/models/sync/destinations/vercel-project.js b/ui/app/models/sync/destinations/vercel-project.js index aaa88f671c..b788a780c4 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/form-field.hbs b/ui/lib/core/addon/components/form-field.hbs index 2b9b3103ea..84991d19e7 100644 --- a/ui/lib/core/addon/components/form-field.hbs +++ b/ui/lib/core/addon/components/form-field.hbs @@ -247,7 +247,7 @@ diff --git a/ui/lib/core/addon/components/masked-input.hbs b/ui/lib/core/addon/components/masked-input.hbs index eebc6d46f8..833b940a25 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={{or @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 66404f0d1d..5101ab1836 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 (singular)', 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 674a5bb51a..9a04214927 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}"]`, }; diff --git a/ui/tests/helpers/sync/sync-selectors.js b/ui/tests/helpers/sync/sync-selectors.js index 842ca92e7b..db3658f9a3 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 c5d271163f..46c86a0398 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'; @@ -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,6 +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 = ['accessToken', 'clientSecret', 'secretAccessKey', 'accessKeyId']; module(`create destination: ${type}`, function (hooks) { hooks.beforeEach(function () { @@ -209,6 +232,23 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE } }); + test('it masks obfuscated fields', 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';