diff --git a/changelog/24530.txt b/changelog/24530.txt new file mode 100644 index 0000000000..12525d48b8 --- /dev/null +++ b/changelog/24530.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: obscure JSON values when KV v2 secret has nested objects +``` diff --git a/ui/lib/core/addon/components/json-editor.hbs b/ui/lib/core/addon/components/json-editor.hbs index c3a29b7233..9df380c59a 100644 --- a/ui/lib/core/addon/components/json-editor.hbs +++ b/ui/lib/core/addon/components/json-editor.hbs @@ -25,6 +25,14 @@ data-test-restore-example /> {{/if}} + {{#if (and @obscure @readOnly)}} + {{! For safety we only use obscured values in readonly mode }} +
+ + Reveal values + +
+ {{/if}}
'********'); + } else if (typeof obj[key] === 'object') { + newObj[key] = obfuscateData(obj[key]); + } else { + newObj[key] = '********'; + } + } + return newObj; +} diff --git a/ui/lib/kv/addon/components/kv-data-fields.hbs b/ui/lib/kv/addon/components/kv-data-fields.hbs index 554c4cbdd1..7f7a26db3c 100644 --- a/ui/lib/kv/addon/components/kv-data-fields.hbs +++ b/ui/lib/kv/addon/components/kv-data-fields.hbs @@ -16,6 +16,7 @@ diff --git a/ui/lib/kv/addon/components/kv-data-fields.js b/ui/lib/kv/addon/components/kv-data-fields.js index 7dc6d8a865..1dc2944765 100644 --- a/ui/lib/kv/addon/components/kv-data-fields.js +++ b/ui/lib/kv/addon/components/kv-data-fields.js @@ -24,6 +24,7 @@ import { stringify } from 'core/helpers/stringify'; * @param {object} [modelValidations] - object of errors. If attr.name is in object and has error message display in AlertInline. * @param {callback} [pathValidations] - callback function fired for the path input on key up * @param {boolean} [type=null] - can be edit, create, or details. Used to change text for some form labels + * @param {boolean} [obscureJson=false] - used to obfuscate json values in JsonEditor */ export default class KvDataFields extends Component { diff --git a/ui/lib/kv/addon/components/page/secret/details.hbs b/ui/lib/kv/addon/components/page/secret/details.hbs index 1a30e16159..8d1bb666dd 100644 --- a/ui/lib/kv/addon/components/page/secret/details.hbs +++ b/ui/lib/kv/addon/components/page/secret/details.hbs @@ -140,6 +140,7 @@ {{else}} `); + assert.dom('.CodeMirror-code').hasText(`{ "test": "********"}`, 'shows data with obscured values'); + assert.dom('[data-test-toggle-input="revealValues"]').isNotChecked('reveal values toggle is unchecked'); + await click('[data-test-toggle-input="revealValues"]'); + assert.dom('.CodeMirror-code').hasText(JSON_BLOB, 'shows data with real values'); + assert.dom('[data-test-toggle-input="revealValues"]').isChecked('reveal values toggle is checked'); + // turn obscure back on to ensure readonly overrides reveal setting + await click('[data-test-toggle-input="revealValues"]'); + this.set('readOnly', false); + assert.dom('[data-test-toggle-input="revealValues"]').doesNotExist('reveal values toggle is hidden'); + assert.dom('.CodeMirror-code').hasText(JSON_BLOB, 'shows data with real values on edit mode'); + }); }); diff --git a/ui/tests/integration/components/kv/page/kv-page-secret-details-test.js b/ui/tests/integration/components/kv/page/kv-page-secret-details-test.js index 3241a257a5..ec076ac575 100644 --- a/ui/tests/integration/components/kv/page/kv-page-secret-details-test.js +++ b/ui/tests/integration/components/kv/page/kv-page-secret-details-test.js @@ -140,7 +140,7 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook ); assert.dom(PAGE.infoRowValue('foo')).doesNotExist('does not render rows of secret data'); assert.dom(FORM.toggleJson).isDisabled(); - assert.dom('[data-test-component="code-mirror-modifier"]').includesText(`{ "foo": { "bar": "baz" }}`); + assert.dom('[data-test-component="code-mirror-modifier"]').exists('shows json editor'); }); test('it renders deleted empty state', async function (assert) { diff --git a/ui/tests/unit/utils/advanced-secret-test.js b/ui/tests/unit/utils/advanced-secret-test.js index 891a13ea9d..657ffb348a 100644 --- a/ui/tests/unit/utils/advanced-secret-test.js +++ b/ui/tests/unit/utils/advanced-secret-test.js @@ -3,38 +3,114 @@ * SPDX-License-Identifier: BUSL-1.1 */ -import { isAdvancedSecret } from 'core/utils/advanced-secret'; +import { isAdvancedSecret, obfuscateData } from 'core/utils/advanced-secret'; import { module, test } from 'qunit'; module('Unit | Utility | advanced-secret', function () { - test('it returns false for non-valid JSON', function (assert) { - assert.expect(5); - let result; - ['some-string', 'character{string', '{value}', '[blah]', 'multi\nline\nstring'].forEach((value) => { - result = isAdvancedSecret('some-string'); - assert.false(result, `returns false for ${value}`); + module('isAdvancedSecret', function () { + test('it returns false for non-valid JSON', function (assert) { + assert.expect(5); + let result; + ['some-string', 'character{string', '{value}', '[blah]', 'multi\nline\nstring'].forEach((value) => { + result = isAdvancedSecret('some-string'); + assert.false(result, `returns false for ${value}`); + }); + }); + + test('it returns false for single-level objects', function (assert) { + assert.expect(3); + let result; + [{ single: 'one' }, { first: '1', two: 'three' }, ['my', 'array']].forEach((value) => { + result = isAdvancedSecret(JSON.stringify(value)); + assert.false(result, `returns false for object ${JSON.stringify(value)}`); + }); + }); + + test('it returns true for any nested object', function (assert) { + assert.expect(3); + let result; + [ + { single: { one: 'uno' } }, + { first: ['this', 'counts\ntoo'] }, + { deeply: { nested: { item: 1 } } }, + ].forEach((value) => { + result = isAdvancedSecret(JSON.stringify(value)); + assert.true(result, `returns true for object ${JSON.stringify(value)}`); + }); }); }); - - test('it returns false for single-level objects', function (assert) { - assert.expect(3); - let result; - [{ single: 'one' }, { first: '1', two: 'three' }, ['my', 'array']].forEach((value) => { - result = isAdvancedSecret(JSON.stringify(value)); - assert.false(result, `returns false for object ${JSON.stringify(value)}`); + module('obfuscateData', function () { + test('it obfuscates values of an object', function (assert) { + assert.expect(4); + [ + { + name: 'flat map', + data: { + first: 'one', + second: 'two', + third: 'three', + }, + obscured: { + first: '********', + second: '********', + third: '********', + }, + }, + { + name: 'nested map', + data: { + first: 'one', + second: { + third: 'two', + }, + }, + obscured: { + first: '********', + second: { + third: '********', + }, + }, + }, + { + name: 'numbers and arrays', + data: { + first: 1, + list: ['one', 'two'], + second: { + third: ['one', 'two'], + number: 5, + }, + }, + obscured: { + first: '********', + list: ['********', '********'], + second: { + third: ['********', '********'], + number: '********', + }, + }, + }, + { + name: 'object arrays', + data: { + list: [{ one: 'one' }, { two: 'two' }], + }, + obscured: { + list: ['********', '********'], + }, + }, + ].forEach((test) => { + const result = obfuscateData(test.data); + assert.deepEqual(result, test.obscured, `obfuscates values of ${test.name}`); + }); }); - }); - test('it returns true for any nested object', function (assert) { - assert.expect(3); - let result; - [ - { single: { one: 'uno' } }, - { first: ['this', 'counts\ntoo'] }, - { deeply: { nested: { item: 1 } } }, - ].forEach((value) => { - result = isAdvancedSecret(JSON.stringify(value)); - assert.true(result, `returns true for object ${JSON.stringify(value)}`); + test('it does not obfuscate non-object values', function (assert) { + assert.expect(3); + ['some-string', 5, ['my', 'array']].forEach((test) => { + const result = obfuscateData(test); + assert.deepEqual(result, test, `does not obfuscate value ${JSON.stringify(test)}`); + }); }); }); });