From 0c557c6ab8e606a73219a053bee56481e5ccb39d Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Wed, 17 Apr 2024 09:13:05 -0500 Subject: [PATCH 1/6] Guard Background Only and MV2 Only Actions (#8773) --- .../browser/src/background/main.background.ts | 206 +++++++++--------- .../src/popup/services/services.module.ts | 2 +- 2 files changed, 104 insertions(+), 104 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 642510b4de..69ed4cfa3d 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -342,11 +342,11 @@ export default class MainBackground { private syncTimeout: any; private isSafari: boolean; private nativeMessagingBackground: NativeMessagingBackground; - popupOnlyContext: boolean; - - constructor(public isPrivateMode: boolean = false) { - this.popupOnlyContext = isPrivateMode || BrowserApi.isManifestVersion(3); + constructor( + public isPrivateMode: boolean = false, + public popupOnlyContext: boolean = false, + ) { // Services const lockedCallback = async (userId?: string) => { if (this.notificationsService != null) { @@ -889,82 +889,83 @@ export default class MainBackground { this.isSafari = this.platformUtilsService.isSafari(); // Background - this.runtimeBackground = new RuntimeBackground( - this, - this.autofillService, - this.platformUtilsService as BrowserPlatformUtilsService, - this.i18nService, - this.notificationsService, - this.stateService, - this.autofillSettingsService, - this.systemService, - this.environmentService, - this.messagingService, - this.logService, - this.configService, - this.fido2Service, - ); - this.nativeMessagingBackground = new NativeMessagingBackground( - this.accountService, - this.masterPasswordService, - this.cryptoService, - this.cryptoFunctionService, - this.runtimeBackground, - this.messagingService, - this.appIdService, - this.platformUtilsService, - this.stateService, - this.logService, - this.authService, - this.biometricStateService, - ); - this.commandsBackground = new CommandsBackground( - this, - this.passwordGenerationService, - this.platformUtilsService, - this.vaultTimeoutService, - this.authService, - ); - this.notificationBackground = new NotificationBackground( - this.autofillService, - this.cipherService, - this.authService, - this.policyService, - this.folderService, - this.stateService, - this.userNotificationSettingsService, - this.domainSettingsService, - this.environmentService, - this.logService, - themeStateService, - this.configService, - ); - this.overlayBackground = new OverlayBackground( - this.cipherService, - this.autofillService, - this.authService, - this.environmentService, - this.domainSettingsService, - this.stateService, - this.autofillSettingsService, - this.i18nService, - this.platformUtilsService, - themeStateService, - ); - this.filelessImporterBackground = new FilelessImporterBackground( - this.configService, - this.authService, - this.policyService, - this.notificationBackground, - this.importService, - this.syncService, - ); - this.tabsBackground = new TabsBackground( - this, - this.notificationBackground, - this.overlayBackground, - ); if (!this.popupOnlyContext) { + this.runtimeBackground = new RuntimeBackground( + this, + this.autofillService, + this.platformUtilsService as BrowserPlatformUtilsService, + this.i18nService, + this.notificationsService, + this.stateService, + this.autofillSettingsService, + this.systemService, + this.environmentService, + this.messagingService, + this.logService, + this.configService, + this.fido2Service, + ); + this.nativeMessagingBackground = new NativeMessagingBackground( + this.accountService, + this.masterPasswordService, + this.cryptoService, + this.cryptoFunctionService, + this.runtimeBackground, + this.messagingService, + this.appIdService, + this.platformUtilsService, + this.stateService, + this.logService, + this.authService, + this.biometricStateService, + ); + this.commandsBackground = new CommandsBackground( + this, + this.passwordGenerationService, + this.platformUtilsService, + this.vaultTimeoutService, + this.authService, + ); + this.notificationBackground = new NotificationBackground( + this.autofillService, + this.cipherService, + this.authService, + this.policyService, + this.folderService, + this.stateService, + this.userNotificationSettingsService, + this.domainSettingsService, + this.environmentService, + this.logService, + themeStateService, + this.configService, + ); + this.overlayBackground = new OverlayBackground( + this.cipherService, + this.autofillService, + this.authService, + this.environmentService, + this.domainSettingsService, + this.stateService, + this.autofillSettingsService, + this.i18nService, + this.platformUtilsService, + themeStateService, + ); + this.filelessImporterBackground = new FilelessImporterBackground( + this.configService, + this.authService, + this.policyService, + this.notificationBackground, + this.importService, + this.syncService, + ); + this.tabsBackground = new TabsBackground( + this, + this.notificationBackground, + this.overlayBackground, + ); + const contextMenuClickedHandler = new ContextMenuClickedHandler( (options) => this.platformUtilsService.copyToClipboard(options.text), async (_tab) => { @@ -1006,11 +1007,6 @@ export default class MainBackground { this.notificationsService, this.accountService, ); - this.webRequestBackground = new WebRequestBackground( - this.platformUtilsService, - this.cipherService, - this.authService, - ); this.usernameGenerationService = new UsernameGenerationService( this.cryptoService, @@ -1032,34 +1028,40 @@ export default class MainBackground { this.authService, this.cipherService, ); + + if (BrowserApi.isManifestVersion(2)) { + this.webRequestBackground = new WebRequestBackground( + this.platformUtilsService, + this.cipherService, + this.authService, + ); + } } } async bootstrap() { this.containerService.attachToGlobal(self); - await this.stateService.init(); + await this.stateService.init({ runMigrations: !this.isPrivateMode }); - await this.vaultTimeoutService.init(true); await (this.i18nService as I18nService).init(); - await (this.eventUploadService as EventUploadService).init(true); - await this.runtimeBackground.init(); - await this.notificationBackground.init(); - this.filelessImporterBackground.init(); - await this.commandsBackground.init(); - + (this.eventUploadService as EventUploadService).init(true); this.twoFactorService.init(); - await this.overlayBackground.init(); - - await this.tabsBackground.init(); if (!this.popupOnlyContext) { + await this.vaultTimeoutService.init(true); + await this.runtimeBackground.init(); + await this.notificationBackground.init(); + this.filelessImporterBackground.init(); + await this.commandsBackground.init(); + await this.overlayBackground.init(); + await this.tabsBackground.init(); this.contextMenusBackground?.init(); + await this.idleBackground.init(); + if (BrowserApi.isManifestVersion(2)) { + await this.webRequestBackground.init(); + } } - await this.idleBackground.init(); - await this.webRequestBackground.init(); - - await this.fido2Service.init(); if (this.platformUtilsService.isFirefox() && !this.isPrivateMode) { // Set Private Mode windows to the default icon - they do not share state with the background page @@ -1082,9 +1084,7 @@ export default class MainBackground { if (!this.isPrivateMode) { await this.refreshBadge(); } - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.fullSync(true); + await this.fullSync(true); setTimeout(() => this.notificationsService.init(), 2500); resolve(); }, 500); @@ -1205,7 +1205,7 @@ export default class MainBackground { BrowserApi.sendMessage("updateBadge"); } await this.refreshBadge(); - await this.mainContextMenuHandler.noAccess(); + await this.mainContextMenuHandler?.noAccess(); // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises this.notificationsService.updateConnection(false); diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 40daf1b04d..fe70058640 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -120,7 +120,7 @@ const mainBackground: MainBackground = needsBackgroundInit : BrowserApi.getBackgroundPage().bitwardenMain; function createLocalBgService() { - const localBgService = new MainBackground(isPrivateMode); + const localBgService = new MainBackground(isPrivateMode, true); // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises localBgService.bootstrap(); From 1cde2dbaefb6c303f58678786faca89dacfd3e0b Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Wed, 17 Apr 2024 09:38:47 -0500 Subject: [PATCH 2/6] [PM-7527] Add `Build Manifest v3` build step to the build-browser.yml Github action (#8777) * [PM-7527] Get MV3 build artifacts in main branch with clear messaging that that the build is not to be released * [PM-7527] Add `Build Manifest v3` build step to the build-browser.yml Github action --- .github/workflows/build-browser.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-browser.yml b/.github/workflows/build-browser.yml index 4fb72a47de..23f4bd35f1 100644 --- a/.github/workflows/build-browser.yml +++ b/.github/workflows/build-browser.yml @@ -160,9 +160,9 @@ jobs: run: npm run dist working-directory: browser-source/apps/browser - # - name: Build Manifest v3 - # run: npm run dist:mv3 - # working-directory: browser-source/apps/browser + - name: Build Manifest v3 + run: npm run dist:mv3 + working-directory: browser-source/apps/browser - name: Gulp run: gulp ci From 3179867310e2fec5913b73f67e364b69273b590f Mon Sep 17 00:00:00 2001 From: Tom <144813356+ttalty@users.noreply.github.com> Date: Wed, 17 Apr 2024 10:39:15 -0400 Subject: [PATCH 3/6] [PM-7352] - Browser Send Type Groupings Fix (#8727) * Removed type counts and just calculated counts off the sends * Fixing key spec test case --- .../src/models/browserSendComponentState.ts | 10 ---------- .../popup/send/send-groupings.component.html | 4 ++-- .../popup/send/send-groupings.component.ts | 20 ++----------------- .../browser-send-state.service.spec.ts | 2 -- .../services/browser-send-state.service.ts | 2 +- .../popup/services/key-definitions.spec.ts | 5 ++--- 6 files changed, 7 insertions(+), 36 deletions(-) diff --git a/apps/browser/src/models/browserSendComponentState.ts b/apps/browser/src/models/browserSendComponentState.ts index 9158efc21d..81dd93323b 100644 --- a/apps/browser/src/models/browserSendComponentState.ts +++ b/apps/browser/src/models/browserSendComponentState.ts @@ -1,5 +1,3 @@ -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SendType } from "@bitwarden/common/tools/send/enums/send-type"; import { SendView } from "@bitwarden/common/tools/send/models/view/send.view"; import { DeepJsonify } from "@bitwarden/common/types/deep-jsonify"; @@ -7,13 +5,6 @@ import { BrowserComponentState } from "./browserComponentState"; export class BrowserSendComponentState extends BrowserComponentState { sends: SendView[]; - typeCounts: Map; - - toJSON() { - return Utils.merge(this, { - typeCounts: Utils.mapToRecord(this.typeCounts), - }); - } static fromJSON(json: DeepJsonify) { if (json == null) { @@ -22,7 +13,6 @@ export class BrowserSendComponentState extends BrowserComponentState { return Object.assign(new BrowserSendComponentState(), json, { sends: json.sends?.map((s) => SendView.fromJSON(s)), - typeCounts: Utils.recordToMap(json.typeCounts), }); } } diff --git a/apps/browser/src/tools/popup/send/send-groupings.component.html b/apps/browser/src/tools/popup/send/send-groupings.component.html index edeabd6546..213afdfa22 100644 --- a/apps/browser/src/tools/popup/send/send-groupings.component.html +++ b/apps/browser/src/tools/popup/send/send-groupings.component.html @@ -61,7 +61,7 @@
{{ "sendTypeText" | i18n }} - {{ typeCounts.get(sendType.Text) || 0 }} + {{ getSendCount(sends, sendType.Text) }} diff --git a/apps/browser/src/tools/popup/send/send-groupings.component.ts b/apps/browser/src/tools/popup/send/send-groupings.component.ts index a49773367d..87d03c4b76 100644 --- a/apps/browser/src/tools/popup/send/send-groupings.component.ts +++ b/apps/browser/src/tools/popup/send/send-groupings.component.ts @@ -29,8 +29,6 @@ const ComponentId = "SendComponent"; export class SendGroupingsComponent extends BaseSendComponent { // Header showLeftHeader = true; - // Send Type Calculations - typeCounts = new Map(); // State Handling state: BrowserSendComponentState; private loadedTimeout: number; @@ -65,7 +63,6 @@ export class SendGroupingsComponent extends BaseSendComponent { dialogService, ); super.onSuccessfulLoad = async () => { - this.calculateTypeCounts(); this.selectAll(); }; } @@ -174,17 +171,8 @@ export class SendGroupingsComponent extends BaseSendComponent { return this.hasSearched || (!this.searchPending && this.isSearchable); } - private calculateTypeCounts() { - // Create type counts - const typeCounts = new Map(); - this.sends.forEach((s) => { - if (typeCounts.has(s.type)) { - typeCounts.set(s.type, typeCounts.get(s.type) + 1); - } else { - typeCounts.set(s.type, 1); - } - }); - this.typeCounts = typeCounts; + getSendCount(sends: SendView[], type: SendType): number { + return sends.filter((s) => s.type === type).length; } private async saveState() { @@ -192,7 +180,6 @@ export class SendGroupingsComponent extends BaseSendComponent { scrollY: BrowserPopupUtils.getContentScrollY(window), searchText: this.searchText, sends: this.sends, - typeCounts: this.typeCounts, }); await this.stateService.setBrowserSendComponentState(this.state); } @@ -206,9 +193,6 @@ export class SendGroupingsComponent extends BaseSendComponent { if (this.state.sends != null) { this.sends = this.state.sends; } - if (this.state.typeCounts != null) { - this.typeCounts = this.state.typeCounts; - } return true; } diff --git a/apps/browser/src/tools/popup/services/browser-send-state.service.spec.ts b/apps/browser/src/tools/popup/services/browser-send-state.service.spec.ts index 3dafc0934a..6f0ae1455a 100644 --- a/apps/browser/src/tools/popup/services/browser-send-state.service.spec.ts +++ b/apps/browser/src/tools/popup/services/browser-send-state.service.spec.ts @@ -6,7 +6,6 @@ import { FakeStateProvider } from "@bitwarden/common/../spec/fake-state-provider import { awaitAsync } from "@bitwarden/common/../spec/utils"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SendType } from "@bitwarden/common/tools/send/enums/send-type"; import { UserId } from "@bitwarden/common/types/guid"; import { BrowserComponentState } from "../../../models/browserComponentState"; @@ -33,7 +32,6 @@ describe("Browser Send State Service", () => { const state = new BrowserSendComponentState(); state.scrollY = 0; state.searchText = "test"; - state.typeCounts = new Map().set(SendType.File, 1); await stateService.setBrowserSendComponentState(state); diff --git a/apps/browser/src/tools/popup/services/browser-send-state.service.ts b/apps/browser/src/tools/popup/services/browser-send-state.service.ts index b814ee5bc9..52aeb01a92 100644 --- a/apps/browser/src/tools/popup/services/browser-send-state.service.ts +++ b/apps/browser/src/tools/popup/services/browser-send-state.service.ts @@ -42,7 +42,7 @@ export class BrowserSendStateService { } /** Set the active user's browser send component state - * @param { BrowserSendComponentState } value sets the sends and type counts along with the scroll position and search text for + * @param { BrowserSendComponentState } value sets the sends along with the scroll position and search text for * the send component on the browser */ async setBrowserSendComponentState(value: BrowserSendComponentState): Promise { diff --git a/apps/browser/src/tools/popup/services/key-definitions.spec.ts b/apps/browser/src/tools/popup/services/key-definitions.spec.ts index 3ba574efa3..7517771669 100644 --- a/apps/browser/src/tools/popup/services/key-definitions.spec.ts +++ b/apps/browser/src/tools/popup/services/key-definitions.spec.ts @@ -1,7 +1,5 @@ import { Jsonify } from "type-fest"; -import { SendType } from "@bitwarden/common/tools/send/enums/send-type"; - import { BrowserSendComponentState } from "../../../models/browserSendComponentState"; import { BROWSER_SEND_COMPONENT, BROWSER_SEND_TYPE_COMPONENT } from "./key-definitions"; @@ -12,7 +10,8 @@ describe("Key definitions", () => { const keyDef = BROWSER_SEND_COMPONENT; const expectedState = { - typeCounts: new Map(), + scrollY: 0, + searchText: "test", }; const result = keyDef.deserializer( From 395f2e806e616baffcb7f5f869ddf0ce3acd9381 Mon Sep 17 00:00:00 2001 From: aj-rosado <109146700+aj-rosado@users.noreply.github.com> Date: Wed, 17 Apr 2024 16:06:25 +0100 Subject: [PATCH 4/6] Calling closeAll from dialogService on Send ngOnDestroy (#8763) --- apps/web/src/app/tools/send/send.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/web/src/app/tools/send/send.component.ts b/apps/web/src/app/tools/send/send.component.ts index b5edd1433e..755435882a 100644 --- a/apps/web/src/app/tools/send/send.component.ts +++ b/apps/web/src/app/tools/send/send.component.ts @@ -92,6 +92,7 @@ export class SendComponent extends BaseSendComponent { } ngOnDestroy() { + this.dialogService.closeAll(); this.broadcasterService.unsubscribe(BroadcasterSubscriptionId); } From 28a89ddb8605832d4ff8147dfbc2ccf30c2daf53 Mon Sep 17 00:00:00 2001 From: aj-rosado <109146700+aj-rosado@users.noreply.github.com> Date: Wed, 17 Apr 2024 16:19:06 +0100 Subject: [PATCH 5/6] [PM-7304] Add missing i18n keys for import errors (#8743) * Add static error message when import fails * Adding missing string on CLI for unassigned items * Added missing string on setImportTarget * fixed tests --- apps/browser/src/_locales/en/messages.json | 6 ++++++ apps/cli/src/locales/en/messages.json | 9 +++++++++ apps/desktop/src/locales/en/messages.json | 6 ++++++ apps/web/src/locales/en/messages.json | 6 ++++++ libs/importer/src/services/import.service.spec.ts | 4 ++-- libs/importer/src/services/import.service.ts | 4 ++-- 6 files changed, 31 insertions(+), 4 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 5e941083df..f599df470b 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -3011,5 +3011,11 @@ }, "unassignedItemsBannerSelfHost": { "message": "Notice: On May 2, 2024, unassigned organization items will no longer be visible in the All Vaults view and will only be accessible via the Admin Console. Assign these items to a collection from the Admin Console to make them visible." + }, + "errorAssigningTargetCollection": { + "message": "Error assigning target collection." + }, + "errorAssigningTargetFolder": { + "message": "Error assigning target folder." } } diff --git a/apps/cli/src/locales/en/messages.json b/apps/cli/src/locales/en/messages.json index e12b30af2c..8364e0b328 100644 --- a/apps/cli/src/locales/en/messages.json +++ b/apps/cli/src/locales/en/messages.json @@ -49,5 +49,14 @@ }, "unsupportedEncryptedImport": { "message": "Importing encrypted files is currently not supported." + }, + "importUnassignedItemsError": { + "message": "File contains unassigned items." + }, + "errorAssigningTargetCollection": { + "message": "Error assigning target collection." + }, + "errorAssigningTargetFolder": { + "message": "Error assigning target folder." } } diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index a0d34e4075..00eace54e2 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -2705,5 +2705,11 @@ }, "passkeyRemoved": { "message": "Passkey removed" + }, + "errorAssigningTargetCollection": { + "message": "Error assigning target collection." + }, + "errorAssigningTargetFolder": { + "message": "Error assigning target folder." } } diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index c8dfa14c8b..680014f1f0 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -7944,5 +7944,11 @@ }, "deleteProviderWarning": { "message": "Deleting your provider is permanent. It cannot be undone." + }, + "errorAssigningTargetCollection": { + "message": "Error assigning target collection." + }, + "errorAssigningTargetFolder": { + "message": "Error assigning target folder." } } diff --git a/libs/importer/src/services/import.service.spec.ts b/libs/importer/src/services/import.service.spec.ts index eb21f384b5..7bbcd3287a 100644 --- a/libs/importer/src/services/import.service.spec.ts +++ b/libs/importer/src/services/import.service.spec.ts @@ -196,7 +196,7 @@ describe("ImportService", () => { new Object() as FolderView, ); - await expect(setImportTargetMethod).rejects.toThrow("Error assigning target collection"); + await expect(setImportTargetMethod).rejects.toThrow(); }); it("passing importTarget as null on setImportTarget throws error", async () => { @@ -206,7 +206,7 @@ describe("ImportService", () => { new Object() as CollectionView, ); - await expect(setImportTargetMethod).rejects.toThrow("Error assigning target folder"); + await expect(setImportTargetMethod).rejects.toThrow(); }); it("passing importTarget, collectionRelationship has the expected values", async () => { diff --git a/libs/importer/src/services/import.service.ts b/libs/importer/src/services/import.service.ts index 62961a77c4..f5cab933f3 100644 --- a/libs/importer/src/services/import.service.ts +++ b/libs/importer/src/services/import.service.ts @@ -432,7 +432,7 @@ export class ImportService implements ImportServiceAbstraction { if (organizationId) { if (!(importTarget instanceof CollectionView)) { - throw new Error("Error assigning target collection"); + throw new Error(this.i18nService.t("errorAssigningTargetCollection")); } const noCollectionRelationShips: [number, number][] = []; @@ -463,7 +463,7 @@ export class ImportService implements ImportServiceAbstraction { } if (!(importTarget instanceof FolderView)) { - throw new Error("Error assigning target folder"); + throw new Error(this.i18nService.t("errorAssigningTargetFolder")); } const noFolderRelationShips: [number, number][] = []; From f15bffb040afa8b5d40405d042d8baf951ce8193 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Wed, 17 Apr 2024 12:02:21 -0400 Subject: [PATCH 6/6] Handle null values coming from state (#8784) --- libs/common/src/vault/services/cipher.service.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index e8544d7f98..dce58d3ba5 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -1,4 +1,4 @@ -import { Observable, firstValueFrom } from "rxjs"; +import { Observable, firstValueFrom, map } from "rxjs"; import { SemVer } from "semver"; import { ApiService } from "../../abstractions/api.service"; @@ -100,9 +100,9 @@ export class CipherService implements CipherServiceAbstraction { this.decryptedCiphersState = this.stateProvider.getActive(DECRYPTED_CIPHERS); this.addEditCipherInfoState = this.stateProvider.getActive(ADD_EDIT_CIPHER_INFO_KEY); - this.localData$ = this.localDataState.state$; - this.ciphers$ = this.encryptedCiphersState.state$; - this.cipherViews$ = this.decryptedCiphersState.state$; + this.localData$ = this.localDataState.state$.pipe(map((data) => data ?? {})); + this.ciphers$ = this.encryptedCiphersState.state$.pipe(map((ciphers) => ciphers ?? {})); + this.cipherViews$ = this.decryptedCiphersState.state$.pipe(map((views) => views ?? {})); this.addEditCipherInfo$ = this.addEditCipherInfoState.state$; }