move auth request notification to service (#8451)

- cleanup hanging promises

apps/browser/src/auth/popup/login-via-auth-request.component.ts

@@ -1,5 +1,5 @@
 import { Location } from "@angular/common";
-import { Component, OnDestroy, OnInit } from "@angular/core";
+import { Component } from "@angular/core";
 import { Router } from "@angular/router";
 
 import { LoginViaAuthRequestComponent as BaseLoginWithDeviceComponent } from "@bitwarden/angular/auth/components/login-via-auth-request.component";
@@ -28,10 +28,7 @@ import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.serv
   selector: "app-login-via-auth-request",
   templateUrl: "login-via-auth-request.component.html",
 })
-export class LoginViaAuthRequestComponent
+export class LoginViaAuthRequestComponent extends BaseLoginWithDeviceComponent {
-  extends BaseLoginWithDeviceComponent
-  implements OnInit, OnDestroy
-{
   constructor(
     router: Router,
     cryptoService: CryptoService,

apps/desktop/src/auth/login/login-via-auth-request.component.ts

@@ -1,5 +1,5 @@
 import { Location } from "@angular/common";
-import { Component, OnDestroy, OnInit, ViewChild, ViewContainerRef } from "@angular/core";
+import { Component, ViewChild, ViewContainerRef } from "@angular/core";
 import { Router } from "@angular/router";
 
 import { LoginViaAuthRequestComponent as BaseLoginWithDeviceComponent } from "@bitwarden/angular/auth/components/login-via-auth-request.component";
@@ -31,10 +31,7 @@ import { EnvironmentComponent } from "../environment.component";
   selector: "app-login-via-auth-request",
   templateUrl: "login-via-auth-request.component.html",
 })
-export class LoginViaAuthRequestComponent
+export class LoginViaAuthRequestComponent extends BaseLoginWithDeviceComponent {
-  extends BaseLoginWithDeviceComponent
-  implements OnInit, OnDestroy
-{
   @ViewChild("environment", { read: ViewContainerRef, static: true })
   environmentModal: ViewContainerRef;
   showingModal = false;
@@ -109,10 +106,6 @@ export class LoginViaAuthRequestComponent
     });
   }
 
-  ngOnDestroy(): void {
-    super.ngOnDestroy();
-  }
-
   back() {
     this.location.back();
   }

apps/web/src/app/auth/login/login-via-auth-request.component.ts

@@ -1,75 +1,9 @@
-import { Component, OnDestroy, OnInit } from "@angular/core";
+import { Component } from "@angular/core";
-import { Router } from "@angular/router";
 
 import { LoginViaAuthRequestComponent as BaseLoginWithDeviceComponent } from "@bitwarden/angular/auth/components/login-via-auth-request.component";
-import {
-  AuthRequestServiceAbstraction,
-  LoginStrategyServiceAbstraction,
-} from "@bitwarden/auth/common";
-import { ApiService } from "@bitwarden/common/abstractions/api.service";
-import { AnonymousHubService } from "@bitwarden/common/auth/abstractions/anonymous-hub.service";
-import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
-import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust-crypto.service.abstraction";
-import { LoginService } from "@bitwarden/common/auth/abstractions/login.service";
-import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
-import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
-import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
-import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
-import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
-import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
-import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
-import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
-import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password";
-
-import { StateService } from "../../core";
 
 @Component({
   selector: "app-login-via-auth-request",
   templateUrl: "login-via-auth-request.component.html",
 })
