1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-12 06:13:38 +00:00

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
This commit is contained in:
Addison Beck
2024-07-01 11:52:39 -04:00
committed by GitHub
parent 9531d1c655
commit 71e8fdb73d
7 changed files with 108 additions and 41 deletions

View File

@@ -89,7 +89,7 @@ describe("Fido2UserVerificationService", () => {
); );
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true, verificationType: "client",
}); });
expect(result).toBe(true); expect(result).toBe(true);
}); });
@@ -105,7 +105,7 @@ describe("Fido2UserVerificationService", () => {
); );
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true, verificationType: "client",
}); });
expect(result).toBe(true); expect(result).toBe(true);
}); });
@@ -122,7 +122,7 @@ describe("Fido2UserVerificationService", () => {
); );
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true, verificationType: "client",
}); });
expect(result).toBe(true); expect(result).toBe(true);
}); });
@@ -135,7 +135,7 @@ describe("Fido2UserVerificationService", () => {
); );
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true, verificationType: "client",
}); });
expect(result).toBe(true); expect(result).toBe(true);
}); });
@@ -198,7 +198,7 @@ describe("Fido2UserVerificationService", () => {
); );
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true, verificationType: "client",
}); });
expect(result).toBe(true); expect(result).toBe(true);
}); });
@@ -214,7 +214,7 @@ describe("Fido2UserVerificationService", () => {
); );
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true, verificationType: "client",
}); });
expect(result).toBe(true); expect(result).toBe(true);
}); });
@@ -231,7 +231,7 @@ describe("Fido2UserVerificationService", () => {
); );
expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, { expect(UserVerificationDialogComponent.open).toHaveBeenCalledWith(dialogService, {
clientSideOnlyVerification: true, verificationType: "client",
}); });
expect(result).toBe(true); expect(result).toBe(true);
}); });

View File

@@ -54,7 +54,7 @@ export class Fido2UserVerificationService {
private async showUserVerificationDialog(): Promise<boolean> { private async showUserVerificationDialog(): Promise<boolean> {
const result = await UserVerificationDialogComponent.open(this.dialogService, { const result = await UserVerificationDialogComponent.open(this.dialogService, {
clientSideOnlyVerification: true, verificationType: "client",
}); });
if (result.userAction === "cancel") { if (result.userAction === "cancel") {

View File

@@ -2,6 +2,8 @@ import { UserVerificationDialogComponent } from "@bitwarden/auth/angular";
import { OrganizationUserService } from "@bitwarden/common/admin-console/abstractions/organization-user/organization-user.service"; 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 { OrganizationUserResetPasswordEnrollmentRequest } from "@bitwarden/common/admin-console/abstractions/organization-user/requests";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; 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 { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@@ -26,6 +28,7 @@ export class EnrollMasterPasswordReset {
i18nService: I18nService, i18nService: I18nService,
syncService: SyncService, syncService: SyncService,
logService: LogService, logService: LogService,
userVerificationService: UserVerificationService,
) { ) {
const result = await UserVerificationDialogComponent.open(dialogService, { const result = await UserVerificationDialogComponent.open(dialogService, {
title: "enrollAccountRecovery", title: "enrollAccountRecovery",
@@ -33,36 +36,42 @@ export class EnrollMasterPasswordReset {
text: "resetPasswordEnrollmentWarning", text: "resetPasswordEnrollmentWarning",
type: "warning", 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") { if (result.userAction === "cancel") {
return; return;
} }
// User confirmed the dialog so check verification success // Enrollment failed
if (!result.verificationSuccess) { if (!result.verificationSuccess) {
// verification failed
return; return;
} }
// Verification succeeded // Enrollment succeeded
try { 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")); platformUtilsService.showToast("success", null, i18nService.t("enrollPasswordResetSuccess"));
await syncService.fullSync(true); await syncService.fullSync(true);
} catch (e) { } catch (e) {
logService.error(e); logService.error(e);

View File

@@ -10,6 +10,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli
import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; 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 { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.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 userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction,
private dialogService: DialogService, private dialogService: DialogService,
private resetPasswordService: OrganizationUserResetPasswordService, private resetPasswordService: OrganizationUserResetPasswordService,
private userVerificationService: UserVerificationService,
) {} ) {}
async ngOnInit() { async ngOnInit() {
@@ -155,6 +157,7 @@ export class OrganizationOptionsComponent implements OnInit, OnDestroy {
this.i18nService, this.i18nService,
this.syncService, this.syncService,
this.logService, this.logService,
this.userVerificationService,
); );
} else { } else {
// Remove reset password // Remove reset password

View File

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

View File

@@ -142,6 +142,31 @@ export class UserVerificationDialogComponent {
* return; * 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( static async open(
dialogService: DialogService, dialogService: DialogService,
@@ -202,6 +227,18 @@ export class UserVerificationDialogComponent {
} }
try { 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, // TODO: once we migrate all user verification scenarios to use this new implementation,
// we should consider refactoring the user verification service handling of the // we should consider refactoring the user verification service handling of the
// OTP and MP flows to not throw errors on verification failure. // OTP and MP flows to not throw errors on verification failure.

View File

@@ -1,3 +1,4 @@
import { VerificationWithSecret } from "@bitwarden/common/auth/types/verification";
import { ButtonType } from "@bitwarden/components"; import { ButtonType } from "@bitwarden/components";
/** /**
@@ -60,12 +61,27 @@ export type UserVerificationDialogOptions = {
*/ */
confirmButtonOptions?: UserVerificationConfirmButtonOptions; confirmButtonOptions?: UserVerificationConfirmButtonOptions;
/** /** The validation method used to verify the secret.
* Indicates whether the verification is only performed client-side. Includes local MP verification, PIN, and Biometrics. *
* Optional. * Possible values:
* **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) *
* - "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> };
}; };
/** /**