Add the ability for custom validation logic to be injected into `User…

…VerificationDialogComponent` (#8770) * Introduce `verificationType` * Update template to use `verificationType` * Implement a path for `verificationType = 'custom'` * Delete `clientSideOnlyVerification` * Update `EnrollMasterPasswordResetComponent` to include a server-side hash check * Better describe the custom scenerio through comments * Add an example of the custom verficiation scenerio * Move execution of verification function into try/catch * Migrate existing uses of `clientSideOnlyVerification` * Use generic type option instead of casting * Change "given" to "determined" in a comment
bitwarden · Jul 1, 2024 · c69c4b6 · c69c4b6
1 parent 5260e5a
commit c69c4b6
Show file tree

Hide file tree

Showing 7 changed files with 108 additions and 41 deletions.
diff --git a/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts b/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts
@@ -89,7 +89,7 @@ describe("Fido2UserVerificationService", () => {
         );
 
         expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
-          clientSideOnlyVerification: true,
+          verificationType: "client",
         });
         expect(result).toBe(true);
       });
@@ -105,7 +105,7 @@ describe("Fido2UserVerificationService", () => {
         );
 
         expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
-          clientSideOnlyVerification: true,
+          verificationType: "client",
         });
         expect(result).toBe(true);
       });
@@ -122,7 +122,7 @@ describe("Fido2UserVerificationService", () => {
         );
 
         expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
-          clientSideOnlyVerification: true,
+          verificationType: "client",
         });
         expect(result).toBe(true);
       });
@@ -135,7 +135,7 @@ describe("Fido2UserVerificationService", () => {
         );
 
         expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
-          clientSideOnlyVerification: true,
+          verificationType: "client",
         });
         expect(result).toBe(true);
       });
@@ -198,7 +198,7 @@ describe("Fido2UserVerificationService", () => {
         );
 
         expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
-          clientSideOnlyVerification: true,
+          verificationType: "client",
         });
         expect(result).toBe(true);
       });
@@ -214,7 +214,7 @@ describe("Fido2UserVerificationService", () => {
         );
 
         expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
-          clientSideOnlyVerification: true,
+          verificationType: "client",
         });
         expect(result).toBe(true);
       });
@@ -231,7 +231,7 @@ describe("Fido2UserVerificationService", () => {
         );
 
         expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
-          clientSideOnlyVerification: true,
+          verificationType: "client",
         });
         expect(result).toBe(true);
       });

