From 51f46e797c3787046bac9fad13bb28d3a5e83175 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Fri, 22 Mar 2024 18:32:03 +0100 Subject: [PATCH] [PM-5571] Migrate enableDDG to state provider framework (#8384) Migrate enableDuckDuckGo to state provider framework. --- .github/CODEOWNERS | 1 + .../src/app/accounts/settings.component.ts | 9 ++-- .../src/app/services/services.module.ts | 6 +++ .../desktop-autofill-settings.service.ts | 29 ++++++++++++ apps/desktop/src/main.ts | 10 ++-- apps/desktop/src/main/messaging.main.ts | 4 -- .../desktop/src/main/native-messaging.main.ts | 2 +- .../native-message-handler.service.ts | 6 ++- .../platform/abstractions/state.service.ts | 5 -- .../platform/models/domain/global-state.ts | 1 - .../src/platform/services/state.service.ts | 21 --------- libs/common/src/state-migrations/migrate.ts | 7 ++- .../48-move-ddg-to-state-provider.spec.ts | 47 +++++++++++++++++++ .../48-move-ddg-to-state-provider.ts | 40 ++++++++++++++++ 14 files changed, 147 insertions(+), 41 deletions(-) create mode 100644 apps/desktop/src/autofill/services/desktop-autofill-settings.service.ts create mode 100644 libs/common/src/state-migrations/migrations/48-move-ddg-to-state-provider.spec.ts create mode 100644 libs/common/src/state-migrations/migrations/48-move-ddg-to-state-provider.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index bfce3c1547..bfad3f2628 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -83,6 +83,7 @@ apps/web/src/translation-constants.ts @bitwarden/team-platform-dev ## Autofill team files ## apps/browser/src/autofill @bitwarden/team-autofill-dev +apps/desktop/src/autofill @bitwarden/team-autofill-dev libs/common/src/autofill @bitwarden/team-autofill-dev ## Component Library ## diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index 4e6af9bff4..60aa2ebae8 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -23,6 +23,7 @@ import { ThemeStateService } from "@bitwarden/common/platform/theming/theme-stat import { DialogService } from "@bitwarden/components"; import { SetPinComponent } from "../../auth/components/set-pin.component"; +import { DesktopAutofillSettingsService } from "../../autofill/services/desktop-autofill-settings.service"; import { flagEnabled } from "../../platform/flags"; import { DesktopSettingsService } from "../../platform/services/desktop-settings.service"; @@ -122,6 +123,7 @@ export class SettingsComponent implements OnInit { private userVerificationService: UserVerificationServiceAbstraction, private desktopSettingsService: DesktopSettingsService, private biometricStateService: BiometricStateService, + private desktopAutofillSettingsService: DesktopAutofillSettingsService, ) { const isMac = this.platformUtilsService.getDevice() === DeviceType.MacOsDesktop; @@ -262,11 +264,12 @@ export class SettingsComponent implements OnInit { enableBrowserIntegration: await this.stateService.getEnableBrowserIntegration(), enableBrowserIntegrationFingerprint: await this.stateService.getEnableBrowserIntegrationFingerprint(), + enableDuckDuckGoBrowserIntegration: await firstValueFrom( + this.desktopAutofillSettingsService.enableDuckDuckGoBrowserIntegration$, + ), enableHardwareAcceleration: await firstValueFrom( this.desktopSettingsService.hardwareAcceleration$, ), - enableDuckDuckGoBrowserIntegration: - await this.stateService.getEnableDuckDuckGoBrowserIntegration(), theme: await firstValueFrom(this.themeStateService.selectedTheme$), locale: await firstValueFrom(this.i18nService.userSetLocale$), }; @@ -640,7 +643,7 @@ export class SettingsComponent implements OnInit { } async saveDdgBrowserIntegration() { - await this.stateService.setEnableDuckDuckGoBrowserIntegration( + await this.desktopAutofillSettingsService.setEnableDuckDuckGoBrowserIntegration( this.form.value.enableDuckDuckGoBrowserIntegration, ); diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index bed8f21bbe..495d6abcf1 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -53,6 +53,7 @@ import { CipherService as CipherServiceAbstraction } from "@bitwarden/common/vau import { DialogService } from "@bitwarden/components"; import { LoginGuard } from "../../auth/guards/login.guard"; +import { DesktopAutofillSettingsService } from "../../autofill/services/desktop-autofill-settings.service"; import { Account } from "../../models/account"; import { DesktopSettingsService } from "../../platform/services/desktop-settings.service"; import { ElectronCryptoService } from "../../platform/services/electron-crypto.service"; @@ -218,6 +219,11 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK"); useClass: DesktopSettingsService, deps: [StateProvider], }, + { + provide: DesktopAutofillSettingsService, + useClass: DesktopAutofillSettingsService, + deps: [StateProvider], + }, ], }) export class ServicesModule {} diff --git a/apps/desktop/src/autofill/services/desktop-autofill-settings.service.ts b/apps/desktop/src/autofill/services/desktop-autofill-settings.service.ts new file mode 100644 index 0000000000..d60a08a9fe --- /dev/null +++ b/apps/desktop/src/autofill/services/desktop-autofill-settings.service.ts @@ -0,0 +1,29 @@ +import { map } from "rxjs"; + +import { + AUTOFILL_SETTINGS_DISK, + KeyDefinition, + StateProvider, +} from "@bitwarden/common/platform/state"; + +const ENABLE_DUCK_DUCK_GO_BROWSER_INTEGRATION = new KeyDefinition( + AUTOFILL_SETTINGS_DISK, + "enableDuckDuckGoBrowserIntegration", + { + deserializer: (v: boolean) => v, + }, +); + +export class DesktopAutofillSettingsService { + private enableDuckDuckGoBrowserIntegrationState = this.stateProvider.getGlobal( + ENABLE_DUCK_DUCK_GO_BROWSER_INTEGRATION, + ); + readonly enableDuckDuckGoBrowserIntegration$ = + this.enableDuckDuckGoBrowserIntegrationState.state$.pipe(map((x) => x ?? false)); + + constructor(private stateProvider: StateProvider) {} + + async setEnableDuckDuckGoBrowserIntegration(newValue: boolean): Promise { + await this.enableDuckDuckGoBrowserIntegrationState.update(() => newValue); + } +} diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index bae1cc0c06..5cb6abac58 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -26,6 +26,7 @@ import { StateEventRegistrarService } from "@bitwarden/common/platform/state/sta import { MemoryStorageService as MemoryStorageServiceForStateProviders } from "@bitwarden/common/platform/state/storage/memory-storage.service"; /* eslint-enable import/no-restricted-paths */ +import { DesktopAutofillSettingsService } from "./autofill/services/desktop-autofill-settings.service"; import { MenuMain } from "./main/menu/menu.main"; import { MessagingMain } from "./main/messaging.main"; import { NativeMessagingMain } from "./main/native-messaging.main"; @@ -71,6 +72,7 @@ export class Main { biometricsService: BiometricsServiceAbstraction; nativeMessagingMain: NativeMessagingMain; clipboardMain: ClipboardMain; + desktopAutofillSettingsService: DesktopAutofillSettingsService; constructor() { // Set paths for portable builds @@ -233,6 +235,8 @@ export class Main { app.getPath("exe"), ); + this.desktopAutofillSettingsService = new DesktopAutofillSettingsService(stateProvider); + this.clipboardMain = new ClipboardMain(); this.clipboardMain.init(); @@ -268,10 +272,10 @@ export class Main { if ( (await this.stateService.getEnableBrowserIntegration()) || - (await this.stateService.getEnableDuckDuckGoBrowserIntegration()) + (await firstValueFrom( + this.desktopAutofillSettingsService.enableDuckDuckGoBrowserIntegration$, + )) ) { - // 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.nativeMessagingMain.listen(); } diff --git a/apps/desktop/src/main/messaging.main.ts b/apps/desktop/src/main/messaging.main.ts index cc67e312b5..256d551560 100644 --- a/apps/desktop/src/main/messaging.main.ts +++ b/apps/desktop/src/main/messaging.main.ts @@ -77,14 +77,10 @@ export class MessagingMain { break; case "enableBrowserIntegration": this.main.nativeMessagingMain.generateManifests(); - // 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.main.nativeMessagingMain.listen(); break; case "enableDuckDuckGoBrowserIntegration": this.main.nativeMessagingMain.generateDdgManifests(); - // 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.main.nativeMessagingMain.listen(); break; case "disableBrowserIntegration": diff --git a/apps/desktop/src/main/native-messaging.main.ts b/apps/desktop/src/main/native-messaging.main.ts index 77a0b31314..05e987e20b 100644 --- a/apps/desktop/src/main/native-messaging.main.ts +++ b/apps/desktop/src/main/native-messaging.main.ts @@ -24,7 +24,7 @@ export class NativeMessagingMain { private exePath: string, ) {} - async listen() { + listen() { ipc.config.id = "bitwarden"; ipc.config.retry = 1500; const ipcSocketRoot = getIpcSocketRoot(); diff --git a/apps/desktop/src/services/native-message-handler.service.ts b/apps/desktop/src/services/native-message-handler.service.ts index c90271d25c..785b65195a 100644 --- a/apps/desktop/src/services/native-message-handler.service.ts +++ b/apps/desktop/src/services/native-message-handler.service.ts @@ -12,6 +12,7 @@ import { StateService } from "@bitwarden/common/platform/services/state.service" import { DialogService } from "@bitwarden/components"; import { VerifyNativeMessagingDialogComponent } from "../app/components/verify-native-messaging-dialog.component"; +import { DesktopAutofillSettingsService } from "../autofill/services/desktop-autofill-settings.service"; import { DecryptedCommandData } from "../models/native-messaging/decrypted-command-data"; import { EncryptedMessage } from "../models/native-messaging/encrypted-message"; import { EncryptedMessageResponse } from "../models/native-messaging/encrypted-message-response"; @@ -34,6 +35,7 @@ export class NativeMessageHandlerService { private messagingService: MessagingService, private encryptedMessageHandlerService: EncryptedMessageHandlerService, private dialogService: DialogService, + private desktopAutofillSettingsService: DesktopAutofillSettingsService, ) {} async handleMessage(message: Message) { @@ -71,7 +73,9 @@ export class NativeMessageHandlerService { try { const remotePublicKey = Utils.fromB64ToArray(publicKey); - const ddgEnabled = await this.stateService.getEnableDuckDuckGoBrowserIntegration(); + const ddgEnabled = await firstValueFrom( + this.desktopAutofillSettingsService.enableDuckDuckGoBrowserIntegration$, + ); if (!ddgEnabled) { this.sendResponse({ diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index b795db73fc..514689313f 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -188,11 +188,6 @@ export abstract class StateService { value: boolean, options?: StorageOptions, ) => Promise; - getEnableDuckDuckGoBrowserIntegration: (options?: StorageOptions) => Promise; - setEnableDuckDuckGoBrowserIntegration: ( - value: boolean, - options?: StorageOptions, - ) => Promise; getEncryptedCiphers: (options?: StorageOptions) => Promise<{ [id: string]: CipherData }>; setEncryptedCiphers: ( value: { [id: string]: CipherData }, diff --git a/libs/common/src/platform/models/domain/global-state.ts b/libs/common/src/platform/models/domain/global-state.ts index 3fd6f38200..7e35606e26 100644 --- a/libs/common/src/platform/models/domain/global-state.ts +++ b/libs/common/src/platform/models/domain/global-state.ts @@ -13,6 +13,5 @@ export class GlobalState { mainWindowSize?: number; enableBrowserIntegration?: boolean; enableBrowserIntegrationFingerprint?: boolean; - enableDuckDuckGoBrowserIntegration?: boolean; deepLinkRedirectUrl?: string; } diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 6ff1c63b50..56fb91dd52 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -867,27 +867,6 @@ export class StateService< ); } - async getEnableDuckDuckGoBrowserIntegration(options?: StorageOptions): Promise { - return ( - (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.enableDuckDuckGoBrowserIntegration ?? false - ); - } - - async setEnableDuckDuckGoBrowserIntegration( - value: boolean, - options?: StorageOptions, - ): Promise { - const globals = await this.getGlobals( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - globals.enableDuckDuckGoBrowserIntegration = value; - await this.saveGlobals( - globals, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - @withPrototypeForObjectValues(CipherData) async getEncryptedCiphers(options?: StorageOptions): Promise<{ [id: string]: CipherData }> { return ( diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 3c03854780..1b057fda4d 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -43,6 +43,7 @@ import { UserDecryptionOptionsMigrator } from "./migrations/44-move-user-decrypt import { MergeEnvironmentState } from "./migrations/45-merge-environment-state"; import { DeleteBiometricPromptCancelledData } from "./migrations/46-delete-orphaned-biometric-prompt-data"; import { MoveDesktopSettingsMigrator } from "./migrations/47-move-desktop-settings"; +import { MoveDdgToStateProviderMigrator } from "./migrations/48-move-ddg-to-state-provider"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; import { MoveBiometricAutoPromptToAccount } from "./migrations/7-move-biometric-auto-prompt-to-account"; @@ -51,7 +52,8 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 3; -export const CURRENT_VERSION = 47; +export const CURRENT_VERSION = 48; + export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -100,7 +102,8 @@ export function createMigrationBuilder() { .with(UserDecryptionOptionsMigrator, 43, 44) .with(MergeEnvironmentState, 44, 45) .with(DeleteBiometricPromptCancelledData, 45, 46) - .with(MoveDesktopSettingsMigrator, 46, CURRENT_VERSION); + .with(MoveDesktopSettingsMigrator, 46, 47) + .with(MoveDdgToStateProviderMigrator, 47, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/48-move-ddg-to-state-provider.spec.ts b/libs/common/src/state-migrations/migrations/48-move-ddg-to-state-provider.spec.ts new file mode 100644 index 0000000000..6b6ebffb58 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/48-move-ddg-to-state-provider.spec.ts @@ -0,0 +1,47 @@ +import { runMigrator } from "../migration-helper.spec"; + +import { MoveDdgToStateProviderMigrator } from "./48-move-ddg-to-state-provider"; + +describe("MoveDdgToStateProviderMigrator", () => { + const migrator = new MoveDdgToStateProviderMigrator(47, 48); + + it("migrate", async () => { + const output = await runMigrator(migrator, { + global: { + enableDuckDuckGoBrowserIntegration: true, + otherStuff: "otherStuff1", + }, + otherStuff: "otherStuff2", + }); + + expect(output).toEqual({ + global_autofillSettings_enableDuckDuckGoBrowserIntegration: true, + global: { + otherStuff: "otherStuff1", + }, + otherStuff: "otherStuff2", + }); + }); + + it("rollback", async () => { + const output = await runMigrator( + migrator, + { + global_autofillSettings_enableDuckDuckGoBrowserIntegration: true, + global: { + otherStuff: "otherStuff1", + }, + otherStuff: "otherStuff2", + }, + "rollback", + ); + + expect(output).toEqual({ + global: { + enableDuckDuckGoBrowserIntegration: true, + otherStuff: "otherStuff1", + }, + otherStuff: "otherStuff2", + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/48-move-ddg-to-state-provider.ts b/libs/common/src/state-migrations/migrations/48-move-ddg-to-state-provider.ts new file mode 100644 index 0000000000..51676c1d7b --- /dev/null +++ b/libs/common/src/state-migrations/migrations/48-move-ddg-to-state-provider.ts @@ -0,0 +1,40 @@ +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +type ExpectedGlobal = { + enableDuckDuckGoBrowserIntegration?: boolean; +}; + +export const DDG_KEY: KeyDefinitionLike = { + key: "enableDuckDuckGoBrowserIntegration", + stateDefinition: { + name: "autofillSettings", + }, +}; + +export class MoveDdgToStateProviderMigrator extends Migrator<47, 48> { + async migrate(helper: MigrationHelper): Promise { + // global state + const global = await helper.get("global"); + if (global?.enableDuckDuckGoBrowserIntegration == null) { + return; + } + + await helper.setToGlobal(DDG_KEY, global.enableDuckDuckGoBrowserIntegration); + delete global.enableDuckDuckGoBrowserIntegration; + await helper.set("global", global); + } + + async rollback(helper: MigrationHelper): Promise { + const enableDdg = await helper.getFromGlobal(DDG_KEY); + + if (!enableDdg) { + return; + } + + const global = (await helper.get("global")) ?? {}; + global.enableDuckDuckGoBrowserIntegration = enableDdg; + await helper.set("global", global); + await helper.removeFromGlobal(DDG_KEY); + } +}