diff --git a/.eslintignore b/.eslintignore index ded57319f..e7b2eddf3 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1102,8 +1102,10 @@ packages/lib/services/interop/Module.js packages/lib/services/interop/types.js packages/lib/services/joplinCloudUtils.js packages/lib/services/joplinServer/personalizedUserContentBaseUrl.js +packages/lib/services/keychain/KeychainService.test.js packages/lib/services/keychain/KeychainService.js packages/lib/services/keychain/KeychainServiceDriver.dummy.js +packages/lib/services/keychain/KeychainServiceDriver.electron.js packages/lib/services/keychain/KeychainServiceDriver.mobile.js packages/lib/services/keychain/KeychainServiceDriver.node.js packages/lib/services/keychain/KeychainServiceDriverBase.js diff --git a/.gitignore b/.gitignore index eb7c81679..8bf8aa28f 100644 --- a/.gitignore +++ b/.gitignore @@ -1080,8 +1080,10 @@ packages/lib/services/interop/Module.js packages/lib/services/interop/types.js packages/lib/services/joplinCloudUtils.js packages/lib/services/joplinServer/personalizedUserContentBaseUrl.js +packages/lib/services/keychain/KeychainService.test.js packages/lib/services/keychain/KeychainService.js packages/lib/services/keychain/KeychainServiceDriver.dummy.js +packages/lib/services/keychain/KeychainServiceDriver.electron.js packages/lib/services/keychain/KeychainServiceDriver.mobile.js packages/lib/services/keychain/KeychainServiceDriver.node.js packages/lib/services/keychain/KeychainServiceDriverBase.js diff --git a/packages/app-desktop/bridge.ts b/packages/app-desktop/bridge.ts index 185abf129..a207d6e6c 100644 --- a/packages/app-desktop/bridge.ts +++ b/packages/app-desktop/bridge.ts @@ -1,7 +1,7 @@ import ElectronAppWrapper from './ElectronAppWrapper'; import shim from '@joplin/lib/shim'; import { _, setLocale } from '@joplin/lib/locale'; -import { BrowserWindow, nativeTheme, nativeImage, shell, dialog, MessageBoxSyncOptions } from 'electron'; +import { BrowserWindow, nativeTheme, nativeImage, shell, dialog, MessageBoxSyncOptions, safeStorage } from 'electron'; import { dirname, toSystemSlashes } from '@joplin/lib/path-utils'; import { fileUriToPath } from '@joplin/utils/url'; import { urlDecode } from '@joplin/lib/string-utils'; @@ -485,6 +485,21 @@ export class Bridge { return nativeImage.createFromPath(path); } + public safeStorage = { + isEncryptionAvailable() { + return safeStorage.isEncryptionAvailable(); + }, + encryptString(data: string) { + return safeStorage.encryptString(data).toString('base64'); + }, + decryptString(base64Data: string) { + return safeStorage.decryptString(Buffer.from(base64Data, 'base64')); + }, + + getSelectedStorageBackend() { + return safeStorage.getSelectedStorageBackend(); + }, + }; } let bridge_: Bridge = null; diff --git a/packages/app-desktop/main.js b/packages/app-desktop/main.js index 4d936665e..8353febd7 100644 --- a/packages/app-desktop/main.js +++ b/packages/app-desktop/main.js @@ -18,7 +18,7 @@ const registerCustomProtocols = require('./utils/customProtocols/registerCustomP // our case it's a string like "@joplin/app-desktop". It's also supposed to // check the productName key but is not doing it, so here set the // application name to the right string. -electronApp.name = packageInfo.name; +electronApp.setName(packageInfo.name); process.on('unhandledRejection', (reason, p) => { console.error('Unhandled promise rejection', p, 'reason:', reason); diff --git a/packages/app-mobile/root.tsx b/packages/app-mobile/root.tsx index 4e59a791c..b025b6b2b 100644 --- a/packages/app-mobile/root.tsx +++ b/packages/app-mobile/root.tsx @@ -624,7 +624,7 @@ async function initialize(dispatch: Dispatch) { reg.logger().info('Database is ready.'); reg.logger().info('Loading settings...'); - await loadKeychainServiceAndSettings(KeychainServiceDriverMobile); + await loadKeychainServiceAndSettings([KeychainServiceDriverMobile]); await migrateMasterPassword(); if (!Setting.value('clientId')) Setting.setValue('clientId', uuid.create()); diff --git a/packages/lib/BaseApplication.ts b/packages/lib/BaseApplication.ts index 4a651a911..2b830f332 100644 --- a/packages/lib/BaseApplication.ts +++ b/packages/lib/BaseApplication.ts @@ -4,8 +4,8 @@ import shim from './shim'; const { setupProxySettings } = require('./shim-init-node'); import BaseService from './services/BaseService'; import reducer, { getNotesParent, serializeNotesParent, setStore, State } from './reducer'; -import KeychainServiceDriver from './services/keychain/KeychainServiceDriver.node'; -import KeychainServiceDriverDummy from './services/keychain/KeychainServiceDriver.dummy'; +import KeychainServiceDriverNode from './services/keychain/KeychainServiceDriver.node'; +import KeychainServiceDriverElectron from './services/keychain/KeychainServiceDriver.electron'; import { setLocale } from './locale'; import KvStore from './services/KvStore'; import SyncTargetJoplinServer from './SyncTargetJoplinServer'; @@ -754,10 +754,13 @@ export default class BaseApplication { reg.setDb(this.database_); BaseModel.setDb(this.database_); + KvStore.instance().setDb(reg.db()); setRSA(RSA); - await loadKeychainServiceAndSettings(options.keychainEnabled ? KeychainServiceDriver : KeychainServiceDriverDummy); + await loadKeychainServiceAndSettings( + options.keychainEnabled ? [KeychainServiceDriverElectron, KeychainServiceDriverNode] : [], + ); await migrateMasterPassword(); await handleSyncStartupOperation(); @@ -841,7 +844,6 @@ export default class BaseApplication { BaseItem.revisionService_ = RevisionService.instance(); - KvStore.instance().setDb(reg.db()); BaseItem.encryptionService_ = EncryptionService.instance(); BaseItem.shareService_ = ShareService.instance(); diff --git a/packages/lib/models/Setting.ts b/packages/lib/models/Setting.ts index 9da782f19..30849bb3d 100644 --- a/packages/lib/models/Setting.ts +++ b/packages/lib/models/Setting.ts @@ -142,7 +142,6 @@ export type SettingMetadataSection = { export type MetadataBySection = SettingMetadataSection[]; class Setting extends BaseModel { - public static schemaUrl = 'https://joplinapp.org/schema/settings.json'; // For backward compatibility @@ -976,19 +975,10 @@ class Setting extends BaseModel { // Also we don't control what happens on the keychain - the values can be edited or deleted // outside the application. For that reason, we rewrite it every time the values are saved, // even if, internally, they haven't changed. - // As an optimisation, we check if the value exists on the keychain before writing it again. try { const passwordName = `setting.${s.key}`; - const currentValue = await this.keychainService().password(passwordName); - if (currentValue !== valueAsString) { - const wasSet = await this.keychainService().setPassword(passwordName, valueAsString); - if (wasSet) continue; - } else { - // The value is already in the keychain - so nothing to do - // Make sure to `continue` here otherwise it will save the password - // in clear text in the database. - continue; - } + const wasSet = await this.keychainService().setPassword(passwordName, valueAsString); + if (wasSet) continue; } catch (error) { logger.error(`Could not set setting on the keychain. Will be saved to database instead: ${s.key}:`, error); } diff --git a/packages/lib/models/settings/builtInMetadata.ts b/packages/lib/models/settings/builtInMetadata.ts index 1cbb5b14b..b37742d36 100644 --- a/packages/lib/models/settings/builtInMetadata.ts +++ b/packages/lib/models/settings/builtInMetadata.ts @@ -928,6 +928,7 @@ const builtInMetadata = (Setting: typeof SettingType) => { collapsedFolderIds: { value: [] as string[], type: SettingItemType.Array, public: false }, 'keychain.supported': { value: -1, type: SettingItemType.Int, public: false }, + 'keychain.lastAvailableDrivers': { value: [] as string[], type: SettingItemType.Array, public: false }, 'db.ftsEnabled': { value: -1, type: SettingItemType.Int, public: false }, 'db.fuzzySearchEnabled': { value: -1, type: SettingItemType.Int, public: false }, 'encryption.enabled': { value: false, type: SettingItemType.Bool, public: false }, diff --git a/packages/lib/services/SettingUtils.ts b/packages/lib/services/SettingUtils.ts index c94c75a6b..682b8882d 100644 --- a/packages/lib/services/SettingUtils.ts +++ b/packages/lib/services/SettingUtils.ts @@ -4,6 +4,9 @@ import KeychainService from './keychain/KeychainService'; import Setting from '../models/Setting'; import uuid from '../uuid'; import { migrateLocalSyncInfo } from './synchronizer/syncInfoUtils'; +import KeychainServiceDriverBase from './keychain/KeychainServiceDriverBase'; + +type KeychainServiceDriverConstructor = new (appId: string, clientId: string)=> KeychainServiceDriverBase; // This function takes care of initialising both the keychain service and settings. // @@ -13,11 +16,12 @@ import { migrateLocalSyncInfo } from './synchronizer/syncInfoUtils'; // In other words, it's not possible to load the settings without the KS service and it's not // possible to initialise the KS service without the settings. // The solution is to fetch just the client ID directly from the database. -// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied -export async function loadKeychainServiceAndSettings(KeychainServiceDriver: any) { +export async function loadKeychainServiceAndSettings(keychainServiceDrivers: KeychainServiceDriverConstructor[]) { const clientIdSetting = await Setting.loadOne('clientId'); const clientId = clientIdSetting ? clientIdSetting.value : uuid.create(); - KeychainService.instance().initialize(new KeychainServiceDriver(Setting.value('appId'), clientId)); + await KeychainService.instance().initialize( + keychainServiceDrivers.map(Driver => new Driver(Setting.value('appId'), clientId)), + ); Setting.setKeychainService(KeychainService.instance()); await Setting.load(); diff --git a/packages/lib/services/keychain/KeychainService.test.ts b/packages/lib/services/keychain/KeychainService.test.ts new file mode 100644 index 000000000..4f7914bb2 --- /dev/null +++ b/packages/lib/services/keychain/KeychainService.test.ts @@ -0,0 +1,128 @@ +import Setting from '../../models/Setting'; +import shim from '../../shim'; +import { switchClient, setupDatabaseAndSynchronizer } from '../../testing/test-utils'; +import KeychainService from './KeychainService'; +import KeychainServiceDriverDummy from './KeychainServiceDriver.dummy'; +import KeychainServiceDriverElectron from './KeychainServiceDriver.electron'; +import KeychainServiceDriverNode from './KeychainServiceDriver.node'; + +interface SafeStorageMockOptions { + isEncryptionAvailable?: ()=> boolean; + encryptString?: (str: string)=> Promise; + decryptString?: (str: string)=> Promise; +} + +const mockSafeStorage = ({ // Safe storage + isEncryptionAvailable = jest.fn(() => true), + encryptString = jest.fn(async s => (`e:${s}`)), + decryptString = jest.fn(async s => s.substring(2)), +}: SafeStorageMockOptions) => { + shim.electronBridge = () => ({ + safeStorage: { + isEncryptionAvailable, + encryptString, + decryptString, + getSelectedStorageBackend: () => 'mock', + }, + }); +}; + +const mockKeytar = () => { + const storage = new Map(); + + const keytarMock = { + getPassword: jest.fn(async (key, client) => { + return storage.get(`${client}--${key}`); + }), + setPassword: jest.fn(async (key, client, password) => { + if (!password) throw new Error('Keytar doesn\'t support empty passwords.'); + storage.set(`${client}--${key}`, password); + }), + deletePassword: jest.fn(async (key, client) => { + storage.delete(`${client}--${key}`); + }), + }; + shim.keytar = () => keytarMock; + return keytarMock; +}; + +const makeDrivers = () => [ + new KeychainServiceDriverElectron(Setting.value('appId'), Setting.value('clientId')), + new KeychainServiceDriverNode(Setting.value('appId'), Setting.value('clientId')), +]; + +describe('KeychainService', () => { + beforeEach(async () => { + await setupDatabaseAndSynchronizer(0); + await switchClient(0); + Setting.setValue('keychain.supported', 1); + shim.electronBridge = null; + shim.keytar = null; + }); + + test('should copy keys from keytar to safeStorage', async () => { + const keytarMock = mockKeytar(); + await KeychainService.instance().initialize(makeDrivers()); + + // Set a secure setting + Setting.setValue('encryption.masterPassword', 'testing'); + await Setting.saveAll(); + + mockSafeStorage({}); + + await KeychainService.instance().initialize(makeDrivers()); + await Setting.load(); + expect(Setting.value('encryption.masterPassword')).toBe('testing'); + + await Setting.saveAll(); + + // For now, passwords should not be removed from old backends -- this allows + // users to revert to an earlier version of Joplin without data loss. + expect(keytarMock.deletePassword).not.toHaveBeenCalled(); + + expect(shim.electronBridge().safeStorage.encryptString).toHaveBeenCalled(); + expect(shim.electronBridge().safeStorage.encryptString).toHaveBeenCalledWith('testing'); + + await Setting.load(); + expect(Setting.value('encryption.masterPassword')).toBe('testing'); + }); + + test('should use keytar when safeStorage is unavailable', async () => { + const keytarMock = mockKeytar(); + await KeychainService.instance().initialize(makeDrivers()); + + Setting.setValue('encryption.masterPassword', 'test-password'); + await Setting.saveAll(); + expect(keytarMock.setPassword).toHaveBeenCalledWith( + `${Setting.value('appId')}.setting.encryption.masterPassword`, + `${Setting.value('clientId')}@joplin`, + 'test-password', + ); + + await Setting.load(); + expect(Setting.value('encryption.masterPassword')).toBe('test-password'); + }); + + test('should re-check for keychain support when a new driver is added', async () => { + mockKeytar(); + mockSafeStorage({}); + Setting.setValue('keychain.supported', -1); + + await KeychainService.instance().initialize([ + new KeychainServiceDriverDummy(Setting.value('appId'), Setting.value('clientId')), + ]); + await KeychainService.instance().detectIfKeychainSupported(); + + expect(Setting.value('keychain.supported')).toBe(0); + + // Should re-run the check after keytar and safeStorage are available. + await KeychainService.instance().initialize(makeDrivers()); + await KeychainService.instance().detectIfKeychainSupported(); + expect(Setting.value('keychain.supported')).toBe(1); + + // Should re-run the check if safeStorage and keytar are both no longer available. + await KeychainService.instance().initialize([]); + await KeychainService.instance().detectIfKeychainSupported(); + expect(Setting.value('keychain.supported')).toBe(0); + }); +}); diff --git a/packages/lib/services/keychain/KeychainService.ts b/packages/lib/services/keychain/KeychainService.ts index 873edd5ff..48ebc24bf 100644 --- a/packages/lib/services/keychain/KeychainService.ts +++ b/packages/lib/services/keychain/KeychainService.ts @@ -1,10 +1,14 @@ import KeychainServiceDriverBase from './KeychainServiceDriverBase'; import Setting from '../../models/Setting'; import BaseService from '../BaseService'; +import Logger from '@joplin/utils/Logger'; + +const logger = Logger.create('KeychainService'); export default class KeychainService extends BaseService { - private driver: KeychainServiceDriverBase; + private drivers_: KeychainServiceDriverBase[]; + private keysNeedingMigration_: Set; private static instance_: KeychainService; private enabled_ = true; @@ -13,9 +17,23 @@ export default class KeychainService extends BaseService { return this.instance_; } - public initialize(driver: KeychainServiceDriverBase) { - if (!driver.appId || !driver.clientId) throw new Error('appId and clientId must be set on the KeychainServiceDriver'); - this.driver = driver; + // The drivers list should be provided in order of preference, with the most preferred driver + // first. If not present in the first supported driver, the keychain service will attempt to + // migrate keys to it. + public async initialize(drivers: KeychainServiceDriverBase[]) { + if (drivers.some(driver => !driver.appId || !driver.clientId)) { + throw new Error('appId and clientId must be set on the KeychainServiceDriver'); + } + + this.drivers_ = []; + this.keysNeedingMigration_ = new Set(); + for (const driver of drivers) { + if (await driver.supported()) { + this.drivers_.push(driver); + } else { + logger.warn(`Driver unsupported:${driver.driverId}`); + } + } } // This is to programatically disable the keychain service, whether keychain @@ -38,32 +56,87 @@ export default class KeychainService extends BaseService { public async setPassword(name: string, password: string): Promise { if (!this.enabled) return false; + // Optimization: Handles the case where the password doesn't need to change. + // TODO: Re-evaluate whether this optimization is necessary after refactoring the driver + // logic. + if (!this.keysNeedingMigration_.has(name) && await this.password(name) === password) { + return true; + } + // Due to a bug in macOS, this may throw an exception "The user name or passphrase you entered is not correct." // The fix is to open Keychain Access.app. Right-click on the login keychain and try locking it and then unlocking it again. // https://github.com/atom/node-keytar/issues/76 - return this.driver.setPassword(name, password); + let i = 0; + let didSet = false; + for (; i < this.drivers_.length && !didSet; i++) { + didSet = await this.drivers_[i].setPassword(name, password); + } + + if (didSet && this.keysNeedingMigration_.has(name)) { + logger.info(`Marking key ${name} as copied to new keychain backend...`); + + // At this point, the key has been saved in drivers[i - 1]. + // + // Deleting the key from the less-preferred drivers would complete the + // migration. However, to allow users to roll back to a previous Joplin + // version without data loss, avoid deleting old keys here. + + this.keysNeedingMigration_.delete(name); + } + + return didSet; } public async password(name: string): Promise { if (!this.enabled) return null; - return this.driver.password(name); + let foundInPreferredDriver = true; + let password: string|null = null; + for (const driver of this.drivers_) { + password = await driver.password(name); + if (password) { + break; + } + foundInPreferredDriver = false; + } + + if (password && !foundInPreferredDriver) { + this.keysNeedingMigration_.add(name); + } + + return password; } public async deletePassword(name: string): Promise { if (!this.enabled) return; - await this.driver.deletePassword(name); + for (const driver of this.drivers_) { + await driver.deletePassword(name); + } } public async detectIfKeychainSupported() { this.logger().info('KeychainService: checking if keychain supported'); - if (Setting.value('keychain.supported') >= 0) { + const lastAvailableDrivers = Setting.value('keychain.lastAvailableDrivers'); + const availableDriversChanged = (() => { + if (lastAvailableDrivers.length !== this.drivers_.length) return true; + return this.drivers_.some(driver => { + return !lastAvailableDrivers.includes(driver.driverId); + }); + })(); + + const checkAlreadyDone = Setting.value('keychain.supported') >= 0; + if (checkAlreadyDone && !availableDriversChanged) { this.logger().info('KeychainService: check was already done - skipping. Supported:', Setting.value('keychain.supported')); return; } + if (availableDriversChanged) { + // Reset supported -- this allows the test .setPassword to work. + Setting.setValue('keychain.supported', -1); + } + const passwordIsSet = await this.setPassword('zz_testingkeychain', 'mytest'); if (!passwordIsSet) { @@ -75,6 +148,6 @@ export default class KeychainService extends BaseService { this.logger().info('KeychainService: tried to set and get password. Result was:', result); Setting.setValue('keychain.supported', result === 'mytest' ? 1 : 0); } + Setting.setValue('keychain.lastAvailableDrivers', this.drivers_.map(driver => driver.driverId)); } - } diff --git a/packages/lib/services/keychain/KeychainServiceDriver.dummy.ts b/packages/lib/services/keychain/KeychainServiceDriver.dummy.ts index 7617aadc6..6d0b0147c 100644 --- a/packages/lib/services/keychain/KeychainServiceDriver.dummy.ts +++ b/packages/lib/services/keychain/KeychainServiceDriver.dummy.ts @@ -1,6 +1,11 @@ import KeychainServiceDriverBase from './KeychainServiceDriverBase'; export default class KeychainServiceDriver extends KeychainServiceDriverBase { + public override readonly driverId = 'dummy'; + + public async supported() { + return false; + } public async setPassword(/* name:string, password:string*/): Promise { return false; @@ -14,4 +19,6 @@ export default class KeychainServiceDriver extends KeychainServiceDriverBase { } + public async upgradeStorageBackend(_secureKeys: string[], _newDatabaseVersion: number): Promise { + } } diff --git a/packages/lib/services/keychain/KeychainServiceDriver.electron.ts b/packages/lib/services/keychain/KeychainServiceDriver.electron.ts new file mode 100644 index 000000000..ce5db9648 --- /dev/null +++ b/packages/lib/services/keychain/KeychainServiceDriver.electron.ts @@ -0,0 +1,69 @@ +import KeychainServiceDriverBase from './KeychainServiceDriverBase'; +import shim from '../../shim'; +import Logger from '@joplin/utils/Logger'; +import KvStore from '../KvStore'; +import Setting from '../../models/Setting'; + +const logger = Logger.create('KeychainServiceDriver.node'); + +const canUseSafeStorage = () => { + return !!shim.electronBridge?.()?.safeStorage?.isEncryptionAvailable(); +}; + +const kvStorePrefix = 'KeychainServiceDriver.secureStore.'; + +export default class KeychainServiceDriver extends KeychainServiceDriverBase { + public override readonly driverId = 'electron-safeStorage'; + + public constructor(appId: string, clientId: string) { + super(appId, clientId); + + if (canUseSafeStorage() && shim.isLinux()) { + logger.info('KeychainService Linux backend: ', shim.electronBridge()?.safeStorage?.getSelectedStorageBackend()); + } + } + + public async supported() { + return canUseSafeStorage(); + } + + public async setPassword(name: string, password: string): Promise { + if (canUseSafeStorage()) { + logger.debug('Saving password with electron safe storage. ID: ', name); + + const encrypted = await shim.electronBridge().safeStorage.encryptString(password); + await KvStore.instance().setValue(`${kvStorePrefix}${name}`, encrypted); + } else { + // Unsupported. + return false; + } + return true; + } + + public async password(name: string): Promise { + let result: string|null = null; + + if (canUseSafeStorage()) { + const data = await KvStore.instance().value(`${kvStorePrefix}${name}`); + if (data !== null) { + try { + result = await shim.electronBridge().safeStorage.decryptString(data); + } catch (e) { + logger.warn('Decryption of a setting failed. Corrupted data or new keychain password? Error:', e); + if (shim.isLinux() && Setting.value('env') === 'dev') { + logger.warn('If running Joplin in development mode with NodeJS installed from the Snap store, consider retrying with NodeJS installed from a different source.'); + } + } + } + } + + return result; + } + + public async deletePassword(name: string): Promise { + if (canUseSafeStorage()) { + logger.debug('Trying to delete encrypted password with id ', name); + await KvStore.instance().deleteValue(`${kvStorePrefix}${name}`); + } + } +} diff --git a/packages/lib/services/keychain/KeychainServiceDriver.mobile.ts b/packages/lib/services/keychain/KeychainServiceDriver.mobile.ts index 7617aadc6..ac83ec308 100644 --- a/packages/lib/services/keychain/KeychainServiceDriver.mobile.ts +++ b/packages/lib/services/keychain/KeychainServiceDriver.mobile.ts @@ -1,6 +1,11 @@ import KeychainServiceDriverBase from './KeychainServiceDriverBase'; export default class KeychainServiceDriver extends KeychainServiceDriverBase { + public override readonly driverId: string = 'mobile-unknown'; + + public async supported(): Promise { + return false; + } public async setPassword(/* name:string, password:string*/): Promise { return false; @@ -14,4 +19,6 @@ export default class KeychainServiceDriver extends KeychainServiceDriverBase { } + public async upgradeStorageBackend(_secureKeys: string[], _newDatabaseVersion: number): Promise { + } } diff --git a/packages/lib/services/keychain/KeychainServiceDriver.node.ts b/packages/lib/services/keychain/KeychainServiceDriver.node.ts index 1c3039cbc..23dcca593 100644 --- a/packages/lib/services/keychain/KeychainServiceDriver.node.ts +++ b/packages/lib/services/keychain/KeychainServiceDriver.node.ts @@ -2,20 +2,22 @@ import KeychainServiceDriverBase from './KeychainServiceDriverBase'; import shim from '../../shim'; export default class KeychainServiceDriver extends KeychainServiceDriverBase { + public override readonly driverId: string = 'node-keytar'; + + public async supported(): Promise { + return !!shim.keytar(); + } public async setPassword(name: string, password: string): Promise { - if (!shim.keytar()) return false; await shim.keytar().setPassword(`${this.appId}.${name}`, `${this.clientId}@joplin`, password); return true; } public async password(name: string): Promise { - if (!shim.keytar()) return null; return shim.keytar().getPassword(`${this.appId}.${name}`, `${this.clientId}@joplin`); } public async deletePassword(name: string): Promise { - if (!shim.keytar()) return; await shim.keytar().deletePassword(`${this.appId}.${name}`, `${this.clientId}@joplin`); } diff --git a/packages/lib/services/keychain/KeychainServiceDriverBase.ts b/packages/lib/services/keychain/KeychainServiceDriverBase.ts index e76392726..172d7275d 100644 --- a/packages/lib/services/keychain/KeychainServiceDriverBase.ts +++ b/packages/lib/services/keychain/KeychainServiceDriverBase.ts @@ -16,10 +16,12 @@ abstract class KeychainServiceDriverBase { return this.clientId_; } + public abstract readonly driverId: string; + public abstract supported(): Promise; + public abstract setPassword(name: string, password: string): Promise; public abstract setPassword(name: string, password: string): Promise; public abstract password(name: string): Promise; public abstract deletePassword(name: string): Promise; - } export default KeychainServiceDriverBase; diff --git a/packages/lib/shim.ts b/packages/lib/shim.ts index fb7bd0861..d90b89ea6 100644 --- a/packages/lib/shim.ts +++ b/packages/lib/shim.ts @@ -19,6 +19,12 @@ export interface PdfInfo { pageCount: number; } +export interface Keytar { + setPassword(key: string, client: string, password: string): Promise; + getPassword(key: string, client: string): Promise; + deletePassword(key: string, client: string): Promise; +} + interface FetchOptions { method?: string; headers?: Record; @@ -485,8 +491,7 @@ const shim = { return (shim.isWindows() || shim.isMac()) && !shim.isPortable(); }, - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - keytar: (): any => { + keytar: (): Keytar => { throw new Error('Not implemented'); }, diff --git a/packages/lib/testing/test-utils.ts b/packages/lib/testing/test-utils.ts index 169b4674c..9d8950772 100644 --- a/packages/lib/testing/test-utils.ts +++ b/packages/lib/testing/test-utils.ts @@ -11,7 +11,7 @@ import uuid from '../uuid'; import ResourceService from '../services/ResourceService'; import KeymapService from '../services/KeymapService'; import KvStore from '../services/KvStore'; -import KeychainServiceDriver from '../services/keychain/KeychainServiceDriver.node'; +import KeychainServiceDriverNode from '../services/keychain/KeychainServiceDriver.node'; import KeychainServiceDriverDummy from '../services/keychain/KeychainServiceDriver.dummy'; import FileApiDriverJoplinServer from '../file-api-driver-joplinServer'; import OneDriveApi from '../onedrive-api'; @@ -281,6 +281,7 @@ async function switchClient(id: number, options: any = null) { currentClient_ = id; BaseModel.setDb(databases_[id]); + KvStore.instance().setDb(databases_[id]); BaseItem.encryptionService_ = encryptionServices_[id]; Resource.encryptionService_ = encryptionServices_[id]; @@ -296,7 +297,7 @@ async function switchClient(id: number, options: any = null) { Setting.setConstant('pluginDir', pluginDir(id)); Setting.setConstant('isSubProfile', false); - await loadKeychainServiceAndSettings(options.keychainEnabled ? KeychainServiceDriver : KeychainServiceDriverDummy); + await loadKeychainServiceAndSettings([options.keychainEnabled ? KeychainServiceDriverNode : KeychainServiceDriverDummy]); Setting.setValue('sync.target', syncTargetId()); Setting.setValue('sync.wipeOutFailSafe', false); // To keep things simple, always disable fail-safe unless explicitly set in the test itself @@ -360,7 +361,7 @@ async function setupDatabase(id: number = null, options: any = null) { if (databases_[id]) { BaseModel.setDb(databases_[id]); await clearDatabase(id); - await loadKeychainServiceAndSettings(options.keychainEnabled ? KeychainServiceDriver : KeychainServiceDriverDummy); + await loadKeychainServiceAndSettings([options.keychainEnabled ? KeychainServiceDriverNode : KeychainServiceDriverDummy]); Setting.setValue('sync.target', syncTargetId()); return; } @@ -379,7 +380,7 @@ async function setupDatabase(id: number = null, options: any = null) { BaseModel.setDb(databases_[id]); await clearSettingFile(id); - await loadKeychainServiceAndSettings(options.keychainEnabled ? KeychainServiceDriver : KeychainServiceDriverDummy); + await loadKeychainServiceAndSettings([options.keychainEnabled ? KeychainServiceDriverNode : KeychainServiceDriverDummy]); reg.setDb(databases_[id]); Setting.setValue('sync.target', syncTargetId());