From 42128d41f014d9b04c9f74223d3386741425691c Mon Sep 17 00:00:00 2001 From: Daniel James Smith Date: Tue, 29 Nov 2022 18:14:42 +0100 Subject: [PATCH] [PS-815] MV3: Replace usage of delayed/periodic operations with AlarmsAPI (#4103) * Register alarms and listen to them * Wire up alarms and actions Register actions(commands) which can be executed by an alarm Create methods in alarm-state to persists actions and execution times Flesh out AlarmListener to iterate over registered commands and check if they need to execute Simplify clearClipboard action as it only handles the action instead of also worrying if it should fire. Enable previously disabled clear-clipboard tests (#3532) Adjust clear-clipboard tests to new simpler execution Co-authored-by: Matt Gibson * Make linter happy * Revert accidentally commited with merging master * Add jsdoc per PR comment * Fixed types to simplify adding new alarm actions Create a new alarm action (i.e `clear-clipboard.ts`) Export a name for the alarm action (`clearClipboardAlarmName`) `alarm-state.ts` Import alarm action name Extend `alarmKeys` and `alarmState` `on-alarm-listener` Import alarm action method and alarm action name Add it to the switch case * Add comment to clearClipboard action Add comment to replace clearClipboard impl once clipboardApi's are accessible by service-workers https://bugs.chromium.org/p/chromium/issues/detail?id=1160302 Co-authored-by: Matt Gibson --- apps/browser/src/alarms/alarm-state.ts | 66 ++++++++++++++++ apps/browser/src/alarms/on-alarm-listener.ts | 26 ++++++ apps/browser/src/alarms/register-alarms.ts | 29 +++++++ apps/browser/src/background.ts | 16 +--- .../src/clipboard/clear-clipboard.spec.ts | 39 +++++++++ .../clear-clipboard.spec.ts.disabled | 79 ------------------- apps/browser/src/clipboard/clear-clipboard.ts | 44 ++--------- ...rate-password-to-clipboard-command.spec.ts | 16 ++-- .../generate-password-to-clipboard-command.ts | 5 +- 9 files changed, 183 insertions(+), 137 deletions(-) create mode 100644 apps/browser/src/alarms/alarm-state.ts create mode 100644 apps/browser/src/alarms/on-alarm-listener.ts create mode 100644 apps/browser/src/alarms/register-alarms.ts create mode 100644 apps/browser/src/clipboard/clear-clipboard.spec.ts delete mode 100644 apps/browser/src/clipboard/clear-clipboard.spec.ts.disabled diff --git a/apps/browser/src/alarms/alarm-state.ts b/apps/browser/src/alarms/alarm-state.ts new file mode 100644 index 00000000000..774bcfe1c03 --- /dev/null +++ b/apps/browser/src/alarms/alarm-state.ts @@ -0,0 +1,66 @@ +import { BrowserApi } from "../browser/browserApi"; +import { clearClipboardAlarmName } from "../clipboard"; + +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.manifestVersion == 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.manifestVersion == 3) { + await chrome.storage.session.set({ [commandName]: time }); + } else { + alarmState[commandName] = time; + } +} diff --git a/apps/browser/src/alarms/on-alarm-listener.ts b/apps/browser/src/alarms/on-alarm-listener.ts new file mode 100644 index 00000000000..4b246908778 --- /dev/null +++ b/apps/browser/src/alarms/on-alarm-listener.ts @@ -0,0 +1,26 @@ +import { ClearClipboard, clearClipboardAlarmName } from "../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: + ClearClipboard.run(); + break; + default: + } + }); +}; diff --git a/apps/browser/src/alarms/register-alarms.ts b/apps/browser/src/alarms/register-alarms.ts new file mode 100644 index 00000000000..c4da2fc27bc --- /dev/null +++ b/apps/browser/src/alarms/register-alarms.ts @@ -0,0 +1,29 @@ +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 numnber 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++) { + chrome.alarms.create(`bw_alarm${index}`, { periodInMinutes: 1, when: calculatedWhen }); + calculatedWhen += offset; + } +} diff --git a/apps/browser/src/background.ts b/apps/browser/src/background.ts index 504b5afb5c0..fc020b19aa3 100644 --- a/apps/browser/src/background.ts +++ b/apps/browser/src/background.ts @@ -1,6 +1,7 @@ +import { onAlarmListener } from "./alarms/on-alarm-listener"; +import { registerAlarms } from "./alarms/register-alarms"; import MainBackground from "./background/main.background"; import { BrowserApi } from "./browser/browserApi"; -import { ClearClipboard } from "./clipboard"; import { onCommandListener } from "./listeners/onCommandListener"; import { onInstallListener } from "./listeners/onInstallListener"; import { UpdateBadge } from "./listeners/update-badge"; @@ -9,13 +10,12 @@ const manifestV3MessageListeners: (( serviceCache: Record, message: { command: string } ) => void | Promise)[] = [UpdateBadge.messageListener]; -type AlarmAction = (executionTime: Date, serviceCache: Record) => void; - -const AlarmActions: AlarmAction[] = [ClearClipboard.run]; if (BrowserApi.manifestVersion === 3) { chrome.commands.onCommand.addListener(onCommandListener); chrome.runtime.onInstalled.addListener(onInstallListener); + chrome.alarms.onAlarm.addListener(onAlarmListener); + registerAlarms(); chrome.tabs.onActivated.addListener(UpdateBadge.tabsOnActivatedListener); chrome.tabs.onReplaced.addListener(UpdateBadge.tabsOnReplacedListener); chrome.tabs.onUpdated.addListener(UpdateBadge.tabsOnUpdatedListener); @@ -26,14 +26,6 @@ if (BrowserApi.manifestVersion === 3) { listener(serviceCache, message); }); }); - chrome.alarms.onAlarm.addListener((_alarm) => { - const executionTime = new Date(); - const serviceCache = {}; - - for (const alarmAction of AlarmActions) { - alarmAction(executionTime, serviceCache); - } - }); } else { const bitwardenMain = ((window as any).bitwardenMain = new MainBackground()); bitwardenMain.bootstrap().then(() => { diff --git a/apps/browser/src/clipboard/clear-clipboard.spec.ts b/apps/browser/src/clipboard/clear-clipboard.spec.ts new file mode 100644 index 00000000000..0166bdb47af --- /dev/null +++ b/apps/browser/src/clipboard/clear-clipboard.spec.ts @@ -0,0 +1,39 @@ +import { BrowserApi } from "../browser/browserApi"; + +import { ClearClipboard } from "./clear-clipboard"; + +describe("clearClipboard", () => { + describe("run", () => { + it("Does not clear clipboard when no active tabs are retrieved", async () => { + jest.spyOn(BrowserApi, "getActiveTabs").mockResolvedValue([] as any); + + jest.spyOn(BrowserApi, "sendTabsMessage").mockReturnValue(); + + await ClearClipboard.run(); + + expect(jest.spyOn(BrowserApi, "sendTabsMessage")).not.toHaveBeenCalled(); + + expect(jest.spyOn(BrowserApi, "sendTabsMessage")).not.toHaveBeenCalledWith(1, { + command: "clearClipboard", + }); + }); + + it("Sends a message to the content script to clear the clipboard", async () => { + jest.spyOn(BrowserApi, "getActiveTabs").mockResolvedValue([ + { + id: 1, + }, + ] as any); + + jest.spyOn(BrowserApi, "sendTabsMessage").mockReturnValue(); + + await ClearClipboard.run(); + + expect(jest.spyOn(BrowserApi, "sendTabsMessage")).toHaveBeenCalledTimes(1); + + expect(jest.spyOn(BrowserApi, "sendTabsMessage")).toHaveBeenCalledWith(1, { + command: "clearClipboard", + }); + }); + }); +}); diff --git a/apps/browser/src/clipboard/clear-clipboard.spec.ts.disabled b/apps/browser/src/clipboard/clear-clipboard.spec.ts.disabled deleted file mode 100644 index 6f7314d91ac..00000000000 --- a/apps/browser/src/clipboard/clear-clipboard.spec.ts.disabled +++ /dev/null @@ -1,79 +0,0 @@ -import { mock, MockProxy } from "jest-mock-extended"; - -import { BrowserApi } from "../browser/browserApi"; -import { StateService } from "../services/abstractions/state.service"; - -import { ClearClipboard } from "./clear-clipboard"; -import { getClearClipboardTime, setClearClipboardTime } from "./clipboard-state"; - -jest.mock("./clipboard-state", () => { - return { - getClearClipboardTime: jest.fn(), - setClearClipboardTime: jest.fn(), - }; -}); - -const getClearClipboardTimeMock = getClearClipboardTime as jest.Mock; -const setClearClipboardTimeMock = setClearClipboardTime as jest.Mock; - -describe("clearClipboard", () => { - describe("run", () => { - let stateService: MockProxy; - let serviceCache: Record; - - beforeEach(() => { - stateService = mock(); - serviceCache = { - stateService: stateService, - }; - }); - - afterEach(() => { - jest.resetAllMocks(); - }); - - it("has a clear time that is past execution time", async () => { - const executionTime = new Date(2022, 1, 1, 12); - const clearTime = new Date(2022, 1, 1, 12, 1); - - jest.spyOn(BrowserApi, "getActiveTabs").mockResolvedValue([ - { - id: 1, - }, - ] as any); - - jest.spyOn(BrowserApi, "sendTabsMessage").mockReturnValue(); - - getClearClipboardTimeMock.mockResolvedValue(clearTime.getTime()); - - await ClearClipboard.run(executionTime, serviceCache); - - expect(jest.spyOn(BrowserApi, "sendTabsMessage")).toHaveBeenCalledTimes(1); - - expect(jest.spyOn(BrowserApi, "sendTabsMessage")).toHaveBeenCalledWith(1, { - command: "clearClipboard", - }); - }); - - it("has a clear time before execution time", async () => { - const executionTime = new Date(2022, 1, 1, 12); - const clearTime = new Date(2022, 1, 1, 11); - - setClearClipboardTimeMock.mockResolvedValue(clearTime.getTime()); - - await ClearClipboard.run(executionTime, serviceCache); - - expect(jest.spyOn(BrowserApi, "getActiveTabs")).not.toHaveBeenCalled(); - }); - - it("has an undefined clearTime", async () => { - const executionTime = new Date(2022, 1, 1); - - getClearClipboardTimeMock.mockResolvedValue(undefined); - - await ClearClipboard.run(executionTime, serviceCache); - - expect(jest.spyOn(BrowserApi, "getActiveTabs")).not.toHaveBeenCalled(); - }); - }); -}); diff --git a/apps/browser/src/clipboard/clear-clipboard.ts b/apps/browser/src/clipboard/clear-clipboard.ts index 00bf329f8d3..149992555b9 100644 --- a/apps/browser/src/clipboard/clear-clipboard.ts +++ b/apps/browser/src/clipboard/clear-clipboard.ts @@ -1,43 +1,15 @@ -import { StateFactory } from "@bitwarden/common/factories/stateFactory"; -import { GlobalState } from "@bitwarden/common/models/domain/global-state"; - -import { stateServiceFactory } from "../background/service_factories/state-service.factory"; import { BrowserApi } from "../browser/browserApi"; -import { Account } from "../models/account"; -import { getClearClipboardTime } from "./clipboard-state"; +export const clearClipboardAlarmName = "clearClipboard"; export class ClearClipboard { - static async run(executionTime: Date, serviceCache: Record) { - const stateFactory = new StateFactory(GlobalState, Account); - const stateService = await stateServiceFactory(serviceCache, { - cryptoFunctionServiceOptions: { - win: self, - }, - encryptServiceOptions: { - logMacFailures: false, - }, - logServiceOptions: { - isDev: false, - }, - stateMigrationServiceOptions: { - stateFactory: stateFactory, - }, - stateServiceOptions: { - stateFactory: stateFactory, - }, - }); - - const clearClipboardTime = await getClearClipboardTime(stateService); - - if (!clearClipboardTime) { - return; - } - - if (clearClipboardTime < executionTime.getTime()) { - return; - } - + /** + 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 + */ + static async run() { const activeTabs = await BrowserApi.getActiveTabs(); if (!activeTabs || activeTabs.length === 0) { return; diff --git a/apps/browser/src/clipboard/generate-password-to-clipboard-command.spec.ts b/apps/browser/src/clipboard/generate-password-to-clipboard-command.spec.ts index 5ab36b06fef..5d119513f43 100644 --- a/apps/browser/src/clipboard/generate-password-to-clipboard-command.spec.ts +++ b/apps/browser/src/clipboard/generate-password-to-clipboard-command.spec.ts @@ -2,20 +2,20 @@ import { mock, MockProxy } from "jest-mock-extended"; import { PasswordGenerationService } from "@bitwarden/common/abstractions/passwordGeneration.service"; +import { setAlarmTime } from "../alarms/alarm-state"; import { BrowserApi } from "../browser/browserApi"; import { BrowserStateService } from "../services/abstractions/browser-state.service"; -import { setClearClipboardTime } from "./clipboard-state"; +import { clearClipboardAlarmName } from "./clear-clipboard"; import { GeneratePasswordToClipboardCommand } from "./generate-password-to-clipboard-command"; -jest.mock("./clipboard-state", () => { +jest.mock("../alarms/alarm-state", () => { return { - getClearClipboardTime: jest.fn(), - setClearClipboardTime: jest.fn(), + setAlarmTime: jest.fn(), }; }); -const setClearClipboardTimeMock = setClearClipboardTime as jest.Mock; +const setAlarmTimeMock = setAlarmTime as jest.Mock; describe("GeneratePasswordToClipboardCommand", () => { let passwordGenerationService: MockProxy; @@ -53,9 +53,9 @@ describe("GeneratePasswordToClipboardCommand", () => { text: "PASSWORD", }); - expect(setClearClipboardTimeMock).toHaveBeenCalledTimes(1); + expect(setAlarmTimeMock).toHaveBeenCalledTimes(1); - expect(setClearClipboardTimeMock).toHaveBeenCalledWith(stateService, expect.any(Number)); + expect(setAlarmTimeMock).toHaveBeenCalledWith(clearClipboardAlarmName, expect.any(Number)); }); it("does not have clear clipboard value", async () => { @@ -70,7 +70,7 @@ describe("GeneratePasswordToClipboardCommand", () => { text: "PASSWORD", }); - expect(setClearClipboardTimeMock).not.toHaveBeenCalled(); + expect(setAlarmTimeMock).not.toHaveBeenCalled(); }); }); }); diff --git a/apps/browser/src/clipboard/generate-password-to-clipboard-command.ts b/apps/browser/src/clipboard/generate-password-to-clipboard-command.ts index e6d4d6b8b0c..0cd8eec2c93 100644 --- a/apps/browser/src/clipboard/generate-password-to-clipboard-command.ts +++ b/apps/browser/src/clipboard/generate-password-to-clipboard-command.ts @@ -1,8 +1,9 @@ import { PasswordGenerationService } from "@bitwarden/common/abstractions/passwordGeneration.service"; +import { setAlarmTime } from "../alarms/alarm-state"; import { BrowserStateService } from "../services/abstractions/browser-state.service"; -import { setClearClipboardTime } from "./clipboard-state"; +import { clearClipboardAlarmName } from "./clear-clipboard"; import { copyToClipboard } from "./copy-to-clipboard-command"; export class GeneratePasswordToClipboardCommand { @@ -20,7 +21,7 @@ export class GeneratePasswordToClipboardCommand { const clearClipboard = await this.stateService.getClearClipboard(); if (clearClipboard != null) { - await setClearClipboardTime(this.stateService, Date.now() + clearClipboard * 1000); + await setAlarmTime(clearClipboardAlarmName, clearClipboard * 1000); } } }