From 08eaac1ac0a8b7ba04c59904915a001be3f96130 Mon Sep 17 00:00:00 2001
From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com>
Date: Tue, 6 Feb 2024 11:50:22 -0600
Subject: [PATCH 1/8] UI: KV version 2 advanced secret updates (#25235)

* Add obfuscation for JSON values on KV data view

* Show JSON editor by default if secret data is complex, but do not disable JSON toggle

* Add text object warning to kv object editor

* a11y fix: do not disable show diff toggle

* test and language cleanup

* update language

* Update tests for expected behavior

* language!

* Add changelog
---
 changelog/25235.txt                           |   3 +
 ui/lib/core/addon/components/json-editor.hbs  |   8 ++
 ui/lib/core/addon/components/json-editor.js   |  11 ++
 .../addon/components/kv-object-editor.hbs     |  12 +-
 .../core/addon/components/kv-object-editor.js |  30 ++---
 ui/lib/core/addon/utils/advanced-secret.js    |  20 +++
 ui/lib/kv/addon/components/kv-data-fields.hbs |   2 +
 .../addon/components/page/secret/details.hbs  |   5 +-
 .../addon/components/page/secret/details.js   |   6 +-
 .../kv/addon/components/page/secret/edit.hbs  |  12 +-
 .../kv/addon/components/page/secret/edit.js   |   6 +-
 .../kv/kv-v2-workflow-edge-cases-test.js      |   2 +-
 ui/tests/helpers/kv/kv-selectors.js           |   1 +
 .../components/json-editor-test.js            |  21 +++
 .../components/kv-object-editor-test.js       |  13 ++
 .../components/kv/kv-data-fields-test.js      |   6 +-
 .../kv/page/kv-page-secret-details-test.js    |   6 +-
 .../kv/page/kv-page-secret-edit-test.js       |   2 +-
 ui/tests/unit/utils/advanced-secret-test.js   | 124 ++++++++++++++----
 19 files changed, 227 insertions(+), 63 deletions(-)
 create mode 100644 changelog/25235.txt

diff --git a/changelog/25235.txt b/changelog/25235.txt
new file mode 100644
index 000000000000..d4df23ebf49b
--- /dev/null
+++ b/changelog/25235.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Do not disable JSON display toggle for KV version 2 secrets
+```
\ No newline at end of file
diff --git a/ui/lib/core/addon/components/json-editor.hbs b/ui/lib/core/addon/components/json-editor.hbs
index c92b5dd3f2d2..b9ac08404543 100644
--- a/ui/lib/core/addon/components/json-editor.hbs
+++ b/ui/lib/core/addon/components/json-editor.hbs
@@ -27,6 +27,14 @@
               <Icon @name="reload" />
             </button>
           {{/if}}
+          {{#if (and @allowObscure @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>
           <CopyButton
             class="button is-transparent"
diff --git a/ui/lib/core/addon/components/json-editor.js b/ui/lib/core/addon/components/json-editor.js
index c9e6c2d84fd2..d37208620886 100644
--- a/ui/lib/core/addon/components/json-editor.js
+++ b/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
@@ -30,10 +33,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.allowObscure && !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
diff --git a/ui/lib/core/addon/components/kv-object-editor.hbs b/ui/lib/core/addon/components/kv-object-editor.hbs
index 30e8b5e562b1..b5c077c74751 100644
--- a/ui/lib/core/addon/components/kv-object-editor.hbs
+++ b/ui/lib/core/addon/components/kv-object-editor.hbs
@@ -25,7 +25,6 @@
             @value={{row.name}}
             placeholder={{this.placeholders.key}}
             {{on "change" (fn this.updateRow row index)}}
-            {{on "input" (fn this.validateKey index)}}
             class="input"
           />
         </div>
@@ -71,7 +70,7 @@
           {{/if}}
         </div>
       </div>
-      {{#if (includes index this.whitespaceWarningRows)}}
+      {{#if (this.showWhitespaceWarning row.name)}}
         <div class="has-bottom-margin-s">
           <AlertInline
             @type="warning"
@@ -80,6 +79,15 @@
           />
         </div>
       {{/if}}
+      {{#if (this.showNonStringWarning row.value)}}
+        <div class="has-bottom-margin-s">
+          <AlertInline
+            @type="warning"
+            @message="This value will be saved as a string. If you need to save a non-string value, please use the JSON editor."
+            data-test-kv-object-warning={{index}}
+          />
+        </div>
+      {{/if}}
     {{/each}}
     {{#if this.hasDuplicateKeys}}
       <Hds::Alert data-test-duplicate-keys-warning @type="inline" @color="warning" as |A|>
diff --git a/ui/lib/core/addon/components/kv-object-editor.js b/ui/lib/core/addon/components/kv-object-editor.js
index 40d0f3974f56..7fd7d17234ec 100644
--- a/ui/lib/core/addon/components/kv-object-editor.js
+++ b/ui/lib/core/addon/components/kv-object-editor.js
@@ -9,7 +9,6 @@ import { isNone } from '@ember/utils';
 import { assert } from '@ember/debug';
 import { action } from '@ember/object';
 import { guidFor } from '@ember/object/internals';
-import { A } from '@ember/array';
 import KVObject from 'vault/lib/kv-object';
 
 /**
@@ -26,7 +25,7 @@ import KVObject from 'vault/lib/kv-object';
  * ```
  * @param {string} value - the value is captured from the model.
  * @param {function} onChange - function that captures the value on change
- * @param {boolean} [isMasked = false] - when true the <MaskedInput> renders instead of the default <textarea> to input the value portion of the key/value object 
+ * @param {boolean} [isMasked = false] - when true the <MaskedInput> renders instead of the default <textarea> to input the value portion of the key/value object
  * @param {function} [onKeyUp] - function passed in that handles the dom keyup event. Used for validation on the kv custom metadata.
  * @param {string} [label] - label displayed over key value inputs
  * @param {string} [labelClass] - override default label class in FormFieldLabel component
@@ -35,11 +34,12 @@ import KVObject from 'vault/lib/kv-object';
  * @param {string} [subText] - placed under label.
  * @param {string} [keyPlaceholder] - placeholder for key input
  * @param {string} [valuePlaceholder] - placeholder for value input
+ * @param {boolean} [allowWhiteSpace = false] - when true, allows whitespace in the key input
+ * @param {boolean} [warnNonStringValues = false] - when true, shows a warning if the value is a non-string
  */
 
 export default class KvObjectEditor extends Component {
   @tracked kvData;
-  whitespaceWarningRows = A();
 
   get placeholders() {
     return {
@@ -75,7 +75,6 @@ export default class KvObjectEditor extends Component {
     const oldObj = this.kvData.objectAt(index);
     assert('object guids match', guidFor(oldObj) === guidFor(object));
     this.kvData.removeAt(index);
-    this.whitespaceWarningRows.removeObject(index);
     this.args.onChange(this.kvData.toJSON());
   }
   @action
@@ -84,16 +83,17 @@ export default class KvObjectEditor extends Component {
       this.args.onKeyUp(event.target.value);
     }
   }
-  @action
-  validateKey(rowIndex, event) {
-    const { value } = event.target;
-    const keyHasWhitespace = new RegExp('\\s', 'g').test(value);
-    const rows = [...this.whitespaceWarningRows];
-    const rowHasWarning = rows.includes(rowIndex);
-    if (!keyHasWhitespace && rowHasWarning) {
-      this.whitespaceWarningRows.removeObject(rowIndex);
-    } else if (keyHasWhitespace && !rowHasWarning) {
-      this.whitespaceWarningRows.addObject(rowIndex);
+  showWhitespaceWarning = (name) => {
+    if (this.args.allowWhiteSpace) return false;
+    return new RegExp('\\s', 'g').test(name);
+  };
+  showNonStringWarning = (value) => {
+    if (!this.args.warnNonStringValues) return false;
+    try {
+      JSON.parse(value);
+      return true;
+    } catch (e) {
+      return false;
     }
-  }
+  };
 }
diff --git a/ui/lib/core/addon/utils/advanced-secret.js b/ui/lib/core/addon/utils/advanced-secret.js
index e1082a48d302..bbf885db1743 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/lib/kv/addon/components/kv-data-fields.hbs b/ui/lib/kv/addon/components/kv-data-fields.hbs
index 9ca39c1ec983..a2410bdee4ac 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 @@
   <JsonEditor
     @title="{{if (eq @type 'create') 'Secret' 'Version'}} data"
     @value={{this.stringifiedSecretData}}
+    @allowObscure={{true}}
     @valueUpdated={{this.handleJson}}
     @readOnly={{eq @type "details"}}
   />
@@ -43,5 +44,6 @@
     @value={{@secret.secretData}}
     @onChange={{fn (mut @secret.secretData)}}
     @isMasked={{true}}
+    @warnNonStringValues={{true}}
   />
 {{/if}}
\ No newline at end of file
diff --git a/ui/lib/kv/addon/components/page/secret/details.hbs b/ui/lib/kv/addon/components/page/secret/details.hbs
index 8d67fdb77cec..855b1a89558a 100644
--- a/ui/lib/kv/addon/components/page/secret/details.hbs
+++ b/ui/lib/kv/addon/components/page/secret/details.hbs
@@ -19,9 +19,8 @@
         @name="json"
         @status="success"
         @size="small"
-        @checked={{or this.showJsonView this.secretDataIsAdvanced}}
+        @checked={{this.showJsonView}}
         @onChange={{fn (mut this.showJsonView)}}
-        @disabled={{this.secretDataIsAdvanced}}
       >
         <span class="has-text-grey">JSON</span>
       </Toggle>
@@ -101,7 +100,7 @@
   </EmptyState>
 {{else}}
   <KvDataFields
-    @showJson={{or this.showJsonView this.secretDataIsAdvanced}}
+    @showJson={{this.showJsonView}}
     @secret={{@secret}}
     @modelValidations={{this.modelValidations}}
     @type="details"
diff --git a/ui/lib/kv/addon/components/page/secret/details.js b/ui/lib/kv/addon/components/page/secret/details.js
index ae3edc59f663..c09222806b5b 100644
--- a/ui/lib/kv/addon/components/page/secret/details.js
+++ b/ui/lib/kv/addon/components/page/secret/details.js
@@ -36,12 +36,14 @@ export default class KvSecretDetails extends Component {
 
   @tracked showJsonView = false;
   @tracked wrappedData = null;
-  secretDataIsAdvanced;
 
   constructor() {
     super(...arguments);
     this.originalSecret = JSON.stringify(this.args.secret.secretData || {});
-    this.secretDataIsAdvanced = isAdvancedSecret(this.originalSecret);
+    if (isAdvancedSecret(this.originalSecret)) {
+      // Default to JSON view if advanced
+      this.showJsonView = true;
+    }
   }
 
   @action
diff --git a/ui/lib/kv/addon/components/page/secret/edit.hbs b/ui/lib/kv/addon/components/page/secret/edit.hbs
index 66be3373b3e1..d7b2a7e61a66 100644
--- a/ui/lib/kv/addon/components/page/secret/edit.hbs
+++ b/ui/lib/kv/addon/components/page/secret/edit.hbs
@@ -5,14 +5,7 @@
 
 <KvPageHeader @breadcrumbs={{@breadcrumbs}} @pageTitle="Create New Version">
   <:toolbarFilters>
-    <Toggle
-      @name="json"
-      @status="success"
-      @size="small"
-      @checked={{or this.showJsonView this.secretDataIsAdvanced}}
-      @onChange={{fn (mut this.showJsonView)}}
-      @disabled={{this.secretDataIsAdvanced}}
-    >
+    <Toggle @name="json" @checked={{this.showJsonView}} @onChange={{fn (mut this.showJsonView)}}>
       <span class="has-text-grey">JSON</span>
     </Toggle>
   </:toolbarFilters>
@@ -45,7 +38,7 @@
     <MessageError @model={{@secret}} @errorMessage={{this.errorMessage}} />
 
     <KvDataFields
-      @showJson={{or this.showJsonView this.secretDataIsAdvanced}}
+      @showJson={{this.showJsonView}}
       @secret={{@secret}}
       @modelValidations={{this.modelValidations}}
       @type="edit"
@@ -58,7 +51,6 @@
         @size="small"
         @onChange={{fn (mut this.showDiff)}}
         @checked={{this.showDiff}}
-        @disabled={{not this.diffDelta}}
       >
         <span class="ttl-picker-label is-large">Show diff</span><br />
         <div class="description has-text-grey" data-test-diff-description>{{if
diff --git a/ui/lib/kv/addon/components/page/secret/edit.js b/ui/lib/kv/addon/components/page/secret/edit.js
index 3bad5c3170e9..2d50cc213a01 100644
--- a/ui/lib/kv/addon/components/page/secret/edit.js
+++ b/ui/lib/kv/addon/components/page/secret/edit.js
@@ -39,12 +39,14 @@ export default class KvSecretEdit extends Component {
   @tracked modelValidations;
   @tracked invalidFormAlert;
   originalSecret;
-  secretDataIsAdvanced;
 
   constructor() {
     super(...arguments);
     this.originalSecret = JSON.stringify(this.args.secret.secretData || {});
-    this.secretDataIsAdvanced = isAdvancedSecret(this.originalSecret);
+    if (isAdvancedSecret(this.originalSecret)) {
+      // Default to JSON view if advanced
+      this.showJsonView = true;
+    }
   }
 
   get showOldVersionAlert() {
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 71b288f279a8..280c61c1f69f 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
@@ -281,7 +281,7 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
     await click(FORM.saveBtn);
     // Future: test that json is automatic on details too
     await click(PAGE.detail.createNewVersion);
-    assert.dom(FORM.toggleJson).isDisabled();
+    assert.dom(FORM.toggleJson).isNotDisabled();
     assert.dom(FORM.toggleJson).isChecked();
   });
 
diff --git a/ui/tests/helpers/kv/kv-selectors.js b/ui/tests/helpers/kv/kv-selectors.js
index 14a783c2c638..a5a6fe38e0c3 100644
--- a/ui/tests/helpers/kv/kv-selectors.js
+++ b/ui/tests/helpers/kv/kv-selectors.js
@@ -95,6 +95,7 @@ export const FORM = {
   inputByAttr: (attr) => `[data-test-input="${attr}"]`,
   fieldByAttr: (attr) => `[data=test=field="${attr}"]`, // formfield
   toggleJson: '[data-test-toggle-input="json"]',
+  toggleJsonValues: '[data-test-toggle-input="revealValues"]',
   toggleMasked: '[data-test-button="toggle-masked"]',
   toggleMetadata: '[data-test-metadata-toggle]',
   jsonEditor: '[data-test-component="code-mirror-modifier"]',
diff --git a/ui/tests/integration/components/json-editor-test.js b/ui/tests/integration/components/json-editor-test.js
index f38e17f8aecd..8c7a949cadff 100644
--- a/ui/tests/integration/components/json-editor-test.js
+++ b/ui/tests/integration/components/json-editor-test.js
@@ -100,4 +100,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}}
+      @allowObscure={{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');
+  });
 });
diff --git a/ui/tests/integration/components/kv-object-editor-test.js b/ui/tests/integration/components/kv-object-editor-test.js
index 8f7eeccdbb21..97c82fa083cf 100644
--- a/ui/tests/integration/components/kv-object-editor-test.js
+++ b/ui/tests/integration/components/kv-object-editor-test.js
@@ -110,4 +110,17 @@ module('Integration | Component | kv-object-editor', function (hooks) {
     await click('[data-test-kv-delete-row="0"]');
     assert.dom('[data-test-kv-whitespace-warning="0"]').doesNotExist();
   });
+
+  test('it should display object warning for values when warnNonStringValues true', async function (assert) {
+    const objValue = `{
+      "a": "b"
+    }`;
+    await render(hbs`<KvObjectEditor @onChange={{this.spy}} @warnNonStringValues={{true}} />`);
+    await fillIn('[data-test-kv-value="0"]', objValue);
+    assert.dom('[data-test-kv-object-warning="0"]').exists();
+    await fillIn('[data-test-kv-value="0"]', 'test ');
+    assert.dom('[data-test-kv-object-warning="0"]').doesNotExist();
+    await fillIn('[data-test-kv-value="0"]', 7);
+    assert.dom('[data-test-kv-object-warning="0"]').exists();
+  });
 });
diff --git a/ui/tests/integration/components/kv/kv-data-fields-test.js b/ui/tests/integration/components/kv/kv-data-fields-test.js
index bbaaa87e8d9a..b8be49b45d00 100644
--- a/ui/tests/integration/components/kv/kv-data-fields-test.js
+++ b/ui/tests/integration/components/kv/kv-data-fields-test.js
@@ -93,7 +93,7 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
   });
 
   test('it shows readonly json editor when viewing secret details of complex secret', async function (assert) {
-    assert.expect(3);
+    assert.expect(4);
     this.secret.secretData = {
       foo: {
         bar: 'baz',
@@ -106,6 +106,10 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
     });
     assert.dom(PAGE.infoRowValue('foo')).doesNotExist('does not render rows of secret data');
     assert.dom('[data-test-component="code-mirror-modifier"]').hasClass('readonly-codemirror');
+    assert
+      .dom('[data-test-component="code-mirror-modifier"]')
+      .includesText(`{ "foo": { "bar": "********" }}`);
+    await click(FORM.toggleJsonValues);
     assert.dom('[data-test-component="code-mirror-modifier"]').includesText(`{ "foo": { "bar": "baz" }}`);
   });
 });
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 a1b3e630edec..f7d2105c730b 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
@@ -109,6 +109,7 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook
     await click(FORM.toggleMasked);
     assert.dom(PAGE.infoRowValue('foo')).hasText('bar', 'renders secret value');
     await click(FORM.toggleJson);
