[desktop] changes after review by jom

* use assert for checking parameters in ElectronCredentialsEncryption
* rename KeyProvider to KeyStoreFacade (like in android)
* don't use one-line if statements

src/desktop/DesktopMain.ts

@@ -30,8 +30,8 @@ import child_process from "child_process"
 import {LocalShortcutManager} from "./electron-localshortcut/LocalShortcut"
 import {cryptoFns} from "./CryptoFns"
 import {DesktopConfigMigrator} from "./config/migrations/DesktopConfigMigrator"
-import type {DesktopDeviceKeyProvider} from "./DeviceKeyProviderImpl"
-import {DeviceKeyProviderImpl} from "./DeviceKeyProviderImpl"
+import type {DesktopKeyStoreFacade} from "./KeyStoreFacadeImpl"
+import {KeyStoreFacadeImpl} from "./KeyStoreFacadeImpl"
 import {AlarmSchedulerImpl} from "../calendar/date/AlarmScheduler"
 import {SchedulerImpl} from "../misc/Scheduler"
 import {DateProviderImpl} from "../calendar/date/CalendarUtils"
@@ -46,7 +46,7 @@ type Components = {
 	readonly dl: DesktopDownloadManager
 	readonly sse: DesktopSseClient
 	readonly conf: DesktopConfig
-	readonly deviceKeyProvider: DesktopDeviceKeyProvider
+	readonly keyStoreFacade: DesktopKeyStoreFacade
 	readonly notifier: DesktopNotifier
 	readonly sock: Socketeer
 	readonly updater: ElectronUpdater
@@ -94,9 +94,9 @@ if (opts.registerAsMailHandler && opts.unregisterAsMailHandler) {
 async function createComponents(): Promise<Components> {
 	lang.init(en)
 	const secretStorage = new KeytarSecretStorage()
-	const deviceKeyProvider = new DeviceKeyProviderImpl(secretStorage, desktopCrypto)
-	const configMigrator = new DesktopConfigMigrator(desktopCrypto, deviceKeyProvider, electron)
-	const conf = new DesktopConfig(app, configMigrator, deviceKeyProvider, desktopCrypto)
+	const keyStoreFacade = new KeyStoreFacadeImpl(secretStorage, desktopCrypto)
+	const configMigrator = new DesktopConfigMigrator(desktopCrypto, keyStoreFacade, electron)
+	const conf = new DesktopConfig(app, configMigrator, keyStoreFacade, desktopCrypto)
 	// Fire config loading, dont wait for it
 	conf.init().catch(e => {
 		console.error("Could not load config", e)
@@ -108,11 +108,11 @@ async function createComponents(): Promise<Components> {
 	const notifier = new DesktopNotifier(tray, new ElectronNotificationFactory())
 	const dateProvider = new DateProviderImpl()
 	const dl = new DesktopDownloadManager(conf, desktopNet, desktopUtils, dateProvider, fs, electron)
-	const alarmStorage = new DesktopAlarmStorage(conf, desktopCrypto, deviceKeyProvider)
+	const alarmStorage = new DesktopAlarmStorage(conf, desktopCrypto, keyStoreFacade)
 	const updater = new ElectronUpdater(conf, notifier, desktopCrypto, app, tray, new UpdaterWrapperImpl())
 	const shortcutManager = new LocalShortcutManager()
 	const themeManager = new ThemeManager(conf)
-	const credentialsEncryption = new ElectronCredentialsEncryptionImpl(deviceKeyProvider, desktopCrypto)
+	const credentialsEncryption = new ElectronCredentialsEncryptionImpl(keyStoreFacade, desktopCrypto)
 	const wm = new WindowManager(conf, tray, notifier, electron, shortcutManager, dl, themeManager)
 	const alarmScheduler = new AlarmSchedulerImpl(dateProvider, new SchedulerImpl(dateProvider, global))
 	const desktopAlarmScheduler = new DesktopAlarmScheduler(wm, notifier, alarmStorage, desktopCrypto, alarmScheduler)
@@ -153,7 +153,7 @@ async function createComponents(): Promise<Components> {
 		dl,
 		sse,
 		conf,
-		deviceKeyProvider,
+		keyStoreFacade: keyStoreFacade,
 		sock,
 		notifier,
 		updater,
@@ -205,8 +205,8 @@ async function startupInstance(components: Components) {
 }
 
 async function onAppReady(components: Components) {
-	const {ipc, wm, deviceKeyProvider, conf} = components
-	deviceKeyProvider.getDeviceKey().catch(() => {
+	const {ipc, wm, keyStoreFacade, conf} = components
+	keyStoreFacade.getDeviceKey().catch(() => {
 		electron.dialog.showErrorBox("Could not access secret storage", "Please see the FAQ at tutanota.com/faq/#secretstorage")
 	})
 	app.on("window-all-closed", async () => {

src/desktop/DeviceKeyProviderImpl.ts → src/desktop/KeyStoreFacadeImpl.ts

@@ -13,13 +13,13 @@ export enum KeyAccountName {
 	CREDENTIALS_KEY = "credentials-device-lock-key"
 }
 
-export interface DesktopDeviceKeyProvider {
+export interface DesktopKeyStoreFacade {
 	getDeviceKey(): Promise<Aes256Key>
 
 	getCredentialsKey(): Promise<Aes256Key>
 }
 
-export class DeviceKeyProviderImpl implements DesktopDeviceKeyProvider {
+export class KeyStoreFacadeImpl implements DesktopKeyStoreFacade {
 	_secretStorage: SecretStorage
 	_resolvedKeys: Record<string, DeferredObject<Aes256Key>>
 	_crypto: DesktopCryptoFacade

src/desktop/config/DesktopConfig.ts

@@ -7,7 +7,7 @@ import fs from "fs"
 import type {Config} from "./ConfigCommon"
 import {BuildConfigKey, DesktopConfigEncKey, DesktopConfigKey} from "./ConfigKeys"
 import type {App} from "electron"
-import type {DesktopDeviceKeyProvider} from "../DeviceKeyProviderImpl"
+import type {DesktopKeyStoreFacade} from "../KeyStoreFacadeImpl"
 import {DesktopCryptoFacade} from "../DesktopCryptoFacade"
 import {CryptoError} from "../../api/common/error/CryptoError"
 import {log} from "../DesktopLog"
@@ -28,14 +28,14 @@ export class DesktopConfig {
 	_buildConfig: DeferredObject<Config>;
 	_desktopConfig: DeferredObject<Config>; // user preferences as set for this installation
 	_desktopConfigFile: ConfigFileType;
-	_deviceKeyProvider: DesktopDeviceKeyProvider
+	_keyStoreFacade: DesktopKeyStoreFacade
 	_cryptoFacade: DesktopCryptoFacade
 	_app: App
 	_migrator: DesktopConfigMigrator
 	_onValueSetListeners: OnValueSetListeners
 
-	constructor(app: App, migrator: DesktopConfigMigrator, deviceKeyProvider: DesktopDeviceKeyProvider, cryptFacade: DesktopCryptoFacade) {
-		this._deviceKeyProvider = deviceKeyProvider
+	constructor(app: App, migrator: DesktopConfigMigrator, keyStoreFacade: DesktopKeyStoreFacade, cryptFacade: DesktopCryptoFacade) {
+		this._keyStoreFacade = keyStoreFacade
 		this._cryptoFacade = cryptFacade
 		this._app = app
 		this._migrator = migrator
@@ -94,7 +94,7 @@ export class DesktopConfig {
 			return null
 		}
 
-		const deviceKey = await this._deviceKeyProvider.getDeviceKey()
+		const deviceKey = await this._keyStoreFacade.getDeviceKey()
 		try {
 			return this._cryptoFacade.aesDecryptObject(deviceKey, downcast<string>(encryptedValue))
 		} catch (e) {
@@ -107,7 +107,7 @@ export class DesktopConfig {
 	}
 
 	async _setEncryptedVar(key: DesktopConfigEncKey, value: ConfigValue | null) {
-		const deviceKey = await this._deviceKeyProvider.getDeviceKey()
+		const deviceKey = await this._keyStoreFacade.getDeviceKey()
 		let encryptedValue
 		if (value != null) {
 			encryptedValue = this._cryptoFacade.aesEncryptObject(deviceKey, value)

src/desktop/config/migrations/DesktopConfigMigrator.ts

@@ -11,19 +11,19 @@ import * as migration0004 from "./migration-0004"
 import * as migration0005 from "./migration-0005"
 import type {Config, ConfigMigration} from "../ConfigCommon"
 import {DesktopCryptoFacade} from "../../DesktopCryptoFacade"
-import type {DesktopDeviceKeyProvider} from "../../DeviceKeyProviderImpl"
+import type {DesktopKeyStoreFacade} from "../../KeyStoreFacadeImpl"
 
 export type MigrationKind = "migrateClient" | "migrateAdmin"
 export type ElectronExports = typeof Electron.CrossProcessExports;
 
 export class DesktopConfigMigrator {
 	readonly crypto: DesktopCryptoFacade
-	_deviceKeyProvider: DesktopDeviceKeyProvider
+	_keyStoreFacade: DesktopKeyStoreFacade
 	_electron: ElectronExports
 
-	constructor(crypto: DesktopCryptoFacade, deviceKeyProvider: DesktopDeviceKeyProvider, electron: ElectronExports) {
+	constructor(crypto: DesktopCryptoFacade, keyStoreFacade: DesktopKeyStoreFacade, electron: ElectronExports) {
 		this.crypto = crypto
-		this._deviceKeyProvider = deviceKeyProvider
+		this._keyStoreFacade = keyStoreFacade
 		this._electron = electron
 	}
 
@@ -41,7 +41,7 @@ export class DesktopConfigMigrator {
 				await applyMigration(migration0002[migrationFunction], oldConfig)
 
 			case 2:
-				await applyMigration(config => migration0003[migrationFunction](config, this.crypto, this._deviceKeyProvider), oldConfig)
+				await applyMigration(config => migration0003[migrationFunction](config, this.crypto, this._keyStoreFacade), oldConfig)
 
 			case 3:
 				await applyMigration(migration0004[migrationFunction], oldConfig)

src/desktop/config/migrations/migration-0003.ts

@@ -1,17 +1,17 @@
 import {DesktopCryptoFacade} from "../../DesktopCryptoFacade"
 import type {Config} from "../ConfigCommon"
 import {downcast} from "@tutao/tutanota-utils"
-import type {DesktopDeviceKeyProvider} from "../../DeviceKeyProviderImpl"
+import type {DesktopKeyStoreFacade} from "../../KeyStoreFacadeImpl"
 import {log} from "../../DesktopLog"
 
-async function migrate(oldConfig: Config, crypto: DesktopCryptoFacade, deviceKeyProvider: DesktopDeviceKeyProvider): Promise<void> {
+async function migrate(oldConfig: Config, crypto: DesktopCryptoFacade, keyStoreFacade: DesktopKeyStoreFacade): Promise<void> {
 	Object.assign(oldConfig, {
 		desktopConfigVersion: 3,
 	})
 
 	if (oldConfig.pushIdentifier) {
 		try {
-			const deviceKey = await deviceKeyProvider.getDeviceKey()
+			const deviceKey = await keyStoreFacade.getDeviceKey()
 			Object.assign(oldConfig, {
 				sseInfo: crypto.aesEncryptObject(deviceKey, downcast(oldConfig.pushIdentifier)),
 			})

src/desktop/credentials/ElectronCredentialsEncryption.ts

@@ -1,7 +1,7 @@
 import {CredentialEncryptionMode} from "../../misc/credentials/CredentialEncryptionMode"
-import {ProgrammingError} from "../../api/common/error/ProgrammingError"
-import {DesktopDeviceKeyProvider} from "../DeviceKeyProviderImpl"
+import {DesktopKeyStoreFacade} from "../KeyStoreFacadeImpl"
 import {DesktopCryptoFacade} from "../DesktopCryptoFacade"
+import {assert} from "@tutao/tutanota-utils"
 
 export interface ElectronCredentialsEncryption {
 	/**
@@ -20,28 +20,26 @@ export interface ElectronCredentialsEncryption {
 
 export class ElectronCredentialsEncryptionImpl implements ElectronCredentialsEncryption {
 
-	private readonly _desktopDeviceKeyProvider: DesktopDeviceKeyProvider
+	private readonly _desktopKeyStoreFacade: DesktopKeyStoreFacade
 	private readonly _crypto: DesktopCryptoFacade
 
-	constructor(deviceKeyProvider: DesktopDeviceKeyProvider, crypto: DesktopCryptoFacade) {
-		this._desktopDeviceKeyProvider = deviceKeyProvider
+	constructor(keyStoreFacade: DesktopKeyStoreFacade, crypto: DesktopCryptoFacade) {
+		this._desktopKeyStoreFacade = keyStoreFacade
 		this._crypto = crypto
 	}
 
-	async decryptUsingKeychain(base64EncodedEncryptedData: string, encryptionMode: CredentialEncryptionMode): Promise<string> {
-		if (encryptionMode !== CredentialEncryptionMode.DEVICE_LOCK) {
-			throw new ProgrammingError("should not use unsupported encryption mode")
-		}
-		const key = await this._desktopDeviceKeyProvider.getCredentialsKey()
+	async decryptUsingKeychain(base64EncodedEncryptedData: string, encryptionMode: CredentialEncryptionMode.DEVICE_LOCK): Promise<string> {
+		// making extra sure that the mode is the right one since this comes over IPC
+		assert(encryptionMode === CredentialEncryptionMode.DEVICE_LOCK, "should not use unsupported encryption mode")
+		const key = await this._desktopKeyStoreFacade.getCredentialsKey()
 		const decryptedData = this._crypto.aes256DecryptKeyToB64(key, base64EncodedEncryptedData)
 		return Promise.resolve(decryptedData)
 	}
 
-	async encryptUsingKeychain(base64EncodedData: string, encryptionMode: CredentialEncryptionMode): Promise<string> {
-		if (encryptionMode !== CredentialEncryptionMode.DEVICE_LOCK) {
-			throw new ProgrammingError("should not use unsupported encryption mode")
-		}
-		const key = await this._desktopDeviceKeyProvider.getCredentialsKey()
+	async encryptUsingKeychain(base64EncodedData: string, encryptionMode: CredentialEncryptionMode.DEVICE_LOCK): Promise<string> {
+		// making extra sure that the mode is the right one since this comes over IPC
+		assert(encryptionMode === CredentialEncryptionMode.DEVICE_LOCK, "should not use unsupported encryption mode")
+		const key = await this._desktopKeyStoreFacade.getCredentialsKey()
 		const encryptedData = this._crypto.aes256EncryptKeyToB64(key, base64EncodedData)
 		return Promise.resolve(encryptedData)
 	}

src/desktop/sse/DesktopAlarmStorage.ts

@@ -3,7 +3,7 @@ import type {EncryptedAlarmNotification, NotificationSessionKey} from "./Desktop
 import {DesktopCryptoFacade} from "../DesktopCryptoFacade"
 import {elementIdPart} from "../../api/common/utils/EntityUtils"
 import {DesktopConfigKey} from "../config/ConfigKeys"
-import type {DesktopDeviceKeyProvider} from "../DeviceKeyProviderImpl"
+import type {DesktopKeyStoreFacade} from "../KeyStoreFacadeImpl"
 import type {Base64} from "@tutao/tutanota-utils"
 import {findAllAndRemove} from "@tutao/tutanota-utils"
 import {log} from "../DesktopLog"
@@ -12,13 +12,13 @@ import {log} from "../DesktopLog"
  * manages session keys used for decrypting alarm notifications, encrypting & persisting them to disk
  */
 export class DesktopAlarmStorage {
-	_deviceKeyProvider: DesktopDeviceKeyProvider
+	_desktopKeyStoreFacade: DesktopKeyStoreFacade
 	_conf: DesktopConfig
 	_crypto: DesktopCryptoFacade
 	_sessionKeysB64: Record<string, string>
 
-	constructor(conf: DesktopConfig, desktopCryptoFacade: DesktopCryptoFacade, deviceKeyProvider: DesktopDeviceKeyProvider) {
-		this._deviceKeyProvider = deviceKeyProvider
+	constructor(conf: DesktopConfig, desktopCryptoFacade: DesktopCryptoFacade, keyStoreFacade: DesktopKeyStoreFacade) {
+		this._desktopKeyStoreFacade = keyStoreFacade
 		this._conf = conf
 		this._crypto = desktopCryptoFacade
 		this._sessionKeysB64 = {}
@@ -35,7 +35,7 @@ export class DesktopAlarmStorage {
 
 		if (!keys[pushIdentifierId]) {
 			this._sessionKeysB64[pushIdentifierId] = pushIdentifierSessionKeyB64
-			return this._deviceKeyProvider.getDeviceKey().then(pw => {
+			return this._desktopKeyStoreFacade.getDeviceKey().then(pw => {
 				keys[pushIdentifierId] = this._crypto.aes256EncryptKeyToB64(pw, pushIdentifierSessionKeyB64)
 				return this._conf.setVar(DesktopConfigKey.pushEncSessionKeys, keys)
 			})
@@ -61,7 +61,7 @@ export class DesktopAlarmStorage {
 	 * @return {Promise<?Base64>} a stored pushIdentifierSessionKey that should be able to decrypt the given notificationSessionKey
 	 */
 	async getPushIdentifierSessionKey(notificationSessionKey: NotificationSessionKey): Promise<Base64 | null> {
-		const pw = await this._deviceKeyProvider.getDeviceKey()
+		const pw = await this._desktopKeyStoreFacade.getDeviceKey()
 		const pushIdentifierId = elementIdPart(notificationSessionKey.pushIdentifier)
 
 		if (this._sessionKeysB64[pushIdentifierId]) {

src/desktop/sse/SecretStorage.ts

@@ -11,7 +11,9 @@ export class KeytarSecretStorage implements SecretStorage {
 	getPassword(service: string, account: string): Promise<string | null> {
 		return getPassword(service, account)
 			.catch(e => {
-				if (e.message === CANCELLED) throw new CancelledError("user cancelled keychain unlock")
+				if (e.message === CANCELLED) {
+					throw new CancelledError("user cancelled keychain unlock")
+				}
 				throw e
 			})
 	}

test/api/TestUtils.ts

@@ -5,7 +5,7 @@ import {IndexerCore} from "../../src/api/worker/search/IndexerCore"
 import {EventQueue} from "../../src/api/worker/search/EventQueue"
 import {DbFacade, DbTransaction} from "../../src/api/worker/search/DbFacade"
 import {assertNotNull, neverNull} from "@tutao/tutanota-utils"
-import type {DesktopDeviceKeyProvider} from "../../src/desktop/DeviceKeyProviderImpl"
+import type {DesktopKeyStoreFacade} from "../../src/desktop/KeyStoreFacadeImpl"
 import {mock} from "@tutao/tutanota-test-utils"
 import {aes256RandomKey, fixedIv, uint8ArrayToKey} from "@tutao/tutanota-crypto"
 
@@ -64,7 +64,7 @@ export function reportTest(results: any, stats: any) {
 	})()
 }
 
-export function makeDeviceKeyProvider(uint8ArrayKey: Uint8Array): DesktopDeviceKeyProvider {
+export function makeKeyStoreFacade(uint8ArrayKey: Uint8Array): DesktopKeyStoreFacade {
 	return {
 		getDeviceKey() {
 			return Promise.resolve(uint8ArrayToKey(uint8ArrayKey))

test/client/Suite.ts

@@ -75,7 +75,7 @@ import {preTest, reportTest} from "../api/TestUtils"
 		await import("./desktop/integration/RegistryScriptGeneratorTest.js")
 		await import("./desktop/DesktopCryptoFacadeTest.js")
 		await import("./desktop/DesktopContextMenuTest.js")
-		await import("./desktop/DeviceKeyProviderTest.js")
+		await import("./desktop/KeyStoreFacadeTest.js")
 		await import ("./desktop/config/ConfigFileTest.js")
 		await import ("./desktop/credentials/ElectronCredentialsEncryptionTest")
 	}

test/client/desktop/DeviceKeyProviderTest.ts → test/client/desktop/KeyStoreFacadeTest.ts

@@ -1,16 +1,16 @@
 import o from "ospec"
-import {DeviceKeyProviderImpl, KeyAccountName, SERVICE_NAME} from "../../../src/desktop/DeviceKeyProviderImpl"
+import {KeyAccountName, KeyStoreFacadeImpl, SERVICE_NAME} from "../../../src/desktop/KeyStoreFacadeImpl"
 import {DesktopCryptoFacade} from "../../../src/desktop/DesktopCryptoFacade"
 import type {SecretStorage} from "../../../src/desktop/sse/SecretStorage"
 import {spyify} from "../nodemocker"
 import {downcast} from "@tutao/tutanota-utils"
 import {keyToBase64, uint8ArrayToKey} from "@tutao/tutanota-crypto"
 
-function initDeviceKeyProvider(secretStorage: SecretStorage, crypto: DesktopCryptoFacade): DeviceKeyProviderImpl {
-	return new DeviceKeyProviderImpl(secretStorage, crypto)
+function initKeyStoreFacade(secretStorage: SecretStorage, crypto: DesktopCryptoFacade): KeyStoreFacadeImpl {
+	return new KeyStoreFacadeImpl(secretStorage, crypto)
 }
 
-o.spec("DeviceKeyProvider test", function () {
+o.spec("KeyStoreFacade test", function () {
 	const aes256Key = uint8ArrayToKey(new Uint8Array([1, 2]))
 	o("getDeviceKey should return stored key", async function () {
 		const secretStorageSpy: SecretStorage = spyify({
@@ -22,8 +22,8 @@ o.spec("DeviceKeyProvider test", function () {
 			},
 		})
 		const cryptoFacadeSpy: DesktopCryptoFacade = spyify(downcast({}))
-		const deviceKeyProvider = initDeviceKeyProvider(secretStorageSpy, cryptoFacadeSpy)
-		const actualKey = await deviceKeyProvider.getDeviceKey()
+		const keyStoreFacade = initKeyStoreFacade(secretStorageSpy, cryptoFacadeSpy)
+		const actualKey = await keyStoreFacade.getDeviceKey()
 		o(actualKey).deepEquals(aes256Key)
 		o(secretStorageSpy.getPassword.callCount).equals(1)
 		o(secretStorageSpy.getPassword.calls[0].args).deepEquals([SERVICE_NAME, KeyAccountName.DEVICE_KEY])
@@ -42,8 +42,8 @@ o.spec("DeviceKeyProvider test", function () {
 				return aes256Key
 			},
 		})
-		const deviceKeyProvider = initDeviceKeyProvider(secretStorageSpy, cryptoFacadeSpy)
-		await deviceKeyProvider.getDeviceKey()
+		const keyStoreFacade = initKeyStoreFacade(secretStorageSpy, cryptoFacadeSpy)
+		await keyStoreFacade.getDeviceKey()
 		o(secretStorageSpy.setPassword.args).deepEquals([SERVICE_NAME, KeyAccountName.DEVICE_KEY, keyToBase64(aes256Key)])
 	})
 })

test/client/desktop/config/migrations/DesktopConfigMigratorTest.ts

@@ -2,13 +2,13 @@ import o from "ospec"
 import {DesktopConfigMigrator} from "../../../../../src/desktop/config/migrations/DesktopConfigMigrator"
 import {DesktopCryptoFacade} from "../../../../../src/desktop/DesktopCryptoFacade"
 import {downcast} from "@tutao/tutanota-utils"
-import {makeDeviceKeyProvider} from "../../../../api/TestUtils"
-import {DesktopDeviceKeyProvider} from "../../../../../src/desktop/DeviceKeyProviderImpl";
+import {makeKeyStoreFacade} from "../../../../api/TestUtils"
+import {DesktopKeyStoreFacade} from "../../../../../src/desktop/KeyStoreFacadeImpl";
 
 o.spec('desktop config migrator test', function () {
 	let migrator
 	let crypto: DesktopCryptoFacade
-	let deviceKeyProvider: DesktopDeviceKeyProvider
+	let keyStoreFacade: DesktopKeyStoreFacade
 	const key = new Uint8Array([1, 2, 3])
 
 	o.before(function () {
@@ -27,8 +27,8 @@ o.spec('desktop config migrator test', function () {
 		})
 
 
-		deviceKeyProvider = makeDeviceKeyProvider(key)
-		migrator = new DesktopConfigMigrator(crypto, deviceKeyProvider, electron)
+		keyStoreFacade = makeKeyStoreFacade(key)
+		migrator = new DesktopConfigMigrator(crypto, keyStoreFacade, electron)
 	})
 	o("migrations result in correct default config, client", async function () {
 		const oldConfig = {