Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-7541] Move Last Desktop Settings #9310

Merged
merged 14 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 15 additions & 7 deletions apps/desktop/src/app/accounts/settings.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,17 +278,20 @@
approveLoginRequests:
(await this.authRequestService.getAcceptAuthRequests(this.currentUserId)) ?? false,
clearClipboard: await firstValueFrom(this.autofillSettingsService.clearClipboardDelay$),
minimizeOnCopyToClipboard: await this.stateService.getMinimizeOnCopyToClipboard(),
minimizeOnCopyToClipboard: await firstValueFrom(this.desktopSettingsService.minimizeOnCopy$),
enableFavicons: await firstValueFrom(this.domainSettingsService.showFavicons$),
enableTray: await firstValueFrom(this.desktopSettingsService.trayEnabled$),
enableMinToTray: await firstValueFrom(this.desktopSettingsService.minimizeToTray$),
enableCloseToTray: await firstValueFrom(this.desktopSettingsService.closeToTray$),
startToTray: await firstValueFrom(this.desktopSettingsService.startToTray$),
openAtLogin: await firstValueFrom(this.desktopSettingsService.openAtLogin$),
alwaysShowDock: await firstValueFrom(this.desktopSettingsService.alwaysShowDock$),
enableBrowserIntegration: await this.stateService.getEnableBrowserIntegration(),
enableBrowserIntegrationFingerprint:
await this.stateService.getEnableBrowserIntegrationFingerprint(),
enableBrowserIntegration: await firstValueFrom(
this.desktopSettingsService.browserIntegrationEnabled$,
),
enableBrowserIntegrationFingerprint: await firstValueFrom(
this.desktopSettingsService.browserIntegrationFingerprintEnabled$,
),

Check warning on line 294 in apps/desktop/src/app/accounts/settings.component.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ Getting worse: Complex Method

SettingsComponent.ngOnInit already has high cyclomatic complexity, and now it increases in Lines of Code from 135 to 138. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
enableDuckDuckGoBrowserIntegration: await firstValueFrom(
this.desktopAutofillSettingsService.enableDuckDuckGoBrowserIntegration$,
),
Expand Down Expand Up @@ -598,7 +601,10 @@
}

async saveMinOnCopyToClipboard() {
await this.stateService.setMinimizeOnCopyToClipboard(this.form.value.minimizeOnCopyToClipboard);
await this.desktopSettingsService.setMinimizeOnCopy(

Check warning on line 604 in apps/desktop/src/app/accounts/settings.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/accounts/settings.component.ts#L604

Added line #L604 was not covered by tests
this.form.value.minimizeOnCopyToClipboard,
this.currentUserId,
);
}

async saveClearClipboard() {
Expand Down Expand Up @@ -656,7 +662,9 @@
return;
}

await this.stateService.setEnableBrowserIntegration(this.form.value.enableBrowserIntegration);
await this.desktopSettingsService.setBrowserIntegrationEnabled(

Check warning on line 665 in apps/desktop/src/app/accounts/settings.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/accounts/settings.component.ts#L665

Added line #L665 was not covered by tests
this.form.value.enableBrowserIntegration,
);

const errorResult = await this.nativeMessagingManifestService.generate(
this.form.value.enableBrowserIntegration,
Expand Down Expand Up @@ -703,7 +711,7 @@
}

