-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Mask obfuscated Secret sync create/edit fields #27348
Changes from all commits
a4fc962
87b9b4b
e962667
9449204
9a1b545
98b04db
d0912a6
7de21b2
455098f
00944a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. | ||
``` |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 ""}} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we don't need or if the default is an empty string
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought that as well, but without the conditional it broke any test selector that didn't pass in a name. I suspect because it comes in as undefined? |
||||||
/> | ||||||
{{/if}} | ||||||
{{#if @allowCopy}} | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👏 |
||
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'; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should default to true. not ideal (rather it default to false) but working with an old pattern and trying to keep backporting/scope simple.