[PM-469] [PM-1325] [PS-1165] [PS-1257] Small refactorings/improvements on the desktop app main (#4704)

* Only pass necessary service to power-monitor

PowerMonitorMain only requires the messagingService instead of a full reference to Main

* Remove never changing constructor params

Window.main has a defaultWidth and defaultHeight that never change, so they do not need to get passed in from outside

hideTitleBar is always true, so there is no need to make it a param

* Remove projectName from updater

This is likely another relict from sharing this previously with dircetory-connector and is not needed anymore

* Only pass necessary service to MenuMain

MenuMain only needs service references instead of a full reference to Main

* Refactor biometrics service

Create BiometricsService that takes care of loading the platformspecifc services, hiding the implementation details
Make it clearer which dependencies are needed by a specific biometrics-service (compile-error vs runtime-error)
Add unit tests
Isolate biometrics import/exports with a barrel file

* Fix #3148

recordActivity was only getting called when user-activity in the main window is recognized
When using biometrics to unlock, the Windows Hello/TouchID prompt would be focused and no input would be recognised. LastActive would have an old value and the vault would get locked

* Improve reloading with biometrics

* Mock import of desktop-native

* Add mock for "@bitwarden/desktop-native-linux-x64-musl"

* Revert "Add mock for "@bitwarden/desktop-native-linux-x64-musl""

This reverts commit 69771b94bf.

* mock the exports of desktop-native

* Pass process.platform inot BiometricsService
This commit is contained in:
Daniel James Smith 2023-03-10 21:32:26 +01:00 committed by GitHub
parent e480f7cfbc
commit e9d0f75b8a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 219 additions and 78 deletions

View File

@ -156,6 +156,7 @@ export class AppComponent implements OnInit, OnDestroy {
switch (message.command) {
case "loggedIn":
case "unlocked":
this.recordActivity();
this.notificationsService.updateConnection();
this.updateAppMenu();
this.systemService.cancelProcessReload();
@ -198,6 +199,12 @@ export class AppComponent implements OnInit, OnDestroy {
await this.systemService.clearPendingClipboard();
await this.systemService.startProcessReload(this.authService);
break;
case "startProcessReload":
this.systemService.startProcessReload(this.authService);
break;
case "cancelProcessReload":
this.systemService.cancelProcessReload();
break;
case "reloadProcess":
(window.location as any).reload(true);
break;

View File

@ -7,7 +7,7 @@ import { GlobalState } from "@bitwarden/common/models/domain/global-state";
import { MemoryStorageService } from "@bitwarden/common/services/memoryStorage.service";
import { StateService } from "@bitwarden/common/services/state.service";
import { BiometricMain } from "./main/biometric/biometric.main";
import { BiometricsService, BiometricsServiceAbstraction } from "./main/biometric/index";
import { DesktopCredentialStorageListener } from "./main/desktop-credential-storage-listener";
import { MenuMain } from "./main/menu/menu.main";
import { MessagingMain } from "./main/messaging.main";
@ -37,7 +37,7 @@ export class Main {
menuMain: MenuMain;
powerMonitorMain: PowerMonitorMain;
trayMain: TrayMain;
biometricMain: BiometricMain;
biometricsService: BiometricsServiceAbstraction;
nativeMessagingMain: NativeMessagingMain;
constructor() {
@ -98,40 +98,37 @@ export class Main {
this.windowMain = new WindowMain(
this.stateService,
this.logService,
true,
undefined,
undefined,
(arg) => this.processDeepLink(arg),
(win) => this.trayMain.setupWindowListeners(win)
);
this.messagingMain = new MessagingMain(this, this.stateService);
this.updaterMain = new UpdaterMain(this.i18nService, this.windowMain, "bitwarden");
this.menuMain = new MenuMain(this);
this.powerMonitorMain = new PowerMonitorMain(this);
this.updaterMain = new UpdaterMain(this.i18nService, this.windowMain);
this.trayMain = new TrayMain(this.windowMain, this.i18nService, this.stateService);
this.messagingService = new ElectronMainMessagingService(this.windowMain, (message) => {
this.messagingMain.onMessage(message);
});
this.powerMonitorMain = new PowerMonitorMain(this.messagingService);
this.menuMain = new MenuMain(
this.i18nService,
this.messagingService,
this.stateService,
this.windowMain,
this.updaterMain
);
if (process.platform === "win32") {
// eslint-disable-next-line
const BiometricWindowsMain = require("./main/biometric/biometric.windows.main").default;
this.biometricMain = new BiometricWindowsMain(
this.i18nService,
this.windowMain,
this.stateService,
this.logService
);
} else if (process.platform === "darwin") {
// eslint-disable-next-line
const BiometricDarwinMain = require("./main/biometric/biometric.darwin.main").default;
this.biometricMain = new BiometricDarwinMain(this.i18nService, this.stateService);
}
this.biometricsService = new BiometricsService(
this.i18nService,
this.windowMain,
this.stateService,
this.logService,
this.messagingService,
process.platform
);
this.desktopCredentialStorageListener = new DesktopCredentialStorageListener(
"Bitwarden",
this.biometricMain
this.biometricsService
);
this.nativeMessagingMain = new NativeMessagingMain(
@ -163,8 +160,8 @@ export class Main {
}
this.powerMonitorMain.init();
await this.updaterMain.init();
if (this.biometricMain != null) {
await this.biometricMain.init();
if (this.biometricsService != null) {
await this.biometricsService.init();
}
if (

View File

@ -3,9 +3,9 @@ import { ipcMain, systemPreferences } from "electron";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
import { StateService } from "@bitwarden/common/abstractions/state.service";
import { BiometricMain } from "../biometric/biometric.main";
import { BiometricsServiceAbstraction } from "./biometrics.service.abstraction";
export default class BiometricDarwinMain implements BiometricMain {
export default class BiometricDarwinMain implements BiometricsServiceAbstraction {
constructor(private i18nservice: I18nService, private stateService: StateService) {}
async init() {

View File

@ -7,9 +7,9 @@ import { biometrics } from "@bitwarden/desktop-native";
import { WindowMain } from "../window.main";
import { BiometricMain } from "./biometric.main";
import { BiometricsServiceAbstraction } from "./biometrics.service.abstraction";
export default class BiometricWindowsMain implements BiometricMain {
export default class BiometricWindowsMain implements BiometricsServiceAbstraction {
constructor(
private i18nservice: I18nService,
private windowMain: WindowMain,

View File

@ -1,4 +1,4 @@
export abstract class BiometricMain {
export abstract class BiometricsServiceAbstraction {
init: () => Promise<void>;
supportsBiometric: () => Promise<boolean>;
authenticateBiometric: () => Promise<boolean>;

View File

@ -0,0 +1,81 @@
import { mock } from "jest-mock-extended";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/abstractions/log.service";
import { MessagingService } from "@bitwarden/common/abstractions/messaging.service";
import { StateService } from "@bitwarden/common/abstractions/state.service";
import { WindowMain } from "../window.main";
import BiometricDarwinMain from "./biometric.darwin.main";
import BiometricWindowsMain from "./biometric.windows.main";
import { BiometricsService } from "./biometrics.service";
import { BiometricsServiceAbstraction } from "./biometrics.service.abstraction";
jest.mock("@bitwarden/desktop-native", () => {
return {
biometrics: jest.fn(),
passwords: jest.fn(),
};
});
describe("biometrics tests", function () {
const i18nService = mock<I18nService>();
const windowMain = mock<WindowMain>();
const stateService = mock<StateService>();
const logService = mock<LogService>();
const messagingService = mock<MessagingService>();
it("Should call the platformspecific methods", () => {
const sut = new BiometricsService(
i18nService,
windowMain,
stateService,
logService,
messagingService,
process.platform
);
const mockService = mock<BiometricsServiceAbstraction>();
(sut as any).platformSpecificService = mockService;
sut.init();
expect(mockService.init).toBeCalled();
sut.supportsBiometric();
expect(mockService.supportsBiometric).toBeCalled();
sut.authenticateBiometric();
expect(mockService.authenticateBiometric).toBeCalled();
});
describe("Should create a platform specific service", function () {
it("Should create a biometrics service specific for Windows", () => {
const sut = new BiometricsService(
i18nService,
windowMain,
stateService,
logService,
messagingService,
"win32"
);
const internalService = (sut as any).platformSpecificService;
expect(internalService).not.toBeNull();
expect(internalService).toBeInstanceOf(BiometricWindowsMain);
});
it("Should create a biometrics service specific for MacOs", () => {
const sut = new BiometricsService(
i18nService,
windowMain,
stateService,
logService,
messagingService,
"darwin"
);
const internalService = (sut as any).platformSpecificService;
expect(internalService).not.toBeNull();
expect(internalService).toBeInstanceOf(BiometricDarwinMain);
});
});
});

View File

@ -0,0 +1,65 @@
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/abstractions/log.service";
import { MessagingService } from "@bitwarden/common/abstractions/messaging.service";
import { StateService } from "@bitwarden/common/abstractions/state.service";
import { WindowMain } from "../window.main";
import { BiometricsServiceAbstraction } from "./biometrics.service.abstraction";
export class BiometricsService implements BiometricsServiceAbstraction {
private platformSpecificService: BiometricsServiceAbstraction;
constructor(
private i18nService: I18nService,
private windowMain: WindowMain,
private stateService: StateService,
private logService: LogService,
private messagingService: MessagingService,
private platform: NodeJS.Platform
) {
this.loadPlatformSpecificService(this.platform);
}
private loadPlatformSpecificService(platform: NodeJS.Platform) {
if (platform === "win32") {
this.loadWindowsHelloService();
} else if (platform === "darwin") {
this.loadMacOSService();
}
}
private loadWindowsHelloService() {
// eslint-disable-next-line
const BiometricWindowsMain = require("./biometric.windows.main").default;
this.platformSpecificService = new BiometricWindowsMain(
this.i18nService,
this.windowMain,
this.stateService,
this.logService
);
}
private loadMacOSService() {
// eslint-disable-next-line
const BiometricDarwinMain = require("./biometric.darwin.main").default;
this.platformSpecificService = new BiometricDarwinMain(this.i18nService, this.stateService);
}
async init() {
return await this.platformSpecificService.init();
}
async supportsBiometric(): Promise<boolean> {
return await this.platformSpecificService.supportsBiometric();
}
async authenticateBiometric(): Promise<boolean> {
this.messagingService.send("cancelProcessReload");
const response = await this.platformSpecificService.authenticateBiometric();
if (!response) {
this.messagingService.send("startProcessReload");
}
return response;
}
}

View File

@ -0,0 +1,2 @@
export * from "./biometrics.service.abstraction";
export * from "./biometrics.service";

View File

@ -2,13 +2,16 @@ import { ipcMain } from "electron";
import { passwords } from "@bitwarden/desktop-native";
import { BiometricMain } from "./biometric/biometric.main";
import { BiometricsServiceAbstraction } from "./biometric/index";
const AuthRequiredSuffix = "_biometric";
const AuthenticatedActions = ["getPassword"];
export class DesktopCredentialStorageListener {
constructor(private serviceName: string, private biometricService: BiometricMain) {}
constructor(
private serviceName: string,
private biometricService: BiometricsServiceAbstraction
) {}
init() {
ipcMain.handle("keytar", async (event: any, message: any) => {

View File

@ -1,8 +1,10 @@
import { app, Menu } from "electron";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
import { MessagingService } from "@bitwarden/common/abstractions/messaging.service";
import { StateService } from "@bitwarden/common/abstractions/state.service";
import { Main } from "../../main";
import { UpdaterMain } from "../updater.main";
import { WindowMain } from "../window.main";
import { MenuUpdateRequest } from "./menu.updater";
@ -11,13 +13,13 @@ import { Menubar } from "./menubar";
const cloudWebVaultUrl = "https://vault.bitwarden.com";
export class MenuMain {
private i18nService: I18nService;
private windowMain: WindowMain;
constructor(private main: Main) {
this.i18nService = main.i18nService;
this.windowMain = main.windowMain;
}
constructor(
private i18nService: I18nService,
private messagingService: MessagingService,
private stateService: StateService,
private windowMain: WindowMain,
private updaterMain: UpdaterMain
) {}
async init() {
this.initContextMenu();
@ -31,9 +33,9 @@ export class MenuMain {
private async setMenu(updateRequest?: MenuUpdateRequest) {
Menu.setApplicationMenu(
new Menubar(
this.main.i18nService,
this.main.messagingService,
this.main.updaterMain,
this.i18nService,
this.messagingService,
this.updaterMain,
this.windowMain,
await this.getWebVaultUrl(),
app.getVersion(),
@ -44,7 +46,7 @@ export class MenuMain {
private async getWebVaultUrl() {
let webVaultUrl = cloudWebVaultUrl;
const urlsObj: any = await this.main.stateService.getEnvironmentUrls();
const urlsObj = await this.stateService.getEnvironmentUrls();
if (urlsObj != null) {
if (urlsObj.base != null) {
webVaultUrl = urlsObj.base;

View File

@ -1,6 +1,6 @@
import { powerMonitor } from "electron";
import { Main } from "../main";
import { ElectronMainMessagingService } from "../services/electron-main-messaging.service";
import { isSnapStore } from "../utils";
// tslint:disable-next-line
@ -10,21 +10,21 @@ const IdleCheckInterval = 30 * 1000; // 30 seconds
export class PowerMonitorMain {
private idle = false;
constructor(private main: Main) {}
constructor(private messagingService: ElectronMainMessagingService) {}
init() {
// ref: https://github.com/electron/electron/issues/13767
if (!isSnapStore()) {
// System sleep
powerMonitor.on("suspend", () => {
this.main.messagingService.send("systemSuspended");
this.messagingService.send("systemSuspended");
});
}
if (process.platform !== "linux") {
// System locked
powerMonitor.on("lock-screen", () => {
this.main.messagingService.send("systemLocked");
this.messagingService.send("systemLocked");
});
}
@ -37,7 +37,7 @@ export class PowerMonitorMain {
return;
}
this.main.messagingService.send("systemIdle");
this.messagingService.send("systemIdle");
}
this.idle = idle;

View File

@ -16,11 +16,7 @@ export class UpdaterMain {
private doingUpdateCheckWithFeedback = false;
private canUpdate = false;
constructor(
private i18nService: I18nService,
private windowMain: WindowMain,
private projectName: string
) {
constructor(private i18nService: I18nService, private windowMain: WindowMain) {
autoUpdater.logger = log;
const linuxCanUpdate = process.platform === "linux" && isAppImage();
@ -49,8 +45,7 @@ export class UpdaterMain {
const result = await dialog.showMessageBox(this.windowMain.win, {
type: "info",
title:
this.i18nService.t(this.projectName) + " - " + this.i18nService.t("updateAvailable"),
title: this.i18nService.t("bitwarden") + " - " + this.i18nService.t("updateAvailable"),
message: this.i18nService.t("updateAvailable"),
detail: this.i18nService.t("updateAvailableDesc"),
buttons: [this.i18nService.t("yes"), this.i18nService.t("no")],
@ -87,7 +82,7 @@ export class UpdaterMain {
const result = await dialog.showMessageBox(this.windowMain.win, {
type: "info",
title: this.i18nService.t(this.projectName) + " - " + this.i18nService.t("restartToUpdate"),
title: this.i18nService.t("bitwarden") + " - " + this.i18nService.t("restartToUpdate"),
message: this.i18nService.t("restartToUpdate"),
detail: this.i18nService.t("restartToUpdateDesc", info.version),
buttons: [this.i18nService.t("restart"), this.i18nService.t("later")],

View File

@ -20,12 +20,12 @@ export class WindowMain {
private windowStates: { [key: string]: WindowState } = {};
private enableAlwaysOnTop = false;
readonly defaultWidth = 950;
readonly defaultHeight = 600;
constructor(
private stateService: StateService,
private logService: LogService,
private hideTitleBar = false,
private defaultWidth = 950,
private defaultHeight = 600,
private argvCallback: (argv: string[]) => void = null,
private createWindowCallback: (win: BrowserWindow) => void
) {}
@ -118,7 +118,7 @@ export class WindowMain {
y: this.windowStates[mainWindowSizeKey].y,
title: app.name,
icon: process.platform === "linux" ? path.join(__dirname, "/images/icon.png") : undefined,
titleBarStyle: this.hideTitleBar && process.platform === "darwin" ? "hiddenInset" : undefined,
titleBarStyle: process.platform === "darwin" ? "hiddenInset" : undefined,
show: false,
backgroundColor: "#fff",
alwaysOnTop: this.enableAlwaysOnTop,

View File

@ -45,34 +45,23 @@ export class SystemService implements SystemServiceAbstraction {
}
this.cancelProcessReload();
this.reloadInterval = setInterval(async () => await this.executeProcessReload(), 10000);
}
private async inactiveMoreThanSeconds(seconds: number): Promise<boolean> {
const lastActive = await this.stateService.getLastActive();
if (lastActive != null) {
const diffMs = new Date().getTime() - lastActive;
return diffMs >= seconds * 1000;
}
return true;
await this.executeProcessReload();
}
private async executeProcessReload() {
const accounts = await firstValueFrom(this.stateService.accounts$);
const doRefresh =
accounts == null ||
Object.keys(accounts).length == 0 ||
(await this.inactiveMoreThanSeconds(5));
const biometricLockedFingerprintValidated =
await this.stateService.getBiometricFingerprintValidated();
if (doRefresh && !biometricLockedFingerprintValidated) {
if (!biometricLockedFingerprintValidated) {
clearInterval(this.reloadInterval);
this.reloadInterval = null;
this.messagingService.send("reloadProcess");
if (this.reloadCallback != null) {
await this.reloadCallback();
}
return;
}
if (this.reloadInterval == null) {
this.reloadInterval = setInterval(async () => await this.executeProcessReload(), 1000);
}
}