From 0568a09212f56c58435a59935b3284ad93734bbf Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Mon, 10 Mar 2025 12:17:46 -0700 Subject: [PATCH] refactor(device-trust-toasts): [Auth/PM-11225] Refactor Toasts from Auth Services (#13665) Refactor toast calls out of auth services. Toasts are now triggered by an observable emission that gets picked up by an observable pipeline in a new `DeviceTrustToastService` (libs/angular). That observable pipeline is then subscribed by by consuming the `AppComponent` for each client. --- apps/browser/src/popup/app.component.ts | 7 +- apps/desktop/src/app/app.component.ts | 7 +- apps/web/src/app/app.component.ts | 7 +- .../device-trust-toast.service.abstraction.ts | 9 + ...vice-trust-toast.service.implementation.ts | 44 +++++ .../device-trust-toast.service.spec.ts | 167 ++++++++++++++++++ .../src/services/jslib-services.module.ts | 12 ++ .../auth-request.service.abstraction.ts | 15 ++ .../login-strategies/sso-login.strategy.ts | 3 +- .../auth-request/auth-request.service.ts | 9 + .../device-trust.service.abstraction.ts | 6 + .../device-trust.service.implementation.ts | 9 +- 12 files changed, 289 insertions(+), 6 deletions(-) create mode 100644 libs/angular/src/auth/services/device-trust-toast.service.abstraction.ts create mode 100644 libs/angular/src/auth/services/device-trust-toast.service.implementation.ts create mode 100644 libs/angular/src/auth/services/device-trust-toast.service.spec.ts diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 2a7fbdce254..6a08bf007bb 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -1,9 +1,11 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit, inject } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { NavigationEnd, Router, RouterOutlet } from "@angular/router"; import { Subject, takeUntil, firstValueFrom, concatMap, filter, tap } from "rxjs"; +import { DeviceTrustToastService } from "@bitwarden/angular/auth/services/device-trust-toast.service.abstraction"; import { LogoutReason } from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; @@ -68,7 +70,10 @@ export class AppComponent implements OnInit, OnDestroy { private animationControlService: AnimationControlService, private biometricStateService: BiometricStateService, private biometricsService: BiometricsService, - ) {} + private deviceTrustToastService: DeviceTrustToastService, + ) { + this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); + } async ngOnInit() { initPopupClosedListener(); diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 85c159f0278..1ef03e5bb71 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -10,10 +10,12 @@ import { ViewChild, ViewContainerRef, } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { Router } from "@angular/router"; import { filter, firstValueFrom, map, Subject, takeUntil, timeout, withLatestFrom } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; +import { DeviceTrustToastService } from "@bitwarden/angular/auth/services/device-trust-toast.service.abstraction"; import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref"; import { ModalService } from "@bitwarden/angular/services/modal.service"; import { FingerprintDialogComponent, LoginApprovalComponent } from "@bitwarden/auth/angular"; @@ -157,7 +159,10 @@ export class AppComponent implements OnInit, OnDestroy { private stateEventRunnerService: StateEventRunnerService, private accountService: AccountService, private organizationService: OrganizationService, - ) {} + private deviceTrustToastService: DeviceTrustToastService, + ) { + this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); + } ngOnInit() { this.accountService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((account) => { diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 9d2afb22688..f6e038f85d9 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -2,11 +2,13 @@ // @ts-strict-ignore import { DOCUMENT } from "@angular/common"; import { Component, Inject, NgZone, OnDestroy, OnInit } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { NavigationEnd, Router } from "@angular/router"; import * as jq from "jquery"; import { Subject, filter, firstValueFrom, map, takeUntil, timeout } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; +import { DeviceTrustToastService } from "@bitwarden/angular/auth/services/device-trust-toast.service.abstraction"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; @@ -95,7 +97,10 @@ export class AppComponent implements OnDestroy, OnInit { private apiService: ApiService, private appIdService: AppIdService, private processReloadService: ProcessReloadServiceAbstraction, - ) {} + private deviceTrustToastService: DeviceTrustToastService, + ) { + this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); + } ngOnInit() { this.i18nService.locale$.pipe(takeUntil(this.destroy$)).subscribe((locale) => { diff --git a/libs/angular/src/auth/services/device-trust-toast.service.abstraction.ts b/libs/angular/src/auth/services/device-trust-toast.service.abstraction.ts new file mode 100644 index 00000000000..3de288168b1 --- /dev/null +++ b/libs/angular/src/auth/services/device-trust-toast.service.abstraction.ts @@ -0,0 +1,9 @@ +import { Observable } from "rxjs"; + +export abstract class DeviceTrustToastService { + /** + * An observable pipeline that observes any cross-application toast messages + * that need to be shown as part of the trusted device encryption (TDE) process. + */ + abstract setupListeners$: Observable; +} diff --git a/libs/angular/src/auth/services/device-trust-toast.service.implementation.ts b/libs/angular/src/auth/services/device-trust-toast.service.implementation.ts new file mode 100644 index 00000000000..330519683f3 --- /dev/null +++ b/libs/angular/src/auth/services/device-trust-toast.service.implementation.ts @@ -0,0 +1,44 @@ +import { merge, Observable, tap } from "rxjs"; + +import { AuthRequestServiceAbstraction } from "@bitwarden/auth/common"; +import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { ToastService } from "@bitwarden/components"; + +import { DeviceTrustToastService as DeviceTrustToastServiceAbstraction } from "./device-trust-toast.service.abstraction"; + +export class DeviceTrustToastService implements DeviceTrustToastServiceAbstraction { + private adminLoginApproved$: Observable; + private deviceTrusted$: Observable; + + setupListeners$: Observable; + + constructor( + private authRequestService: AuthRequestServiceAbstraction, + private deviceTrustService: DeviceTrustServiceAbstraction, + private i18nService: I18nService, + private toastService: ToastService, + ) { + this.adminLoginApproved$ = this.authRequestService.adminLoginApproved$.pipe( + tap(() => { + this.toastService.showToast({ + variant: "success", + title: "", + message: this.i18nService.t("loginApproved"), + }); + }), + ); + + this.deviceTrusted$ = this.deviceTrustService.deviceTrusted$.pipe( + tap(() => { + this.toastService.showToast({ + variant: "success", + title: "", + message: this.i18nService.t("deviceTrusted"), + }); + }), + ); + + this.setupListeners$ = merge(this.adminLoginApproved$, this.deviceTrusted$); + } +} diff --git a/libs/angular/src/auth/services/device-trust-toast.service.spec.ts b/libs/angular/src/auth/services/device-trust-toast.service.spec.ts new file mode 100644 index 00000000000..cd9c6b0acf5 --- /dev/null +++ b/libs/angular/src/auth/services/device-trust-toast.service.spec.ts @@ -0,0 +1,167 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { EMPTY, of } from "rxjs"; + +import { AuthRequestServiceAbstraction } from "@bitwarden/auth/common"; +import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { ToastService } from "@bitwarden/components"; + +import { DeviceTrustToastService as DeviceTrustToastServiceAbstraction } from "./device-trust-toast.service.abstraction"; +import { DeviceTrustToastService } from "./device-trust-toast.service.implementation"; + +describe("DeviceTrustToastService", () => { + let authRequestService: MockProxy; + let deviceTrustService: MockProxy; + let i18nService: MockProxy; + let toastService: MockProxy; + + let sut: DeviceTrustToastServiceAbstraction; + + beforeEach(() => { + authRequestService = mock(); + deviceTrustService = mock(); + i18nService = mock(); + toastService = mock(); + + i18nService.t.mockImplementation((key: string) => key); // just return the key that was given + }); + + const initService = () => { + return new DeviceTrustToastService( + authRequestService, + deviceTrustService, + i18nService, + toastService, + ); + }; + + const loginApprovalToastOptions = { + variant: "success", + title: "", + message: "loginApproved", + }; + + const deviceTrustedToastOptions = { + variant: "success", + title: "", + message: "deviceTrusted", + }; + + describe("setupListeners$", () => { + describe("given adminLoginApproved$ emits and deviceTrusted$ emits", () => { + beforeEach(() => { + // Arrange + authRequestService.adminLoginApproved$ = of(undefined); + deviceTrustService.deviceTrusted$ = of(undefined); + sut = initService(); + }); + + it("should trigger a toast for login approval", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).toHaveBeenCalledWith(loginApprovalToastOptions); // Assert + done(); + }, + }); + }); + + it("should trigger a toast for device trust", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).toHaveBeenCalledWith(deviceTrustedToastOptions); // Assert + done(); + }, + }); + }); + }); + + describe("given adminLoginApproved$ emits and deviceTrusted$ does not emit", () => { + beforeEach(() => { + // Arrange + authRequestService.adminLoginApproved$ = of(undefined); + deviceTrustService.deviceTrusted$ = EMPTY; + sut = initService(); + }); + + it("should trigger a toast for login approval", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).toHaveBeenCalledWith(loginApprovalToastOptions); // Assert + done(); + }, + }); + }); + + it("should NOT trigger a toast for device trust", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).not.toHaveBeenCalledWith(deviceTrustedToastOptions); // Assert + done(); + }, + }); + }); + }); + + describe("given adminLoginApproved$ does not emit and deviceTrusted$ emits", () => { + beforeEach(() => { + // Arrange + authRequestService.adminLoginApproved$ = EMPTY; + deviceTrustService.deviceTrusted$ = of(undefined); + sut = initService(); + }); + + it("should NOT trigger a toast for login approval", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).not.toHaveBeenCalledWith(loginApprovalToastOptions); // Assert + done(); + }, + }); + }); + + it("should trigger a toast for device trust", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).toHaveBeenCalledWith(deviceTrustedToastOptions); // Assert + done(); + }, + }); + }); + }); + + describe("given adminLoginApproved$ does not emit and deviceTrusted$ does not emit", () => { + beforeEach(() => { + // Arrange + authRequestService.adminLoginApproved$ = EMPTY; + deviceTrustService.deviceTrusted$ = EMPTY; + sut = initService(); + }); + + it("should NOT trigger a toast for login approval", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).not.toHaveBeenCalledWith(loginApprovalToastOptions); // Assert + done(); + }, + }); + }); + + it("should NOT trigger a toast for device trust", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).not.toHaveBeenCalledWith(deviceTrustedToastOptions); // Assert + done(); + }, + }); + }); + }); + }); +}); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 9ee49a30689..93e29846e69 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -317,6 +317,8 @@ import { IndividualVaultExportServiceAbstraction, } from "@bitwarden/vault-export-core"; +import { DeviceTrustToastService as DeviceTrustToastServiceAbstraction } from "../auth/services/device-trust-toast.service.abstraction"; +import { DeviceTrustToastService } from "../auth/services/device-trust-toast.service.implementation"; import { FormValidationErrorsService as FormValidationErrorsServiceAbstraction } from "../platform/abstractions/form-validation-errors.service"; import { ViewCacheService } from "../platform/abstractions/view-cache.service"; import { FormValidationErrorsService } from "../platform/services/form-validation-errors.service"; @@ -1463,6 +1465,16 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultTaskService, deps: [StateProvider, ApiServiceAbstraction, OrganizationServiceAbstraction, ConfigService], }), + safeProvider({ + provide: DeviceTrustToastServiceAbstraction, + useClass: DeviceTrustToastService, + deps: [ + AuthRequestServiceAbstraction, + DeviceTrustServiceAbstraction, + I18nServiceAbstraction, + ToastService, + ], + }), ]; @NgModule({ diff --git a/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts b/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts index 1829fe6b0c9..75bb8686163 100644 --- a/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts +++ b/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts @@ -12,6 +12,12 @@ export abstract class AuthRequestServiceAbstraction { /** Emits an auth request id when an auth request has been approved. */ authRequestPushNotification$: Observable; + /** + * Emits when a login has been approved by an admin. This emission is specifically for the + * purpose of notifying the consuming component to display a toast informing the user. + */ + adminLoginApproved$: Observable; + /** * Returns an admin auth request for the given user if it exists. * @param userId The user id. @@ -106,4 +112,13 @@ export abstract class AuthRequestServiceAbstraction { * @returns The dash-delimited fingerprint phrase. */ abstract getFingerprintPhrase(email: string, publicKey: Uint8Array): Promise; + + /** + * Passes a value to the adminLoginApprovedSubject via next(), which causes the + * adminLoginApproved$ observable to emit. + * + * The purpose is to notify consuming components (of adminLoginApproved$) to display + * a toast informing the user that a login has been approved by an admin. + */ + abstract emitAdminLoginApproved(): void; } diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index e9d1e0a6c88..ac00ff69a4d 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -278,7 +278,8 @@ export class SsoLoginStrategy extends LoginStrategy { // TODO: eventually we post and clean up DB as well once consumed on client await this.authRequestService.clearAdminAuthRequest(userId); - this.platformUtilsService.showToast("success", null, this.i18nService.t("loginApproved")); + // This notification will be picked up by the SsoComponent to handle displaying a toast to the user + this.authRequestService.emitAdminLoginApproved(); } } } diff --git a/libs/auth/src/common/services/auth-request/auth-request.service.ts b/libs/auth/src/common/services/auth-request/auth-request.service.ts index 5bc200ae1e8..a6841afe0ff 100644 --- a/libs/auth/src/common/services/auth-request/auth-request.service.ts +++ b/libs/auth/src/common/services/auth-request/auth-request.service.ts @@ -43,6 +43,10 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { private authRequestPushNotificationSubject = new Subject(); authRequestPushNotification$: Observable; + // Observable emission is used to trigger a toast in consuming components + private adminLoginApprovedSubject = new Subject(); + adminLoginApproved$: Observable; + constructor( private appIdService: AppIdService, private accountService: AccountService, @@ -53,6 +57,7 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { private stateProvider: StateProvider, ) { this.authRequestPushNotification$ = this.authRequestPushNotificationSubject.asObservable(); + this.adminLoginApproved$ = this.adminLoginApprovedSubject.asObservable(); } async getAdminAuthRequest(userId: UserId): Promise { @@ -207,4 +212,8 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { async getFingerprintPhrase(email: string, publicKey: Uint8Array): Promise { return (await this.keyService.getFingerprint(email.toLowerCase(), publicKey)).join("-"); } + + emitAdminLoginApproved(): void { + this.adminLoginApprovedSubject.next(); + } } diff --git a/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts b/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts index 24a5d4e8413..2de63b36cc7 100644 --- a/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts @@ -16,6 +16,12 @@ export abstract class DeviceTrustServiceAbstraction { */ supportsDeviceTrust$: Observable; + /** + * Emits when a device has been trusted. This emission is specifically for the purpose of notifying + * the consuming component to display a toast informing the user the device has been trusted. + */ + deviceTrusted$: Observable; + /** * @description Checks if the device trust feature is supported for the given user. */ diff --git a/libs/common/src/auth/services/device-trust.service.implementation.ts b/libs/common/src/auth/services/device-trust.service.implementation.ts index fe43df53c48..4a1b901d5ef 100644 --- a/libs/common/src/auth/services/device-trust.service.implementation.ts +++ b/libs/common/src/auth/services/device-trust.service.implementation.ts @@ -1,6 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { firstValueFrom, map, Observable } from "rxjs"; +import { firstValueFrom, map, Observable, Subject } from "rxjs"; import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { KeyService } from "@bitwarden/key-management"; @@ -63,6 +63,10 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { supportsDeviceTrust$: Observable; + // Observable emission is used to trigger a toast in consuming components + private deviceTrustedSubject = new Subject(); + deviceTrusted$ = this.deviceTrustedSubject.asObservable(); + constructor( private keyGenerationService: KeyGenerationService, private cryptoFunctionService: CryptoFunctionService, @@ -177,7 +181,8 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { // store device key in local/secure storage if enc keys posted to server successfully await this.setDeviceKey(userId, deviceKey); - this.platformUtilsService.showToast("success", null, this.i18nService.t("deviceTrusted")); + // This emission will be picked up by consuming components to handle displaying a toast to the user + this.deviceTrustedSubject.next(); return deviceResponse; }