1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-19 19:04:01 +00:00

fix(inactive-user-server-notification): [PM-25130] Inactive User Server Notify - Fixed tests and updated with Justin's comments.

This commit is contained in:
Patrick Pimentel
2025-08-28 16:35:19 -04:00
parent 805d58ad30
commit 865ffa2739
4 changed files with 24 additions and 21 deletions

View File

@@ -0,0 +1,5 @@
import { NotificationType } from "@bitwarden/common/enums";
export const AllowedMultiUserNotificationTypes = new Set<NotificationType>([
NotificationType.AuthRequest,
]);

View File

@@ -3,9 +3,8 @@ import { BehaviorSubject, bufferCount, firstValueFrom, Subject, ObservedValueOf
// eslint-disable-next-line no-restricted-imports
import { LogoutReason } from "@bitwarden/auth/common";
import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
// TODO: When PM-14943 goes in, uncomment
// import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
import { AccountService } from "../../../auth/abstractions/account.service";
import { AuthService } from "../../../auth/abstractions/auth.service";
@@ -33,8 +32,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
let signalRNotificationConnectionService: MockProxy<SignalRConnectionService>;
let authService: MockProxy<AuthService>;
let webPushNotificationConnectionService: MockProxy<WebPushConnectionService>;
// TODO: When PM-14943 goes in, uncomment
// let authRequestAnsweringService: MockProxy<AuthRequestAnsweringServiceAbstraction>;
let authRequestAnsweringService: MockProxy<AuthRequestAnsweringServiceAbstraction>;
let configService: MockProxy<ConfigService>;
let activeUserAccount$: BehaviorSubject<ObservedValueOf<AccountService["activeAccount$"]>>;
@@ -75,6 +73,10 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
getNotificationsUrl: () => "http://test.example.com",
} as Environment);
environmentConfigurationService.environment$ = environmentConfiguration$ as any;
// Ensure user-scoped environment lookups return the same test environment stream
environmentConfigurationService.getEnvironment$.mockImplementation(
(_userId: UserId) => environmentConfiguration$.asObservable() as any,
);
userLogoutCallback = jest.fn<Promise<void>, [LogoutReason, UserId]>();
@@ -123,8 +125,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
return webPushSupportStatusByUser.get(userId)!.asObservable();
});
// TODO: When PM-14943 goes in, uncomment
// authRequestAnsweringService = mock<AuthRequestAnsweringServiceAbstraction>();
authRequestAnsweringService = mock<AuthRequestAnsweringServiceAbstraction>();
configService = mock<ConfigService>();
configService.getFeatureFlag$.mockImplementation((flag: FeatureFlag) => {
@@ -145,7 +146,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
signalRNotificationConnectionService,
authService,
webPushNotificationConnectionService,
// authRequestAnsweringService,
authRequestAnsweringService,
configService,
);
});

View File

@@ -1,11 +1,10 @@
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
import { LogoutReason } from "@bitwarden/auth/common";
import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { awaitAsync } from "../../../../spec";
import { Matrix } from "../../../../spec/matrix";
@@ -74,13 +73,7 @@ describe("NotificationsService", () => {
configService = mock<ConfigService>();
// For these tests, use the active-user implementation (feature flag disabled)
configService.getFeatureFlag$.mockImplementation((flag: FeatureFlag) => {
const flagValueByFlag: Partial<Record<FeatureFlag, boolean>> = {
[FeatureFlag.InactiveUserServerNotification]: false,
[FeatureFlag.PushNotificationsWhenLocked]: true,
};
return new BehaviorSubject(flagValueByFlag[flag] ?? false) as any;
});
configService.getFeatureFlag$.mockImplementation(() => of(true));
activeAccount = new BehaviorSubject<ObservedValueOf<AccountService["activeAccount$"]>>(null);
accountService.activeAccount$ = activeAccount.asObservable();
@@ -93,6 +86,10 @@ describe("NotificationsService", () => {
} as Environment);
environmentService.environment$ = environment;
// Ensure user-scoped environment lookups return the same test environment stream
environmentService.getEnvironment$.mockImplementation(
(_userId: UserId) => environment.asObservable() as any,
);
authStatusGetter = Matrix.autoMockMethod(
authService.authStatusFor$,

View File

@@ -18,6 +18,7 @@ import {
import { LogoutReason } from "@bitwarden/auth/common";
import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { AllowedMultiUserNotificationTypes } from "@bitwarden/common/platform/enums/multiUserAllowList.enum";
import { AccountService } from "../../../auth/abstractions/account.service";
import { AuthService } from "../../../auth/abstractions/auth.service";
@@ -66,10 +67,12 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer
this.notifications$ = this.configService
.getFeatureFlag$(FeatureFlag.InactiveUserServerNotification)
.pipe(
distinctUntilChanged(),
switchMap((inactiveUserServerNotificationEnabled) => {
if (inactiveUserServerNotificationEnabled) {
return this.accountService.accounts$.pipe(
map((accounts) => Object.keys(accounts) as UserId[]),
distinctUntilChanged(),
switchMap((userIds) => {
if (userIds.length === 0) {
return EMPTY;
@@ -110,7 +113,7 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer
* @param userId The user id of the user to get the push server notifications for.
*/
private userNotifications$(userId: UserId) {
return this.environmentService.environment$.pipe(
return this.environmentService.getEnvironment$(userId).pipe(
map((env) => env.getNotificationsUrl()),
distinctUntilChanged(),
switchMap((notificationsUrl) => {
@@ -202,15 +205,12 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer
this.configService.getFeatureFlag$(FeatureFlag.InactiveUserServerNotification),
)
) {
// Allow-list of notification types that are safe to process for non-active users
const multiUserNotificationTypes = new Set<NotificationType>([NotificationType.AuthRequest]);
const activeAccountId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const isActiveUser = activeAccountId === userId;
if (!isActiveUser && !multiUserNotificationTypes.has(notification.type)) {
if (!isActiveUser && !AllowedMultiUserNotificationTypes.has(notification.type)) {
return;
}
}