PM-4945 Update Two Factor verify dialog (#8580)

* PM-4945 Update Two Factor verify dialog * PM-4945 Addressed review comments * PM-4945 Removed legacy User verification component and used new one
2024-12-21 16:18:28 +01:00 · 2024-04-26 18:24:48 +05:30 · 2024-04-26 18:24:48 +05:30 · 2fa4c6e4f9
commit 2fa4c6e4f9
parent 11ba8d188d
11 changed files with 142 additions and 87 deletions
--- a/apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts
+++ b/apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts
@ -10,6 +10,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli
 import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type";
 import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
 import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
+import { DialogService } from "@bitwarden/components";

 import { TwoFactorDuoComponent } from "../../../auth/settings/two-factor-duo.component";
 import { TwoFactorSetupComponent as BaseTwoFactorSetupComponent } from "../../../auth/settings/two-factor-setup.component";
@ -22,6 +23,7 @@ import { TwoFactorSetupComponent as BaseTwoFactorSetupComponent } from "../../..
 export class TwoFactorSetupComponent extends BaseTwoFactorSetupComponent {
  tabbedHeader = false;
  constructor(
+    dialogService: DialogService,
    apiService: ApiService,
    modalService: ModalService,
    messagingService: MessagingService,
@ -31,6 +33,7 @@ export class TwoFactorSetupComponent extends BaseTwoFactorSetupComponent {
    billingAccountProfileStateService: BillingAccountProfileStateService,
  ) {
    super(
+      dialogService,
      apiService,
      modalService,
      messagingService,
--- a/apps/web/src/app/auth/settings/two-factor-authenticator.component.html
+++ b/apps/web/src/app/auth/settings/two-factor-authenticator.component.html
@ -15,13 +15,6 @@
          <span aria-hidden="true">&times;</span>
        </button>
      </div>
-      <app-two-factor-verify
-        [organizationId]="organizationId"
-        [type]="type"
-        (onAuthed)="auth($any($event))"
-        *ngIf="!authed"
-      >
-      </app-two-factor-verify>
      <form
        #form
        (ngSubmit)="submit()"
--- a/apps/web/src/app/auth/settings/two-factor-duo.component.html
+++ b/apps/web/src/app/auth/settings/two-factor-duo.component.html
@ -15,13 +15,6 @@
          <span aria-hidden="true">&times;</span>
        </button>
      </div>
-      <app-two-factor-verify
-        [organizationId]="organizationId"
-        [type]="type"
-        (onAuthed)="auth($any($event))"
-        *ngIf="!authed"
-      >
-      </app-two-factor-verify>
      <form
        #form
        (ngSubmit)="submit()"
--- a/apps/web/src/app/auth/settings/two-factor-email.component.html
+++ b/apps/web/src/app/auth/settings/two-factor-email.component.html
@ -15,13 +15,6 @@
          <span aria-hidden="true">&times;</span>
        </button>
      </div>
-      <app-two-factor-verify
-        [organizationId]="organizationId"
-        [type]="type"
-        (onAuthed)="auth($any($event))"
-        *ngIf="!authed"
-      >
-      </app-two-factor-verify>
      <form
        #form
        (ngSubmit)="submit()"
--- a/apps/web/src/app/auth/settings/two-factor-recovery.component.html
+++ b/apps/web/src/app/auth/settings/two-factor-recovery.component.html
@ -15,8 +15,6 @@
          <span aria-hidden="true">&times;</span>
        </button>
      </div>
-      <app-two-factor-verify [type]="type" (onAuthed)="auth($event)" *ngIf="!authed">
-      </app-two-factor-verify>
      <ng-container *ngIf="authed">
        <div class="modal-body text-center">
          <ng-container *ngIf="code">
--- a/apps/web/src/app/auth/settings/two-factor-setup.component.ts
+++ b/apps/web/src/app/auth/settings/two-factor-setup.component.ts
@ -1,5 +1,5 @@
 import { Component, OnDestroy, OnInit, Type, ViewChild, ViewContainerRef } from "@angular/core";
-import { firstValueFrom, Observable, Subject, takeUntil } from "rxjs";
+import { firstValueFrom, lastValueFrom, Observable, Subject, takeUntil } from "rxjs";

 import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref";
 import { ModalService } from "@bitwarden/angular/services/modal.service";
@ -8,15 +8,23 @@ 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 { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type";
+import { TwoFactorAuthenticatorResponse } from "@bitwarden/common/auth/models/response/two-factor-authenticator.response";
+import { TwoFactorDuoResponse } from "@bitwarden/common/auth/models/response/two-factor-duo.response";
+import { TwoFactorEmailResponse } from "@bitwarden/common/auth/models/response/two-factor-email.response";
+import { TwoFactorWebAuthnResponse } from "@bitwarden/common/auth/models/response/two-factor-web-authn.response";
+import { TwoFactorYubiKeyResponse } from "@bitwarden/common/auth/models/response/two-factor-yubi-key.response";
 import { TwoFactorProviders } from "@bitwarden/common/auth/services/two-factor.service";
+import { AuthResponse } from "@bitwarden/common/auth/types/auth-response";
 import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
 import { ProductType } from "@bitwarden/common/enums";
 import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
+import { DialogService } from "@bitwarden/components";

 import { TwoFactorAuthenticatorComponent } from "./two-factor-authenticator.component";
 import { TwoFactorDuoComponent } from "./two-factor-duo.component";
 import { TwoFactorEmailComponent } from "./two-factor-email.component";
 import { TwoFactorRecoveryComponent } from "./two-factor-recovery.component";
+import { TwoFactorVerifyComponent } from "./two-factor-verify.component";
 import { TwoFactorWebAuthnComponent } from "./two-factor-webauthn.component";
 import { TwoFactorYubiKeyComponent } from "./two-factor-yubikey.component";

@ -52,6 +60,7 @@ export class TwoFactorSetupComponent implements OnInit, OnDestroy {
  private twoFactorAuthPolicyAppliesToActiveUser: boolean;

  constructor(
+    protected dialogService: DialogService,
    protected apiService: ApiService,
    protected modalService: ModalService,
    protected messagingService: MessagingService,
@ -114,50 +123,82 @@ export class TwoFactorSetupComponent implements OnInit, OnDestroy {
    this.loading = false;
  }

+  async callTwoFactorVerifyDialog(type?: TwoFactorProviderType) {
+    const twoFactorVerifyDialogRef = TwoFactorVerifyComponent.open(this.dialogService, {
+      data: { type: type, organizationId: this.organizationId },
+    });
+    return await lastValueFrom(twoFactorVerifyDialogRef.closed);
+  }
+
  async manage(type: TwoFactorProviderType) {
    switch (type) {
      case TwoFactorProviderType.Authenticator: {
+        const result: AuthResponse<TwoFactorAuthenticatorResponse> =
+          await this.callTwoFactorVerifyDialog(type);
+        if (!result) {
+          return;
+        }
        const authComp = await this.openModal(
          this.authenticatorModalRef,
          TwoFactorAuthenticatorComponent,
        );
-        // eslint-disable-next-line rxjs-angular/prefer-takeuntil
-        authComp.onUpdated.subscribe((enabled: boolean) => {
+        await authComp.auth(result);
+        authComp.onUpdated.pipe(takeUntil(this.destroy$)).subscribe((enabled: boolean) => {
          this.updateStatus(enabled, TwoFactorProviderType.Authenticator);
        });
        break;
      }
      case TwoFactorProviderType.Yubikey: {
+        const result: AuthResponse<TwoFactorYubiKeyResponse> =
+          await this.callTwoFactorVerifyDialog(type);
+        if (!result) {
+          return;
+        }
        const yubiComp = await this.openModal(this.yubikeyModalRef, TwoFactorYubiKeyComponent);
-        // eslint-disable-next-line rxjs-angular/prefer-takeuntil
-        yubiComp.onUpdated.subscribe((enabled: boolean) => {
+        yubiComp.auth(result);
+        yubiComp.onUpdated.pipe(takeUntil(this.destroy$)).subscribe((enabled: boolean) => {
          this.updateStatus(enabled, TwoFactorProviderType.Yubikey);
        });
        break;
      }
      case TwoFactorProviderType.Duo: {
+        const result: AuthResponse<TwoFactorDuoResponse> =
+          await this.callTwoFactorVerifyDialog(type);
+        if (!result) {
+          return;
+        }
        const duoComp = await this.openModal(this.duoModalRef, TwoFactorDuoComponent);
-        // eslint-disable-next-line rxjs-angular/prefer-takeuntil
-        duoComp.onUpdated.subscribe((enabled: boolean) => {
+        duoComp.auth(result);
+        duoComp.onUpdated.pipe(takeUntil(this.destroy$)).subscribe((enabled: boolean) => {
          this.updateStatus(enabled, TwoFactorProviderType.Duo);
        });
        break;
      }
      case TwoFactorProviderType.Email: {
+        const result: AuthResponse<TwoFactorEmailResponse> =
+          await this.callTwoFactorVerifyDialog(type);
+        if (!result) {
+          return;
+        }
        const emailComp = await this.openModal(this.emailModalRef, TwoFactorEmailComponent);
-        // eslint-disable-next-line rxjs-angular/prefer-takeuntil
-        emailComp.onUpdated.subscribe((enabled: boolean) => {
+        await emailComp.auth(result);
+        emailComp.onUpdated.pipe(takeUntil(this.destroy$)).subscribe((enabled: boolean) => {
          this.updateStatus(enabled, TwoFactorProviderType.Email);
        });
        break;
      }
      case TwoFactorProviderType.WebAuthn: {
+        const result: AuthResponse<TwoFactorWebAuthnResponse> =
+          await this.callTwoFactorVerifyDialog(type);
+        if (!result) {
+          return;
+        }
        const webAuthnComp = await this.openModal(
          this.webAuthnModalRef,
          TwoFactorWebAuthnComponent,
        );
-        // eslint-disable-next-line rxjs-angular/prefer-takeuntil
-        webAuthnComp.onUpdated.subscribe((enabled: boolean) => {
+        webAuthnComp.auth(result);
+        webAuthnComp.onUpdated.pipe(takeUntil(this.destroy$)).subscribe((enabled: boolean) => {
          this.updateStatus(enabled, TwoFactorProviderType.WebAuthn);
        });
        break;
@ -167,10 +208,12 @@ export class TwoFactorSetupComponent implements OnInit, OnDestroy {
    }
  }

-  recoveryCode() {
-    // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-    // eslint-disable-next-line @typescript-eslint/no-floating-promises
-    this.openModal(this.recoveryModalRef, TwoFactorRecoveryComponent);
+  async recoveryCode() {
+    const result = await this.callTwoFactorVerifyDialog(-1 as TwoFactorProviderType);
+    if (result) {
+      const recoverComp = await this.openModal(this.recoveryModalRef, TwoFactorRecoveryComponent);
+      recoverComp.auth(result);
+    }
  }

  async premiumRequired() {
--- a/apps/web/src/app/auth/settings/two-factor-verify.component.html
+++ b/apps/web/src/app/auth/settings/two-factor-verify.component.html
@ -1,15 +1,23 @@
-<form #form (ngSubmit)="submit()" [appApiAction]="formPromise" ngNativeValidate>
-  <div class="modal-body">
-    <app-user-verification [(ngModel)]="secret" ngDefaultControl name="secret">
-    </app-user-verification>
-  </div>
-  <div class="modal-footer">
-    <button type="submit" class="btn btn-primary btn-submit" [disabled]="form.loading">
-      <i class="bwi bwi-spinner bwi-spin" title="{{ 'loading' | i18n }}" aria-hidden="true"></i>
-      <span>{{ "continue" | i18n }}</span>
-    </button>
-    <button type="button" class="btn btn-outline-secondary" data-dismiss="modal">
-      {{ "close" | i18n }}
-    </button>
-  </div>
+<form [formGroup]="formGroup" [bitSubmit]="submit">
+  <bit-dialog dialogSize="default">
+    <span bitDialogTitle>
+      {{ "twoStepLogin" | i18n }}
+      <small class="tw-text-muted">{{ dialogTitle }}</small>
+    </span>
+    <ng-container bitDialogContent>
+      <app-user-verification-form-input
+        formControlName="secret"
+        ngDefaultControl
+        name="secret"
+      ></app-user-verification-form-input>
+    </ng-container>
+    <ng-container bitDialogFooter>
+      <button bitButton bitFormButton type="submit" buttonType="primary">
+        {{ "continue" | i18n }}
+      </button>
+      <button bitButton type="button" buttonType="secondary" bitDialogClose>
+        {{ "close" | i18n }}
+      </button>
+    </ng-container>
+  </bit-dialog>
 </form>
--- a/apps/web/src/app/auth/settings/two-factor-verify.component.ts
+++ b/apps/web/src/app/auth/settings/two-factor-verify.component.ts
@ -1,4 +1,6 @@
-import { Component, EventEmitter, Input, Output } from "@angular/core";
+import { DIALOG_DATA, DialogConfig, DialogRef } from "@angular/cdk/dialog";
+import { Component, EventEmitter, Inject, Output } from "@angular/core";
+import { FormControl, FormGroup } from "@angular/forms";

 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
@ -8,46 +10,74 @@ import { SecretVerificationRequest } from "@bitwarden/common/auth/models/request
 import { AuthResponse } from "@bitwarden/common/auth/types/auth-response";
 import { TwoFactorResponse } from "@bitwarden/common/auth/types/two-factor-response";
 import { Verification } from "@bitwarden/common/auth/types/verification";
-import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { DialogService } from "@bitwarden/components";
+
+type TwoFactorVerifyDialogData = {
+  type: TwoFactorProviderType;
+  organizationId: string;
+};

@Component({
  selector: "app-two-factor-verify",
  templateUrl: "two-factor-verify.component.html",
 })
 export class TwoFactorVerifyComponent {
-  @Input() type: TwoFactorProviderType;
-  @Input() organizationId: string;
+  type: TwoFactorProviderType;
+  organizationId: string;
  @Output() onAuthed = new EventEmitter<AuthResponse<TwoFactorResponse>>();

-  secret: Verification;
  formPromise: Promise<TwoFactorResponse>;

+  protected formGroup = new FormGroup({
+    secret: new FormControl<Verification | null>(null),
+  });
+
  constructor(
+    @Inject(DIALOG_DATA) protected data: TwoFactorVerifyDialogData,
+    private dialogRef: DialogRef,
    private apiService: ApiService,
-    private logService: LogService,
+    private i18nService: I18nService,
    private userVerificationService: UserVerificationService,
-  ) {}
+  ) {
+    this.type = data.type;
+    this.organizationId = data.organizationId;
+  }

-  async submit() {
+  submit = async () => {
    let hashedSecret: string;
-
-    try {
-      this.formPromise = this.userVerificationService.buildRequest(this.secret).then((request) => {
+    this.formPromise = this.userVerificationService
+      .buildRequest(this.formGroup.value.secret)
+      .then((request) => {
        hashedSecret =
-          this.secret.type === VerificationType.MasterPassword
+          this.formGroup.value.secret.type === VerificationType.MasterPassword
            ? request.masterPasswordHash
            : request.otp;
        return this.apiCall(request);
      });

-      const response = await this.formPromise;
-      this.onAuthed.emit({
-        response: response,
-        secret: hashedSecret,
-        verificationType: this.secret.type,
-      });
-    } catch (e) {
-      this.logService.error(e);
+    const response = await this.formPromise;
+    this.dialogRef.close({
+      response: response,
+      secret: hashedSecret,
+      verificationType: this.formGroup.value.secret.type,
+    });
+  };
+
+  get dialogTitle(): string {
+    switch (this.type) {
+      case -1 as TwoFactorProviderType:
+        return this.i18nService.t("recoveryCodeTitle");
+      case TwoFactorProviderType.Duo:
+        return "Duo";
+      case TwoFactorProviderType.Email:
+        return this.i18nService.t("emailTitle");
+      case TwoFactorProviderType.WebAuthn:
+        return this.i18nService.t("webAuthnTitle");
+      case TwoFactorProviderType.Authenticator:
+        return this.i18nService.t("authenticatorAppTitle");
+      case TwoFactorProviderType.Yubikey:
+        return "Yubikey";
    }
  }

@ -72,4 +102,8 @@ export class TwoFactorVerifyComponent {
        return this.apiService.getTwoFactorYubiKey(request);
    }
  }
+
+  static open(dialogService: DialogService, config: DialogConfig<TwoFactorVerifyDialogData>) {
+    return dialogService.open<AuthResponse<any>>(TwoFactorVerifyComponent, config);
+  }
 }
--- a/apps/web/src/app/auth/settings/two-factor-webauthn.component.html
+++ b/apps/web/src/app/auth/settings/two-factor-webauthn.component.html
@ -15,13 +15,6 @@
          <span aria-hidden="true">&times;</span>
        </button>
      </div>
-      <app-two-factor-verify
-        [organizationId]="organizationId"
-        [type]="type"
-        (onAuthed)="auth($any($event))"
-        *ngIf="!authed"
-      >
-      </app-two-factor-verify>
      <form
        #form
        (ngSubmit)="submit()"
--- a/apps/web/src/app/auth/settings/two-factor-yubikey.component.html
+++ b/apps/web/src/app/auth/settings/two-factor-yubikey.component.html
@ -15,13 +15,6 @@
          <span aria-hidden="true">&times;</span>
        </button>
      </div>
-      <app-two-factor-verify
-        [organizationId]="organizationId"
-        [type]="type"
-        (onAuthed)="auth($any($event))"
-        *ngIf="!authed"
-      >
-      </app-two-factor-verify>
      <form
        #form
        (ngSubmit)="submit()"
--- a/apps/web/src/app/shared/loose-components.module.ts
+++ b/apps/web/src/app/shared/loose-components.module.ts
@ -1,6 +1,9 @@
 import { NgModule } from "@angular/core";

-import { PasswordCalloutComponent } from "@bitwarden/auth/angular";
+import {
+  PasswordCalloutComponent,
+  UserVerificationFormInputComponent,
+} from "@bitwarden/auth/angular";
 import { LayoutComponent, NavigationModule } from "@bitwarden/components";

 import { OrganizationLayoutComponent } from "../admin-console/organizations/layouts/organization-layout.component";
@ -106,6 +109,7 @@ import { SharedModule } from "./shared.module";
    OrganizationBadgeModule,
    PipesModule,
    PasswordCalloutComponent,
+    UserVerificationFormInputComponent,
    DangerZoneComponent,
    LayoutComponent,
    NavigationModule,