[PM-2014] fix: move error handling to components

After discussing it with Jake we decided that following convention was best.
2025-01-09 19:28:06 +01:00 · 2023-05-16 15:16:24 +02:00 · 2023-05-16 15:16:24 +02:00 · 72f10bab65
commit 72f10bab65
parent 38adb9ee0a
4 changed files with 76 additions and 151 deletions
--- a/apps/web/src/app/auth/core/services/webauthn/webauthn.service.spec.ts
+++ b/apps/web/src/app/auth/core/services/webauthn/webauthn.service.spec.ts
@ -1,20 +1,12 @@
 import { mock, MockProxy } from "jest-mock-extended";

-import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
-import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
-import { VerificationType } from "@bitwarden/common/auth/enums/verification-type";
-import { Verification } from "@bitwarden/common/types/verification";
-
 import { CredentialCreateOptionsView } from "../../views/credential-create-options.view";

-import { CredentialCreateOptionsResponse } from "./response/credential-create-options.response";
 import { WebauthnApiService } from "./webauthn-api.service";
 import { WebauthnService } from "./webauthn.service";

 describe("WebauthnService", () => {
  let apiService!: MockProxy<WebauthnApiService>;
-  let platformUtilsService!: MockProxy<PlatformUtilsService>;
-  let i18nService!: MockProxy<I18nService>;
  let credentials: MockProxy<CredentialsContainer>;
  let webauthnService!: WebauthnService;

@ -23,39 +15,8 @@ describe("WebauthnService", () => {
    window.PublicKeyCredential = class {} as any;
    window.AuthenticatorAttestationResponse = class {} as any;
    apiService = mock<WebauthnApiService>();
-    platformUtilsService = mock<PlatformUtilsService>();
-    i18nService = mock<I18nService>();
    credentials = mock<CredentialsContainer>();
-    webauthnService = new WebauthnService(
-      apiService,
-      platformUtilsService,
-      i18nService,
-      credentials
-    );
-  });
-
-  describe("getNewCredentialOptions", () => {
-    it("should return undefined and show toast when api service call throws", async () => {
-      apiService.getCredentialCreateOptions.mockRejectedValue(new Error("Mock error"));
-      const verification = createVerification();
-
-      const result = await webauthnService.getCredentialCreateOptions(verification);
-
-      expect(result).toBeUndefined();
-      expect(platformUtilsService.showToast).toHaveBeenCalled();
-    });
-
-    it("should return options when api service call is successfull", async () => {
-      const options = Symbol() as any;
-      const token = Symbol() as any;
-      const response = { options, token } as CredentialCreateOptionsResponse;
-      apiService.getCredentialCreateOptions.mockResolvedValue(response);
-      const verification = createVerification();
-
-      const result = await webauthnService.getCredentialCreateOptions(verification);
-
-      expect(result).toEqual({ options, token });
-    });
+    webauthnService = new WebauthnService(apiService, credentials);
  });

  describe("createCredential", () => {
@ -78,40 +39,8 @@ describe("WebauthnService", () => {
      expect(result).toBe(credential);
    });
  });
-
-  describe("saveCredential", () => {
-    it("should return false and show toast when api service call throws", async () => {
-      apiService.saveCredential.mockRejectedValue(new Error("Mock error"));
-      const options = createCredentialCreateOptions();
-      const deviceResponse = Symbol() as any;
-
-      const result = await webauthnService.saveCredential(options, deviceResponse, "name");
-
-      expect(result).toBe(false);
-      expect(platformUtilsService.showToast).toHaveBeenCalled();
-    });
-
-    it("should return true when api service call is successfull", async () => {
-      apiService.saveCredential.mockResolvedValue(true);
-      const options = createCredentialCreateOptions();
-      const deviceResponse = createDeviceResponse();
-
-      const result = await webauthnService.saveCredential(options, deviceResponse, "name");
-
-      expect(result).toBe(true);
-      expect(apiService.saveCredential).toHaveBeenCalled();
-      expect(platformUtilsService.showToast).toHaveBeenCalled();
-    });
-  });
 });

-function createVerification(): Verification {
-  return {
-    type: VerificationType.MasterPassword,
-    secret: "secret",
-  };
-}
-
 function createCredentialCreateOptions(): CredentialCreateOptionsView {
  return new CredentialCreateOptionsView(Symbol() as any, Symbol() as any);
 }
--- a/apps/web/src/app/auth/core/services/webauthn/webauthn.service.ts
+++ b/apps/web/src/app/auth/core/services/webauthn/webauthn.service.ts
@ -1,10 +1,7 @@
 import { Injectable, Optional } from "@angular/core";
 import { BehaviorSubject, filter, from, map, Observable, shareReplay, switchMap, tap } from "rxjs";

