Default to Json editor if KV secret is nested (#24290)

* initial fix

* changelog

* fix

* fix test and add test coverage

* remove useless escape characters

* pr comments add more test coverage

changelog/24290.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: When Kv v2 secret is an object, fix so details view defaults to readOnly JSON editor.
+```

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

@@ -4,9 +4,9 @@
 ~}}
 
 {{#let (find-by "name" "path" @secret.allFields) as |attr|}}
-  {{#if @isEdit}}
+  {{#if (eq @type "edit")}}
     <ReadonlyFormField @attr={{attr}} @value={{get @secret attr.name}} />
-  {{else}}
+  {{else if (eq @type "create")}}
     <FormField @attr={{attr}} @model={{@secret}} @modelValidations={{@modelValidations}} @onKeyUp={{@pathValidations}} />
   {{/if}}
 {{/let}}
@@ -14,9 +14,10 @@
 <hr class="is-marginless has-background-gray-200" />
 {{#if @showJson}}
   <JsonEditor
-    @title="{{if @isEdit 'Version' 'Secret'}} data"
+    @title="{{if (eq @type 'create') 'Secret' 'Version'}} data"
     @value={{this.codeMirrorString}}
     @valueUpdated={{this.handleJson}}
+    @readOnly={{eq @type "details"}}
   />
   {{#if (or @modelValidations.secretData.errors this.lintingErrors)}}
     <AlertInline @type={{if this.lintingErrors "warning" "danger"}} @paddingTop={{true}}>
@@ -27,10 +28,18 @@
       {{/if}}
     </AlertInline>
   {{/if}}
+{{else if (eq @type "details")}}
+  {{#each-in @secret.secretData as |key value|}}
+    <InfoTableRow @label={{key}} @value={{value}} @alwaysRender={{true}}>
+      <MaskedInput @name={{key}} @value={{value}} @displayOnly={{true}} @allowCopy={{true}} @allowDownload={{true}} />
+    </InfoTableRow>
+  {{else}}
+    <InfoTableRow @label="" @value="" @alwaysRender={{true}} />
+  {{/each-in}}
 {{else}}
   <KvObjectEditor
     class="has-top-margin-m"
-    @label="{{if @isEdit 'Version' 'Secret'}} data"
+    @label="{{if (eq @type 'create') 'Secret' 'Version'}} data"
     @value={{@secret.secretData}}
     @onChange={{fn (mut @secret.secretData)}}
     @isMasked={{true}}

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

@@ -14,7 +14,7 @@ import { stringify } from 'core/helpers/stringify';
  * <KvDataFields
  *  @showJson={{true}}
  *  @secret={{@secret}}
- *  @isEdit={{true}}
+ *  @type="edit"
  *  @modelValidations={{this.modelValidations}}
  *  @pathValidations={{this.pathValidations}}
  * />
@@ -23,7 +23,7 @@ import { stringify } from 'core/helpers/stringify';
  * @param {boolean} showJson - boolean passed from parent to hide/show json editor
  * @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} [isEdit=false] - if true, this is a new secret version rather than a new secret. Used to change text for some form labels
+ * @param {boolean} [type=null] - can be edit, create, or details. Used to change text for some form labels
  */
 
 export default class KvDataFields extends Component {

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

@@ -15,7 +15,12 @@
 
   <:toolbarFilters>
     {{#unless this.emptyState}}
-      <Toggle @name="json" @checked={{this.showJsonView}} @onChange={{fn (mut this.showJsonView)}}>
+      <Toggle
+        @name="json"
+        @checked={{or this.showJsonView this.secretDataIsAdvanced}}
+        @onChange={{fn (mut this.showJsonView)}}
+        @disabled={{this.secretDataIsAdvanced}}
+      >
         <span class="has-text-grey">JSON</span>
       </Toggle>
     {{/unless}}
@@ -93,15 +98,10 @@
     {{/if}}
   </EmptyState>
 {{else}}
-  {{#if this.showJsonView}}
-    <JsonEditor @title="Version data" @value={{stringify @secret.secretData}} @readOnly={{true}} />
-  {{else}}
-    {{#each-in @secret.secretData as |key value|}}
-      <InfoTableRow @label={{key}} @value={{value}} @alwaysRender={{true}}>
-        <MaskedInput @name={{key}} @value={{value}} @displayOnly={{true}} @allowCopy={{true}} @allowDownload={{true}} />
-      </InfoTableRow>
-    {{else}}
-      <InfoTableRow @label="" @value="" @alwaysRender={{true}} />
-    {{/each-in}}
-  {{/if}}
+  <KvDataFields
+    @showJson={{or this.showJsonView this.secretDataIsAdvanced}}
+    @secret={{@secret}}
+    @modelValidations={{this.modelValidations}}
+    @type="details"
+  />
 {{/if}}

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

@@ -35,6 +35,16 @@ export default class KvSecretDetails extends Component {
 
   @tracked showJsonView = false;
   @tracked wrappedData = null;
+  secretDataIsAdvanced;
+
+  constructor() {
+    super(...arguments);
+    this.originalSecret = JSON.stringify(this.args.secret.secretData || {});
+    if (this.originalSecret.lastIndexOf('{') > 0) {
+      // Dumb way to check if there's a nested object in the secret
+      this.secretDataIsAdvanced = true;
+    }
+  }
 
   @action
   closeVersionMenu(dropdown) {

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

@@ -46,7 +46,7 @@
       @showJson={{or this.showJsonView this.secretDataIsAdvanced}}
       @secret={{@secret}}
       @modelValidations={{this.modelValidations}}
-      @isEdit={{true}}
+      @type="edit"
     />
 
     <div class="has-top-margin-m">

ui/lib/kv/addon/components/page/secrets/create.hbs

@@ -21,6 +21,7 @@
       @secret={{@secret}}
       @modelValidations={{this.modelValidations}}
       @pathValidations={{this.pathValidations}}
+      @type="create"
     />
 
     <ToggleButton

ui/tests/integration/components/kv/kv-data-fields-test.js

@@ -8,9 +8,9 @@ import { setupRenderingTest } from 'vault/tests/helpers';
 import { setupEngine } from 'ember-engines/test-support';
 import { setupMirage } from 'ember-cli-mirage/test-support';
 import { hbs } from 'ember-cli-htmlbars';
-import { fillIn, render } from '@ember/test-helpers';
+import { fillIn, render, click } from '@ember/test-helpers';
 import codemirror from 'vault/tests/helpers/codemirror';
-import { FORM } from 'vault/tests/helpers/kv/kv-selectors';
+import { PAGE, FORM } from 'vault/tests/helpers/kv/kv-selectors';
 
 module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
   setupRenderingTest(hooks);
@@ -27,8 +27,9 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
   test('it updates the secret model', async function (assert) {
     assert.expect(2);
 
-    await render(hbs`<KvDataFields @showJson={{false}} @secret={{this.secret}} />`, { owner: this.engine });
-
+    await render(hbs`<KvDataFields @showJson={{false}} @secret={{this.secret}} @type="create" />`, {
+      owner: this.engine,
+    });
     await fillIn(FORM.inputByAttr('path'), this.path);
     await fillIn(FORM.keyInput(), 'foo');
     await fillIn(FORM.maskedValueInput(), 'bar');
@@ -40,7 +41,6 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
     assert.expect(3);
 
     await render(hbs`<KvDataFields @showJson={{true}} @secret={{this.secret}} />`, { owner: this.engine });
-
     assert.strictEqual(
       codemirror().getValue(' '),
       `{ \"\": \"\" }`, // eslint-disable-line no-useless-escape
@@ -63,7 +63,7 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
       secretData: this.secret.secretData,
     });
 
