Skip to content
This repository has been archived by the owner on Jan 4, 2025. It is now read-only.

Commit

Permalink
Merge pull request #100 from johnnygerard/refactor
Browse files Browse the repository at this point in the history
refactor: improve password validation
  • Loading branch information
johnnygerard authored Nov 10, 2024
2 parents 047ddf6 + 9b4d074 commit 8986c7a
Show file tree
Hide file tree
Showing 14 changed files with 216 additions and 163 deletions.
2 changes: 2 additions & 0 deletions client/e2e/register-user.function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export const registerUser = async (

await usernameInput.fill(username);
await passwordInput.fill(password);
// Wait for asynchronous validation to complete
await expect(passwordInput).toHaveClass(/ng-valid/);
await page.keyboard.press("Enter");

await responsePromise.then((response) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,23 @@ import {
ChangeDetectionStrategy,
Component,
computed,
DestroyRef,
inject,
input,
OnInit,
signal,
} from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { FormControl, ReactiveFormsModule } from "@angular/forms";
import { MatButtonModule } from "@angular/material/button";
import { MatFormFieldModule } from "@angular/material/form-field";
import { MatIconModule } from "@angular/material/icon";
import { MatInputModule } from "@angular/material/input";
import { MatTooltipModule } from "@angular/material/tooltip";
import { PASSWORD_MAX_LENGTH } from "_server/constants/password";
import { zxcvbnDefaultResult } from "_server/constants/zxcvbn-default-result";
import { PasswordErrorPipe } from "../../pipes/password-error.pipe";
import { PasswordStrengthService } from "../../services/password-strength.service";

@Component({
selector: "app-password-field",
Expand All @@ -33,7 +39,7 @@ import { PasswordErrorPipe } from "../../pipes/password-error.pipe";
// https://stackoverflow.com/questions/56584244/can-the-angular-material-errorstatematcher-detect-when-a-parent-form-is-submitte
changeDetection: ChangeDetectionStrategy.Default,
})
export class PasswordFieldComponent {
export class PasswordFieldComponent implements OnInit {
readonly PASSWORD_MAX_LENGTH = PASSWORD_MAX_LENGTH;
autocomplete = input.required<"new-password" | "current-password">();
isPasswordVisible = signal(false);
Expand All @@ -42,6 +48,24 @@ export class PasswordFieldComponent {
() => `${this.isPasswordVisible() ? "Hide" : "Show"} password`,
);

#destroyRef = inject(DestroyRef);
#strength = inject(PasswordStrengthService);

/**
* Reset the password strength meter when the password input is empty.
*
* This is necessary because the "required" validator prevents the password
* strength async validator from running when the input is empty.
* @see https://angular.dev/guide/forms/form-validation#creating-asynchronous-validators
*/
ngOnInit(): void {
this.passwordControl()
.valueChanges.pipe(takeUntilDestroyed(this.#destroyRef))
.subscribe((value) => {
if (value === "") this.#strength.result$.next(zxcvbnDefaultResult);
});
}

togglePasswordVisibility(event: MouseEvent): void {
this.isPasswordVisible.update((value) => !value);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import {
computed,
inject,
} from "@angular/core";
import { toSignal } from "@angular/core/rxjs-interop";
import { MatIconModule } from "@angular/material/icon";
import { MatTooltipModule } from "@angular/material/tooltip";
import { zxcvbnDefaultResult } from "_server/constants/zxcvbn-default-result";
import { PasswordStrengthService } from "../../services/password-strength.service";

@Component({
Expand All @@ -17,7 +19,10 @@ import { PasswordStrengthService } from "../../services/password-strength.servic
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class PasswordStrengthMeterComponent {
result = inject(PasswordStrengthService).result;
result = toSignal(inject(PasswordStrengthService).result$, {
initialValue: zxcvbnDefaultResult,
});

score = computed(() => this.result().score);
ariaValueText = computed(() => this.#scoreLabel);
isPasswordValid = computed(() => this.score() >= 3);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,18 @@ <h1 class="mat-title-medium">Create your account</h1>
<mat-label>Username</mat-label>
<input
[attr.maxlength]="USERNAME_MAX_LENGTH"
appUsernameValidator
autocomplete="username"
data-testid="username"
formControlName="username"
matInput
spellcheck="false"
type="text"
/>
<mat-error
>{{ form.controls.username.errors | usernameError }}
</mat-error>
<mat-error>{{ usernameControl.errors | usernameError }}</mat-error>
</mat-form-field>

<app-password-field
[passwordControl]="form.controls.password"
[passwordControl]="passwordControl"
autocomplete="new-password"
/>
<app-password-strength-meter />
Expand Down
116 changes: 35 additions & 81 deletions client/src/app/components/register-form/register-form.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ import {
ChangeDetectionStrategy,
Component,
DestroyRef,
effect,
inject,
signal,
} from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import {
FormBuilder,
FormControl,
NonNullableFormBuilder,
ReactiveFormsModule,
Validators,
} from "@angular/forms";
Expand All @@ -21,22 +20,20 @@ import { MatIconModule } from "@angular/material/icon";
import { MatInputModule } from "@angular/material/input";
import { Router, RouterLink } from "@angular/router";
import { CONFLICT } from "_server/constants/http-status-code";
import {
PASSWORD_MAX_LENGTH,
ZXCVBN_MIN_SCORE,
} from "_server/constants/password";
import { PASSWORD_MAX_LENGTH } from "_server/constants/password";
import { ClientSession } from "_server/types/client-session";
import {
USERNAME_MAX_LENGTH,
USERNAME_MIN_LENGTH,
} from "_server/validation/username";
import { finalize } from "rxjs";
import { UsernameValidatorDirective } from "../../directives/username-validator.directive";
import { UsernameErrorPipe } from "../../pipes/username-error.pipe";
import { NotificationService } from "../../services/notification.service";
import { PasswordStrengthService } from "../../services/password-strength.service";
import { SessionService } from "../../services/session.service";
import { UserMessage } from "../../types/user-message.enum";
import { passwordValidatorFactory } from "../../validators/password-validator-factory";
import { usernamePatternValidator } from "../../validators/username-pattern-validator";
import { PasswordFieldComponent } from "../password-field/password-field.component";
import { PasswordStrengthMeterComponent } from "../password-strength-meter/password-strength-meter.component";

Expand All @@ -54,93 +51,59 @@ import { PasswordStrengthMeterComponent } from "../password-strength-meter/passw
ReactiveFormsModule,
RouterLink,
UsernameErrorPipe,
UsernameValidatorDirective,
],
templateUrl: "./register-form.component.html",
styleUrl: "./register-form.component.scss",
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class RegisterFormComponent {
readonly USERNAME_MAX_LENGTH = USERNAME_MAX_LENGTH;
form = inject(FormBuilder).group({
username: [
"",
[
Validators.required,
Validators.minLength(USERNAME_MIN_LENGTH),
Validators.maxLength(USERNAME_MAX_LENGTH),
],
isLoading = signal(false);

usernameControl = new FormControl("", {
nonNullable: true,
validators: [
Validators.required,
Validators.minLength(USERNAME_MIN_LENGTH),
Validators.maxLength(USERNAME_MAX_LENGTH),
usernamePatternValidator,
],
password: [
"",
[Validators.required, Validators.maxLength(PASSWORD_MAX_LENGTH)],
});

passwordControl = new FormControl("", {
nonNullable: true,
validators: [
Validators.required,
Validators.maxLength(PASSWORD_MAX_LENGTH),
],
asyncValidators: passwordValidatorFactory(
inject(PasswordStrengthService),
this.usernameControl,
),
});

form = inject(NonNullableFormBuilder).group({
username: this.usernameControl,
password: this.passwordControl,
});
isLoading = signal(false);

#destroyRef = inject(DestroyRef);
#http = inject(HttpClient);
#notifier = inject(NotificationService);
#router = inject(Router);
#session = inject(SessionService);
#passwordStrength = inject(PasswordStrengthService);
#shouldResubmit = false;

constructor() {
// Send form inputs to password strength validation worker service
this.form.valueChanges
.pipe(
takeUntilDestroyed(),
finalize(() => {
// Reset strength meter when the component is destroyed
this.#passwordStrength.validate("", []);
}),
)
.subscribe((next) => {
const { username, password } = next;

if (typeof password !== "string") {
return;
}
const userInputs = username ? [username] : [];
this.#passwordStrength.validate(password, userInputs);
// Re-evaluate the password strength whenever the username changes
this.usernameControl.valueChanges
.pipe(takeUntilDestroyed())
.subscribe(() => {
this.passwordControl.updateValueAndValidity();
});

// Listen for password strength validation results from the worker service
// and set validation errors on the password control
effect(
(): void => {
try {
const result = this.#passwordStrength.result();
const control = this.form.controls.password;

if (result.score >= ZXCVBN_MIN_SCORE) {
this.#removeStrengthError(control);
return;
}

// Add validation error
control.setErrors({
...control.errors,
strength: result.feedback.warning || "Vulnerable password",
});
} finally {
if (this.#shouldResubmit) {
this.#shouldResubmit = false;
this.onSubmit();
}
}
},
{ allowSignalWrites: true },
);
}

onSubmit(): void {
if (this.form.invalid || this.isLoading()) return;
if (this.#passwordStrength.isWorkerBusy()) {
this.#shouldResubmit = true;
return;
}
if (!this.form.valid || this.isLoading()) return;
this.isLoading.set(true);

this.#http
Expand All @@ -167,13 +130,4 @@ export class RegisterFormComponent {
},
});
}

#removeStrengthError(control: FormControl): void {
if (control.errors === null) return;

delete control.errors["strength"];
control.setErrors(
Object.keys(control.errors).length ? control.errors : null,
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ <h1 class="mat-title-medium">Sign in to your account</h1>
<mat-label>Username</mat-label>
<input
[attr.maxlength]="USERNAME_MAX_LENGTH"
appUsernameValidator
autocomplete="username"
data-testid="username"
formControlName="username"
Expand Down
13 changes: 2 additions & 11 deletions client/src/app/components/sign-in-form/sign-in-form.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,8 @@ import { UNAUTHORIZED } from "_server/constants/http-status-code";
import { PASSWORD_MAX_LENGTH } from "_server/constants/password";
import { ApiError } from "_server/types/api-error.enum";
import { ClientSession } from "_server/types/client-session";
import {
USERNAME_MAX_LENGTH,
USERNAME_MIN_LENGTH,
} from "_server/validation/username";
import { USERNAME_MAX_LENGTH } from "_server/validation/username";
import { finalize } from "rxjs";
import { UsernameValidatorDirective } from "../../directives/username-validator.directive";
import { UsernameErrorPipe } from "../../pipes/username-error.pipe";
import { NotificationService } from "../../services/notification.service";
import { SessionService } from "../../services/session.service";
Expand All @@ -43,7 +39,6 @@ import { PasswordFieldComponent } from "../password-field/password-field.compone
ReactiveFormsModule,
RouterLink,
UsernameErrorPipe,
UsernameValidatorDirective,
],
templateUrl: "./sign-in-form.component.html",
styleUrl: "./sign-in-form.component.scss",
Expand All @@ -54,11 +49,7 @@ export class SignInFormComponent {
form = inject(FormBuilder).group({
username: [
"",
[
Validators.required,
Validators.minLength(USERNAME_MIN_LENGTH),
Validators.maxLength(USERNAME_MAX_LENGTH),
],
[Validators.required, Validators.maxLength(USERNAME_MAX_LENGTH)],
],
password: [
"",
Expand Down
29 changes: 0 additions & 29 deletions client/src/app/directives/username-validator.directive.ts

This file was deleted.

Loading

0 comments on commit 8986c7a

Please sign in to comment.