From cf2d8b266a9ae2e01754d6608180a9dd3c6b27b9 Mon Sep 17 00:00:00 2001 From: Daniel James Smith Date: Thu, 6 Apr 2023 22:41:09 +0200 Subject: [PATCH] [PM-1071] Display import-details-dialog on successful import (#4817) * Prefer callback over error-flow to prompt for password Remove error-flow to request file password Prefer callback, which has to be provided when retrieving/creating an instance. Delete ImportError Call BitwardenPasswordProtector for all Bitwarden json imports, as it extends BitwardenJsonImporter Throw errors instead of returning Return ImportResult Fix and extend tests import.service Replace "@fluffy-spoon/substitute" with "jest-mock-extended" * Fix up test cases Delete bitwarden-json-importer.spec.ts Add test case to ensure bitwarden-json-importer.ts is called given unencrypted or account-protected files * Move file-password-prompt into dialog-folder * Add import success dialog * Fix typo * Only list the type when at least one got imported * update copy based on design feedback * Remove unnecessary /index import * Remove promptForPassword_callback from interface PR feedback from @MGibson1 that giving every importer the ability to request a password is unnecessary. Instead, we can pass the callback into the constructor for every importer that needs this functionality * Remove unneeded import of BitwardenJsonImporter * Fix spec constructor * Fixed organizational import Added an else statement, or else we'd import into an org and then also import into an individual vault --- apps/cli/src/tools/import.command.ts | 41 ++--- .../import-export/org-import.component.ts | 7 +- .../file-password-prompt.component.html | 0 .../file-password-prompt.component.ts | 0 .../import-success-dialog.component.html | 32 ++++ .../dialog/import-success-dialog.component.ts | 78 ++++++++ .../app/tools/import-export/dialog/index.ts | 2 + .../import-export/import-export.module.ts | 9 +- .../tools/import-export/import.component.ts | 57 +++--- apps/web/src/locales/en/messages.json | 9 + .../spec/bitwarden-json-importer.spec.ts | 33 ---- ...warden-password-protected-importer.spec.ts | 116 ++++++------ .../bitwarden-json/account-encrypted.json.ts | 6 + .../test-data/bitwarden-json/empty.json.ts | 1 - .../bitwarden-json/password-protected.json.ts | 19 +- .../bitwarden-json/unencrypted.json.ts | 1 + .../bitwarden/bitwarden-json-importer.ts | 12 +- .../bitwarden-password-protected-importer.ts | 35 +++- libs/importer/src/index.ts | 2 +- libs/importer/src/models/import-error.ts | 5 - libs/importer/src/models/import-result.ts | 1 - .../services/import.service.abstraction.ts | 8 +- .../src/services/import.service.spec.ts | 43 ++--- libs/importer/src/services/import.service.ts | 167 +++++++++--------- 24 files changed, 397 insertions(+), 287 deletions(-) rename apps/web/src/app/tools/import-export/{ => dialog}/file-password-prompt.component.html (100%) rename apps/web/src/app/tools/import-export/{ => dialog}/file-password-prompt.component.ts (100%) create mode 100644 apps/web/src/app/tools/import-export/dialog/import-success-dialog.component.html create mode 100644 apps/web/src/app/tools/import-export/dialog/import-success-dialog.component.ts create mode 100644 apps/web/src/app/tools/import-export/dialog/index.ts delete mode 100644 libs/importer/spec/bitwarden-json-importer.spec.ts create mode 100644 libs/importer/spec/test-data/bitwarden-json/account-encrypted.json.ts delete mode 100644 libs/importer/spec/test-data/bitwarden-json/empty.json.ts create mode 100644 libs/importer/spec/test-data/bitwarden-json/unencrypted.json.ts delete mode 100644 libs/importer/src/models/import-error.ts diff --git a/apps/cli/src/tools/import.command.ts b/apps/cli/src/tools/import.command.ts index 948077ff0e..3ee4ddae98 100644 --- a/apps/cli/src/tools/import.command.ts +++ b/apps/cli/src/tools/import.command.ts @@ -2,7 +2,7 @@ import * as program from "commander"; import * as inquirer from "inquirer"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; -import { ImportServiceAbstraction, Importer, ImportType } from "@bitwarden/importer"; +import { ImportServiceAbstraction, ImportType } from "@bitwarden/importer"; import { Response } from "../models/response"; import { MessageResponse } from "../models/response/message.response"; @@ -50,8 +50,14 @@ export class ImportCommand { if (filepath == null || filepath === "") { return Response.badRequest("`filepath` was not provided."); } - - const importer = await this.importService.getImporter(format, organizationId); + const promptForPassword_callback = async () => { + return await this.promptPassword(); + }; + const importer = await this.importService.getImporter( + format, + promptForPassword_callback, + organizationId + ); if (importer === null) { return Response.badRequest("Proper importer type required."); } @@ -68,12 +74,14 @@ export class ImportCommand { return Response.badRequest("Import file was empty."); } - const response = await this.doImport(importer, contents, organizationId); + const response = await this.importService.import(importer, contents, organizationId); if (response.success) { - response.data = new MessageResponse("Imported " + filepath, null); + return Response.success(new MessageResponse("Imported " + filepath, null)); } - return response; } catch (err) { + if (err.message) { + return Response.badRequest(err.message); + } return Response.badRequest(err); } } @@ -91,27 +99,6 @@ export class ImportCommand { return Response.success(res); } - private async doImport( - importer: Importer, - contents: string, - organizationId?: string - ): Promise { - const err = await this.importService.import(importer, contents, organizationId); - if (err != null) { - if (err.passwordRequired) { - importer = this.importService.getImporter( - "bitwardenpasswordprotected", - organizationId, - await this.promptPassword() - ); - return this.doImport(importer, contents, organizationId); - } - return Response.badRequest(err.message); - } - - return Response.success(); - } - private async promptPassword() { const answer: inquirer.Answers = await inquirer.createPromptModule({ output: process.stderr, diff --git a/apps/web/src/app/admin-console/organizations/tools/import-export/org-import.component.ts b/apps/web/src/app/admin-console/organizations/tools/import-export/org-import.component.ts index 14b24eec61..5802706406 100644 --- a/apps/web/src/app/admin-console/organizations/tools/import-export/org-import.component.ts +++ b/apps/web/src/app/admin-console/organizations/tools/import-export/org-import.component.ts @@ -8,6 +8,7 @@ import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUti import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; +import { DialogService } from "@bitwarden/components"; import { ImportServiceAbstraction } from "@bitwarden/importer"; import { ImportComponent } from "../../../../tools/import-export/import.component"; @@ -30,7 +31,8 @@ export class OrganizationImportComponent extends ImportComponent { private organizationService: OrganizationService, logService: LogService, modalService: ModalService, - syncService: SyncService + syncService: SyncService, + dialogService: DialogService ) { super( i18nService, @@ -40,7 +42,8 @@ export class OrganizationImportComponent extends ImportComponent { policyService, logService, modalService, - syncService + syncService, + dialogService ); } diff --git a/apps/web/src/app/tools/import-export/file-password-prompt.component.html b/apps/web/src/app/tools/import-export/dialog/file-password-prompt.component.html similarity index 100% rename from apps/web/src/app/tools/import-export/file-password-prompt.component.html rename to apps/web/src/app/tools/import-export/dialog/file-password-prompt.component.html diff --git a/apps/web/src/app/tools/import-export/file-password-prompt.component.ts b/apps/web/src/app/tools/import-export/dialog/file-password-prompt.component.ts similarity index 100% rename from apps/web/src/app/tools/import-export/file-password-prompt.component.ts rename to apps/web/src/app/tools/import-export/dialog/file-password-prompt.component.ts diff --git a/apps/web/src/app/tools/import-export/dialog/import-success-dialog.component.html b/apps/web/src/app/tools/import-export/dialog/import-success-dialog.component.html new file mode 100644 index 0000000000..82d19e2d91 --- /dev/null +++ b/apps/web/src/app/tools/import-export/dialog/import-success-dialog.component.html @@ -0,0 +1,32 @@ + + + {{ "importSuccess" | i18n }} + + +
+ {{ "importSuccessNumberOfItems" | i18n : this.data.ciphers.length }} + + + + {{ "type" | i18n }} + {{ "total" | i18n }} + + + + + + + {{ r.type | i18n }} + + {{ r.count }} + + + +
+ +
+ +
+ diff --git a/apps/web/src/app/tools/import-export/dialog/import-success-dialog.component.ts b/apps/web/src/app/tools/import-export/dialog/import-success-dialog.component.ts new file mode 100644 index 0000000000..7a287e1ace --- /dev/null +++ b/apps/web/src/app/tools/import-export/dialog/import-success-dialog.component.ts @@ -0,0 +1,78 @@ +import { DialogRef, DIALOG_DATA } from "@angular/cdk/dialog"; +import { Component, Inject, OnInit } from "@angular/core"; + +import { CipherType } from "@bitwarden/common/vault/enums/cipher-type"; +import { TableDataSource } from "@bitwarden/components"; +import { ImportResult } from "@bitwarden/importer"; + +export interface ResultList { + icon: string; + type: string; + count: number; +} + +@Component({ + selector: "app-import-success-dialog", + templateUrl: "./import-success-dialog.component.html", +}) +export class ImportSuccessDialogComponent implements OnInit { + protected dataSource = new TableDataSource(); + + constructor(public dialogRef: DialogRef, @Inject(DIALOG_DATA) public data: ImportResult) {} + + ngOnInit(): void { + if (this.data != null) { + this.dataSource.data = this.buildResultList(); + } + } + + private buildResultList(): ResultList[] { + let logins = 0; + let cards = 0; + let identities = 0; + let secureNotes = 0; + this.data.ciphers.map((c) => { + switch (c.type) { + case CipherType.Login: + logins++; + break; + case CipherType.Card: + cards++; + break; + case CipherType.SecureNote: + secureNotes++; + break; + case CipherType.Identity: + identities++; + break; + default: + break; + } + }); + + const list: ResultList[] = []; + if (logins > 0) { + list.push({ icon: "globe", type: "typeLogin", count: logins }); + } + if (cards > 0) { + list.push({ icon: "credit-card", type: "typeCard", count: cards }); + } + if (identities > 0) { + list.push({ icon: "id-card", type: "typeIdentity", count: identities }); + } + if (secureNotes > 0) { + list.push({ icon: "sticky-note", type: "typeSecureNote", count: secureNotes }); + } + if (this.data.folders.length > 0) { + list.push({ icon: "folder", type: "folders", count: this.data.folders.length }); + } + if (this.data.collections.length > 0) { + list.push({ + icon: "collection", + type: "collections", + count: this.data.collections.length, + }); + } + return list; + } +} diff --git a/apps/web/src/app/tools/import-export/dialog/index.ts b/apps/web/src/app/tools/import-export/dialog/index.ts new file mode 100644 index 0000000000..7ad42fe1db --- /dev/null +++ b/apps/web/src/app/tools/import-export/dialog/index.ts @@ -0,0 +1,2 @@ +export * from "./import-success-dialog.component"; +export * from "./file-password-prompt.component"; diff --git a/apps/web/src/app/tools/import-export/import-export.module.ts b/apps/web/src/app/tools/import-export/import-export.module.ts index 08200baa31..770fdcf4d5 100644 --- a/apps/web/src/app/tools/import-export/import-export.module.ts +++ b/apps/web/src/app/tools/import-export/import-export.module.ts @@ -15,14 +15,19 @@ import { import { LooseComponentsModule, SharedModule } from "../../shared"; +import { ImportSuccessDialogComponent, FilePasswordPromptComponent } from "./dialog"; import { ExportComponent } from "./export.component"; -import { FilePasswordPromptComponent } from "./file-password-prompt.component"; import { ImportExportRoutingModule } from "./import-export-routing.module"; import { ImportComponent } from "./import.component"; @NgModule({ imports: [SharedModule, LooseComponentsModule, ImportExportRoutingModule], - declarations: [ImportComponent, ExportComponent, FilePasswordPromptComponent], + declarations: [ + ImportComponent, + ExportComponent, + FilePasswordPromptComponent, + ImportSuccessDialogComponent, + ], providers: [ { provide: ImportApiServiceAbstraction, diff --git a/apps/web/src/app/tools/import-export/import.component.ts b/apps/web/src/app/tools/import-export/import.component.ts index feb4c46f2d..1121015521 100644 --- a/apps/web/src/app/tools/import-export/import.component.ts +++ b/apps/web/src/app/tools/import-export/import.component.ts @@ -11,14 +11,15 @@ import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUti import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; +import { DialogService } from "@bitwarden/components"; import { ImportOption, ImportType, - ImportError, + ImportResult, ImportServiceAbstraction, } from "@bitwarden/importer"; -import { FilePasswordPromptComponent } from "./file-password-prompt.component"; +import { ImportSuccessDialogComponent, FilePasswordPromptComponent } from "./dialog"; @Component({ selector: "app-import", @@ -30,7 +31,6 @@ export class ImportComponent implements OnInit { format: ImportType = null; fileContents: string; fileSelected: File; - formPromise: Promise; loading = false; importBlockedByPolicy = false; @@ -45,7 +45,8 @@ export class ImportComponent implements OnInit { protected policyService: PolicyService, private logService: LogService, protected modalService: ModalService, - protected syncService: SyncService + protected syncService: SyncService, + protected dialogService: DialogService ) {} async ngOnInit() { @@ -68,7 +69,15 @@ export class ImportComponent implements OnInit { this.loading = true; - const importer = this.importService.getImporter(this.format, this.organizationId); + const promptForPassword_callback = async () => { + return await this.getFilePassword(); + }; + + const importer = this.importService.getImporter( + this.format, + promptForPassword_callback, + this.organizationId + ); if (importer === null) { this.platformUtilsService.showToast( "error", @@ -117,30 +126,17 @@ export class ImportComponent implements OnInit { } try { - this.formPromise = this.importService.import(importer, fileContents, this.organizationId); - let error = await this.formPromise; - - if (error?.passwordRequired) { - const filePassword = await this.getFilePassword(); - if (filePassword == null) { - this.loading = false; - return; - } - - error = await this.doPasswordProtectedImport(filePassword, fileContents); - } - - if (error != null) { - this.error(error); - this.loading = false; - return; - } + const result = await this.importService.import(importer, fileContents, this.organizationId); //No errors, display success message - this.platformUtilsService.showToast("success", null, this.i18nService.t("importSuccess")); + this.dialogService.open(ImportSuccessDialogComponent, { + data: result, + }); + this.syncService.fullSync(true); this.router.navigate(this.successNavigate); } catch (e) { + this.error(e); this.logService.error(e); } @@ -268,17 +264,4 @@ export class ImportComponent implements OnInit { return await ref.onClosedPromise(); } - - async doPasswordProtectedImport( - filePassword: string, - fileContents: string - ): Promise { - const passwordProtectedImporter = this.importService.getImporter( - "bitwardenpasswordprotected", - this.organizationId, - filePassword - ); - - return this.importService.import(passwordProtectedImporter, fileContents, this.organizationId); - } } diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 93d9bd24c4..41531e46ba 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -1256,6 +1256,15 @@ "importSuccess": { "message": "Data successfully imported" }, + "importSuccessNumberOfItems": { + "message": "A total of $AMOUNT$ items were imported.", + "placeholders": { + "amount": { + "content": "$1", + "example": "2" + } + } + }, "dataExportSuccess": { "message": "Data successfully exported" }, diff --git a/libs/importer/spec/bitwarden-json-importer.spec.ts b/libs/importer/spec/bitwarden-json-importer.spec.ts deleted file mode 100644 index 19384ed5c6..0000000000 --- a/libs/importer/spec/bitwarden-json-importer.spec.ts +++ /dev/null @@ -1,33 +0,0 @@ -// eslint-disable-next-line no-restricted-imports -import { Substitute, SubstituteOf } from "@fluffy-spoon/substitute"; - -import { CryptoService } from "@bitwarden/common/abstractions/crypto.service"; -import { I18nService } from "@bitwarden/common/abstractions/i18n.service"; - -import { BitwardenJsonImporter } from "../src/importers"; - -import { data as passwordProtectedData } from "./test-data/bitwarden-json/password-protected.json"; - -describe("bitwarden json importer", () => { - let sut: BitwardenJsonImporter; - let cryptoService: SubstituteOf; - let i18nService: SubstituteOf; - - beforeEach(() => { - cryptoService = Substitute.for(); - i18nService = Substitute.for(); - - sut = new BitwardenJsonImporter(cryptoService, i18nService); - }); - - it("should fail if password is needed", async () => { - expect((await sut.parse(passwordProtectedData)).success).toBe(false); - }); - - it("should return password needed error message", async () => { - const expected = "Password required error message"; - i18nService.t("importPasswordRequired").returns(expected); - - expect((await sut.parse(passwordProtectedData)).errorMessage).toEqual(expected); - }); -}); diff --git a/libs/importer/spec/bitwarden-password-protected-importer.spec.ts b/libs/importer/spec/bitwarden-password-protected-importer.spec.ts index 2ca41e1ee3..0b50560b93 100644 --- a/libs/importer/spec/bitwarden-password-protected-importer.spec.ts +++ b/libs/importer/spec/bitwarden-password-protected-importer.spec.ts @@ -1,77 +1,89 @@ -// eslint-disable-next-line no-restricted-imports -import { Substitute, Arg, SubstituteOf } from "@fluffy-spoon/substitute"; +import { mock, MockProxy } from "jest-mock-extended"; import { CryptoService } from "@bitwarden/common/abstractions/crypto.service"; import { I18nService } from "@bitwarden/common/abstractions/i18n.service"; import { KdfType } from "@bitwarden/common/enums"; import { Utils } from "@bitwarden/common/misc/utils"; -import { BitwardenPasswordProtectedImporter } from "../src/importers"; -import { ImportResult } from "../src/models/import-result"; +import { + BitwardenPasswordProtectedImporter, + BitwardenJsonImporter, +} from "../src/importers/bitwarden"; -import { data as emptyDecryptedData } from "./test-data/bitwarden-json/empty.json"; +import { emptyAccountEncrypted } from "./test-data/bitwarden-json/account-encrypted.json"; +import { emptyUnencryptedExport } from "./test-data/bitwarden-json/unencrypted.json"; describe("BitwardenPasswordProtectedImporter", () => { let importer: BitwardenPasswordProtectedImporter; - let cryptoService: SubstituteOf; - let i18nService: SubstituteOf; + let cryptoService: MockProxy; + let i18nService: MockProxy; const password = Utils.newGuid(); - const result = new ImportResult(); - let jDoc: { - encrypted?: boolean; - passwordProtected?: boolean; - salt?: string; - kdfIterations?: any; - kdfType?: any; - encKeyValidation_DO_NOT_EDIT?: string; - data?: string; + const promptForPassword_callback = async () => { + return password; }; beforeEach(() => { - cryptoService = Substitute.for(); - i18nService = Substitute.for(); + cryptoService = mock(); + i18nService = mock(); - jDoc = { - encrypted: true, - passwordProtected: true, - salt: "c2FsdA==", - kdfIterations: 100000, - kdfType: KdfType.PBKDF2_SHA256, - encKeyValidation_DO_NOT_EDIT: Utils.newGuid(), - data: Utils.newGuid(), - }; - - result.success = true; - importer = new BitwardenPasswordProtectedImporter(cryptoService, i18nService, password); + importer = new BitwardenPasswordProtectedImporter( + cryptoService, + i18nService, + promptForPassword_callback + ); }); - describe("Required Json Data", () => { + describe("Unencrypted", () => { + beforeAll(() => { + jest.spyOn(BitwardenJsonImporter.prototype, "parse"); + }); + + it("Should call BitwardenJsonImporter", async () => { + expect((await importer.parse(emptyUnencryptedExport)).success).toEqual(true); + expect(BitwardenJsonImporter.prototype.parse).toHaveBeenCalledWith(emptyUnencryptedExport); + }); + }); + + describe("Account encrypted", () => { + beforeAll(() => { + jest.spyOn(BitwardenJsonImporter.prototype, "parse"); + }); + + it("Should call BitwardenJsonImporter", async () => { + expect((await importer.parse(emptyAccountEncrypted)).success).toEqual(true); + expect(BitwardenJsonImporter.prototype.parse).toHaveBeenCalledWith(emptyAccountEncrypted); + }); + }); + + describe("Password protected", () => { + let jDoc: { + encrypted?: boolean; + passwordProtected?: boolean; + salt?: string; + kdfIterations?: any; + kdfType?: any; + encKeyValidation_DO_NOT_EDIT?: string; + data?: string; + }; + + beforeEach(() => { + jDoc = { + encrypted: true, + passwordProtected: true, + salt: "c2FsdA==", + kdfIterations: 100000, + kdfType: KdfType.PBKDF2_SHA256, + encKeyValidation_DO_NOT_EDIT: Utils.newGuid(), + data: Utils.newGuid(), + }; + }); + it("succeeds with default jdoc", async () => { - cryptoService.decryptToUtf8(Arg.any(), Arg.any()).resolves(emptyDecryptedData); + cryptoService.decryptToUtf8.mockReturnValue(Promise.resolve(emptyUnencryptedExport)); expect((await importer.parse(JSON.stringify(jDoc))).success).toEqual(true); }); - it("fails if encrypted === false", async () => { - jDoc.encrypted = false; - expect((await importer.parse(JSON.stringify(jDoc))).success).toEqual(false); - }); - - it("fails if encrypted === null", async () => { - jDoc.encrypted = null; - expect((await importer.parse(JSON.stringify(jDoc))).success).toEqual(false); - }); - - it("fails if passwordProtected === false", async () => { - jDoc.passwordProtected = false; - expect((await importer.parse(JSON.stringify(jDoc))).success).toEqual(false); - }); - - it("fails if passwordProtected === null", async () => { - jDoc.passwordProtected = null; - expect((await importer.parse(JSON.stringify(jDoc))).success).toEqual(false); - }); - it("fails if salt === null", async () => { jDoc.salt = null; expect((await importer.parse(JSON.stringify(jDoc))).success).toEqual(false); diff --git a/libs/importer/spec/test-data/bitwarden-json/account-encrypted.json.ts b/libs/importer/spec/test-data/bitwarden-json/account-encrypted.json.ts new file mode 100644 index 0000000000..77da21faef --- /dev/null +++ b/libs/importer/spec/test-data/bitwarden-json/account-encrypted.json.ts @@ -0,0 +1,6 @@ +export const emptyAccountEncrypted = `{ + "encrypted": true, + "encKeyValidation_DO_NOT_EDIT": "", + "folders": [], + "items": [] +}`; diff --git a/libs/importer/spec/test-data/bitwarden-json/empty.json.ts b/libs/importer/spec/test-data/bitwarden-json/empty.json.ts deleted file mode 100644 index 43ab8c763a..0000000000 --- a/libs/importer/spec/test-data/bitwarden-json/empty.json.ts +++ /dev/null @@ -1 +0,0 @@ -export const data = '{"encrypted":false,"folders":[],"items":[]}'; diff --git a/libs/importer/spec/test-data/bitwarden-json/password-protected.json.ts b/libs/importer/spec/test-data/bitwarden-json/password-protected.json.ts index 0462e83423..cbaf5d6742 100644 --- a/libs/importer/spec/test-data/bitwarden-json/password-protected.json.ts +++ b/libs/importer/spec/test-data/bitwarden-json/password-protected.json.ts @@ -1,9 +1,10 @@ -export const data = `{ - "encrypted": true, - "passwordProtected": true, - "salt": "Oy0xcgVRzxQ+9NpB5GLehw==", - "kdfIterations": 100000, - "kdfType": 0, - "encKeyValidation_DO_NOT_EDIT": "2.sZs4Jc1HW9rhABzRRYR/gQ==|8kTDaDxafulnybpWoqVX8RAybhVRTr+dffNjms271Y7amQmIE1VSMwLbk+b2vxZb|IqOo6oXQtmv/Xb/GHDi42XG9c9ILePYtP5qq584VWcg=", - "data": "2.D0AXAf7G/XIwq6EC7A0Suw==|4w+m0wHRo25y1T1Syh5wdAUyF8voqEy54waMEsbnK0Nzee959w54ru5D1NntvxZL4HFqkQLyR6jCFkn5g40f+MGJgihS/wvf4NcJJfLiiFo6MEDOQNBkxw7ZBGuHiKfVuBO5u36JgzQtZ8lyFaduGxFszuF5c+URiE9PDh9jY0//poVgHKwuLZuYFIW+f7h6T+shUWK0ya11lcHn/B/CA2xiI+YiKdNZreJrwN0yslpJ/f+MrOzagvftRjt0GNkwveCtwcYUw/zFvqvibUpKeHcRiXs8SaGoHJ5RTm69FbJ7C5tnLwoVT89Af156uvRAXV7yAC4oPcbU/3TGb6hqYosvi1QNyaqG3M9gxS6+AK0C4yWuNbMLDEr+MWiw0SWLVMKQEkCZ4oM+oTCx52otW3+2V9I8Pv3KmmhkvVvE4wBdweOJeRX53Tf5ySkmpIhCfzj6JMmxO+nmTXIhWnJChr4hPVh+ixv1GQK5thIPTCMXmAtXoTIFUx1KWjS6LjOdi2hKQueVI+XZjf0qnY2vTMxRg0ZsLBA2znQTx+DSEqumORb5T/lV73pWZiCNePSAE2msOm7tep+lm4O/VCViCfXjITAY196syhOK0XnhxJvPALchZY8sYRAfuw6hHoDiVr+JUieRoI7eUrhXBp+D6Py9TL/dS/rHe+C2Zhx+xwx2NfGt+xEp8ZAOOCxgZ0UTeSA/abm0Oz7tJIK1n26acQrgbr7rMeBymAX+5L5OWlwI1hGgEBfj6W0rrbSXf3VMfaFXZ5UsXi1VhzQmU3LyWENoDeImXFQj6zMbUSfcVwLsG5Fg8Ee/kO/wJPfG5BO51+/vFqQj6AkaMEcwg5xNrObHYfQ/DMhIn7YDM2zdzbNTdhnobGkz6YRKFPCgFe3EmIEPEpeh9S3eKE9C7MQsrR8jVSiseR/FipJLsN+W7iOwzeXdwxUFlC/0a98bTKvdrbMgNi6ZVXykHY/t2UyEGpxZGTHoZwhX01kiQrwzC4/+v/676ldxPluO9GY7MtrLveCDsiyBz15u43IGHayDEBNT0rqrOKLYmfzwCWoahRLZQrSmepe/FXqgPqRfyWc/Ro+w3sT9dXUkx3B5xxWgSyABowPV48yBUSJuefhKTpqgzkU+LzhNnWHjnxJzzQ2/|IhlRjnyhIoDM85qHX/bY2zaIU5YaRO/iFVTQDd3uFDo=" -}`; +export const data = { + encrypted: true, + passwordProtected: true, + salt: "Oy0xcgVRzxQ+9NpB5GLehw==", + kdfIterations: 100000, + kdfType: 0, + encKeyValidation_DO_NOT_EDIT: + "2.sZs4Jc1HW9rhABzRRYR/gQ==|8kTDaDxafulnybpWoqVX8RAybhVRTr+dffNjms271Y7amQmIE1VSMwLbk+b2vxZb|IqOo6oXQtmv/Xb/GHDi42XG9c9ILePYtP5qq584VWcg=", + data: "2.D0AXAf7G/XIwq6EC7A0Suw==|4w+m0wHRo25y1T1Syh5wdAUyF8voqEy54waMEsbnK0Nzee959w54ru5D1NntvxZL4HFqkQLyR6jCFkn5g40f+MGJgihS/wvf4NcJJfLiiFo6MEDOQNBkxw7ZBGuHiKfVuBO5u36JgzQtZ8lyFaduGxFszuF5c+URiE9PDh9jY0//poVgHKwuLZuYFIW+f7h6T+shUWK0ya11lcHn/B/CA2xiI+YiKdNZreJrwN0yslpJ/f+MrOzagvftRjt0GNkwveCtwcYUw/zFvqvibUpKeHcRiXs8SaGoHJ5RTm69FbJ7C5tnLwoVT89Af156uvRAXV7yAC4oPcbU/3TGb6hqYosvi1QNyaqG3M9gxS6+AK0C4yWuNbMLDEr+MWiw0SWLVMKQEkCZ4oM+oTCx52otW3+2V9I8Pv3KmmhkvVvE4wBdweOJeRX53Tf5ySkmpIhCfzj6JMmxO+nmTXIhWnJChr4hPVh+ixv1GQK5thIPTCMXmAtXoTIFUx1KWjS6LjOdi2hKQueVI+XZjf0qnY2vTMxRg0ZsLBA2znQTx+DSEqumORb5T/lV73pWZiCNePSAE2msOm7tep+lm4O/VCViCfXjITAY196syhOK0XnhxJvPALchZY8sYRAfuw6hHoDiVr+JUieRoI7eUrhXBp+D6Py9TL/dS/rHe+C2Zhx+xwx2NfGt+xEp8ZAOOCxgZ0UTeSA/abm0Oz7tJIK1n26acQrgbr7rMeBymAX+5L5OWlwI1hGgEBfj6W0rrbSXf3VMfaFXZ5UsXi1VhzQmU3LyWENoDeImXFQj6zMbUSfcVwLsG5Fg8Ee/kO/wJPfG5BO51+/vFqQj6AkaMEcwg5xNrObHYfQ/DMhIn7YDM2zdzbNTdhnobGkz6YRKFPCgFe3EmIEPEpeh9S3eKE9C7MQsrR8jVSiseR/FipJLsN+W7iOwzeXdwxUFlC/0a98bTKvdrbMgNi6ZVXykHY/t2UyEGpxZGTHoZwhX01kiQrwzC4/+v/676ldxPluO9GY7MtrLveCDsiyBz15u43IGHayDEBNT0rqrOKLYmfzwCWoahRLZQrSmepe/FXqgPqRfyWc/Ro+w3sT9dXUkx3B5xxWgSyABowPV48yBUSJuefhKTpqgzkU+LzhNnWHjnxJzzQ2/|IhlRjnyhIoDM85qHX/bY2zaIU5YaRO/iFVTQDd3uFDo=", +}; diff --git a/libs/importer/spec/test-data/bitwarden-json/unencrypted.json.ts b/libs/importer/spec/test-data/bitwarden-json/unencrypted.json.ts new file mode 100644 index 0000000000..b2acd16a69 --- /dev/null +++ b/libs/importer/spec/test-data/bitwarden-json/unencrypted.json.ts @@ -0,0 +1 @@ +export const emptyUnencryptedExport = `{ "encrypted": false, "folders": [], "items": [] }`; diff --git a/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts b/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts index 20dbb7c4f8..5958c7e5c0 100644 --- a/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts +++ b/libs/importer/src/importers/bitwarden/bitwarden-json-importer.ts @@ -13,7 +13,10 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer { private results: any; private result: ImportResult; - constructor(protected cryptoService: CryptoService, protected i18nService: I18nService) { + protected constructor( + protected cryptoService: CryptoService, + protected i18nService: I18nService + ) { super(); } @@ -21,13 +24,6 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer { this.result = new ImportResult(); this.results = JSON.parse(data); if (this.results == null || this.results.items == null) { - if (this.results?.passwordProtected) { - this.result.success = false; - this.result.missingPassword = true; - this.result.errorMessage = this.i18nService.t("importPasswordRequired"); - return this.result; - } - this.result.success = false; return this.result; } diff --git a/libs/importer/src/importers/bitwarden/bitwarden-password-protected-importer.ts b/libs/importer/src/importers/bitwarden/bitwarden-password-protected-importer.ts index 4473fbfcb3..5baa34cb72 100644 --- a/libs/importer/src/importers/bitwarden/bitwarden-password-protected-importer.ts +++ b/libs/importer/src/importers/bitwarden/bitwarden-password-protected-importer.ts @@ -13,19 +13,41 @@ import { BitwardenPasswordProtectedFileFormat } from "./bitwarden-password-prote export class BitwardenPasswordProtectedImporter extends BitwardenJsonImporter implements Importer { private key: SymmetricCryptoKey; - constructor(cryptoService: CryptoService, i18nService: I18nService, private password: string) { + constructor( + cryptoService: CryptoService, + i18nService: I18nService, + private promptForPassword_callback: () => Promise + ) { super(cryptoService, i18nService); } async parse(data: string): Promise { const result = new ImportResult(); - const parsedData = JSON.parse(data); + const parsedData: BitwardenPasswordProtectedFileFormat = JSON.parse(data); + + if (!parsedData) { + result.success = false; + return result; + } + + // File is unencrypted + if (!parsedData?.encrypted) { + return await super.parse(data); + } + + // File is account-encrypted + if (!parsedData?.passwordProtected) { + return await super.parse(data); + } + if (this.cannotParseFile(parsedData)) { result.success = false; return result; } - if (!(await this.checkPassword(parsedData))) { + // File is password-protected + const password = await this.promptForPassword_callback(); + if (!(await this.checkPassword(parsedData, password))) { result.success = false; result.errorMessage = this.i18nService.t("invalidFilePassword"); return result; @@ -36,9 +58,12 @@ export class BitwardenPasswordProtectedImporter extends BitwardenJsonImporter im return await super.parse(clearTextData); } - private async checkPassword(jdoc: BitwardenPasswordProtectedFileFormat): Promise { + private async checkPassword( + jdoc: BitwardenPasswordProtectedFileFormat, + password: string + ): Promise { this.key = await this.cryptoService.makePinKey( - this.password, + password, jdoc.salt, jdoc.kdfType, new KdfConfig(jdoc.kdfIterations, jdoc.kdfMemory, jdoc.kdfParallelism) diff --git a/libs/importer/src/index.ts b/libs/importer/src/index.ts index 685cb5e626..4586407659 100644 --- a/libs/importer/src/index.ts +++ b/libs/importer/src/index.ts @@ -1,6 +1,6 @@ export { ImportType, ImportOption } from "./models/import-options"; -export { ImportError } from "./models/import-error"; +export { ImportResult } from "./models/import-result"; export { ImportApiServiceAbstraction } from "./services/import-api.service.abstraction"; export { ImportApiService } from "./services/import-api.service"; diff --git a/libs/importer/src/models/import-error.ts b/libs/importer/src/models/import-error.ts deleted file mode 100644 index 6a74d66e9f..0000000000 --- a/libs/importer/src/models/import-error.ts +++ /dev/null @@ -1,5 +0,0 @@ -export class ImportError extends Error { - constructor(message?: string, public passwordRequired: boolean = false) { - super(message); - } -} diff --git a/libs/importer/src/models/import-result.ts b/libs/importer/src/models/import-result.ts index 6169117635..d6d6726b64 100644 --- a/libs/importer/src/models/import-result.ts +++ b/libs/importer/src/models/import-result.ts @@ -4,7 +4,6 @@ import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; export class ImportResult { success = false; - missingPassword = false; errorMessage: string; ciphers: CipherView[] = []; folders: FolderView[] = []; diff --git a/libs/importer/src/services/import.service.abstraction.ts b/libs/importer/src/services/import.service.abstraction.ts index c4b9d39356..b2f9a10cf3 100644 --- a/libs/importer/src/services/import.service.abstraction.ts +++ b/libs/importer/src/services/import.service.abstraction.ts @@ -1,6 +1,6 @@ import { Importer } from "../importers/importer"; -import { ImportError } from "../models/import-error"; import { ImportOption, ImportType } from "../models/import-options"; +import { ImportResult } from "../models/import-result"; export abstract class ImportServiceAbstraction { featuredImportOptions: readonly ImportOption[]; @@ -10,10 +10,10 @@ export abstract class ImportServiceAbstraction { importer: Importer, fileContents: string, organizationId?: string - ) => Promise; + ) => Promise; getImporter: ( format: ImportType | "bitwardenpasswordprotected", - organizationId: string, - password?: string + promptForPassword_callback: () => Promise, + organizationId: string ) => Importer; } diff --git a/libs/importer/src/services/import.service.spec.ts b/libs/importer/src/services/import.service.spec.ts index 2ba0f960ca..9dafaed14f 100644 --- a/libs/importer/src/services/import.service.spec.ts +++ b/libs/importer/src/services/import.service.spec.ts @@ -1,5 +1,4 @@ -// eslint-disable-next-line no-restricted-imports -import { Substitute, SubstituteOf } from "@fluffy-spoon/substitute"; +import { mock, MockProxy } from "jest-mock-extended"; import { CryptoService } from "@bitwarden/common/abstractions/crypto.service"; import { I18nService } from "@bitwarden/common/abstractions/i18n.service"; @@ -16,20 +15,20 @@ import { ImportService } from "./import.service"; describe("ImportService", () => { let importService: ImportService; - let cipherService: SubstituteOf; - let folderService: SubstituteOf; - let importApiService: SubstituteOf; - let i18nService: SubstituteOf; - let collectionService: SubstituteOf; - let cryptoService: SubstituteOf; + let cipherService: MockProxy; + let folderService: MockProxy; + let importApiService: MockProxy; + let i18nService: MockProxy; + let collectionService: MockProxy; + let cryptoService: MockProxy; beforeEach(() => { - cipherService = Substitute.for(); - folderService = Substitute.for(); - importApiService = Substitute.for(); - i18nService = Substitute.for(); - collectionService = Substitute.for(); - cryptoService = Substitute.for(); + cipherService = mock(); + folderService = mock(); + importApiService = mock(); + i18nService = mock(); + collectionService = mock(); + cryptoService = mock(); importService = new ImportService( cipherService, @@ -46,12 +45,15 @@ describe("ImportService", () => { let importer: Importer; const organizationId = Utils.newGuid(); const password = Utils.newGuid(); + const promptForPassword_callback = async () => { + return password; + }; beforeEach(() => { importer = importService.getImporter( "bitwardenpasswordprotected", - organizationId, - password + promptForPassword_callback, + organizationId ); }); @@ -59,12 +61,13 @@ describe("ImportService", () => { expect(importer).toBeInstanceOf(BitwardenPasswordProtectedImporter); }); - it("has the appropriate organization Id", () => { - expect(importer.organizationId).toEqual(organizationId); + it("has the promptForPassword_callback set", async () => { + expect(importer.promptForPassword_callback).not.toBeNull(); + expect(await importer.promptForPassword_callback()).toEqual(password); }); - it("has the appropriate password", () => { - expect(Object.entries(importer)).toEqual(expect.arrayContaining([["password", password]])); + it("has the appropriate organization Id", () => { + expect(importer.organizationId).toEqual(organizationId); }); }); }); diff --git a/libs/importer/src/services/import.service.ts b/libs/importer/src/services/import.service.ts index 9175bd91d1..94d3b50362 100644 --- a/libs/importer/src/services/import.service.ts +++ b/libs/importer/src/services/import.service.ts @@ -20,7 +20,6 @@ import { AvastJsonImporter, AviraCsvImporter, BitwardenCsvImporter, - BitwardenJsonImporter, BitwardenPasswordProtectedImporter, BlackBerryCsvImporter, BlurCsvImporter, @@ -76,7 +75,6 @@ import { ZohoVaultCsvImporter, } from "../importers"; import { Importer } from "../importers/importer"; -import { ImportError } from "../models/import-error"; import { featuredImportOptions, ImportOption, @@ -109,48 +107,55 @@ export class ImportService implements ImportServiceAbstraction { importer: Importer, fileContents: string, organizationId: string = null - ): Promise { + ): Promise { const importResult = await importer.parse(fileContents); - if (importResult.success) { - if (importResult.folders.length === 0 && importResult.ciphers.length === 0) { - return new ImportError(this.i18nService.t("importNothingError")); - } else if (importResult.ciphers.length > 0) { - const halfway = Math.floor(importResult.ciphers.length / 2); - const last = importResult.ciphers.length - 1; - - if ( - this.badData(importResult.ciphers[0]) && - this.badData(importResult.ciphers[halfway]) && - this.badData(importResult.ciphers[last]) - ) { - return new ImportError(this.i18nService.t("importFormatError")); - } - } - try { - await this.postImport(importResult, organizationId); - } catch (error) { - const errorResponse = new ErrorResponse(error, 400); - return this.handleServerError(errorResponse, importResult); - } - return null; - } else { + if (!importResult.success) { if (!Utils.isNullOrWhitespace(importResult.errorMessage)) { - return new ImportError(importResult.errorMessage, importResult.missingPassword); - } else { - return new ImportError( - this.i18nService.t("importFormatError"), - importResult.missingPassword - ); + throw new Error(importResult.errorMessage); + } + throw new Error(this.i18nService.t("importFormatError")); + } + + if (importResult.folders.length === 0 && importResult.ciphers.length === 0) { + throw new Error(this.i18nService.t("importNothingError")); + } + + if (importResult.ciphers.length > 0) { + const halfway = Math.floor(importResult.ciphers.length / 2); + const last = importResult.ciphers.length - 1; + + if ( + this.badData(importResult.ciphers[0]) && + this.badData(importResult.ciphers[halfway]) && + this.badData(importResult.ciphers[last]) + ) { + throw new Error(this.i18nService.t("importFormatError")); } } + + try { + if (organizationId != null) { + await this.handleOrganizationalImport(importResult, organizationId); + } else { + await this.handleIndividualImport(importResult); + } + } catch (error) { + const errorResponse = new ErrorResponse(error, 400); + throw this.handleServerError(errorResponse, importResult); + } + return importResult; } getImporter( format: ImportType | "bitwardenpasswordprotected", - organizationId: string = null, - password: string = null + promptForPassword_callback: () => Promise, + organizationId: string = null ): Importer { - const importer = this.getImporterInstance(format, password); + if (promptForPassword_callback == null) { + return null; + } + + const importer = this.getImporterInstance(format, promptForPassword_callback); if (importer == null) { return null; } @@ -158,7 +163,10 @@ export class ImportService implements ImportServiceAbstraction { return importer; } - private getImporterInstance(format: ImportType | "bitwardenpasswordprotected", password: string) { + private getImporterInstance( + format: ImportType | "bitwardenpasswordprotected", + promptForPassword_callback: () => Promise + ) { if (format == null) { return null; } @@ -167,12 +175,11 @@ export class ImportService implements ImportServiceAbstraction { case "bitwardencsv": return new BitwardenCsvImporter(); case "bitwardenjson": - return new BitwardenJsonImporter(this.cryptoService, this.i18nService); case "bitwardenpasswordprotected": return new BitwardenPasswordProtectedImporter( this.cryptoService, this.i18nService, - password + promptForPassword_callback ); case "lastpasscsv": case "passboltcsv": @@ -294,46 +301,46 @@ export class ImportService implements ImportServiceAbstraction { } } - private async postImport(importResult: ImportResult, organizationId: string = null) { - if (organizationId == null) { - const request = new ImportCiphersRequest(); - for (let i = 0; i < importResult.ciphers.length; i++) { - const c = await this.cipherService.encrypt(importResult.ciphers[i]); - request.ciphers.push(new CipherRequest(c)); - } - if (importResult.folders != null) { - for (let i = 0; i < importResult.folders.length; i++) { - const f = await this.folderService.encrypt(importResult.folders[i]); - request.folders.push(new FolderWithIdRequest(f)); - } - } - if (importResult.folderRelationships != null) { - importResult.folderRelationships.forEach((r) => - request.folderRelationships.push(new KvpRequest(r[0], r[1])) - ); - } - return await this.importApiService.postImportCiphers(request); - } else { - const request = new ImportOrganizationCiphersRequest(); - for (let i = 0; i < importResult.ciphers.length; i++) { - importResult.ciphers[i].organizationId = organizationId; - const c = await this.cipherService.encrypt(importResult.ciphers[i]); - request.ciphers.push(new CipherRequest(c)); - } - if (importResult.collections != null) { - for (let i = 0; i < importResult.collections.length; i++) { - importResult.collections[i].organizationId = organizationId; - const c = await this.collectionService.encrypt(importResult.collections[i]); - request.collections.push(new CollectionWithIdRequest(c)); - } - } - if (importResult.collectionRelationships != null) { - importResult.collectionRelationships.forEach((r) => - request.collectionRelationships.push(new KvpRequest(r[0], r[1])) - ); - } - return await this.importApiService.postImportOrganizationCiphers(organizationId, request); + private async handleIndividualImport(importResult: ImportResult) { + const request = new ImportCiphersRequest(); + for (let i = 0; i < importResult.ciphers.length; i++) { + const c = await this.cipherService.encrypt(importResult.ciphers[i]); + request.ciphers.push(new CipherRequest(c)); } + if (importResult.folders != null) { + for (let i = 0; i < importResult.folders.length; i++) { + const f = await this.folderService.encrypt(importResult.folders[i]); + request.folders.push(new FolderWithIdRequest(f)); + } + } + if (importResult.folderRelationships != null) { + importResult.folderRelationships.forEach((r) => + request.folderRelationships.push(new KvpRequest(r[0], r[1])) + ); + } + return await this.importApiService.postImportCiphers(request); + } + + private async handleOrganizationalImport(importResult: ImportResult, organizationId: string) { + const request = new ImportOrganizationCiphersRequest(); + for (let i = 0; i < importResult.ciphers.length; i++) { + importResult.ciphers[i].organizationId = organizationId; + const c = await this.cipherService.encrypt(importResult.ciphers[i]); + request.ciphers.push(new CipherRequest(c)); + } + if (importResult.collections != null) { + for (let i = 0; i < importResult.collections.length; i++) { + importResult.collections[i].organizationId = organizationId; + const c = await this.collectionService.encrypt(importResult.collections[i]); + request.collections.push(new CollectionWithIdRequest(c)); + } + } + if (importResult.collectionRelationships != null) { + importResult.collectionRelationships.forEach((r) => + request.collectionRelationships.push(new KvpRequest(r[0], r[1])) + ); + } + return await this.importApiService.postImportOrganizationCiphers(organizationId, request); } private badData(c: CipherView) { @@ -345,9 +352,9 @@ export class ImportService implements ImportServiceAbstraction { ); } - private handleServerError(errorResponse: ErrorResponse, importResult: ImportResult): ImportError { + private handleServerError(errorResponse: ErrorResponse, importResult: ImportResult): Error { if (errorResponse.validationErrors == null) { - return new ImportError(errorResponse.message); + return new Error(errorResponse.message); } let errorMessage = ""; @@ -385,6 +392,6 @@ export class ImportService implements ImportServiceAbstraction { errorMessage += "[" + itemType + '] "' + item.name + '": ' + value; }); - return new ImportError(errorMessage); + return new Error(errorMessage); } }