Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create sections for Secrets sync destination fields for create/edit view #27538

Merged
merged 18 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/27538.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: Creates separate section for updating sensitive creds for Secrets sync create/edit view.
```
5 changes: 4 additions & 1 deletion ui/app/models/sync/destinations/aws-sm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 {
Expand Down
15 changes: 4 additions & 11 deletions ui/app/models/sync/destinations/azure-kv.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions ui/app/models/sync/destinations/gcp-sm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 {
Expand Down
7 changes: 5 additions & 2 deletions ui/app/models/sync/destinations/gh.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions ui/app/models/sync/destinations/vercel-project.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")}}
<hr class="has-top-margin-xl has-bottom-margin-l has-background-gray-200" />
<Hds::Text::Display @tag="h2" @size="400" @weight="bold">Credentials</Hds::Text::Display>
<Hds::Text::Body @tag="p" @size="100" @color="faint" class="has-bottom-margin-m">
Connection credentials are sensitive information and the value cannot be read. Enable the input to update.
<Hds::Text::Display
@tag="h2"
@size="400"
@weight="bold"
data-test-destination-header={{group}}
>{{group}}</Hds::Text::Display>
<Hds::Text::Body
@tag="p"
@size="100"
@color="faint"
class="has-bottom-margin-m"
data-test-destination-subText={{group}}
>
{{this.groupSubtext group @destination.isNew}}
</Hds::Text::Body>
{{#each fields as |attr|}}
{{/if}}
{{#each fields as |attr|}}
{{#if (and (eq group "Credentials") (not @destination.isNew))}}
<EnableInput data-test-enable-field={{attr.name}} class="field" @attr={{attr}}>
<FormField @attr={{attr}} @model={{@destination}} @modelValidations={{this.modelValidations}} />
</EnableInput>
{{/each}}
{{else}}
{{#each fields as |attr|}}
{{else}}
<FormField
@attr={{attr}}
@model={{@destination}}
@modelValidations={{this.modelValidations}}
@onKeyUp={{this.updateWarningValidation}}
/>
{{/each}}
{{/if}}
{{/if}}
{{/each}}
{{/each-in}}
{{/each}}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,20 @@ export default class DestinationsCreateForm extends Component<Args> {
};
}

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';
Comment on lines +59 to +62
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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';
groupSubtext(group: string) {
const dynamicText = this.args.destination.isNew
? 'used to authenticate with the destination'
: 'and the value cannot be read. Enable the input to update';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went this exact same route, but it fails to read the args. It might be because I'm iterating over via a each and then each-in and then an each again, but passing in via the template solved the problem
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strange! it must be how the template helper functions compile 🤔

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) {
Expand Down
2 changes: 2 additions & 0 deletions ui/tests/helpers/sync/sync-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -56,21 +56,57 @@ 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

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');

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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved this into a subtext specific test

.dom('p.hds-foreground-faint')
.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.'
);
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');
assert
.dom(PAGE.form.fieldGroupSubtext('Advanced configuration'))
.hasText('Configuration options for the destination.');
});

test('edit: it PATCH updates custom_tags', async function (assert) {
Expand Down Expand Up @@ -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
Expand All @@ -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}`);
});
});

Expand Down
Loading