From 61b375973622da17f619e42738b96e263aac3722 Mon Sep 17 00:00:00 2001
From: aj-rosado <109146700+aj-rosado@users.noreply.github.com>
Date: Tue, 19 Mar 2024 14:19:41 +0000
Subject: [PATCH 1/3] [PM-6334] Passing CollectionView or FolderView from
 Import component to ImportService (#8291)

* Passing CollectionView or FolderView from Import component to ImportService

* Corrected import service tests

* Added tests to validate if the incorrect object type error is thrown on setImportTarget
---
 .../src/components/import.component.html      |  4 +-
 .../services/import.service.abstraction.ts    |  5 +-
 .../src/services/import.service.spec.ts       | 66 ++++++++++++++-----
 libs/importer/src/services/import.service.ts  | 34 +++++-----
 4 files changed, 71 insertions(+), 38 deletions(-)

diff --git a/libs/importer/src/components/import.component.html b/libs/importer/src/components/import.component.html
index 6c24b80f92d..836a1d9a1a1 100644
--- a/libs/importer/src/components/import.component.html
+++ b/libs/importer/src/components/import.component.html
@@ -37,7 +37,7 @@
         <bit-option [value]="null" label="-- {{ 'selectImportFolder' | i18n }} --" />
         <bit-option
           *ngFor="let f of folders$ | async"
-          [value]="f.id"
+          [value]="f"
           [label]="f.name"
           icon="bwi-folder"
         />
@@ -46,7 +46,7 @@
         <bit-option [value]="null" label="-- {{ 'selectImportCollection' | i18n }} --" />
         <bit-option
           *ngFor="let c of collections$ | async"
-          [value]="c.id"
+          [value]="c"
           [label]="c.name"
           icon="bwi-collection"
         />
diff --git a/libs/importer/src/services/import.service.abstraction.ts b/libs/importer/src/services/import.service.abstraction.ts
index 95208c9b99f..dc77e76390e 100644
--- a/libs/importer/src/services/import.service.abstraction.ts
+++ b/libs/importer/src/services/import.service.abstraction.ts
@@ -1,3 +1,6 @@
+import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
+import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
+
 import { Importer } from "../importers/importer";
 import { ImportOption, ImportType } from "../models/import-options";
 import { ImportResult } from "../models/import-result";
@@ -10,7 +13,7 @@ export abstract class ImportServiceAbstraction {
     importer: Importer,
     fileContents: string,
     organizationId?: string,
-    selectedImportTarget?: string,
+    selectedImportTarget?: FolderView | CollectionView,
     canAccessImportExport?: boolean,
   ) => Promise<ImportResult>;
   getImporter: (
diff --git a/libs/importer/src/services/import.service.spec.ts b/libs/importer/src/services/import.service.spec.ts
index 6a2e0a339c7..a95b74d792c 100644
--- a/libs/importer/src/services/import.service.spec.ts
+++ b/libs/importer/src/services/import.service.spec.ts
@@ -86,7 +86,7 @@ describe("ImportService", () => {
     });
 
     it("empty importTarget does nothing", async () => {
-      await importService["setImportTarget"](importResult, null, "");
+      await importService["setImportTarget"](importResult, null, null);
       expect(importResult.folders.length).toBe(0);
     });
 
@@ -99,9 +99,9 @@ describe("ImportService", () => {
         Promise.resolve([mockImportTargetFolder]),
       );
 
-      await importService["setImportTarget"](importResult, null, "myImportTarget");
+      await importService["setImportTarget"](importResult, null, mockImportTargetFolder);
       expect(importResult.folders.length).toBe(1);
-      expect(importResult.folders[0].name).toBe("myImportTarget");
+      expect(importResult.folders[0]).toBe(mockImportTargetFolder);
     });
 
     const mockFolder1 = new FolderView();
@@ -119,16 +119,18 @@ describe("ImportService", () => {
         mockFolder2,
       ]);
 
-      const myImportTarget = "myImportTarget";
-
       importResult.folders.push(mockFolder1);
       importResult.folders.push(mockFolder2);
 
-      await importService["setImportTarget"](importResult, null, myImportTarget);
+      await importService["setImportTarget"](importResult, null, mockImportTargetFolder);
       expect(importResult.folders.length).toBe(3);
-      expect(importResult.folders[0].name).toBe(myImportTarget);
-      expect(importResult.folders[1].name).toBe(`${myImportTarget}/${mockFolder1.name}`);
-      expect(importResult.folders[2].name).toBe(`${myImportTarget}/${mockFolder2.name}`);
+      expect(importResult.folders[0]).toBe(mockImportTargetFolder);
+      expect(importResult.folders[1].name).toBe(
+        `${mockImportTargetFolder.name}/${mockFolder1.name}`,
+      );
+      expect(importResult.folders[2].name).toBe(
+        `${mockImportTargetFolder.name}/${mockFolder2.name}`,
+      );
     });
 
     const mockImportTargetCollection = new CollectionView();
