Skip to content

Commit 84a4bdb

Browse files
authored
[PM-32743] Convert collections to folders when importing into My Items (#20453)
* [PM-32743] Convert collections to folders when importing into My Items * Fix multi collection detection bug * Add tests
1 parent 83b7f64 commit 84a4bdb

6 files changed

Lines changed: 101 additions & 19 deletions

File tree

apps/browser/src/_locales/en/messages.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6641,5 +6641,8 @@
66416641
},
66426642
"hideEmailPolicyInEffect": {
66436643
"message": "Your organization requires email visibility. Uncheck \"Hide your email from viewers\" to save."
6644+
},
6645+
"errorImportingMyItemsMultiCollection": {
6646+
"message": "You cannot import items into My Items that are part of multiple collections"
66446647
}
66456648
}

apps/desktop/src/locales/en/messages.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5233,6 +5233,9 @@
52335233
"message": "Progress bar",
52345234
"description": "This is the name of a page component that displays progress to the user. This string will be used to label the progress bar for a screenreader when multiple progress bars are on a page, like 'Progress bar 1' and 'Progress bar 2'."
52355235
},
5236+
"errorImportingMyItemsMultiCollection": {
5237+
"message": "You cannot import items into My Items that are part of multiple collections"
5238+
},
52365239
"driversLicenseDetails": {
52375240
"message": "License details"
52385241
},

apps/web/src/locales/en/messages.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13919,6 +13919,9 @@
1391913919
"message": "Progress bar",
1392013920
"description": "This is the name of a page component that displays progress to the user. This string will be used to label the progress bar for a screenreader when multiple progress bars are on a page, like 'Progress bar 1' and 'Progress bar 2'."
1392113921
},
13922+
"errorImportingMyItemsMultiCollection": {
13923+
"message": "You cannot import items into My Items that are part of multiple collections"
13924+
},
1392213925
"searchDriversLicense": {
1392313926
"message": "Search licenses"
1392413927
},

libs/common/src/models/request/import-organization-ciphers.request.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
22
// eslint-disable-next-line no-restricted-imports
33
import { CollectionWithIdRequest } from "@bitwarden/admin-console/common";
4+
import { FolderWithOptionalIdRequest } from "@bitwarden/common/vault/models/request/folder-with-optional-id.request";
45

56
import { CipherRequest } from "../../vault/models/request/cipher.request";
67

@@ -10,4 +11,6 @@ export class ImportOrganizationCiphersRequest {
1011
ciphers: CipherRequest[] = [];
1112
collections: CollectionWithIdRequest[] = [];
1213
collectionRelationships: KvpRequest<number, number>[] = [];
14+
folders: FolderWithOptionalIdRequest[] = [];
15+
folderRelationships: KvpRequest<number, number>[] = [];
1316
}

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,47 @@ describe("ImportService", () => {
281281
expect(importResult.collectionRelationships.map((r) => r[0])).toEqual([0, 1, 2]);
282282
expect(importResult.collectionRelationships.every((r) => r[1] === 0)).toBe(true);
283283
});
284+
285+
it("If importTarget is of type DefaultUserCollection throw an error if trying to import cipher with multiple collections", async () => {
286+
importResult.collections.push(mockCollection1);
287+
importResult.collections.push(mockCollection2);
288+
importResult.ciphers.push(createCipher({ name: "cipher1" }));
289+
importResult.collectionRelationships.push([0, 0], [0, 1]);
290+
291+
mockImportTargetCollection.type = CollectionTypes.DefaultUserCollection;
292+
const setImportTargetMethod = importService["setImportTarget"](
293+
importResult,
294+
organizationId,
295+
mockImportTargetCollection,
296+
);
297+
await expect(setImportTargetMethod).rejects.toThrow();
298+
});
299+
300+
it("If importTarget is of type DefaultUserCollection create any collections as folders", async () => {
301+
importResult.collections.push(mockCollection1);
302+
importResult.collections.push(mockCollection2);
303+
importResult.ciphers.push(
304+
createCipher({ name: "cipher1", collectionIds: [mockCollection1.id] }),
305+
);
306+
importResult.ciphers.push(
307+
createCipher({ name: "cipher2", collectionIds: [mockCollection2.id] }),
308+
);
309+
310+
mockImportTargetCollection.type = CollectionTypes.DefaultUserCollection;
311+
await importService["setImportTarget"](
312+
importResult,
313+
organizationId,
314+
mockImportTargetCollection,
315+
);
316+
317+
expect(importResult.collections.length).toEqual(1);
318+
expect(importResult.collectionRelationships.length).toEqual(2);
319+
expect(importResult.collectionRelationships[0]).toEqual([0, 0]);
320+
expect(importResult.collectionRelationships[1]).toEqual([1, 0]);
321+
expect(importResult.folders.length).toEqual(2);
322+
expect(importResult.folders[0].name).toEqual(mockCollection1.name);
323+
expect(importResult.folders[1].name).toEqual(mockCollection2.name);
324+
});
284325
});
285326