-export class LoginViaAuthRequestComponent
+export class LoginViaAuthRequestComponent extends BaseLoginWithDeviceComponent {}
-  extends BaseLoginWithDeviceComponent
-  implements OnInit, OnDestroy
-{
-  constructor(
-    router: Router,
-    cryptoService: CryptoService,
-    cryptoFunctionService: CryptoFunctionService,
-    appIdService: AppIdService,
-    passwordGenerationService: PasswordGenerationServiceAbstraction,
-    apiService: ApiService,
-    authService: AuthService,
-    logService: LogService,
-    environmentService: EnvironmentService,
-    i18nService: I18nService,
-    platformUtilsService: PlatformUtilsService,
-    anonymousHubService: AnonymousHubService,
-    validationService: ValidationService,
-    stateService: StateService,
-    loginService: LoginService,
-    deviceTrustCryptoService: DeviceTrustCryptoServiceAbstraction,
-    authRequestService: AuthRequestServiceAbstraction,
-    loginStrategyService: LoginStrategyServiceAbstraction,
-  ) {
-    super(
-      router,
-      cryptoService,
-      cryptoFunctionService,
-      appIdService,
-      passwordGenerationService,
-      apiService,
-      authService,
-      logService,
-      environmentService,
-      i18nService,
-      platformUtilsService,
-      anonymousHubService,
-      validationService,
-      stateService,
-      loginService,
-      deviceTrustCryptoService,
-      authRequestService,
-      loginStrategyService,
-    );
-  }
-}

libs/angular/src/auth/components/login-via-auth-request.component.ts

@@ -68,7 +68,6 @@ export class LoginViaAuthRequestComponent
 
   private authRequestKeyPair: { publicKey: Uint8Array; privateKey: Uint8Array };
 
-  // TODO: in future, go to child components and remove child constructors and let deps fall through to the super class
   constructor(
     protected router: Router,
     private cryptoService: CryptoService,
@@ -98,14 +97,16 @@ export class LoginViaAuthRequestComponent
       this.email = this.loginService.getEmail();
     }
 
-    //gets signalR push notification
+    // Gets signalR push notification
-    this.loginStrategyService.authRequestPushNotification$
+    // Only fires on approval to prevent enumeration
+    this.authRequestService.authRequestPushNotification$
       .pipe(takeUntil(this.destroy$))
       .subscribe((id) => {
-        // Only fires on approval currently
-        // 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.verifyAndHandleApprovedAuthReq(id);
+        this.verifyAndHandleApprovedAuthReq(id).catch((e: Error) => {
+          this.platformUtilsService.showToast("error", this.i18nService.t("error"), e.message);
+          this.logService.error("Failed to use approved auth request: " + e.message);
+        });
       });
   }
 
@@ -164,10 +165,10 @@ export class LoginViaAuthRequestComponent
     }
   }
 