diff --git a/apps/browser/src/vault/services/fido2-user-verification.service.ts b/apps/browser/src/vault/services/fido2-user-verification.service.ts
@@ -54,7 +54,7 @@ export class Fido2UserVerificationService {
 
   private async showUserVerificationDialog(): Promise<boolean> {
     const result = await UserVerificationDialogComponent.open(this.dialogService, {
-      clientSideOnlyVerification: true,
+      verificationType: "client",
     });
 
     if (result.userAction === "cancel") {

diff --git a/apps/web/src/app/admin-console/organizations/users/enroll-master-password-reset.component.ts b/apps/web/src/app/admin-console/organizations/users/enroll-master-password-reset.component.ts
@@ -2,6 +2,8 @@ import { UserVerificationDialogComponent } from "@bitwarden/auth/angular";
 import { OrganizationUserService } from "@bitwarden/common/admin-console/abstractions/organization-user/organization-user.service";
 import { OrganizationUserResetPasswordEnrollmentRequest } from "@bitwarden/common/admin-console/abstractions/organization-user/requests";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
+import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
+import { VerificationWithSecret } from "@bitwarden/common/auth/types/verification";
 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";
@@ -26,43 +28,50 @@ export class EnrollMasterPasswordReset {
     i18nService: I18nService,
     syncService: SyncService,
     logService: LogService,
+    userVerificationService: UserVerificationService,
   ) {
     const result = await UserVerificationDialogComponent.open(dialogService, {
       title: "enrollAccountRecovery",
       calloutOptions: {
         text: "resetPasswordEnrollmentWarning",
         type: "warning",
       },
+      verificationType: {
+        type: "custom",
+        verificationFn: async (secret: VerificationWithSecret) => {
+          const request =
+            await userVerificationService.buildRequest<OrganizationUserResetPasswordEnrollmentRequest>(
+              secret,
+            );
+          request.resetPasswordKey = await resetPasswordService.buildRecoveryKey(
+            data.organization.id,
+          );
+
+          // Process the enrollment request, which is an endpoint that is
+          // gated by a server-side check of the master password hash
+          await organizationUserService.putOrganizationUserResetPasswordEnrollment(
+            data.organization.id,
+            data.organization.userId,
+            request,
+          );
+          return true;
+        },
+      },
     });
 
-    // Handle the result of the dialog based on user action and verification success
+    // User canceled enrollment
     if (result.userAction === "cancel") {
       return;
     }
 
-    // User confirmed the dialog so check verification success
+    // Enrollment failed
     if (!result.verificationSuccess) {
-      // verification failed
       return;
     }
 
-    // Verification succeeded
+    // Enrollment succeeded
     try {
-      // This object is missing most of the properties in the
-      // `OrganizationUserResetPasswordEnrollmentRequest()`, but those
-      // properties don't carry over to the server model anyway and are
-      // never used by this flow.
-      const request = new OrganizationUserResetPasswordEnrollmentRequest();
-      request.resetPasswordKey = await resetPasswordService.buildRecoveryKey(data.organization.id);
-
-      await organizationUserService.putOrganizationUserResetPasswordEnrollment(
-        data.organization.id,
-        data.organization.userId,
-        request,
-      );
-
       platformUtilsService.showToast("success", null, i18nService.t("enrollPasswordResetSuccess"));
-
       await syncService.fullSync(true);
     } catch (e) {
       logService.error(e);

diff --git a/.../src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts b/.../src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts
@@ -10,6 +10,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli
 import { PolicyType } from "@bitwarden/common/admin-console/enums";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { Policy } from "@bitwarden/common/admin-console/models/domain/policy";
+import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
 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";
@@ -48,6 +49,7 @@ export class OrganizationOptionsComponent implements OnInit, OnDestroy {
     private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction,
     private dialogService: DialogService,
     private resetPasswordService: OrganizationUserResetPasswordService,
+    private userVerificationService: UserVerificationService,
   ) {}
 
   async ngOnInit() {
@@ -155,6 +157,7 @@ export class OrganizationOptionsComponent implements OnInit, OnDestroy {
         this.i18nService,
         this.syncService,
         this.logService,
+        this.userVerificationService,
       );
     } else {
       // Remove reset password

diff --git a/libs/auth/src/angular/user-verification/user-verification-dialog.component.html b/libs/auth/src/angular/user-verification/user-verification-dialog.component.html
@@ -9,8 +9,8 @@
       <!-- Show optional content when verification is server side or client side and verification methods were found. -->
       <ng-container
         *ngIf="
-          !dialogOptions.clientSideOnlyVerification ||
-          (dialogOptions.clientSideOnlyVerification &&
+          dialogOptions.verificationType !== 'client' ||
+          (dialogOptions.verificationType === 'client' &&
             activeClientVerificationOption !== ActiveClientVerificationOption.None)
         "
       >
@@ -29,7 +29,7 @@
       <!-- Shown when client side verification methods picked and no verification methods found -->
       <ng-container
         *ngIf="
-          dialogOptions.clientSideOnlyVerification &&
+          dialogOptions.verificationType === 'client' &&
           activeClientVerificationOption === ActiveClientVerificationOption.None
         "
       >
@@ -41,7 +41,7 @@
       <app-user-verification-form-input
         [(invalidSecret)]="invalidSecret"
         formControlName="secret"
-        [verificationType]="dialogOptions.clientSideOnlyVerification ? 'client' : 'server'"
+        [verificationType]="dialogOptions.verificationType === 'client' ? 'client' : 'server'"
         (activeClientVerificationOptionChange)="handleActiveClientVerificationOptionChange($event)"
         (biometricsVerificationResultChange)="handleBiometricsVerificationResultChange($event)"
       ></app-user-verification-form-input>
@@ -50,8 +50,8 @@
       <!-- Confirm button container - shown for server side validation but hidden if client side validation + biometrics  -->
       <ng-container
         *ngIf="
-          !dialogOptions.clientSideOnlyVerification ||
-          (dialogOptions.clientSideOnlyVerification &&
+          dialogOptions.verificationType !== 'client' ||
+          (dialogOptions.verificationType === 'client' &&
             activeClientVerificationOption !== ActiveClientVerificationOption.Biometrics)
         "
       >
@@ -85,10 +85,12 @@
         <ng-container
           *ngIf="activeClientVerificationOption === ActiveClientVerificationOption.None"
         >
-          <!-- For no client verifications found, show set a pin confirm button. 
-            Note: this doesn't make sense for web as web doesn't support PINs, but this is how we are handling it for now 
-          as the expectation is that only browser and desktop will use the new clientSideOnlyVerification flow. 
-          We might genericize this in the future to just tell the user they need to configure a valid user verification option like PIN or Biometrics.  -->
+          <!--
+            For no client verifications found, show set a pin confirm button.
+            Note: this doesn't make sense for web as web doesn't support PINs, but this is how we are handling it for now
+            as the expectation is that only browser and desktop will use the new verificationType 'client' flow.
+            We might genericize this in the future to just tell the user they need to configure a valid user verification option like PIN or Biometrics.
+          -->
           <button type="submit" bitButton bitFormButton buttonType="primary">
             {{ "setPin" | i18n }}
           </button>

diff --git a/libs/auth/src/angular/user-verification/user-verification-dialog.component.ts b/libs/auth/src/angular/user-verification/user-verification-dialog.component.ts
@@ -142,6 +142,31 @@ export class UserVerificationDialogComponent {
    *   return;
    * }
    *
+   * ----------------------------------------------------------
+   *
+   * @example
+   * // Example 4: Custom user verification validation
+   *
+   * const result = await UserVerificationDialogComponent.open(dialogService, {
+   *   verificationType: {
+   *     type: "custom",
+   *     // Pass in a function that will be used to validate the input of the
+   *     // verification dialog, returning true when finished.
+   *     verificationFn: async (secret: VerificationWithSecret) => {
+   *       const request = await userVerificationService.buildRequest<CustomRequestType>(secret);
+   *
+   *      // ... Do something with the custom request type
+   *
+   *       await someServicer.sendMyRequestThatVerfiesUserIdentity(
+   *         // ... Some other data
+   *         request,
+   *       );
+   *       return true;
+   *     },
+   *   },
+   * });
+   *
+   * // ... Evaluate the result as usual
    */
   static async open(
     dialogService: DialogService,
@@ -202,6 +227,18 @@ export class UserVerificationDialogComponent {
     }
 
     try {
+      if (
+        typeof this.dialogOptions.verificationType === "object" &&
+        this.dialogOptions.verificationType.type === "custom"
+      ) {
+        const success = await this.dialogOptions.verificationType.verificationFn(this.secret.value);
+        this.close({
+          userAction: "confirm",
+          verificationSuccess: success,
+        });
+        return;
+      }
+
       // TODO: once we migrate all user verification scenarios to use this new implementation,
       // we should consider refactoring the user verification service handling of the
       // OTP and MP flows to not throw errors on verification failure.

diff --git a/libs/auth/src/angular/user-verification/user-verification-dialog.types.ts b/libs/auth/src/angular/user-verification/user-verification-dialog.types.ts
@@ -1,3 +1,4 @@
+import { VerificationWithSecret } from "@bitwarden/common/auth/types/verification";
 import { ButtonType } from "@bitwarden/components";
 
 /**
@@ -60,12 +61,27 @@ export type UserVerificationDialogOptions = {
    */
   confirmButtonOptions?: UserVerificationConfirmButtonOptions;
 
-  /**
-   * Indicates whether the verification is only performed client-side. Includes local MP verification, PIN, and Biometrics.
-   * Optional.
-   * **Important:** Only for use on desktop and browser platforms as when there are no client verification methods, the user is instructed to set a pin (which is not supported on web)
+  /** The validation method used to verify the secret.
+   *
+   * Possible values:
+   *
+   * - "default": Perform the default validation operation for the determined
+   *   secret type. This would, for example, validate master passwords
+   *   locally but OTPs on the server.
+   * - "client": Only do a client-side verification with no possible server
+   *   request. Includes local MP verification, PIN, and Biometrics.
+   *   **Important:** This option is only for use on desktop and browser
+   *   platforms. When there are no client verification methods the user is
+   *   instructed to set a pin, and this is not supported on web.
+   * - "custom": Custom validation is done to verify the secret. This is
+   *   passed in from callers when opening the dialog. The custom type is
+   *   meant to provide a mechanism where users can call a secured endpoint
+   *   that performs user verification server side.
    */
-  clientSideOnlyVerification?: boolean;
+  verificationType?:
+    | "default"
+    | "client"
+    | { type: "custom"; verificationFn: (secret: VerificationWithSecret) => Promise<boolean> };
 };
 
 /**