diff --git a/packages/app-desktop/gui/EncryptionConfigScreen/EncryptionConfigScreen.tsx b/packages/app-desktop/gui/EncryptionConfigScreen/EncryptionConfigScreen.tsx
index 3fa39cff8b..8fd628e14a 100644
--- a/packages/app-desktop/gui/EncryptionConfigScreen/EncryptionConfigScreen.tsx
+++ b/packages/app-desktop/gui/EncryptionConfigScreen/EncryptionConfigScreen.tsx
@@ -5,7 +5,7 @@ import { _ } from '@joplin/lib/locale';
import time from '@joplin/lib/time';
import shim from '@joplin/lib/shim';
import dialogs from '../dialogs';
-import { decryptedStatText, dontReencryptData, enableEncryptionConfirmationMessages, onSavePasswordClick, onToggleEnabledClick, reencryptData, upgradeMasterKey, useInputPasswords, usePasswordChecker, useStats, useToggleShowDisabledMasterKeys } from '@joplin/lib/components/EncryptionConfigScreen/utils';
+import { decryptedStatText, dontReencryptData, enableEncryptionConfirmationMessages, onSavePasswordClick, onToggleEnabledClick, reencryptData, upgradeMasterKey, useInputPasswords, useNeedMasterPassword, usePasswordChecker, useStats, useToggleShowDisabledMasterKeys } from '@joplin/lib/components/EncryptionConfigScreen/utils';
import { MasterKeyEntity } from '@joplin/lib/services/e2ee/types';
import { getEncryptionEnabled, masterKeyEnabled, SyncInfo } from '@joplin/lib/services/synchronizer/syncInfoUtils';
import { getDefaultMasterKey, getMasterPasswordStatusMessage, masterPasswordIsValid, toggleAndSetupEncryption } from '@joplin/lib/services/e2ee/utils';
@@ -39,6 +39,7 @@ const EncryptionConfigScreen = (props: Props) => {
const stats = useStats();
const { passwordChecks, masterPasswordKeys, masterPasswordStatus } = usePasswordChecker(props.masterKeys, props.activeMasterKeyId, props.masterPassword, props.passwords);
const { showDisabledMasterKeys, toggleShowDisabledMasterKeys } = useToggleShowDisabledMasterKeys();
+ const needMasterPassword = useNeedMasterPassword(passwordChecks, props.masterKeys);
const onUpgradeMasterKey = useCallback((mk: MasterKeyEntity) => {
void upgradeMasterKey(mk, passwordChecks, props.passwords);
@@ -263,9 +264,8 @@ const EncryptionConfigScreen = (props: Props) => {
};
const buttonTitle = CommandService.instance().label('openMasterPasswordDialog');
- const needPassword = Object.values(passwordChecks).includes(false);
- const needPasswordMessage = !needPassword ? null : (
+ const needPasswordMessage = !needMasterPassword ? null : (
{_('Your master password is needed to decrypt some of your data.')}
{_('Please click on "%s" to proceed', buttonTitle)}
);
@@ -275,7 +275,7 @@ const EncryptionConfigScreen = (props: Props) => {
{_('Master password')}
{_('Master password:')} {getMasterPasswordStatusMessage(masterPasswordStatus)}
{needPasswordMessage}
-
+
);
diff --git a/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx b/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx
index b6bdab3475..ee7b635dac 100644
--- a/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx
+++ b/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx
@@ -146,6 +146,7 @@ export default function(props: Props) {
const renderResetMasterPasswordLink = () => {
if (mode === Mode.Reset) return null;
+ if (status === MasterPasswordStatus.Valid) return null;
return Reset master password
;
};
diff --git a/packages/app-mobile/root.tsx b/packages/app-mobile/root.tsx
index e29b45b490..7c7076b03c 100644
--- a/packages/app-mobile/root.tsx
+++ b/packages/app-mobile/root.tsx
@@ -487,6 +487,18 @@ async function initialize(dispatch: Function) {
await loadKeychainServiceAndSettings(KeychainServiceDriverMobile);
await migrateMasterPassword();
+ if (!Setting.value('clientId')) Setting.setValue('clientId', uuid.create());
+
+ if (Setting.value('firstStart')) {
+ let locale = NativeModules.I18nManager.localeIdentifier;
+ if (!locale) locale = defaultLocale();
+ Setting.setValue('locale', closestSupportedLocale(locale));
+ Setting.skipDefaultMigrations();
+ Setting.setValue('firstStart', 0);
+ } else {
+ Setting.applyDefaultMigrations();
+ }
+
if (Setting.value('env') === Env.Dev) {
// Setting.setValue('sync.10.path', 'https://api.joplincloud.com');
// Setting.setValue('sync.10.userContentPath', 'https://joplinusercontent.com');
@@ -498,16 +510,6 @@ async function initialize(dispatch: Function) {
Setting.setValue('sync.10.password', 'hunter1hunter2hunter3');
}
- if (!Setting.value('clientId')) Setting.setValue('clientId', uuid.create());
-
- if (Setting.value('firstStart')) {
- let locale = NativeModules.I18nManager.localeIdentifier;
- if (!locale) locale = defaultLocale();
- Setting.setValue('locale', closestSupportedLocale(locale));
- Setting.setValue('sync.target', 0);
- Setting.setValue('firstStart', 0);
- }
-
if (Setting.value('db.ftsEnabled') === -1) {
const ftsEnabled = await db.ftsEnabled();
Setting.setValue('db.ftsEnabled', ftsEnabled ? 1 : 0);
diff --git a/packages/lib/BaseApplication.ts b/packages/lib/BaseApplication.ts
index 164007165f..905d4a95fa 100644
--- a/packages/lib/BaseApplication.ts
+++ b/packages/lib/BaseApplication.ts
@@ -787,15 +787,17 @@ export default class BaseApplication {
const locale = shim.detectAndSetLocale(Setting);
reg.logger().info(`First start: detected locale as ${locale}`);
+ Setting.skipDefaultMigrations();
+
if (Setting.value('env') === 'dev') {
Setting.setValue('showTrayIcon', 0);
Setting.setValue('autoUpdateEnabled', 0);
Setting.setValue('sync.interval', 3600);
}
- Setting.setValue('sync.target', 0);
Setting.setValue('firstStart', 0);
} else {
+ Setting.applyDefaultMigrations();
setLocale(Setting.value('locale'));
}
diff --git a/packages/lib/components/EncryptionConfigScreen/utils.ts b/packages/lib/components/EncryptionConfigScreen/utils.ts
index bede33a86b..b39879ebfd 100644
--- a/packages/lib/components/EncryptionConfigScreen/utils.ts
+++ b/packages/lib/components/EncryptionConfigScreen/utils.ts
@@ -188,6 +188,16 @@ export const usePasswordChecker = (masterKeys: MasterKeyEntity[], activeMasterKe
return { passwordChecks, masterPasswordKeys, masterPasswordStatus };
};
+export const useNeedMasterPassword = (passwordChecks: PasswordChecks, masterKeys: MasterKeyEntity[]) => {
+ for (const [mkId, valid] of Object.entries(passwordChecks)) {
+ const mk = masterKeys.find(mk => mk.id === mkId);
+ if (!mk) continue;
+ if (!masterKeyEnabled(mk)) continue;
+ if (!valid) return true;
+ }
+ return false;
+};
+
export const upgradeMasterKey = async (masterKey: MasterKeyEntity, passwordChecks: PasswordChecks, passwords: Record): Promise => {
const passwordCheck = passwordChecks[masterKey.id];
if (!passwordCheck) {
diff --git a/packages/lib/models/Setting.test.ts b/packages/lib/models/Setting.test.ts
index 1b15c05c4e..b6ded4edad 100644
--- a/packages/lib/models/Setting.test.ts
+++ b/packages/lib/models/Setting.test.ts
@@ -216,4 +216,36 @@ describe('models/Setting', function() {
expect(await fs.readFile(`${Setting.value('profileDir')}/${files[0]}`, 'utf8')).toBe(badContent);
}));
+ it('should allow applying default migrations', (async () => {
+ await Setting.reset();
+
+ expect(Setting.value('sync.target')).toBe(0);
+ expect(Setting.value('style.editor.contentMaxWidth')).toBe(0);
+ Setting.applyDefaultMigrations();
+ expect(Setting.value('sync.target')).toBe(7); // Changed
+ expect(Setting.value('style.editor.contentMaxWidth')).toBe(600); // Changed
+ }));
+
+ it('should allow skipping default migrations', (async () => {
+ await Setting.reset();
+
+ expect(Setting.value('sync.target')).toBe(0);
+ expect(Setting.value('style.editor.contentMaxWidth')).toBe(0);
+ Setting.skipDefaultMigrations();
+ Setting.applyDefaultMigrations();
+ expect(Setting.value('sync.target')).toBe(0); // Not changed
+ expect(Setting.value('style.editor.contentMaxWidth')).toBe(0); // Not changed
+ }));
+
+ it('should not apply migrations that have already been applied', (async () => {
+ await Setting.reset();
+
+ Setting.setValue('lastSettingDefaultMigration', 0);
+ expect(Setting.value('sync.target')).toBe(0);
+ expect(Setting.value('style.editor.contentMaxWidth')).toBe(0);
+ Setting.applyDefaultMigrations();
+ expect(Setting.value('sync.target')).toBe(0); // Not changed
+ expect(Setting.value('style.editor.contentMaxWidth')).toBe(600); // Changed
+ }));
+
});
diff --git a/packages/lib/models/Setting.ts b/packages/lib/models/Setting.ts
index 3fe91c65a5..a079d42df8 100644
--- a/packages/lib/models/Setting.ts
+++ b/packages/lib/models/Setting.ts
@@ -6,11 +6,14 @@ import Database from '../database';
import SyncTargetRegistry from '../SyncTargetRegistry';
import time from '../time';
import FileHandler, { SettingValues } from './settings/FileHandler';
+import Logger from '../Logger';
const { sprintf } = require('sprintf-js');
const ObjectUtils = require('../ObjectUtils');
const { toTitleCase } = require('../string-utils.js');
const { rtrimSlashes, toSystemSlashes } = require('../path-utils');
+const logger = Logger.create('models/Setting');
+
export enum SettingItemType {
Int = 1,
String = 2,
@@ -122,6 +125,54 @@ interface SettingSections {
[key: string]: SettingSection;
}
+// "Default migrations" are used to migrate previous setting defaults to new
+// values. If we simply change the default in the metadata, it might cause
+// problems if the user has never previously set the value.
+//
+// It happened for example when changing the "sync.target" from 7 (Dropbox) to 0
+// (None). Users who had never explicitly set the sync target and were using
+// Dropbox would suddenly have their sync target set to "none".
+//
+// So the technique is like this:
+//
+// - If the app has previously been executed, we run the migrations, which do
+// something like this:
+// - If the setting has never been set, set it to the previous default
+// value. For example, for sync.target, it would set it to "7".
+// - If the setting has been explicitly set, keep the current value.
+// - If the app runs for the first time, skip all the migrations. So
+// "sync.target" would be set to 0.
+//
+// A default migration runs only once (or never, if it is skipped).
+//
+// The handlers to either apply or skip the migrations must be called from the
+// application, in the initialization code.
+
+interface DefaultMigration {
+ name: string;
+ previousDefault: any;
+}
+
+// To create a default migration:
+//
+// - Set the new default value in the setting metadata
+// - Add an entry below with the name of the setting and the **previous**
+// default value.
+//
+// **Never** removes an item from this array, as the array index is essentially
+// the migration ID.
+
+const defaultMigrations: DefaultMigration[] = [
+ {
+ name: 'sync.target',
+ previousDefault: 7,
+ },
+ {
+ name: 'style.editor.contentMaxWidth',
+ previousDefault: 600,
+ },
+];
+
class Setting extends BaseModel {
public static schemaUrl = 'https://joplinapp.org/schema/settings.json';
@@ -319,7 +370,7 @@ class Setting extends BaseModel {
},
'sync.target': {
- value: 7, // Dropbox
+ value: 0,
type: SettingItemType.Int,
isEnum: true,
public: true,
@@ -985,7 +1036,7 @@ class Setting extends BaseModel {
storage: SettingStorage.File,
},
- 'style.editor.contentMaxWidth': { value: 600, type: SettingItemType.Int, public: true, storage: SettingStorage.File, appTypes: [AppType.Desktop], section: 'appearance', label: () => _('Editor maximum width'), description: () => _('Set it to 0 to make it take the complete available space.') },
+ 'style.editor.contentMaxWidth': { value: 0, type: SettingItemType.Int, public: true, storage: SettingStorage.File, appTypes: [AppType.Desktop], section: 'appearance', label: () => _('Editor maximum width'), description: () => _('Set it to 0 to make it take the complete available space. Recommended width is 600.') },
'ui.layout': { value: {}, type: SettingItemType.Object, storage: SettingStorage.File, public: false, appTypes: [AppType.Desktop] },
@@ -1289,6 +1340,12 @@ class Setting extends BaseModel {
storage: SettingStorage.Database,
},
+ lastSettingDefaultMigration: {
+ value: -1,
+ type: SettingItemType.Int,
+ public: false,
+ },
+
// 'featureFlag.syncAccurateTimestamps': {
// value: false,
// type: SettingItemType.Bool,
@@ -1310,6 +1367,35 @@ class Setting extends BaseModel {
return this.metadata_;
}
+ public static skipDefaultMigrations() {
+ logger.info('Skipping all default migrations...');
+
+ this.setValue('lastSettingDefaultMigration', defaultMigrations.length - 1);
+ }
+
+ public static applyDefaultMigrations() {
+ logger.info('Applying default migrations...');
+ const lastSettingDefaultMigration: number = this.value('lastSettingDefaultMigration');
+
+ for (let i = 0; i < defaultMigrations.length; i++) {
+ if (i <= lastSettingDefaultMigration) continue;
+
+ const migration = defaultMigrations[i];
+
+ logger.info(`Applying default migration: ${migration.name}`);
+
+ if (this.isSet(migration.name)) {
+ logger.info('Skipping because value is already set');
+ continue;
+ } else {
+ logger.info(`Applying previous default: ${migration.previousDefault}`);
+ this.setValue(migration.name, migration.previousDefault);
+ }
+ }
+
+ this.setValue('lastSettingDefaultMigration', defaultMigrations.length - 1);
+ }
+
public static featureFlagKeys(appType: AppType): string[] {
const keys = this.keys(false, appType);
return keys.filter(k => k.indexOf('featureFlag.') === 0);
@@ -1370,6 +1456,10 @@ class Setting extends BaseModel {
return key in this.metadata();
}
+ public static isSet(key: string) {
+ return key in this.cache_;
+ }
+
static keyDescription(key: string, appType: AppType = null) {
const md = this.settingMetadata(key);
if (!md.description) return null;
@@ -1545,7 +1635,7 @@ class Setting extends BaseModel {
this.changedKeys_.push(key);
// Don't log this to prevent sensitive info (passwords, auth tokens...) to end up in logs
- // this.logger().info('Setting: ' + key + ' = ' + c.value + ' => ' + value);
+ // logger.info('Setting: ' + key + ' = ' + c.value + ' => ' + value);
if ('minimum' in md && value < md.minimum) value = md.minimum;
if ('maximum' in md && value > md.maximum) value = md.maximum;
@@ -1774,7 +1864,7 @@ class Setting extends BaseModel {
public static async saveAll() {
if (Setting.autoSaveEnabled && !this.saveTimeoutId_) return Promise.resolve();
- this.logger().debug('Saving settings...');
+ logger.debug('Saving settings...');
shim.clearTimeout(this.saveTimeoutId_);
this.saveTimeoutId_ = null;
@@ -1814,7 +1904,7 @@ class Setting extends BaseModel {
continue;
}
} catch (error) {
- this.logger().error(`Could not set setting on the keychain. Will be saved to database instead: ${s.key}:`, error);
+ logger.error(`Could not set setting on the keychain. Will be saved to database instead: ${s.key}:`, error);
}
}
@@ -1832,7 +1922,7 @@ class Setting extends BaseModel {
if (this.canUseFileStorage()) await this.fileHandler.save(valuesForFile);
- this.logger().debug('Settings have been saved.');
+ logger.debug('Settings have been saved.');
}
static scheduleChangeEvent() {
@@ -1856,7 +1946,7 @@ class Setting extends BaseModel {
if (!this.changedKeys_.length) {
// Sanity check - shouldn't happen
- this.logger().warn('Trying to dispatch a change event without any changed keys');
+ logger.warn('Trying to dispatch a change event without any changed keys');
return;
}
@@ -1874,7 +1964,7 @@ class Setting extends BaseModel {
try {
await this.saveAll();
} catch (error) {
- this.logger().error('Could not save settings', error);
+ logger.error('Could not save settings', error);
}
}, 500);
}