@@ -152,9 +154,13 @@ describe("ImportService", () => {
         mockCollection1,
       ]);
 
-      await importService["setImportTarget"](importResult, organizationId, "myImportTarget");
+      await importService["setImportTarget"](
+        importResult,
+        organizationId,
+        mockImportTargetCollection,
+      );
       expect(importResult.collections.length).toBe(1);
-      expect(importResult.collections[0].name).toBe("myImportTarget");
+      expect(importResult.collections[0]).toBe(mockImportTargetCollection);
     });
 
     it("passing importTarget sets it as new root for all existing collections", async () => {
@@ -164,16 +170,42 @@ describe("ImportService", () => {
         mockCollection2,
       ]);
 
-      const myImportTarget = "myImportTarget";
-
       importResult.collections.push(mockCollection1);
       importResult.collections.push(mockCollection2);
 
-      await importService["setImportTarget"](importResult, organizationId, myImportTarget);
+      await importService["setImportTarget"](
+        importResult,
+        organizationId,
+        mockImportTargetCollection,
+      );
       expect(importResult.collections.length).toBe(3);
-      expect(importResult.collections[0].name).toBe(myImportTarget);
-      expect(importResult.collections[1].name).toBe(`${myImportTarget}/${mockCollection1.name}`);
-      expect(importResult.collections[2].name).toBe(`${myImportTarget}/${mockCollection2.name}`);
+      expect(importResult.collections[0]).toBe(mockImportTargetCollection);
+      expect(importResult.collections[1].name).toBe(
+        `${mockImportTargetCollection.name}/${mockCollection1.name}`,
+      );
+      expect(importResult.collections[2].name).toBe(
+        `${mockImportTargetCollection.name}/${mockCollection2.name}`,
+      );
+    });
+
+    it("passing importTarget as null on setImportTarget with organizationId throws error", async () => {
+      const setImportTargetMethod = importService["setImportTarget"](
+        null,
+        organizationId,
+        new Object() as FolderView,
+      );
+
+      await expect(setImportTargetMethod).rejects.toThrow("Error assigning target collection");
+    });
+
+    it("passing importTarget as null on setImportTarget throws error", async () => {
+      const setImportTargetMethod = importService["setImportTarget"](
+        null,
+        "",
+        new Object() as CollectionView,
+      );
+
+      await expect(setImportTargetMethod).rejects.toThrow("Error assigning target folder");
     });
   });
 });
diff --git a/libs/importer/src/services/import.service.ts b/libs/importer/src/services/import.service.ts
index e26b768ab67..a6fd233dcf6 100644
--- a/libs/importer/src/services/import.service.ts
+++ b/libs/importer/src/services/import.service.ts
@@ -110,7 +110,7 @@ export class ImportService implements ImportServiceAbstraction {
     importer: Importer,
     fileContents: string,
     organizationId: string = null,
-    selectedImportTarget: string = null,
+    selectedImportTarget: FolderView | CollectionView = null,
     canAccessImportExport: boolean,
   ): Promise<ImportResult> {
     let importResult: ImportResult;
@@ -147,11 +147,7 @@ export class ImportService implements ImportServiceAbstraction {
       }
     }
 
