From 1c44e2a4c63f54081385b22eb848f3a430998a8c Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Tue, 16 Dec 2025 16:07:16 -0500 Subject: [PATCH 1/4] chore(deps): Improve Platform dependency inflow (#17981) * Send Platform minor updates to dashboard. * Assign Platform as lock file owners. --- .github/renovate.json5 | 277 ++++++++++++++++++++++++++++------------- 1 file changed, 189 insertions(+), 88 deletions(-) diff --git a/.github/renovate.json5 b/.github/renovate.json5 index ca57ccf4f86..acd181310d6 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -3,6 +3,7 @@ extends: ["github>bitwarden/renovate-config"], // Extends our default configuration for pinned dependencies enabledManagers: ["cargo", "github-actions", "npm"], packageRules: [ + // ==================== Repo-Wide Update Behavior Rules ==================== { // Group all Github Action minor updates together to reduce PR noise. groupName: "Minor github-actions updates", @@ -16,13 +17,6 @@ matchDepNames: ["rust"], commitMessageTopic: "Rust", }, - { - // By default, we send patch updates to the Dependency Dashboard and do not generate a PR. - // We want to generate PRs for a select number of dependencies to ensure we stay up to date on these. - matchPackageNames: ["browserslist", "electron", "rxjs", "typescript", "webpack", "zone.js"], - matchUpdateTypes: ["patch"], - dependencyDashboardApproval: false, - }, { // Disable major and minor updates for TypeScript and Zone.js because they are managed by Angular. matchPackageNames: ["typescript", "zone.js"], @@ -44,6 +38,8 @@ description: "Manually updated using ng update", enabled: false, }, + + // ==================== Team Ownership Rules ==================== { matchPackageNames: ["buffer", "bufferutil", "core-js", "process", "url", "util"], description: "Admin Console owned dependencies", @@ -79,28 +75,6 @@ commitMessagePrefix: "[deps] Architecture:", reviewers: ["team:dept-architecture"], }, - { - matchPackageNames: [ - "@angular-eslint/schematics", - "@eslint/compat", - "@typescript-eslint/rule-tester", - "@typescript-eslint/utils", - "angular-eslint", - "eslint-config-prettier", - "eslint-import-resolver-typescript", - "eslint-plugin-import", - "eslint-plugin-rxjs-angular", - "eslint-plugin-rxjs", - "eslint-plugin-storybook", - "eslint-plugin-tailwindcss", - "eslint", - "husky", - "lint-staged", - "typescript-eslint", - ], - groupName: "Minor and patch linting updates", - matchUpdateTypes: ["minor", "patch"], - }, { matchPackageNames: [ "@emotion/css", @@ -241,60 +215,10 @@ reviewers: ["team:team-platform-dev"], }, { - // We need to group all napi-related packages together to avoid build errors caused by version incompatibilities. - groupName: "napi", - matchPackageNames: ["napi", "napi-build", "napi-derive"], - }, - { - // We need to group all macOS/iOS binding-related packages together to avoid build errors caused by version incompatibilities. - groupName: "macOS/iOS bindings", - matchPackageNames: ["core-foundation", "security-framework", "security-framework-sys"], - }, - { - // We need to group all zbus-related packages together to avoid build errors caused by version incompatibilities. - groupName: "zbus", - matchPackageNames: ["zbus", "zbus_polkit"], - }, - { - // We need to group all windows-related packages together to avoid build errors caused by version incompatibilities. - groupName: "windows", - matchPackageNames: ["windows", "windows-core", "windows-future", "windows-registry"], - }, - { - // We need to group all tokio-related packages together to avoid build errors caused by version incompatibilities. - groupName: "tokio", - matchPackageNames: ["bytes", "tokio", "tokio-util"], - }, - { - // We group all webpack build-related minor and patch updates together to reduce PR noise. - // We include patch updates here because we want PRs for webpack patch updates and it's in this group. - matchPackageNames: [ - "@babel/core", - "@babel/preset-env", - "babel-loader", - "base64-loader", - "browserslist", - "copy-webpack-plugin", - "css-loader", - "html-loader", - "html-webpack-injector", - "html-webpack-plugin", - "mini-css-extract-plugin", - "postcss-loader", - "postcss", - "sass-loader", - "sass", - "style-loader", - "ts-loader", - "tsconfig-paths-webpack-plugin", - "webpack-cli", - "webpack-dev-server", - "webpack-node-externals", - "webpack", - ], - description: "webpack-related build dependencies", - groupName: "Minor and patch webpack updates", - matchUpdateTypes: ["minor", "patch"], + matchUpdateTypes: ["lockFileMaintenance"], + description: "Platform owns lock file maintenance", + commitMessagePrefix: "[deps] Platform:", + reviewers: ["team:team-platform-dev"], }, { matchPackageNames: [ @@ -353,11 +277,6 @@ commitMessagePrefix: "[deps] SM:", reviewers: ["team:team-secrets-manager-dev"], }, - { - // We need to update several Jest-related packages together, for version compatibility. - groupName: "jest", - matchPackageNames: ["@types/jest", "jest", "ts-jest", "jest-preset-angular"], - }, { matchPackageNames: [ "@microsoft/signalr-protocol-msgpack", @@ -428,6 +347,188 @@ commitMessagePrefix: "[deps] KM:", reviewers: ["team:team-key-management-dev"], }, + + // ==================== Grouping Rules ==================== + // These come after any specific team assignment rules to ensure + // that grouping is not overridden by subsequent rule definitions. + { + matchPackageNames: [ + "@angular-eslint/schematics", + "@eslint/compat", + "@typescript-eslint/rule-tester", + "@typescript-eslint/utils", + "angular-eslint", + "eslint-config-prettier", + "eslint-import-resolver-typescript", + "eslint-plugin-import", + "eslint-plugin-rxjs-angular", + "eslint-plugin-rxjs", + "eslint-plugin-storybook", + "eslint-plugin-tailwindcss", + "eslint", + "husky", + "lint-staged", + "typescript-eslint", + ], + groupName: "Minor and patch linting updates", + matchUpdateTypes: ["minor", "patch"], + }, + { + // We need to group all napi-related packages together to avoid build errors caused by version incompatibilities. + groupName: "napi", + matchPackageNames: ["napi", "napi-build", "napi-derive"], + }, + { + // We need to group all macOS/iOS binding-related packages together to avoid build errors caused by version incompatibilities. + groupName: "macOS/iOS bindings", + matchPackageNames: ["core-foundation", "security-framework", "security-framework-sys"], + }, + { + // We need to group all zbus-related packages together to avoid build errors caused by version incompatibilities. + groupName: "zbus", + matchPackageNames: ["zbus", "zbus_polkit"], + }, + { + // We need to group all windows-related packages together to avoid build errors caused by version incompatibilities. + groupName: "windows", + matchPackageNames: ["windows", "windows-core", "windows-future", "windows-registry"], + }, + { + // We need to group all tokio-related packages together to avoid build errors caused by version incompatibilities. + groupName: "tokio", + matchPackageNames: ["bytes", "tokio", "tokio-util"], + }, + { + // We group all webpack build-related minor and patch updates together to reduce PR noise. + // We include patch updates here because we want PRs for webpack patch updates and it's in this group. + matchPackageNames: [ + "@babel/core", + "@babel/preset-env", + "babel-loader", + "base64-loader", + "browserslist", + "copy-webpack-plugin", + "css-loader", + "html-loader", + "html-webpack-injector", + "html-webpack-plugin", + "mini-css-extract-plugin", + "postcss-loader", + "postcss", + "sass-loader", + "sass", + "style-loader", + "ts-loader", + "tsconfig-paths-webpack-plugin", + "webpack-cli", + "webpack-dev-server", + "webpack-node-externals", + "webpack", + ], + description: "webpack-related build dependencies", + groupName: "Minor and patch webpack updates", + matchUpdateTypes: ["minor", "patch"], + }, + { + // We need to update several Jest-related packages together, for version compatibility. + groupName: "jest", + matchPackageNames: ["@types/jest", "jest", "ts-jest", "jest-preset-angular"], + }, + + // ==================== Dashboard Rules ==================== + { + // For the packages below, we have decided we will only be creating PRs + // for major updates, and sending minor (as well as patch) to the dashboard. + // This rule comes AFTER grouping rules so that groups are respected while still + // sending minor/patch updates to the dependency dashboard for approval. + matchPackageNames: [ + "anyhow", + "arboard", + "babel-loader", + "base64-loader", + "base64", + "bindgen", + "byteorder", + "bytes", + "core-foundation", + "copy-webpack-plugin", + "css-loader", + "dirs", + "electron-builder", + "electron-log", + "electron-reload", + "electron-store", + "electron-updater", + "embed_plist", + "futures", + "hex", + "homedir", + "html-loader", + "html-webpack-injector", + "html-webpack-plugin", + "interprocess", + "json5", + "keytar", + "libc", + "lowdb", + "mini-css-extract-plugin", + "napi", + "napi-build", + "napi-derive", + "node-ipc", + "nx", + "oo7", + "oslog", + "pin-project", + "pkg", + "postcss", + "postcss-loader", + "rand", + "sass", + "sass-loader", + "scopeguard", + "security-framework", + "security-framework-sys", + "semver", + "serde", + "serde_json", + "simplelog", + "style-loader", + "sysinfo", + "tokio", + "tokio-util", + "tracing", + "tracing-subscriber", + "ts-node", + "ts-loader", + "tsconfig-paths-webpack-plugin", + "type-fest", + "typenum", + "typescript-strict-plugin", + "uniffi", + "webpack-cli", + "webpack-dev-server", + "webpack-node-externals", + "widestring", + "windows", + "windows-core", + "windows-future", + "windows-registry", + "zbus", + "zbus_polkit", + ], + matchUpdateTypes: ["minor", "patch"], + dependencyDashboardApproval: true, + }, + { + // By default, we send patch updates to the Dependency Dashboard and do not generate a PR. + // We want to generate PRs for a select number of dependencies to ensure we stay up to date on these. + matchPackageNames: ["browserslist", "electron", "rxjs", "typescript", "webpack", "zone.js"], + matchUpdateTypes: ["patch"], + dependencyDashboardApproval: false, + }, + + // ==================== Special Version Constraints ==================== { // Any versions of lowdb above 1.0.0 are not compatible with CommonJS. matchPackageNames: ["lowdb"], From 3049cfad7dfa1f6c7975ff47d7f4d08822eadf18 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Tue, 16 Dec 2025 15:47:58 -0600 Subject: [PATCH 2/4] [PM-29103] Premium Prompt for Archive (#17800) * prompt for premium using badge workflow rather than premium page * update test * address claude feedback --- .../settings/vault-settings-v2.component.html | 9 +- .../vault-settings-v2.component.spec.ts | 199 ++++++++++++++++++ .../settings/vault-settings-v2.component.ts | 18 +- 3 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 apps/browser/src/vault/popup/settings/vault-settings-v2.component.spec.ts diff --git a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.html b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.html index c042af8cbac..407015d3a06 100644 --- a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.html +++ b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.html @@ -37,14 +37,19 @@ @if (showArchiveItem()) { @if (userCanArchive()) { - + {{ "archiveNoun" | i18n }} } @else { - + {{ "archiveNoun" | i18n }} @if (!userHasArchivedItems()) { diff --git a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.spec.ts b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.spec.ts new file mode 100644 index 00000000000..fc30a3f8899 --- /dev/null +++ b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.spec.ts @@ -0,0 +1,199 @@ +import { ChangeDetectionStrategy, Component, DebugElement, input } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { By } from "@angular/platform-browser"; +import { provideRouter, Router } from "@angular/router"; +import { mock } from "jest-mock-extended"; +import { BehaviorSubject } from "rxjs"; + +import { NudgesService } from "@bitwarden/angular/vault"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { DialogService, ToastService } from "@bitwarden/components"; + +import { PopOutComponent } from "../../../platform/popup/components/pop-out.component"; +import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component"; +import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component"; + +import { VaultSettingsV2Component } from "./vault-settings-v2.component"; + +@Component({ + selector: "popup-header", + template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class MockPopupHeaderComponent { + readonly pageTitle = input(); + readonly showBackButton = input(); +} + +@Component({ + selector: "popup-page", + template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class MockPopupPageComponent {} + +@Component({ + selector: "app-pop-out", + template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class MockPopOutComponent { + readonly show = input(true); +} + +describe("VaultSettingsV2Component", () => { + let component: VaultSettingsV2Component; + let fixture: ComponentFixture; + let router: Router; + let mockCipherArchiveService: jest.Mocked; + + const mockActiveAccount$ = new BehaviorSubject<{ id: string }>({ + id: "user-id", + }); + const mockUserCanArchive$ = new BehaviorSubject(false); + const mockHasArchiveFlagEnabled$ = new BehaviorSubject(true); + const mockArchivedCiphers$ = new BehaviorSubject([]); + const mockShowNudgeBadge$ = new BehaviorSubject(false); + + const queryByTestId = (testId: string): DebugElement | null => { + return fixture.debugElement.query(By.css(`[data-test-id="${testId}"]`)); + }; + + const setArchiveState = ( + canArchive: boolean, + archivedItems: CipherView[] = [], + flagEnabled = true, + ) => { + mockUserCanArchive$.next(canArchive); + mockArchivedCiphers$.next(archivedItems); + mockHasArchiveFlagEnabled$.next(flagEnabled); + fixture.detectChanges(); + }; + + beforeEach(async () => { + mockCipherArchiveService = mock({ + userCanArchive$: jest.fn().mockReturnValue(mockUserCanArchive$), + hasArchiveFlagEnabled$: jest.fn().mockReturnValue(mockHasArchiveFlagEnabled$), + archivedCiphers$: jest.fn().mockReturnValue(mockArchivedCiphers$), + }); + + await TestBed.configureTestingModule({ + imports: [VaultSettingsV2Component], + providers: [ + provideRouter([ + { path: "archive", component: VaultSettingsV2Component }, + { path: "premium", component: VaultSettingsV2Component }, + ]), + { provide: SyncService, useValue: mock() }, + { provide: ToastService, useValue: mock() }, + { provide: ConfigService, useValue: mock() }, + { provide: DialogService, useValue: mock() }, + { provide: I18nService, useValue: { t: (key: string) => key } }, + { provide: CipherArchiveService, useValue: mockCipherArchiveService }, + { + provide: NudgesService, + useValue: { showNudgeBadge$: jest.fn().mockReturnValue(mockShowNudgeBadge$) }, + }, + + { + provide: BillingAccountProfileStateService, + useValue: mock(), + }, + { + provide: AccountService, + useValue: { activeAccount$: mockActiveAccount$ }, + }, + ], + }) + .overrideComponent(VaultSettingsV2Component, { + remove: { + imports: [PopupHeaderComponent, PopupPageComponent, PopOutComponent], + }, + add: { + imports: [MockPopupHeaderComponent, MockPopupPageComponent, MockPopOutComponent], + }, + }) + .compileComponents(); + + fixture = TestBed.createComponent(VaultSettingsV2Component); + component = fixture.componentInstance; + router = TestBed.inject(Router); + jest.spyOn(router, "navigate"); + }); + + describe("archive link", () => { + it("shows direct archive link when user can archive", () => { + setArchiveState(true); + + const archiveLink = queryByTestId("archive-link"); + + expect(archiveLink.nativeElement.getAttribute("routerLink")).toBe("/archive"); + }); + + it("routes to archive when user has archived items but cannot archive", async () => { + setArchiveState(false, [{ id: "cipher1" } as CipherView]); + + const premiumArchiveLink = queryByTestId("premium-archive-link"); + + premiumArchiveLink.nativeElement.click(); + await fixture.whenStable(); + + expect(router.navigate).toHaveBeenCalledWith(["/archive"]); + }); + + it("prompts for premium when user cannot archive and has no archived items", async () => { + setArchiveState(false, []); + const badge = component["premiumBadgeComponent"](); + jest.spyOn(badge, "promptForPremium"); + + const premiumArchiveLink = queryByTestId("premium-archive-link"); + + premiumArchiveLink.nativeElement.click(); + await fixture.whenStable(); + + expect(badge.promptForPremium).toHaveBeenCalled(); + }); + }); + + describe("archive visibility", () => { + it("displays archive link when user can archive", () => { + setArchiveState(true); + + const archiveLink = queryByTestId("archive-link"); + + expect(archiveLink).toBeTruthy(); + expect(component["userCanArchive"]()).toBe(true); + }); + + it("hides archive link when feature flag is disabled", () => { + setArchiveState(false, [], false); + + const archiveLink = queryByTestId("archive-link"); + const premiumArchiveLink = queryByTestId("premium-archive-link"); + + expect(archiveLink).toBeNull(); + expect(premiumArchiveLink).toBeNull(); + expect(component["showArchiveItem"]()).toBe(false); + }); + + it("shows premium badge when user has no archived items and cannot archive", () => { + setArchiveState(false, []); + + expect(component["premiumBadgeComponent"]()).toBeTruthy(); + expect(component["userHasArchivedItems"]()).toBe(false); + }); + + it("hides premium badge when user has archived items", () => { + setArchiveState(false, [{ id: "cipher1" } as CipherView]); + + expect(component["premiumBadgeComponent"]()).toBeUndefined(); + expect(component["userHasArchivedItems"]()).toBe(true); + }); + }); +}); diff --git a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts index e085cb21c2d..c1d90d678cb 100644 --- a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts +++ b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts @@ -1,5 +1,5 @@ import { CommonModule } from "@angular/common"; -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Component, OnDestroy, OnInit, viewChild } from "@angular/core"; import { toSignal } from "@angular/core/rxjs-interop"; import { Router, RouterModule } from "@angular/router"; import { firstValueFrom, map, switchMap } from "rxjs"; @@ -42,6 +42,8 @@ import { BrowserPremiumUpgradePromptService } from "../services/browser-premium- ], }) export class VaultSettingsV2Component implements OnInit, OnDestroy { + private readonly premiumBadgeComponent = viewChild(PremiumBadgeComponent); + lastSync = "--"; private userId$ = this.accountService.activeAccount$.pipe(getUserId); @@ -117,4 +119,18 @@ export class VaultSettingsV2Component implements OnInit, OnDestroy { this.lastSync = this.i18nService.t("never"); } } + + /** + * When a user can archive or has previously archived items, route them to + * the archive page. Otherwise, prompt them to upgrade to premium. + */ + async conditionallyRouteToArchive(event: Event) { + event.preventDefault(); + const premiumBadge = this.premiumBadgeComponent(); + if (this.userCanArchive() || this.userHasArchivedItems()) { + await this.router.navigate(["/archive"]); + } else if (premiumBadge) { + await premiumBadge.promptForPremium(event); + } + } } From 06d15e96811f709f285299978424ae537abe4f6a Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Tue, 16 Dec 2025 15:03:48 -0800 Subject: [PATCH 3/4] [PM-27675] Browser item transfer integration (#17918) * [PM-27675] Integrate dialogs into VaultItemTransferService * [PM-27675] Update tests for new dialogs * [PM-27675] Center dialogs and prevent closing with escape or pointer events * [PM-27675] Add transferInProgress$ observable to VaultItemsTransferService * [PM-27675] Hook vault item transfer service into browser vault component * [PM-27675] Move defaultUserCollection$ to collection service * [PM-27675] Cleanup dialog styles * [PM-27675] Introduce readySubject to popup vault component to keep prevent flashing content while item transfer is in progress * [PM-27675] Fix vault-v2 tests --- .../vault-v2/vault-v2.component.html | 2 +- .../vault-v2/vault-v2.component.spec.ts | 18 +- .../components/vault-v2/vault-v2.component.ts | 39 +- .../abstractions/collection.service.ts | 8 + .../default-collection.service.spec.ts | 80 +++- .../services/default-collection.service.ts | 11 + .../vault-items-transfer.service.ts | 5 + .../leave-confirmation-dialog.component.html | 4 +- .../leave-confirmation-dialog.component.ts | 3 + .../transfer-items-dialog.component.html | 2 +- .../transfer-items-dialog.component.ts | 3 + ...fault-vault-items-transfer.service.spec.ts | 342 +++++++++++++----- .../default-vault-items-transfer.service.ts | 84 +++-- 13 files changed, 464 insertions(+), 137 deletions(-) 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", From ba1c74b9052a79806e1f159547f4e5cc442227e7 Mon Sep 17 00:00:00 2001 From: Jason Ng Date: Tue, 16 Dec 2025 18:04:54 -0500 Subject: [PATCH 4/4] [PM-29286] clear selected items when filter is changed (#17929) * check filter inside vault-items and clear on change --- .../vault-items/vault-items.component.spec.ts | 31 ++++++++++++++++++- .../vault-items/vault-items.component.ts | 28 ++++++++++++++++- .../vault-items/vault-items.stories.ts | 12 +++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.component.spec.ts b/apps/web/src/app/vault/components/vault-items/vault-items.component.spec.ts index 1eccb4c49ce..c1c25c625da 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.component.spec.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.spec.ts @@ -1,6 +1,6 @@ import { ScrollingModule } from "@angular/cdk/scrolling"; import { TestBed } from "@angular/core/testing"; -import { of } from "rxjs"; +import { of, Subject } from "rxjs"; import { CollectionView } from "@bitwarden/admin-console/common"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -11,12 +11,15 @@ import { RestrictedItemTypesService } from "@bitwarden/common/vault/services/res import { CipherViewLike } from "@bitwarden/common/vault/utils/cipher-view-like-utils"; import { MenuModule, TableModule } from "@bitwarden/components"; import { I18nPipe } from "@bitwarden/ui-common"; +import { RoutedVaultFilterService } from "@bitwarden/web-vault/app/vault/individual-vault/vault-filter/services/routed-vault-filter.service"; +import { RoutedVaultFilterModel } from "@bitwarden/web-vault/app/vault/individual-vault/vault-filter/shared/models/routed-vault-filter.model"; import { VaultItem } from "./vault-item"; import { VaultItemsComponent } from "./vault-items.component"; describe("VaultItemsComponent", () => { let component: VaultItemsComponent; + let filterSelect: Subject; const cipher1: Partial = { id: "cipher-1", @@ -31,6 +34,8 @@ describe("VaultItemsComponent", () => { }; beforeEach(async () => { + filterSelect = new Subject(); + await TestBed.configureTestingModule({ declarations: [VaultItemsComponent], imports: [ScrollingModule, TableModule, I18nPipe, MenuModule], @@ -61,6 +66,12 @@ describe("VaultItemsComponent", () => { hasArchiveFlagEnabled$: of(true), }, }, + { + provide: RoutedVaultFilterService, + useValue: { + filter$: filterSelect, + }, + }, ], }); @@ -143,4 +154,22 @@ describe("VaultItemsComponent", () => { expect(component.bulkUnarchiveAllowed).toBe(false); }); }); + + describe("filter change handling", () => { + it("clears selection when routed filter changes", () => { + const items: VaultItem[] = [ + { cipher: cipher1 as CipherView }, + { cipher: cipher2 as CipherView }, + ]; + + component["selection"].select(...items); + expect(component["selection"].selected.length).toBeGreaterThan(0); + + filterSelect.next({ + folderId: "folderId", + }); + + expect(component["selection"].selected.length).toBe(0); + }); + }); }); diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts index a935314eb3a..a51009a1e5b 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts @@ -3,7 +3,15 @@ import { SelectionModel } from "@angular/cdk/collections"; import { Component, EventEmitter, Input, Output } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; -import { Observable, combineLatest, map, of, startWith, switchMap } from "rxjs"; +import { + Observable, + combineLatest, + distinctUntilChanged, + map, + of, + startWith, + switchMap, +} from "rxjs"; import { CollectionView, Unassigned, CollectionAdminView } from "@bitwarden/admin-console/common"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; @@ -19,6 +27,7 @@ import { } from "@bitwarden/common/vault/utils/cipher-view-like-utils"; import { SortDirection, TableDataSource } from "@bitwarden/components"; import { OrganizationId } from "@bitwarden/sdk-internal"; +import { RoutedVaultFilterService } from "@bitwarden/web-vault/app/vault/individual-vault/vault-filter/services/routed-vault-filter.service"; import { GroupView } from "../../../admin-console/organizations/core"; @@ -152,6 +161,7 @@ export class VaultItemsComponent { protected cipherAuthorizationService: CipherAuthorizationService, protected restrictedItemTypesService: RestrictedItemTypesService, protected cipherArchiveService: CipherArchiveService, + protected routedVaultFilterService: RoutedVaultFilterService, ) { this.canDeleteSelected$ = this.selection.changed.pipe( startWith(null), @@ -219,6 +229,22 @@ export class VaultItemsComponent { ); }), ); + + this.routedVaultFilterService.filter$ + .pipe( + distinctUntilChanged( + (prev, curr) => + prev.organizationId === curr.organizationId && + prev.collectionId === curr.collectionId && + prev.folderId === curr.folderId && + prev.type === curr.type && + prev.organizationIdParamType === curr.organizationIdParamType, + ), + takeUntilDestroyed(), + ) + .subscribe(() => { + this.clearSelection(); + }); } clearSelection() { diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts index d973fbcbbc7..a71427cf475 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts @@ -40,6 +40,7 @@ import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cip import { RestrictedItemTypesService } from "@bitwarden/common/vault/services/restricted-item-types.service"; import { CipherViewLike } from "@bitwarden/common/vault/utils/cipher-view-like-utils"; import { LayoutComponent } from "@bitwarden/components"; +import { RoutedVaultFilterService } from "@bitwarden/web-vault/app/vault/individual-vault/vault-filter/services/routed-vault-filter.service"; import { GroupView } from "../../../admin-console/organizations/core"; import { PreloadedEnglishI18nModule } from "../../../core/tests"; @@ -150,6 +151,17 @@ export default { hasArchiveFlagEnabled$: of(true), }, }, + { + provide: RoutedVaultFilterService, + useValue: { + filter$: of({ + organizationId: null, + collectionId: null, + folderId: null, + type: null, + }), + }, + }, ], }), applicationConfig({