From 71e8fdb73d935fe8d92d0da7a06aecd21e3efe2b Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Mon, 1 Jul 2024 11:52:39 -0400 Subject: [PATCH] Add the ability for custom validation logic to be injected into `UserVerificationDialogComponent` (#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 --- .../fido2-user-verification.service.spec.ts | 14 +++--- .../fido2-user-verification.service.ts | 2 +- .../enroll-master-password-reset.component.ts | 45 +++++++++++-------- .../organization-options.component.ts | 3 ++ .../user-verification-dialog.component.html | 22 ++++----- .../user-verification-dialog.component.ts | 37 +++++++++++++++ .../user-verification-dialog.types.ts | 26 ++++++++--- 7 files changed, 108 insertions(+), 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 index acee6ba20f..2e715c15af 100644 --- 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 index 90c4d8ca61..3c91001fd5 100644 --- 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 { 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 index b228a4d135..e262fa51ff 100644 --- 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,6 +28,7 @@ export class EnrollMasterPasswordReset { i18nService: I18nService, syncService: SyncService, logService: LogService, + userVerificationService: UserVerificationService, ) { const result = await UserVerificationDialogComponent.open(dialogService, { title: "enrollAccountRecovery", @@ -33,36 +36,42 @@ export class EnrollMasterPasswordReset { text: "resetPasswordEnrollmentWarning", type: "warning", }, + verificationType: { + type: "custom", + verificationFn: async (secret: VerificationWithSecret) => { + const request = + await userVerificationService.buildRequest( + 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/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts b/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts index 5a138c3147..bdcd409dbf 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts +++ b/apps/web/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 index aa4d26ae61..fb3b7cf4cb 100644 --- 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 @@ @@ -29,7 +29,7 @@ @@ -41,7 +41,7 @@ @@ -50,8 +50,8 @@ @@ -85,10 +85,12 @@ - + 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 index 7b2c869e3a..f8746b5b24 100644 --- 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(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 index f4637c770a..cb03f4e18f 100644 --- 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 }; }; /**