From ee2e7be98ceaced1a7842d61ddee15f3cb621d37 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Mon, 20 May 2024 09:21:09 -0400 Subject: [PATCH 1/9] Clone Initial Data In `runMigrator` - When using test cases, mutating the input data causes problems. --- libs/common/src/state-migrations/migration-helper.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/common/src/state-migrations/migration-helper.spec.ts b/libs/common/src/state-migrations/migration-helper.spec.ts index 9f6fa7474788..49e6e7fe9cf3 100644 --- a/libs/common/src/state-migrations/migration-helper.spec.ts +++ b/libs/common/src/state-migrations/migration-helper.spec.ts @@ -356,10 +356,12 @@ export async function runMigrator< initalData?: InitialDataHint, direction: "migrate" | "rollback" = "migrate", ): Promise> { + const clonedData = JSON.parse(JSON.stringify(initalData ?? {})); + // Inject fake data at every level of the object - const allInjectedData = injectData(initalData, []); + const allInjectedData = injectData(clonedData, []); - const fakeStorageService = new FakeStorageService(initalData); + const fakeStorageService = new FakeStorageService(clonedData); const helper = new MigrationHelper( migrator.fromVersion, fakeStorageService, From f068625547eaf9a3c6841417d70d47cff9fadd5d Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Mon, 20 May 2024 09:26:43 -0400 Subject: [PATCH 2/9] Migrate `minimizeOnCopy` & `browserIntegrationEnabled` --- .../src/app/accounts/settings.component.ts | 12 +- apps/desktop/src/main.ts | 63 +------- apps/desktop/src/main/messaging.main.ts | 22 +-- apps/desktop/src/main/window.main.ts | 2 - .../services/desktop-settings.service.ts | 53 +++++++ .../platform/abstractions/state.service.ts | 4 - .../src/platform/models/domain/account.ts | 1 - .../platform/models/domain/global-state.ts | 1 - .../src/platform/services/state.service.ts | 36 ----- .../63-move-desktop-settings.spec.ts | 139 ++++++++++++++++++ .../migrations/63-move-desktop-settings.ts | 84 +++++++++++ 11 files changed, 299 insertions(+), 118 deletions(-) create mode 100644 libs/common/src/state-migrations/migrations/63-move-desktop-settings.spec.ts create mode 100644 libs/common/src/state-migrations/migrations/63-move-desktop-settings.ts diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index cfc646337acc..575b40f262b2 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -278,7 +278,7 @@ export class SettingsComponent implements OnInit { approveLoginRequests: (await this.authRequestService.getAcceptAuthRequests(this.currentUserId)) ?? false, clearClipboard: await firstValueFrom(this.autofillSettingsService.clearClipboardDelay$), - minimizeOnCopyToClipboard: await this.stateService.getMinimizeOnCopyToClipboard(), + minimizeOnCopyToClipboard: await firstValueFrom(this.desktopSettingsService.minimizeOnCopy$), enableFavicons: await firstValueFrom(this.domainSettingsService.showFavicons$), enableTray: await firstValueFrom(this.desktopSettingsService.trayEnabled$), enableMinToTray: await firstValueFrom(this.desktopSettingsService.minimizeToTray$), @@ -286,7 +286,9 @@ export class SettingsComponent implements OnInit { startToTray: await firstValueFrom(this.desktopSettingsService.startToTray$), openAtLogin: await firstValueFrom(this.desktopSettingsService.openAtLogin$), alwaysShowDock: await firstValueFrom(this.desktopSettingsService.alwaysShowDock$), - enableBrowserIntegration: await this.stateService.getEnableBrowserIntegration(), + enableBrowserIntegration: await firstValueFrom( + this.desktopSettingsService.browserIntegrationEnabled$, + ), enableBrowserIntegrationFingerprint: await this.stateService.getEnableBrowserIntegrationFingerprint(), enableDuckDuckGoBrowserIntegration: await firstValueFrom( @@ -598,7 +600,7 @@ export class SettingsComponent implements OnInit { } async saveMinOnCopyToClipboard() { - await this.stateService.setMinimizeOnCopyToClipboard(this.form.value.minimizeOnCopyToClipboard); + await this.desktopSettingsService.setMinimizeOnCopy(this.form.value.minimizeOnCopyToClipboard); } async saveClearClipboard() { @@ -656,7 +658,9 @@ export class SettingsComponent implements OnInit { return; } - await this.stateService.setEnableBrowserIntegration(this.form.value.enableBrowserIntegration); + await this.desktopSettingsService.setBrowserIntegrationEnabled( + this.form.value.enableBrowserIntegration, + ); const errorResult = await this.nativeMessagingManifestService.generate( this.form.value.enableBrowserIntegration, diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 63d6e062a1eb..f5799784f687 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -3,22 +3,13 @@ import * as path from "path"; import { app } from "electron"; import { Subject, firstValueFrom } from "rxjs"; -import { TokenService as TokenServiceAbstraction } from "@bitwarden/common/auth/abstractions/token.service"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; -import { TokenService } from "@bitwarden/common/auth/services/token.service"; import { ClientType } from "@bitwarden/common/enums"; -import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; -import { KeyGenerationService as KeyGenerationServiceAbstraction } from "@bitwarden/common/platform/abstractions/key-generation.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { DefaultBiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; -import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; import { Message, MessageSender } from "@bitwarden/common/platform/messaging"; // eslint-disable-next-line no-restricted-imports -- For dependency creation import { SubjectMessageSender } from "@bitwarden/common/platform/messaging/internal"; -import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; -import { EncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/encrypt.service.implementation"; import { DefaultEnvironmentService } from "@bitwarden/common/platform/services/default-environment.service"; -import { KeyGenerationService } from "@bitwarden/common/platform/services/key-generation.service"; import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service"; import { MigrationBuilderService } from "@bitwarden/common/platform/services/migration-builder.service"; import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner"; @@ -41,18 +32,13 @@ import { PowerMonitorMain } from "./main/power-monitor.main"; import { TrayMain } from "./main/tray.main"; import { UpdaterMain } from "./main/updater.main"; import { WindowMain } from "./main/window.main"; -import { Account } from "./models/account"; import { BiometricsService, BiometricsServiceAbstraction } from "./platform/main/biometric/index"; import { ClipboardMain } from "./platform/main/clipboard.main"; import { DesktopCredentialStorageListener } from "./platform/main/desktop-credential-storage-listener"; -import { MainCryptoFunctionService } from "./platform/main/main-crypto-function.service"; import { DesktopSettingsService } from "./platform/services/desktop-settings.service"; import { ElectronLogMainService } from "./platform/services/electron-log.main.service"; -import { ELECTRON_SUPPORTS_SECURE_STORAGE } from "./platform/services/electron-platform-utils.service"; -import { ElectronStateService } from "./platform/services/electron-state.service"; import { ElectronStorageService } from "./platform/services/electron-storage.service"; import { I18nMainService } from "./platform/services/i18n.main.service"; -import { IllegalSecureStorageService } from "./platform/services/illegal-secure-storage.service"; import { ElectronMainMessagingService } from "./services/electron-main-messaging.service"; import { isMacAppStore } from "./utils"; @@ -63,15 +49,10 @@ export class Main { memoryStorageService: MemoryStorageService; memoryStorageForStateProviders: MemoryStorageServiceForStateProviders; messagingService: MessageSender; - stateService: StateService; environmentService: DefaultEnvironmentService; - mainCryptoFunctionService: MainCryptoFunctionService; desktopCredentialStorageListener: DesktopCredentialStorageListener; desktopSettingsService: DesktopSettingsService; migrationRunner: MigrationRunner; - tokenService: TokenServiceAbstraction; - keyGenerationService: KeyGenerationServiceAbstraction; - encryptService: EncryptService; windowMain: WindowMain; messagingMain: MessagingMain; @@ -160,30 +141,6 @@ export class Main { this.environmentService = new DefaultEnvironmentService(stateProvider, accountService); - this.mainCryptoFunctionService = new MainCryptoFunctionService(); - this.mainCryptoFunctionService.init(); - - this.keyGenerationService = new KeyGenerationService(this.mainCryptoFunctionService); - - this.encryptService = new EncryptServiceImplementation( - this.mainCryptoFunctionService, - this.logService, - true, // log mac failures - ); - - // Note: secure storage service is not available and should not be called in the main background process. - const illegalSecureStorageService = new IllegalSecureStorageService(); - - this.tokenService = new TokenService( - singleUserStateProvider, - globalStateProvider, - ELECTRON_SUPPORTS_SECURE_STORAGE, - illegalSecureStorageService, - this.keyGenerationService, - this.encryptService, - this.logService, - ); - this.migrationRunner = new MigrationRunner( this.storageService, this.logService, @@ -191,27 +148,11 @@ export class Main { ClientType.Desktop, ); - // TODO: this state service will have access to on disk storage, but not in memory storage. - // If we could get this to work using the stateService singleton that the rest of the app uses we could save - // ourselves from some hacks, like having to manually update the app menu vs. the menu subscribing to events. - this.stateService = new ElectronStateService( - this.storageService, - null, - this.memoryStorageService, - this.logService, - new StateFactory(GlobalState, Account), - accountService, // will not broadcast logouts. This is a hack until we can remove messaging dependency - this.environmentService, - this.tokenService, - this.migrationRunner, - ); - this.desktopSettingsService = new DesktopSettingsService(stateProvider); const biometricStateService = new DefaultBiometricStateService(stateProvider); this.windowMain = new WindowMain( - this.stateService, biometricStateService, this.logService, this.storageService, @@ -219,7 +160,7 @@ export class Main { (arg) => this.processDeepLink(arg), (win) => this.trayMain.setupWindowListeners(win), ); - this.messagingMain = new MessagingMain(this, this.stateService, this.desktopSettingsService); + this.messagingMain = new MessagingMain(this, this.desktopSettingsService, this.logService); this.updaterMain = new UpdaterMain(this.i18nService, this.windowMain); this.trayMain = new TrayMain(this.windowMain, this.i18nService, this.desktopSettingsService); @@ -298,7 +239,7 @@ export class Main { await this.updaterMain.init(); const [browserIntegrationEnabled, ddgIntegrationEnabled] = await Promise.all([ - this.stateService.getEnableBrowserIntegration(), + firstValueFrom(this.desktopSettingsService.browserIntegrationEnabled$), firstValueFrom(this.desktopAutofillSettingsService.enableDuckDuckGoBrowserIntegration$), ]); diff --git a/apps/desktop/src/main/messaging.main.ts b/apps/desktop/src/main/messaging.main.ts index a9f80b7d207f..0f2a3c25c3c1 100644 --- a/apps/desktop/src/main/messaging.main.ts +++ b/apps/desktop/src/main/messaging.main.ts @@ -2,8 +2,9 @@ import * as fs from "fs"; import * as path from "path"; import { app, ipcMain } from "electron"; +import { firstValueFrom } from "rxjs"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { Main } from "../main"; import { DesktopSettingsService } from "../platform/services/desktop-settings.service"; @@ -17,8 +18,8 @@ export class MessagingMain { constructor( private main: Main, - private stateService: StateService, private desktopSettingsService: DesktopSettingsService, + private readonly logService: LogService, ) {} async init() { @@ -44,13 +45,16 @@ export class MessagingMain { this.updateTrayMenu(message.updateRequest); break; case "minimizeOnCopy": - // 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.stateService.getMinimizeOnCopyToClipboard().then((shouldMinimize) => { - if (shouldMinimize && this.main.windowMain.win !== null) { - this.main.windowMain.win.minimize(); - } - }); + firstValueFrom(this.desktopSettingsService.minimizeOnCopy$).then( + (shouldMinimize) => { + if (shouldMinimize && this.main.windowMain.win !== null) { + this.main.windowMain.win.minimize(); + } + }, + (error) => { + this.logService.error("Error while retrieving minimize on copy setting", error); + }, + ); break; case "showTray": this.main.trayMain.showTray(); diff --git a/apps/desktop/src/main/window.main.ts b/apps/desktop/src/main/window.main.ts index 64b4bc48d288..e82d16ee9fd4 100644 --- a/apps/desktop/src/main/window.main.ts +++ b/apps/desktop/src/main/window.main.ts @@ -6,7 +6,6 @@ import { app, BrowserWindow, ipcMain, nativeTheme, screen, session } from "elect import { firstValueFrom } from "rxjs"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; @@ -38,7 +37,6 @@ export class WindowMain { readonly defaultHeight = 600; constructor( - private stateService: StateService, private biometricStateService: BiometricStateService, private logService: LogService, private storageService: AbstractStorageService, diff --git a/apps/desktop/src/platform/services/desktop-settings.service.ts b/apps/desktop/src/platform/services/desktop-settings.service.ts index 09ddad07c1bc..aade7d3d4622 100644 --- a/apps/desktop/src/platform/services/desktop-settings.service.ts +++ b/apps/desktop/src/platform/services/desktop-settings.service.ts @@ -4,7 +4,9 @@ import { DESKTOP_SETTINGS_DISK, KeyDefinition, StateProvider, + UserKeyDefinition, } from "@bitwarden/common/platform/state"; +import { UserId } from "@bitwarden/common/types/guid"; import { WindowState } from "../models/domain/window-state"; @@ -48,6 +50,19 @@ const ALWAYS_ON_TOP_KEY = new KeyDefinition(DESKTOP_SETTINGS_DISK, "alw deserializer: (b) => b, }); +const BROWSER_INTEGRATION_ENABLED = new KeyDefinition( + DESKTOP_SETTINGS_DISK, + "browserIntegrationEnabled", + { + deserializer: (b) => b, + }, +); + +const MINIMIZE_ON_COPY = new UserKeyDefinition(DESKTOP_SETTINGS_DISK, "minimizeOnCopy", { + deserializer: (b) => b, + clearOn: [], // User setting, no need to clear +}); + /** * Various settings for controlling application behavior specific to the desktop client. */ @@ -97,6 +112,25 @@ export class DesktopSettingsService { alwaysOnTop$ = this.alwaysOnTopState.state$.pipe(map((value) => value ?? false)); + private readonly browserIntegrationEnabledState = this.stateProvider.getGlobal( + BROWSER_INTEGRATION_ENABLED, + ); + + /** + * The application setting for whether or not the browser integration is enabled. + */ + browserIntegrationEnabled$ = this.browserIntegrationEnabledState.state$.pipe( + map((value) => value ?? false), + ); + + private readonly minimizeOnCopyState = this.stateProvider.getActive(MINIMIZE_ON_COPY); + + /** + * The active users setting for whether or not the application should minimize itself + * when a value is copied to the clipboard. + */ + minimizeOnCopy$ = this.minimizeOnCopyState.state$.pipe(map((value) => value ?? false)); + constructor(private stateProvider: StateProvider) { this.window$ = this.windowState.state$.pipe( map((window) => @@ -177,4 +211,23 @@ export class DesktopSettingsService { async setAlwaysOnTop(value: boolean) { await this.alwaysOnTopState.update(() => value); } + + /** + * Sets a setting for whether or not the browser integration has been enabled. + * @param value `true` if the integration with the browser extension is enabled, + * `false` if it is not. + */ + async setBrowserIntegrationEnabled(value: boolean) { + await this.browserIntegrationEnabledState.update(() => value); + } + + /** + * Sets the minimize on copy value for the current user. + * @param value `true` if the application should minimize when a value is copied, + * `false` if it should not. + * @param userId The user id of the user to update the setting for. + */ + async setMinimizeOnCopy(value: boolean, userId: UserId) { + await this.stateProvider.getUser(userId, MINIMIZE_ON_COPY).update(() => value); + } } diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index b2ea27ecb05f..8466688bdd75 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -82,8 +82,6 @@ export abstract class StateService { ) => Promise; getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise; setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise; - getEnableBrowserIntegration: (options?: StorageOptions) => Promise; - setEnableBrowserIntegration: (value: boolean, options?: StorageOptions) => Promise; getEnableBrowserIntegrationFingerprint: (options?: StorageOptions) => Promise; setEnableBrowserIntegrationFingerprint: ( value: boolean, @@ -99,8 +97,6 @@ export abstract class StateService { getIsAuthenticated: (options?: StorageOptions) => Promise; getLastSync: (options?: StorageOptions) => Promise; setLastSync: (value: string, options?: StorageOptions) => Promise; - getMinimizeOnCopyToClipboard: (options?: StorageOptions) => Promise; - setMinimizeOnCopyToClipboard: (value: boolean, options?: StorageOptions) => Promise; getOrganizationInvitation: (options?: StorageOptions) => Promise; setOrganizationInvitation: (value: any, options?: StorageOptions) => Promise; getPasswordGenerationOptions: (options?: StorageOptions) => Promise; diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 8c7d70948cb8..8e26e4ed30ee 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -143,7 +143,6 @@ export class AccountProfile { export class AccountSettings { defaultUriMatch?: UriMatchStrategySetting; - minimizeOnCopyToClipboard?: boolean; passwordGenerationOptions?: PasswordGeneratorOptions; usernameGenerationOptions?: UsernameGeneratorOptions; generatorOptions?: GeneratorOptions; diff --git a/libs/common/src/platform/models/domain/global-state.ts b/libs/common/src/platform/models/domain/global-state.ts index 2d48f09e9250..a1f494922fab 100644 --- a/libs/common/src/platform/models/domain/global-state.ts +++ b/libs/common/src/platform/models/domain/global-state.ts @@ -1,6 +1,5 @@ export class GlobalState { organizationInvitation?: any; - enableBrowserIntegration?: boolean; enableBrowserIntegrationFingerprint?: boolean; enableDuckDuckGoBrowserIntegration?: boolean; } diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 0339baa3fa35..493b5be894fb 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -347,24 +347,6 @@ export class StateService< : await this.secureStorageService.save(DDG_SHARED_KEY, value, options); } - async getEnableBrowserIntegration(options?: StorageOptions): Promise { - return ( - (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.enableBrowserIntegration ?? false - ); - } - - async setEnableBrowserIntegration(value: boolean, options?: StorageOptions): Promise { - const globals = await this.getGlobals( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - globals.enableBrowserIntegration = value; - await this.saveGlobals( - globals, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getEnableBrowserIntegrationFingerprint(options?: StorageOptions): Promise { return ( (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) @@ -456,24 +438,6 @@ export class StateService< ); } - async getMinimizeOnCopyToClipboard(options?: StorageOptions): Promise { - return ( - (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.settings?.minimizeOnCopyToClipboard ?? false - ); - } - - async setMinimizeOnCopyToClipboard(value: boolean, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.settings.minimizeOnCopyToClipboard = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getOrganizationInvitation(options?: StorageOptions): Promise { return ( await this.getGlobals(this.reconcileOptions(options, await this.defaultInMemoryOptions())) diff --git a/libs/common/src/state-migrations/migrations/63-move-desktop-settings.spec.ts b/libs/common/src/state-migrations/migrations/63-move-desktop-settings.spec.ts new file mode 100644 index 000000000000..f3572b6256e6 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/63-move-desktop-settings.spec.ts @@ -0,0 +1,139 @@ +import { runMigrator } from "../migration-helper.spec"; + +import { MoveDesktopSettingsMigrator } from "./63-move-desktop-settings"; + +describe("MoveDesktopSettings", () => { + const sut = new MoveDesktopSettingsMigrator(62, 63); + + const cases: { + it: string; + preMigration: Record; + postMigration: Record; + }[] = [ + { + it: "moves truthy values", + preMigration: { + global_account_accounts: { + user1: {}, + otherUser: {}, + }, + user1: { + settings: { + minimizeOnCopyToClipboard: true, + }, + }, + otherUser: { + settings: { + random: "stuff", + }, + }, + global: { + enableBrowserIntegration: true, + }, + }, + postMigration: { + global_account_accounts: { + user1: {}, + otherUser: {}, + }, + global: {}, + user1: { + settings: {}, + }, + otherUser: { + settings: { + random: "stuff", + }, + }, + global_desktopSettings_browserIntegrationEnabled: true, + user_user1_desktopSettings_minimizeOnCopy: true, + }, + }, + { + it: "moves falsey values", + preMigration: { + global_account_accounts: { + user1: {}, + otherUser: {}, + }, + user1: { + settings: { + minimizeOnCopyToClipboard: false, + }, + }, + otherUser: { + settings: { + random: "stuff", + }, + }, + global: { + enableBrowserIntegration: false, + }, + }, + postMigration: { + global_account_accounts: { + user1: {}, + otherUser: {}, + }, + global: {}, + user1: { + settings: {}, + }, + otherUser: { + settings: { + random: "stuff", + }, + }, + global_desktopSettings_browserIntegrationEnabled: false, + user_user1_desktopSettings_minimizeOnCopy: false, + }, + }, + { + it: "does not move non-existant values", + preMigration: { + global_account_accounts: { + user1: {}, + otherUser: {}, + }, + user1: { + settings: {}, + }, + otherUser: { + settings: { + random: "stuff", + }, + }, + global: {}, + }, + postMigration: { + global_account_accounts: { + user1: {}, + otherUser: {}, + }, + global: {}, + user1: { + settings: {}, + }, + otherUser: { + settings: { + random: "stuff", + }, + }, + }, + }, + ]; + + describe("migrate", () => { + it.each(cases)("$it", async ({ preMigration, postMigration }) => { + const actualOutput = await runMigrator(sut, preMigration, "migrate"); + expect(actualOutput).toEqual(postMigration); + }); + }); + + describe("rollback", () => { + it.each(cases)("$it", async ({ postMigration, preMigration }) => { + const actualOutput = await runMigrator(sut, postMigration, "rollback"); + expect(actualOutput).toEqual(preMigration); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/63-move-desktop-settings.ts b/libs/common/src/state-migrations/migrations/63-move-desktop-settings.ts new file mode 100644 index 000000000000..937c83faaf96 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/63-move-desktop-settings.ts @@ -0,0 +1,84 @@ +import { KeyDefinitionLike, MigrationHelper, StateDefinitionLike } from "../migration-helper"; +import { Migrator } from "../migrator"; + +type ExpectedGlobal = { + enableBrowserIntegration?: boolean; +}; + +type ExpectedAccount = { + settings?: { + minimizeOnCopyToClipboard?: boolean; + }; +}; + +const DESKTOP_SETTINGS_DISK: StateDefinitionLike = { + name: "desktopSettings", +}; + +const BROWSER_INTEGRATION_ENABLED: KeyDefinitionLike = { + key: "browserIntegrationEnabled", + stateDefinition: DESKTOP_SETTINGS_DISK, +}; + +const MINIMIZE_ON_COPY: KeyDefinitionLike = { + key: "minimizeOnCopy", + stateDefinition: DESKTOP_SETTINGS_DISK, +}; + +export class MoveDesktopSettingsMigrator extends Migrator<62, 63> { + async migrate(helper: MigrationHelper): Promise { + const legacyGlobal = await helper.get("global"); + const enabledBrowserIntegrationValue = legacyGlobal?.enableBrowserIntegration; + + if (enabledBrowserIntegrationValue != null) { + await helper.setToGlobal(BROWSER_INTEGRATION_ENABLED, enabledBrowserIntegrationValue); + delete legacyGlobal.enableBrowserIntegration; + await helper.set("global", legacyGlobal); + } + + async function migrateAccount(userId: string, account: ExpectedAccount) { + const minimizeOnCopyToClipboardValue = account?.settings?.minimizeOnCopyToClipboard; + + if (minimizeOnCopyToClipboardValue != null) { + await helper.setToUser(userId, MINIMIZE_ON_COPY, minimizeOnCopyToClipboardValue); + delete account.settings.minimizeOnCopyToClipboard; + await helper.set(userId, account); + } + } + + const accounts = await helper.getAccounts(); + + await Promise.all(accounts.map(({ userId, account }) => migrateAccount(userId, account))); + } + + async rollback(helper: MigrationHelper): Promise { + const browserIntegrationEnabledValue = await helper.getFromGlobal( + BROWSER_INTEGRATION_ENABLED, + ); + + if (browserIntegrationEnabledValue != null) { + let legacyGlobal = await helper.get("global"); + legacyGlobal ??= {}; + legacyGlobal.enableBrowserIntegration = browserIntegrationEnabledValue; + await helper.set("global", legacyGlobal); + await helper.removeFromGlobal(BROWSER_INTEGRATION_ENABLED); + } + + async function rollbackAccount(userId: string, account: ExpectedAccount) { + const minimizeOnCopyToClipboardValue = await helper.getFromUser( + userId, + MINIMIZE_ON_COPY, + ); + + if (minimizeOnCopyToClipboardValue != null) { + account ??= { settings: {} }; + account.settings.minimizeOnCopyToClipboard = minimizeOnCopyToClipboardValue; + await helper.set(userId, account); + await helper.removeFromUser(userId, MINIMIZE_ON_COPY); + } + } + + const accounts = await helper.getAccounts(); + await Promise.all(accounts.map(({ userId, account }) => rollbackAccount(userId, account))); + } +} From ab8155f04497bb2f94b873279afe45c99047608a Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Mon, 20 May 2024 14:42:32 -0400 Subject: [PATCH 3/9] Update From Main --- apps/desktop/src/app/accounts/settings.component.ts | 5 ++++- ...top-settings.spec.ts => 66-move-desktop-settings.spec.ts} | 2 +- ...-move-desktop-settings.ts => 66-move-desktop-settings.ts} | 0 3 files changed, 5 insertions(+), 2 deletions(-) rename libs/common/src/state-migrations/migrations/{63-move-desktop-settings.spec.ts => 66-move-desktop-settings.spec.ts} (98%) rename libs/common/src/state-migrations/migrations/{63-move-desktop-settings.ts => 66-move-desktop-settings.ts} (100%) diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index c7ba9ab73940..351df9d90a6a 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -600,7 +600,10 @@ export class SettingsComponent implements OnInit { } async saveMinOnCopyToClipboard() { - await this.desktopSettingsService.setMinimizeOnCopy(this.form.value.minimizeOnCopyToClipboard); + await this.desktopSettingsService.setMinimizeOnCopy( + this.form.value.minimizeOnCopyToClipboard, + this.currentUserId, + ); } async saveClearClipboard() { diff --git a/libs/common/src/state-migrations/migrations/63-move-desktop-settings.spec.ts b/libs/common/src/state-migrations/migrations/66-move-desktop-settings.spec.ts similarity index 98% rename from libs/common/src/state-migrations/migrations/63-move-desktop-settings.spec.ts rename to libs/common/src/state-migrations/migrations/66-move-desktop-settings.spec.ts index f3572b6256e6..cd33aa7a0cd2 100644 --- a/libs/common/src/state-migrations/migrations/63-move-desktop-settings.spec.ts +++ b/libs/common/src/state-migrations/migrations/66-move-desktop-settings.spec.ts @@ -1,6 +1,6 @@ import { runMigrator } from "../migration-helper.spec"; -import { MoveDesktopSettingsMigrator } from "./63-move-desktop-settings"; +import { MoveDesktopSettingsMigrator } from "./66-move-desktop-settings"; describe("MoveDesktopSettings", () => { const sut = new MoveDesktopSettingsMigrator(62, 63); diff --git a/libs/common/src/state-migrations/migrations/63-move-desktop-settings.ts b/libs/common/src/state-migrations/migrations/66-move-desktop-settings.ts similarity index 100% rename from libs/common/src/state-migrations/migrations/63-move-desktop-settings.ts rename to libs/common/src/state-migrations/migrations/66-move-desktop-settings.ts From bdb22440fbe3827786e6f667ad3117c3b0d1a15c Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Mon, 20 May 2024 16:04:42 -0400 Subject: [PATCH 4/9] Move Fingerprint Setting - No Migration Yet --- .../src/app/accounts/settings.component.ts | 7 ++--- .../services/desktop-settings.service.ts | 27 +++++++++++++++++++ .../src/services/native-messaging.service.ts | 6 ++--- .../platform/abstractions/state.service.ts | 5 ---- .../src/platform/services/state.service.ts | 21 --------------- 5 files changed, 34 insertions(+), 32 deletions(-) diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index 351df9d90a6a..496483fed3d6 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -289,8 +289,9 @@ export class SettingsComponent implements OnInit { enableBrowserIntegration: await firstValueFrom( this.desktopSettingsService.browserIntegrationEnabled$, ), - enableBrowserIntegrationFingerprint: - await this.stateService.getEnableBrowserIntegrationFingerprint(), + enableBrowserIntegrationFingerprint: await firstValueFrom( + this.desktopSettingsService.browserIntegrationFingerprintEnabled$, + ), enableDuckDuckGoBrowserIntegration: await firstValueFrom( this.desktopAutofillSettingsService.enableDuckDuckGoBrowserIntegration$, ), @@ -710,7 +711,7 @@ export class SettingsComponent implements OnInit { } async saveBrowserIntegrationFingerprint() { - await this.stateService.setEnableBrowserIntegrationFingerprint( + await this.desktopAutofillSettingsService.setEnableBrowserIntegrationFingerprint( this.form.value.enableBrowserIntegrationFingerprint, ); } diff --git a/apps/desktop/src/platform/services/desktop-settings.service.ts b/apps/desktop/src/platform/services/desktop-settings.service.ts index aade7d3d4622..33e2e5a54d44 100644 --- a/apps/desktop/src/platform/services/desktop-settings.service.ts +++ b/apps/desktop/src/platform/services/desktop-settings.service.ts @@ -58,6 +58,14 @@ const BROWSER_INTEGRATION_ENABLED = new KeyDefinition( }, ); +const BROWSER_INTEGRATION_FINGERPRINT_ENABLED = new KeyDefinition( + DESKTOP_SETTINGS_DISK, + "browserIntegrationFingerprintEnabled", + { + deserializer: (b) => b, + }, +); + const MINIMIZE_ON_COPY = new UserKeyDefinition(DESKTOP_SETTINGS_DISK, "minimizeOnCopy", { deserializer: (b) => b, clearOn: [], // User setting, no need to clear @@ -123,6 +131,16 @@ export class DesktopSettingsService { map((value) => value ?? false), ); + private readonly browserIntegrationFingerprintEnabledState = this.stateProvider.getGlobal( + BROWSER_INTEGRATION_FINGERPRINT_ENABLED, + ); + + /** + * The application setting for whether or not the fingerprint should be verified before browser communication. + */ + browserIntegrationFingerprintEnabled$ = + this.browserIntegrationFingerprintEnabledState.state$.pipe(map((value) => value ?? false)); + private readonly minimizeOnCopyState = this.stateProvider.getActive(MINIMIZE_ON_COPY); /** @@ -221,6 +239,15 @@ export class DesktopSettingsService { await this.browserIntegrationEnabledState.update(() => value); } + /** + * Sets a setting for whether or not the browser fingerprint should be verified before + * communication with the browser integration should be done. + * @param value `true` if the fingerprint should be validated before use, `false` if it should not. + */ + async setBrowserIntegrationFingerprintEnabled(value: boolean) { + await this.browserIntegrationFingerprintEnabledState.update(() => value); + } + /** * Sets the minimize on copy value for the current user. * @param value `true` if the application should minimize when a value is copied, diff --git a/apps/desktop/src/services/native-messaging.service.ts b/apps/desktop/src/services/native-messaging.service.ts index 9abd3b635ade..8dc0a57c99c6 100644 --- a/apps/desktop/src/services/native-messaging.service.ts +++ b/apps/desktop/src/services/native-messaging.service.ts @@ -10,7 +10,6 @@ import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.se import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { KeySuffixOptions } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -23,6 +22,7 @@ import { BrowserSyncVerificationDialogComponent } from "../app/components/browse import { LegacyMessage } from "../models/native-messaging/legacy-message"; import { LegacyMessageWrapper } from "../models/native-messaging/legacy-message-wrapper"; import { Message } from "../models/native-messaging/message"; +import { DesktopSettingsService } from "../platform/services/desktop-settings.service"; import { NativeMessageHandlerService } from "./native-message-handler.service"; @@ -40,7 +40,7 @@ export class NativeMessagingService { private platformUtilService: PlatformUtilsService, private logService: LogService, private messagingService: MessagingService, - private stateService: StateService, + private desktopSettingService: DesktopSettingsService, private biometricStateService: BiometricStateService, private nativeMessageHandler: NativeMessageHandlerService, private dialogService: DialogService, @@ -78,7 +78,7 @@ export class NativeMessagingService { return; } - if (await this.stateService.getEnableBrowserIntegrationFingerprint()) { + if (await firstValueFrom(this.desktopSettingService.browserIntegrationFingerprintEnabled$)) { ipc.platform.nativeMessaging.sendMessage({ command: "verifyFingerprint", appId: appId, diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 8466688bdd75..a67e58771156 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -82,11 +82,6 @@ export abstract class StateService { ) => Promise; getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise; setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise; - getEnableBrowserIntegrationFingerprint: (options?: StorageOptions) => Promise; - setEnableBrowserIntegrationFingerprint: ( - value: boolean, - options?: StorageOptions, - ) => Promise; getEncryptedPasswordGenerationHistory: ( options?: StorageOptions, ) => Promise; diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 493b5be894fb..02e49d74a4ea 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -347,27 +347,6 @@ export class StateService< : await this.secureStorageService.save(DDG_SHARED_KEY, value, options); } - async getEnableBrowserIntegrationFingerprint(options?: StorageOptions): Promise { - return ( - (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.enableBrowserIntegrationFingerprint ?? false - ); - } - - async setEnableBrowserIntegrationFingerprint( - value: boolean, - options?: StorageOptions, - ): Promise { - const globals = await this.getGlobals( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - globals.enableBrowserIntegrationFingerprint = value; - await this.saveGlobals( - globals, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async setEnableDuckDuckGoBrowserIntegration( value: boolean, options?: StorageOptions, From 9fa0d864e84f0a57e96ee396858e618ad6e3e3d9 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Wed, 22 May 2024 10:37:59 -0400 Subject: [PATCH 5/9] Add Fingerprint to Migrations --- .../src/app/accounts/settings.component.ts | 2 +- .../platform/models/domain/global-state.ts | 1 - libs/common/src/state-migrations/migrate.ts | 6 ++- ...=> 66-move-final-desktop-settings.spec.ts} | 8 +++- ...s.ts => 66-move-final-desktop-settings.ts} | 43 +++++++++++++++++-- 5 files changed, 50 insertions(+), 10 deletions(-) rename libs/common/src/state-migrations/migrations/{66-move-desktop-settings.spec.ts => 66-move-final-desktop-settings.spec.ts} (88%) rename libs/common/src/state-migrations/migrations/{66-move-desktop-settings.ts => 66-move-final-desktop-settings.ts} (65%) diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index 496483fed3d6..6b6a18958f8c 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -711,7 +711,7 @@ export class SettingsComponent implements OnInit { } async saveBrowserIntegrationFingerprint() { - await this.desktopAutofillSettingsService.setEnableBrowserIntegrationFingerprint( + await this.desktopSettingsService.setBrowserIntegrationFingerprintEnabled( this.form.value.enableBrowserIntegrationFingerprint, ); } diff --git a/libs/common/src/platform/models/domain/global-state.ts b/libs/common/src/platform/models/domain/global-state.ts index a1f494922fab..2552544cd64f 100644 --- a/libs/common/src/platform/models/domain/global-state.ts +++ b/libs/common/src/platform/models/domain/global-state.ts @@ -1,5 +1,4 @@ export class GlobalState { organizationInvitation?: any; - enableBrowserIntegrationFingerprint?: boolean; enableDuckDuckGoBrowserIntegration?: boolean; } diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index d0543fb8c3a3..3d849cfbf77f 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -63,13 +63,14 @@ import { VaultTimeoutSettingsServiceStateProviderMigrator } from "./migrations/6 import { PasswordOptionsMigrator } from "./migrations/63-migrate-password-settings"; import { GeneratorHistoryMigrator } from "./migrations/64-migrate-generator-history"; import { ForwarderOptionsMigrator } from "./migrations/65-migrate-forwarder-settings"; +import { MoveFinalDesktopSettingsMigrator } from "./migrations/66-move-final-desktop-settings"; import { MoveBiometricAutoPromptToAccount } from "./migrations/7-move-biometric-auto-prompt-to-account"; import { MoveStateVersionMigrator } from "./migrations/8-move-state-version"; import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-settings-to-global"; import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 3; -export const CURRENT_VERSION = 65; +export const CURRENT_VERSION = 66; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -136,7 +137,8 @@ export function createMigrationBuilder() { .with(VaultTimeoutSettingsServiceStateProviderMigrator, 61, 62) .with(PasswordOptionsMigrator, 62, 63) .with(GeneratorHistoryMigrator, 63, 64) - .with(ForwarderOptionsMigrator, 64, CURRENT_VERSION); + .with(ForwarderOptionsMigrator, 64, 65) + .with(MoveFinalDesktopSettingsMigrator, 65, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/66-move-desktop-settings.spec.ts b/libs/common/src/state-migrations/migrations/66-move-final-desktop-settings.spec.ts similarity index 88% rename from libs/common/src/state-migrations/migrations/66-move-desktop-settings.spec.ts rename to libs/common/src/state-migrations/migrations/66-move-final-desktop-settings.spec.ts index cd33aa7a0cd2..19be45640fb1 100644 --- a/libs/common/src/state-migrations/migrations/66-move-desktop-settings.spec.ts +++ b/libs/common/src/state-migrations/migrations/66-move-final-desktop-settings.spec.ts @@ -1,9 +1,9 @@ import { runMigrator } from "../migration-helper.spec"; -import { MoveDesktopSettingsMigrator } from "./66-move-desktop-settings"; +import { MoveFinalDesktopSettingsMigrator } from "./66-move-final-desktop-settings"; describe("MoveDesktopSettings", () => { - const sut = new MoveDesktopSettingsMigrator(62, 63); + const sut = new MoveFinalDesktopSettingsMigrator(65, 66); const cases: { it: string; @@ -29,6 +29,7 @@ describe("MoveDesktopSettings", () => { }, global: { enableBrowserIntegration: true, + enableBrowserIntegrationFingerprint: true, }, }, postMigration: { @@ -47,6 +48,7 @@ describe("MoveDesktopSettings", () => { }, global_desktopSettings_browserIntegrationEnabled: true, user_user1_desktopSettings_minimizeOnCopy: true, + global_desktopSettings_browserIntegrationFingerprintEnabled: true, }, }, { @@ -68,6 +70,7 @@ describe("MoveDesktopSettings", () => { }, global: { enableBrowserIntegration: false, + enableBrowserIntegrationFingerprint: false, }, }, postMigration: { @@ -86,6 +89,7 @@ describe("MoveDesktopSettings", () => { }, global_desktopSettings_browserIntegrationEnabled: false, user_user1_desktopSettings_minimizeOnCopy: false, + global_desktopSettings_browserIntegrationFingerprintEnabled: false, }, }, { diff --git a/libs/common/src/state-migrations/migrations/66-move-desktop-settings.ts b/libs/common/src/state-migrations/migrations/66-move-final-desktop-settings.ts similarity index 65% rename from libs/common/src/state-migrations/migrations/66-move-desktop-settings.ts rename to libs/common/src/state-migrations/migrations/66-move-final-desktop-settings.ts index 937c83faaf96..67fbc6ec00c9 100644 --- a/libs/common/src/state-migrations/migrations/66-move-desktop-settings.ts +++ b/libs/common/src/state-migrations/migrations/66-move-final-desktop-settings.ts @@ -3,6 +3,7 @@ import { Migrator } from "../migrator"; type ExpectedGlobal = { enableBrowserIntegration?: boolean; + enableBrowserIntegrationFingerprint?: boolean; }; type ExpectedAccount = { @@ -20,19 +21,41 @@ const BROWSER_INTEGRATION_ENABLED: KeyDefinitionLike = { stateDefinition: DESKTOP_SETTINGS_DISK, }; +const BROWSER_INTEGRATION_FINGERPRINT_ENABLED: KeyDefinitionLike = { + key: "browserIntegrationFingerprintEnabled", + stateDefinition: DESKTOP_SETTINGS_DISK, +}; + const MINIMIZE_ON_COPY: KeyDefinitionLike = { key: "minimizeOnCopy", stateDefinition: DESKTOP_SETTINGS_DISK, }; -export class MoveDesktopSettingsMigrator extends Migrator<62, 63> { +export class MoveFinalDesktopSettingsMigrator extends Migrator<65, 66> { async migrate(helper: MigrationHelper): Promise { const legacyGlobal = await helper.get("global"); - const enabledBrowserIntegrationValue = legacyGlobal?.enableBrowserIntegration; + const enableBrowserIntegrationValue = legacyGlobal?.enableBrowserIntegration; + const enableBrowserIntegrationFingerprintValue = + legacyGlobal?.enableBrowserIntegrationFingerprint; + + let updatedGlobal = false; - if (enabledBrowserIntegrationValue != null) { - await helper.setToGlobal(BROWSER_INTEGRATION_ENABLED, enabledBrowserIntegrationValue); + if (enableBrowserIntegrationValue != null) { + await helper.setToGlobal(BROWSER_INTEGRATION_ENABLED, enableBrowserIntegrationValue); delete legacyGlobal.enableBrowserIntegration; + updatedGlobal = true; + } + + if (enableBrowserIntegrationValue != null) { + await helper.setToGlobal( + BROWSER_INTEGRATION_FINGERPRINT_ENABLED, + enableBrowserIntegrationFingerprintValue, + ); + delete legacyGlobal.enableBrowserIntegrationFingerprint; + updatedGlobal = true; + } + + if (updatedGlobal) { await helper.set("global", legacyGlobal); } @@ -56,6 +79,10 @@ export class MoveDesktopSettingsMigrator extends Migrator<62, 63> { BROWSER_INTEGRATION_ENABLED, ); + const browserIntegrationFingerprintEnabled = await helper.getFromGlobal( + BROWSER_INTEGRATION_FINGERPRINT_ENABLED, + ); + if (browserIntegrationEnabledValue != null) { let legacyGlobal = await helper.get("global"); legacyGlobal ??= {}; @@ -64,6 +91,14 @@ export class MoveDesktopSettingsMigrator extends Migrator<62, 63> { await helper.removeFromGlobal(BROWSER_INTEGRATION_ENABLED); } + if (browserIntegrationFingerprintEnabled != null) { + let legacyGlobal = await helper.get("global"); + legacyGlobal ??= {}; + legacyGlobal.enableBrowserIntegrationFingerprint = browserIntegrationFingerprintEnabled; + await helper.set("global", legacyGlobal); + await helper.removeFromGlobal(BROWSER_INTEGRATION_FINGERPRINT_ENABLED); + } + async function rollbackAccount(userId: string, account: ExpectedAccount) { const minimizeOnCopyToClipboardValue = await helper.getFromUser( userId, From eff15620fae3fac2ab9bfc712204d1066c5f1b0a Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Tue, 28 May 2024 15:45:08 -0400 Subject: [PATCH 6/9] Convert Messaging to `async` --- apps/desktop/src/main/messaging.main.ts | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/apps/desktop/src/main/messaging.main.ts b/apps/desktop/src/main/messaging.main.ts index 0f2a3c25c3c1..4e9c82e77783 100644 --- a/apps/desktop/src/main/messaging.main.ts +++ b/apps/desktop/src/main/messaging.main.ts @@ -30,10 +30,13 @@ export class MessagingMain { const loginSettings = app.getLoginItemSettings(); await this.desktopSettingsService.setOpenAtLogin(loginSettings.openAtLogin); } - ipcMain.on("messagingService", async (event: any, message: any) => this.onMessage(message)); + ipcMain.on( + "messagingService", + async (event: any, message: any) => await this.onMessage(message), + ); } - onMessage(message: any) { + async onMessage(message: any) { switch (message.command) { case "scheduleNextSync": this.scheduleNextSync(); @@ -45,16 +48,14 @@ export class MessagingMain { this.updateTrayMenu(message.updateRequest); break; case "minimizeOnCopy": - firstValueFrom(this.desktopSettingsService.minimizeOnCopy$).then( - (shouldMinimize) => { - if (shouldMinimize && this.main.windowMain.win !== null) { - this.main.windowMain.win.minimize(); - } - }, - (error) => { - this.logService.error("Error while retrieving minimize on copy setting", error); - }, - ); + { + const shouldMinimizeOnCopy = await firstValueFrom( + this.desktopSettingsService.minimizeOnCopy$, + ); + if (shouldMinimizeOnCopy && this.main.windowMain.win !== null) { + this.main.windowMain.win.minimize(); + } + } break; case "showTray": this.main.trayMain.showTray(); From 1cd4515e762417f1ee31b22d8bd50d70b7b327dd Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Tue, 28 May 2024 15:58:10 -0400 Subject: [PATCH 7/9] Switch to calling `Boolean` for Map Function --- .../services/desktop-settings.service.ts | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/apps/desktop/src/platform/services/desktop-settings.service.ts b/apps/desktop/src/platform/services/desktop-settings.service.ts index 33e2e5a54d44..ff29ce50a0f3 100644 --- a/apps/desktop/src/platform/services/desktop-settings.service.ts +++ b/apps/desktop/src/platform/services/desktop-settings.service.ts @@ -84,41 +84,41 @@ export class DesktopSettingsService { /** * Tha applications setting for whether or not to close the application into the system tray. */ - closeToTray$ = this.closeToTrayState.state$.pipe(map((value) => value ?? false)); + closeToTray$ = this.closeToTrayState.state$.pipe(map(Boolean)); private readonly minimizeToTrayState = this.stateProvider.getGlobal(MINIMIZE_TO_TRAY_KEY); /** * The application setting for whether or not to minimize the applicaiton into the system tray. */ - minimizeToTray$ = this.minimizeToTrayState.state$.pipe(map((value) => value ?? false)); + minimizeToTray$ = this.minimizeToTrayState.state$.pipe(map(Boolean)); private readonly startToTrayState = this.stateProvider.getGlobal(START_TO_TRAY_KEY); /** * The application setting for whether or not to start the application into the system tray. */ - startToTray$ = this.startToTrayState.state$.pipe(map((value) => value ?? false)); + startToTray$ = this.startToTrayState.state$.pipe(map(Boolean)); private readonly trayEnabledState = this.stateProvider.getGlobal(TRAY_ENABLED_KEY); /** * Whether or not the system tray has been enabled. */ - trayEnabled$ = this.trayEnabledState.state$.pipe(map((value) => value ?? false)); + trayEnabled$ = this.trayEnabledState.state$.pipe(map(Boolean)); private readonly openAtLoginState = this.stateProvider.getGlobal(OPEN_AT_LOGIN_KEY); /** * The application setting for whether or not the application should open at system login. */ - openAtLogin$ = this.openAtLoginState.state$.pipe(map((value) => value ?? false)); + openAtLogin$ = this.openAtLoginState.state$.pipe(map(Boolean)); private readonly alwaysShowDockState = this.stateProvider.getGlobal(ALWAYS_SHOW_DOCK_KEY); /** * The application setting for whether or not the application should show up in the dock. */ - alwaysShowDock$ = this.alwaysShowDockState.state$.pipe(map((value) => value ?? false)); + alwaysShowDock$ = this.alwaysShowDockState.state$.pipe(map(Boolean)); private readonly alwaysOnTopState = this.stateProvider.getGlobal(ALWAYS_ON_TOP_KEY); - alwaysOnTop$ = this.alwaysOnTopState.state$.pipe(map((value) => value ?? false)); + alwaysOnTop$ = this.alwaysOnTopState.state$.pipe(map(Boolean)); private readonly browserIntegrationEnabledState = this.stateProvider.getGlobal( BROWSER_INTEGRATION_ENABLED, @@ -127,9 +127,7 @@ export class DesktopSettingsService { /** * The application setting for whether or not the browser integration is enabled. */ - browserIntegrationEnabled$ = this.browserIntegrationEnabledState.state$.pipe( - map((value) => value ?? false), - ); + browserIntegrationEnabled$ = this.browserIntegrationEnabledState.state$.pipe(map(Boolean)); private readonly browserIntegrationFingerprintEnabledState = this.stateProvider.getGlobal( BROWSER_INTEGRATION_FINGERPRINT_ENABLED, @@ -139,7 +137,7 @@ export class DesktopSettingsService { * The application setting for whether or not the fingerprint should be verified before browser communication. */ browserIntegrationFingerprintEnabled$ = - this.browserIntegrationFingerprintEnabledState.state$.pipe(map((value) => value ?? false)); + this.browserIntegrationFingerprintEnabledState.state$.pipe(map(Boolean)); private readonly minimizeOnCopyState = this.stateProvider.getActive(MINIMIZE_ON_COPY); @@ -147,7 +145,7 @@ export class DesktopSettingsService { * The active users setting for whether or not the application should minimize itself * when a value is copied to the clipboard. */ - minimizeOnCopy$ = this.minimizeOnCopyState.state$.pipe(map((value) => value ?? false)); + minimizeOnCopy$ = this.minimizeOnCopyState.state$.pipe(map(Boolean)); constructor(private stateProvider: StateProvider) { this.window$ = this.windowState.state$.pipe( From 8af02a8684a85901c9df4177e6e2d16f83462f1a Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Tue, 28 May 2024 17:04:20 -0400 Subject: [PATCH 8/9] Catch Errors --- apps/desktop/src/main.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index f5799784f687..7724058fc5c6 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -171,7 +171,13 @@ export class Main { ); messageSubject.asObservable().subscribe((message) => { - this.messagingMain.onMessage(message); + void this.messagingMain.onMessage(message).catch((err) => { + this.logService.error( + "Error while handling message", + message?.command ?? "Unknown command", + err, + ); + }); }); this.powerMonitorMain = new PowerMonitorMain(this.messagingService); From 38c68847347a7ec43dbfc25760d4663224e2a8e6 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Wed, 29 May 2024 08:04:44 -0400 Subject: [PATCH 9/9] Remove LogService --- apps/desktop/src/main.ts | 2 +- apps/desktop/src/main/messaging.main.ts | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 7724058fc5c6..a8ed5dbe8d45 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -160,7 +160,7 @@ export class Main { (arg) => this.processDeepLink(arg), (win) => this.trayMain.setupWindowListeners(win), ); - this.messagingMain = new MessagingMain(this, this.desktopSettingsService, this.logService); + this.messagingMain = new MessagingMain(this, this.desktopSettingsService); this.updaterMain = new UpdaterMain(this.i18nService, this.windowMain); this.trayMain = new TrayMain(this.windowMain, this.i18nService, this.desktopSettingsService); diff --git a/apps/desktop/src/main/messaging.main.ts b/apps/desktop/src/main/messaging.main.ts index 4e9c82e77783..68b1597ac452 100644 --- a/apps/desktop/src/main/messaging.main.ts +++ b/apps/desktop/src/main/messaging.main.ts @@ -4,8 +4,6 @@ import * as path from "path"; import { app, ipcMain } from "electron"; import { firstValueFrom } from "rxjs"; -import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; - import { Main } from "../main"; import { DesktopSettingsService } from "../platform/services/desktop-settings.service"; @@ -19,7 +17,6 @@ export class MessagingMain { constructor( private main: Main, private desktopSettingsService: DesktopSettingsService, - private readonly logService: LogService, ) {} async init() {