diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html index 347c5fe6286..6382b5fee0e 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html @@ -108,7 +108,7 @@ - + { stop: jest.fn(), } as Partial; + const vaultItemsTransferSvc = { + transferInProgress$: new BehaviorSubject(false), + enforceOrganizationDataOwnership: jest.fn().mockResolvedValue(undefined), + } as Partial; + function getObs(cmp: any, key: string): Observable { return cmp[key] as Observable; } @@ -283,6 +292,9 @@ describe("VaultV2Component", () => { AutofillVaultListItemsComponent, VaultListItemsContainerComponent, ], + providers: [ + { provide: VaultItemsTransferService, useValue: DefaultVaultItemsTransferService }, + ], }, add: { imports: [ @@ -296,6 +308,7 @@ describe("VaultV2Component", () => { AutofillVaultListItemsStubComponent, VaultListItemsContainerStubComponent, ], + providers: [{ provide: VaultItemsTransferService, useValue: vaultItemsTransferSvc }], }, }); @@ -344,6 +357,7 @@ describe("VaultV2Component", () => { it("loading$ is true when items loading or filters missing; false when both ready", () => { const itemsLoading$ = itemsSvc.loading$ as unknown as BehaviorSubject; const allFilters$ = filtersSvc.allFilters$ as unknown as Subject; + const readySubject$ = component["readySubject"] as unknown as BehaviorSubject; const values: boolean[] = []; getObs(component, "loading$").subscribe((v) => values.push(!!v)); @@ -354,6 +368,8 @@ describe("VaultV2Component", () => { itemsLoading$.next(false); + readySubject$.next(true); + expect(values[values.length - 1]).toBe(false); }); diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts index 63d971081df..30d1d21abfb 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts @@ -16,6 +16,7 @@ import { switchMap, take, tap, + BehaviorSubject, } from "rxjs"; import { PremiumUpgradeDialogComponent } from "@bitwarden/angular/billing/components"; @@ -42,7 +43,11 @@ import { NoItemsModule, TypographyModule, } from "@bitwarden/components"; -import { DecryptionFailureDialogComponent } from "@bitwarden/vault"; +import { + DecryptionFailureDialogComponent, + VaultItemsTransferService, + DefaultVaultItemsTransferService, +} from "@bitwarden/vault"; import { CurrentAccountComponent } from "../../../../auth/popup/account-switching/current-account.component"; import { BrowserApi } from "../../../../platform/browser/browser-api"; @@ -105,6 +110,7 @@ type VaultState = UnionOfValues; VaultFadeInOutSkeletonComponent, VaultFadeInOutComponent, ], + providers: [{ provide: VaultItemsTransferService, useClass: DefaultVaultItemsTransferService }], }) export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals @@ -125,7 +131,22 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { activeUserId: UserId | null = null; - private loading$ = this.vaultPopupLoadingService.loading$.pipe( + /** + * Subject that indicates whether the vault is ready to render + * and that all initialization tasks have been completed (ngOnInit). + * @private + */ + private readySubject = new BehaviorSubject(false); + + /** + * Indicates whether the vault is loading and not yet ready to be displayed. + * @protected + */ + protected loading$ = combineLatest([ + this.vaultPopupLoadingService.loading$, + this.readySubject.asObservable(), + ]).pipe( + map(([loading, ready]) => loading || !ready), distinctUntilChanged(), tap((loading) => { const key = loading ? "loadingVault" : "vaultLoaded"; @@ -200,14 +221,15 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { protected showSkeletonsLoaders$ = combineLatest([ this.loading$, this.searchService.isCipherSearching$, + this.vaultItemsTransferService.transferInProgress$, this.skeletonFeatureFlag$, ]).pipe( - map( - ([loading, cipherSearching, skeletonsEnabled]) => - (loading || cipherSearching) && skeletonsEnabled, - ), + map(([loading, cipherSearching, transferInProgress, skeletonsEnabled]) => { + return (loading || cipherSearching || transferInProgress) && skeletonsEnabled; + }), distinctUntilChanged(), skeletonLoadingDelay(), + shareReplay({ bufferSize: 1, refCount: true }), ); protected newItemItemValues$: Observable = @@ -251,6 +273,7 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { private i18nService: I18nService, private configService: ConfigService, private searchService: SearchService, + private vaultItemsTransferService: VaultItemsTransferService, ) { combineLatest([ this.vaultPopupItemsService.emptyVault$, @@ -305,6 +328,10 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { cipherIds: ciphers.map((c) => c.id as CipherId), }); }); + + await this.vaultItemsTransferService.enforceOrganizationDataOwnership(this.activeUserId); + + this.readySubject.next(true); } ngOnDestroy() { diff --git a/libs/admin-console/src/common/collections/abstractions/collection.service.ts b/libs/admin-console/src/common/collections/abstractions/collection.service.ts index f879831324d..f0f02ee377e 100644 --- a/libs/admin-console/src/common/collections/abstractions/collection.service.ts +++ b/libs/admin-console/src/common/collections/abstractions/collection.service.ts @@ -9,6 +9,14 @@ import { CollectionData, Collection, CollectionView } from "../models"; export abstract class CollectionService { abstract encryptedCollections$(userId: UserId): Observable; abstract decryptedCollections$(userId: UserId): Observable; + + /** + * Gets the default collection for a user in a given organization, if it exists. + */ + abstract defaultUserCollection$( + userId: UserId, + orgId: OrganizationId, + ): Observable; abstract upsert(collection: CollectionData, userId: UserId): Promise; abstract replace(collections: { [id: string]: CollectionData }, userId: UserId): Promise; /** diff --git a/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts b/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts index ced3b720e3c..2eaaa48594e 100644 --- a/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts +++ b/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts @@ -15,9 +15,10 @@ import { } from "@bitwarden/common/spec"; import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { OrgKey } from "@bitwarden/common/types/key"; +import { newGuid } from "@bitwarden/guid"; import { KeyService } from "@bitwarden/key-management"; -import { CollectionData, CollectionView } from "../models"; +import { CollectionData, CollectionTypes, CollectionView } from "../models"; import { DECRYPTED_COLLECTION_DATA_KEY, ENCRYPTED_COLLECTION_DATA_KEY } from "./collection.state"; import { DefaultCollectionService } from "./default-collection.service"; @@ -389,6 +390,83 @@ describe("DefaultCollectionService", () => { }); }); + describe("defaultUserCollection$", () => { + it("returns the default collection when one exists matching the org", async () => { + const orgId = newGuid() as OrganizationId; + const defaultCollection = collectionViewDataFactory(orgId); + defaultCollection.type = CollectionTypes.DefaultUserCollection; + + const regularCollection = collectionViewDataFactory(orgId); + regularCollection.type = CollectionTypes.SharedCollection; + + await setDecryptedState([defaultCollection, regularCollection]); + + const result = await firstValueFrom(collectionService.defaultUserCollection$(userId, orgId)); + + expect(result).toBeDefined(); + expect(result?.id).toBe(defaultCollection.id); + expect(result?.isDefaultCollection).toBe(true); + }); + + it("returns undefined when no default collection exists", async () => { + const orgId = newGuid() as OrganizationId; + const collection1 = collectionViewDataFactory(orgId); + collection1.type = CollectionTypes.SharedCollection; + + const collection2 = collectionViewDataFactory(orgId); + collection2.type = CollectionTypes.SharedCollection; + + await setDecryptedState([collection1, collection2]); + + const result = await firstValueFrom(collectionService.defaultUserCollection$(userId, orgId)); + + expect(result).toBeUndefined(); + }); + + it("returns undefined when default collection exists but for different org", async () => { + const orgA = newGuid() as OrganizationId; + const orgB = newGuid() as OrganizationId; + + const defaultCollectionForOrgA = collectionViewDataFactory(orgA); + defaultCollectionForOrgA.type = CollectionTypes.DefaultUserCollection; + + await setDecryptedState([defaultCollectionForOrgA]); + + const result = await firstValueFrom(collectionService.defaultUserCollection$(userId, orgB)); + + expect(result).toBeUndefined(); + }); + + it("returns undefined when collections array is empty", async () => { + const orgId = newGuid() as OrganizationId; + + await setDecryptedState([]); + + const result = await firstValueFrom(collectionService.defaultUserCollection$(userId, orgId)); + + expect(result).toBeUndefined(); + }); + + it("returns correct collection when multiple orgs have default collections", async () => { + const orgA = newGuid() as OrganizationId; + const orgB = newGuid() as OrganizationId; + + const defaultCollectionForOrgA = collectionViewDataFactory(orgA); + defaultCollectionForOrgA.type = CollectionTypes.DefaultUserCollection; + + const defaultCollectionForOrgB = collectionViewDataFactory(orgB); + defaultCollectionForOrgB.type = CollectionTypes.DefaultUserCollection; + + await setDecryptedState([defaultCollectionForOrgA, defaultCollectionForOrgB]); + + const result = await firstValueFrom(collectionService.defaultUserCollection$(userId, orgB)); + + expect(result).toBeDefined(); + expect(result?.id).toBe(defaultCollectionForOrgB.id); + expect(result?.organizationId).toBe(orgB); + }); + }); + const setEncryptedState = (collectionData: CollectionData[] | null) => stateProvider.setUserState( ENCRYPTED_COLLECTION_DATA_KEY, diff --git a/libs/admin-console/src/common/collections/services/default-collection.service.ts b/libs/admin-console/src/common/collections/services/default-collection.service.ts index 0511b692b38..ccc2e6f0de5 100644 --- a/libs/admin-console/src/common/collections/services/default-collection.service.ts +++ b/libs/admin-console/src/common/collections/services/default-collection.service.ts @@ -87,6 +87,17 @@ export class DefaultCollectionService implements CollectionService { return result$; } + defaultUserCollection$( + userId: UserId, + orgId: OrganizationId, + ): Observable { + return this.decryptedCollections$(userId).pipe( + map((collections) => { + return collections.find((c) => c.isDefaultCollection && c.organizationId === orgId); + }), + ); + } + private initializeDecryptedState(userId: UserId): Observable { return combineLatest([ this.encryptedCollections$(userId), diff --git a/libs/vault/src/abstractions/vault-items-transfer.service.ts b/libs/vault/src/abstractions/vault-items-transfer.service.ts index ced9f71eb83..0fc19cc0e79 100644 --- a/libs/vault/src/abstractions/vault-items-transfer.service.ts +++ b/libs/vault/src/abstractions/vault-items-transfer.service.ts @@ -31,6 +31,11 @@ export type UserMigrationInfo = }; export abstract class VaultItemsTransferService { + /** + * Indicates whether a vault items transfer is currently in progress. + */ + abstract transferInProgress$: Observable; + /** * Gets information about whether the given user requires migration of their vault items * from My Vault to a My Items collection, and whether they are capable of performing that migration. diff --git a/libs/vault/src/components/vault-items-transfer/leave-confirmation-dialog.component.html b/libs/vault/src/components/vault-items-transfer/leave-confirmation-dialog.component.html index f0d644fecff..6d1045e1a86 100644 --- a/libs/vault/src/components/vault-items-transfer/leave-confirmation-dialog.component.html +++ b/libs/vault/src/components/vault-items-transfer/leave-confirmation-dialog.component.html @@ -11,7 +11,7 @@

{{ "leaveConfirmationDialogContentOne" | i18n }}

-

+

{{ "leaveConfirmationDialogContentTwo" | i18n }}

@@ -25,7 +25,7 @@ {{ "goBack" | i18n }} - + {{ "howToManageMyVault" | i18n }} diff --git a/libs/vault/src/components/vault-items-transfer/leave-confirmation-dialog.component.ts b/libs/vault/src/components/vault-items-transfer/leave-confirmation-dialog.component.ts index bd32a1ea6dd..af106376a79 100644 --- a/libs/vault/src/components/vault-items-transfer/leave-confirmation-dialog.component.ts +++ b/libs/vault/src/components/vault-items-transfer/leave-confirmation-dialog.component.ts @@ -12,6 +12,7 @@ import { DialogModule, LinkModule, TypographyModule, + CenterPositionStrategy, } from "@bitwarden/components"; export interface LeaveConfirmationDialogParams { @@ -58,6 +59,8 @@ export class LeaveConfirmationDialogComponent { static open(dialogService: DialogService, config: DialogConfig) { return dialogService.open(LeaveConfirmationDialogComponent, { + positionStrategy: new CenterPositionStrategy(), + disableClose: true, ...config, }); } diff --git a/libs/vault/src/components/vault-items-transfer/transfer-items-dialog.component.html b/libs/vault/src/components/vault-items-transfer/transfer-items-dialog.component.html index 0b77d4ba7d8..3cf626baaf7 100644 --- a/libs/vault/src/components/vault-items-transfer/transfer-items-dialog.component.html +++ b/libs/vault/src/components/vault-items-transfer/transfer-items-dialog.component.html @@ -14,7 +14,7 @@ {{ "declineAndLeave" | i18n }} - + {{ "whyAmISeeingThis" | i18n }} diff --git a/libs/vault/src/components/vault-items-transfer/transfer-items-dialog.component.ts b/libs/vault/src/components/vault-items-transfer/transfer-items-dialog.component.ts index f28ad2ab3ec..619181f37fc 100644 --- a/libs/vault/src/components/vault-items-transfer/transfer-items-dialog.component.ts +++ b/libs/vault/src/components/vault-items-transfer/transfer-items-dialog.component.ts @@ -12,6 +12,7 @@ import { DialogModule, LinkModule, TypographyModule, + CenterPositionStrategy, } from "@bitwarden/components"; export interface TransferItemsDialogParams { @@ -58,6 +59,8 @@ export class TransferItemsDialogComponent { static open(dialogService: DialogService, config: DialogConfig) { return dialogService.open(TransferItemsDialogComponent, { + positionStrategy: new CenterPositionStrategy(), + disableClose: true, ...config, }); } diff --git a/libs/vault/src/services/default-vault-items-transfer.service.spec.ts b/libs/vault/src/services/default-vault-items-transfer.service.spec.ts index d85fe2ffd43..d78cf95ebf2 100644 --- a/libs/vault/src/services/default-vault-items-transfer.service.spec.ts +++ b/libs/vault/src/services/default-vault-items-transfer.service.spec.ts @@ -1,5 +1,5 @@ import { mock, MockProxy } from "jest-mock-extended"; -import { firstValueFrom, of } from "rxjs"; +import { firstValueFrom, of, Subject } from "rxjs"; // eslint-disable-next-line no-restricted-imports import { CollectionService, CollectionView } from "@bitwarden/admin-console/common"; @@ -14,14 +14,20 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { OrganizationId, CollectionId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; -import { DialogService, ToastService } from "@bitwarden/components"; +import { DialogRef, DialogService, ToastService } from "@bitwarden/components"; import { LogService } from "@bitwarden/logging"; import { UserId } from "@bitwarden/user-core"; +import { + LeaveConfirmationDialogResult, + TransferItemsDialogResult, +} from "../components/vault-items-transfer"; + import { DefaultVaultItemsTransferService } from "./default-vault-items-transfer.service"; describe("DefaultVaultItemsTransferService", () => { let service: DefaultVaultItemsTransferService; + let transferInProgressValues: boolean[]; let mockCipherService: MockProxy; let mockPolicyService: MockProxy; @@ -37,6 +43,25 @@ describe("DefaultVaultItemsTransferService", () => { const organizationId = "org-id" as OrganizationId; const collectionId = "collection-id" as CollectionId; + /** + * Creates a mock DialogRef that emits the provided result when closed + */ + function createMockDialogRef(result: T): DialogRef { + const closed$ = new Subject(); + const dialogRef = { + closed: closed$.asObservable(), + close: jest.fn(), + } as unknown as DialogRef; + + // Emit the result asynchronously to simulate dialog closing + setTimeout(() => { + closed$.next(result); + closed$.complete(); + }, 0); + + return dialogRef; + } + beforeEach(() => { mockCipherService = mock(); mockPolicyService = mock(); @@ -49,6 +74,7 @@ describe("DefaultVaultItemsTransferService", () => { mockConfigService = mock(); mockI18nService.t.mockImplementation((key) => key); + transferInProgressValues = []; service = new DefaultVaultItemsTransferService( mockCipherService, @@ -69,12 +95,12 @@ describe("DefaultVaultItemsTransferService", () => { policies?: Policy[]; organizations?: Organization[]; ciphers?: CipherView[]; - collections?: CollectionView[]; + defaultCollection?: CollectionView; }): void { mockPolicyService.policiesByType$.mockReturnValue(of(options.policies ?? [])); mockOrganizationService.organizations$.mockReturnValue(of(options.organizations ?? [])); mockCipherService.cipherViews$.mockReturnValue(of(options.ciphers ?? [])); - mockCollectionService.decryptedCollections$.mockReturnValue(of(options.collections ?? [])); + mockCollectionService.defaultUserCollection$.mockReturnValue(of(options.defaultCollection)); } it("calls policiesByType$ with correct PolicyType", async () => { @@ -151,39 +177,12 @@ describe("DefaultVaultItemsTransferService", () => { }); it("includes defaultCollectionId when a default collection exists", async () => { - mockCollectionService.decryptedCollections$.mockReturnValue( - of([ - { - id: collectionId, - organizationId: organizationId, - isDefaultCollection: true, - } as CollectionView, - ]), - ); - - const result = await firstValueFrom(service.userMigrationInfo$(userId)); - - expect(result).toEqual({ - requiresMigration: true, - enforcingOrganization: organization, - defaultCollectionId: collectionId, - }); - }); - - it("returns default collection only for the enforcing organization", async () => { - mockCollectionService.decryptedCollections$.mockReturnValue( - of([ - { - id: "wrong-collection-id" as CollectionId, - organizationId: "wrong-org-id" as OrganizationId, - isDefaultCollection: true, - } as CollectionView, - { - id: collectionId, - organizationId: organizationId, - isDefaultCollection: true, - } as CollectionView, - ]), + mockCollectionService.defaultUserCollection$.mockReturnValue( + of({ + id: collectionId, + organizationId: organizationId, + isDefaultCollection: true, + } as CollectionView), ); const result = await firstValueFrom(service.userMigrationInfo$(userId)); @@ -542,13 +541,13 @@ describe("DefaultVaultItemsTransferService", () => { policies?: Policy[]; organizations?: Organization[]; ciphers?: CipherView[]; - collections?: CollectionView[]; + defaultCollection?: CollectionView; }): void { mockConfigService.getFeatureFlag.mockResolvedValue(options.featureEnabled ?? true); mockPolicyService.policiesByType$.mockReturnValue(of(options.policies ?? [])); mockOrganizationService.organizations$.mockReturnValue(of(options.organizations ?? [])); mockCipherService.cipherViews$.mockReturnValue(of(options.ciphers ?? [])); - mockCollectionService.decryptedCollections$.mockReturnValue(of(options.collections ?? [])); + mockCollectionService.defaultUserCollection$.mockReturnValue(of(options.defaultCollection)); } it("does nothing when feature flag is disabled", async () => { @@ -557,13 +556,11 @@ describe("DefaultVaultItemsTransferService", () => { policies: [policy], organizations: [organization], ciphers: [{ id: "cipher-1" } as CipherView], - collections: [ - { - id: collectionId, - organizationId: organizationId, - isDefaultCollection: true, - } as CollectionView, - ], + defaultCollection: { + id: collectionId, + organizationId: organizationId, + isDefaultCollection: true, + } as CollectionView, }); await service.enforceOrganizationDataOwnership(userId); @@ -571,7 +568,7 @@ describe("DefaultVaultItemsTransferService", () => { expect(mockConfigService.getFeatureFlag).toHaveBeenCalledWith( FeatureFlag.MigrateMyVaultToMyItems, ); - expect(mockDialogService.openSimpleDialog).not.toHaveBeenCalled(); + expect(mockDialogService.open).not.toHaveBeenCalled(); expect(mockCipherService.shareManyWithServer).not.toHaveBeenCalled(); }); @@ -580,7 +577,7 @@ describe("DefaultVaultItemsTransferService", () => { await service.enforceOrganizationDataOwnership(userId); - expect(mockDialogService.openSimpleDialog).not.toHaveBeenCalled(); + expect(mockDialogService.open).not.toHaveBeenCalled(); expect(mockCipherService.shareManyWithServer).not.toHaveBeenCalled(); }); @@ -593,7 +590,7 @@ describe("DefaultVaultItemsTransferService", () => { await service.enforceOrganizationDataOwnership(userId); - expect(mockDialogService.openSimpleDialog).not.toHaveBeenCalled(); + expect(mockDialogService.open).not.toHaveBeenCalled(); expect(mockCipherService.shareManyWithServer).not.toHaveBeenCalled(); }); @@ -602,7 +599,6 @@ describe("DefaultVaultItemsTransferService", () => { policies: [policy], organizations: [organization], ciphers: [{ id: "cipher-1" } as CipherView], - collections: [], }); await service.enforceOrganizationDataOwnership(userId); @@ -610,69 +606,48 @@ describe("DefaultVaultItemsTransferService", () => { expect(mockLogService.warning).toHaveBeenCalledWith( "Default collection is missing for user during organization data ownership enforcement", ); - expect(mockDialogService.openSimpleDialog).not.toHaveBeenCalled(); + expect(mockDialogService.open).not.toHaveBeenCalled(); expect(mockCipherService.shareManyWithServer).not.toHaveBeenCalled(); }); - it("shows confirmation dialog when migration is required", async () => { + it("does not transfer items when user declines and confirms leaving", async () => { setupMocksForEnforcementScenario({ policies: [policy], organizations: [organization], ciphers: [{ id: "cipher-1" } as CipherView], - collections: [ - { - id: collectionId, - organizationId: organizationId, - isDefaultCollection: true, - } as CollectionView, - ], + defaultCollection: { + id: collectionId, + organizationId: organizationId, + isDefaultCollection: true, + } as CollectionView, }); - mockDialogService.openSimpleDialog.mockResolvedValue(false); - await service.enforceOrganizationDataOwnership(userId); - - expect(mockDialogService.openSimpleDialog).toHaveBeenCalledWith({ - title: "Requires migration", - content: "Your vault requires migration of personal items to your organization.", - type: "warning", - }); - }); - - it("does not transfer items when user declines confirmation", async () => { - setupMocksForEnforcementScenario({ - policies: [policy], - organizations: [organization], - ciphers: [{ id: "cipher-1" } as CipherView], - collections: [ - { - id: collectionId, - organizationId: organizationId, - isDefaultCollection: true, - } as CollectionView, - ], - }); - mockDialogService.openSimpleDialog.mockResolvedValue(false); + // User declines transfer, then confirms leaving + mockDialogService.open + .mockReturnValueOnce(createMockDialogRef(TransferItemsDialogResult.Declined)) + .mockReturnValueOnce(createMockDialogRef(LeaveConfirmationDialogResult.Confirmed)); await service.enforceOrganizationDataOwnership(userId); expect(mockCipherService.shareManyWithServer).not.toHaveBeenCalled(); }); - it("transfers items and shows success toast when user confirms", async () => { + it("transfers items and shows success toast when user accepts transfer", async () => { const personalCiphers = [{ id: "cipher-1" } as CipherView]; setupMocksForEnforcementScenario({ policies: [policy], organizations: [organization], ciphers: personalCiphers, - collections: [ - { - id: collectionId, - organizationId: organizationId, - isDefaultCollection: true, - } as CollectionView, - ], + defaultCollection: { + id: collectionId, + organizationId: organizationId, + isDefaultCollection: true, + } as CollectionView, }); - mockDialogService.openSimpleDialog.mockResolvedValue(true); + + mockDialogService.open.mockReturnValueOnce( + createMockDialogRef(TransferItemsDialogResult.Accepted), + ); mockCipherService.shareManyWithServer.mockResolvedValue(undefined); await service.enforceOrganizationDataOwnership(userId); @@ -695,15 +670,16 @@ describe("DefaultVaultItemsTransferService", () => { policies: [policy], organizations: [organization], ciphers: personalCiphers, - collections: [ - { - id: collectionId, - organizationId: organizationId, - isDefaultCollection: true, - } as CollectionView, - ], + defaultCollection: { + id: collectionId, + organizationId: organizationId, + isDefaultCollection: true, + } as CollectionView, }); - mockDialogService.openSimpleDialog.mockResolvedValue(true); + + mockDialogService.open.mockReturnValueOnce( + createMockDialogRef(TransferItemsDialogResult.Accepted), + ); mockCipherService.shareManyWithServer.mockRejectedValue(new Error("Transfer failed")); await service.enforceOrganizationDataOwnership(userId); @@ -717,5 +693,171 @@ describe("DefaultVaultItemsTransferService", () => { message: "errorOccurred", }); }); + + it("re-shows transfer dialog when user goes back from leave confirmation", async () => { + const personalCiphers = [{ id: "cipher-1" } as CipherView]; + setupMocksForEnforcementScenario({ + policies: [policy], + organizations: [organization], + ciphers: personalCiphers, + defaultCollection: { + id: collectionId, + organizationId: organizationId, + isDefaultCollection: true, + } as CollectionView, + }); + + // User declines, goes back, then accepts + mockDialogService.open + .mockReturnValueOnce(createMockDialogRef(TransferItemsDialogResult.Declined)) + .mockReturnValueOnce(createMockDialogRef(LeaveConfirmationDialogResult.Back)) + .mockReturnValueOnce(createMockDialogRef(TransferItemsDialogResult.Accepted)); + mockCipherService.shareManyWithServer.mockResolvedValue(undefined); + + await service.enforceOrganizationDataOwnership(userId); + + // Dialog should have been opened 3 times: transfer -> leave -> transfer (after going back) + expect(mockDialogService.open).toHaveBeenCalledTimes(3); + expect(mockCipherService.shareManyWithServer).toHaveBeenCalled(); + }); + + it("allows multiple back navigations before accepting transfer", async () => { + const personalCiphers = [{ id: "cipher-1" } as CipherView]; + setupMocksForEnforcementScenario({ + policies: [policy], + organizations: [organization], + ciphers: personalCiphers, + defaultCollection: { + id: collectionId, + organizationId: organizationId, + isDefaultCollection: true, + } as CollectionView, + }); + + // User declines, goes back, declines again, goes back again, then accepts + mockDialogService.open + .mockReturnValueOnce(createMockDialogRef(TransferItemsDialogResult.Declined)) + .mockReturnValueOnce(createMockDialogRef(LeaveConfirmationDialogResult.Back)) + .mockReturnValueOnce(createMockDialogRef(TransferItemsDialogResult.Declined)) + .mockReturnValueOnce(createMockDialogRef(LeaveConfirmationDialogResult.Back)) + .mockReturnValueOnce(createMockDialogRef(TransferItemsDialogResult.Accepted)); + mockCipherService.shareManyWithServer.mockResolvedValue(undefined); + + await service.enforceOrganizationDataOwnership(userId); + + // Dialog should have been opened 5 times + expect(mockDialogService.open).toHaveBeenCalledTimes(5); + expect(mockCipherService.shareManyWithServer).toHaveBeenCalled(); + }); + + it("allows user to go back and then confirm leaving", async () => { + setupMocksForEnforcementScenario({ + policies: [policy], + organizations: [organization], + ciphers: [{ id: "cipher-1" } as CipherView], + defaultCollection: { + id: collectionId, + organizationId: organizationId, + isDefaultCollection: true, + } as CollectionView, + }); + + // User declines, goes back, declines again, then confirms leaving + mockDialogService.open + .mockReturnValueOnce(createMockDialogRef(TransferItemsDialogResult.Declined)) + .mockReturnValueOnce(createMockDialogRef(LeaveConfirmationDialogResult.Back)) + .mockReturnValueOnce(createMockDialogRef(TransferItemsDialogResult.Declined)) + .mockReturnValueOnce(createMockDialogRef(LeaveConfirmationDialogResult.Confirmed)); + + await service.enforceOrganizationDataOwnership(userId); + + expect(mockDialogService.open).toHaveBeenCalledTimes(4); + expect(mockCipherService.shareManyWithServer).not.toHaveBeenCalled(); + }); + }); + + describe("transferInProgress$", () => { + const policy = { + organizationId: organizationId, + revisionDate: new Date("2024-01-01"), + } as Policy; + const organization = { + id: organizationId, + name: "Test Org", + } as Organization; + + function setupMocksForTransferScenario(options: { + featureEnabled?: boolean; + policies?: Policy[]; + organizations?: Organization[]; + ciphers?: CipherView[]; + defaultCollection?: CollectionView; + }): void { + mockConfigService.getFeatureFlag.mockResolvedValue(options.featureEnabled ?? true); + mockPolicyService.policiesByType$.mockReturnValue(of(options.policies ?? [])); + mockOrganizationService.organizations$.mockReturnValue(of(options.organizations ?? [])); + mockCipherService.cipherViews$.mockReturnValue(of(options.ciphers ?? [])); + mockCollectionService.defaultUserCollection$.mockReturnValue(of(options.defaultCollection)); + } + + it("emits false initially", async () => { + const result = await firstValueFrom(service.transferInProgress$); + + expect(result).toBe(false); + }); + + it("emits true during transfer and false after successful completion", async () => { + const personalCiphers = [{ id: "cipher-1" } as CipherView]; + setupMocksForTransferScenario({ + policies: [policy], + organizations: [organization], + ciphers: personalCiphers, + defaultCollection: { + id: collectionId, + organizationId: organizationId, + isDefaultCollection: true, + } as CollectionView, + }); + + mockDialogService.open.mockReturnValueOnce( + createMockDialogRef(TransferItemsDialogResult.Accepted), + ); + mockCipherService.shareManyWithServer.mockResolvedValue(undefined); + + // Subscribe to track all emitted values + service.transferInProgress$.subscribe((value) => transferInProgressValues.push(value)); + + await service.enforceOrganizationDataOwnership(userId); + + // Should have emitted: false (initial), true (transfer started), false (transfer completed) + expect(transferInProgressValues).toEqual([false, true, false]); + }); + + it("emits false after transfer fails with error", async () => { + const personalCiphers = [{ id: "cipher-1" } as CipherView]; + setupMocksForTransferScenario({ + policies: [policy], + organizations: [organization], + ciphers: personalCiphers, + defaultCollection: { + id: collectionId, + organizationId: organizationId, + isDefaultCollection: true, + } as CollectionView, + }); + + mockDialogService.open.mockReturnValueOnce( + createMockDialogRef(TransferItemsDialogResult.Accepted), + ); + mockCipherService.shareManyWithServer.mockRejectedValue(new Error("Transfer failed")); + + // Subscribe to track all emitted values + service.transferInProgress$.subscribe((value) => transferInProgressValues.push(value)); + + await service.enforceOrganizationDataOwnership(userId); + + // Should have emitted: false (initial), true (transfer started), false (transfer failed) + expect(transferInProgressValues).toEqual([false, true, false]); + }); }); }); diff --git a/libs/vault/src/services/default-vault-items-transfer.service.ts b/libs/vault/src/services/default-vault-items-transfer.service.ts index d9c490f870e..d7088873071 100644 --- a/libs/vault/src/services/default-vault-items-transfer.service.ts +++ b/libs/vault/src/services/default-vault-items-transfer.service.ts @@ -1,5 +1,13 @@ import { Injectable } from "@angular/core"; -import { firstValueFrom, switchMap, map, of, Observable, combineLatest } from "rxjs"; +import { + firstValueFrom, + switchMap, + map, + of, + Observable, + combineLatest, + BehaviorSubject, +} from "rxjs"; // eslint-disable-next-line no-restricted-imports import { CollectionService } from "@bitwarden/admin-console/common"; @@ -23,6 +31,12 @@ import { VaultItemsTransferService, UserMigrationInfo, } from "../abstractions/vault-items-transfer.service"; +import { + TransferItemsDialogComponent, + TransferItemsDialogResult, + LeaveConfirmationDialogComponent, + LeaveConfirmationDialogResult, +} from "../components/vault-items-transfer"; @Injectable() export class DefaultVaultItemsTransferService implements VaultItemsTransferService { @@ -38,6 +52,10 @@ export class DefaultVaultItemsTransferService implements VaultItemsTransferServi private configService: ConfigService, ) {} + private _transferInProgressSubject = new BehaviorSubject(false); + + transferInProgress$ = this._transferInProgressSubject.asObservable(); + private enforcingOrganization$(userId: UserId): Observable { return this.policyService.policiesByType$(PolicyType.OrganizationDataOwnership, userId).pipe( map( @@ -60,18 +78,6 @@ export class DefaultVaultItemsTransferService implements VaultItemsTransferServi ); } - private defaultUserCollection$( - userId: UserId, - organizationId: OrganizationId, - ): Observable { - return this.collectionService.decryptedCollections$(userId).pipe( - map((collections) => { - return collections.find((c) => c.isDefaultCollection && c.organizationId === organizationId) - ?.id; - }), - ); - } - userMigrationInfo$(userId: UserId): Observable { return this.enforcingOrganization$(userId).pipe( switchMap((enforcingOrganization) => { @@ -82,13 +88,13 @@ export class DefaultVaultItemsTransferService implements VaultItemsTransferServi } return combineLatest([ this.personalCiphers$(userId), - this.defaultUserCollection$(userId, enforcingOrganization.id), + this.collectionService.defaultUserCollection$(userId, enforcingOrganization.id), ]).pipe( - map(([personalCiphers, defaultCollectionId]): UserMigrationInfo => { + map(([personalCiphers, defaultCollection]): UserMigrationInfo => { return { requiresMigration: personalCiphers.length > 0, enforcingOrganization, - defaultCollectionId, + defaultCollectionId: defaultCollection?.id, }; }), ); @@ -96,6 +102,35 @@ export class DefaultVaultItemsTransferService implements VaultItemsTransferServi ); } + /** + * Prompts the user to accept or decline the vault items transfer. + * If declined, shows a leave confirmation dialog with option to go back. + * @returns true if user accepts transfer, false if user confirms leaving + */ + private async promptUserForTransfer(organizationName: string): Promise { + const confirmDialogRef = TransferItemsDialogComponent.open(this.dialogService, { + data: { organizationName }, + }); + + const confirmResult = await firstValueFrom(confirmDialogRef.closed); + + if (confirmResult === TransferItemsDialogResult.Accepted) { + return true; + } + + const leaveDialogRef = LeaveConfirmationDialogComponent.open(this.dialogService, { + data: { organizationName }, + }); + + const leaveResult = await firstValueFrom(leaveDialogRef.closed); + + if (leaveResult === LeaveConfirmationDialogResult.Back) { + return this.promptUserForTransfer(organizationName); + } + + return false; + } + async enforceOrganizationDataOwnership(userId: UserId): Promise { const featureEnabled = await this.configService.getFeatureFlag( FeatureFlag.MigrateMyVaultToMyItems, @@ -119,30 +154,29 @@ export class DefaultVaultItemsTransferService implements VaultItemsTransferServi return; } - // Temporary confirmation dialog. Full implementation in PM-27663 - const confirmMigration = await this.dialogService.openSimpleDialog({ - title: "Requires migration", - content: "Your vault requires migration of personal items to your organization.", - type: "warning", - }); + const userAcceptedTransfer = await this.promptUserForTransfer( + migrationInfo.enforcingOrganization.name, + ); - if (!confirmMigration) { - // TODO: Show secondary confirmation dialog in PM-27663, for now we just exit - // TODO: Revoke user from organization if they decline migration PM-29465 + if (!userAcceptedTransfer) { + // TODO: Revoke user from organization if they decline migration and show toast PM-29465 return; } try { + this._transferInProgressSubject.next(true); await this.transferPersonalItems( userId, migrationInfo.enforcingOrganization.id, migrationInfo.defaultCollectionId, ); + this._transferInProgressSubject.next(false); this.toastService.showToast({ variant: "success", message: this.i18nService.t("itemsTransferred"), }); } catch (error) { + this._transferInProgressSubject.next(false); this.logService.error("Error transferring personal items to organization", error); this.toastService.showToast({ variant: "error",