286327
describe("handleIndividualImport", () => {

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

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { ErrorResponse } from "@bitwarden/common/models/response/error.response"
2020
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
2121
import { Utils } from "@bitwarden/common/platform/misc/utils";
2222
import { OrganizationId, UserId } from "@bitwarden/common/types/guid";
23+
import { UserKey } from "@bitwarden/common/types/key";
2324
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
2425
import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
2526
import { CipherType, toCipherTypeName } from "@bitwarden/common/vault/enums";
@@ -385,17 +386,8 @@ export class ImportService implements ImportServiceAbstraction {
385386

386387
const userKey = await firstValueFrom(this.keyService.userKey$(userId));
387388

388-
if (importResult.folders != null) {
389-
for (let i = 0; i < importResult.folders.length; i++) {
390-
const f = await this.folderService.encrypt(importResult.folders[i], userKey);
391-
request.folders.push(new FolderWithOptionalIdRequest(f));
392-
}
393-
}
394-
if (importResult.folderRelationships != null) {
395-
importResult.folderRelationships.forEach((r) =>
396-
request.folderRelationships.push(new KvpRequest(r[0], r[1])),
397-
);
398-
}
389+
await this.addFolders(request, importResult, userKey);
390+
399391
return await this.importApiService.postImportCiphers(request);
400392
}
401393

@@ -417,6 +409,10 @@ export class ImportService implements ImportServiceAbstraction {
417409
request.ciphers.push(new CipherRequest(encryptedCipher));
418410
}
419411

412+
const userKey = await firstValueFrom(this.keyService.userKey$(userId));
413+
414+
await this.addFolders(request, importResult, userKey);
415+
420416
if (importResult.collections != null) {
421417
for (let i = 0; i < importResult.collections.length; i++) {
422418
importResult.collections[i].organizationId = organizationId;
@@ -432,6 +428,24 @@ export class ImportService implements ImportServiceAbstraction {
432428
return await this.importApiService.postImportOrganizationCiphers(organizationId, request);
433429
}
434430

431+
private async addFolders(
432+
request: ImportCiphersRequest | ImportOrganizationCiphersRequest,
433+
importResult: ImportResult,
434+
userKey: UserKey,
435+
) {
436+
if (importResult.folders != null) {
437+
for (let i = 0; i < importResult.folders.length; i++) {
438+
const f = await this.folderService.encrypt(importResult.folders[i], userKey);
439+
request.folders.push(new FolderWithOptionalIdRequest(f));
440+
}
441+
}
442+
if (importResult.folderRelationships != null) {
443+
importResult.folderRelationships.forEach((r) =>
444+
request.folderRelationships.push(new KvpRequest(r[0], r[1])),
445+
);
446+
}
447+
}
448+
435449
private badData(c: CipherView) {
436450
return (
437451
(c.name == null || c.name === "--") &&
@@ -509,16 +523,31 @@ export class ImportService implements ImportServiceAbstraction {
509523
}
510524
});
511525

512-
// My Items collections do not support collection nesting.
513-
// Flatten all ciphers from nested collections into the import target.
514526
if (importTarget.type === CollectionTypes.DefaultUserCollection) {
515-
importResult.collections = [importTarget];
516-
517-
const flattenRelationships: CollectionRelationship[] = [];
518-
importResult.ciphers.forEach((c, index) => {
519-
flattenRelationships.push([index, 0]);
527+
// Since ciphers can only have one folder (for now)
528+
// we bail if any are a part of multiple Collections
529+
if (
530+
importResult.ciphers.some(
531+
(_c, c_idx) =>
532+
importResult.collectionRelationships.filter((cr) => cr[0] === c_idx).length > 1,
533+
)
534+
) {
535+
throw new Error(this.i18nService.t("errorImportingMyItemsMultiCollection"));
536+
}
537+
importResult.folders = importResult.collections.map((c) => {
538+
const f = new FolderView();
539+
f.name = c.name;
540+
return f;
520541
});
521-
importResult.collectionRelationships = flattenRelationships;
542+
// We use the collection relationships to create the folder relationships
543+
importResult.folderRelationships = importResult.collectionRelationships.map((c) => [
544+
c[0],
545+
c[1],
546+
]);
547+
// We then set the target collection to My Items...
548+
importResult.collections = [importTarget];
549+
// ...and set the collection relationships accordingly
550+
importResult.collectionRelationships = importResult.ciphers.map((_c, idx) => [idx, 0]);
522551
return;
523552
}
524553

0 commit comments

Comments
 (0)