-    if (
-      organizationId &&
-      Utils.isNullOrWhitespace(selectedImportTarget) &&
-      !canAccessImportExport
-    ) {
+    if (organizationId && !selectedImportTarget && !canAccessImportExport) {
       const hasUnassignedCollections =
         importResult.collectionRelationships.length < importResult.ciphers.length;
       if (hasUnassignedCollections) {
@@ -428,29 +424,30 @@ export class ImportService implements ImportServiceAbstraction {
   private async setImportTarget(
     importResult: ImportResult,
     organizationId: string,
-    importTarget: string,
+    importTarget: FolderView | CollectionView,
   ) {
-    if (Utils.isNullOrWhitespace(importTarget)) {
+    if (!importTarget) {
       return;
     }
 
     if (organizationId) {
-      const collectionViews: CollectionView[] = await this.collectionService.getAllDecrypted();
-      const targetCollection = collectionViews.find((c) => c.id === importTarget);
+      if (!(importTarget instanceof CollectionView)) {
+        throw new Error("Error assigning target collection");
+      }
 
       const noCollectionRelationShips: [number, number][] = [];
       importResult.ciphers.forEach((c, index) => {
         if (!Array.isArray(c.collectionIds) || c.collectionIds.length == 0) {
-          c.collectionIds = [targetCollection.id];
+          c.collectionIds = [importTarget.id];
           noCollectionRelationShips.push([index, 0]);
         }
       });
 
       const collections: CollectionView[] = [...importResult.collections];
-      importResult.collections = [targetCollection];
+      importResult.collections = [importTarget as CollectionView];
       collections.map((x) => {
         const f = new CollectionView();
-        f.name = `${targetCollection.name}/${x.name}`;
+        f.name = `${importTarget.name}/${x.name}`;
         importResult.collections.push(f);
       });
 
@@ -463,21 +460,22 @@ export class ImportService implements ImportServiceAbstraction {
       return;
     }
 
-    const folderViews = await this.folderService.getAllDecryptedFromState();
-    const targetFolder = folderViews.find((f) => f.id === importTarget);
+    if (!(importTarget instanceof FolderView)) {
+      throw new Error("Error assigning target folder");
+    }
 
     const noFolderRelationShips: [number, number][] = [];
     importResult.ciphers.forEach((c, index) => {
       if (Utils.isNullOrEmpty(c.folderId)) {
-        c.folderId = targetFolder.id;
+        c.folderId = importTarget.id;
         noFolderRelationShips.push([index, 0]);
       }
     });
 
     const folders: FolderView[] = [...importResult.folders];
-    importResult.folders = [targetFolder];
+    importResult.folders = [importTarget as FolderView];
     folders.map((x) => {
-      const newFolderName = `${targetFolder.name}/${x.name}`;
+      const newFolderName = `${importTarget.name}/${x.name}`;
       const f = new FolderView();
       f.name = newFolderName;
       importResult.folders.push(f);

From ea0035f658fde9d433910d877830223277757b31 Mon Sep 17 00:00:00 2001
From: Daniel James Smith <2670567+djsmith85@users.noreply.github.com>
Date: Tue, 19 Mar 2024 17:20:57 +0100
Subject: [PATCH 2/3] [PM-6755] Fix password generation defaults on CLI (#8308)

* Fix minSpecial for pwd generation being set to 1 instead of zero

* Use less magic numbers

---------

Co-authored-by: Daniel James Smith <djsmith85@users.noreply.github.com>
---
 apps/cli/src/tools/generate.command.ts        | 20 +++++++++++++++----
 .../password/password-generation-options.ts   |  4 ++--
 .../password/password-generation.service.ts   |  2 +-
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/apps/cli/src/tools/generate.command.ts b/apps/cli/src/tools/generate.command.ts
index 4763c6992ed..2bbfb02267b 100644
--- a/apps/cli/src/tools/generate.command.ts
+++ b/apps/cli/src/tools/generate.command.ts
@@ -1,6 +1,9 @@
 import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 import { DefaultPassphraseGenerationOptions } from "@bitwarden/common/tools/generator/passphrase";
-import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password";
+import {
+  DefaultPasswordGenerationOptions,
+  PasswordGenerationServiceAbstraction,
+} from "@bitwarden/common/tools/generator/password";
 import { PasswordGeneratorOptions } from "@bitwarden/common/tools/generator/password/password-generator-options";
 
 import { Response } from "../models/response";
@@ -64,7 +67,10 @@ class Options {
     this.capitalize = CliUtils.convertBooleanOption(passedOptions?.capitalize);
     this.includeNumber = CliUtils.convertBooleanOption(passedOptions?.includeNumber);
     this.ambiguous = CliUtils.convertBooleanOption(passedOptions?.ambiguous);
-    this.length = CliUtils.convertNumberOption(passedOptions?.length, 14);
+    this.length = CliUtils.convertNumberOption(
+      passedOptions?.length,
+      DefaultPasswordGenerationOptions.length,
+    );
     this.type = passedOptions?.passphrase ? "passphrase" : "password";
     this.separator = CliUtils.convertStringOption(
       passedOptions?.separator,
@@ -74,8 +80,14 @@ class Options {
       passedOptions?.words,
       DefaultPassphraseGenerationOptions.numWords,
     );
-    this.minNumber = CliUtils.convertNumberOption(passedOptions?.minNumber, 1);
-    this.minSpecial = CliUtils.convertNumberOption(passedOptions?.minSpecial, 1);
+    this.minNumber = CliUtils.convertNumberOption(
+      passedOptions?.minNumber,
+      DefaultPasswordGenerationOptions.minNumber,
+    );
+    this.minSpecial = CliUtils.convertNumberOption(
+      passedOptions?.minSpecial,
+      DefaultPasswordGenerationOptions.minSpecial,
+    );
 
     if (!this.uppercase && !this.lowercase && !this.special && !this.number) {
       this.lowercase = true;
diff --git a/libs/common/src/tools/generator/password/password-generation-options.ts b/libs/common/src/tools/generator/password/password-generation-options.ts
index 55b27e4e7a0..a48eeb77c6e 100644
--- a/libs/common/src/tools/generator/password/password-generation-options.ts
+++ b/libs/common/src/tools/generator/password/password-generation-options.ts
@@ -78,6 +78,6 @@ export const DefaultPasswordGenerationOptions: Partial<PasswordGenerationOptions
   lowercase: true,
   number: true,
   minNumber: 1,
-  special: true,
-  minSpecial: 1,
+  special: false,
+  minSpecial: 0,
 });
diff --git a/libs/common/src/tools/generator/password/password-generation.service.ts b/libs/common/src/tools/generator/password/password-generation.service.ts
index 0b37e81fdc1..eb1f08d97e6 100644
--- a/libs/common/src/tools/generator/password/password-generation.service.ts
+++ b/libs/common/src/tools/generator/password/password-generation.service.ts
@@ -23,7 +23,7 @@ const DefaultOptions: PasswordGeneratorOptions = {
   lowercase: true,
   minLowercase: 0,
   special: false,
-  minSpecial: 1,
+  minSpecial: 0,
   type: "password",
   numWords: 3,
   wordSeparator: "-",

From dcea4682814f718d3462ee7fa669f83516b6bc3e Mon Sep 17 00:00:00 2001
From: Daniel James Smith <2670567+djsmith85@users.noreply.github.com>
Date: Tue, 19 Mar 2024 20:49:59 +0100
Subject: [PATCH 3/3] [PM-6790][Tech Debt] Cleanup export web component (#8323)

* Remove formPromise and use bitSubmit

* Use formGroup.invalid instead of !valid

* Move variables related to encrypted exports into base component.

* Migrate to use new userVerificationDialogComponent

---------

Co-authored-by: Daniel James Smith <djsmith85@users.noreply.github.com>
---
 .../tools/vault-export/export.component.html  | 10 +---
 .../tools/vault-export/export.component.ts    | 58 ++++++++-----------
 .../export/components/export.component.ts     | 21 ++++++-
 3 files changed, 45 insertions(+), 44 deletions(-)

diff --git a/apps/web/src/app/tools/vault-export/export.component.html b/apps/web/src/app/tools/vault-export/export.component.html
index 9d615d329d7..8ed82b9fd9e 100644
--- a/apps/web/src/app/tools/vault-export/export.component.html
+++ b/apps/web/src/app/tools/vault-export/export.component.html
@@ -1,13 +1,7 @@
 <app-header></app-header>
 
 <bit-container>
-  <form
-    #form
-    (ngSubmit)="submit()"
-    [appApiAction]="formPromise"
-    [formGroup]="exportForm"
-    *ngIf="exportForm"
-  >
+  <form [formGroup]="exportForm" [bitSubmit]="submit">
     <bit-callout type="danger" title="{{ 'vaultExportDisabled' | i18n }}" *ngIf="disabledByPolicy">
       {{ "personalVaultExportPolicyInEffect" | i18n }}
     </bit-callout>
@@ -110,9 +104,9 @@
 
     <button
       bitButton
+      bitFormButton
       type="submit"
       buttonType="primary"
-      [loading]="form.loading"
       [disabled]="disabledByPolicy"
     >
       {{ "confirmFormat" | i18n }}
diff --git a/apps/web/src/app/tools/vault-export/export.component.ts b/apps/web/src/app/tools/vault-export/export.component.ts
index e3b153bfccb..8b14092f20e 100644
--- a/apps/web/src/app/tools/vault-export/export.component.ts
+++ b/apps/web/src/app/tools/vault-export/export.component.ts
@@ -1,8 +1,8 @@
 import { Component } from "@angular/core";
 import { UntypedFormBuilder } from "@angular/forms";
-import { firstValueFrom } from "rxjs";
 
 import { ExportComponent as BaseExportComponent } from "@bitwarden/angular/tools/export/components/export.component";
+import { UserVerificationDialogComponent } from "@bitwarden/auth/angular";
 import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
@@ -11,20 +11,14 @@ import { FileDownloadService } from "@bitwarden/common/platform/abstractions/fil
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
-import { EncryptedExportType } from "@bitwarden/common/tools/enums/encrypted-export-type.enum";
 import { DialogService } from "@bitwarden/components";
 import { VaultExportServiceAbstraction } from "@bitwarden/vault-export-core";
 
-import { openUserVerificationPrompt } from "../../auth/shared/components/user-verification";
-
 @Component({
   selector: "app-export",
   templateUrl: "export.component.html",
 })
 export class ExportComponent extends BaseExportComponent {
-  encryptedExportType = EncryptedExportType;
-  protected showFilePassword: boolean;
-
   constructor(
     i18nService: I18nService,
     platformUtilsService: PlatformUtilsService,
@@ -53,7 +47,7 @@ export class ExportComponent extends BaseExportComponent {
     );
   }
 
-  async submit() {
+  submit = async () => {
     if (this.isFileEncryptedExport && this.filePassword != this.confirmFilePassword) {
       this.platformUtilsService.showToast(
         "error",
@@ -64,7 +58,7 @@ export class ExportComponent extends BaseExportComponent {
     }
 
     this.exportForm.markAllAsTouched();
-    if (!this.exportForm.valid) {
+    if (this.exportForm.invalid) {
       return;
     }
 
@@ -85,14 +79,14 @@ export class ExportComponent extends BaseExportComponent {
     // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
     // eslint-disable-next-line @typescript-eslint/no-floating-promises
     this.doExport();
-  }
+  };
 
   protected saved() {
     super.saved();
     this.platformUtilsService.showToast("success", null, this.i18nService.t("exportSuccess"));
   }
 
-  private verifyUser() {
+  private async verifyUser(): Promise<boolean> {
     let confirmDescription = "exportWarningDesc";
     if (this.isFileEncryptedExport) {
       confirmDescription = "fileEncryptedExportWarningDesc";
@@ -100,32 +94,30 @@ export class ExportComponent extends BaseExportComponent {
       confirmDescription = "encExportKeyWarningDesc";
     }
 
-    const ref = openUserVerificationPrompt(this.dialogService, {
-      data: {
-        confirmDescription: confirmDescription,
-        confirmButtonText: "exportVault",
-        modalTitle: "confirmVaultExport",
+    const result = await UserVerificationDialogComponent.open(this.dialogService, {
+      clientSideOnlyVerification: true,
+      title: "confirmVaultExport",
+      bodyText: confirmDescription,
+      confirmButtonOptions: {
+        text: "exportVault",
+        type: "primary",
       },
     });
 
-    if (ref == null) {
-      return;
+    // Handle the result of the dialog based on user action and verification success
+    if (result.userAction === "cancel") {
+      // User cancelled the dialog
+      return false;
     }
 
-    return firstValueFrom(ref.closed);
-  }
-
-  get isFileEncryptedExport() {
-    return (
-      this.format === "encrypted_json" &&
-      this.fileEncryptionType === EncryptedExportType.FileEncrypted
-    );
-  }
-
-  get isAccountEncryptedExport() {
-    return (
-      this.format === "encrypted_json" &&
-      this.fileEncryptionType === EncryptedExportType.AccountEncrypted
-    );
+    // User confirmed the dialog so check verification success
+    if (!result.verificationSuccess) {
+      if (result.noAvailableClientVerificationMethods) {
+        // No client-side verification methods are available
+        // Could send user to configure a verification method like PIN or biometrics
+      }
+      return false;
+    }
+    return true;
   }
 }
diff --git a/libs/angular/src/tools/export/components/export.component.ts b/libs/angular/src/tools/export/components/export.component.ts
index 6c7ac0f3659..400071e59cc 100644
--- a/libs/angular/src/tools/export/components/export.component.ts
+++ b/libs/angular/src/tools/export/components/export.component.ts
@@ -25,8 +25,10 @@ export class ExportComponent implements OnInit, OnDestroy {
   @Output() onSaved = new EventEmitter();
   @ViewChild(PasswordStrengthComponent) passwordStrengthComponent: PasswordStrengthComponent;
 
+  encryptedExportType = EncryptedExportType;
+  protected showFilePassword: boolean;
+
   filePasswordValue: string = null;
-  formPromise: Promise<string>;
   private _disabledByPolicy = false;
 
   protected organizationId: string = null;
@@ -126,10 +128,23 @@ export class ExportComponent implements OnInit, OnDestroy {
     return this.format === "encrypted_json";
   }
 
+  get isFileEncryptedExport() {
+    return (
+      this.format === "encrypted_json" &&
+      this.fileEncryptionType === EncryptedExportType.FileEncrypted
+    );
+  }
+
+  get isAccountEncryptedExport() {
+    return (
+      this.format === "encrypted_json" &&
+      this.fileEncryptionType === EncryptedExportType.AccountEncrypted
+    );
+  }
+
   protected async doExport() {
     try {
-      this.formPromise = this.getExportData();
-      const data = await this.formPromise;
+      const data = await this.getExportData();
       this.downloadFile(data);
       this.saved();
       await this.collectEvent();