1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-09-13 01:58:44 +02:00

[PM-6426] Implementing approach for incorporating the user UUID when setting task handlers

This commit is contained in:
Cesar Gonzalez 2024-04-17 13:57:25 -05:00
parent 8fdc9df4a7
commit 6cb2577f90
No known key found for this signature in database
GPG Key ID: 3381A5457F8CCECF
14 changed files with 97 additions and 42 deletions

View File

@ -17,8 +17,9 @@ export class GeneratePasswordToClipboardCommand {
private autofillSettingsService: AutofillSettingsServiceAbstraction, private autofillSettingsService: AutofillSettingsServiceAbstraction,
private taskSchedulerService: BrowserTaskSchedulerService, private taskSchedulerService: BrowserTaskSchedulerService,
) { ) {
this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.clearClipboardTimeout, () => void this.taskSchedulerService.registerTaskHandler(
ClearClipboard.run(), ScheduledTaskNames.clearClipboardTimeout,
() => ClearClipboard.run(),
); );
} }

View File

@ -448,8 +448,9 @@ export default class MainBackground {
this.logService, this.logService,
this.stateProvider, this.stateProvider,
); );
this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.scheduleNextSyncInterval, () => void this.taskSchedulerService.registerTaskHandler(
this.fullSync(), ScheduledTaskNames.scheduleNextSyncInterval,
() => this.fullSync(),
); );
this.environmentService = new BrowserEnvironmentService( this.environmentService = new BrowserEnvironmentService(
this.logService, this.logService,

View File

@ -1,10 +1,11 @@
import { mock, MockProxy } from "jest-mock-extended"; import { mock, MockProxy } from "jest-mock-extended";
import { Observable } from "rxjs"; import { BehaviorSubject, Observable } from "rxjs";
import { TaskIdentifier } from "@bitwarden/common/platform/abstractions/task-scheduler.service"; import { TaskIdentifier } from "@bitwarden/common/platform/abstractions/task-scheduler.service";
import { ScheduledTaskNames } from "@bitwarden/common/platform/enums/scheduled-task-name.enum"; import { ScheduledTaskNames } from "@bitwarden/common/platform/enums/scheduled-task-name.enum";
import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service";
import { GlobalState, StateProvider } from "@bitwarden/common/platform/state"; import { GlobalState, StateProvider } from "@bitwarden/common/platform/state";
import { UserId } from "@bitwarden/common/types/guid";
import { ActiveAlarm } from "./abstractions/browser-task-scheduler.service"; import { ActiveAlarm } from "./abstractions/browser-task-scheduler.service";
import { BrowserTaskSchedulerService } from "./browser-task-scheduler.service"; import { BrowserTaskSchedulerService } from "./browser-task-scheduler.service";
@ -18,6 +19,7 @@ jest.mock("rxjs", () => ({
// TODO CG - Likely need to rethink how to test this service a bit more carefully. // TODO CG - Likely need to rethink how to test this service a bit more carefully.
describe("BrowserTaskSchedulerService", () => { describe("BrowserTaskSchedulerService", () => {
let activeUserIdMock$: BehaviorSubject<UserId>;
let logService: MockProxy<ConsoleLogService>; let logService: MockProxy<ConsoleLogService>;
let stateProvider: MockProxy<StateProvider>; let stateProvider: MockProxy<StateProvider>;
let browserTaskSchedulerService: BrowserTaskSchedulerService; let browserTaskSchedulerService: BrowserTaskSchedulerService;
@ -41,8 +43,10 @@ describe("BrowserTaskSchedulerService", () => {
createInfo: { delayInMinutes: 1, periodInMinutes: undefined }, createInfo: { delayInMinutes: 1, periodInMinutes: undefined },
}), }),
]; ];
activeUserIdMock$ = new BehaviorSubject("user-uuid" as UserId);
logService = mock<ConsoleLogService>(); logService = mock<ConsoleLogService>();
stateProvider = mock<StateProvider>({ stateProvider = mock<StateProvider>({
activeUserId$: activeUserIdMock$,
getGlobal: jest.fn(() => getGlobal: jest.fn(() =>
mock<GlobalState<any>>({ mock<GlobalState<any>>({
state$: mock<Observable<any>>(), state$: mock<Observable<any>>(),
@ -108,7 +112,7 @@ describe("BrowserTaskSchedulerService", () => {
const callback = jest.fn(); const callback = jest.fn();
const delayInMs = 999; const delayInMs = 999;
jest.spyOn(globalThis, "setTimeout"); jest.spyOn(globalThis, "setTimeout");
browserTaskSchedulerService.registerTaskHandler( await browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout, ScheduledTaskNames.loginStrategySessionTimeout,
callback, callback,
); );
@ -132,7 +136,7 @@ describe("BrowserTaskSchedulerService", () => {
ScheduledTaskNames.loginStrategySessionTimeout, ScheduledTaskNames.loginStrategySessionTimeout,
); );
const callback = jest.fn(); const callback = jest.fn();
browserTaskSchedulerService.registerTaskHandler( await browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout, ScheduledTaskNames.loginStrategySessionTimeout,
callback, callback,
); );
@ -153,7 +157,7 @@ describe("BrowserTaskSchedulerService", () => {
it("creates a timeout alarm", async () => { it("creates a timeout alarm", async () => {
const callback = jest.fn(); const callback = jest.fn();
const delayInMinutes = 2; const delayInMinutes = 2;
browserTaskSchedulerService.registerTaskHandler( await browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout, ScheduledTaskNames.loginStrategySessionTimeout,
callback, callback,
); );
@ -179,7 +183,7 @@ describe("BrowserTaskSchedulerService", () => {
}), }),
); );
jest.spyOn(browserTaskSchedulerService, "createAlarm"); jest.spyOn(browserTaskSchedulerService, "createAlarm");
browserTaskSchedulerService.registerTaskHandler( await browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout, ScheduledTaskNames.loginStrategySessionTimeout,
callback, callback,
); );
@ -210,7 +214,7 @@ describe("BrowserTaskSchedulerService", () => {
const callback = jest.fn(); const callback = jest.fn();
const intervalInMs = 999; const intervalInMs = 999;
jest.spyOn(globalThis, "setInterval"); jest.spyOn(globalThis, "setInterval");
browserTaskSchedulerService.registerTaskHandler( await browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout, ScheduledTaskNames.loginStrategySessionTimeout,
callback, callback,
); );
@ -235,7 +239,7 @@ describe("BrowserTaskSchedulerService", () => {
ScheduledTaskNames.loginStrategySessionTimeout, ScheduledTaskNames.loginStrategySessionTimeout,
); );
const callback = jest.fn(); const callback = jest.fn();
browserTaskSchedulerService.registerTaskHandler( await browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout, ScheduledTaskNames.loginStrategySessionTimeout,
callback, callback,
); );
@ -257,7 +261,7 @@ describe("BrowserTaskSchedulerService", () => {
const callback = jest.fn(); const callback = jest.fn();
const periodInMinutes = 2; const periodInMinutes = 2;
const initialDelayInMs = 1000; const initialDelayInMs = 1000;
browserTaskSchedulerService.registerTaskHandler( await browserTaskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout, ScheduledTaskNames.loginStrategySessionTimeout,
callback, callback,
); );

View File

@ -30,11 +30,8 @@ export class BrowserTaskSchedulerService
readonly activeAlarms$: Observable<ActiveAlarm[]>; readonly activeAlarms$: Observable<ActiveAlarm[]>;
private recoveredAlarms: Set<string> = new Set(); private recoveredAlarms: Set<string> = new Set();
constructor( constructor(logService: LogService, stateProvider: StateProvider) {
logService: LogService, super(logService, stateProvider);
private stateProvider: StateProvider,
) {
super(logService);
this.activeAlarmsState = this.stateProvider.getGlobal(ACTIVE_ALARMS); this.activeAlarmsState = this.stateProvider.getGlobal(ACTIVE_ALARMS);
this.activeAlarms$ = this.activeAlarmsState.state$.pipe( this.activeAlarms$ = this.activeAlarmsState.state$.pipe(

View File

@ -255,7 +255,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({ safeProvider({
provide: TaskSchedulerService, provide: TaskSchedulerService,
useClass: DefaultTaskSchedulerService, useClass: DefaultTaskSchedulerService,
deps: [LogServiceAbstraction], deps: [LogServiceAbstraction, StateProvider],
}), }),
]; ];

View File

@ -1130,7 +1130,7 @@ const safeProviders: SafeProvider[] = [
safeProvider({ safeProvider({
provide: TaskSchedulerService, provide: TaskSchedulerService,
useClass: DefaultTaskSchedulerService, useClass: DefaultTaskSchedulerService,
deps: [LogService], deps: [LogService, StateProvider],
}), }),
safeProvider({ safeProvider({
provide: ProviderApiServiceAbstraction, provide: ProviderApiServiceAbstraction,

View File

@ -115,7 +115,7 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
this.authRequestPushNotificationState = this.stateProvider.get( this.authRequestPushNotificationState = this.stateProvider.get(
AUTH_REQUEST_PUSH_NOTIFICATION_KEY, AUTH_REQUEST_PUSH_NOTIFICATION_KEY,
); );
this.taskSchedulerService.registerTaskHandler( void this.taskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout, ScheduledTaskNames.loginStrategySessionTimeout,
() => this.clearCache(), () => this.clearCache(),
); );

View File

@ -1,4 +1,5 @@
import { ScheduledTaskName } from "../enums/scheduled-task-name.enum"; import { ScheduledTaskName } from "../enums/scheduled-task-name.enum";
import { StateProvider } from "../state";
import { LogService } from "./log.service"; import { LogService } from "./log.service";
@ -11,11 +12,14 @@ export type TaskIdentifier = {
export abstract class TaskSchedulerService { export abstract class TaskSchedulerService {
protected taskHandlers: Map<string, () => void>; protected taskHandlers: Map<string, () => void>;
constructor(protected logService: LogService) {} constructor(
protected logService: LogService,
protected stateProvider: StateProvider,
) {}
abstract registerTaskHandler(taskName: ScheduledTaskName, handler: () => void): void; abstract registerTaskHandler(taskName: ScheduledTaskName, handler: () => void): Promise<void>;
abstract unregisterTaskHandler(taskName: ScheduledTaskName): void; abstract unregisterTaskHandler(taskName: ScheduledTaskName): Promise<void>;
abstract setTimeout( abstract setTimeout(
taskName: ScheduledTaskName, taskName: ScheduledTaskName,

View File

@ -1,14 +1,34 @@
import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject } from "rxjs";
import { UserId } from "../../../../common/src/types/guid";
import { LogService } from "../abstractions/log.service";
import { ScheduledTaskNames } from "../enums/scheduled-task-name.enum";
import { StateProvider } from "../state";
import { DefaultTaskSchedulerService } from "./default-task-scheduler.service"; import { DefaultTaskSchedulerService } from "./default-task-scheduler.service";
describe("TaskSchedulerService", () => { describe("TaskSchedulerService", () => {
const callback = jest.fn(); const callback = jest.fn();
const delayInMs = 1000; const delayInMs = 1000;
const intervalInMs = 1100; const intervalInMs = 1100;
let activeUserIdMock$: BehaviorSubject<UserId>;
let logService: MockProxy<LogService>;
let stateProvider: MockProxy<StateProvider>;
let taskSchedulerService: DefaultTaskSchedulerService; let taskSchedulerService: DefaultTaskSchedulerService;
beforeEach(() => { beforeEach(() => {
jest.useFakeTimers(); jest.useFakeTimers();
taskSchedulerService = new DefaultTaskSchedulerService(); activeUserIdMock$ = new BehaviorSubject<UserId>("user-uuid" as UserId);
logService = mock<LogService>();
stateProvider = mock<StateProvider>({
activeUserId$: activeUserIdMock$,
});
taskSchedulerService = new DefaultTaskSchedulerService(logService, stateProvider);
void taskSchedulerService.registerTaskHandler(
ScheduledTaskNames.loginStrategySessionTimeout,
callback,
);
}); });
afterEach(() => { afterEach(() => {
@ -17,7 +37,10 @@ describe("TaskSchedulerService", () => {
}); });
it("sets a timeout and returns the timeout id", async () => { it("sets a timeout and returns the timeout id", async () => {
const timeoutId = await taskSchedulerService.setTimeout(callback, delayInMs); const timeoutId = await taskSchedulerService.setTimeout(
ScheduledTaskNames.loginStrategySessionTimeout,
delayInMs,
);
expect(timeoutId).toBeDefined(); expect(timeoutId).toBeDefined();
expect(callback).not.toHaveBeenCalled(); expect(callback).not.toHaveBeenCalled();
@ -28,7 +51,10 @@ describe("TaskSchedulerService", () => {
}); });
it("sets an interval timeout and results the interval id", async () => { it("sets an interval timeout and results the interval id", async () => {
const intervalId = await taskSchedulerService.setInterval(callback, intervalInMs); const intervalId = await taskSchedulerService.setInterval(
ScheduledTaskNames.loginStrategySessionTimeout,
intervalInMs,
);
expect(intervalId).toBeDefined(); expect(intervalId).toBeDefined();
expect(callback).not.toHaveBeenCalled(); expect(callback).not.toHaveBeenCalled();
@ -43,7 +69,10 @@ describe("TaskSchedulerService", () => {
}); });
it("clears scheduled tasks using the timeout id", async () => { it("clears scheduled tasks using the timeout id", async () => {
const timeoutId = await taskSchedulerService.setTimeout(callback, delayInMs); const timeoutId = await taskSchedulerService.setTimeout(
ScheduledTaskNames.loginStrategySessionTimeout,
delayInMs,
);
expect(timeoutId).toBeDefined(); expect(timeoutId).toBeDefined();
expect(callback).not.toHaveBeenCalled(); expect(callback).not.toHaveBeenCalled();
@ -56,7 +85,10 @@ describe("TaskSchedulerService", () => {
}); });
it("clears scheduled tasks using the interval id", async () => { it("clears scheduled tasks using the interval id", async () => {
const intervalId = await taskSchedulerService.setInterval(callback, intervalInMs); const intervalId = await taskSchedulerService.setInterval(
ScheduledTaskNames.loginStrategySessionTimeout,
intervalInMs,
);
expect(intervalId).toBeDefined(); expect(intervalId).toBeDefined();
expect(callback).not.toHaveBeenCalled(); expect(callback).not.toHaveBeenCalled();

View File

@ -1,26 +1,31 @@
import { firstValueFrom } from "rxjs";
import { LogService } from "../abstractions/log.service"; import { LogService } from "../abstractions/log.service";
import { TaskIdentifier, TaskSchedulerService } from "../abstractions/task-scheduler.service"; import { TaskIdentifier, TaskSchedulerService } from "../abstractions/task-scheduler.service";
import { ScheduledTaskName } from "../enums/scheduled-task-name.enum"; import { ScheduledTaskName } from "../enums/scheduled-task-name.enum";
import { StateProvider } from "../state";
export class DefaultTaskSchedulerService extends TaskSchedulerService { export class DefaultTaskSchedulerService extends TaskSchedulerService {
constructor(logService: LogService) { constructor(logService: LogService, stateProvider: StateProvider) {
super(logService); super(logService, stateProvider);
this.taskHandlers = new Map(); this.taskHandlers = new Map();
} }
registerTaskHandler(taskName: ScheduledTaskName, handler: () => void): void { async registerTaskHandler(taskName: ScheduledTaskName, handler: () => void): Promise<void> {
const existingHandler = this.taskHandlers.get(taskName); const activeUserTaskName = await this.getActiveUserTaskName(taskName);
const existingHandler = this.taskHandlers.get(activeUserTaskName);
if (existingHandler) { if (existingHandler) {
this.logService.warning(`Task handler for ${taskName} already exists. Overwriting.`); this.logService.warning(`Task handler for ${taskName} already exists. Overwriting.`);
this.unregisterTaskHandler(taskName); await this.unregisterTaskHandler(taskName);
} }
this.taskHandlers.set(taskName, handler); this.taskHandlers.set(activeUserTaskName, handler);
} }
unregisterTaskHandler(taskName: ScheduledTaskName): void { async unregisterTaskHandler(taskName: ScheduledTaskName): Promise<void> {
this.taskHandlers.delete(taskName); const activeUserTaskName = await this.getActiveUserTaskName(taskName);
this.taskHandlers.delete(activeUserTaskName);
} }
protected triggerTask(taskName: ScheduledTaskName, _periodInMinutes?: number): void { protected triggerTask(taskName: ScheduledTaskName, _periodInMinutes?: number): void {
@ -72,4 +77,13 @@ export class DefaultTaskSchedulerService extends TaskSchedulerService {
globalThis.clearInterval(taskIdentifier.intervalId); globalThis.clearInterval(taskIdentifier.intervalId);
} }
} }
private async getActiveUserTaskName(taskName: ScheduledTaskName): Promise<string> {
const activeUserId = await firstValueFrom(this.stateProvider.activeUserId$);
if (!activeUserId) {
return taskName;
}
return `${activeUserId}_${taskName}`;
}
} }

View File

@ -29,7 +29,7 @@ export class SystemService implements SystemServiceAbstraction {
private biometricStateService: BiometricStateService, private biometricStateService: BiometricStateService,
private taskSchedulerService: TaskSchedulerService, private taskSchedulerService: TaskSchedulerService,
) { ) {
this.taskSchedulerService.registerTaskHandler( void this.taskSchedulerService.registerTaskHandler(
ScheduledTaskNames.systemClearClipboardTimeout, ScheduledTaskNames.systemClearClipboardTimeout,
() => this.clearPendingClipboard(), () => this.clearPendingClipboard(),
); );

View File

@ -23,8 +23,9 @@ export class EventUploadService implements EventUploadServiceAbstraction {
private authService: AuthService, private authService: AuthService,
private taskSchedulerService: TaskSchedulerService, private taskSchedulerService: TaskSchedulerService,
) { ) {
this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.eventUploadsInterval, () => void this.taskSchedulerService.registerTaskHandler(
this.uploadEvents(), ScheduledTaskNames.eventUploadsInterval,
() => this.uploadEvents(),
); );
} }

View File

@ -46,7 +46,7 @@ export class NotificationsService implements NotificationsServiceAbstraction {
private messagingService: MessagingService, private messagingService: MessagingService,
private taskSchedulerService: TaskSchedulerService, private taskSchedulerService: TaskSchedulerService,
) { ) {
this.taskSchedulerService.registerTaskHandler( void this.taskSchedulerService.registerTaskHandler(
ScheduledTaskNames.notificationsReconnectTimeout, ScheduledTaskNames.notificationsReconnectTimeout,
() => this.reconnect(this.isSyncingOnReconnect), () => this.reconnect(this.isSyncingOnReconnect),
); );

View File

@ -63,8 +63,9 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
private taskSchedulerService: TaskSchedulerService, private taskSchedulerService: TaskSchedulerService,
private logService?: LogService, private logService?: LogService,
) { ) {
this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.fido2ClientAbortTimeout, () => void this.taskSchedulerService.registerTaskHandler(
this.timeoutAbortController?.abort(), ScheduledTaskNames.fido2ClientAbortTimeout,
() => this.timeoutAbortController?.abort(),
); );
} }