From 3855332279852e85ab6203ffe46a576c94134405 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:48:44 -0600 Subject: [PATCH] [PM-31431] Refactor vault prompts (#18740) * move existing prompting into separate service for the web vault * add unit tests for web vault prompt service * add provider * remove `autoConfirmDialogRef` * rename auto confirm dialog --- .../vault/individual-vault/vault.component.ts | 90 +------ .../services/web-vault-prompt.service.spec.ts | 234 ++++++++++++++++++ .../services/web-vault-prompt.service.ts | 113 +++++++++ 3 files changed, 351 insertions(+), 86 deletions(-) create mode 100644 apps/web/src/app/vault/services/web-vault-prompt.service.spec.ts create mode 100644 apps/web/src/app/vault/services/web-vault-prompt.service.ts diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index fe4b7f1f96f..65846810cd2 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -9,7 +9,6 @@ import { lastValueFrom, Observable, Subject, - zip, } from "rxjs"; import { concatMap, @@ -35,7 +34,6 @@ import { ItemTypes, BitSvg, } from "@bitwarden/assets/svg"; -import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { @@ -60,9 +58,7 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions/billing-api.service.abstraction"; import { EventType } from "@bitwarden/common/enums"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -113,15 +109,9 @@ import { VaultItemsTransferService, DefaultVaultItemsTransferService, } from "@bitwarden/vault"; -import { UnifiedUpgradePromptService } from "@bitwarden/web-vault/app/billing/individual/upgrade/services"; import { OrganizationWarningsModule } from "@bitwarden/web-vault/app/billing/organizations/warnings/organization-warnings.module"; import { OrganizationWarningsService } from "@bitwarden/web-vault/app/billing/organizations/warnings/services"; -import { - AutoConfirmPolicy, - AutoConfirmPolicyDialogComponent, - PolicyEditDialogResult, -} from "../../admin-console/organizations/policies"; import { CollectionDialogAction, CollectionDialogTabType, @@ -138,6 +128,7 @@ import { VaultItem } from "../components/vault-items/vault-item"; import { VaultItemEvent } from "../components/vault-items/vault-item-event"; import { VaultItemsComponent } from "../components/vault-items/vault-items.component"; import { VaultItemsModule } from "../components/vault-items/vault-items.module"; +import { WebVaultPromptService } from "../services/web-vault-prompt.service"; import { BulkDeleteDialogResult, @@ -183,6 +174,7 @@ type EmptyStateMap = Record; RoutedVaultFilterService, RoutedVaultFilterBridgeService, DefaultCipherFormConfigService, + WebVaultPromptService, { provide: VaultItemsTransferService, useClass: DefaultVaultItemsTransferService }, ], }) @@ -195,7 +187,6 @@ export class VaultComponent implements OnInit, OnDestr @ViewChild("vaultItems", { static: false }) vaultItemsComponent: VaultItemsComponent; trashCleanupWarning: string = null; - kdfIterations: number; activeFilter: VaultFilter = new VaultFilter(); protected deactivatedOrgIcon = DeactivatedOrg; @@ -224,7 +215,6 @@ export class VaultComponent implements OnInit, OnDestr private destroy$ = new Subject(); private vaultItemDialogRef?: DialogRef | undefined; - private autoConfirmDialogRef?: DialogRef | undefined; protected showAddCipherBtn: boolean = false; @@ -346,11 +336,8 @@ export class VaultComponent implements OnInit, OnDestr private cipherArchiveService: CipherArchiveService, private organizationWarningsService: OrganizationWarningsService, private policyService: PolicyService, - private unifiedUpgradePromptService: UnifiedUpgradePromptService, private premiumUpgradePromptService: PremiumUpgradePromptService, - private autoConfirmService: AutomaticUserConfirmationService, - private configService: ConfigService, - private vaultItemTransferService: VaultItemsTransferService, + private webVaultPromptService: WebVaultPromptService, ) {} async ngOnInit() { @@ -646,11 +633,8 @@ export class VaultComponent implements OnInit, OnDestr this.changeDetectorRef.markForCheck(); }, ); - void this.unifiedUpgradePromptService.displayUpgradePromptConditionally(); - this.setupAutoConfirm(); - - void this.vaultItemTransferService.enforceOrganizationDataOwnership(activeUserId); + void this.webVaultPromptService.conditionallyPromptUser(); } ngOnDestroy() { @@ -1608,72 +1592,6 @@ export class VaultComponent implements OnInit, OnDestr const cipherView = await this.cipherService.decrypt(_cipher, activeUserId); return cipherView.login?.password; } - - private async openAutoConfirmFeatureDialog(organization: Organization) { - if (this.autoConfirmDialogRef) { - return; - } - - this.autoConfirmDialogRef = AutoConfirmPolicyDialogComponent.open(this.dialogService, { - data: { - policy: new AutoConfirmPolicy(), - organizationId: organization.id, - firstTimeDialog: true, - }, - }); - - await lastValueFrom(this.autoConfirmDialogRef.closed); - this.autoConfirmDialogRef = undefined; - } - - private setupAutoConfirm() { - // if the policy is enabled, then the user may only belong to one organization at most. - const organization$ = this.organizations$.pipe(map((organizations) => organizations[0])); - - const featureFlag$ = this.configService.getFeatureFlag$(FeatureFlag.AutoConfirm); - - const autoConfirmState$ = this.userId$.pipe( - switchMap((userId) => this.autoConfirmService.configuration$(userId)), - ); - - const policyEnabled$ = combineLatest([ - this.userId$.pipe( - switchMap((userId) => this.policyService.policies$(userId)), - map((policies) => policies.find((p) => p.type === PolicyType.AutoConfirm && p.enabled)), - ), - organization$, - ]).pipe( - map( - ([policy, organization]) => (policy && policy.organizationId === organization?.id) ?? false, - ), - ); - - zip([organization$, featureFlag$, autoConfirmState$, policyEnabled$, this.userId$]) - .pipe( - first(), - switchMap(async ([organization, flagEnabled, autoConfirmState, policyEnabled, userId]) => { - const showDialog = - flagEnabled && - !policyEnabled && - autoConfirmState.showSetupDialog && - !!organization && - organization.canEnableAutoConfirmPolicy; - - if (showDialog) { - await this.openAutoConfirmFeatureDialog(organization); - - await this.autoConfirmService.upsert(userId, { - ...autoConfirmState, - showSetupDialog: false, - }); - } - }), - takeUntil(this.destroy$), - ) - .subscribe({ - error: (err: unknown) => this.logService.error("Failed to update auto-confirm state", err), - }); - } } /** diff --git a/apps/web/src/app/vault/services/web-vault-prompt.service.spec.ts b/apps/web/src/app/vault/services/web-vault-prompt.service.spec.ts new file mode 100644 index 00000000000..a224b8e7c4b --- /dev/null +++ b/apps/web/src/app/vault/services/web-vault-prompt.service.spec.ts @@ -0,0 +1,234 @@ +import { fakeAsync, TestBed, tick } from "@angular/core/testing"; +import { BehaviorSubject, of } from "rxjs"; + +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { UserId } from "@bitwarden/common/types/guid"; +import { DialogRef, DialogService } from "@bitwarden/components"; +import { LogService } from "@bitwarden/logging"; +import { VaultItemsTransferService } from "@bitwarden/vault"; + +import { + AutoConfirmPolicyDialogComponent, + PolicyEditDialogResult, +} from "../../admin-console/organizations/policies"; +import { UnifiedUpgradePromptService } from "../../billing/individual/upgrade/services"; + +import { WebVaultPromptService } from "./web-vault-prompt.service"; + +describe("WebVaultPromptService", () => { + let service: WebVaultPromptService; + + const mockUserId = "user-123" as UserId; + const mockOrganizationId = "org-456"; + + const getFeatureFlag$ = jest.fn().mockReturnValue(of(false)); + const open = jest.fn(); + const policies$ = jest.fn().mockReturnValue(of([])); + const configurationAutoConfirm$ = jest + .fn() + .mockReturnValue( + of({ showSetupDialog: false, enabled: false, showBrowserNotification: false }), + ); + const upsertAutoConfirm = jest.fn().mockResolvedValue(undefined); + const organizations$ = jest.fn().mockReturnValue(of([])); + const displayUpgradePromptConditionally = jest.fn().mockResolvedValue(undefined); + const enforceOrganizationDataOwnership = jest.fn().mockResolvedValue(undefined); + const logError = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + + TestBed.configureTestingModule({ + providers: [ + WebVaultPromptService, + { provide: UnifiedUpgradePromptService, useValue: { displayUpgradePromptConditionally } }, + { provide: VaultItemsTransferService, useValue: { enforceOrganizationDataOwnership } }, + { provide: PolicyService, useValue: { policies$ } }, + { provide: AccountService, useValue: { activeAccount$: of({ id: mockUserId }) } }, + { + provide: AutomaticUserConfirmationService, + useValue: { configuration$: configurationAutoConfirm$, upsert: upsertAutoConfirm }, + }, + { provide: OrganizationService, useValue: { organizations$ } }, + { provide: ConfigService, useValue: { getFeatureFlag$ } }, + { provide: DialogService, useValue: { open } }, + { provide: LogService, useValue: { error: logError } }, + ], + }); + + service = TestBed.inject(WebVaultPromptService); + }); + + describe("conditionallyPromptUser", () => { + it("calls displayUpgradePromptConditionally", async () => { + await service.conditionallyPromptUser(); + + expect( + service["unifiedUpgradePromptService"].displayUpgradePromptConditionally, + ).toHaveBeenCalled(); + }); + + it("calls enforceOrganizationDataOwnership with the userId", async () => { + await service.conditionallyPromptUser(); + + expect( + service["vaultItemTransferService"].enforceOrganizationDataOwnership, + ).toHaveBeenCalledWith(mockUserId); + }); + }); + + describe("setupAutoConfirm", () => { + it("shows dialog when all conditions are met", fakeAsync(() => { + getFeatureFlag$.mockReturnValueOnce(of(true)); + configurationAutoConfirm$.mockReturnValueOnce( + of({ showSetupDialog: true, enabled: false, showBrowserNotification: false }), + ); + policies$.mockReturnValueOnce(of([])); + + const mockOrg = { + id: mockOrganizationId, + canManagePolicies: true, + canEnableAutoConfirmPolicy: true, + } as Organization; + organizations$.mockReturnValueOnce(of([mockOrg])); + + const dialogClosedSubject = new BehaviorSubject(null); + const dialogRefMock = { + closed: dialogClosedSubject.asObservable(), + } as unknown as DialogRef; + + const openSpy = jest + .spyOn(AutoConfirmPolicyDialogComponent, "open") + .mockReturnValue(dialogRefMock); + + void service.conditionallyPromptUser(); + + tick(); + + expect(openSpy).toHaveBeenCalledWith(expect.anything(), { + data: { + policy: expect.any(Object), + organizationId: mockOrganizationId, + firstTimeDialog: true, + }, + }); + + dialogClosedSubject.next(null); + })); + + it("does not show dialog when feature flag is disabled", fakeAsync(() => { + getFeatureFlag$.mockReturnValueOnce(of(false)); + configurationAutoConfirm$.mockReturnValueOnce( + of({ showSetupDialog: true, enabled: false, showBrowserNotification: false }), + ); + policies$.mockReturnValueOnce(of([])); + + const mockOrg = { + id: mockOrganizationId, + } as Organization; + organizations$.mockReturnValueOnce(of([mockOrg])); + + const openSpy = jest.spyOn(AutoConfirmPolicyDialogComponent, "open"); + + void service.conditionallyPromptUser(); + + tick(); + + expect(openSpy).not.toHaveBeenCalled(); + })); + + it("does not show dialog when policy is already enabled", fakeAsync(() => { + getFeatureFlag$.mockReturnValueOnce(of(true)); + configurationAutoConfirm$.mockReturnValueOnce( + of({ showSetupDialog: true, enabled: false, showBrowserNotification: false }), + ); + + const mockPolicy = { + type: PolicyType.AutoConfirm, + enabled: true, + } as Policy; + policies$.mockReturnValueOnce(of([mockPolicy])); + + const mockOrg = { + id: mockOrganizationId, + } as Organization; + organizations$.mockReturnValueOnce(of([mockOrg])); + + const openSpy = jest.spyOn(AutoConfirmPolicyDialogComponent, "open"); + + void service.conditionallyPromptUser(); + + tick(); + + expect(openSpy).not.toHaveBeenCalled(); + })); + + it("does not show dialog when showSetupDialog is false", fakeAsync(() => { + getFeatureFlag$.mockReturnValueOnce(of(true)); + configurationAutoConfirm$.mockReturnValueOnce( + of({ showSetupDialog: false, enabled: false, showBrowserNotification: false }), + ); + policies$.mockReturnValueOnce(of([])); + + const mockOrg = { + id: mockOrganizationId, + } as Organization; + organizations$.mockReturnValueOnce(of([mockOrg])); + + const openSpy = jest.spyOn(AutoConfirmPolicyDialogComponent, "open"); + + void service.conditionallyPromptUser(); + + tick(); + + expect(openSpy).not.toHaveBeenCalled(); + })); + + it("does not show dialog when organization is undefined", fakeAsync(() => { + getFeatureFlag$.mockReturnValueOnce(of(true)); + configurationAutoConfirm$.mockReturnValueOnce( + of({ showSetupDialog: true, enabled: false, showBrowserNotification: false }), + ); + policies$.mockReturnValueOnce(of([])); + organizations$.mockReturnValueOnce(of([])); + + const openSpy = jest.spyOn(AutoConfirmPolicyDialogComponent, "open"); + + void service.conditionallyPromptUser(); + + tick(); + + expect(openSpy).not.toHaveBeenCalled(); + })); + + it("does not show dialog when organization cannot enable auto-confirm policy", fakeAsync(() => { + getFeatureFlag$.mockReturnValueOnce(of(true)); + configurationAutoConfirm$.mockReturnValueOnce( + of({ showSetupDialog: true, enabled: false, showBrowserNotification: false }), + ); + policies$.mockReturnValueOnce(of([])); + + const mockOrg = { + id: mockOrganizationId, + canManagePolicies: false, + } as Organization; + + organizations$.mockReturnValueOnce(of([mockOrg])); + + const openSpy = jest.spyOn(AutoConfirmPolicyDialogComponent, "open"); + + void service.conditionallyPromptUser(); + + tick(); + + expect(openSpy).not.toHaveBeenCalled(); + })); + }); +}); diff --git a/apps/web/src/app/vault/services/web-vault-prompt.service.ts b/apps/web/src/app/vault/services/web-vault-prompt.service.ts new file mode 100644 index 00000000000..1774bfcc085 --- /dev/null +++ b/apps/web/src/app/vault/services/web-vault-prompt.service.ts @@ -0,0 +1,113 @@ +import { inject, Injectable } from "@angular/core"; +import { map, switchMap, combineLatest, zip, first, firstValueFrom } from "rxjs"; + +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { DialogService } from "@bitwarden/components"; +import { LogService } from "@bitwarden/logging"; +import { VaultItemsTransferService } from "@bitwarden/vault"; + +import { + AutoConfirmPolicyDialogComponent, + AutoConfirmPolicy, +} from "../../admin-console/organizations/policies"; +import { UnifiedUpgradePromptService } from "../../billing/individual/upgrade/services"; + +@Injectable() +export class WebVaultPromptService { + private unifiedUpgradePromptService = inject(UnifiedUpgradePromptService); + private vaultItemTransferService = inject(VaultItemsTransferService); + private policyService = inject(PolicyService); + private accountService = inject(AccountService); + private autoConfirmService = inject(AutomaticUserConfirmationService); + private organizationService = inject(OrganizationService); + private configService = inject(ConfigService); + private dialogService = inject(DialogService); + private logService = inject(LogService); + + private userId$ = this.accountService.activeAccount$.pipe(getUserId); + + private organizations$ = this.userId$.pipe( + switchMap((id) => this.organizationService.organizations$(id)), + ); + + /** + * Conditionally initiates prompts for users. + * All logic for users should be handled within this method to avoid + * the user seeing multiple onboarding prompts at different times. + */ + async conditionallyPromptUser() { + const userId = await firstValueFrom(this.userId$); + + void this.unifiedUpgradePromptService.displayUpgradePromptConditionally(); + + void this.vaultItemTransferService.enforceOrganizationDataOwnership(userId); + + this.checkForAutoConfirm(); + } + + private async openAutoConfirmFeatureDialog(organization: Organization) { + AutoConfirmPolicyDialogComponent.open(this.dialogService, { + data: { + policy: new AutoConfirmPolicy(), + organizationId: organization.id, + firstTimeDialog: true, + }, + }); + } + + private checkForAutoConfirm() { + // if the policy is enabled, then the user may only belong to one organization at most. + const organization$ = this.organizations$.pipe(map((organizations) => organizations[0])); + + const featureFlag$ = this.configService.getFeatureFlag$(FeatureFlag.AutoConfirm); + + const autoConfirmState$ = this.userId$.pipe( + switchMap((userId) => this.autoConfirmService.configuration$(userId)), + ); + + const policyEnabled$ = combineLatest([ + this.userId$.pipe( + switchMap((userId) => this.policyService.policies$(userId)), + map((policies) => policies.find((p) => p.type === PolicyType.AutoConfirm && p.enabled)), + ), + organization$, + ]).pipe( + map( + ([policy, organization]) => (policy && policy.organizationId === organization?.id) ?? false, + ), + ); + + zip([organization$, featureFlag$, autoConfirmState$, policyEnabled$, this.userId$]) + .pipe( + first(), + switchMap(async ([organization, flagEnabled, autoConfirmState, policyEnabled, userId]) => { + const showDialog = + flagEnabled && + !policyEnabled && + autoConfirmState.showSetupDialog && + !!organization && + organization.canEnableAutoConfirmPolicy; + + if (showDialog) { + await this.openAutoConfirmFeatureDialog(organization); + + await this.autoConfirmService.upsert(userId, { + ...autoConfirmState, + showSetupDialog: false, + }); + } + }), + ) + .subscribe({ + error: (err: unknown) => this.logService.error("Failed to update auto-confirm state", err), + }); + } +}