[PM-27083] Prevent collection nesting on import into a MyItems-collection (#16937)

* Prevent collection nesting on import into a my items collection

My Items collections do not support nested collections. The import source hierarchy needs to be flattened into the My Items collection

* Introduce new types for folder and collection relationship
Makes it easier to identify which position is for the cipherIndex and which is for the folder-/collection-index

* Fix assignment of ciphers to My items collection

* Remove unneeded type cast or assertions

* Add clarifying comment

---------

Co-authored-by: Daniel James Smith <djsmith85@users.noreply.github.com>

libs/importer/src/models/import-result.ts

@@ -6,12 +6,15 @@ import { CollectionView } from "@bitwarden/admin-console/common";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
 
+export type FolderRelationship = [cipherIndex: number, folderIndex: number];
+export type CollectionRelationship = [cipherIndex: number, collectionIndex: number];
+
 export class ImportResult {
   success = false;
   errorMessage: string;
   ciphers: CipherView[] = [];
   folders: FolderView[] = [];
-  folderRelationships: [number, number][] = [];
+  folderRelationships: FolderRelationship[] = [];
   collections: CollectionView[] = [];
-  collectionRelationships: [number, number][] = [];
+  collectionRelationships: CollectionRelationship[] = [];
 }

libs/importer/src/services/import.service.spec.ts

@@ -2,7 +2,11 @@ import { mock, MockProxy } from "jest-mock-extended";
 
 // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
 // eslint-disable-next-line no-restricted-imports
-import { CollectionService, CollectionView } from "@bitwarden/admin-console/common";
+import {
+  CollectionService,
+  CollectionTypes,
+  CollectionView,
+} from "@bitwarden/admin-console/common";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
 import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction";
@@ -194,7 +198,7 @@ describe("ImportService", () => {
       );
     });
 
