From 84aeec051312d8f0c87335b6235014b00aef8f68 Mon Sep 17 00:00:00 2001 From: Angel Garbarino Date: Thu, 27 Jun 2024 12:46:24 -0600 Subject: [PATCH] Create sections for Secrets sync destination fields for create/edit view (#27538) * initial shuffling of credentials and advanced configuration options * update all destination models * wip changelog * Update 27538.txt * remove custom_tags from gh * missed vercel and remove custom_tags from base * refactor conditional logic on templace * things * test coverage and dynamic subText * add assert to not see enableInput on create * clean up * remove extra parens * test clean up to clarify what the header subtext vs breadcrumb transition are testing --- changelog/27538.txt | 3 + ui/app/models/sync/destinations/aws-sm.js | 5 +- ui/app/models/sync/destinations/azure-kv.js | 15 ++--- ui/app/models/sync/destinations/gcp-sm.js | 6 +- ui/app/models/sync/destinations/gh.js | 7 ++- .../sync/destinations/vercel-project.js | 6 +- .../page/destinations/create-and-edit.hbs | 31 ++++++---- .../page/destinations/create-and-edit.ts | 14 +++++ ui/tests/helpers/sync/sync-selectors.js | 2 + .../page/destinations/create-and-edit-test.js | 57 ++++++++++++++++--- 10 files changed, 109 insertions(+), 37 deletions(-) create mode 100644 changelog/27538.txt diff --git a/changelog/27538.txt b/changelog/27538.txt new file mode 100644 index 0000000000..e1d1448150 --- /dev/null +++ b/changelog/27538.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: Creates separate section for updating sensitive creds for Secrets sync create/edit view. +``` diff --git a/ui/app/models/sync/destinations/aws-sm.js b/ui/app/models/sync/destinations/aws-sm.js index 5574d19a2e..2cb37a79a5 100644 --- a/ui/app/models/sync/destinations/aws-sm.js +++ b/ui/app/models/sync/destinations/aws-sm.js @@ -7,6 +7,7 @@ import SyncDestinationModel from '../destination'; import { attr } from '@ember-data/model'; import { withFormFields } from 'vault/decorators/model-form-fields'; +// displayFields are used on the destination details view const displayFields = [ // connection details 'name', @@ -20,11 +21,13 @@ const displayFields = [ 'secretNameTemplate', 'customTags', ]; +// formFieldGroups are used on the create-edit destination view const formFieldGroups = [ { - default: ['name', 'region', 'roleArn', 'externalId', 'granularity', 'secretNameTemplate', 'customTags'], + default: ['name', 'region', 'roleArn', 'externalId'], }, { Credentials: ['accessKeyId', 'secretAccessKey'] }, + { 'Advanced configuration': ['granularity', 'secretNameTemplate', 'customTags'] }, ]; @withFormFields(displayFields, formFieldGroups) export default class SyncDestinationsAwsSecretsManagerModel extends SyncDestinationModel { diff --git a/ui/app/models/sync/destinations/azure-kv.js b/ui/app/models/sync/destinations/azure-kv.js index fa363d6959..0fcf98e447 100644 --- a/ui/app/models/sync/destinations/azure-kv.js +++ b/ui/app/models/sync/destinations/azure-kv.js @@ -6,7 +6,7 @@ import SyncDestinationModel from '../destination'; import { attr } from '@ember-data/model'; import { withFormFields } from 'vault/decorators/model-form-fields'; - +// displayFields are used on the destination details view const displayFields = [ // connection details 'name', @@ -20,20 +20,13 @@ const displayFields = [ 'secretNameTemplate', 'customTags', ]; +// formFieldGroups are used on the create-edit destination view const formFieldGroups = [ { - default: [ - 'name', - 'keyVaultUri', - 'tenantId', - 'cloud', - 'clientId', - 'granularity', - 'secretNameTemplate', - 'customTags', - ], + default: ['name', 'keyVaultUri', 'tenantId', 'cloud', 'clientId'], }, { Credentials: ['clientSecret'] }, + { 'Advanced configuration': ['granularity', 'secretNameTemplate', 'customTags'] }, ]; @withFormFields(displayFields, formFieldGroups) export default class SyncDestinationsAzureKeyVaultModel extends SyncDestinationModel { diff --git a/ui/app/models/sync/destinations/gcp-sm.js b/ui/app/models/sync/destinations/gcp-sm.js index 6de8200eeb..784dd24857 100644 --- a/ui/app/models/sync/destinations/gcp-sm.js +++ b/ui/app/models/sync/destinations/gcp-sm.js @@ -6,7 +6,7 @@ import SyncDestinationModel from '../destination'; import { attr } from '@ember-data/model'; import { withFormFields } from 'vault/decorators/model-form-fields'; - +// displayFields are used on the destination details view const displayFields = [ // connection details 'name', @@ -17,9 +17,11 @@ const displayFields = [ 'secretNameTemplate', 'customTags', ]; +// formFieldGroups are used on the create-edit destination view const formFieldGroups = [ - { default: ['name', 'projectId', 'granularity', 'secretNameTemplate', 'customTags'] }, + { default: ['name', 'projectId'] }, { Credentials: ['credentials'] }, + { 'Advanced configuration': ['granularity', 'secretNameTemplate', 'customTags'] }, ]; @withFormFields(displayFields, formFieldGroups) export default class SyncDestinationsGoogleCloudSecretManagerModel extends SyncDestinationModel { diff --git a/ui/app/models/sync/destinations/gh.js b/ui/app/models/sync/destinations/gh.js index 8cd6e7bd14..5815b8e9b0 100644 --- a/ui/app/models/sync/destinations/gh.js +++ b/ui/app/models/sync/destinations/gh.js @@ -6,7 +6,7 @@ import SyncDestinationModel from '../destination'; import { attr } from '@ember-data/model'; import { withFormFields } from 'vault/decorators/model-form-fields'; - +// displayFields are used on the destination details view const displayFields = [ // connection details 'name', @@ -17,9 +17,12 @@ const displayFields = [ 'granularity', 'secretNameTemplate', ]; + +// formFieldGroups are used on the create-edit destination view const formFieldGroups = [ - { default: ['name', 'repositoryOwner', 'repositoryName', 'granularity', 'secretNameTemplate'] }, + { default: ['name', 'repositoryOwner', 'repositoryName'] }, { Credentials: ['accessToken'] }, + { 'Advanced configuration': ['granularity', 'secretNameTemplate'] }, ]; @withFormFields(displayFields, formFieldGroups) diff --git a/ui/app/models/sync/destinations/vercel-project.js b/ui/app/models/sync/destinations/vercel-project.js index b788a780c4..8599ff202d 100644 --- a/ui/app/models/sync/destinations/vercel-project.js +++ b/ui/app/models/sync/destinations/vercel-project.js @@ -21,7 +21,7 @@ const validations = { // getter/setter for the deploymentEnvironments model attribute deploymentEnvironmentsArray: [{ type: 'presence', message: 'At least one environment is required.' }], }; - +// displayFields are used on the destination details view const displayFields = [ // connection details 'name', @@ -33,9 +33,11 @@ const displayFields = [ 'granularity', 'secretNameTemplate', ]; +// formFieldGroups are used on the create-edit destination view const formFieldGroups = [ - { default: ['name', 'projectId', 'teamId', 'deploymentEnvironments', 'granularity', 'secretNameTemplate'] }, + { default: ['name', 'projectId', 'teamId', 'deploymentEnvironments'] }, { Credentials: ['accessToken'] }, + { 'Advanced configuration': ['granularity', 'secretNameTemplate'] }, ]; @withModelValidations(validations) @withFormFields(displayFields, formFieldGroups) diff --git a/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.hbs b/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.hbs index 5593fa5911..1349b170f1 100644 --- a/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.hbs +++ b/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.hbs @@ -12,27 +12,38 @@ {{#each @destination.formFieldGroups as |fieldGroup|}} {{#each-in fieldGroup as |group fields|}} - {{#if (and (eq group "Credentials") (not @destination.isNew))}} + {{#if (not-eq group "default")}}
- Credentials - - Connection credentials are sensitive information and the value cannot be read. Enable the input to update. + {{group}} + + {{this.groupSubtext group @destination.isNew}} - {{#each fields as |attr|}} + {{/if}} + {{#each fields as |attr|}} + {{#if (and (eq group "Credentials") (not @destination.isNew))}} - {{/each}} - {{else}} - {{#each fields as |attr|}} + {{else}} - {{/each}} - {{/if}} + {{/if}} + {{/each}} {{/each-in}} {{/each}} diff --git a/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.ts b/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.ts index 35d7d4221e..30198e7133 100644 --- a/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.ts +++ b/ui/lib/sync/addon/components/secrets/page/destinations/create-and-edit.ts @@ -56,6 +56,20 @@ export default class DestinationsCreateForm extends Component { }; } + groupSubtext(group: string, isNew: boolean) { + const dynamicText = isNew + ? 'used to authenticate with the destination' + : 'and the value cannot be read. Enable the input to update'; + switch (group) { + case 'Advanced configuration': + return 'Configuration options for the destination.'; + case 'Credentials': + return `Connection credentials are sensitive information ${dynamicText}.`; + default: + return ''; + } + } + @task @waitFor *save(event: Event) { diff --git a/ui/tests/helpers/sync/sync-selectors.js b/ui/tests/helpers/sync/sync-selectors.js index db3658f9a3..39ba75f840 100644 --- a/ui/tests/helpers/sync/sync-selectors.js +++ b/ui/tests/helpers/sync/sync-selectors.js @@ -86,6 +86,8 @@ export const PAGE = { toolbar: (btnText) => `[data-test-toolbar="${btnText}"]`, form: { enableInput: (attr) => `[data-test-enable-field="${attr}"] [data-test-icon="edit"]`, + fieldGroupHeader: (group) => `[data-test-destination-header="${group}"]`, + fieldGroupSubtext: (group) => `[data-test-destination-subText="${group}"]`, fillInByAttr: async (attr, value) => { // for handling more complex form input elements by attr name switch (attr) { 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 46c86a0398..302e7faaea 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 @@ -44,7 +44,7 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE }; }); - test('create: it renders and navigates back to create on cancel', async function (assert) { + test('create: it renders breadcrumbs and navigates back to create on cancel', async function (assert) { assert.expect(2); const { type } = SYNC_DESTINATIONS[0]; this.model = this.store.createRecord(`sync/destinations/${type}`, { type }); @@ -56,23 +56,59 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE assert.true(transition, 'transitions to vault.cluster.sync.secrets.destinations.create on cancel'); }); - test('edit: it renders and navigates back to details on cancel', async function (assert) { + test('create: it renders headers and fieldGroups subtext', async function (assert) { assert.expect(4); + const { type } = SYNC_DESTINATIONS[0]; + this.model = this.store.createRecord(`sync/destinations/${type}`, { type }); + + await this.renderFormComponent(); + assert + .dom(PAGE.form.fieldGroupHeader('Credentials')) + .hasText('Credentials', 'renders credentials section on create'); + assert + .dom(PAGE.form.fieldGroupHeader('Advanced configuration')) + .hasText('Advanced configuration', 'renders advanced configuration section on create'); + assert + .dom(PAGE.form.fieldGroupSubtext('Credentials')) + .hasText('Connection credentials are sensitive information used to authenticate with the destination.'); + assert + .dom(PAGE.form.fieldGroupSubtext('Advanced configuration')) + .hasText('Configuration options for the destination.'); + }); + + test('edit: it renders breadcrumbs and navigates back to details on cancel', async function (assert) { + assert.expect(2); this.model = this.generateModel(); await this.renderFormComponent(); assert.dom(PAGE.breadcrumbs).hasText('Secrets Sync Destinations Destination Edit Destination'); - assert.dom('h2').hasText('Credentials', 'renders credentials section on edit'); - assert - .dom('p.hds-foreground-faint') - .hasText( - 'Connection credentials are sensitive information and the value cannot be read. Enable the input to update.' - ); + await click(PAGE.cancelButton); const transition = this.transitionStub.calledWith('vault.cluster.sync.secrets.destinations.destination'); assert.true(transition, 'transitions to vault.cluster.sync.secrets.destinations.destination on cancel'); }); + test('edit: it renders headers and fieldGroup subtext', async function (assert) { + assert.expect(4); + this.model = this.generateModel(); + + await this.renderFormComponent(); + assert + .dom(PAGE.form.fieldGroupHeader('Credentials')) + .hasText('Credentials', 'renders credentials section on edit'); + assert + .dom(PAGE.form.fieldGroupHeader('Advanced configuration')) + .hasText('Advanced configuration', 'renders advanced configuration section on edit'); + assert + .dom(PAGE.form.fieldGroupSubtext('Credentials')) + .hasText( + 'Connection credentials are sensitive information and the value cannot be read. Enable the input to update.' + ); + assert + .dom(PAGE.form.fieldGroupSubtext('Advanced configuration')) + .hasText('Configuration options for the destination.'); + }); + test('edit: it PATCH updates custom_tags', async function (assert) { assert.expect(1); this.model = this.generateModel(); @@ -236,7 +272,7 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE const filteredObfuscatedFields = this.model.formFields.filter((field) => obfuscatedFields.includes(field.name) ); - assert.expect(filteredObfuscatedFields.length); + assert.expect(filteredObfuscatedFields.length * 2); await this.renderFormComponent(); // iterate over the form fields and filter for those that are obfuscated // fill those in and assert that they are masked @@ -246,6 +282,9 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE assert .dom(PAGE.maskedInput(field.name)) .hasClass('masked-font', `it renders ${field.name} for ${destination} with masked font`); + assert + .dom(PAGE.form.enableInput(field.name)) + .doesNotExist(`it does not render enable input for ${field.name}`); }); });