self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2025-06-12 22:57:38 +02:00
Code Issues Releases Activity

Desktop: Resolves #2773: Add support for system keychain to save sensitive settings (#3207)

Browse Source
This commit is contained in:
Laurent Cozic
2020-06-03 17:07:50 +01:00
committed by GitHub
parent 5082181c49
commit 26ce102113
28 changed files with 661 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

115
ReactNativeClient/lib/models/Setting.js
View File

@ -18,6 +18,15 @@ class Setting extends BaseModel {
return BaseModel.TYPE_SETTING;
}
static keychainService() {
if (!this.keychainService_) throw new Error('keychainService has not been set!!');
return this.keychainService_;
}
static setKeychainService(s) {
this.keychainService_ = s;
}
static metadata() {
if (this.metadata_) return this.metadata_;
@ -295,7 +304,8 @@ class Setting extends BaseModel {
options: () => themeOptions(),
},
showNoteCounts: { value: true, type: Setting.TYPE_BOOL, public: true, advanced: true, appTypes: ['desktop'], label: () => _('Show note counts') },
showNoteCounts: { value: true, type: Setting.TYPE_BOOL, public: false, advanced: true, appTypes: ['desktop'], label: () => _('Show note counts') },
layoutButtonSequence: {
value: Setting.LAYOUT_ALL,
type: Setting.TYPE_INT,
@ -441,6 +451,7 @@ class Setting extends BaseModel {
collapsedFolderIds: { value: [], type: Setting.TYPE_ARRAY, public: false },
'keychain.supported': { value: -1, type: Setting.TYPE_INT, public: false },
'db.ftsEnabled': { value: -1, type: Setting.TYPE_INT, public: false },
'encryption.enabled': { value: false, type: Setting.TYPE_BOOL, public: false },
'encryption.activeMasterKeyId': { value: '', type: Setting.TYPE_STRING, public: false },
@ -735,7 +746,15 @@ class Setting extends BaseModel {
return md.description(appType);
}
static keys(publicOnly = false, appType = null) {
static isSecureKey(key) {
return this.metadata()[key] && this.metadata()[key].secure === true;
}
static keys(publicOnly = false, appType = null, options = null) {
options = Object.assign({}, {
secureOnly: false,
}, options);
if (!this.keys_) {
const metadata = this.metadata();
this.keys_ = [];
@ -745,12 +764,13 @@ class Setting extends BaseModel {
}
}
if (appType || publicOnly) {
if (appType || publicOnly || options.secureOnly) {
const output = [];
for (let i = 0; i < this.keys_.length; i++) {
const md = this.settingMetadata(this.keys_[i]);
if (publicOnly && !md.public) continue;
if (appType && md.appTypes && md.appTypes.indexOf(appType) < 0) continue;
if (options.secureOnly && !md.secure) continue;
output.push(md.key);
}
return output;
@ -763,22 +783,53 @@ class Setting extends BaseModel {
return this.keys(true).indexOf(key) >= 0;
}
// Low-level method to load a setting directly from the database. Should not be used in most cases.
static loadOne(key) {
return this.modelSelectOne('SELECT * FROM settings WHERE key = ?', [key]);
}
static load() {
this.cancelScheduleSave();
this.cache_ = [];
return this.modelSelectAll('SELECT * FROM settings').then(rows => {
return this.modelSelectAll('SELECT * FROM settings').then(async (rows) => {
this.cache_ = [];
for (let i = 0; i < rows.length; i++) {
const c = rows[i];
const pushItemsToCache = (items) => {
for (let i = 0; i < items.length; i++) {
const c = items[i];
if (!this.keyExists(c.key)) continue;
c.value = this.formatValue(c.key, c.value);
c.value = this.filterValue(c.key, c.value);
if (!this.keyExists(c.key)) continue;
this.cache_.push(c);
c.value = this.formatValue(c.key, c.value);
c.value = this.filterValue(c.key, c.value);
this.cache_.push(c);
}
};
// Keys in the database takes precedence over keys in the keychain because
// they are more likely to be up to date (saving to keychain can fail, but
// saving to database shouldn't). When the keychain works, the secure keys
// are deleted from the database and transfered to the keychain in saveAll().
const rowKeys = rows.map(r => r.key);
const secureKeys = this.keys(false, null, { secureOnly: true });
const secureItems = [];
for (const key of secureKeys) {
if (rowKeys.includes(key)) continue;
const password = await this.keychainService().password(`setting.${key}`);
if (password) {
secureItems.push({
key: key,
value: password,
});
}
}
pushItemsToCache(rows);
pushItemsToCache(secureItems);
this.dispatchUpdateAll();
});
}
@ -878,6 +929,13 @@ class Setting extends BaseModel {
this.setValue(settingKey, o);
}
static async deleteKeychainPasswords() {
const secureKeys = this.keys(false, null, { secureOnly: true });
for (const key of secureKeys) {
await this.keychainService().deletePassword(`setting.${key}`);
}
}
static valueToString(key, value) {
const md = this.settingMetadata(key);
value = this.formatValue(key, value);
@ -947,7 +1005,7 @@ class Setting extends BaseModel {
if (key in this.constants_) {
const v = this.constants_[key];
const output = typeof v === 'function' ? v() : v;
if (output == 'SET_ME') throw new Error(`Setting constant has not been set: ${key}`);
if (output == 'SET_ME') throw new Error(`SET_ME constant has not been set: ${key}`);
return output;
}
@ -1028,7 +1086,7 @@ class Setting extends BaseModel {
}
static async saveAll() {
if (!this.saveTimeoutId_) return Promise.resolve();
if (Setting.autoSaveEnabled && !this.saveTimeoutId_) return Promise.resolve();
this.logger().info('Saving settings...');
clearTimeout(this.saveTimeoutId_);
@ -1039,6 +1097,31 @@ class Setting extends BaseModel {
for (let i = 0; i < this.cache_.length; i++) {
const s = Object.assign({}, this.cache_[i]);
s.value = this.valueToString(s.key, s.value);
if (this.isSecureKey(s.key)) {
// We need to be careful here because there's a bug in the macOS keychain that can
// make it fail to save a password. https://github.com/desktop/desktop/issues/3263
// So we try to set it and if it fails, we set it on the database instead. This is not
// ideal because they won't be crypted, but better than losing all the user's passwords.
// The passwords would be set again on the keychain once it starts working again (probably
// after the user switch their computer off and on again).
//
// 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 !== s.value) {
const wasSet = await this.keychainService().setPassword(passwordName, s.value);
if (wasSet) continue;
}
} catch (error) {
this.logger().error(`Could not set setting on the keychain. Will be saved to database instead: ${s.key}:`, error);
}
}
queries.push(Database.insertQuery(this.tableName(), s));
}
@ -1052,8 +1135,12 @@ class Setting extends BaseModel {
if (this.saveTimeoutId_) clearTimeout(this.saveTimeoutId_);
this.saveTimeoutId_ = setTimeout(() => {
this.saveAll();
this.saveTimeoutId_ = setTimeout(async () => {
try {
await this.saveAll();
} catch (error) {
this.logger().error('Could not save settings', error);
}
}, 500);
}