-    it("passing importTarget as null on setImportTarget with organizationId throws error", async () => {
+    it("passing importTarget as undefined on setImportTarget with organizationId throws error", async () => {
       const setImportTargetMethod = importService["setImportTarget"](
         null,
         organizationId,
@@ -204,10 +208,10 @@ describe("ImportService", () => {
       await expect(setImportTargetMethod).rejects.toThrow();
     });
 
-    it("passing importTarget as null on setImportTarget throws error", async () => {
+    it("passing importTarget as undefined on setImportTarget throws error", async () => {
       const setImportTargetMethod = importService["setImportTarget"](
         null,
-        "",
+        undefined,
         new Object() as CollectionView,
       );
 
@@ -239,11 +243,40 @@ describe("ImportService", () => {
       importResult.ciphers.push(createCipher({ name: "cipher2" }));
       importResult.folderRelationships.push([0, 0]);
 
-      await importService["setImportTarget"](importResult, "", mockImportTargetFolder);
+      await importService["setImportTarget"](importResult, undefined, mockImportTargetFolder);
       expect(importResult.folderRelationships.length).toEqual(2);
       expect(importResult.folderRelationships[0]).toEqual([1, 0]);
       expect(importResult.folderRelationships[1]).toEqual([0, 1]);
     });
+
+    it("If importTarget is of type DefaultUserCollection sets it as new root for all ciphers as nesting is not supported", async () => {
+      importResult.collections.push(mockCollection1);
+      importResult.collections.push(mockCollection2);
+      importResult.ciphers.push(createCipher({ name: "cipher1" }));
+      importResult.ciphers.push(createCipher({ name: "cipher2" }));
+      importResult.ciphers.push(createCipher({ name: "cipher3" }));
+
+      importResult.collectionRelationships.push([0, 0]);
+      importResult.collectionRelationships.push([1, 1]);
+      importResult.collectionRelationships.push([2, 0]);
+
+      mockImportTargetCollection.type = CollectionTypes.DefaultUserCollection;
+      await importService["setImportTarget"](
+        importResult,
+        organizationId,
+        mockImportTargetCollection,
+      );
+      expect(importResult.collections.length).toBe(1);
+      expect(importResult.collections[0]).toBe(mockImportTargetCollection);
+
+      expect(importResult.collectionRelationships.length).toEqual(3);
+      expect(importResult.collectionRelationships[0]).toEqual([0, 0]);
+      expect(importResult.collectionRelationships[1]).toEqual([1, 0]);
+      expect(importResult.collectionRelationships[2]).toEqual([2, 0]);
+
+      expect(importResult.collectionRelationships.map((r) => r[0])).toEqual([0, 1, 2]);
+      expect(importResult.collectionRelationships.every((r) => r[1] === 0)).toBe(true);
+    });
   });
 });
 

libs/importer/src/services/import.service.ts

@@ -8,6 +8,7 @@ import {
   CollectionService,
   CollectionWithIdRequest,
   CollectionView,
+  CollectionTypes,
 } from "@bitwarden/admin-console/common";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { getUserId } from "@bitwarden/common/auth/services/account.service";
@@ -101,7 +102,7 @@ import {
   ImportType,
   regularImportOptions,
 } from "../models/import-options";
-import { ImportResult } from "../models/import-result";
+import { CollectionRelationship, FolderRelationship, ImportResult } from "../models/import-result";
 import { ImportApiServiceAbstraction } from "../services/import-api.service.abstraction";
 import { ImportServiceAbstraction } from "../services/import.service.abstraction";
 
@@ -473,19 +474,20 @@ export class ImportService implements ImportServiceAbstraction {
 
   private async setImportTarget(
     importResult: ImportResult,
-    organizationId: string,
+    organizationId: OrganizationId | undefined,
     importTarget: FolderView | CollectionView,
   ) {
     if (!importTarget) {
       return;
     }
 
+    // Importing into an organization
     if (organizationId) {
       if (!(importTarget instanceof CollectionView)) {
         throw new Error(this.i18nService.t("errorAssigningTargetCollection"));
       }
 
-      const noCollectionRelationShips: [number, number][] = [];
+      const noCollectionRelationShips: CollectionRelationship[] = [];
       importResult.ciphers.forEach((c, index) => {
         if (
           !Array.isArray(importResult.collectionRelationships) ||
@@ -495,15 +497,28 @@ export class ImportService implements ImportServiceAbstraction {
         }
       });
 
-      const collections: CollectionView[] = [...importResult.collections];
+      // My Items collections do not support collection nesting.
-      importResult.collections = [importTarget as CollectionView];
+      // Flatten all ciphers from nested collections into the import target.
+      if (importTarget.type === CollectionTypes.DefaultUserCollection) {
+        importResult.collections = [importTarget];
+
+        const flattenRelationships: CollectionRelationship[] = [];
+        importResult.ciphers.forEach((c, index) => {
+          flattenRelationships.push([index, 0]);
+        });
+        importResult.collectionRelationships = flattenRelationships;
+        return;
+      }
+
+      const collections = [...importResult.collections];
+      importResult.collections = [importTarget];
       collections.map((x) => {
         const f = new CollectionView(x);
         f.name = `${importTarget.name}/${x.name}`;
         importResult.collections.push(f);
       });
 
-      const relationships: [number, number][] = [...importResult.collectionRelationships];
+      const relationships = [...importResult.collectionRelationships];
       importResult.collectionRelationships = [...noCollectionRelationShips];
       relationships.map((x) => {
         importResult.collectionRelationships.push([x[0], x[1] + 1]);
@@ -512,11 +527,12 @@ export class ImportService implements ImportServiceAbstraction {
       return;
     }
 
+    // Importing into personal vault
     if (!(importTarget instanceof FolderView)) {
       throw new Error(this.i18nService.t("errorAssigningTargetFolder"));
     }
 
-    const noFolderRelationShips: [number, number][] = [];
+    const noFolderRelationShips: FolderRelationship[] = [];
     importResult.ciphers.forEach((c, index) => {
       if (Utils.isNullOrEmpty(c.folderId)) {
         c.folderId = importTarget.id;
@@ -524,8 +540,8 @@ export class ImportService implements ImportServiceAbstraction {
       }
     });
 
-    const folders: FolderView[] = [...importResult.folders];
+    const folders = [...importResult.folders];
-    importResult.folders = [importTarget as FolderView];
+    importResult.folders = [importTarget];
     folders.map((x) => {
       const newFolderName = `${importTarget.name}/${x.name}`;
       const f = new FolderView();
@@ -533,7 +549,7 @@ export class ImportService implements ImportServiceAbstraction {
       importResult.folders.push(f);
     });
 
-    const relationships: [number, number][] = [...importResult.folderRelationships];
+    const relationships = [...importResult.folderRelationships];
     importResult.folderRelationships = [...noFolderRelationShips];
     relationships.map((x) => {
       importResult.folderRelationships.push([x[0], x[1] + 1]);