-    await render(hbs`<KvDataFields @showJson={{false}} @isEdit={{true}} @secret={{this.secret}} />`, {
+    await render(hbs`<KvDataFields @showJson={{false}} @secret={{this.secret}} @type="edit" />`, {
       owner: this.engine,
     });
 
@@ -73,4 +73,35 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
     assert.dom(FORM.maskedValueInput()).hasValue('bar');
     assert.dom(FORM.dataInputLabel({ isJson: false })).hasText('Version data');
   });
+
+  test('it shows readonly info rows when viewing secret details of simple secret', async function (assert) {
+    assert.expect(3);
+    this.secret.secretData = { foo: 'bar' };
+    this.secret.path = this.path;
+
+    await render(hbs`<KvDataFields @showJson={{false}} @secret={{this.secret}} @type="details" />`, {
+      owner: this.engine,
+    });
+    assert.dom(PAGE.infoRow).exists({ count: 1 }, '1 row of data shows');
+    assert.dom(PAGE.infoRowValue('foo')).hasText('***********');
+    await click(PAGE.infoRowToggleMasked('foo'));
+    assert.dom(PAGE.infoRowValue('foo')).hasText('bar', 'secret value shows after toggle');
+  });
+
+  test('it shows readonly json editor when viewing secret details of complex secret', async function (assert) {
+    assert.expect(3);
+    this.secret.secretData = {
+      foo: {
+        bar: 'baz',
+      },
+    };
+    this.secret.path = this.path;
+
+    await render(hbs`<KvDataFields @showJson={{true}} @secret={{this.secret}} @type="details" />`, {
+      owner: this.engine,
+    });
+    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": "baz" }}`);
+  });
 });