async saveBrowserIntegrationFingerprint() {
await this.stateService.setEnableBrowserIntegrationFingerprint(
await this.desktopSettingsService.setBrowserIntegrationFingerprintEnabled(

Check warning on line 714 in apps/desktop/src/app/accounts/settings.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/app/accounts/settings.component.ts#L714

Added line #L714 was not covered by tests
this.form.value.enableBrowserIntegrationFingerprint,
);
}
Expand Down
71 changes: 9 additions & 62 deletions apps/desktop/src/main.ts
justindbaur marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,13 @@
import { app } from "electron";
import { Subject, firstValueFrom } from "rxjs";

import { TokenService as TokenServiceAbstraction } from "@bitwarden/common/auth/abstractions/token.service";
import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service";
import { TokenService } from "@bitwarden/common/auth/services/token.service";
import { ClientType } from "@bitwarden/common/enums";
import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
import { KeyGenerationService as KeyGenerationServiceAbstraction } from "@bitwarden/common/platform/abstractions/key-generation.service";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { DefaultBiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service";
import { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
import { Message, MessageSender } from "@bitwarden/common/platform/messaging";
// eslint-disable-next-line no-restricted-imports -- For dependency creation
import { SubjectMessageSender } from "@bitwarden/common/platform/messaging/internal";
import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state";
import { EncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/encrypt.service.implementation";
import { DefaultEnvironmentService } from "@bitwarden/common/platform/services/default-environment.service";
import { KeyGenerationService } from "@bitwarden/common/platform/services/key-generation.service";
import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service";
import { MigrationBuilderService } from "@bitwarden/common/platform/services/migration-builder.service";
import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner";
Expand All @@ -41,18 +32,13 @@
import { TrayMain } from "./main/tray.main";
import { UpdaterMain } from "./main/updater.main";
import { WindowMain } from "./main/window.main";
import { Account } from "./models/account";
import { BiometricsService, BiometricsServiceAbstraction } from "./platform/main/biometric/index";
import { ClipboardMain } from "./platform/main/clipboard.main";
import { DesktopCredentialStorageListener } from "./platform/main/desktop-credential-storage-listener";
import { MainCryptoFunctionService } from "./platform/main/main-crypto-function.service";
import { DesktopSettingsService } from "./platform/services/desktop-settings.service";
import { ElectronLogMainService } from "./platform/services/electron-log.main.service";
import { ELECTRON_SUPPORTS_SECURE_STORAGE } from "./platform/services/electron-platform-utils.service";
import { ElectronStateService } from "./platform/services/electron-state.service";
import { ElectronStorageService } from "./platform/services/electron-storage.service";
import { I18nMainService } from "./platform/services/i18n.main.service";
import { IllegalSecureStorageService } from "./platform/services/illegal-secure-storage.service";
import { ElectronMainMessagingService } from "./services/electron-main-messaging.service";
import { isMacAppStore } from "./utils";

Expand All @@ -63,15 +49,10 @@
memoryStorageService: MemoryStorageService;
memoryStorageForStateProviders: MemoryStorageServiceForStateProviders;
messagingService: MessageSender;
stateService: StateService;
environmentService: DefaultEnvironmentService;
mainCryptoFunctionService: MainCryptoFunctionService;
desktopCredentialStorageListener: DesktopCredentialStorageListener;
desktopSettingsService: DesktopSettingsService;
migrationRunner: MigrationRunner;
tokenService: TokenServiceAbstraction;
keyGenerationService: KeyGenerationServiceAbstraction;
encryptService: EncryptService;

windowMain: WindowMain;
messagingMain: MessagingMain;
Expand Down Expand Up @@ -159,67 +140,27 @@
);

this.environmentService = new DefaultEnvironmentService(stateProvider, accountService);

this.mainCryptoFunctionService = new MainCryptoFunctionService();
this.mainCryptoFunctionService.init();

this.keyGenerationService = new KeyGenerationService(this.mainCryptoFunctionService);

this.encryptService = new EncryptServiceImplementation(
this.mainCryptoFunctionService,
this.logService,
true, // log mac failures
);

// Note: secure storage service is not available and should not be called in the main background process.
const illegalSecureStorageService = new IllegalSecureStorageService();

this.tokenService = new TokenService(
singleUserStateProvider,
globalStateProvider,
ELECTRON_SUPPORTS_SECURE_STORAGE,
illegalSecureStorageService,
this.keyGenerationService,
this.encryptService,
this.logService,
);

this.migrationRunner = new MigrationRunner(
this.storageService,
this.logService,
new MigrationBuilderService(),
ClientType.Desktop,
);

// TODO: this state service will have access to on disk storage, but not in memory storage.
// If we could get this to work using the stateService singleton that the rest of the app uses we could save
// ourselves from some hacks, like having to manually update the app menu vs. the menu subscribing to events.
this.stateService = new ElectronStateService(
this.storageService,
null,
this.memoryStorageService,
this.logService,
new StateFactory(GlobalState, Account),
accountService, // will not broadcast logouts. This is a hack until we can remove messaging dependency
this.environmentService,
this.tokenService,
this.migrationRunner,
);

this.desktopSettingsService = new DesktopSettingsService(stateProvider);

const biometricStateService = new DefaultBiometricStateService(stateProvider);

this.windowMain = new WindowMain(
this.stateService,
biometricStateService,
this.logService,
this.storageService,
this.desktopSettingsService,
(arg) => this.processDeepLink(arg),
(win) => this.trayMain.setupWindowListeners(win),
);
this.messagingMain = new MessagingMain(this, this.stateService, this.desktopSettingsService);
this.messagingMain = new MessagingMain(this, this.desktopSettingsService);

Check warning on line 163 in apps/desktop/src/main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main.ts#L163

Added line #L163 was not covered by tests
this.updaterMain = new UpdaterMain(this.i18nService, this.windowMain);
this.trayMain = new TrayMain(this.windowMain, this.i18nService, this.desktopSettingsService);

Expand All @@ -230,7 +171,13 @@
);

messageSubject.asObservable().subscribe((message) => {
this.messagingMain.onMessage(message);
void this.messagingMain.onMessage(message).catch((err) => {
this.logService.error(

Check warning on line 175 in apps/desktop/src/main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main.ts#L174-L175

Added lines #L174 - L175 were not covered by tests
"Error while handling message",
message?.command ?? "Unknown command",
err,
);
});

Check notice on line 180 in apps/desktop/src/main.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ Getting worse: Complex Method

Main.constructor increases in cyclomatic complexity from 9 to 11, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
});

this.powerMonitorMain = new PowerMonitorMain(this.messagingService);
Expand Down Expand Up @@ -298,7 +245,7 @@
await this.updaterMain.init();

const [browserIntegrationEnabled, ddgIntegrationEnabled] = await Promise.all([
this.stateService.getEnableBrowserIntegration(),
firstValueFrom(this.desktopSettingsService.browserIntegrationEnabled$),
firstValueFrom(this.desktopAutofillSettingsService.enableDuckDuckGoBrowserIntegration$),
]);

Expand Down
22 changes: 12 additions & 10 deletions apps/desktop/src/main/messaging.main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
import * as path from "path";

import { app, ipcMain } from "electron";

import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { firstValueFrom } from "rxjs";

Check warning on line 5 in apps/desktop/src/main/messaging.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/messaging.main.ts#L5

Added line #L5 was not covered by tests

import { Main } from "../main";
import { DesktopSettingsService } from "../platform/services/desktop-settings.service";
Expand All @@ -17,7 +16,6 @@

constructor(
private main: Main,
private stateService: StateService,
private desktopSettingsService: DesktopSettingsService,
) {}

Expand All @@ -29,10 +27,13 @@
const loginSettings = app.getLoginItemSettings();
await this.desktopSettingsService.setOpenAtLogin(loginSettings.openAtLogin);
}
ipcMain.on("messagingService", async (event: any, message: any) => this.onMessage(message));
ipcMain.on(

Check warning on line 30 in apps/desktop/src/main/messaging.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/messaging.main.ts#L30

Added line #L30 was not covered by tests
"messagingService",
async (event: any, message: any) => await this.onMessage(message),

Check warning on line 32 in apps/desktop/src/main/messaging.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/messaging.main.ts#L32

Added line #L32 was not covered by tests
);
}

