UI: better calculation for advanced secret in KV v2 (#24513)

* Add util for determining whether secret data is advanced

* Add test coverage for bug

* use non-dumb logic for detecting advanced object

* Add changelog

* Add header

* Move util to core

* Add escaped newline to test coverage

* headers again *eyeroll*

changelog/24513.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: fix KV v2 details view defaulting to JSON view when secret value includes `{`
+```

ui/lib/core/addon/utils/advanced-secret.js

@@ -0,0 +1,20 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: MPL-2.0
+ */
+
+/**
+ * Method to check whether the secret value is a nested object (returns true)
+ * All other values return false
+ * @param value string or stringified JSON
+ * @returns boolean
+ */
+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');
+  } catch (e) {
+    return false;
+  }
+}

ui/lib/core/app/utils/advanced-secret.js

@@ -0,0 +1,6 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: BUSL-1.1
+ */
+
+export { default } from 'core/utils/advanced-secret';

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

@@ -11,6 +11,7 @@ import { inject as service } from '@ember/service';
 import { task } from 'ember-concurrency';
 import { waitFor } from '@ember/test-waiters';
 import { isDeleted } from 'kv/utils/kv-deleted';
+import { isAdvancedSecret } from 'core/utils/advanced-secret';
 
 /**
  * @module KvSecretDetails renders the key/value data of a KV secret.
@@ -42,10 +43,7 @@ export default class KvSecretDetails extends Component {
     super(...arguments);
     this.fetchSyncStatus.perform();
     this.originalSecret = JSON.stringify(this.args.secret.secretData || {});
-    if (this.originalSecret.lastIndexOf('{') > 0) {
+    this.secretDataIsAdvanced = isAdvancedSecret(this.originalSecret);
-      // Dumb way to check if there's a nested object in the secret
-      this.secretDataIsAdvanced = true;
-    }
   }
 
   @action

ui/lib/kv/addon/components/page/secret/edit.js

@@ -9,6 +9,7 @@ import { tracked } from '@glimmer/tracking';
 import { task } from 'ember-concurrency';
 import { inject as service } from '@ember/service';
 import errorMessage from 'vault/utils/error-message';
+import { isAdvancedSecret } from 'core/utils/advanced-secret';
 
 /**
  * @module KvSecretEdit is used for creating a new version of a secret
@@ -43,10 +44,7 @@ export default class KvSecretEdit extends Component {
   constructor() {
     super(...arguments);
     this.originalSecret = JSON.stringify(this.args.secret.secretData || {});
-    if (this.originalSecret.lastIndexOf('{') > 0) {
+    this.secretDataIsAdvanced = isAdvancedSecret(this.originalSecret);
-      // Dumb way to check if there's a nested object in the secret
-      this.secretDataIsAdvanced = true;
-    }
   }
 
   get showOldVersionAlert() {

ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js

@@ -271,7 +271,7 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
     assert.dom(PAGE.list.item()).exists({ count: 2 }, 'two secrets are listed');
   });
 
-  test('complex values default to JSON display', async function (assert) {
+  test('advanced secret values default to JSON display', async function (assert) {
     await visit(`/vault/secrets/${this.backend}/kv/create`);
     await fillIn(FORM.inputByAttr('path'), 'complex');
 
@@ -284,6 +284,17 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
     assert.dom(FORM.toggleJson).isDisabled();
     assert.dom(FORM.toggleJson).isChecked();
   });
+  test('does not register as advanced when value includes {', async function (assert) {
+    await visit(`/vault/secrets/${this.backend}/kv/create`);
+    await fillIn(FORM.inputByAttr('path'), 'not-advanced');
+
+    await fillIn(FORM.keyInput(), 'foo');
+    await fillIn(FORM.maskedValueInput(), '{bar}');
+    await click(FORM.saveBtn);
+    await click(PAGE.detail.createNewVersion);
+    assert.dom(FORM.toggleJson).isNotDisabled();
+    assert.dom(FORM.toggleJson).isNotChecked();
+  });
 });
 
 // NAMESPACE TESTS

ui/tests/unit/utils/advanced-secret-test.js

@@ -0,0 +1,40 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: BUSL-1.1
+ */
+
+import { isAdvancedSecret } 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}`);
+    });
+  });
+
+  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)}`);
+    });
+  });
+});