-import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/abstractions/log.service";
-import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
-import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
 import { Verification } from "@bitwarden/common/types/verification";

 import { CoreAuthModule } from "../../core.module";
@ -31,8 +28,6 @@ export class WebauthnService {

  constructor(
    private apiService: WebauthnApiService,
-    private platformUtilsService: PlatformUtilsService,
-    private i18nService: I18nService,
    @Optional() navigatorCredentials?: CredentialsContainer,
    @Optional() private logService?: LogService
  ) {
@ -43,22 +38,8 @@ export class WebauthnService {
  async getCredentialCreateOptions(
    verification: Verification
  ): Promise<CredentialCreateOptionsView | undefined> {
-    try {
-      const response = await this.apiService.getCredentialCreateOptions(verification);
-      return new CredentialCreateOptionsView(response.options, response.token);
-    } catch (error) {
-      if (error instanceof ErrorResponse && error.statusCode === 400) {
-        this.platformUtilsService.showToast(
-          "error",
-          this.i18nService.t("error"),
-          this.i18nService.t("invalidMasterPassword")
-        );
-      } else {
-        this.logService?.error(error);
-        this.platformUtilsService.showToast("error", null, this.i18nService.t("unexpectedError"));
-      }
-      return undefined;
-    }
+    const response = await this.apiService.getCredentialCreateOptions(verification);
+    return new CredentialCreateOptionsView(response.options, response.token);
  }

  async createCredential(
@ -85,24 +66,12 @@ export class WebauthnService {
    deviceResponse: PublicKeyCredential,
    name: string
  ) {
-    try {
-      const request = new SaveCredentialRequest();
-      request.deviceResponse = new WebauthnAttestationResponseRequest(deviceResponse);
-      request.token = credentialOptions.token;
-      request.name = name;
-      await this.apiService.saveCredential(request);
-      this.platformUtilsService.showToast(
-        "success",
-        null,
-        this.i18nService.t("passkeySaved", name)
-      );
-      this.refresh();
-      return true;
-    } catch (error) {
-      this.logService?.error(error);
-      this.platformUtilsService.showToast("error", null, this.i18nService.t("unexpectedError"));
-      return false;
-    }
+    const request = new SaveCredentialRequest();
+    request.deviceResponse = new WebauthnAttestationResponseRequest(deviceResponse);
+    request.token = credentialOptions.token;
+    request.name = name;
+    await this.apiService.saveCredential(request);
+    this.refresh();
  }

  getCredential$(credentialId: string): Observable<WebauthnCredentialView> {
@ -114,25 +83,9 @@ export class WebauthnService {
    );
  }

-  async deleteCredential(credentialId: string, verification: Verification): Promise<boolean> {
-    try {
-      await this.apiService.deleteCredential(credentialId, verification);
-      this.platformUtilsService.showToast("success", null, this.i18nService.t("passkeyRemoved"));
-      this.refresh();
-      return true;
-    } catch (error) {
-      if (error instanceof ErrorResponse && error.statusCode === 400) {
-        this.platformUtilsService.showToast(
-          "error",
-          this.i18nService.t("error"),
-          this.i18nService.t("invalidMasterPassword")
-        );
-      } else {
-        this.logService?.error(error);
-        this.platformUtilsService.showToast("error", null, this.i18nService.t("unexpectedError"));
-      }
-      return false;
-    }
+  async deleteCredential(credentialId: string, verification: Verification): Promise<void> {
+    await this.apiService.deleteCredential(credentialId, verification);
+    this.refresh();
  }

  private getCredentials$(): Observable<WebauthnCredentialView[]> {
--- a/apps/web/src/app/auth/settings/fido2-login-settings/create-credential-dialog/create-credential-dialog.component.ts
+++ b/apps/web/src/app/auth/settings/fido2-login-settings/create-credential-dialog/create-credential-dialog.component.ts
@ -4,7 +4,11 @@ import { FormBuilder, Validators } from "@angular/forms";
 import { map, Observable } from "rxjs";

 import { DialogServiceAbstraction } from "@bitwarden/angular/services/dialog";
+import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
+import { LogService } from "@bitwarden/common/abstractions/log.service";
+import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
 import { VerificationType } from "@bitwarden/common/auth/enums/verification-type";
+import { ErrorResponse } from "@bitwarden/common/models/response/error.response";

 import { WebauthnService } from "../../../core";
 import { CredentialCreateOptionsView } from "../../../core/views/credential-create-options.view";
@ -46,7 +50,10 @@ export class CreateCredentialDialogComponent implements OnInit {
  constructor(
    private formBuilder: FormBuilder,
    private dialogRef: DialogRef,
-    private webauthnService: WebauthnService
+    private webauthnService: WebauthnService,
+    private platformUtilsService: PlatformUtilsService,
+    private i18nService: I18nService,
+    private logService: LogService
  ) {}

  ngOnInit(): void {
@ -80,12 +87,22 @@ export class CreateCredentialDialogComponent implements OnInit {
      return;
    }

-    this.credentialOptions = await this.webauthnService.getCredentialCreateOptions({
-      type: VerificationType.MasterPassword,
-      secret: this.formGroup.value.userVerification.masterPassword,
-    });
-
-    if (this.credentialOptions === undefined) {
+    try {
+      this.credentialOptions = await this.webauthnService.getCredentialCreateOptions({
+        type: VerificationType.MasterPassword,
+        secret: this.formGroup.value.userVerification.masterPassword,
+      });
+    } catch (error) {
+      if (error instanceof ErrorResponse && error.statusCode === 400) {
+        this.platformUtilsService.showToast(
+          "error",
+          this.i18nService.t("error"),
+          this.i18nService.t("invalidMasterPassword")
+        );
+      } else {
+        this.logService?.error(error);
+        this.platformUtilsService.showToast("error", null, this.i18nService.t("unexpectedError"));
+      }
      return;
    }

@ -114,16 +131,21 @@ export class CreateCredentialDialogComponent implements OnInit {
      return;
    }

-    const result = await this.webauthnService.saveCredential(
-      this.credentialOptions,
-      this.deviceResponse,
-      this.formGroup.value.credentialNaming.name
-    );
-
-    if (!result) {
+    const name = this.formGroup.value.credentialNaming.name;
+    try {
+      await this.webauthnService.saveCredential(
+        this.credentialOptions,
+        this.deviceResponse,
+        this.formGroup.value.credentialNaming.name
+      );
+    } catch (error) {
+      this.logService?.error(error);
+      this.platformUtilsService.showToast("error", null, this.i18nService.t("unexpectedError"));
      return;
    }

+    this.platformUtilsService.showToast("success", null, this.i18nService.t("passkeySaved", name));
+
    this.dialogRef.close(CreateCredentialDialogResult.Success);
  }
 }
--- a/apps/web/src/app/auth/settings/fido2-login-settings/delete-credential-dialog/delete-credential-dialog.component.ts
+++ b/apps/web/src/app/auth/settings/fido2-login-settings/delete-credential-dialog/delete-credential-dialog.component.ts
@ -4,7 +4,11 @@ import { FormBuilder, Validators } from "@angular/forms";
 import { Subject, takeUntil } from "rxjs";

 import { DialogServiceAbstraction } from "@bitwarden/angular/services/dialog";
+import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
+import { LogService } from "@bitwarden/common/abstractions/log.service";
+import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service";
 import { VerificationType } from "@bitwarden/common/auth/enums/verification-type";
+import { ErrorResponse } from "@bitwarden/common/models/response/error.response";

 import { WebauthnService } from "../../../core";
 import { WebauthnCredentialView } from "../../../core/views/webauth-credential.view";
@ -28,7 +32,10 @@ export class DeleteCredentialDialogComponent implements OnInit, OnDestroy {
    @Inject(DIALOG_DATA) private params: DeleteCredentialDialogParams,
    private formBuilder: FormBuilder,
    private dialogRef: DialogRef,
-    private webauthnService: WebauthnService
+    private webauthnService: WebauthnService,
+    private platformUtilsService: PlatformUtilsService,
+    private i18nService: I18nService,
+    private logService: LogService
  ) {}

  ngOnInit(): void {
@ -44,14 +51,28 @@ export class DeleteCredentialDialogComponent implements OnInit, OnDestroy {
    }

    this.dialogRef.disableClose = true;
-    const success = await this.webauthnService.deleteCredential(this.credential.id, {
-      type: VerificationType.MasterPassword,
-      secret: this.formGroup.value.masterPassword,
-    });
-    if (!success) {
+    try {
+      await this.webauthnService.deleteCredential(this.credential.id, {
+        type: VerificationType.MasterPassword,
+        secret: this.formGroup.value.masterPassword,
+      });
+      this.platformUtilsService.showToast("success", null, this.i18nService.t("passkeyRemoved"));
+    } catch (error) {
+      if (error instanceof ErrorResponse && error.statusCode === 400) {
+        this.platformUtilsService.showToast(
+          "error",
+          this.i18nService.t("error"),
+          this.i18nService.t("invalidMasterPassword")
+        );
+      } else {
+        this.logService.error(error);
+        this.platformUtilsService.showToast("error", null, this.i18nService.t("unexpectedError"));
+      }
+      return false;
+    } finally {
      this.dialogRef.disableClose = false;
-      return;
    }
+
    this.dialogRef.close();
  };