mirror of
https://github.com/bitwarden/browser
synced 2026-02-12 06:23:38 +00:00
[PM-31839] Only allow a single item transfer (#18889)
* add property to track inflight enforcements of item transfers * update naming in tests
This commit is contained in:
@@ -938,4 +938,142 @@ describe("DefaultVaultItemsTransferService", () => {
|
||||
expect(transferInProgressValues).toEqual([false, true, false]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("enforcementInFlight", () => {
|
||||
const policy = {
|
||||
organizationId: organizationId,
|
||||
revisionDate: new Date("2024-01-01"),
|
||||
} as Policy;
|
||||
const organization = {
|
||||
id: organizationId,
|
||||
name: "Test Org",
|
||||
} as Organization;
|
||||
const personalCiphers = [{ id: "cipher-1" } as CipherView];
|
||||
const defaultCollection = {
|
||||
id: collectionId,
|
||||
organizationId: organizationId,
|
||||
isDefaultCollection: true,
|
||||
} as CollectionView;
|
||||
|
||||
beforeEach(() => {
|
||||
mockConfigService.getFeatureFlag.mockResolvedValue(true);
|
||||
mockPolicyService.policiesByType$.mockReturnValue(of([policy]));
|
||||
mockOrganizationService.organizations$.mockReturnValue(of([organization]));
|
||||
mockCipherService.cipherViews$.mockReturnValue(of(personalCiphers));
|
||||
mockCollectionService.defaultUserCollection$.mockReturnValue(of(defaultCollection));
|
||||
mockSyncService.fullSync.mockResolvedValue(true);
|
||||
mockOrganizationUserApiService.revokeSelf.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it("prevents re-entry when enforcement is already in flight", async () => {
|
||||
// Create a dialog that resolves after a delay
|
||||
const delayedSubject = new Subject<any>();
|
||||
const delayedDialog = {
|
||||
closed: delayedSubject.asObservable(),
|
||||
close: jest.fn(),
|
||||
} as unknown as DialogRef<any>;
|
||||
|
||||
mockDialogService.open.mockReturnValue(delayedDialog);
|
||||
|
||||
// Start first call (won't complete immediately)
|
||||
const firstCall = service.enforceOrganizationDataOwnership(userId);
|
||||
|
||||
// Flush microtasks to allow first call to set enforcementInFlight
|
||||
await Promise.resolve();
|
||||
|
||||
// Second call should return immediately without opening dialog
|
||||
await service.enforceOrganizationDataOwnership(userId);
|
||||
|
||||
// Verify re-entry was prevented - only the first call should proceed
|
||||
expect(mockDialogService.open).toHaveBeenCalledTimes(1);
|
||||
expect(mockPolicyService.policiesByType$).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Clean up - resolve the first call's dialog
|
||||
delayedSubject.next(TransferItemsDialogResult.Declined);
|
||||
delayedSubject.complete();
|
||||
|
||||
// Mock the leave dialog
|
||||
mockDialogService.open.mockReturnValueOnce(
|
||||
createMockDialogRef(LeaveConfirmationDialogResult.Confirmed),
|
||||
);
|
||||
|
||||
await firstCall;
|
||||
});
|
||||
|
||||
it("allows subsequent calls after user declines and leaves", async () => {
|
||||
// First call: user declines and confirms leaving
|
||||
mockDialogService.open
|
||||
.mockReturnValueOnce(createMockDialogRef(TransferItemsDialogResult.Declined))
|
||||
.mockReturnValueOnce(createMockDialogRef(LeaveConfirmationDialogResult.Confirmed));
|
||||
|
||||
await service.enforceOrganizationDataOwnership(userId);
|
||||
|
||||
// Reset mocks for second call
|
||||
mockDialogService.open.mockClear();
|
||||
|
||||
// Second call: user accepts transfer
|
||||
mockDialogService.open.mockReturnValueOnce(
|
||||
createMockDialogRef(TransferItemsDialogResult.Accepted),
|
||||
);
|
||||
mockCipherService.shareManyWithServer.mockResolvedValue(undefined);
|
||||
|
||||
await service.enforceOrganizationDataOwnership(userId);
|
||||
|
||||
// Second call should proceed (dialog opened again)
|
||||
expect(mockDialogService.open).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("allows subsequent calls after successful transfer", async () => {
|
||||
// First call: user accepts transfer
|
||||
mockDialogService.open.mockReturnValueOnce(
|
||||
createMockDialogRef(TransferItemsDialogResult.Accepted),
|
||||
);
|
||||
mockCipherService.shareManyWithServer.mockResolvedValue(undefined);
|
||||
|
||||
await service.enforceOrganizationDataOwnership(userId);
|
||||
|
||||
// Reset mocks for second call
|
||||
mockDialogService.open.mockClear();
|
||||
mockCipherService.shareManyWithServer.mockClear();
|
||||
|
||||
// Second call should be allowed (though no migration needed after first transfer)
|
||||
// Set up scenario where migration is needed again
|
||||
mockCipherService.cipherViews$.mockReturnValue(of([{ id: "cipher-2" } as CipherView]));
|
||||
|
||||
mockDialogService.open.mockReturnValueOnce(
|
||||
createMockDialogRef(TransferItemsDialogResult.Accepted),
|
||||
);
|
||||
mockCipherService.shareManyWithServer.mockResolvedValue(undefined);
|
||||
|
||||
await service.enforceOrganizationDataOwnership(userId);
|
||||
|
||||
// Second call should proceed (dialog opened again)
|
||||
expect(mockDialogService.open).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("allows subsequent calls after transfer fails with error", async () => {
|
||||
// First call: transfer fails
|
||||
mockDialogService.open.mockReturnValueOnce(
|
||||
createMockDialogRef(TransferItemsDialogResult.Accepted),
|
||||
);
|
||||
mockCipherService.shareManyWithServer.mockRejectedValue(new Error("Transfer failed"));
|
||||
|
||||
await service.enforceOrganizationDataOwnership(userId);
|
||||
|
||||
// Reset mocks for second call
|
||||
mockDialogService.open.mockClear();
|
||||
mockCipherService.shareManyWithServer.mockClear();
|
||||
|
||||
// Second call: user accepts transfer successfully
|
||||
mockDialogService.open.mockReturnValueOnce(
|
||||
createMockDialogRef(TransferItemsDialogResult.Accepted),
|
||||
);
|
||||
mockCipherService.shareManyWithServer.mockResolvedValue(undefined);
|
||||
|
||||
await service.enforceOrganizationDataOwnership(userId);
|
||||
|
||||
// Second call should proceed (dialog opened again)
|
||||
expect(mockDialogService.open).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -62,6 +62,12 @@ export class DefaultVaultItemsTransferService implements VaultItemsTransferServi
|
||||
|
||||
transferInProgress$ = this._transferInProgressSubject.asObservable();
|
||||
|
||||
/**
|
||||
* Only a single enforcement should be allowed to run at a time to prevent multiple dialogs
|
||||
* or multiple simultaneous transfers.
|
||||
*/
|
||||
private enforcementInFlight: boolean = false;
|
||||
|
||||
private enforcingOrganization$(userId: UserId): Observable<Organization | undefined> {
|
||||
return this.policyService.policiesByType$(PolicyType.OrganizationDataOwnership, userId).pipe(
|
||||
map(
|
||||
@@ -142,7 +148,7 @@ export class DefaultVaultItemsTransferService implements VaultItemsTransferServi
|
||||
FeatureFlag.MigrateMyVaultToMyItems,
|
||||
);
|
||||
|
||||
if (!featureEnabled) {
|
||||
if (!featureEnabled || this.enforcementInFlight) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -160,6 +166,8 @@ export class DefaultVaultItemsTransferService implements VaultItemsTransferServi
|
||||
return;
|
||||
}
|
||||
|
||||
this.enforcementInFlight = true;
|
||||
|
||||
const userAcceptedTransfer = await this.promptUserForTransfer(
|
||||
migrationInfo.enforcingOrganization.name,
|
||||
);
|
||||
@@ -179,6 +187,7 @@ export class DefaultVaultItemsTransferService implements VaultItemsTransferServi
|
||||
);
|
||||
// Sync to reflect organization removal
|
||||
await this.syncService.fullSync(true);
|
||||
this.enforcementInFlight = false;
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -208,6 +217,8 @@ export class DefaultVaultItemsTransferService implements VaultItemsTransferServi
|
||||
variant: "error",
|
||||
message: this.i18nService.t("errorOccurred"),
|
||||
});
|
||||
} finally {
|
||||
this.enforcementInFlight = false;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user