From ad2dfe1e99c151e068c58665d75d43389aa581ea Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Tue, 26 Aug 2025 11:41:15 -0400 Subject: [PATCH] feat(notifications): [PM-19388] Enable push notifications on locked clients * Add back notifications connection on locked accounts * Updated tests. * Make sure web push connection service is started synchronously * Fixed merge conflicts. --------- Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> --- .../browser/src/background/main.background.ts | 7 +++-- .../src/services/jslib-services.module.ts | 1 + libs/common/src/enums/feature-flag.enum.ts | 2 ++ .../default-notifications.service.spec.ts | 21 +++++++------- .../default-server-notifications.service.ts | 28 ++++++++++++++----- 5 files changed, 40 insertions(+), 19 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index c2c62625bb6..096bbe76e40 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1125,6 +1125,7 @@ export default class MainBackground { new SignalRConnectionService(this.apiService, this.logService), this.authService, this.webPushConnectionService, + this.configService, ); this.fido2UserInterfaceService = new BrowserFido2UserInterfaceService(this.authService); @@ -1370,12 +1371,14 @@ export default class MainBackground { this.accountService, this.authService, ); - } - async bootstrap() { + // Synchronous startup if (this.webPushConnectionService instanceof WorkerWebPushConnectionService) { this.webPushConnectionService.start(); } + } + + async bootstrap() { this.containerService.attachToGlobal(self); await this.sdkLoadService.loadAndInit(); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index fa7de5484d9..f62407eeafc 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -943,6 +943,7 @@ const safeProviders: SafeProvider[] = [ SignalRConnectionService, AuthServiceAbstraction, WebPushConnectionService, + ConfigService, ], }), safeProvider({ diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 7ab85139f4c..67f68e12847 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -53,6 +53,7 @@ export enum FeatureFlag { /* Platform */ IpcChannelFramework = "ipc-channel-framework", + PushNotificationsWhenLocked = "pm-19388-push-notifications-when-locked", } export type AllowedFeatureFlagTypes = boolean | number | string; @@ -112,6 +113,7 @@ export const DefaultFeatureFlagValue = { /* Platform */ [FeatureFlag.IpcChannelFramework]: FALSE, + [FeatureFlag.PushNotificationsWhenLocked]: FALSE, } satisfies Record; export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue; diff --git a/libs/common/src/platform/server-notifications/internal/default-notifications.service.spec.ts b/libs/common/src/platform/server-notifications/internal/default-notifications.service.spec.ts index 567e0fbfc3d..9f42328d573 100644 --- a/libs/common/src/platform/server-notifications/internal/default-notifications.service.spec.ts +++ b/libs/common/src/platform/server-notifications/internal/default-notifications.service.spec.ts @@ -1,5 +1,5 @@ import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject, bufferCount, firstValueFrom, ObservedValueOf, Subject } from "rxjs"; +import { BehaviorSubject, bufferCount, firstValueFrom, ObservedValueOf, of, Subject } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports @@ -14,6 +14,7 @@ import { NotificationType } from "../../../enums"; import { NotificationResponse } from "../../../models/response/notification.response"; import { UserId } from "../../../types/guid"; import { AppIdService } from "../../abstractions/app-id.service"; +import { ConfigService } from "../../abstractions/config/config.service"; import { Environment, EnvironmentService } from "../../abstractions/environment.service"; import { LogService } from "../../abstractions/log.service"; import { MessageSender } from "../../messaging"; @@ -38,6 +39,7 @@ describe("NotificationsService", () => { let signalRNotificationConnectionService: MockProxy; let authService: MockProxy; let webPushNotificationConnectionService: MockProxy; + let configService: MockProxy; let activeAccount: BehaviorSubject>; @@ -64,6 +66,9 @@ describe("NotificationsService", () => { signalRNotificationConnectionService = mock(); authService = mock(); webPushNotificationConnectionService = mock(); + configService = mock(); + + configService.getFeatureFlag$.mockReturnValue(of(true)); activeAccount = new BehaviorSubject>(null); accountService.activeAccount$ = activeAccount.asObservable(); @@ -104,6 +109,7 @@ describe("NotificationsService", () => { signalRNotificationConnectionService, authService, webPushNotificationConnectionService, + configService, ); }); @@ -227,10 +233,9 @@ describe("NotificationsService", () => { }); it.each([ - // Temporarily rolling back server notifications being connected while locked - // { initialStatus: AuthenticationStatus.Locked, updatedStatus: AuthenticationStatus.Unlocked }, - // { initialStatus: AuthenticationStatus.Unlocked, updatedStatus: AuthenticationStatus.Locked }, - // { initialStatus: AuthenticationStatus.Locked, updatedStatus: AuthenticationStatus.Locked }, + { initialStatus: AuthenticationStatus.Locked, updatedStatus: AuthenticationStatus.Unlocked }, + { initialStatus: AuthenticationStatus.Unlocked, updatedStatus: AuthenticationStatus.Locked }, + { initialStatus: AuthenticationStatus.Locked, updatedStatus: AuthenticationStatus.Locked }, { initialStatus: AuthenticationStatus.Unlocked, updatedStatus: AuthenticationStatus.Unlocked }, ])( "does not re-connect when the user transitions from $initialStatus to $updatedStatus", @@ -255,11 +260,7 @@ describe("NotificationsService", () => { }, ); - it.each([ - // Temporarily disabling server notifications connecting while in a locked state - // AuthenticationStatus.Locked, - AuthenticationStatus.Unlocked, - ])( + it.each([AuthenticationStatus.Locked, AuthenticationStatus.Unlocked])( "connects when a user transitions from logged out to %s", async (newStatus: AuthenticationStatus) => { emitActiveUser(mockUser1); diff --git a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts index 4502d9663a3..d21074f5bbf 100644 --- a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts +++ b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts @@ -14,6 +14,7 @@ import { // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { LogoutReason } from "@bitwarden/auth/common"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { AccountService } from "../../../auth/abstractions/account.service"; import { AuthService } from "../../../auth/abstractions/auth.service"; @@ -28,6 +29,7 @@ import { import { UserId } from "../../../types/guid"; import { SyncService } from "../../../vault/abstractions/sync/sync.service.abstraction"; import { AppIdService } from "../../abstractions/app-id.service"; +import { ConfigService } from "../../abstractions/config/config.service"; import { EnvironmentService } from "../../abstractions/environment.service"; import { LogService } from "../../abstractions/log.service"; import { MessagingService } from "../../abstractions/messaging.service"; @@ -55,6 +57,7 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer private readonly signalRConnectionService: SignalRConnectionService, private readonly authService: AuthService, private readonly webPushConnectionService: WebPushConnectionService, + private readonly configService: ConfigService, ) { this.notifications$ = this.accountService.activeAccount$.pipe( map((account) => account?.id), @@ -132,14 +135,25 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer ); } - // This method name is a lie currently as we also have an access token - // when locked, this is eventually where we want to be but it increases load - // on signalR so we are rolling back until we can move the load of browser to - // web push. private hasAccessToken$(userId: UserId) { - return this.authService.authStatusFor$(userId).pipe( - map((authStatus) => authStatus === AuthenticationStatus.Unlocked), - distinctUntilChanged(), + return this.configService.getFeatureFlag$(FeatureFlag.PushNotificationsWhenLocked).pipe( + switchMap((featureFlagEnabled) => { + if (featureFlagEnabled) { + return this.authService.authStatusFor$(userId).pipe( + map( + (authStatus) => + authStatus === AuthenticationStatus.Locked || + authStatus === AuthenticationStatus.Unlocked, + ), + distinctUntilChanged(), + ); + } else { + return this.authService.authStatusFor$(userId).pipe( + map((authStatus) => authStatus === AuthenticationStatus.Unlocked), + distinctUntilChanged(), + ); + } + }), ); }