From 2f517336db57d71e6a4431c7a19a6af92f87b90f Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Mon, 1 Apr 2024 11:45:16 -0500 Subject: [PATCH] [PM-6426] Implementing clear clipboard call on generatePasswordToClipboard with the TaskSchedulerService --- .../browser/context-menu-clicked-handler.ts | 2 + .../src/autofill/clipboard/clear-clipboard.ts | 4 +- ...rate-password-to-clipboard-command.spec.ts | 27 ++++---- .../generate-password-to-clipboard-command.ts | 30 ++++++--- .../src/platform/alarms/alarm-state.ts | 66 ------------------- .../src/platform/alarms/on-alarm-listener.ts | 28 -------- .../src/platform/alarms/register-alarms.ts | 31 --------- apps/browser/src/platform/background.ts | 4 -- .../browser-task-scheduler-service.factory.ts | 27 ++++++++ .../platform/listeners/on-command-listener.ts | 2 + 10 files changed, 65 insertions(+), 156 deletions(-) delete mode 100644 apps/browser/src/platform/alarms/alarm-state.ts delete mode 100644 apps/browser/src/platform/alarms/on-alarm-listener.ts delete mode 100644 apps/browser/src/platform/alarms/register-alarms.ts create mode 100644 apps/browser/src/platform/background/service-factories/browser-task-scheduler-service.factory.ts diff --git a/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts b/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts index 596d6b7235..55f7608ef0 100644 --- a/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts +++ b/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts @@ -36,6 +36,7 @@ import { openUnlockPopout } from "../../auth/popup/utils/auth-popout-window"; import { autofillSettingsServiceFactory } from "../../autofill/background/service_factories/autofill-settings-service.factory"; import { eventCollectionServiceFactory } from "../../background/service-factories/event-collection-service.factory"; import { Account } from "../../models/account"; +import { browserTaskSchedulerServiceFactory } from "../../platform/background/service-factories/browser-task-scheduler-service.factory"; import { CachedServices } from "../../platform/background/service-factories/factory-options"; import { stateServiceFactory } from "../../platform/background/service-factories/state-service.factory"; import { BrowserApi } from "../../platform/browser/browser-api"; @@ -116,6 +117,7 @@ export class ContextMenuClickedHandler { const generatePasswordToClipboardCommand = new GeneratePasswordToClipboardCommand( await passwordGenerationServiceFactory(cachedServices, serviceOptions), await autofillSettingsServiceFactory(cachedServices, serviceOptions), + await browserTaskSchedulerServiceFactory(cachedServices, serviceOptions), ); const autofillCommand = new AutofillTabCommand( diff --git a/apps/browser/src/autofill/clipboard/clear-clipboard.ts b/apps/browser/src/autofill/clipboard/clear-clipboard.ts index f8018bb036..426d653951 100644 --- a/apps/browser/src/autofill/clipboard/clear-clipboard.ts +++ b/apps/browser/src/autofill/clipboard/clear-clipboard.ts @@ -1,11 +1,9 @@ import { BrowserApi } from "../../platform/browser/browser-api"; -export const clearClipboardAlarmName = "clearClipboard"; - export class ClearClipboard { /** We currently rely on an active tab with an injected content script (`../content/misc-utils.ts`) to clear the clipboard via `window.navigator.clipboard.writeText(text)` - + With https://bugs.chromium.org/p/chromium/issues/detail?id=1160302 it was said that service workers, would have access to the clipboard api and then we could migrate to a simpler solution */ diff --git a/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.spec.ts b/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.spec.ts index e3639ef81f..c882a5fa1d 100644 --- a/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.spec.ts +++ b/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.spec.ts @@ -1,30 +1,24 @@ import { mock, MockProxy } from "jest-mock-extended"; import { AutofillSettingsService } from "@bitwarden/common/autofill/services/autofill-settings.service"; +import { ScheduledTaskNames } from "@bitwarden/common/platform/enums/scheduled-task-name.enum"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; -import { setAlarmTime } from "../../platform/alarms/alarm-state"; import { BrowserApi } from "../../platform/browser/browser-api"; +import { BrowserTaskSchedulerService } from "../../platform/services/browser-task-scheduler.service"; -import { clearClipboardAlarmName } from "./clear-clipboard"; import { GeneratePasswordToClipboardCommand } from "./generate-password-to-clipboard-command"; -jest.mock("../../platform/alarms/alarm-state", () => { - return { - setAlarmTime: jest.fn(), - }; -}); - -const setAlarmTimeMock = setAlarmTime as jest.Mock; - describe("GeneratePasswordToClipboardCommand", () => { let passwordGenerationService: MockProxy; let autofillSettingsService: MockProxy; + let browserTaskSchedulerService: MockProxy; let sut: GeneratePasswordToClipboardCommand; beforeEach(() => { passwordGenerationService = mock(); + browserTaskSchedulerService = mock(); passwordGenerationService.getOptions.mockResolvedValue([{ length: 8 }, {} as any]); @@ -35,6 +29,7 @@ describe("GeneratePasswordToClipboardCommand", () => { sut = new GeneratePasswordToClipboardCommand( passwordGenerationService, autofillSettingsService, + browserTaskSchedulerService, ); }); @@ -55,9 +50,12 @@ describe("GeneratePasswordToClipboardCommand", () => { text: "PASSWORD", }); - expect(setAlarmTimeMock).toHaveBeenCalledTimes(1); - - expect(setAlarmTimeMock).toHaveBeenCalledWith(clearClipboardAlarmName, expect.any(Number)); + expect(browserTaskSchedulerService.setTimeout).toHaveBeenCalledTimes(1); + expect(browserTaskSchedulerService.setTimeout).toHaveBeenCalledWith( + expect.any(Function), + expect.any(Number), + ScheduledTaskNames.clearClipboardTimeout, + ); }); it("does not have clear clipboard value", async () => { @@ -71,8 +69,7 @@ describe("GeneratePasswordToClipboardCommand", () => { command: "copyText", text: "PASSWORD", }); - - expect(setAlarmTimeMock).not.toHaveBeenCalled(); + expect(browserTaskSchedulerService.setTimeout).not.toHaveBeenCalled(); }); }); }); diff --git a/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.ts b/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.ts index c46718ebad..8347043cb9 100644 --- a/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.ts +++ b/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.ts @@ -1,17 +1,21 @@ import { firstValueFrom } from "rxjs"; import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service"; +import { ScheduledTaskNames } from "@bitwarden/common/platform/enums/scheduled-task-name.enum"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; -import { setAlarmTime } from "../../platform/alarms/alarm-state"; +import { BrowserTaskSchedulerService } from "../../platform/services/abstractions/browser-task-scheduler.service"; -import { clearClipboardAlarmName } from "./clear-clipboard"; +import { ClearClipboard } from "./clear-clipboard"; import { copyToClipboard } from "./copy-to-clipboard-command"; export class GeneratePasswordToClipboardCommand { + private clearClipboardTimeout: number | NodeJS.Timeout; + constructor( private passwordGenerationService: PasswordGenerationServiceAbstraction, private autofillSettingsService: AutofillSettingsServiceAbstraction, + private taskSchedulerService: BrowserTaskSchedulerService, ) {} async getClearClipboard() { @@ -22,14 +26,22 @@ export class GeneratePasswordToClipboardCommand { const [options] = await this.passwordGenerationService.getOptions(); const password = await this.passwordGenerationService.generatePassword(options); - // 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 - copyToClipboard(tab, password); + await copyToClipboard(tab, password); - const clearClipboard = await this.getClearClipboard(); - - if (clearClipboard != null) { - await setAlarmTime(clearClipboardAlarmName, clearClipboard * 1000); + const clearClipboardDelayInSeconds = await this.getClearClipboard(); + if (!clearClipboardDelayInSeconds) { + return; } + + const timeoutInMs = clearClipboardDelayInSeconds * 1000; + await this.taskSchedulerService.clearScheduledTask({ + taskName: ScheduledTaskNames.clearClipboardTimeout, + timeoutId: this.clearClipboardTimeout, + }); + await this.taskSchedulerService.setTimeout( + () => ClearClipboard.run(), + timeoutInMs, + ScheduledTaskNames.clearClipboardTimeout, + ); } } diff --git a/apps/browser/src/platform/alarms/alarm-state.ts b/apps/browser/src/platform/alarms/alarm-state.ts deleted file mode 100644 index fa18e26ed1..0000000000 --- a/apps/browser/src/platform/alarms/alarm-state.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { clearClipboardAlarmName } from "../../autofill/clipboard"; -import { BrowserApi } from "../browser/browser-api"; - -export const alarmKeys = [clearClipboardAlarmName] as const; -export type AlarmKeys = (typeof alarmKeys)[number]; - -type AlarmState = { [T in AlarmKeys]: number | undefined }; - -const alarmState: AlarmState = { - clearClipboard: null, - //TODO once implemented vaultTimeout: null; - //TODO once implemented checkNotifications: null; - //TODO once implemented (if necessary) processReload: null; -}; - -/** - * Retrieves the set alarm time (planned execution) for a give an commandName {@link AlarmState} - * @param commandName A command that has been previously registered with {@link AlarmState} - * @returns {Promise} null or Unix epoch timestamp when the alarm action is supposed to execute - * @example - * // getAlarmTime(clearClipboard) - */ -export async function getAlarmTime(commandName: AlarmKeys): Promise { - let alarmTime: number; - if (BrowserApi.isManifestVersion(3)) { - const fromSessionStore = await chrome.storage.session.get(commandName); - alarmTime = fromSessionStore[commandName]; - } else { - alarmTime = alarmState[commandName]; - } - - return alarmTime; -} - -/** - * Registers an action that should execute after the given time has passed - * @param commandName A command that has been previously registered with {@link AlarmState} - * @param delay_ms The number of ms from now in which the command should execute from - * @example - * // setAlarmTime(clearClipboard, 5000) register the clearClipboard action which will execute when at least 5 seconds from now have passed - */ -export async function setAlarmTime(commandName: AlarmKeys, delay_ms: number): Promise { - if (!delay_ms || delay_ms === 0) { - await this.clearAlarmTime(commandName); - return; - } - - const time = Date.now() + delay_ms; - await setAlarmTimeInternal(commandName, time); -} - -/** - * Clears the time currently set for a given command - * @param commandName A command that has been previously registered with {@link AlarmState} - */ -export async function clearAlarmTime(commandName: AlarmKeys): Promise { - await setAlarmTimeInternal(commandName, null); -} - -async function setAlarmTimeInternal(commandName: AlarmKeys, time: number): Promise { - if (BrowserApi.isManifestVersion(3)) { - await chrome.storage.session.set({ [commandName]: time }); - } else { - alarmState[commandName] = time; - } -} diff --git a/apps/browser/src/platform/alarms/on-alarm-listener.ts b/apps/browser/src/platform/alarms/on-alarm-listener.ts deleted file mode 100644 index 274f19f789..0000000000 --- a/apps/browser/src/platform/alarms/on-alarm-listener.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { ClearClipboard, clearClipboardAlarmName } from "../../autofill/clipboard"; - -import { alarmKeys, clearAlarmTime, getAlarmTime } from "./alarm-state"; - -export const onAlarmListener = async (alarm: chrome.alarms.Alarm) => { - alarmKeys.forEach(async (key) => { - const executionTime = await getAlarmTime(key); - if (!executionTime) { - return; - } - - const currentDate = Date.now(); - if (executionTime > currentDate) { - return; - } - - await clearAlarmTime(key); - - switch (key) { - case clearClipboardAlarmName: - // 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 - ClearClipboard.run(); - break; - default: - } - }); -}; diff --git a/apps/browser/src/platform/alarms/register-alarms.ts b/apps/browser/src/platform/alarms/register-alarms.ts deleted file mode 100644 index 86b9fb9774..0000000000 --- a/apps/browser/src/platform/alarms/register-alarms.ts +++ /dev/null @@ -1,31 +0,0 @@ -const NUMBER_OF_ALARMS = 6; - -export function registerAlarms() { - alarmsToBeCreated(NUMBER_OF_ALARMS); -} - -/** - * Creates staggered alarms that periodically (1min) raise OnAlarm events. The staggering is calculated based on the number of alarms passed in. - * @param numberOfAlarms Number of named alarms, that shall be registered - * @example - * // alarmsToBeCreated(2) results in 2 alarms separated by 30 seconds - * @example - * // alarmsToBeCreated(4) results in 4 alarms separated by 15 seconds - * @example - * // alarmsToBeCreated(6) results in 6 alarms separated by 10 seconds - * @example - * // alarmsToBeCreated(60) results in 60 alarms separated by 1 second - */ -function alarmsToBeCreated(numberOfAlarms: number): void { - const oneMinuteInMs = 60 * 1000; - const offset = oneMinuteInMs / numberOfAlarms; - - let calculatedWhen: number = Date.now() + offset; - - for (let index = 0; index < numberOfAlarms; index++) { - // 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 - chrome.alarms.create(`bw_alarm${index}`, { periodInMinutes: 1, when: calculatedWhen }); - calculatedWhen += offset; - } -} diff --git a/apps/browser/src/platform/background.ts b/apps/browser/src/platform/background.ts index 5aa2820e5f..2736d81b2b 100644 --- a/apps/browser/src/platform/background.ts +++ b/apps/browser/src/platform/background.ts @@ -1,7 +1,5 @@ import MainBackground from "../background/main.background"; -import { onAlarmListener } from "./alarms/on-alarm-listener"; -import { registerAlarms } from "./alarms/register-alarms"; import { BrowserApi } from "./browser/browser-api"; import { contextMenusClickedListener, @@ -17,8 +15,6 @@ import { if (BrowserApi.isManifestVersion(3)) { chrome.commands.onCommand.addListener(onCommandListener); chrome.runtime.onInstalled.addListener(onInstallListener); - chrome.alarms.onAlarm.addListener(onAlarmListener); - registerAlarms(); chrome.windows.onFocusChanged.addListener(windowsOnFocusChangedListener); chrome.tabs.onActivated.addListener(tabsOnActivatedListener); chrome.tabs.onReplaced.addListener(tabsOnReplacedListener); diff --git a/apps/browser/src/platform/background/service-factories/browser-task-scheduler-service.factory.ts b/apps/browser/src/platform/background/service-factories/browser-task-scheduler-service.factory.ts new file mode 100644 index 0000000000..1d078ae5db --- /dev/null +++ b/apps/browser/src/platform/background/service-factories/browser-task-scheduler-service.factory.ts @@ -0,0 +1,27 @@ +import { BrowserTaskSchedulerService } from "../../services/browser-task-scheduler.service"; + +import { CachedServices, factory, FactoryOptions } from "./factory-options"; +import { logServiceFactory, LogServiceInitOptions } from "./log-service.factory"; +import { stateProviderFactory, StateProviderInitOptions } from "./state-provider.factory"; + +type BrowserTaskSchedulerServiceFactoryOptions = FactoryOptions; + +export type BrowserTaskSchedulerServiceInitOptions = BrowserTaskSchedulerServiceFactoryOptions & + LogServiceInitOptions & + StateProviderInitOptions; + +export function browserTaskSchedulerServiceFactory( + cache: { browserTaskSchedulerService?: BrowserTaskSchedulerService } & CachedServices, + opts: BrowserTaskSchedulerServiceInitOptions, +): Promise { + return factory( + cache, + "browserTaskSchedulerService", + opts, + async () => + new BrowserTaskSchedulerService( + await logServiceFactory(cache, opts), + await stateProviderFactory(cache, opts), + ), + ); +} diff --git a/apps/browser/src/platform/listeners/on-command-listener.ts b/apps/browser/src/platform/listeners/on-command-listener.ts index fc2939dce8..4820e3214d 100644 --- a/apps/browser/src/platform/listeners/on-command-listener.ts +++ b/apps/browser/src/platform/listeners/on-command-listener.ts @@ -12,6 +12,7 @@ import { passwordGenerationServiceFactory, PasswordGenerationServiceInitOptions, } from "../../tools/background/service_factories/password-generation-service.factory"; +import { browserTaskSchedulerServiceFactory } from "../background/service-factories/browser-task-scheduler-service.factory"; import { CachedServices } from "../background/service-factories/factory-options"; import { logServiceFactory } from "../background/service-factories/log-service.factory"; import { BrowserApi } from "../browser/browser-api"; @@ -102,6 +103,7 @@ const doGeneratePasswordToClipboard = async (tab: chrome.tabs.Tab): Promise