Increase error checking on imported Login items (#369)

* Increase error checking on imported Login items

* Check encKey when importing encrypted JSON

* Fix style, use GUID as random string for test

* Revert "Increase error checking on imported Login items"

This reverts commit 17294527863cc53b84ed218f94ffbc21f4e96260.

* fix linting

* Fix tests

spec/common/services/export.service.spec.ts

@@ -2,6 +2,7 @@ import { Substitute, SubstituteOf } from '@fluffy-spoon/substitute';
 
 import { ApiService } from '../../../src/abstractions/api.service';
 import { CipherService } from '../../../src/abstractions/cipher.service';
+import { CryptoService } from '../../../src/abstractions/crypto.service';
 import { FolderService } from '../../../src/abstractions/folder.service';
 
 import { ExportService } from '../../../src/services/export.service';
@@ -74,16 +75,18 @@ describe('ExportService', () => {
     let apiService: SubstituteOf<ApiService>;
     let cipherService: SubstituteOf<CipherService>;
     let folderService: SubstituteOf<FolderService>;
+    let cryptoService: SubstituteOf<CryptoService>;
 
     beforeEach(() => {
         apiService = Substitute.for<ApiService>();
         cipherService = Substitute.for<CipherService>();
         folderService = Substitute.for<FolderService>();
+        cryptoService = Substitute.for<CryptoService>();
 
         folderService.getAllDecrypted().resolves([]);
         folderService.getAll().resolves([]);
 
-        exportService = new ExportService(folderService, cipherService, apiService);
+        exportService = new ExportService(folderService, cipherService, apiService, cryptoService);
     });
 
     it('exports unecrypted user ciphers', async () => {

src/importers/bitwardenJsonImporter.ts

@@ -1,16 +1,24 @@
 import { BaseImporter } from './baseImporter';
 import { Importer } from './importer';
 
+import { EncString } from '../models/domain/encString';
 import { ImportResult } from '../models/domain/importResult';
 
 import { CipherWithIds } from '../models/export/cipherWithIds';
 import { CollectionWithId } from '../models/export/collectionWithId';
 import { FolderWithId } from '../models/export/folderWithId';
 
+import { CryptoService } from '../abstractions/crypto.service';
+import { I18nService } from '../abstractions/i18n.service';
+
 export class BitwardenJsonImporter extends BaseImporter implements Importer {
     private results: any;
     private result: ImportResult;
 
+    constructor(private cryptoService: CryptoService, private i18nService: I18nService) {
+        super();
+    }
+
     async parse(data: string): Promise<ImportResult> {
         this.result = new ImportResult();
         this.results = JSON.parse(data);
@@ -25,11 +33,20 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
             this.parseDecrypted();
         }
 
-        this.result.success = true;
         return this.result;
     }
 
     private async parseEncrypted() {
+        if (this.results.encKeyValidation_DO_NOT_EDIT != null) {
+            const encKeyValidation = new EncString(this.results.encKeyValidation_DO_NOT_EDIT);
+            const encKeyValidationDecrypt = await this.cryptoService.decryptToUtf8(encKeyValidation);
+            if (encKeyValidationDecrypt === null) {
+                this.result.success = false;
+                this.result.errorMessage = this.i18nService.t('importEncKeyError');
+                return;
+            }
+        }
+
         const groupingsMap = new Map<string, number>();
 
         if (this.organization && this.results.collections != null) {
@@ -82,6 +99,8 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
             this.cleanupCipher(view);
             this.result.ciphers.push(view);
         }
+
+        this.result.success = true;
     }
 
     private parseDecrypted() {
@@ -133,5 +152,7 @@ export class BitwardenJsonImporter extends BaseImporter implements Importer {
             this.cleanupCipher(cipher);
             this.result.ciphers.push(cipher);
         });
+
+        this.result.success = true;
     }
 }

src/services/export.service.ts

@@ -4,6 +4,7 @@ import { CipherType } from '../enums/cipherType';
 
 import { ApiService } from '../abstractions/api.service';
 import { CipherService } from '../abstractions/cipher.service';
+import { CryptoService } from '../abstractions/crypto.service';
 import { ExportService as ExportServiceAbstraction } from '../abstractions/export.service';
 import { FolderService } from '../abstractions/folder.service';
 
@@ -23,9 +24,11 @@ import { CipherWithIds as CipherExport } from '../models/export/cipherWithIds';
 import { CollectionWithId as CollectionExport } from '../models/export/collectionWithId';
 import { FolderWithId as FolderExport } from '../models/export/folderWithId';
 
+import { Utils } from '../misc/utils';
+
 export class ExportService implements ExportServiceAbstraction {
     constructor(private folderService: FolderService, private cipherService: CipherService,
-        private apiService: ApiService) { }
+        private apiService: ApiService, private cryptoService: CryptoService) { }
 
     async getExport(format: 'csv' | 'json' | 'encrypted_json' = 'csv'): Promise<string> {
         if (format === 'encrypted_json') {
@@ -141,8 +144,11 @@ export class ExportService implements ExportServiceAbstraction {
 
         await Promise.all(promises);
 
+        const encKeyValidation = await this.cryptoService.encrypt(Utils.newGuid());
+
         const jsonDoc: any = {
             encrypted: true,
+            encKeyValidation_DO_NOT_EDIT: encKeyValidation.encryptedString,
             folders: [],
             items: [],
         };

src/services/import.service.ts

@@ -1,6 +1,7 @@
 import { ApiService } from '../abstractions/api.service';
 import { CipherService } from '../abstractions/cipher.service';
 import { CollectionService } from '../abstractions/collection.service';
+import { CryptoService } from '../abstractions/crypto.service';
 import { FolderService } from '../abstractions/folder.service';
 import { I18nService } from '../abstractions/i18n.service';
 import {
@@ -143,7 +144,8 @@ export class ImportService implements ImportServiceAbstraction {
 
     constructor(private cipherService: CipherService, private folderService: FolderService,
         private apiService: ApiService, private i18nService: I18nService,
-        private collectionService: CollectionService, private platformUtilsService: PlatformUtilsService) { }
+        private collectionService: CollectionService, private platformUtilsService: PlatformUtilsService,
+        private cryptoService: CryptoService) { }
 
     getImportOptions(): ImportOption[] {
         return this.featuredImportOptions.concat(this.regularImportOptions);
@@ -171,10 +173,14 @@ export class ImportService implements ImportServiceAbstraction {
                 return this.handleServerError(errorResponse, importResult);
             }
             return null;
+        } else {
+            if (!Utils.isNullOrWhitespace(importResult.errorMessage)) {
+                return new Error(importResult.errorMessage);
             } else {
                 return new Error(this.i18nService.t('importFormatError'));
             }
         }
+    }
 
     getImporter(format: string, organizationId: string = null): Importer {
         const importer = this.getImporterInstance(format);
@@ -194,7 +200,7 @@ export class ImportService implements ImportServiceAbstraction {
             case 'bitwardencsv':
                 return new BitwardenCsvImporter();
             case 'bitwardenjson':
-                return new BitwardenJsonImporter();
+                return new BitwardenJsonImporter(this.cryptoService, this.i18nService);
             case 'lastpasscsv':
             case 'passboltcsv':
                 return new LastPassCsvImporter();