diff --git a/apps/browser/src/autofill/browser/main-context-menu-handler.spec.ts b/apps/browser/src/autofill/browser/main-context-menu-handler.spec.ts index f2b17d2f08..b827788d75 100644 --- a/apps/browser/src/autofill/browser/main-context-menu-handler.spec.ts +++ b/apps/browser/src/autofill/browser/main-context-menu-handler.spec.ts @@ -1,6 +1,8 @@ import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; import { NOOP_COMMAND_SUFFIX } from "@bitwarden/common/autofill/constants"; +import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { CipherType } from "@bitwarden/common/vault/enums"; @@ -13,6 +15,7 @@ import { MainContextMenuHandler } from "./main-context-menu-handler"; describe("context-menu", () => { let stateService: MockProxy; + let autofillSettingsService: MockProxy; let i18nService: MockProxy; let logService: MockProxy; @@ -26,6 +29,7 @@ describe("context-menu", () => { beforeEach(() => { stateService = mock(); + autofillSettingsService = mock(); i18nService = mock(); logService = mock(); @@ -41,14 +45,20 @@ describe("context-menu", () => { }); i18nService.t.mockImplementation((key) => key); - sut = new MainContextMenuHandler(stateService, i18nService, logService); + sut = new MainContextMenuHandler( + stateService, + autofillSettingsService, + i18nService, + logService, + ); + autofillSettingsService.enableContextMenu$ = of(true); }); afterEach(() => jest.resetAllMocks()); describe("init", () => { it("has menu disabled", async () => { - stateService.getDisableContextMenuItem.mockResolvedValue(true); + autofillSettingsService.enableContextMenu$ = of(false); const createdMenu = await sut.init(); expect(createdMenu).toBeFalsy(); @@ -56,8 +66,6 @@ describe("context-menu", () => { }); it("has menu enabled, but does not have premium", async () => { - stateService.getDisableContextMenuItem.mockResolvedValue(false); - stateService.getCanAccessPremium.mockResolvedValue(false); const createdMenu = await sut.init(); @@ -66,8 +74,6 @@ describe("context-menu", () => { }); it("has menu enabled and has premium", async () => { - stateService.getDisableContextMenuItem.mockResolvedValue(false); - stateService.getCanAccessPremium.mockResolvedValue(true); const createdMenu = await sut.init(); diff --git a/apps/browser/src/autofill/browser/main-context-menu-handler.ts b/apps/browser/src/autofill/browser/main-context-menu-handler.ts index 19154fbfb8..998b5c7258 100644 --- a/apps/browser/src/autofill/browser/main-context-menu-handler.ts +++ b/apps/browser/src/autofill/browser/main-context-menu-handler.ts @@ -1,3 +1,5 @@ +import { firstValueFrom } from "rxjs"; + import { AUTOFILL_CARD_ID, AUTOFILL_ID, @@ -14,6 +16,7 @@ import { ROOT_ID, SEPARATOR_ID, } from "@bitwarden/common/autofill/constants"; +import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; @@ -22,6 +25,7 @@ import { GlobalState } from "@bitwarden/common/platform/models/domain/global-sta import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { autofillSettingsServiceFactory } from "../../autofill/background/service_factories/autofill-settings-service.factory"; import { Account } from "../../models/account"; import { CachedServices } from "../../platform/background/service-factories/factory-options"; import { @@ -156,6 +160,7 @@ export class MainContextMenuHandler { constructor( private stateService: BrowserStateService, + private autofillSettingsService: AutofillSettingsServiceAbstraction, private i18nService: I18nService, private logService: LogService, ) {} @@ -183,6 +188,7 @@ export class MainContextMenuHandler { return new MainContextMenuHandler( await stateServiceFactory(cachedServices, serviceOptions), + await autofillSettingsServiceFactory(cachedServices, serviceOptions), await i18nServiceFactory(cachedServices, serviceOptions), await logServiceFactory(cachedServices, serviceOptions), ); @@ -193,8 +199,8 @@ export class MainContextMenuHandler { * @returns a boolean showing whether or not items were created */ async init(): Promise { - const menuDisabled = await this.stateService.getDisableContextMenuItem(); - if (menuDisabled) { + const menuEnabled = await firstValueFrom(this.autofillSettingsService.enableContextMenu$); + if (!menuEnabled) { await MainContextMenuHandler.removeAll(); return false; } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index ea51fa8805..9028a7cfff 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -950,6 +950,7 @@ export default class MainBackground { if (!this.popupOnlyContext) { this.mainContextMenuHandler = new MainContextMenuHandler( this.stateService, + this.autofillSettingsService, this.i18nService, this.logService, ); diff --git a/apps/browser/src/popup/settings/options.component.ts b/apps/browser/src/popup/settings/options.component.ts index ac416a3493..2af3713171 100644 --- a/apps/browser/src/popup/settings/options.component.ts +++ b/apps/browser/src/popup/settings/options.component.ts @@ -105,7 +105,9 @@ export class OptionsComponent implements OnInit { this.userNotificationSettingsService.enableChangedPasswordPrompt$, ); - this.enableContextMenuItem = !(await this.stateService.getDisableContextMenuItem()); + this.enableContextMenuItem = await firstValueFrom( + this.autofillSettingsService.enableContextMenu$, + ); this.showCardsCurrentTab = !(await this.stateService.getDontShowCardsCurrentTab()); this.showIdentitiesCurrentTab = !(await this.stateService.getDontShowIdentitiesCurrentTab()); @@ -143,7 +145,7 @@ export class OptionsComponent implements OnInit { } async updateContextMenuItem() { - await this.stateService.setDisableContextMenuItem(!this.enableContextMenuItem); + await this.autofillSettingsService.setEnableContextMenu(this.enableContextMenuItem); this.messagingService.send("bgUpdateContextMenu"); } diff --git a/libs/common/src/autofill/services/autofill-settings.service.ts b/libs/common/src/autofill/services/autofill-settings.service.ts index 7a06eed6fa..41452c536c 100644 --- a/libs/common/src/autofill/services/autofill-settings.service.ts +++ b/libs/common/src/autofill/services/autofill-settings.service.ts @@ -53,6 +53,10 @@ const INLINE_MENU_VISIBILITY = new KeyDefinition( }, ); +const ENABLE_CONTEXT_MENU = new KeyDefinition(AUTOFILL_SETTINGS_DISK, "enableContextMenu", { + deserializer: (value: boolean) => value ?? true, +}); + const CLEAR_CLIPBOARD_DELAY = new KeyDefinition( AUTOFILL_SETTINGS_DISK_LOCAL, "clearClipboardDelay", @@ -75,6 +79,8 @@ export abstract class AutofillSettingsServiceAbstraction { setAutoCopyTotp: (newValue: boolean) => Promise; inlineMenuVisibility$: Observable; setInlineMenuVisibility: (newValue: InlineMenuVisibilitySetting) => Promise; + enableContextMenu$: Observable; + setEnableContextMenu: (newValue: boolean) => Promise; clearClipboardDelay$: Observable; setClearClipboardDelay: (newValue: ClearClipboardDelaySetting) => Promise; } @@ -100,6 +106,9 @@ export class AutofillSettingsService implements AutofillSettingsServiceAbstracti private inlineMenuVisibilityState: GlobalState; readonly inlineMenuVisibility$: Observable; + private enableContextMenuState: GlobalState; + readonly enableContextMenu$: Observable; + private clearClipboardDelayState: ActiveUserState; readonly clearClipboardDelay$: Observable; @@ -142,6 +151,9 @@ export class AutofillSettingsService implements AutofillSettingsServiceAbstracti map((x) => x ?? AutofillOverlayVisibility.Off), ); + this.enableContextMenuState = this.stateProvider.getGlobal(ENABLE_CONTEXT_MENU); + this.enableContextMenu$ = this.enableContextMenuState.state$.pipe(map((x) => x ?? true)); + this.clearClipboardDelayState = this.stateProvider.getActive(CLEAR_CLIPBOARD_DELAY); this.clearClipboardDelay$ = this.clearClipboardDelayState.state$.pipe( map((x) => x ?? ClearClipboardDelay.Never), @@ -172,6 +184,10 @@ export class AutofillSettingsService implements AutofillSettingsServiceAbstracti await this.inlineMenuVisibilityState.update(() => newValue); } + async setEnableContextMenu(newValue: boolean): Promise { + await this.enableContextMenuState.update(() => newValue); + } + async setClearClipboardDelay(newValue: ClearClipboardDelaySetting): Promise { await this.clearClipboardDelayState.update(() => newValue); } diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 4061769461..2548913555 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -189,8 +189,6 @@ export abstract class StateService { setDecryptedSends: (value: SendView[], options?: StorageOptions) => Promise; getDefaultUriMatch: (options?: StorageOptions) => Promise; setDefaultUriMatch: (value: UriMatchType, options?: StorageOptions) => Promise; - getDisableContextMenuItem: (options?: StorageOptions) => Promise; - setDisableContextMenuItem: (value: boolean, options?: StorageOptions) => Promise; /** * @deprecated Do not call this, use SettingsService */ diff --git a/libs/common/src/platform/models/domain/global-state.ts b/libs/common/src/platform/models/domain/global-state.ts index d7c1d93d03..b27482fa6a 100644 --- a/libs/common/src/platform/models/domain/global-state.ts +++ b/libs/common/src/platform/models/domain/global-state.ts @@ -26,6 +26,5 @@ export class GlobalState { enableBrowserIntegrationFingerprint?: boolean; enableDuckDuckGoBrowserIntegration?: boolean; neverDomains?: { [id: string]: unknown }; - disableContextMenuItem?: boolean; deepLinkRedirectUrl?: string; } diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 357bd27173..9313af551b 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -832,24 +832,6 @@ export class StateService< ); } - async getDisableContextMenuItem(options?: StorageOptions): Promise { - return ( - (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.disableContextMenuItem ?? false - ); - } - - async setDisableContextMenuItem(value: boolean, options?: StorageOptions): Promise { - const globals = await this.getGlobals( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - globals.disableContextMenuItem = value; - await this.saveGlobals( - globals, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getDisableFavicon(options?: StorageOptions): Promise { return ( ( diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index db29ddf26f..b6bcc45eed 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -26,6 +26,7 @@ import { MoveBiometricUnlockToStateProviders } from "./migrations/28-move-biomet import { UserNotificationSettingsKeyMigrator } from "./migrations/29-move-user-notification-settings-to-state-provider"; import { FixPremiumMigrator } from "./migrations/3-fix-premium"; import { PolicyMigrator } from "./migrations/30-move-policy-state-to-state-provider"; +import { EnableContextMenuMigrator } from "./migrations/31-move-enable-context-menu-to-autofill-settings-state-provider"; import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; @@ -35,7 +36,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 2; -export const CURRENT_VERSION = 30; +export const CURRENT_VERSION = 31; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -68,7 +69,8 @@ export function createMigrationBuilder() { .with(BadgeSettingsMigrator, 26, 27) .with(MoveBiometricUnlockToStateProviders, 27, 28) .with(UserNotificationSettingsKeyMigrator, 28, 29) - .with(PolicyMigrator, 29, CURRENT_VERSION); + .with(PolicyMigrator, 29, 30) + .with(EnableContextMenuMigrator, 30, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/31-move-enable-context-menu-to-autofill-settings-state-provider.spec.ts b/libs/common/src/state-migrations/migrations/31-move-enable-context-menu-to-autofill-settings-state-provider.spec.ts new file mode 100644 index 0000000000..019f546831 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/31-move-enable-context-menu-to-autofill-settings-state-provider.spec.ts @@ -0,0 +1,91 @@ +import { any, MockProxy } from "jest-mock-extended"; + +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { EnableContextMenuMigrator } from "./31-move-enable-context-menu-to-autofill-settings-state-provider"; + +function exampleJSON() { + return { + global: { + disableContextMenuItem: true, + otherStuff: "otherStuff1", + }, + otherStuff: "otherStuff2", + }; +} + +function rollbackJSON() { + return { + global_autofillSettings_enableContextMenu: false, + global: { + otherStuff: "otherStuff1", + }, + otherStuff: "otherStuff2", + }; +} + +const enableContextMenuKeyDefinition: KeyDefinitionLike = { + stateDefinition: { + name: "autofillSettings", + }, + key: "enableContextMenu", +}; + +describe("EnableContextMenuMigrator", () => { + let helper: MockProxy; + let sut: EnableContextMenuMigrator; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(exampleJSON(), 30); + sut = new EnableContextMenuMigrator(30, 31); + }); + + it("should remove global disableContextMenuItem setting", async () => { + await sut.migrate(helper); + + expect(helper.set).toHaveBeenCalledTimes(1); + expect(helper.set).toHaveBeenCalledWith("global", { + otherStuff: "otherStuff1", + }); + }); + + it("should set enableContextMenu globally", async () => { + await sut.migrate(helper); + + expect(helper.setToGlobal).toHaveBeenCalledTimes(1); + expect(helper.setToGlobal).toHaveBeenCalledWith(enableContextMenuKeyDefinition, false); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJSON(), 31); + sut = new EnableContextMenuMigrator(30, 31); + }); + + it("should null out new enableContextMenu global value", async () => { + await sut.rollback(helper); + + expect(helper.setToGlobal).toHaveBeenCalledTimes(1); + expect(helper.setToGlobal).toHaveBeenCalledWith(enableContextMenuKeyDefinition, null); + }); + + it("should add disableContextMenuItem global value back", async () => { + await sut.rollback(helper); + + expect(helper.set).toHaveBeenCalledTimes(1); + expect(helper.set).toHaveBeenCalledWith("global", { + disableContextMenuItem: true, + otherStuff: "otherStuff1", + }); + }); + + it("should not try to restore values to missing accounts", async () => { + await sut.rollback(helper); + + expect(helper.set).not.toHaveBeenCalledWith("user-3", any()); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/31-move-enable-context-menu-to-autofill-settings-state-provider.ts b/libs/common/src/state-migrations/migrations/31-move-enable-context-menu-to-autofill-settings-state-provider.ts new file mode 100644 index 0000000000..d5b768a2bd --- /dev/null +++ b/libs/common/src/state-migrations/migrations/31-move-enable-context-menu-to-autofill-settings-state-provider.ts @@ -0,0 +1,46 @@ +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +type ExpectedGlobalState = { + disableContextMenuItem?: boolean; +}; + +const enableContextMenuKeyDefinition: KeyDefinitionLike = { + stateDefinition: { + name: "autofillSettings", + }, + key: "enableContextMenu", +}; + +export class EnableContextMenuMigrator extends Migrator<30, 31> { + async migrate(helper: MigrationHelper): Promise { + const globalState = await helper.get("global"); + + // disableContextMenuItem -> enableContextMenu + if (globalState?.disableContextMenuItem != null) { + await helper.setToGlobal(enableContextMenuKeyDefinition, !globalState.disableContextMenuItem); + + // delete `disableContextMenuItem` from state global + delete globalState.disableContextMenuItem; + + await helper.set("global", globalState); + } + } + + async rollback(helper: MigrationHelper): Promise { + const globalState = (await helper.get("global")) || {}; + + const enableContextMenu: boolean = await helper.getFromGlobal(enableContextMenuKeyDefinition); + + // enableContextMenu -> disableContextMenuItem + if (enableContextMenu != null) { + await helper.set("global", { + ...globalState, + disableContextMenuItem: !enableContextMenu, + }); + + // remove the global state provider framework key for `enableContextMenu` + await helper.setToGlobal(enableContextMenuKeyDefinition, null); + } + } +}