-  ngOnDestroy(): void {
+  async ngOnDestroy() {
+    await this.anonymousHubService.stopHubConnection();
     this.destroy$.next();
     this.destroy$.complete();
-    this.anonymousHubService.stopHubConnection();
   }
 
   private async handleExistingAdminAuthRequest(adminAuthReqStorable: AdminAuthRequestStorable) {
@@ -213,7 +214,7 @@ export class LoginViaAuthRequestComponent
 
     // Request still pending response from admin
     // So, create hub connection so that any approvals will be received via push notification
-    this.anonymousHubService.createHubConnection(adminAuthReqStorable.id);
+    await this.anonymousHubService.createHubConnection(adminAuthReqStorable.id);
   }
 
   private async handleExistingAdminAuthReqDeletedOrDenied() {
@@ -273,7 +274,7 @@ export class LoginViaAuthRequestComponent
       }
 
       if (reqResponse.id) {
-        this.anonymousHubService.createHubConnection(reqResponse.id);
+        await this.anonymousHubService.createHubConnection(reqResponse.id);
       }
     } catch (e) {
       this.logService.error(e);

libs/angular/src/services/jslib-services.module.ts

@@ -866,7 +866,7 @@ const safeProviders: SafeProvider[] = [
   safeProvider({
     provide: AnonymousHubServiceAbstraction,
     useClass: AnonymousHubService,
-    deps: [EnvironmentService, LoginStrategyServiceAbstraction, LogService],
+    deps: [EnvironmentService, AuthRequestServiceAbstraction],
   }),
   safeProvider({
     provide: ValidationServiceAbstraction,

libs/auth/src/common/abstractions/auth-request.service.abstraction.ts

@@ -1,7 +1,12 @@
+import { Observable } from "rxjs";
+
 import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
+import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response";
 import { UserKey, MasterKey } from "@bitwarden/common/types/key";
 
 export abstract class AuthRequestServiceAbstraction {
+  /** Emits an auth request id when an auth request has been approved. */
+  authRequestPushNotification$: Observable<string>;
   /**
    * Approve or deny an auth request.
    * @param approve True to approve, false to deny.
@@ -54,4 +59,11 @@ export abstract class AuthRequestServiceAbstraction {
     pubKeyEncryptedMasterKeyHash: string,
     privateKey: ArrayBuffer,
   ) => Promise<{ masterKey: MasterKey; masterKeyHash: string }>;
+
+  /**
+   * Handles incoming auth request push notifications.
+   * @param notification push notification.
+   * @remark We should only be receiving approved push notifications to prevent enumeration.
+   */
+  abstract sendAuthRequestPushNotification: (notification: AuthRequestPushNotification) => void;
 }

libs/auth/src/common/abstractions/login-strategy.service.ts

@@ -4,7 +4,6 @@ import { AuthenticationType } from "@bitwarden/common/auth/enums/authentication-
 import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result";
 import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/identity-token/token-two-factor.request";
 import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
-import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response";
 import { MasterKey } from "@bitwarden/common/types/key";
 
 import {
@@ -21,10 +20,6 @@ export abstract class LoginStrategyServiceAbstraction {
    * Emits null if the session has timed out.
    */
   currentAuthType$: Observable<AuthenticationType | null>;
-  /**
-   * Emits when an auth request has been approved.
-   */
-  authRequestPushNotification$: Observable<string>;
   /**
    * If the login strategy uses the email address of the user, this
    * will return it. Otherwise, it will return null.
@@ -77,10 +72,6 @@ export abstract class LoginStrategyServiceAbstraction {
    * Creates a master key from the provided master password and email.
    */
   makePreloginKey: (masterPassword: string, email: string) => Promise<MasterKey>;
-  /**
-   * Sends a notification to {@link LoginStrategyServiceAbstraction.authRequestPushNotification}
-   */
-  sendAuthRequestPushNotification: (notification: AuthRequestPushNotification) => Promise<void>;
   /**
    * Sends a response to an auth request.
    */

libs/auth/src/common/services/auth-request/auth-request.service.ts

@@ -1,6 +1,9 @@
+import { Observable, Subject } from "rxjs";
+
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { PasswordlessAuthRequest } from "@bitwarden/common/auth/models/request/passwordless-auth.request";
 import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
+import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response";
 import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
 import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
 import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
@@ -11,6 +14,9 @@ import { MasterKey, UserKey } from "@bitwarden/common/types/key";
 import { AuthRequestServiceAbstraction } from "../../abstractions/auth-request.service.abstraction";
 
 export class AuthRequestService implements AuthRequestServiceAbstraction {
+  private authRequestPushNotificationSubject = new Subject<string>();
+  authRequestPushNotification$: Observable<string>;
+
   constructor(
     private appIdService: AppIdService,
     private cryptoService: CryptoService,
@@ -126,4 +132,10 @@ export class AuthRequestService implements AuthRequestServiceAbstraction {
       masterKeyHash,
     };
   }
+
+  sendAuthRequestPushNotification(notification: AuthRequestPushNotification): void {
+    if (notification.id != null) {
+      this.authRequestPushNotificationSubject.next(notification.id);
+    }
+  }
 }

libs/auth/src/common/services/login-strategies/login-strategy.service.ts

@@ -1,7 +1,6 @@
 import {
   combineLatestWith,
   distinctUntilChanged,
-  filter,
   firstValueFrom,
   map,
   Observable,
@@ -23,7 +22,6 @@ import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth
 import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
 import { PreloginRequest } from "@bitwarden/common/models/request/prelogin.request";
 import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
-import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response";
 import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
 import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
 import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
@@ -81,8 +79,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
   >;
 
   currentAuthType$: Observable<AuthenticationType | null>;
-  // TODO: move to auth request service
-  authRequestPushNotification$: Observable<string>;
 
   constructor(
     protected cryptoService: CryptoService,
@@ -114,9 +110,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
     );
 
     this.currentAuthType$ = this.currentAuthnTypeState.state$;
-    this.authRequestPushNotification$ = this.authRequestPushNotificationState.state$.pipe(
-      filter((id) => id != null),
-    );
     this.loginStrategy$ = this.currentAuthnTypeState.state$.pipe(
       distinctUntilChanged(),
       combineLatestWith(this.loginStrategyCacheState.state$),
@@ -256,13 +249,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
     return await this.cryptoService.makeMasterKey(masterPassword, email, kdf, kdfConfig);
   }
 
-  // TODO move to auth request service
-  async sendAuthRequestPushNotification(notification: AuthRequestPushNotification): Promise<void> {
-    if (notification.id != null) {
-      await this.authRequestPushNotificationState.update((_) => notification.id);
-    }
-  }
-
   // TODO: move to auth request service
   async passwordlessLogin(
     id: string,

libs/common/src/auth/abstractions/anonymous-hub.service.ts

@@ -1,4 +1,4 @@
 export abstract class AnonymousHubService {
-  createHubConnection: (token: string) => void;
+  createHubConnection: (token: string) => Promise<void>;
-  stopHubConnection: () => void;
+  stopHubConnection: () => Promise<void>;
 }

libs/common/src/auth/services/anonymous-hub.service.ts

@@ -7,13 +7,13 @@ import {
 import { MessagePackHubProtocol } from "@microsoft/signalr-protocol-msgpack";
 import { firstValueFrom } from "rxjs";
 
-import { LoginStrategyServiceAbstraction } from "../../../../auth/src/common/abstractions/login-strategy.service";
+import { AuthRequestServiceAbstraction } from "../../../../auth/src/common/abstractions";
+import { NotificationType } from "../../enums";
 import {
   AuthRequestPushNotification,
   NotificationResponse,
 } from "../../models/response/notification.response";
 import { EnvironmentService } from "../../platform/abstractions/environment.service";
-import { LogService } from "../../platform/abstractions/log.service";
 import { AnonymousHubService as AnonymousHubServiceAbstraction } from "../abstractions/anonymous-hub.service";
 
 export class AnonymousHubService implements AnonymousHubServiceAbstraction {
@@ -22,8 +22,7 @@ export class AnonymousHubService implements AnonymousHubServiceAbstraction {
 
   constructor(
     private environmentService: EnvironmentService,
-    private loginStrategyService: LoginStrategyServiceAbstraction,
+    private authRequestService: AuthRequestServiceAbstraction,
-    private logService: LogService,
   ) {}
 
   async createHubConnection(token: string) {
@@ -37,26 +36,25 @@ export class AnonymousHubService implements AnonymousHubServiceAbstraction {
       .withHubProtocol(new MessagePackHubProtocol() as IHubProtocol)
       .build();
 
-    this.anonHubConnection.start().catch((error) => this.logService.error(error));
+    await this.anonHubConnection.start();
 
     this.anonHubConnection.on("AuthRequestResponseRecieved", (data: any) => {
-      // 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.ProcessNotification(new NotificationResponse(data));
     });
   }
 
-  stopHubConnection() {
+  async stopHubConnection() {
     if (this.anonHubConnection) {
-      // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+      await this.anonHubConnection.stop();
-      // eslint-disable-next-line @typescript-eslint/no-floating-promises
-      this.anonHubConnection.stop();
     }
   }
 
-  private async ProcessNotification(notification: NotificationResponse) {
+  private ProcessNotification(notification: NotificationResponse) {
-    await this.loginStrategyService.sendAuthRequestPushNotification(
+    switch (notification.type) {
-      notification.payload as AuthRequestPushNotification,
+      case NotificationType.AuthRequestResponse:
-    );
+        this.authRequestService.sendAuthRequestPushNotification(
+          notification.payload as AuthRequestPushNotification,
+        );
+    }
   }
 }