ui/tests/integration/components/kv/page/kv-page-secret-details-test.js

@@ -23,8 +23,10 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook
     this.server.post('/sys/capabilities-self', allowAllCapabilitiesStub());
     this.backend = 'kv-engine';
     this.path = 'my-secret';
+    this.pathComplex = 'my-secret-object';
     this.version = 2;
     this.dataId = kvDataPath(this.backend, this.path);
+    this.dataIdComplex = kvDataPath(this.backend, this.pathComplex);
     this.metadataId = kvMetadataPath(this.backend, this.path);
 
     this.secretData = { foo: 'bar' };
@@ -38,6 +40,22 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook
       destroyed: false,
       version: this.version,
     });
+    // nested secret
+    this.secretDataComplex = {
+      foo: {
+        bar: 'baz',
+      },
+    };
+    this.store.pushPayload('kv/data', {
+      modelName: 'kv/data',
+      id: this.dataIdComplex,
+      secret_data: this.secretDataComplex,
+      created_time: '2023-08-20T02:12:17.379762Z',
+      custom_metadata: null,
+      deletion_time: '',
+      destroyed: false,
+      version: this.version,
+    });
 
     const metadata = this.server.create('kv-metadatum');
     metadata.id = this.metadataId;
@@ -48,6 +66,7 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook
 
     this.metadata = this.store.peekRecord('kv/metadata', this.metadataId);
     this.secret = this.store.peekRecord('kv/data', this.dataId);
+    this.secretComplex = this.store.peekRecord('kv/data', this.dataIdComplex);
 
     // this is the route model, not an ember data model
     this.model = {
@@ -61,6 +80,12 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook
       { label: this.model.backend, route: 'list' },
       { label: this.model.path },
     ];
+    this.modelComplex = {
+      backend: this.backend,
+      path: this.pathComplex,
+      secret: this.secretComplex,
+      metadata: this.metadata,
+    };
   });
 
   test('it renders secret details and toggles json view', async function (assert) {
@@ -90,6 +115,23 @@ module('Integration | Component | kv-v2 | Page::Secret::Details', function (hook
       .includesText(`Version ${this.version} created`, 'renders version and time created');
   });
 
+  test('it renders json view when secret is complex', async function (assert) {
+    assert.expect(3);
+    await render(
+      hbs`
+       <Page::Secret::Details
+        @path={{this.modelComplex.path}}
+        @secret={{this.modelComplex.secret}}
+        @breadcrumbs={{this.breadcrumbs}}
+      />
+      `,
+      { owner: this.engine }
+    );
+    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" }}`);
+  });
+
   test('it renders deleted empty state', async function (assert) {
     assert.expect(3);
     this.secret.deletionTime = '2023-07-23T02:12:17.379762Z';