From b9d5724312f49ff26133ad127cfff2e59ef19e98 Mon Sep 17 00:00:00 2001 From: Brandon Treston Date: Mon, 1 Dec 2025 10:21:48 -0500 Subject: [PATCH] [PM-24011] Add handler for new policy sync push notification (#17465) * add handler for new policy sync push notification * fix story book build failure * move logic into policy service, fix tests * add account service * add missing service to clie --- .../browser/src/background/main.background.ts | 7 +- .../service-container/service-container.ts | 6 +- .../src/services/jslib-services.module.ts | 3 +- .../policy/policy.service.abstraction.ts | 5 ++ .../policy/default-policy.service.spec.ts | 10 +-- .../services/policy/default-policy.service.ts | 15 ++++- .../src/enums/notification-type.enum.ts | 2 + .../models/response/notification.response.ts | 13 ++++ ...ult-server-notifications.multiuser.spec.ts | 5 ++ ...fault-server-notifications.service.spec.ts | 67 +++++++++++++++++++ .../default-server-notifications.service.ts | 6 ++ 11 files changed, 131 insertions(+), 8 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 78b5e323798..143f5d1f6b3 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -732,7 +732,11 @@ export default class MainBackground { this.singleUserStateProvider, ); this.organizationService = new DefaultOrganizationService(this.stateProvider); - this.policyService = new DefaultPolicyService(this.stateProvider, this.organizationService); + this.policyService = new DefaultPolicyService( + this.stateProvider, + this.organizationService, + this.accountService, + ); this.vaultTimeoutSettingsService = new DefaultVaultTimeoutSettingsService( this.accountService, @@ -1196,6 +1200,7 @@ export default class MainBackground { this.webPushConnectionService, this.authRequestAnsweringService, this.configService, + this.policyService, ); this.fido2UserInterfaceService = new BrowserFido2UserInterfaceService(this.authService); diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 122dd6ea052..c163b7581b4 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -518,7 +518,11 @@ export class ServiceContainer { this.ssoUrlService = new SsoUrlService(); this.organizationService = new DefaultOrganizationService(this.stateProvider); - this.policyService = new DefaultPolicyService(this.stateProvider, this.organizationService); + this.policyService = new DefaultPolicyService( + this.stateProvider, + this.organizationService, + this.accountService, + ); this.vaultTimeoutSettingsService = new DefaultVaultTimeoutSettingsService( this.accountService, diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index c8a70cf5af6..1589f5c5f30 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1026,6 +1026,7 @@ const safeProviders: SafeProvider[] = [ WebPushConnectionService, AuthRequestAnsweringServiceAbstraction, ConfigService, + InternalPolicyService, ], }), safeProvider({ @@ -1064,7 +1065,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: InternalPolicyService, useClass: DefaultPolicyService, - deps: [StateProvider, OrganizationServiceAbstraction], + deps: [StateProvider, OrganizationServiceAbstraction, AccountServiceAbstraction], }), safeProvider({ provide: PolicyServiceAbstraction, diff --git a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts index 8df7e44986b..ec67ed96d6e 100644 --- a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts @@ -101,4 +101,9 @@ export abstract class InternalPolicyService extends PolicyService { * Replace a policy in the local sync data. This does not update any policies on the server. */ abstract replace: (policies: { [id: string]: PolicyData }, userId: UserId) => Promise; + /** + * Wrapper around upsert that uses account service to sync policies for the logged in user. This comes from + * the server push notification to update local policies. + */ + abstract syncPolicy: (payload: PolicyData) => Promise; } diff --git a/libs/common/src/admin-console/services/policy/default-policy.service.spec.ts b/libs/common/src/admin-console/services/policy/default-policy.service.spec.ts index 8ce1a785516..4b59683ec0a 100644 --- a/libs/common/src/admin-console/services/policy/default-policy.service.spec.ts +++ b/libs/common/src/admin-console/services/policy/default-policy.service.spec.ts @@ -1,6 +1,8 @@ import { mock, MockProxy } from "jest-mock-extended"; import { firstValueFrom, of } from "rxjs"; +import { newGuid } from "@bitwarden/guid"; + import { FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; import { FakeSingleUserState } from "../../../../spec/fake-state"; import { @@ -22,15 +24,15 @@ import { DefaultPolicyService, getFirstPolicy } from "./default-policy.service"; import { POLICIES } from "./policy-state"; describe("PolicyService", () => { - const userId = "userId" as UserId; + const userId = newGuid() as UserId; let stateProvider: FakeStateProvider; let organizationService: MockProxy; let singleUserState: FakeSingleUserState>; + const accountService = mockAccountServiceWith(userId); let policyService: DefaultPolicyService; beforeEach(() => { - const accountService = mockAccountServiceWith(userId); stateProvider = new FakeStateProvider(accountService); organizationService = mock(); singleUserState = stateProvider.singleUser.getFake(userId, POLICIES); @@ -59,7 +61,7 @@ describe("PolicyService", () => { organizationService.organizations$.calledWith(userId).mockReturnValue(organizations$); - policyService = new DefaultPolicyService(stateProvider, organizationService); + policyService = new DefaultPolicyService(stateProvider, organizationService, accountService); }); it("upsert", async () => { @@ -635,7 +637,7 @@ describe("PolicyService", () => { beforeEach(() => { stateProvider = new FakeStateProvider(mockAccountServiceWith(userId)); organizationService = mock(); - policyService = new DefaultPolicyService(stateProvider, organizationService); + policyService = new DefaultPolicyService(stateProvider, organizationService, accountService); }); it("returns undefined when there are no policies", () => { diff --git a/libs/common/src/admin-console/services/policy/default-policy.service.ts b/libs/common/src/admin-console/services/policy/default-policy.service.ts index b9d7655195b..ac3ccbc8ca0 100644 --- a/libs/common/src/admin-console/services/policy/default-policy.service.ts +++ b/libs/common/src/admin-console/services/policy/default-policy.service.ts @@ -1,4 +1,7 @@ -import { combineLatest, map, Observable, of } from "rxjs"; +import { combineLatest, firstValueFrom, map, Observable, of, switchMap } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { StateProvider } from "../../../platform/state"; import { UserId } from "../../../types/guid"; @@ -25,6 +28,7 @@ export class DefaultPolicyService implements PolicyService { constructor( private stateProvider: StateProvider, private organizationService: OrganizationService, + private accountService: AccountService, ) {} private policyState(userId: UserId) { @@ -326,4 +330,13 @@ export class DefaultPolicyService implements PolicyService { target.enforceOnLogin = Boolean(target.enforceOnLogin || source.enforceOnLogin); } } + + async syncPolicy(policyData: PolicyData) { + await firstValueFrom( + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.upsert(policyData, userId)), + ), + ); + } } diff --git a/libs/common/src/enums/notification-type.enum.ts b/libs/common/src/enums/notification-type.enum.ts index e43b4612203..a10e6bf4448 100644 --- a/libs/common/src/enums/notification-type.enum.ts +++ b/libs/common/src/enums/notification-type.enum.ts @@ -33,4 +33,6 @@ export enum NotificationType { OrganizationBankAccountVerified = 23, ProviderBankAccountVerified = 24, + + SyncPolicy = 25, } diff --git a/libs/common/src/models/response/notification.response.ts b/libs/common/src/models/response/notification.response.ts index 167864208ee..2c0c0aae3f1 100644 --- a/libs/common/src/models/response/notification.response.ts +++ b/libs/common/src/models/response/notification.response.ts @@ -1,3 +1,4 @@ +import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { NotificationViewResponse as EndUserNotificationResponse } from "@bitwarden/common/vault/notifications/models"; import { NotificationType, PushNotificationLogOutReasonType } from "../../enums"; @@ -71,6 +72,9 @@ export class NotificationResponse extends BaseResponse { case NotificationType.ProviderBankAccountVerified: this.payload = new ProviderBankAccountVerifiedPushNotification(payload); break; + case NotificationType.SyncPolicy: + this.payload = new SyncPolicyNotification(payload); + break; default: break; } @@ -187,6 +191,15 @@ export class ProviderBankAccountVerifiedPushNotification extends BaseResponse { } } +export class SyncPolicyNotification extends BaseResponse { + policy: Policy; + + constructor(response: any) { + super(response); + this.policy = this.getResponseProperty("Policy"); + } +} + export class LogOutNotification extends BaseResponse { userId: string; reason?: PushNotificationLogOutReasonType; diff --git a/libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts b/libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts index b4d47698e4d..cd1bf97150c 100644 --- a/libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts +++ b/libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts @@ -3,6 +3,7 @@ import { BehaviorSubject, bufferCount, firstValueFrom, Subject, ObservedValueOf // eslint-disable-next-line no-restricted-imports import { LogoutReason } from "@bitwarden/auth/common"; +import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; @@ -34,6 +35,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => { let webPushNotificationConnectionService: MockProxy; let authRequestAnsweringService: MockProxy; let configService: MockProxy; + let policyService: MockProxy; let activeUserAccount$: BehaviorSubject>; let userAccounts$: BehaviorSubject>; @@ -136,6 +138,8 @@ describe("DefaultServerNotificationsService (multi-user)", () => { return new BehaviorSubject(flagValueByFlag[flag] ?? false) as any; }); + policyService = mock(); + defaultServerNotificationsService = new DefaultServerNotificationsService( mock(), syncService, @@ -149,6 +153,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => { webPushNotificationConnectionService, authRequestAnsweringService, configService, + policyService, ); }); diff --git a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts index b2aa4fbd315..4a9b0809ac9 100644 --- a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts +++ b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts @@ -4,6 +4,8 @@ import { BehaviorSubject, bufferCount, firstValueFrom, ObservedValueOf, of, Subj // 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 { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { awaitAsync } from "../../../../spec"; @@ -42,6 +44,7 @@ describe("NotificationsService", () => { let webPushNotificationConnectionService: MockProxy; let authRequestAnsweringService: MockProxy; let configService: MockProxy; + let policyService: MockProxy; let activeAccount: BehaviorSubject>; let accounts: BehaviorSubject>; @@ -71,6 +74,7 @@ describe("NotificationsService", () => { webPushNotificationConnectionService = mock(); authRequestAnsweringService = mock(); configService = mock(); + policyService = mock(); // For these tests, use the active-user implementation (feature flag disabled) configService.getFeatureFlag$.mockImplementation(() => of(true)); @@ -123,6 +127,7 @@ describe("NotificationsService", () => { webPushNotificationConnectionService, authRequestAnsweringService, configService, + policyService, ); }); @@ -391,5 +396,67 @@ describe("NotificationsService", () => { expect(logoutCallback).not.toHaveBeenCalled(); }); }); + + describe("NotificationType.SyncPolicy", () => { + it("should call policyService.syncPolicy with the policy from the notification", async () => { + const mockPolicy = { + id: "policy-id", + organizationId: "org-id", + type: PolicyType.TwoFactorAuthentication, + enabled: true, + data: { test: "data" }, + }; + + policyService.syncPolicy.mockResolvedValue(); + + const notification = new NotificationResponse({ + type: NotificationType.SyncPolicy, + payload: { policy: mockPolicy }, + contextId: "different-app-id", + }); + + await sut["processNotification"](notification, mockUser1); + + expect(policyService.syncPolicy).toHaveBeenCalledTimes(1); + expect(policyService.syncPolicy).toHaveBeenCalledWith( + expect.objectContaining({ + id: mockPolicy.id, + organizationId: mockPolicy.organizationId, + type: mockPolicy.type, + enabled: mockPolicy.enabled, + data: mockPolicy.data, + }), + ); + }); + + it("should handle SyncPolicy notification with minimal policy data", async () => { + const mockPolicy = { + id: "policy-id-2", + organizationId: "org-id-2", + type: PolicyType.RequireSso, + enabled: false, + }; + + policyService.syncPolicy.mockResolvedValue(); + + const notification = new NotificationResponse({ + type: NotificationType.SyncPolicy, + payload: { policy: mockPolicy }, + contextId: "different-app-id", + }); + + await sut["processNotification"](notification, mockUser1); + + expect(policyService.syncPolicy).toHaveBeenCalledTimes(1); + expect(policyService.syncPolicy).toHaveBeenCalledWith( + expect.objectContaining({ + id: mockPolicy.id, + organizationId: mockPolicy.organizationId, + type: mockPolicy.type, + enabled: mockPolicy.enabled, + }), + ); + }); + }); }); }); 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 efe0a8ae408..5ee288351d5 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 @@ -15,6 +15,8 @@ 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 { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyData } from "@bitwarden/common/admin-console/models/data/policy.data"; 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 { trackedMerge } from "@bitwarden/common/platform/misc"; @@ -67,6 +69,7 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer private readonly webPushConnectionService: WebPushConnectionService, private readonly authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction, private readonly configService: ConfigService, + private readonly policyService: InternalPolicyService, ) { this.notifications$ = this.configService .getFeatureFlag$(FeatureFlag.InactiveUserServerNotification) @@ -330,6 +333,9 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer adminId: notification.payload.adminId, }); break; + case NotificationType.SyncPolicy: + await this.policyService.syncPolicy(PolicyData.fromPolicy(notification.payload.policy)); + break; default: break; }