onMessage(message: any) {
async onMessage(message: any) {
switch (message.command) {
case "scheduleNextSync":
this.scheduleNextSync();
Expand All @@ -44,13 +45,14 @@
this.updateTrayMenu(message.updateRequest);
break;
case "minimizeOnCopy":
// 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
this.stateService.getMinimizeOnCopyToClipboard().then((shouldMinimize) => {
if (shouldMinimize && this.main.windowMain.win !== null) {
{
const shouldMinimizeOnCopy = await firstValueFrom(

Check warning on line 49 in apps/desktop/src/main/messaging.main.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/main/messaging.main.ts#L49

Added line #L49 was not covered by tests
this.desktopSettingsService.minimizeOnCopy$,
);
if (shouldMinimizeOnCopy && this.main.windowMain.win !== null) {
this.main.windowMain.win.minimize();
}
});
}
break;
case "showTray":
this.main.trayMain.showTray();
Expand Down
2 changes: 0 additions & 2 deletions apps/desktop/src/main/window.main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { app, BrowserWindow, ipcMain, nativeTheme, screen, session } from "elect
import { firstValueFrom } from "rxjs";

import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service";
import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service";

Expand Down Expand Up @@ -38,7 +37,6 @@ export class WindowMain {
readonly defaultHeight = 600;

constructor(
private stateService: StateService,
private biometricStateService: BiometricStateService,
private logService: LogService,
private storageService: AbstractStorageService,
Expand Down