[PM-4016] Address feedback on [PM-2014] (#6532)

* [PM-4016] feat: use dialog `loading` attribute * [PM-4016] chore: move constant to service * [PM-4016] chore: simplify paddings * [PM-4016] chore: rename to `AuthSettingsModule` * [PM-4016] fix: move request creation to service * [PM-4016] feat: simplify module structure Remove core.module and use `@Injectable({ providedIn: "root" })` instead.
2025-12-10 13:23:34 +00:00 · 2023-10-11 16:09:01 +02:00
parent 524123ac01
commit 56a1229803
13 changed files with 31 additions and 42 deletions
--- a/apps/web/src/app/auth/auth.module.ts
+++ b/apps/web/src/app/auth/auth.module.ts
@@ -1,12 +1,11 @@
 import { NgModule } from "@angular/core";

-import { CoreAuthModule } from "./core";
-import { SettingsModule } from "./settings/settings.module";
+import { AuthSettingsModule } from "./settings/settings.module";

@NgModule({
-  imports: [CoreAuthModule, SettingsModule],
+  imports: [AuthSettingsModule],
  declarations: [],
  providers: [],
-  exports: [SettingsModule],
+  exports: [AuthSettingsModule],
 })
 export class AuthModule {}
--- a/apps/web/src/app/auth/core/core.module.ts
+++ b/apps/web/src/app/auth/core/core.module.ts
@@ -1,15 +0,0 @@
-import { NgModule, Optional, SkipSelf } from "@angular/core";
-
-import { WebauthnLoginApiService } from "./services/webauthn-login/webauthn-login-api.service";
-import { WebauthnLoginService } from "./services/webauthn-login/webauthn-login.service";
-
-@NgModule({
-  providers: [WebauthnLoginService, WebauthnLoginApiService],
-})
-export class CoreAuthModule {
-  constructor(@Optional() @SkipSelf() parentModule?: CoreAuthModule) {
-    if (parentModule) {
-      throw new Error("CoreAuthModule is already loaded. Import it in AuthModule only");
-    }
-  }
-}
--- a/apps/web/src/app/auth/core/index.ts
+++ b/apps/web/src/app/auth/core/index.ts
@@ -1,2 +1 @@
 export * from "./services";
-export * from "./core.module";
--- a/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login-api.service.ts
+++ b/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login-api.service.ts
@@ -1,25 +1,20 @@
 import { Injectable } from "@angular/core";

 import { ApiService } from "@bitwarden/common/abstractions/api.service";
-import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
+import { SecretVerificationRequest } from "@bitwarden/common/auth/models/request/secret-verification.request";
 import { ListResponse } from "@bitwarden/common/models/response/list.response";
-import { Verification } from "@bitwarden/common/types/verification";

 import { SaveCredentialRequest } from "./request/save-credential.request";
 import { WebauthnLoginCredentialCreateOptionsResponse } from "./response/webauthn-login-credential-create-options.response";
 import { WebauthnLoginCredentialResponse } from "./response/webauthn-login-credential.response";

-@Injectable()
+@Injectable({ providedIn: "root" })
 export class WebauthnLoginApiService {
-  constructor(
-    private apiService: ApiService,
-    private userVerificationService: UserVerificationService
-  ) {}
+  constructor(private apiService: ApiService) {}

  async getCredentialCreateOptions(
-    verification: Verification
+    request: SecretVerificationRequest
  ): Promise<WebauthnLoginCredentialCreateOptionsResponse> {
-    const request = await this.userVerificationService.buildRequest(verification);
    const response = await this.apiService.send("POST", "/webauthn/options", request, true, true);
    return new WebauthnLoginCredentialCreateOptionsResponse(response);
  }
@@ -33,8 +28,7 @@ export class WebauthnLoginApiService {
    return this.apiService.send("GET", "/webauthn", null, true, true);
  }

-  async deleteCredential(credentialId: string, verification: Verification): Promise<void> {
-    const request = await this.userVerificationService.buildRequest(verification);
+  async deleteCredential(credentialId: string, request: SecretVerificationRequest): Promise<void> {
    await this.apiService.send("POST", `/webauthn/${credentialId}/delete`, request, true, true);
  }
 }
--- a/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login.service.spec.ts
+++ b/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login.service.spec.ts
@@ -1,5 +1,7 @@
 import { mock, MockProxy } from "jest-mock-extended";

+import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
+
 import { CredentialCreateOptionsView } from "../../views/credential-create-options.view";

 import { WebauthnLoginApiService } from "./webauthn-login-api.service";
@@ -7,6 +9,7 @@ import { WebauthnLoginService } from "./webauthn-login.service";

 describe("WebauthnService", () => {
  let apiService!: MockProxy<WebauthnLoginApiService>;
+  let userVerificationService!: MockProxy<UserVerificationService>;
  let credentials: MockProxy<CredentialsContainer>;
  let webauthnService!: WebauthnLoginService;

@@ -15,8 +18,9 @@ describe("WebauthnService", () => {
    window.PublicKeyCredential = class {} as any;
    window.AuthenticatorAttestationResponse = class {} as any;
    apiService = mock<WebauthnLoginApiService>();
+    userVerificationService = mock<UserVerificationService>();
    credentials = mock<CredentialsContainer>();
-    webauthnService = new WebauthnLoginService(apiService, credentials);
+    webauthnService = new WebauthnLoginService(apiService, userVerificationService, credentials);
  });

  describe("createCredential", () => {
--- a/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login.service.ts
+++ b/apps/web/src/app/auth/core/services/webauthn-login/webauthn-login.service.ts
@@ -1,6 +1,7 @@
 import { Injectable, Optional } from "@angular/core";
 import { BehaviorSubject, filter, from, map, Observable, shareReplay, switchMap, tap } from "rxjs";

+import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { Verification } from "@bitwarden/common/types/verification";

@@ -11,8 +12,10 @@ import { SaveCredentialRequest } from "./request/save-credential.request";
 import { WebauthnLoginAttestationResponseRequest } from "./request/webauthn-login-attestation-response.request";
 import { WebauthnLoginApiService } from "./webauthn-login-api.service";

-@Injectable()
+@Injectable({ providedIn: "root" })
 export class WebauthnLoginService {
+  static readonly MaxCredentialCount = 5;
+
  private navigatorCredentials: CredentialsContainer;
  private _refresh$ = new BehaviorSubject<void>(undefined);
  private _loading$ = new BehaviorSubject<boolean>(true);
@@ -27,6 +30,7 @@ export class WebauthnLoginService {

  constructor(
    private apiService: WebauthnLoginApiService,
+    private userVerificationService: UserVerificationService,
    @Optional() navigatorCredentials?: CredentialsContainer,
    @Optional() private logService?: LogService
  ) {
@@ -37,7 +41,8 @@ export class WebauthnLoginService {
  async getCredentialCreateOptions(
    verification: Verification
  ): Promise<CredentialCreateOptionsView> {
-    const response = await this.apiService.getCredentialCreateOptions(verification);
+    const request = await this.userVerificationService.buildRequest(verification);
+    const response = await this.apiService.getCredentialCreateOptions(request);
    return new CredentialCreateOptionsView(response.options, response.token);
  }

@@ -95,7 +100,8 @@ export class WebauthnLoginService {
  }

  async deleteCredential(credentialId: string, verification: Verification): Promise<void> {
-    await this.apiService.deleteCredential(credentialId, verification);
+    const request = await this.userVerificationService.buildRequest(verification);
+    await this.apiService.deleteCredential(credentialId, request);
    this.refresh();
  }

--- a/apps/web/src/app/auth/settings/settings.module.ts
+++ b/apps/web/src/app/auth/settings/settings.module.ts
@@ -11,6 +11,6 @@ import { WebauthnLoginSettingsModule } from "./webauthn-login-settings";
  imports: [SharedModule, WebauthnLoginSettingsModule, PasswordCalloutComponent],
  declarations: [ChangePasswordComponent],
  providers: [],
-  exports: [WebauthnLoginSettingsModule, ChangePasswordComponent],
+  exports: [ChangePasswordComponent],
 })
-export class SettingsModule {}
+export class AuthSettingsModule {}
--- a/apps/web/src/app/auth/settings/webauthn-login-settings/create-credential-dialog/create-credential-dialog.component.html
+++ b/apps/web/src/app/auth/settings/webauthn-login-settings/create-credential-dialog/create-credential-dialog.component.html
@@ -1,5 +1,5 @@
 <form [formGroup]="formGroup" [bitSubmit]="submit">
-  <bit-dialog dialogSize="large">
+  <bit-dialog dialogSize="large" [loading]="loading$ | async">
    <span bitDialogTitle
      >{{ "loginWithPasskey" | i18n }}
      <span class="tw-text-sm tw-normal-case tw-text-muted">{{ "newPasskey" | i18n }}</span>
--- a/apps/web/src/app/auth/settings/webauthn-login-settings/create-credential-dialog/create-credential-dialog.component.ts
+++ b/apps/web/src/app/auth/settings/webauthn-login-settings/create-credential-dialog/create-credential-dialog.component.ts
@@ -46,6 +46,7 @@ export class CreateCredentialDialogComponent implements OnInit {
  protected credentialOptions?: CredentialCreateOptionsView;
  protected deviceResponse?: PublicKeyCredential;
  protected hasPasskeys$?: Observable<boolean>;
+  protected loading$ = this.webauthnService.loading$;

  constructor(
    private formBuilder: FormBuilder,
--- a/apps/web/src/app/auth/settings/webauthn-login-settings/delete-credential-dialog/delete-credential-dialog.component.html
+++ b/apps/web/src/app/auth/settings/webauthn-login-settings/delete-credential-dialog/delete-credential-dialog.component.html
@@ -1,5 +1,5 @@
 <form [formGroup]="formGroup" [bitSubmit]="submit">
-  <bit-dialog dialogSize="large">
+  <bit-dialog dialogSize="large" [loading]="loading$ | async">
    <span bitDialogTitle
      >{{ "removePasskey" | i18n }}
      <span *ngIf="credential" class="tw-text-sm tw-normal-case tw-text-muted">{{
--- a/apps/web/src/app/auth/settings/webauthn-login-settings/delete-credential-dialog/delete-credential-dialog.component.ts
+++ b/apps/web/src/app/auth/settings/webauthn-login-settings/delete-credential-dialog/delete-credential-dialog.component.ts
@@ -27,6 +27,7 @@ export class DeleteCredentialDialogComponent implements OnInit, OnDestroy {
    masterPassword: ["", [Validators.required]],
  });
  protected credential?: WebauthnCredentialView;
+  protected loading$ = this.webauthnService.loading$;

  constructor(
    @Inject(DIALOG_DATA) private params: DeleteCredentialDialogParams,
--- a/apps/web/src/app/auth/settings/webauthn-login-settings/webauthn-login-settings.component.html
+++ b/apps/web/src/app/auth/settings/webauthn-login-settings/webauthn-login-settings.component.html
@@ -22,7 +22,7 @@
 <table *ngIf="hasCredentials" class="tw-mb-5">
  <tr *ngFor="let credential of credentials">
    <td class="tw-p-2 tw-pl-0 tw-font-semibold">{{ credential.name }}</td>
-    <td class="tw-p-2 tw-pr-0">
+    <td class="tw-p-2 tw-pr-10">
      <ng-container *ngIf="credential.prfSupport">
        <i class="bwi bwi-lock-encrypted"></i>
        {{ "supportsEncryption" | i18n }}
@@ -31,7 +31,7 @@
        {{ "encryptionNotSupported" | i18n }}
      </span>
    </td>
-    <td class="tw-py-2 tw-pl-10 tw-pr-0">
+    <td class="tw-py-2">
      <button
        type="button"
        bitLink
--- a/apps/web/src/app/auth/settings/webauthn-login-settings/webauthn-login-settings.component.ts
+++ b/apps/web/src/app/auth/settings/webauthn-login-settings/webauthn-login-settings.component.ts
@@ -19,7 +19,7 @@ import { openDeleteCredentialDialogComponent } from "./delete-credential-dialog/
 export class WebauthnLoginSettingsComponent implements OnInit, OnDestroy {
  private destroy$ = new Subject<void>();

-  protected readonly MaxCredentialCount = 5;
+  protected readonly MaxCredentialCount = WebauthnLoginService.MaxCredentialCount;

  protected credentials?: WebauthnCredentialView[];
  protected loading = true;