From 5d561f072f9ada4e8a75027f4a1d9de51953fc0d Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 13 Dec 2023 16:00:53 -0600 Subject: [PATCH 1/6] Add obfuscateData method and tests --- ui/lib/core/addon/utils/advanced-secret.js | 20 ++++ ui/tests/unit/utils/advanced-secret-test.js | 124 ++++++++++++++++---- 2 files changed, 120 insertions(+), 24 deletions(-) diff --git a/ui/lib/core/addon/utils/advanced-secret.js b/ui/lib/core/addon/utils/advanced-secret.js index 3046c4b55760..2502501c5826 100644 --- a/ui/lib/core/addon/utils/advanced-secret.js +++ b/ui/lib/core/addon/utils/advanced-secret.js @@ -18,3 +18,23 @@ export function isAdvancedSecret(value) { return false; } } + +/** + * Method to obfuscate all values in a map, including nested values and arrays + * @param obj object + * @returns object + */ +export function obfuscateData(obj) { + if (typeof obj !== 'object' || Array.isArray(obj)) return obj; + const newObj = {}; + for (const key of Object.keys(obj)) { + if (Array.isArray(obj[key])) { + newObj[key] = obj[key].map(() => '********'); + } else if (typeof obj[key] === 'object') { + newObj[key] = obfuscateData(obj[key]); + } else { + newObj[key] = '********'; + } + } + return newObj; +} diff --git a/ui/tests/unit/utils/advanced-secret-test.js b/ui/tests/unit/utils/advanced-secret-test.js index 891a13ea9dce..657ffb348a0d 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 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)}`); + }); }); }); + 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)}`); + }); }); }); }); From f9b1df7b1014a5a1aa3d304f476c56a54f5a08fd Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 13 Dec 2023 16:17:34 -0600 Subject: [PATCH 2/6] add obscure option to JsonEditor + tests --- ui/lib/core/addon/components/json-editor.hbs | 10 ++++++++- ui/lib/core/addon/components/json-editor.js | 11 ++++++++++ .../components/json-editor-test.js | 21 +++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/ui/lib/core/addon/components/json-editor.hbs b/ui/lib/core/addon/components/json-editor.hbs index c3a29b72335b..9df380c59aa3 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}}
`); + 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'); + }); }); From c6897141804d10bf9ba7d0046904538fbe5ca8ad Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 13 Dec 2023 16:19:13 -0600 Subject: [PATCH 3/6] Enable obscured values for KV v2 details when secret is advanced --- ui/lib/kv/addon/components/kv-data-fields.hbs | 1 + ui/lib/kv/addon/components/kv-data-fields.js | 1 + ui/lib/kv/addon/components/page/secret/details.hbs | 1 + 3 files changed, 3 insertions(+) diff --git a/ui/lib/kv/addon/components/kv-data-fields.hbs b/ui/lib/kv/addon/components/kv-data-fields.hbs index 554c4cbdd1f2..7f7a26db3ce4 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 7dc6d8a865d7..1dc29447652b 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 1a30e1615958..8d1bb666dd63 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}} Date: Wed, 13 Dec 2023 16:32:40 -0600 Subject: [PATCH 4/6] coverage on kv acceptance test --- .../kv/kv-v2-workflow-edge-cases-test.js | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js b/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js index c5eb6a2a59cb..c1d73d5b2a92 100644 --- a/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js +++ b/ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js @@ -272,6 +272,11 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) { }); test('advanced secret values default to JSON display', async function (assert) { + const obscuredData = `{ + "foo3": { + "name": "********" + } +}`; await visit(`/vault/secrets/${this.backend}/kv/create`); await fillIn(FORM.inputByAttr('path'), 'complex'); @@ -279,10 +284,23 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) { assert.strictEqual(codemirror().getValue(), '{ "": "" }'); codemirror().setValue('{ "foo3": { "name": "bar3" } }'); await click(FORM.saveBtn); - // Future: test that json is automatic on details too + + // Details view + assert.dom(FORM.toggleJson).isDisabled(); + assert.dom(FORM.toggleJson).isChecked(); + assert.strictEqual( + codemirror().getValue(), + obscuredData, + 'Value is obscured by default on details view when advanced' + ); + await click('[data-test-toggle-input="revealValues"]'); + assert.false(codemirror().getValue().includes('*'), 'Value unobscured after toggle'); + + // New version view await click(PAGE.detail.createNewVersion); assert.dom(FORM.toggleJson).isDisabled(); assert.dom(FORM.toggleJson).isChecked(); + assert.false(codemirror().getValue().includes('*'), 'Values are not obscured on edit view'); }); test('does not register as advanced when value includes {', async function (assert) { await visit(`/vault/secrets/${this.backend}/kv/create`); From 4deca6af101230227ce10fbe7bd2ad350010f9e3 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Wed, 13 Dec 2023 16:43:14 -0600 Subject: [PATCH 5/6] Add changelog --- changelog/24530.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/24530.txt diff --git a/changelog/24530.txt b/changelog/24530.txt new file mode 100644 index 000000000000..12525d48b87f --- /dev/null +++ b/changelog/24530.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: obscure JSON values when KV v2 secret has nested objects +``` From 246ad50343b1b2f4837226306cd1422504940b67 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw Date: Thu, 14 Dec 2023 08:18:15 -0600 Subject: [PATCH 6/6] Fix failing test --- .../components/kv/page/kv-page-secret-details-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3241a257a531..ec076ac575cc 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) {