From c592bcba80a41ebfa946885f65b7bcf307775451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Wed, 1 Nov 2023 18:34:36 +0100 Subject: [PATCH] [PM-3683] Remove ipcRenderer from electron-platform-utils (#6679) * [PM-3683] Remove ipcRenderer from electron-platform-utils * FIx review comments * Formatting * Use isNullOrWhitespace --- apps/desktop/src/auth/lock.component.ts | 4 +-- .../desktop-credential-storage-listener.ts | 6 ++-- apps/desktop/src/platform/preload.ts | 23 +++++++++++++ .../electron-platform-utils.service.ts | 19 +++-------- .../electron-main-messaging.service.ts | 19 ++++++++++- apps/desktop/src/types/biometric-message.ts | 5 +-- .../src/platform/misc/safe-urls.spec.ts | 21 ++++++++++++ libs/common/src/platform/misc/safe-urls.ts | 33 +++++++++++++++++++ .../src/vault/models/view/login-uri.view.ts | 28 +++------------- 9 files changed, 112 insertions(+), 46 deletions(-) create mode 100644 libs/common/src/platform/misc/safe-urls.spec.ts create mode 100644 libs/common/src/platform/misc/safe-urls.ts diff --git a/apps/desktop/src/auth/lock.component.ts b/apps/desktop/src/auth/lock.component.ts index abebe235b0e..43e2b5fd072 100644 --- a/apps/desktop/src/auth/lock.component.ts +++ b/apps/desktop/src/auth/lock.component.ts @@ -22,7 +22,7 @@ import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/pass import { DialogService } from "@bitwarden/components"; import { ElectronStateService } from "../platform/services/electron-state.service.abstraction"; -import { BiometricStorageAction, BiometricMessage } from "../types/biometric-message"; +import { BiometricAction, BiometricMessage } from "../types/biometric-message"; const BroadcasterSubscriptionId = "LockComponent"; @@ -133,7 +133,7 @@ export class LockComponent extends BaseLockComponent { private async canUseBiometric() { const userId = await this.stateService.getUserId(); const val = await ipcRenderer.invoke("biometric", { - action: BiometricStorageAction.EnabledForUser, + action: BiometricAction.EnabledForUser, key: `${userId}_user_biometric`, keySuffix: KeySuffixOptions.Biometric, userId: userId, diff --git a/apps/desktop/src/platform/main/desktop-credential-storage-listener.ts b/apps/desktop/src/platform/main/desktop-credential-storage-listener.ts index c1af4fc5113..212aa8792ea 100644 --- a/apps/desktop/src/platform/main/desktop-credential-storage-listener.ts +++ b/apps/desktop/src/platform/main/desktop-credential-storage-listener.ts @@ -4,7 +4,7 @@ import { BiometricKey } from "@bitwarden/common/auth/types/biometric-key"; import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; import { passwords } from "@bitwarden/desktop-native"; -import { BiometricMessage, BiometricStorageAction } from "../../types/biometric-message"; +import { BiometricMessage, BiometricAction } from "../../types/biometric-message"; import { BiometricsServiceAbstraction } from "./biometric/index"; @@ -66,7 +66,7 @@ export class DesktopCredentialStorageListener { } switch (message.action) { - case BiometricStorageAction.EnabledForUser: + case BiometricAction.EnabledForUser: if (!message.key || !message.userId) { break; } @@ -76,7 +76,7 @@ export class DesktopCredentialStorageListener { userId: message.userId, }); break; - case BiometricStorageAction.OsSupported: + case BiometricAction.OsSupported: val = await this.biometricService.osSupportsBiometric(); break; default: diff --git a/apps/desktop/src/platform/preload.ts b/apps/desktop/src/platform/preload.ts index 5430d3fb35a..b796543d859 100644 --- a/apps/desktop/src/platform/preload.ts +++ b/apps/desktop/src/platform/preload.ts @@ -2,8 +2,11 @@ import { ipcRenderer } from "electron"; import { DeviceType, ThemeType } from "@bitwarden/common/enums"; +import { BiometricMessage, BiometricAction } from "../types/biometric-message"; import { isDev, isWindowsStore } from "../utils"; +import { ClipboardWriteMessage } from "./types/clipboard"; + const storage = { get: (key: string): Promise => ipcRenderer.invoke("storageService", { action: "get", key }), has: (key: string): Promise => @@ -25,6 +28,22 @@ const passwords = { ipcRenderer.invoke("keytar", { action: "deletePassword", key, keySuffix }), }; +const biometric = { + osSupported: (): Promise => + ipcRenderer.invoke("biometric", { + action: BiometricAction.OsSupported, + } satisfies BiometricMessage), + authenticate: (): Promise => + ipcRenderer.invoke("biometric", { + action: BiometricAction.Authenticate, + } satisfies BiometricMessage), +}; + +const clipboard = { + read: (): Promise => ipcRenderer.invoke("clipboard.read"), + write: (message: ClipboardWriteMessage) => ipcRenderer.invoke("clipboard.write", message), +}; + export default { versions: { app: (): Promise => ipcRenderer.invoke("appVersion"), @@ -52,8 +71,12 @@ export default { }); }, + launchUri: (uri: string) => ipcRenderer.invoke("launchUri", uri), + storage, passwords, + biometric, + clipboard, }; function deviceType(): DeviceType { diff --git a/apps/desktop/src/platform/services/electron-platform-utils.service.ts b/apps/desktop/src/platform/services/electron-platform-utils.service.ts index 6c99507b12b..d5d259f86a9 100644 --- a/apps/desktop/src/platform/services/electron-platform-utils.service.ts +++ b/apps/desktop/src/platform/services/electron-platform-utils.service.ts @@ -1,5 +1,3 @@ -import { ipcRenderer, shell } from "electron"; - import { ClientType, DeviceType } from "@bitwarden/common/enums"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -8,7 +6,6 @@ import { PlatformUtilsService, } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { BiometricMessage, BiometricStorageAction } from "../../types/biometric-message"; import { isMacAppStore } from "../../utils"; import { ClipboardWriteMessage } from "../types/clipboard"; @@ -61,7 +58,7 @@ export class ElectronPlatformUtilsService implements PlatformUtilsService { } launchUri(uri: string, options?: any): void { - shell.openExternal(uri); + ipc.platform.launchUri(uri); } getApplicationVersion(): Promise { @@ -108,7 +105,7 @@ export class ElectronPlatformUtilsService implements PlatformUtilsService { const clearing = options?.clearing === true; const clearMs = options?.clearMs ?? null; - ipcRenderer.invoke("clipboard.write", { + ipc.platform.clipboard.write({ text: text, password: (options?.allowHistory ?? false) === false, // default to false } satisfies ClipboardWriteMessage); @@ -123,13 +120,11 @@ export class ElectronPlatformUtilsService implements PlatformUtilsService { } readFromClipboard(): Promise { - return ipcRenderer.invoke("clipboard.read"); + return ipc.platform.clipboard.read(); } async supportsBiometric(): Promise { - return await ipcRenderer.invoke("biometric", { - action: BiometricStorageAction.OsSupported, - } as BiometricMessage); + return await ipc.platform.biometric.osSupported(); } /** This method is used to authenticate the user presence _only_. @@ -137,11 +132,7 @@ export class ElectronPlatformUtilsService implements PlatformUtilsService { * biometric keys, which has a separate authentication mechanism. * For biometric keys, invoke "keytar" with a biometric key suffix */ async authenticateBiometric(): Promise { - const val = await ipcRenderer.invoke("biometric", { - action: "authenticate", - }); - - return val; + return await ipc.platform.biometric.authenticate(); } supportsSecureStorage(): boolean { diff --git a/apps/desktop/src/services/electron-main-messaging.service.ts b/apps/desktop/src/services/electron-main-messaging.service.ts index c37fe3ea49f..1c029edc7f5 100644 --- a/apps/desktop/src/services/electron-main-messaging.service.ts +++ b/apps/desktop/src/services/electron-main-messaging.service.ts @@ -1,9 +1,20 @@ import * as path from "path"; -import { app, dialog, ipcMain, Menu, MenuItem, nativeTheme, session, Notification } from "electron"; +import { + app, + dialog, + ipcMain, + Menu, + MenuItem, + nativeTheme, + session, + Notification, + shell, +} from "electron"; import { ThemeType } from "@bitwarden/common/enums"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { SafeUrls } from "@bitwarden/common/platform/misc/safe-urls"; import { WindowMain } from "../main/window.main"; import { RendererMenuItem } from "../utils"; @@ -68,6 +79,12 @@ export class ElectronMainMessagingService implements MessagingService { alert.show(); }); + ipcMain.handle("launchUri", async (event, uri) => { + if (SafeUrls.canLaunch(uri)) { + shell.openExternal(uri); + } + }); + nativeTheme.on("updated", () => { windowMain.win?.webContents.send( "systemThemeUpdated", diff --git a/apps/desktop/src/types/biometric-message.ts b/apps/desktop/src/types/biometric-message.ts index b3fc24f7b89..001aabc1fe2 100644 --- a/apps/desktop/src/types/biometric-message.ts +++ b/apps/desktop/src/types/biometric-message.ts @@ -1,10 +1,11 @@ -export enum BiometricStorageAction { +export enum BiometricAction { EnabledForUser = "enabled", OsSupported = "osSupported", + Authenticate = "authenticate", } export type BiometricMessage = { - action: BiometricStorageAction; + action: BiometricAction; keySuffix?: string; key?: string; userId?: string; diff --git a/libs/common/src/platform/misc/safe-urls.spec.ts b/libs/common/src/platform/misc/safe-urls.spec.ts new file mode 100644 index 00000000000..60689aee12d --- /dev/null +++ b/libs/common/src/platform/misc/safe-urls.spec.ts @@ -0,0 +1,21 @@ +import { SafeUrls } from "./safe-urls"; + +describe("SafeUrls service", () => { + it("should allow valid URLs", () => { + expect(SafeUrls.canLaunch("https://bitwarden.com")).toBe(true); + expect(SafeUrls.canLaunch("http://bitwarden.com")).toBe(true); + expect(SafeUrls.canLaunch("ssh://my-server")).toBe(true); + }); + + it("should fail invalid URLs", () => { + expect(SafeUrls.canLaunch("bitwarden.com")).toBe(false); + expect(SafeUrls.canLaunch("")).toBe(false); + expect(SafeUrls.canLaunch(null)).toBe(false); + }); + + it("should fail URLs with disallowed protocols", () => { + expect(SafeUrls.canLaunch("file:///etc/passwd")).toBe(false); + expect(SafeUrls.canLaunch("\\\\network.share\\abc")).toBe(false); + expect(SafeUrls.canLaunch("smb://smb.server")).toBe(false); + }); +}); diff --git a/libs/common/src/platform/misc/safe-urls.ts b/libs/common/src/platform/misc/safe-urls.ts new file mode 100644 index 00000000000..d7223a344e4 --- /dev/null +++ b/libs/common/src/platform/misc/safe-urls.ts @@ -0,0 +1,33 @@ +import { Utils } from "./utils"; + +const CanLaunchWhitelist = [ + "https://", + "http://", + "ssh://", + "ftp://", + "sftp://", + "irc://", + "vnc://", + // https://docs.microsoft.com/en-us/windows-server/remote/remote-desktop-services/clients/remote-desktop-uri + "rdp://", // Legacy RDP URI scheme + "ms-rd:", // Preferred RDP URI scheme + "chrome://", + "iosapp://", + "androidapp://", +]; + +export class SafeUrls { + static canLaunch(uri: string): boolean { + if (Utils.isNullOrWhitespace(uri)) { + return false; + } + + for (let i = 0; i < CanLaunchWhitelist.length; i++) { + if (uri.indexOf(CanLaunchWhitelist[i]) === 0) { + return true; + } + } + + return false; + } +} diff --git a/libs/common/src/vault/models/view/login-uri.view.ts b/libs/common/src/vault/models/view/login-uri.view.ts index be08f63d8b1..7bd650f2f2d 100644 --- a/libs/common/src/vault/models/view/login-uri.view.ts +++ b/libs/common/src/vault/models/view/login-uri.view.ts @@ -2,25 +2,10 @@ import { Jsonify } from "type-fest"; import { UriMatchType } from "../../../enums"; import { View } from "../../../models/view/view"; +import { SafeUrls } from "../../../platform/misc/safe-urls"; import { Utils } from "../../../platform/misc/utils"; import { LoginUri } from "../domain/login-uri"; -const CanLaunchWhitelist = [ - "https://", - "http://", - "ssh://", - "ftp://", - "sftp://", - "irc://", - "vnc://", - // https://docs.microsoft.com/en-us/windows-server/remote/remote-desktop-services/clients/remote-desktop-uri - "rdp://", // Legacy RDP URI scheme - "ms-rd:", // Preferred RDP URI scheme - "chrome://", - "iosapp://", - "androidapp://", -]; - export class LoginUriView implements View { match: UriMatchType = null; @@ -108,15 +93,10 @@ export class LoginUriView implements View { return this._canLaunch; } if (this.uri != null && this.match !== UriMatchType.RegularExpression) { - const uri = this.launchUri; - for (let i = 0; i < CanLaunchWhitelist.length; i++) { - if (uri.indexOf(CanLaunchWhitelist[i]) === 0) { - this._canLaunch = true; - return this._canLaunch; - } - } + this._canLaunch = SafeUrls.canLaunch(this.launchUri); + } else { + this._canLaunch = false; } - this._canLaunch = false; return this._canLaunch; }