UI: Obscure values for nested KV v2 secret (#24530)

* Add obfuscateData method and tests

* add obscure option to JsonEditor + tests

* Enable obscured values for KV v2 details when secret is advanced

* coverage on kv acceptance test

* Add changelog

changelog/24530.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+ui: obscure JSON values when KV v2 secret has nested objects
+```

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 }}
+            <div>
+              <Toggle @name="revealValues" @checked={{this.revealValues}} @onChange={{fn (mut this.revealValues)}}>
+                <span class="has-text-grey">Reveal values</span>
+              </Toggle>
+            </div>
+          {{/if}}
           <div class="toolbar-separator"></div>
           <Hds::Copy::Button
             @container={{@container}}
@@ -40,7 +48,7 @@
   {{/if}}
   <div
     {{code-mirror
-      content=(or @value @example)
+      content=(if this.showObfuscatedData this.obfuscatedData (or @value @example))
       extraKeys=@extraKeys
       gutters=@gutters
       lineNumbers=(if @readOnly false true)

ui/lib/core/addon/components/json-editor.js

@@ -5,6 +5,9 @@
 
 import Component from '@glimmer/component';
 import { action } from '@ember/object';
+import { tracked } from '@glimmer/tracking';
+import { stringify } from 'core/helpers/stringify';
+import { obfuscateData } from 'core/utils/advanced-secret';
 
 /**
  * @module JsonEditor
@@ -34,10 +37,18 @@ import { action } from '@ember/object';
  */
 
 export default class JsonEditorComponent extends Component {
+  @tracked revealValues = false;
   get getShowToolbar() {
     return this.args.showToolbar === false ? false : true;
   }
 
+  get showObfuscatedData() {
+    return this.args.readOnly && this.args.obscure && !this.revealValues;
+  }
+  get obfuscatedData() {
+    return stringify([obfuscateData(JSON.parse(this.args.value))], {});
+  }
+
   @action
   onSetup(editor) {
     // store reference to codemirror editor so that it can be passed to valueUpdated when restoring example

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;
+}

ui/lib/kv/addon/components/kv-data-fields.hbs

@@ -16,6 +16,7 @@
   <JsonEditor
     @title="{{if (eq @type 'create') 'Secret' 'Version'}} data"
     @value={{this.codeMirrorString}}
+    @obscure={{@obscureJson}}
     @valueUpdated={{this.handleJson}}
     @readOnly={{eq @type "details"}}
   />

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 {

ui/lib/kv/addon/components/page/secret/details.hbs

@@ -140,6 +140,7 @@
 {{else}}
   <KvDataFields
     @showJson={{or this.showJsonView this.secretDataIsAdvanced}}
+    @obscureJson={{this.secretDataIsAdvanced}}
     @secret={{@secret}}
     @modelValidations={{this.modelValidations}}
     @type="details"

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`);

ui/tests/integration/components/json-editor-test.js

@@ -101,4 +101,25 @@ module('Integration | Component | json-editor', function (hooks) {
     assert.dom('.CodeMirror-code').hasText(`1${this.example}`, 'Example is restored');
     assert.strictEqual(this.value, null, 'Value is cleared on restore example');
   });
+
+  test('obscure option works correctly', async function (assert) {
+    this.set('readOnly', true);
+    await render(hbs`<JsonEditor
+      @value={{this.json_blob}}
+      @obscure={{true}}
+      @readOnly={{this.readOnly}}
+      @valueUpdated={{this.valueUpdated}}
+      @onFocusOut={{this.onFocusOut}}
+    />`);
+    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');
+  });
 });

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) {

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) {
+  module('isAdvancedSecret', function () {
-    assert.expect(5);
+    test('it returns false for non-valid JSON', function (assert) {
-    let result;
+      assert.expect(5);
-    ['some-string', 'character{string', '{value}', '[blah]', 'multi\nline\nstring'].forEach((value) => {
+      let result;
-      result = isAdvancedSecret('some-string');
+      ['some-string', 'character{string', '{value}', '[blah]', 'multi\nline\nstring'].forEach((value) => {
-      assert.false(result, `returns false for ${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)}`);
+      });
     });
   });
-
+  module('obfuscateData', function () {
-  test('it returns false for single-level objects', function (assert) {
+    test('it obfuscates values of an object', function (assert) {
-    assert.expect(3);
+      assert.expect(4);
-    let result;
+      [
-    [{ single: 'one' }, { first: '1', two: 'three' }, ['my', 'array']].forEach((value) => {
+        {
-      result = isAdvancedSecret(JSON.stringify(value));
+          name: 'flat map',
-      assert.false(result, `returns false for object ${JSON.stringify(value)}`);
+          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) {
+    test('it does not obfuscate non-object values', function (assert) {
-    assert.expect(3);
+      assert.expect(3);
-    let result;
+      ['some-string', 5, ['my', 'array']].forEach((test) => {
-    [
+        const result = obfuscateData(test);
-      { single: { one: 'uno' } },
+        assert.deepEqual(result, test, `does not obfuscate value ${JSON.stringify(test)}`);
-      { 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)}`);
     });
   });
 });