1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 13:23:34 +00:00

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>
This commit is contained in:
Todd Martin
2025-08-26 11:41:15 -04:00
committed by GitHub
parent fb4f87958d
commit ad2dfe1e99
5 changed files with 40 additions and 19 deletions

View File

@@ -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();

View File

@@ -943,6 +943,7 @@ const safeProviders: SafeProvider[] = [
SignalRConnectionService,
AuthServiceAbstraction,
WebPushConnectionService,
ConfigService,
],
}),
safeProvider({

View File

@@ -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<FeatureFlag, AllowedFeatureFlagTypes>;
export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue;

View File

@@ -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<SignalRConnectionService>;
let authService: MockProxy<AuthService>;
let webPushNotificationConnectionService: MockProxy<WebPushConnectionService>;
let configService: MockProxy<ConfigService>;
let activeAccount: BehaviorSubject<ObservedValueOf<AccountService["activeAccount$"]>>;
@@ -64,6 +66,9 @@ describe("NotificationsService", () => {
signalRNotificationConnectionService = mock<SignalRConnectionService>();
authService = mock<AuthService>();
webPushNotificationConnectionService = mock<WorkerWebPushConnectionService>();
configService = mock<ConfigService>();
configService.getFeatureFlag$.mockReturnValue(of(true));
activeAccount = new BehaviorSubject<ObservedValueOf<AccountService["activeAccount$"]>>(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);

View File

@@ -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,16 +135,27 @@ 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.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(),
);
}
}),
);
}
private async processNotification(notification: NotificationResponse, userId: UserId) {
const appId = await this.appIdService.getAppId();