From ecb4b2d0b72ce7150571755e7021a1bcd9ca318a Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Mon, 7 Apr 2025 15:04:24 -0700 Subject: [PATCH] refactor(set-change-password): [Auth/PM-17649] Move and test compareInputs validator (#13553) - Move the `compareInputs` validator to `libs/auth` - Add tests for the `compareInputs` validator - Delete the deprecated `InputsFieldMatch` class (inputs-field-match.validator.ts) --- .../inputs-field-match.validator.ts | 167 -------------- libs/auth/src/angular/index.ts | 3 + .../input-password.component.ts | 22 +- .../compare-inputs.validator.spec.ts | 218 ++++++++++++++++++ .../validators/compare-inputs.validator.ts | 134 +++++++++++ 5 files changed, 365 insertions(+), 179 deletions(-) delete mode 100644 libs/angular/src/auth/validators/inputs-field-match.validator.ts create mode 100644 libs/auth/src/angular/validators/compare-inputs.validator.spec.ts create mode 100644 libs/auth/src/angular/validators/compare-inputs.validator.ts diff --git a/libs/angular/src/auth/validators/inputs-field-match.validator.ts b/libs/angular/src/auth/validators/inputs-field-match.validator.ts deleted file mode 100644 index 5f3acd73bc..0000000000 --- a/libs/angular/src/auth/validators/inputs-field-match.validator.ts +++ /dev/null @@ -1,167 +0,0 @@ -import { AbstractControl, UntypedFormGroup, ValidationErrors, ValidatorFn } from "@angular/forms"; - -import { FormGroupControls } from "../../platform/abstractions/form-validation-errors.service"; - -export class InputsFieldMatch { - /** - * Check to ensure two fields do not have the same value - * - * @deprecated Use compareInputs() instead - */ - static validateInputsDoesntMatch(matchTo: string, errorMessage: string): ValidatorFn { - return (control: AbstractControl) => { - if (control.parent && control.parent.controls) { - return control?.value === (control?.parent?.controls as FormGroupControls)[matchTo].value - ? { - inputsMatchError: { - message: errorMessage, - }, - } - : null; - } - - return null; - }; - } - - //check to ensure two fields have the same value - static validateInputsMatch(matchTo: string, errorMessage: string): ValidatorFn { - return (control: AbstractControl) => { - if (control.parent && control.parent.controls) { - return control?.value === (control?.parent?.controls as FormGroupControls)[matchTo].value - ? null - : { - inputsDoesntMatchError: { - message: errorMessage, - }, - }; - } - - return null; - }; - } - - /** - * Checks the formGroup if two fields have the same value and validation is controlled from either field - * - * @deprecated - * Use compareInputs() instead. - * - * For more info on deprecation - * - Do not use untyped `options` object in formBuilder.group() {@link https://angular.dev/api/forms/UntypedFormBuilder} - * - Use formBuilder.group() overload with AbstractControlOptions type instead {@link https://angular.dev/api/forms/AbstractControlOptions} - * - * Remove this method after deprecated instances are replaced - */ - static validateFormInputsMatch(field: string, fieldMatchTo: string, errorMessage: string) { - return (formGroup: UntypedFormGroup) => { - const fieldCtrl = formGroup.controls[field]; - const fieldMatchToCtrl = formGroup.controls[fieldMatchTo]; - - if (fieldCtrl.value !== fieldMatchToCtrl.value) { - fieldMatchToCtrl.setErrors({ - inputsDoesntMatchError: { - message: errorMessage, - }, - }); - } else { - fieldMatchToCtrl.setErrors(null); - } - }; - } - - /** - * Checks whether two form controls do or do not have the same input value (except for empty string values). - * - * - Validation is controlled from either form control. - * - The error message is displayed under controlB by default, but can be set to controlA. - * - * @param validationGoal Whether you want to verify that the form control input values match or do not match - * @param controlNameA The name of the first form control to compare. - * @param controlNameB The name of the second form control to compare. - * @param errorMessage The error message to display if there is an error. This will probably - * be an i18n translated string. - * @param showErrorOn The control under which you want to display the error (default is controlB). - */ - static compareInputs( - validationGoal: "match" | "doNotMatch", - controlNameA: string, - controlNameB: string, - errorMessage: string, - showErrorOn: "controlA" | "controlB" = "controlB", - ): ValidatorFn { - return (control: AbstractControl): ValidationErrors | null => { - const controlA = control.get(controlNameA); - const controlB = control.get(controlNameB); - - if (!controlA || !controlB) { - return null; - } - - const controlThatShowsError = showErrorOn === "controlA" ? controlA : controlB; - - // Don't compare empty strings - if (controlA.value === "" && controlB.value === "") { - return pass(); - } - - const controlValuesMatch = controlA.value === controlB.value; - - if (validationGoal === "match") { - if (controlValuesMatch) { - return pass(); - } else { - return fail(); - } - } - - if (validationGoal === "doNotMatch") { - if (!controlValuesMatch) { - return pass(); - } else { - return fail(); - } - } - - return null; // default return - - function fail() { - controlThatShowsError.setErrors({ - // Preserve any pre-existing errors - ...controlThatShowsError.errors, - // Add new inputMatchError - inputMatchError: { - message: errorMessage, - }, - }); - - return { - inputMatchError: { - message: errorMessage, - }, - }; - } - - function pass(): null { - // Get the current errors object - const errorsObj = controlThatShowsError?.errors; - - if (errorsObj != null) { - // Remove any inputMatchError if it exists, since that is the sole error we are targeting with this validator - if (errorsObj?.inputMatchError) { - delete errorsObj.inputMatchError; - } - - // Check if the errorsObj is now empty - const isEmptyObj = Object.keys(errorsObj).length === 0; - - // If the errorsObj is empty, set errors to null, otherwise set the errors to an object of pre-existing errors (other than inputMatchError) - controlThatShowsError.setErrors(isEmptyObj ? null : errorsObj); - } - - // Return null for this validator - return null; - } - }; - } -} diff --git a/libs/auth/src/angular/index.ts b/libs/auth/src/angular/index.ts index bb2956b756..91d34a3483 100644 --- a/libs/auth/src/angular/index.ts +++ b/libs/auth/src/angular/index.ts @@ -77,3 +77,6 @@ export * from "./two-factor-auth"; // device verification export * from "./new-device-verification/new-device-verification.component"; + +// validators +export * from "./validators/compare-inputs.validator"; diff --git a/libs/auth/src/angular/input-password/input-password.component.ts b/libs/auth/src/angular/input-password/input-password.component.ts index 2f8e5d5b01..bc7f4121fb 100644 --- a/libs/auth/src/angular/input-password/input-password.component.ts +++ b/libs/auth/src/angular/input-password/input-password.component.ts @@ -25,13 +25,11 @@ import { } from "@bitwarden/components"; import { DEFAULT_KDF_CONFIG, KeyService } from "@bitwarden/key-management"; -// FIXME: remove `src` and fix import -// eslint-disable-next-line no-restricted-imports -import { InputsFieldMatch } from "../../../../angular/src/auth/validators/inputs-field-match.validator"; // FIXME: remove `src` and fix import // eslint-disable-next-line no-restricted-imports import { SharedModule } from "../../../../components/src/shared"; import { PasswordCalloutComponent } from "../password-callout/password-callout.component"; +import { compareInputs, ValidationGoal } from "../validators/compare-inputs.validator"; import { PasswordInputResult } from "./password-input-result"; @@ -113,21 +111,21 @@ export class InputPasswordComponent implements OnInit { }, { validators: [ - InputsFieldMatch.compareInputs( - "doNotMatch", + compareInputs( + ValidationGoal.InputsShouldNotMatch, "currentPassword", "newPassword", this.i18nService.t("yourNewPasswordCannotBeTheSameAsYourCurrentPassword"), ), - InputsFieldMatch.compareInputs( - "match", - "newPassword", - "confirmNewPassword", + compareInputs( + ValidationGoal.InputsShouldMatch, + "password", + "confirmedPassword", this.i18nService.t("masterPassDoesntMatch"), ), - InputsFieldMatch.compareInputs( - "doNotMatch", - "newPassword", + compareInputs( + ValidationGoal.InputsShouldNotMatch, + "password", "hint", this.i18nService.t("hintEqualsPassword"), ), diff --git a/libs/auth/src/angular/validators/compare-inputs.validator.spec.ts b/libs/auth/src/angular/validators/compare-inputs.validator.spec.ts new file mode 100644 index 0000000000..b9ce8c1590 --- /dev/null +++ b/libs/auth/src/angular/validators/compare-inputs.validator.spec.ts @@ -0,0 +1,218 @@ +import { FormControl, FormGroup, ValidationErrors } from "@angular/forms"; + +import { compareInputs, ValidationGoal } from "./compare-inputs.validator"; + +describe("compareInputs", () => { + let validationErrorsObj: ValidationErrors; + + beforeEach(() => { + // Use a fresh object for each test so that a mutation in one test doesn't affect another test + validationErrorsObj = { + compareInputsError: { + message: "Custom error message", + }, + }; + }); + + it("should throw an error if compareInputs is not being applied to a FormGroup", () => { + const notAFormGroup = new FormControl("form-control"); + + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + ); + + // Assert + expect(() => validatorFn(notAFormGroup)).toThrow( + "compareInputs only supports validation at the FormGroup level", + ); + }); + + it("should return null if either control is not found", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl("content"), + }); + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", // ctrlB is missing above + "Custom error message", + ); + + const result = validatorFn(formGroup); + + // Assert + expect(result).toBeNull(); + }); + + it("should return null if both controls have empty string values", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl(""), + ctrlB: new FormControl(""), + }); + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + ); + + const result = validatorFn(formGroup); + + // Assert + expect(result).toBeNull(); + }); + + it("should call setErrors() on ctrlB if validation fails", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl("apple"), + ctrlB: new FormControl("banana"), + }); + + const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + ); + + validatorFn(formGroup); + + // Assert + expect(ctrlBSetErrorsSpy).toHaveBeenCalledWith(validationErrorsObj); + }); + + it("should call setErrors() on ctrlA if validation fails and 'showErrorOn' is set to 'controlA'", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl("apple"), + ctrlB: new FormControl("banana"), + }); + + const ctrlASetErrorsSpy = jest.spyOn(formGroup.controls.ctrlA, "setErrors"); + const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + "controlA", + ); + + validatorFn(formGroup); + + // Assert + expect(ctrlASetErrorsSpy).toHaveBeenCalledWith(validationErrorsObj); + expect(ctrlBSetErrorsSpy).not.toHaveBeenCalled(); + }); + + it("should not call setErrors() on ctrlB if validation passes and there is not a pre-existing error on ctrlB", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl("apple"), + ctrlB: new FormControl("apple"), + }); + + const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + ); + + validatorFn(formGroup); + + // Assert + expect(ctrlBSetErrorsSpy).not.toHaveBeenCalled(); + }); + + it("should call setErrors(null) on ctrlB if validation passes and there is a pre-existing error on ctrlB", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl("apple"), + ctrlB: new FormControl("apple"), + }); + + const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); + + formGroup.controls.ctrlB.setErrors(validationErrorsObj); // the pre-existing error + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + ); + + validatorFn(formGroup); + + // Assert + expect(ctrlBSetErrorsSpy).toHaveBeenCalledWith(null); + }); + + const cases = [ + { + expected: null, + goal: ValidationGoal.InputsShouldMatch, + matchStatus: "match", + values: { ctrlA: "apple", ctrlB: "apple" }, + }, + { + expected: "a ValidationErrors object", + goal: ValidationGoal.InputsShouldMatch, + matchStatus: "do not match", + values: { ctrlA: "apple", ctrlB: "banana" }, + }, + { + expected: null, + goal: ValidationGoal.InputsShouldNotMatch, + matchStatus: "do not match", + values: { ctrlA: "apple", ctrlB: "banana" }, + }, + { + expected: "a ValidationErrors object", + goal: ValidationGoal.InputsShouldNotMatch, + matchStatus: "match", + values: { ctrlA: "apple", ctrlB: "apple" }, + }, + ]; + + cases.forEach(({ goal, expected, matchStatus, values }) => { + const goalString = + goal === ValidationGoal.InputsShouldMatch ? "InputsShouldMatch" : "InputsShouldNotMatch"; + + it(`should return ${expected} if the goal is ${goalString} and the inputs ${matchStatus}`, () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl(values.ctrlA), + ctrlB: new FormControl(values.ctrlB), + }); + + // Act + const validatorFn = compareInputs(goal, "ctrlA", "ctrlB", "Custom error message"); + + const result = validatorFn(formGroup); + + // Assert + expect(result).toEqual(expected === null ? null : validationErrorsObj); + }); + }); +}); diff --git a/libs/auth/src/angular/validators/compare-inputs.validator.ts b/libs/auth/src/angular/validators/compare-inputs.validator.ts new file mode 100644 index 0000000000..cb31ac664f --- /dev/null +++ b/libs/auth/src/angular/validators/compare-inputs.validator.ts @@ -0,0 +1,134 @@ +import { AbstractControl, FormGroup, ValidationErrors, ValidatorFn } from "@angular/forms"; + +export enum ValidationGoal { + InputsShouldMatch, + InputsShouldNotMatch, +} + +/** + * A cross-field validator that evaluates whether two form controls do or do + * not have the same input value (except for empty string values). This validator + * gets added to the entire FormGroup, not to an individual FormControl, like so: + * + * > ``` + * > formGroup = new FormGroup({ + * > password: new FormControl(), + * > confirmPassword: new FormControl(), + * > }, + * > { + * > validators: compareInputs(...), + * > }); + * > ``` + * + * Notes: + * - Validation is controlled from either form control. + * - The error message is displayed under controlB by default, but can be set to controlA. + * - For more info on custom validators and cross-field validation: + * - https://v18.angular.dev/guide/forms/form-validation#defining-custom-validators + * - https://v18.angular.dev/guide/forms/form-validation#cross-field-validation + * + * @param validationGoal Whether you want to verify that the form controls do or do not have matching input values. + * @param controlNameA The name of the first form control to compare. + * @param controlNameB The name of the second form control to compare. + * @param errorMessage The error message to display if there is an error. This will probably + * be an i18n translated string. + * @param showErrorOn The control under which you want to display the error (default is controlB). + * + * @returns A validator function that can be used on a FormGroup. + */ +export function compareInputs( + validationGoal: ValidationGoal, + controlNameA: string, + controlNameB: string, + errorMessage: string, + showErrorOn: "controlA" | "controlB" = "controlB", +): ValidatorFn { + /** + * Documentation for the inner ValidatorFn that gets returned: + * + * @param formGroup The AbstractControl that we want to perform validation on. In this case we + * perform validation on the FormGroup, which is a subclass of AbstractControl. + * The reason we validate at the FormGroup level and not at the FormControl level + * is because we want to compare two child FormControls in a single validator, so + * we use the FormGroup as the common ancestor. + * + * @returns A ValidationErrors object if the validation fails, or null if the validation passes. + */ + return (formGroup: AbstractControl): ValidationErrors | null => { + if (!(formGroup instanceof FormGroup)) { + throw new Error("compareInputs only supports validation at the FormGroup level"); + } + + const controlA = formGroup.get(controlNameA); + const controlB = formGroup.get(controlNameB); + + if (!controlA || !controlB) { + return null; + } + + const controlThatShowsError = showErrorOn === "controlA" ? controlA : controlB; + + // Don't compare empty strings + if (controlA.value === "" && controlB.value === "") { + return pass(); + } + + const controlValuesMatch = controlA.value === controlB.value; + + if (validationGoal === ValidationGoal.InputsShouldMatch) { + if (controlValuesMatch) { + return pass(); + } else { + return fail(); + } + } + + if (validationGoal === ValidationGoal.InputsShouldNotMatch) { + if (!controlValuesMatch) { + return pass(); + } else { + return fail(); + } + } + + return null; // default return + + function fail() { + controlThatShowsError.setErrors({ + // Preserve any pre-existing errors + ...(controlThatShowsError.errors || {}), + // Add new compareInputsError + compareInputsError: { + message: errorMessage, + }, + }); + + return { + compareInputsError: { + message: errorMessage, + }, + }; + } + + function pass(): null { + // Get the current errors object + const errorsObj = controlThatShowsError?.errors; + + if (errorsObj != null) { + // Remove any compareInputsError if it exists, since that is the sole error we are targeting with this validator + if (errorsObj?.compareInputsError) { + delete errorsObj.compareInputsError; + } + + // Check if the errorsObj is now empty + const isEmptyObj = Object.keys(errorsObj).length === 0; + + // If the errorsObj is empty, set errors to null, otherwise set the errors to an object of pre-existing errors (other than compareInputsError) + controlThatShowsError.setErrors(isEmptyObj ? null : errorsObj); + } + + // Return null for this validator + return null; + } + }; +}