Skip to content

Commit

Permalink
Mask obfuscated Secret sync create/edit fields (#27348)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Monkeychip authored Jun 18, 2024
1 parent 027888b commit 66e78db
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 5 deletions.
3 changes: 3 additions & 0 deletions changelog/27348.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: Mask obfuscated fields when creating/editing a Secrets sync destination.
```
4 changes: 4 additions & 0 deletions ui/app/models/sync/destinations/aws-sm.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,17 @@ 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

@attr('string', {
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

Expand Down
2 changes: 2 additions & 0 deletions ui/app/models/sync/destinations/azure-kv.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion ui/app/models/sync/destinations/gcp-sm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions ui/app/models/sync/destinations/gh.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions ui/app/models/sync/destinations/vercel-project.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion ui/lib/core/addon/components/form-field.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@
<MaskedInput
@name={{@attr.name}}
@value={{or (get @model this.valuePath) @attr.options.defaultValue}}
@allowCopy="true"
@allowCopy={{if @attr.options.noCopy false true}}
@onChange={{this.setAndBroadcast}}
@onKeyUp={{@onKeyUp}}
/>
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/core/addon/components/masked-input.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
2 changes: 1 addition & 1 deletion ui/tests/acceptance/sync/secrets/destination-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions ui/tests/helpers/general-selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}"]`,
};
5 changes: 5 additions & 0 deletions ui/tests/helpers/sync/sync-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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 () {
Expand All @@ -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';
Expand Down

0 comments on commit 66e78db

Please sign in to comment.