From 76b7a66587e97742e12bce3e65b137ab85062054 Mon Sep 17 00:00:00 2001
From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com>
Date: Wed, 3 May 2023 12:02:39 -0500
Subject: [PATCH] UI: Handle edge cases for PKI import results (#20474)

---
 .../components/pki-import-pem-bundle.hbs      | 21 ++++++-----
 .../addon/components/pki-import-pem-bundle.ts | 26 ++++++++++++--
 .../acceptance/pki/pki-action-forms-test.js   | 36 +++++++++++++++----
 .../pki/pki-import-pem-bundle-test.js         |  9 +++--
 ui/types/vault/app-types.ts                   |  5 +++
 ui/types/vault/models/pki/action.d.ts         |  8 ++---
 6 files changed, 78 insertions(+), 27 deletions(-)

diff --git a/ui/lib/pki/addon/components/pki-import-pem-bundle.hbs b/ui/lib/pki/addon/components/pki-import-pem-bundle.hbs
index 3453d5accc..33b20fa27d 100644
--- a/ui/lib/pki/addon/components/pki-import-pem-bundle.hbs
+++ b/ui/lib/pki/addon/components/pki-import-pem-bundle.hbs
@@ -12,22 +12,25 @@
     </div>
   </div>
   <div class="box is-fullwidth is-sideless is-marginless is-paddingless" data-test-imported-bundle-mapping>
-    {{#each-in this.importedResponse as |issuer key|}}
-      <div class="box is-marginless no-top-shadow has-slim-padding" data-test-import-pair={{concat issuer "_" key}}>
+    {{#each this.importedResponse as |imported|}}
+      <div
+        class="box is-marginless no-top-shadow has-slim-padding"
+        data-test-import-pair={{concat imported.issuer "_" imported.key}}
+      >
         <div class="is-flex-start">
           <div class="is-flex-1 basis-0 has-bottom-margin-xs" data-test-imported-issuer>
-            {{#if issuer}}
-              <LinkTo @route="issuers.issuer.details" @model={{issuer}}>
-                {{issuer}}
+            {{#if imported.issuer}}
+              <LinkTo @route="issuers.issuer.details" @model={{imported.issuer}}>
+                {{imported.issuer}}
               </LinkTo>
             {{else}}
               None
             {{/if}}
           </div>
           <div class="is-flex-1 basis-0 has-bottom-margin-xs" data-test-imported-key>
-            {{#if key}}
-              <LinkTo @route="keys.key.details" @model={{key}}>
-                {{key}}
+            {{#if imported.key}}
+              <LinkTo @route="keys.key.details" @model={{imported.key}}>
+                {{imported.key}}
               </LinkTo>
             {{else}}
               None
@@ -35,7 +38,7 @@
           </div>
         </div>
       </div>
-    {{/each-in}}
+    {{/each}}
   </div>
   <footer>
     <div class="field is-grouped is-fullwidth has-top-margin-l">
diff --git a/ui/lib/pki/addon/components/pki-import-pem-bundle.ts b/ui/lib/pki/addon/components/pki-import-pem-bundle.ts
index 66070ce9b2..2c2caf8ab5 100644
--- a/ui/lib/pki/addon/components/pki-import-pem-bundle.ts
+++ b/ui/lib/pki/addon/components/pki-import-pem-bundle.ts
@@ -47,9 +47,29 @@ export default class PkiImportPemBundle extends Component<Args> {
   @tracked errorBanner = '';
 
   get importedResponse() {
-    // mapping only exists after success
-    // TODO VAULT-14791: handle issuer already exists, but key doesn't -- empty object returned here
-    return this.args.model.mapping;
+    const { mapping, importedIssuers, importedKeys } = this.args.model;
+    // Even if there are no imported items, mapping will be an empty object from API response
+    if (undefined === mapping) return null;
+
+    const importList = (importedIssuers || []).map((issuer: string) => {
+      const key = mapping[issuer];
+      return { issuer, key };
+    });
+
+    // Check each imported key and make sure it's in the list
+    (importedKeys || []).forEach((key) => {
+      const matchIdx = importList.findIndex((item) => item.key === key);
+      // If key isn't accounted for, add it without a matching issuer
+      if (matchIdx === -1) {
+        importList.push({ issuer: '', key });
+      }
+    });
+
+    if (importList.length === 0) {
+      // If no new items were imported but the import call was successful, the UI will show accordingly
+      return [{ issuer: '', key: '' }];
+    }
+    return importList;
   }
 
   @task
diff --git a/ui/tests/acceptance/pki/pki-action-forms-test.js b/ui/tests/acceptance/pki/pki-action-forms-test.js
index 409c87bcb2..e4b2c892bb 100644
--- a/ui/tests/acceptance/pki/pki-action-forms-test.js
+++ b/ui/tests/acceptance/pki/pki-action-forms-test.js
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: MPL-2.0
  */
 
-import { module, skip, test } from 'qunit';
+import { module, test } from 'qunit';
 import { setupApplicationTest } from 'ember-qunit';
 import { click, currentURL, fillIn, typeIn, visit } from '@ember/test-helpers';
 import { setupMirage } from 'ember-cli-mirage/test-support';
@@ -76,8 +76,7 @@ module('Acceptance | pki action forms test', function (hooks) {
         'redirects to overview when done'
       );
     });
-    skip('with many imports', async function (assert) {
-      // TODO VAULT-14791
+    test('with many imports', async function (assert) {
       this.server.post(`${this.mountPath}/config/ca`, () => {
         return {
           request_id: 'some-config-id',
@@ -94,6 +93,7 @@ module('Acceptance | pki action forms test', function (hooks) {
       await authPage.login(this.pkiAdminToken);
       await visit(`/vault/secrets/${this.mountPath}/pki/configuration/create`);
       await click(S.configuration.optionByKey('import'));
+      assert.dom(S.configuration.importForm).exists('import form is shown save');
       await click('[data-test-text-toggle]');
       await fillIn('[data-test-text-file-textarea]', this.pemBundle);
       await click('[data-test-pki-import-pem-bundle]');
@@ -115,8 +115,7 @@ module('Acceptance | pki action forms test', function (hooks) {
         'redirects to overview when done'
       );
     });
-    skip('shows imported items when keys is empty', async function (assert) {
-      // TODO VAULT-14791
+    test('shows imported items when keys is empty', async function (assert) {
       this.server.post(`${this.mountPath}/config/ca`, () => {
         return {
           request_id: 'some-config-id',
@@ -133,6 +132,7 @@ module('Acceptance | pki action forms test', function (hooks) {
       await authPage.login(this.pkiAdminToken);
       await visit(`/vault/secrets/${this.mountPath}/pki/configuration/create`);
       await click(S.configuration.optionByKey('import'));
+      assert.dom(S.configuration.importForm).exists('import form is shown save');
       await click('[data-test-text-toggle]');
       await fillIn('[data-test-text-file-textarea]', this.pemBundle);
       await click('[data-test-pki-import-pem-bundle]');
@@ -140,7 +140,31 @@ module('Acceptance | pki action forms test', function (hooks) {
       assert.dom(S.configuration.importForm).doesNotExist('import form is hidden after save');
       assert.dom(S.configuration.importMapping).exists('import mapping is shown after save');
       assert.dom(S.configuration.importedIssuer).hasText('my-imported-issuer', 'Issuer value is displayed');
-      assert.dom(S.configuration.importedKey).hasText('my-imported-key', 'Key value is displayed');
+      assert.dom(S.configuration.importedKey).hasText('None', 'Shows placeholder value for key');
+    });
+    test('shows None for imported items if nothing new imported', async function (assert) {
+      this.server.post(`${this.mountPath}/config/ca`, () => {
+        return {
+          request_id: 'some-config-id',
+          data: {
+            imported_issuers: null,
+            imported_keys: null,
+            mapping: {},
+          },
+        };
+      });
+      await authPage.login(this.pkiAdminToken);
+      await visit(`/vault/secrets/${this.mountPath}/pki/configuration/create`);
+      await click(S.configuration.optionByKey('import'));
+      assert.dom(S.configuration.importForm).exists('import form is shown save');
+      await click('[data-test-text-toggle]');
+      await fillIn('[data-test-text-file-textarea]', this.pemBundle);
+      await click('[data-test-pki-import-pem-bundle]');
+
+      assert.dom(S.configuration.importForm).doesNotExist('import form is hidden after save');
+      assert.dom(S.configuration.importMapping).exists('import mapping is shown after save');
+      assert.dom(S.configuration.importedIssuer).hasText('None', 'Shows placeholder value for issuer');
+      assert.dom(S.configuration.importedKey).hasText('None', 'Shows placeholder value for key');
     });
   });
 
diff --git a/ui/tests/integration/components/pki/pki-import-pem-bundle-test.js b/ui/tests/integration/components/pki/pki-import-pem-bundle-test.js
index 33fef18e08..9a5f2078e8 100644
--- a/ui/tests/integration/components/pki/pki-import-pem-bundle-test.js
+++ b/ui/tests/integration/components/pki/pki-import-pem-bundle-test.js
@@ -128,7 +128,7 @@ module('Integration | Component | PkiImportPemBundle', function (hooks) {
   });
 
   test('it shows the bundle mapping on success', async function (assert) {
-    assert.expect(7);
+    assert.expect(9);
     this.server.post(`/${this.backend}/issuers/import/bundle`, () => {
       return {
         data: {
@@ -161,7 +161,7 @@ module('Integration | Component | PkiImportPemBundle', function (hooks) {
 
     assert
       .dom('[data-test-import-pair]')
-      .exists({ count: 2 }, 'Shows correct number of rows for imported items');
+      .exists({ count: 3 }, 'Shows correct number of rows for imported items');
     // Check that each row has expected values
     assert.dom('[data-test-import-pair="issuer-id_key-id"] [data-test-imported-issuer]').hasText('issuer-id');
     assert.dom('[data-test-import-pair="issuer-id_key-id"] [data-test-imported-key]').hasText('key-id');
@@ -169,9 +169,8 @@ module('Integration | Component | PkiImportPemBundle', function (hooks) {
       .dom('[data-test-import-pair="another-issuer_"] [data-test-imported-issuer]')
       .hasText('another-issuer');
     assert.dom('[data-test-import-pair="another-issuer_"] [data-test-imported-key]').hasText('None');
-    // TODO VAULT-14791
-    // assert.dom('[data-test-import-pair="_another-key"] [data-test-imported-issuer]').hasText('None');
-    // assert.dom('[data-test-import-pair="_another-key"] [data-test-imported-key]').hasText('another-key');
+    assert.dom('[data-test-import-pair="_another-key"] [data-test-imported-issuer]').hasText('None');
+    assert.dom('[data-test-import-pair="_another-key"] [data-test-imported-key]').hasText('another-key');
     await click('[data-test-done]');
   });
 
diff --git a/ui/types/vault/app-types.ts b/ui/types/vault/app-types.ts
index 236b8a5414..d1b100a124 100644
--- a/ui/types/vault/app-types.ts
+++ b/ui/types/vault/app-types.ts
@@ -53,3 +53,8 @@ export interface TtlEvent {
   timeString: string;
   goSafeTimeString: string;
 }
+
+// Generic interfaces
+export interface StringMap {
+  [key: string]: string;
+}
diff --git a/ui/types/vault/models/pki/action.d.ts b/ui/types/vault/models/pki/action.d.ts
index e1af7bab29..dba6dd53c4 100644
--- a/ui/types/vault/models/pki/action.d.ts
+++ b/ui/types/vault/models/pki/action.d.ts
@@ -4,16 +4,16 @@
  */
 
 import Model from '@ember-data/model';
-import { FormField, ModelValidations } from 'vault/app-types';
+import { FormField, ModelValidations, StringMap } from 'vault/app-types';
 import CapabilitiesModel from '../capabilities';
 
 export default class PkiActionModel extends Model {
   secretMountPath: unknown;
   actionType: string | null;
   pemBundle: string;
-  importedIssuers: unknown;
-  importedKeys: unknown;
-  mapping: unknown;
+  importedIssuers: string[];
+  importedKeys: string[];
+  mapping: StringMap;
   type: string;
   issuerName: string;
   keyName: string;