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 6 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.
```
7 changes: 7 additions & 0 deletions ui/app/models/sync/destination.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ export default class SyncDestinationModel extends Model {
})
secretNameTemplate;

@attr('object', {
Monkeychip marked this conversation as resolved.
Show resolved Hide resolved
subText:
'An optional set of informational key-value pairs added as additional metadata on secrets synced to this destination. Custom tags are merged with built-in tags.',
editType: 'kv',
})
customTags;

@attr('string', {
editType: 'radio',
label: 'Secret sync granularity',
Expand Down
12 changes: 4 additions & 8 deletions 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'] },
{ config: ['granularity', 'secretNameTemplate', 'customTags'] },
];
@withFormFields(displayFields, formFieldGroups)
export default class SyncDestinationsAwsSecretsManagerModel extends SyncDestinationModel {
Expand Down Expand Up @@ -53,13 +56,6 @@ export default class SyncDestinationsAwsSecretsManagerModel extends SyncDestinat
})
region;

@attr('object', {
subText:
'An optional set of informational key-value pairs added as additional metadata on secrets synced to this destination. Custom tags are merged with built-in tags.',
editType: 'kv',
})
customTags;

@attr('string', {
label: 'Role ARN',
subText:
Expand Down
22 changes: 4 additions & 18 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'] },
{ config: ['granularity', 'secretNameTemplate', 'customTags'] },
];
@withFormFields(displayFields, formFieldGroups)
export default class SyncDestinationsAzureKeyVaultModel extends SyncDestinationModel {
Expand Down Expand Up @@ -73,11 +66,4 @@ export default class SyncDestinationsAzureKeyVaultModel extends SyncDestinationM
editDisabled: true,
})
cloud;

@attr('object', {
subText:
'An optional set of informational key-value pairs added as additional metadata on secrets synced to this destination. Custom tags are merged with built-in tags.',
editType: 'kv',
})
customTags;
}
13 changes: 4 additions & 9 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'] },
{ config: ['granularity', 'secretNameTemplate', 'customTags'] },
];
@withFormFields(displayFields, formFieldGroups)
export default class SyncDestinationsGoogleCloudSecretManagerModel extends SyncDestinationModel {
Expand All @@ -38,11 +40,4 @@ export default class SyncDestinationsGoogleCloudSecretManagerModel extends SyncD
docLink: '/vault/docs/secrets/gcp#authentication',
})
credentials; // obfuscated, never returned by API. Masking handled by EnableInput component

@attr('object', {
subText:
'An optional set of informational key-value pairs added as additional metadata on secrets synced to this destination. Custom tags are merged with built-in tags.',
editType: 'kv',
})
customTags;
}
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'] },
{ config: ['granularity', 'secretNameTemplate'] },
];

@withFormFields(displayFields, formFieldGroups)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,26 @@

{{#each @destination.formFieldGroups as |fieldGroup|}}
{{#each-in fieldGroup as |group fields|}}
{{#if (and (eq group "Credentials") (not @destination.isNew))}}
{{! h2 titles Credentials and Configuration for FormFieldGroups to be displayed in both create and edit }}
{{#if (eq group "Credentials")}}
Monkeychip marked this conversation as resolved.
Show resolved Hide resolved
<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.
Connection credentials are sensitive information
{{#if @destination.isNew}} used to authenticate with the destination. {{else}}
and the value cannot be read. Enable the input to update.
{{/if}}
Copy link
Contributor

@hellobontempo hellobontempo Jun 21, 2024

Choose a reason for hiding this comment

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

There is a lot more logic now that relies on whether a model isNew or not. Could we add test coverage for this logic?

Copy link
Contributor Author

@Monkeychip Monkeychip Jun 26, 2024

Choose a reason for hiding this comment

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

Done and dusted. Note the maskedInput vs wrapped maskedInputs by the enableInput component on create vs edit are covered in the create-and-edit test. They're a little hard to see. I added one for create in this PR. And the edit masked check is done by calling enableInput on the test here. If those values were not wrapped by the correct component, you couldn't find this value.

</Hds::Text::Body>
{{#each fields as |attr|}}
<EnableInput data-test-enable-field={{attr.name}} class="field" @attr={{attr}}>
<FormField @attr={{attr}} @model={{@destination}} @modelValidations={{this.modelValidations}} />
</EnableInput>
{{/each}}
{{else}}
{{/if}}
{{#if (eq group "config")}}
<hr class="has-top-margin-xl has-bottom-margin-l has-background-gray-200" />
<Hds::Text::Display @tag="h2" @size="400" @weight="bold">Advanced configuration</Hds::Text::Display>
<Hds::Text::Body @tag="p" @size="100" @color="faint" class="has-bottom-margin-m">
Configuration options for the destination.
</Hds::Text::Body>
{{/if}}
{{! CREATE fields }}
{{#if @destination.isNew}}
Monkeychip marked this conversation as resolved.
Show resolved Hide resolved
{{#each fields as |attr|}}
<FormField
@attr={{attr}}
Expand All @@ -32,6 +40,24 @@
@onKeyUp={{this.updateWarningValidation}}
/>
{{/each}}
{{else}}
{{! EDIT }}
{{#if (eq group "Credentials")}}
{{#each fields as |attr|}}
<EnableInput data-test-enable-field={{attr.name}} class="field" @attr={{attr}}>
Monkeychip marked this conversation as resolved.
Show resolved Hide resolved
<FormField @attr={{attr}} @model={{@destination}} @modelValidations={{this.modelValidations}} />
</EnableInput>
{{/each}}
{{else}}
{{#each fields as |attr|}}
<FormField
@attr={{attr}}
@model={{@destination}}
@modelValidations={{this.modelValidations}}
@onKeyUp={{this.updateWarningValidation}}
/>
{{/each}}
{{/if}}
{{/if}}
{{/each-in}}
{{/each}}
Expand Down
Loading