+    await click(FORM.toggleJsonValues);
     assert.propEqual(parseJsonEditor(find), this.secretData, 'json editor renders secret data');
     assert
       .dom(PAGE.detail.versionTimestamp)
@@ -116,7 +117,7 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook
   });
 
   test('it renders json view when secret is complex', async function (assert) {
-    assert.expect(3);
+    assert.expect(4);
     await render(
       hbs`
        <Page::Secret::Details
@@ -128,7 +129,8 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook
       { owner: this.engine }
     );
     assert.dom(PAGE.infoRowValue('foo')).doesNotExist('does not render rows of secret data');
-    assert.dom(FORM.toggleJson).isDisabled();
+    assert.dom(FORM.toggleJson).isChecked();
+    assert.dom(FORM.toggleJson).isNotDisabled();
     assert.dom('[data-test-component="code-mirror-modifier"]').includesText(`{ "foo": { "bar": "baz" }}`);
   });
 
diff --git a/ui/tests/integration/components/kv/page/kv-page-secret-edit-test.js b/ui/tests/integration/components/kv/page/kv-page-secret-edit-test.js
index 8b9602ee8ef6..b65df846049e 100644
--- a/ui/tests/integration/components/kv/page/kv-page-secret-edit-test.js
+++ b/ui/tests/integration/components/kv/page/kv-page-secret-edit-test.js
@@ -106,7 +106,7 @@ module('Integration | Component | kv-v2 | Page::Secret::Edit', function (hooks)
       { owner: this.engine }
     );
 
-    assert.dom(PAGE.edit.toggleDiff).isDisabled('Diff toggle is disabled');
+    assert.dom(PAGE.edit.toggleDiff).isNotDisabled('Diff toggle is not disabled');
     assert.dom(PAGE.edit.toggleDiffDescription).hasText('No changes to show. Update secret to view diff');
     assert.dom(PAGE.diff.visualDiff).doesNotExist('Does not show visual diff');
 
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 617e34e8a0b6cedddf3aa1abe1cf4b80ab96ee07 Mon Sep 17 00:00:00 2001
From: Chelsea Shaw <cshaw@hashicorp.com>
Date: Tue, 6 Feb 2024 13:38:00 -0600
Subject: [PATCH 2/8] Json editor shows obscured values

---
 ui/lib/core/addon/components/json-editor.hbs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/lib/core/addon/components/json-editor.hbs b/ui/lib/core/addon/components/json-editor.hbs
index b9ac08404543..f60712998f32 100644
--- a/ui/lib/core/addon/components/json-editor.hbs
+++ b/ui/lib/core/addon/components/json-editor.hbs
@@ -50,7 +50,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)

From 43fac4140240cfd6797757994b4e77a73b93b522 Mon Sep 17 00:00:00 2001
From: Chelsea Shaw <cshaw@hashicorp.com>
Date: Tue, 6 Feb 2024 13:49:00 -0600
Subject: [PATCH 3/8] Fix toggle sizes

---
 ui/lib/core/addon/components/json-editor.hbs    | 7 ++++++-
 ui/lib/kv/addon/components/page/secret/edit.hbs | 8 +++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/ui/lib/core/addon/components/json-editor.hbs b/ui/lib/core/addon/components/json-editor.hbs
index f60712998f32..7aac5d6f773f 100644
--- a/ui/lib/core/addon/components/json-editor.hbs
+++ b/ui/lib/core/addon/components/json-editor.hbs
@@ -30,7 +30,12 @@
           {{#if (and @allowObscure @readOnly)}}
             {{! For safety we only use obscured values in readonly mode }}
             <div>
-              <Toggle @name="revealValues" @checked={{this.revealValues}} @onChange={{fn (mut this.revealValues)}}>
+              <Toggle
+                @name="revealValues"
+                @checked={{this.revealValues}}
+                @size="small"
+                @onChange={{fn (mut this.revealValues)}}
+              >
                 <span class="has-text-grey">Reveal values</span>
               </Toggle>
             </div>
diff --git a/ui/lib/kv/addon/components/page/secret/edit.hbs b/ui/lib/kv/addon/components/page/secret/edit.hbs
index d7b2a7e61a66..817d11a8f34d 100644
--- a/ui/lib/kv/addon/components/page/secret/edit.hbs
+++ b/ui/lib/kv/addon/components/page/secret/edit.hbs
@@ -5,7 +5,13 @@
 
 <KvPageHeader @breadcrumbs={{@breadcrumbs}} @pageTitle="Create New Version">
   <:toolbarFilters>
-    <Toggle @name="json" @checked={{this.showJsonView}} @onChange={{fn (mut this.showJsonView)}}>
+    <Toggle
+      @name="json"
+      @status="success"
+      @size="small"
+      @checked={{this.showJsonView}}
+      @onChange={{fn (mut this.showJsonView)}}
+    >
       <span class="has-text-grey">JSON</span>
     </Toggle>
   </:toolbarFilters>

From 09d6578cd4123f4da021ec3e10eff40b4e9cb608 Mon Sep 17 00:00:00 2001
From: Chelsea Shaw <cshaw@hashicorp.com>
Date: Tue, 6 Feb 2024 14:37:43 -0600
Subject: [PATCH 4/8] fix tests missed in merge conflicts

---
 .../secrets/backend/kv/kv-v2-workflow-edge-cases-test.js      | 1 +
 .../components/kv/page/kv-page-secret-details-test.js         | 4 ++++
 2 files changed, 5 insertions(+)

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 280c61c1f69f..ad9dcdd50191 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
@@ -302,6 +302,7 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
     await fillIn(FORM.inputByAttr('path'), 'complex_version_test');
 
     await click(FORM.toggleJson);
+    await click(FORM.toggleJsonValues);
     codemirror().setValue('{ "foo1": { "name": "bar1" } }');
     await click(FORM.saveBtn);
 
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 f7d2105c730b..0c1ed371c029 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
@@ -131,6 +131,10 @@ 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).isChecked();
     assert.dom(FORM.toggleJson).isNotDisabled();
+    assert
+      .dom('[data-test-component="code-mirror-modifier"]')
+      .includesText(`{ "foo": { "bar": "********" }}`);
+    await click(FORM.toggleJsonValues);
     assert.dom('[data-test-component="code-mirror-modifier"]').includesText(`{ "foo": { "bar": "baz" }}`);
   });
 

From 826a50132c150f6d3008292e5897ec7edcdd9d18 Mon Sep 17 00:00:00 2001
From: Chelsea Shaw <cshaw@hashicorp.com>
Date: Tue, 6 Feb 2024 15:42:02 -0600
Subject: [PATCH 5/8] more test fixes

---
 .../secrets/backend/kv/kv-v2-workflow-edge-cases-test.js        | 2 +-
 .../components/kv/page/kv-page-secret-details-test.js           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

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 ad9dcdd50191..689f83350bb3 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
@@ -302,7 +302,6 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
     await fillIn(FORM.inputByAttr('path'), 'complex_version_test');
 
     await click(FORM.toggleJson);
-    await click(FORM.toggleJsonValues);
     codemirror().setValue('{ "foo1": { "name": "bar1" } }');
     await click(FORM.saveBtn);
 
@@ -314,6 +313,7 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
     // View the first version and make sure the secret data is correct
     await click(PAGE.detail.versionDropdown);
     await click(`${PAGE.detail.version(1)} a`);
+    await click(FORM.toggleJsonValues);
     assert.strictEqual(codemirror().getValue(), obscuredDataV1, 'Version one data is displayed');
 
     // Navigate back the second version and make sure the secret data is correct
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 0c1ed371c029..81f2d4b96a1f 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
@@ -117,7 +117,7 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook
   });
 
   test('it renders json view when secret is complex', async function (assert) {
-    assert.expect(4);
+    assert.expect(5);
     await render(
       hbs`
        <Page::Secret::Details

From a5d0561cb1823c073cd88c598d7480c722dcb3bf Mon Sep 17 00:00:00 2001
From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com>
Date: Wed, 7 Feb 2024 08:47:03 -0600
Subject: [PATCH 6/8] Update advanced-secret tests and logic (#25247)

---
 ui/lib/core/addon/utils/advanced-secret.js  |  6 +++---
 ui/tests/unit/utils/advanced-secret-test.js | 23 ++++++++++++++-------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/ui/lib/core/addon/utils/advanced-secret.js b/ui/lib/core/addon/utils/advanced-secret.js
index bbf885db1743..d17ba71e762e 100644
--- a/ui/lib/core/addon/utils/advanced-secret.js
+++ b/ui/lib/core/addon/utils/advanced-secret.js
@@ -11,9 +11,9 @@
  */
 export function isAdvancedSecret(value) {
   try {
-    const json = JSON.parse(value);
-    if (Array.isArray(json)) return false;
-    return Object.values(json).some((value) => typeof value !== 'string');
+    const obj = typeof value === 'string' ? JSON.parse(value) : value;
+    if (Array.isArray(obj)) return false;
+    return Object.values(obj).any((value) => typeof value !== 'string');
   } catch (e) {
     return false;
   }
diff --git a/ui/tests/unit/utils/advanced-secret-test.js b/ui/tests/unit/utils/advanced-secret-test.js
index 657ffb348a0d..1a58b38e68e7 100644
--- a/ui/tests/unit/utils/advanced-secret-test.js
+++ b/ui/tests/unit/utils/advanced-secret-test.js
@@ -12,30 +12,37 @@ module('Unit | Utility | advanced-secret', function () {
       assert.expect(5);
       let result;
       ['some-string', 'character{string', '{value}', '[blah]', 'multi\nline\nstring'].forEach((value) => {
-        result = isAdvancedSecret('some-string');
+        result = isAdvancedSecret(value);
         assert.false(result, `returns false for ${value}`);
       });
     });
 
     test('it returns false for single-level objects', function (assert) {
-      assert.expect(3);
+      assert.expect(6);
       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)}`);
+        const stringValue = JSON.stringify(value);
+        result = isAdvancedSecret(value);
+        assert.false(result, `returns false for object ${stringValue}`);
+        result = isAdvancedSecret(stringValue);
+        assert.false(result, `returns false for json ${stringValue}`);
       });
     });
 
-    test('it returns true for any nested object', function (assert) {
-      assert.expect(3);
+    test('it returns true for any nested object or number value', function (assert) {
+      assert.expect(8);
       let result;
       [
         { single: { one: 'uno' } },
         { first: ['this', 'counts\ntoo'] },
         { deeply: { nested: { item: 1 } } },
+        { number: 5 },
       ].forEach((value) => {
-        result = isAdvancedSecret(JSON.stringify(value));
-        assert.true(result, `returns true for object ${JSON.stringify(value)}`);
+        const stringValue = JSON.stringify(value);
+        result = isAdvancedSecret(value);
+        assert.true(result, `returns true for object ${stringValue}`);
+        result = isAdvancedSecret(stringValue);
+        assert.true(result, `returns true for json ${stringValue}`);
       });
     });
   });

From 8e428198473817b614bea22232a44e35faf2f354 Mon Sep 17 00:00:00 2001
From: Chelsea Shaw <cshaw@hashicorp.com>
Date: Wed, 7 Feb 2024 14:45:24 -0600
Subject: [PATCH 7/8] Add changes from 25269

---
 changelog/25269.txt                                  | 3 +++
 ui/lib/core/addon/components/masked-input.hbs        | 2 +-
 ui/lib/core/addon/components/masked-input.js         | 7 +++++++
 ui/tests/integration/components/masked-input-test.js | 2 --
 4 files changed, 11 insertions(+), 3 deletions(-)
 create mode 100644 changelog/25269.txt

diff --git a/changelog/25269.txt b/changelog/25269.txt
new file mode 100644
index 000000000000..706c5abe4c67
--- /dev/null
+++ b/changelog/25269.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fix copy button not working on masked input when value is not a string
+```
\ No newline at end of file
diff --git a/ui/lib/core/addon/components/masked-input.hbs b/ui/lib/core/addon/components/masked-input.hbs
index 85e30f941737..b7dff59c43e8 100644
--- a/ui/lib/core/addon/components/masked-input.hbs
+++ b/ui/lib/core/addon/components/masked-input.hbs
@@ -36,7 +36,7 @@
   {{/if}}
   {{#if @allowCopy}}
     <CopyButton
-      @clipboardText={{@value}}
+      @clipboardText={{this.copyValue}}
       @success={{action (set-flash-message "Data copied!")}}
       @error={{action (set-flash-message "Error copying data" "danger")}}
       class="copy-button button {{if @displayOnly 'is-compact'}}"
diff --git a/ui/lib/core/addon/components/masked-input.js b/ui/lib/core/addon/components/masked-input.js
index ac49475de349..f72e63a51a57 100644
--- a/ui/lib/core/addon/components/masked-input.js
+++ b/ui/lib/core/addon/components/masked-input.js
@@ -71,4 +71,11 @@ export default class MaskedInputComponent extends Component {
   @action toggleStringifyDownload(event) {
     this.stringifyDownload = event.target.checked;
   }
+
+  get copyValue() {
+    // Value must be a string to be copied
+    if (typeof this.args.value === 'string') return this.args.value;
+    if (typeof this.args.value === 'object') return JSON.stringify(this.args.value);
+    return this.args.value.toString();
+  }
 }
diff --git a/ui/tests/integration/components/masked-input-test.js b/ui/tests/integration/components/masked-input-test.js
index d5a1bb3d83a4..3a0fda09141a 100644
--- a/ui/tests/integration/components/masked-input-test.js
+++ b/ui/tests/integration/components/masked-input-test.js
@@ -57,8 +57,6 @@ module('Integration | Component | masked input', function (hooks) {
 
     await click('[data-test-download-icon]');
     assert.ok(component.downloadButtonIsPresent, 'clicking download icon opens modal with download button');
-
-    assert;
   });
 
   test('it shortens all outputs when displayOnly and masked', async function (assert) {

From 6af24191ddf5d1b7de11cd38eb9e09b1c225b6ff Mon Sep 17 00:00:00 2001
From: Chelsea Shaw <cshaw@hashicorp.com>
Date: Wed, 14 Feb 2024 13:33:35 -0600
Subject: [PATCH 8/8] fix tests

---
 ui/lib/core/addon/components/masked-input.js | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ui/lib/core/addon/components/masked-input.js b/ui/lib/core/addon/components/masked-input.js
index f72e63a51a57..ac27526fea94 100644
--- a/ui/lib/core/addon/components/masked-input.js
+++ b/ui/lib/core/addon/components/masked-input.js
@@ -74,8 +74,9 @@ export default class MaskedInputComponent extends Component {
 
   get copyValue() {
     // Value must be a string to be copied
-    if (typeof this.args.value === 'string') return this.args.value;
-    if (typeof this.args.value === 'object') return JSON.stringify(this.args.value);
-    return this.args.value.toString();
+    const { value } = this.args;
+    if (!value || typeof value === 'string') return value;
+    if (typeof value === 'object') return JSON.stringify(value);
+    return value.toString();
   }
 }