diff --git a/.gitignore b/.gitignore index 30ff3761b3..b786c37d91 100644 --- a/.gitignore +++ b/.gitignore @@ -426,6 +426,8 @@ packages/app-desktop/gui/NoteListItem/utils/useRenderedNote.js packages/app-desktop/gui/NoteListItem/utils/useRootElement.test.js packages/app-desktop/gui/NoteListItem/utils/useRootElement.js packages/app-desktop/gui/NoteListWrapper/NoteListWrapper.js +packages/app-desktop/gui/NoteLockMigrationDialog/Dialog.test.js +packages/app-desktop/gui/NoteLockMigrationDialog/Dialog.js packages/app-desktop/gui/NoteLockUnlockDialog/Dialog.js packages/app-desktop/gui/NotePropertiesDialog.js packages/app-desktop/gui/NoteRevisionViewer.js @@ -950,6 +952,8 @@ packages/app-mobile/components/screens/Note/commands/insertDateTime.js packages/app-mobile/components/screens/Note/commands/setTags.js packages/app-mobile/components/screens/Note/commands/toggleVisiblePanes.js packages/app-mobile/components/screens/Note/types.js +packages/app-mobile/components/screens/NoteLockMigrationScreen.test.js +packages/app-mobile/components/screens/NoteLockMigrationScreen.js packages/app-mobile/components/screens/NoteRevisionViewer.test.js packages/app-mobile/components/screens/NoteRevisionViewer.js packages/app-mobile/components/screens/NoteTagsDialog.js @@ -1804,6 +1808,8 @@ packages/lib/services/noteList/renderViewProps.test.js packages/lib/services/noteList/renderViewProps.js packages/lib/services/noteList/renderers.js packages/lib/services/noteLock/NoteLockKey.js +packages/lib/services/noteLock/NoteLockKeyMigration.test.js +packages/lib/services/noteLock/NoteLockKeyMigration.js packages/lib/services/noteLock/NoteLockNote.js packages/lib/services/noteLock/NoteLockService.test.js packages/lib/services/noteLock/NoteLockService.js @@ -1965,6 +1971,7 @@ packages/lib/services/synchronizer/Synchronizer.baseFields.test.js packages/lib/services/synchronizer/Synchronizer.basics.test.js packages/lib/services/synchronizer/Synchronizer.conflicts.test.js packages/lib/services/synchronizer/Synchronizer.e2ee.test.js +packages/lib/services/synchronizer/Synchronizer.noteLock.test.js packages/lib/services/synchronizer/Synchronizer.ppk.test.js packages/lib/services/synchronizer/Synchronizer.resources.test.js packages/lib/services/synchronizer/Synchronizer.revisions.test.js diff --git a/.ignore.eslint b/.ignore.eslint index b0123dbbb3..61a90d08d6 100644 --- a/.ignore.eslint +++ b/.ignore.eslint @@ -452,6 +452,8 @@ packages/app-desktop/gui/NoteListItem/utils/useRenderedNote.js packages/app-desktop/gui/NoteListItem/utils/useRootElement.test.js packages/app-desktop/gui/NoteListItem/utils/useRootElement.js packages/app-desktop/gui/NoteListWrapper/NoteListWrapper.js +packages/app-desktop/gui/NoteLockMigrationDialog/Dialog.test.js +packages/app-desktop/gui/NoteLockMigrationDialog/Dialog.js packages/app-desktop/gui/NoteLockUnlockDialog/Dialog.js packages/app-desktop/gui/NotePropertiesDialog.js packages/app-desktop/gui/NoteRevisionViewer.js @@ -976,6 +978,8 @@ packages/app-mobile/components/screens/Note/commands/insertDateTime.js packages/app-mobile/components/screens/Note/commands/setTags.js packages/app-mobile/components/screens/Note/commands/toggleVisiblePanes.js packages/app-mobile/components/screens/Note/types.js +packages/app-mobile/components/screens/NoteLockMigrationScreen.test.js +packages/app-mobile/components/screens/NoteLockMigrationScreen.js packages/app-mobile/components/screens/NoteRevisionViewer.test.js packages/app-mobile/components/screens/NoteRevisionViewer.js packages/app-mobile/components/screens/NoteTagsDialog.js @@ -1830,6 +1834,8 @@ packages/lib/services/noteList/renderViewProps.test.js packages/lib/services/noteList/renderViewProps.js packages/lib/services/noteList/renderers.js packages/lib/services/noteLock/NoteLockKey.js +packages/lib/services/noteLock/NoteLockKeyMigration.test.js +packages/lib/services/noteLock/NoteLockKeyMigration.js packages/lib/services/noteLock/NoteLockNote.js packages/lib/services/noteLock/NoteLockService.test.js packages/lib/services/noteLock/NoteLockService.js @@ -1991,6 +1997,7 @@ packages/lib/services/synchronizer/Synchronizer.baseFields.test.js packages/lib/services/synchronizer/Synchronizer.basics.test.js packages/lib/services/synchronizer/Synchronizer.conflicts.test.js packages/lib/services/synchronizer/Synchronizer.e2ee.test.js +packages/lib/services/synchronizer/Synchronizer.noteLock.test.js packages/lib/services/synchronizer/Synchronizer.ppk.test.js packages/lib/services/synchronizer/Synchronizer.resources.test.js packages/lib/services/synchronizer/Synchronizer.revisions.test.js diff --git a/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx b/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx index 1368e8bbbf..a99e2037de 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx @@ -787,6 +787,7 @@ function NoteEditorContent(props: NoteEditorProps) { noteTitle={lockedNoteMetadata.title} hasNoteLockKey={props.hasNoteLockKey} dispatch={props.dispatch} + migrationRunning={props.noteLockMigrationRunning} lockedInShare={noteIsLockedInShare(lockedNoteMetadata)} /> @@ -921,6 +922,7 @@ const mapStateToProps = (state: AppState, ownProps: ConnectProps) => { whiteboardForceMarkdown: windowState.whiteboardForceMarkdown ?? {}, noteLockSessionUnlocked: state.noteLockSessionUnlocked, hasNoteLockKey: hasNoteLockKey(state.settings['syncInfoCache']), + noteLockMigrationRunning: !!state.noteLockMigrationStatus?.running, editorNoteReloadTimeRequest: windowState.windowEditorNoteReloadTimeRequest, }; }; diff --git a/packages/app-desktop/gui/NoteEditor/NoteLockPanel/NoteLockPanel.tsx b/packages/app-desktop/gui/NoteEditor/NoteLockPanel/NoteLockPanel.tsx index 8f7f7d4c1f..014757299c 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteLockPanel/NoteLockPanel.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteLockPanel/NoteLockPanel.tsx @@ -11,6 +11,7 @@ interface Props { hasNoteLockKey: boolean; dispatch: Dispatch; undecryptable?: boolean; + migrationRunning?: boolean; lockedInShare?: boolean; } @@ -50,6 +51,10 @@ export default function NoteLockPanel(props: Props) { }, [props.dispatch]); const renderAction = () => { + if (props.migrationRunning) { + return

{_('This note cannot be read while a note lock key migration is in progress.')}

; + } + if (props.undecryptable) { return

{_('This note could not be unlocked. If it was locked prior to a password reset, the content is no longer recoverable.')}

; } diff --git a/packages/app-desktop/gui/NoteEditor/utils/types.ts b/packages/app-desktop/gui/NoteEditor/utils/types.ts index 9b2ccf9893..129028cbdf 100644 --- a/packages/app-desktop/gui/NoteEditor/utils/types.ts +++ b/packages/app-desktop/gui/NoteEditor/utils/types.ts @@ -76,6 +76,7 @@ export interface NoteEditorProps { whiteboardForceMarkdown: Record; noteLockSessionUnlocked: boolean; hasNoteLockKey: boolean; + noteLockMigrationRunning: boolean; editorNoteReloadTimeRequest: number; } diff --git a/packages/app-desktop/gui/NoteLockMigrationDialog/Dialog.test.tsx b/packages/app-desktop/gui/NoteLockMigrationDialog/Dialog.test.tsx new file mode 100644 index 0000000000..edd93294dc --- /dev/null +++ b/packages/app-desktop/gui/NoteLockMigrationDialog/Dialog.test.tsx @@ -0,0 +1,68 @@ +import * as React from 'react'; +import * as ReactDom from 'react-dom'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import shim from '@joplin/lib/shim'; +import NoteLockMigrationDialog from './Dialog'; +import { confirmAndSkipNoteLockKeyMigration, startNoteLockKeyMigration } from '@joplin/lib/services/noteLock/NoteLockKeyMigration'; + +jest.mock('@joplin/lib/services/noteLock/NoteLockKeyMigration', () => ({ + startNoteLockKeyMigration: jest.fn(), + confirmAndSkipNoteLockKeyMigration: jest.fn(), +})); + +// The dialog renders through a portal, jsdom has no modal dialog support, and DialogTitle relies on +// the React global the app bundle provides. +shim.setReactDom(ReactDom); +(globalThis as unknown as { React: typeof React }).React = React; +HTMLDialogElement.prototype.showModal = function() { this.open = true; }; +HTMLDialogElement.prototype.close = function() { this.open = false; }; + +const startMock = startNoteLockKeyMigration as jest.Mock; +const skipMock = confirmAndSkipNoteLockKeyMigration as jest.Mock; + +const renderDialog = () => { + const dispatch = jest.fn(); + render(); + fireEvent.change(screen.getByLabelText('Note lock password on this device'), { target: { value: 'local' } }); + fireEvent.change(screen.getByLabelText('Note lock password on the sync target'), { target: { value: 'target' } }); + return dispatch; +}; + +describe('NoteLockMigrationDialog/Dialog', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('should close once both passwords are accepted and leave the run to the banner', async () => { + startMock.mockImplementation(async (_local: string, _target: string, _dispatch: unknown, onStarted: ()=> void) => onStarted()); + const dispatch = renderDialog(); + + fireEvent.click(screen.getByText('Migrate')); + + await waitFor(() => expect(dispatch).toHaveBeenCalledWith({ type: 'DIALOG_CLOSE', name: 'noteLockMigration' })); + expect(startMock).toHaveBeenCalledWith('local', 'target', dispatch, expect.any(Function)); + expect(skipMock).not.toHaveBeenCalled(); + }); + + test('should report a wrong password and stay open', async () => { + startMock.mockRejectedValue(Object.assign(new Error('bad'), { name: 'OperationError' })); + const dispatch = renderDialog(); + + fireEvent.click(screen.getByText('Migrate')); + + expect((await screen.findByRole('alert')).textContent).toContain('Invalid password'); + expect(dispatch).not.toHaveBeenCalled(); + }); + + test.each([true, false])('should close only once the skip is confirmed (%s)', async (confirmed) => { + skipMock.mockResolvedValue(confirmed); + const dispatch = renderDialog(); + + fireEvent.click(screen.getByText('Skip')); + + await waitFor(() => expect(skipMock).toHaveBeenCalledWith(dispatch)); + if (confirmed) await waitFor(() => expect(dispatch).toHaveBeenCalledWith({ type: 'DIALOG_CLOSE', name: 'noteLockMigration' })); + else expect(dispatch).not.toHaveBeenCalled(); + expect(startMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app-desktop/gui/NoteLockMigrationDialog/Dialog.tsx b/packages/app-desktop/gui/NoteLockMigrationDialog/Dialog.tsx new file mode 100644 index 0000000000..64d3105942 --- /dev/null +++ b/packages/app-desktop/gui/NoteLockMigrationDialog/Dialog.tsx @@ -0,0 +1,91 @@ +import * as React from 'react'; +import { Dispatch } from 'redux'; +import { useCallback, useState } from 'react'; +import { _ } from '@joplin/lib/locale'; +import DialogButtonRow, { ClickEvent } from '../DialogButtonRow'; +import Dialog from '@joplin/lib/components/Dialog'; +import DialogTitle from '../DialogTitle'; +import LabelledPasswordInput from '../PasswordInput/LabelledPasswordInput'; +import { confirmAndSkipNoteLockKeyMigration, startNoteLockKeyMigration } from '@joplin/lib/services/noteLock/NoteLockKeyMigration'; + +interface Props { + themeId: number; + dispatch: Dispatch; +} + +export default function(props: Props) { + const [localPassword, setLocalPassword] = useState(''); + const [targetPassword, setTargetPassword] = useState(''); + const [migrating, setMigrating] = useState(false); + const [errorMessage, setErrorMessage] = useState(''); + + const onClose = useCallback(() => { + props.dispatch({ + type: 'DIALOG_CLOSE', + name: 'noteLockMigration', + }); + }, [props.dispatch]); + + const onLocalPasswordChange = useCallback((event: React.ChangeEvent) => { + setLocalPassword(event.target.value); + }, []); + + const onTargetPasswordChange = useCallback((event: React.ChangeEvent) => { + setTargetPassword(event.target.value); + }, []); + + const onButtonRowClick = useCallback(async (event: ClickEvent) => { + if (event.buttonName === 'cancel') { + onClose(); + return; + } + + if (event.buttonName === 'skip') { + if (await confirmAndSkipNoteLockKeyMigration(props.dispatch)) onClose(); + return; + } + + if (event.buttonName === 'ok') { + setMigrating(true); + setErrorMessage(''); + try { + await startNoteLockKeyMigration(localPassword, targetPassword, props.dispatch, onClose); + } catch (error) { + setErrorMessage(error.name === 'OperationError' ? _('Invalid password') : error.message); + setMigrating(false); + } + } + }, [localPassword, targetPassword, onClose, props.dispatch]); + + return ( + +
+ +
+

{_('The sync target uses a different note lock key to the one on your device. Your locked notes must be re-encrypted with the synced key before synchronisation can continue. Locked notes cannot be read while they are being migrated.')}

+
+ + +
+ {!!errorMessage &&

{errorMessage}

} +
+ +
+ + ); +} diff --git a/packages/app-desktop/gui/WarningBanner.tsx b/packages/app-desktop/gui/WarningBanner.tsx index d581304801..d3116d78d8 100644 --- a/packages/app-desktop/gui/WarningBanner.tsx +++ b/packages/app-desktop/gui/WarningBanner.tsx @@ -16,6 +16,9 @@ import { AppState } from '../app.reducer'; import { localSyncInfoFromState } from '@joplin/lib/services/synchronizer/syncInfoUtils'; import EncryptionService from '@joplin/lib/services/e2ee/EncryptionService'; import { showMissingMasterKeyMessage, showUnknownKeyFormatBanner } from '@joplin/lib/services/e2ee/utils'; +import { noteLockKeyConflictFromState } from '@joplin/lib/services/synchronizer/syncInfoUtils'; +import { NoteLockMigrationStatus } from '@joplin/lib/reducer'; +import { confirmAndSkipNoteLockKeyMigration, notMigratedMessage, withSkippedCount } from '@joplin/lib/services/noteLock/NoteLockKeyMigration'; import shouldShowMissingPasswordWarning from '@joplin/lib/components/shared/config/shouldShowMissingPasswordWarning'; import { connect } from 'react-redux'; @@ -34,6 +37,8 @@ interface Props { hasDisabledSyncItems: boolean; showUnknownKeyFormatMessage: boolean; showMissingMasterKeyMessage: boolean; + showNoteLockKeyConflictMessage: boolean; + noteLockMigrationStatus: NoteLockMigrationStatus|null; mustUpgradeAppMessage: string; syncTargetAppMinVersion: string; shouldSwitchToAppleSiliconVersion: boolean; @@ -76,6 +81,20 @@ const WarningBanner: React.FC = props => { }); }; + const onMigrateLockedNotes = () => { + props.dispatch({ + type: 'DIALOG_OPEN', + name: 'noteLockMigration', + }); + }; + + const onDismissNoteLockMigrationStatus = () => { + props.dispatch({ + type: 'NOTE_LOCK_MIGRATION_STATUS_SET', + value: null, + }); + }; + const onViewJoplinCloudLoginScreen = () => { props.dispatch({ type: 'NAV_GO', @@ -143,6 +162,28 @@ const WarningBanner: React.FC = props => { _('Set the password'), onViewSyncSettingsScreen, ); + } else if (props.noteLockMigrationStatus?.running) { + msg = renderNotificationMessage(_('Re-encrypting your locked notes with the synced key. Migration is in progress...')); + } else if (props.noteLockMigrationStatus?.failed || props.noteLockMigrationStatus?.awaitingDecryption) { + msg = renderNotificationMessage( + notMigratedMessage(props.noteLockMigrationStatus), + _('Retry'), + onMigrateLockedNotes, + _('Skip'), + () => void confirmAndSkipNoteLockKeyMigration(props.dispatch), + ); + } else if (props.noteLockMigrationStatus) { + msg = renderNotificationMessage( + withSkippedCount(_('The migration of your locked notes has completed.'), props.noteLockMigrationStatus), + _('Dismiss'), + onDismissNoteLockMigrationStatus, + ); + } else if (props.showNoteLockKeyConflictMessage) { + msg = renderNotificationMessage( + _('Synchronisation is stopped because the sync target uses a different note lock key to the one on your device.'), + _('Migrate your locked notes'), + onMigrateLockedNotes, + ); } else if (props.shouldUpgradeSyncTarget) { msg = renderNotificationMessage( _('The sync target needs to be upgraded before Joplin can sync. The operation may take a few minutes to complete and the app needs to be restarted. To proceed please click on the link.'), @@ -291,6 +332,8 @@ const mapStateToProps = (state: AppState) => { hasDisabledEncryptionItems: state.hasDisabledEncryptionItems, showMissingMasterKeyMessage: showMissingMasterKeyMessage(syncInfo, state.notLoadedMasterKeys), showUnknownKeyFormatMessage: showUnknownKeyFormatBanner(syncInfo), + showNoteLockKeyConflictMessage: noteLockKeyConflictFromState(state), + noteLockMigrationStatus: state.noteLockMigrationStatus, showNeedUpgradingMasterKeyMessage: showNeedUpgradingEnabledMasterKeyMessage, showShouldReencryptMessage: state.settings['encryption.shouldReencrypt'] >= Setting.SHOULD_REENCRYPT_YES, shouldUpgradeSyncTarget: state.settings['sync.upgradeState'] === Setting.SYNC_UPGRADE_STATE_SHOULD_DO, diff --git a/packages/app-desktop/gui/WindowCommandsAndDialogs/commands/enableNoteEncryption.ts b/packages/app-desktop/gui/WindowCommandsAndDialogs/commands/enableNoteEncryption.ts index fcb44078f8..0140be3902 100644 --- a/packages/app-desktop/gui/WindowCommandsAndDialogs/commands/enableNoteEncryption.ts +++ b/packages/app-desktop/gui/WindowCommandsAndDialogs/commands/enableNoteEncryption.ts @@ -65,6 +65,6 @@ export const runtime = (): CommandRuntime => { bridge().showErrorMessageBox(error.message); } }, - enabledCondition: 'oneNoteSelected && !noteIsLocked && !noteIsReadOnly && !noteIsDeleted && !inTrash && !inConflictFolder', + enabledCondition: 'oneNoteSelected && !noteIsLocked && !noteIsReadOnly && !noteIsDeleted && !inTrash && !inConflictFolder && !noteLockMigrationRunning', }; }; diff --git a/packages/app-desktop/gui/WindowCommandsAndDialogs/utils/appDialogs.tsx b/packages/app-desktop/gui/WindowCommandsAndDialogs/utils/appDialogs.tsx index 94a3f27d9b..9b55f33756 100644 --- a/packages/app-desktop/gui/WindowCommandsAndDialogs/utils/appDialogs.tsx +++ b/packages/app-desktop/gui/WindowCommandsAndDialogs/utils/appDialogs.tsx @@ -4,6 +4,7 @@ import { Dispatch } from 'redux'; import SyncWizardDialog from '../../SyncWizard/Dialog'; import MasterPasswordDialog from '../../MasterPasswordDialog/Dialog'; import NoteLockUnlockDialog from '../../NoteLockUnlockDialog/Dialog'; +import NoteLockMigrationDialog from '../../NoteLockMigrationDialog/Dialog'; import EditFolderDialog from '../../EditFolderDialog/Dialog'; import PdfViewer from '../../PdfViewer'; @@ -40,6 +41,13 @@ const appDialogs: Record = { }, }, + noteLockMigration: { + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- See RegisteredDialog.render + render: (props: RegisteredDialogProps, customProps: any) => { + return ; + }, + }, + editFolder: { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- See RegisteredDialog.render render: (props: RegisteredDialogProps, customProps: any) => { diff --git a/packages/app-desktop/main.scss b/packages/app-desktop/main.scss index 869c17ca04..a40b91b807 100644 --- a/packages/app-desktop/main.scss +++ b/packages/app-desktop/main.scss @@ -319,7 +319,13 @@ Component-specific classes max-width: 600px; } -.master-password-dialog .dialog-content, .enable-encryption-dialog .dialog-content, .disable-encryption-dialog .dialog-content { +.note-lock-migration-dialog > .content { + box-sizing: border-box; + width: 500px; + max-width: 100%; +} + +.master-password-dialog .dialog-content, .enable-encryption-dialog .dialog-content, .disable-encryption-dialog .dialog-content, .note-lock-migration-dialog .dialog-content { background-color: var(--joplin-background-color3); padding: 1em; padding-bottom: 1px; diff --git a/packages/app-mobile/components/ScreenHeader/WarningBanner.test.tsx b/packages/app-mobile/components/ScreenHeader/WarningBanner.test.tsx index 296534b46b..d0fc42ce92 100644 --- a/packages/app-mobile/components/ScreenHeader/WarningBanner.test.tsx +++ b/packages/app-mobile/components/ScreenHeader/WarningBanner.test.tsx @@ -11,6 +11,7 @@ import { Store } from 'redux'; import createMockReduxStore from '../../utils/testing/createMockReduxStore'; import setupGlobalStore from '../../utils/testing/setupGlobalStore'; import MasterKey from '@joplin/lib/models/MasterKey'; +import shim from '@joplin/lib/shim'; interface WrapperProps { store: Store; @@ -136,6 +137,47 @@ describe('WarningBanner', () => { expect(screen.getByText(query)).toBeVisible(); }); + test('the note lock key conflict banner should link to the migration screen', () => { + Setting.setValue('featureFlag.noteLock', true); + Setting.setValue('noteLock.conflictNoteLockKey', { noteLockKey: { id: 'sync-target-key' }, syncMigrationId: 'lineage' }); + const mock = createMockStore(); + + render(); + const banner = screen.getByText(/different note lock key/); + expect(banner).toBeVisible(); + fireEvent.press(banner); + + expect(mock.getRouteName()).toBe('NoteLockMigration'); + }); + + test('the note lock migration banner should follow the run and be dismissed once it has completed', () => { + Setting.setValue('featureFlag.noteLock', true); + Setting.setValue('noteLock.conflictNoteLockKey', { noteLockKey: { id: 'sync-target-key' }, syncMigrationId: 'lineage' }); + const mock = createMockStore(); + const setStatus = (value: unknown) => act(() => { + mock.store.dispatch({ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value }); + }); + render(); + + setStatus({ running: true, failed: 0, skipped: 0 }); + expect(screen.getByText(/Migration is in progress/)).toBeVisible(); + expect(screen.queryByText(/different note lock key/)).toBeNull(); + + setStatus({ running: false, failed: 2, skipped: 0 }); + expect(screen.getByText(/2 locked notes could not be migrated/)).toBeVisible(); + const showMessageBox = jest.spyOn(shim, 'showMessageBox').mockResolvedValue(1); + fireEvent.press(screen.getByText('Skip')); + expect(showMessageBox).toHaveBeenCalled(); + showMessageBox.mockRestore(); + fireEvent.press(screen.getByText('Retry')); + expect(mock.getRouteName()).toBe('NoteLockMigration'); + + setStatus({ running: false, failed: 0, skipped: 1 }); + expect(screen.getByText(/has completed.*1 locked note was skipped/)).toBeVisible(); + fireEvent.press(screen.getByText(/has completed/)); + expect(mock.store.getState().noteLockMigrationStatus).toBeNull(); + }); + test('invalid credentials banner for Joplin Cloud should link to a login screen', () => { Setting.setValue('sync.target', 10); const mock = createMockStore(); diff --git a/packages/app-mobile/components/ScreenHeader/WarningBanner.tsx b/packages/app-mobile/components/ScreenHeader/WarningBanner.tsx index c2a4947273..ef6d042d22 100644 --- a/packages/app-mobile/components/ScreenHeader/WarningBanner.tsx +++ b/packages/app-mobile/components/ScreenHeader/WarningBanner.tsx @@ -3,10 +3,14 @@ import { useState } from 'react'; import { connect } from 'react-redux'; import { Platform } from 'react-native'; import { AppState } from '../../utils/types'; -import WarningBox, { WarningBoxTarget } from './WarningBox'; +import WarningBox, { WarningBoxAction, WarningBoxTarget } from './WarningBox'; +import NavService from '@joplin/lib/services/NavService'; import { _ } from '@joplin/lib/locale'; import { showMissingMasterKeyMessage, showUnknownKeyFormatBanner } from '@joplin/lib/services/e2ee/utils'; -import { localSyncInfoFromState } from '@joplin/lib/services/synchronizer/syncInfoUtils'; +import { localSyncInfoFromState, noteLockKeyConflictFromState } from '@joplin/lib/services/synchronizer/syncInfoUtils'; +import { NoteLockMigrationStatus } from '@joplin/lib/reducer'; +import { confirmAndSkipNoteLockKeyMigration, notMigratedMessage, withSkippedCount } from '@joplin/lib/services/noteLock/NoteLockKeyMigration'; +import { Dispatch } from 'redux'; import Setting from '@joplin/lib/models/Setting'; import { ShareInvitation, ShareUserStatus } from '@joplin/lib/services/share/reducer'; import { substrWithEllipsis } from '@joplin/lib/string-utils'; @@ -30,6 +34,10 @@ interface Props { shareInvitations: ShareInvitation[]; processingShareInvitationResponse: boolean; showInvalidJoplinCloudCredential: boolean; + noteLockKeyConflict: boolean; + showNoteLockKeyConflictMessage: boolean|undefined; + noteLockMigrationStatus: NoteLockMigrationStatus|null; + dispatch: Dispatch; } const androidGooglePlayUrl = 'https://play.google.com/store/apps/details?id=net.cozic.joplin'; @@ -60,13 +68,14 @@ const WarningBannerComponent: React.FC = props => { } }, [props.mustUpgradeAppMessage, props.syncTargetAppMinVersion]); - const renderWarningBox = (key: string, message: string, target: WarningBoxTarget) => { + const renderWarningBox = (key: string, message: string, target: WarningBoxTarget, actions?: WarningBoxAction[]) => { return ; }; @@ -128,6 +137,22 @@ const WarningBannerComponent: React.FC = props => { if (props.hasDisabledEncryptionItems) { warningComps.push(renderWarningBox('cannotDecrypt', _('Some items cannot be decrypted.'), { screen: 'Status' })); } + const migration = props.noteLockMigrationStatus; + if (props.showNoteLockKeyConflictMessage !== false) { + if (migration?.running) { + warningComps.push(renderWarningBox('noteLockMigration', _('Re-encrypting your locked notes with the synced key. Migration is in progress...'), null)); + } else if (migration?.failed || migration?.awaitingDecryption) { + warningComps.push(renderWarningBox('noteLockMigration', notMigratedMessage(migration), null, [ + { title: _('Retry'), onPress: () => void NavService.go('NoteLockMigration') }, + { title: _('Skip'), onPress: () => void confirmAndSkipNoteLockKeyMigration(props.dispatch) }, + ])); + } else if (migration) { + const onDismiss = () => props.dispatch({ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: null }); + warningComps.push(renderWarningBox('noteLockMigration', `${withSkippedCount(_('The migration of your locked notes has completed.'), migration)} ${_('Press to dismiss.')}`, { onPress: onDismiss })); + } else if (props.noteLockKeyConflict) { + warningComps.push(renderWarningBox('noteLockMigration', _('The sync target uses a different note lock key to the one on your device. Press to migrate your locked notes.'), { screen: 'NoteLockMigration' })); + } + } if (props.showInvalidJoplinCloudCredential) { const target = { screen: 'JoplinCloudLogin' }; warningComps.push(renderWarningBox('syncLogin', _('Your Joplin Cloud credentials are invalid, please login.'), target)); @@ -185,5 +210,7 @@ export default connect((state: AppState) => { shareInvitations: state.shareService.shareInvitations, processingShareInvitationResponse: state.shareService.processingShareInvitationResponse, showInvalidJoplinCloudCredential: state.settings['sync.target'] === 10 && !isSyncLoginRoute(state) && state.mustAuthenticate, + noteLockKeyConflict: noteLockKeyConflictFromState(state), + noteLockMigrationStatus: state.noteLockMigrationStatus, }; })(WarningBannerComponent); diff --git a/packages/app-mobile/components/ScreenHeader/WarningBox.tsx b/packages/app-mobile/components/ScreenHeader/WarningBox.tsx index f9bbd1037c..7dc3f5eb58 100644 --- a/packages/app-mobile/components/ScreenHeader/WarningBox.tsx +++ b/packages/app-mobile/components/ScreenHeader/WarningBox.tsx @@ -17,13 +17,26 @@ interface ScreenTarget { url?: undefined; } -export type WarningBoxTarget = UrlTarget|ScreenTarget; +interface CallbackTarget { + onPress: ()=> void; + + url?: undefined; + screen?: undefined; +} + +export type WarningBoxTarget = UrlTarget|ScreenTarget|CallbackTarget; + +export interface WarningBoxAction { + title: string; + onPress: ()=> void; +} interface Props { themeId: number; target: WarningBoxTarget; message: string; testID?: string; + actions?: WarningBoxAction[]; } const useStyles = (themeId: number) => { @@ -39,6 +52,12 @@ const useStyles = (themeId: number) => { flex: 1, color: 'black', }, + actionText: { + color: 'black', + fontWeight: 'bold', + textDecorationLine: 'underline', + marginLeft: theme.marginLeft, + }, }); }, [themeId]); }; @@ -56,6 +75,8 @@ const WarningBox: React.FC = props => { const isUrlTarget = (target: WarningBoxTarget): target is UrlTarget => !!target.url; if (isUrlTarget(target)) { void Linking.openURL(target.url); + } else if ('onPress' in target) { + target.onPress(); } else { void NavService.go(target.screen, target.screenProps); } @@ -63,6 +84,18 @@ const WarningBox: React.FC = props => { const hasTarget = !!propsRef.current.target; const bannerContent = {props.message}; + if (props.actions) { + return ( + + {bannerContent} + {props.actions.map(action => ( + + {action.title} + + ))} + + ); + } if (hasTarget) { return ( ); diff --git a/packages/app-mobile/components/screens/Note/Note.tsx b/packages/app-mobile/components/screens/Note/Note.tsx index 0504a2e768..b3b3ca6563 100644 --- a/packages/app-mobile/components/screens/Note/Note.tsx +++ b/packages/app-mobile/components/screens/Note/Note.tsx @@ -39,6 +39,7 @@ import NoteLockSession from '@joplin/lib/services/noteLock/NoteLockSession'; import NoteLockKey, { DecryptedNoteLockKey } from '@joplin/lib/services/noteLock/NoteLockKey'; import NoteLockNote from '@joplin/lib/services/noteLock/NoteLockNote'; import { disableNoteLock, enableNoteLock } from '@joplin/lib/services/noteLock/setNoteLockState'; +import { ErrorCode } from '@joplin/lib/errors'; import eventManager, { EventName, NoteLockNoteStateChangeEvent } from '@joplin/lib/eventManager'; import NoteLockPanel from './NoteLockPanel'; import DismissibleDialog, { DialogVariant } from '../../DismissibleDialog'; @@ -135,6 +136,7 @@ interface Props extends BaseProps { canPublish: boolean; noteVisiblePanes: string[]; noteLockSessionUnlocked: boolean; + noteLockMigrationRunning: boolean; } interface ComponentProps extends Props { @@ -974,7 +976,7 @@ class NoteScreenComponent extends BaseScreenComponent imp } } catch (error) { reg.logger().warn('Could not change the note lock state:', error); - await this.props.dialogs.error(_('Could not update the note lock. Please try again.')); + await this.props.dialogs.error(error.code === ErrorCode.NoteLockKeyConflict ? error.message : _('Could not update the note lock. Please try again.')); } } @@ -1459,7 +1461,7 @@ class NoteScreenComponent extends BaseScreenComponent imp const pluginCommands = pluginUtils.commandNamesFromViews(this.props.plugins, 'noteToolbar'); - const cacheKey = md5([isTodo, isSaved, pluginCommands.join(','), readOnly, this.state.mode, isCodeView, note.is_locked, this.props.noteLockSessionUnlocked, !!this.state.noteLockKey, this.state.noteLockUndecryptable].join('_')); + const cacheKey = md5([isTodo, isSaved, pluginCommands.join(','), readOnly, this.state.mode, isCodeView, note.is_locked, this.props.noteLockSessionUnlocked, !!this.state.noteLockKey, this.state.noteLockUndecryptable, this.props.noteLockMigrationRunning].join('_')); if (!this.menuOptionsCache_) this.menuOptionsCache_ = {}; if (this.menuOptionsCache_[cacheKey]) return this.menuOptionsCache_[cacheKey]; @@ -1591,7 +1593,7 @@ class NoteScreenComponent extends BaseScreenComponent imp onPress: () => { this.enableNoteEncryption_onPress(); }, - disabled: readOnly, + disabled: readOnly || this.props.noteLockMigrationRunning, }); } else { output.push({ @@ -1923,6 +1925,7 @@ class NoteScreenComponent extends BaseScreenComponent imp themeId={this.props.themeId} hasNoteLockKey={!!NoteLockKey.instance().load()} undecryptable={this.state.noteLockUndecryptable} + migrationRunning={this.props.noteLockMigrationRunning} />; } else if (editorView) { bodyComponent = renderPluginEditor(); @@ -2238,6 +2241,7 @@ const NoteScreen = connect((state: AppState) => { useEditorBeta: !state.settings['editor.usePlainText'], canPublish: whenClause.joplinServerConnected && !whenClause.inTrash, noteLockSessionUnlocked: state.noteLockSessionUnlocked, + noteLockMigrationRunning: !!state.noteLockMigrationStatus?.running, }; })(NoteScreenWrapper); diff --git a/packages/app-mobile/components/screens/Note/NoteLockPanel.tsx b/packages/app-mobile/components/screens/Note/NoteLockPanel.tsx index 10efd2d578..9c78fc97f9 100644 --- a/packages/app-mobile/components/screens/Note/NoteLockPanel.tsx +++ b/packages/app-mobile/components/screens/Note/NoteLockPanel.tsx @@ -15,6 +15,7 @@ interface Props { themeId: number; hasNoteLockKey: boolean; undecryptable?: boolean; + migrationRunning?: boolean; onUnlocked?: ()=> void; } @@ -94,6 +95,10 @@ const NoteLockPanel = (props: Props) => { }, [password, unlocking, props.onUnlocked]); const renderForm = () => { + if (props.migrationRunning) { + return {_('This note cannot be read while a note lock key migration is in progress.')}; + } + if (props.undecryptable) { return ( {_('This note could not be unlocked. If it was locked prior to a password reset, the content is no longer recoverable.')} diff --git a/packages/app-mobile/components/screens/NoteLockMigrationScreen.test.tsx b/packages/app-mobile/components/screens/NoteLockMigrationScreen.test.tsx new file mode 100644 index 0000000000..a1f369c2aa --- /dev/null +++ b/packages/app-mobile/components/screens/NoteLockMigrationScreen.test.tsx @@ -0,0 +1,100 @@ +import * as React from 'react'; +import { Store } from 'redux'; +import Setting from '@joplin/lib/models/Setting'; +import { setupDatabaseAndSynchronizer, switchClient } from '@joplin/lib/testing/test-utils'; +import { confirmAndSkipNoteLockKeyMigration, startNoteLockKeyMigration } from '@joplin/lib/services/noteLock/NoteLockKeyMigration'; +import { NoteLockMigrationScreenComponent } from './NoteLockMigrationScreen'; +import TestProviderStack from '../testing/TestProviderStack'; +import createMockReduxStore from '../../utils/testing/createMockReduxStore'; +import setupGlobalStore from '../../utils/testing/setupGlobalStore'; +import { AppState } from '../../utils/types'; +import { fireEvent, render, screen, waitFor } from '../../utils/testing/testingLibrary'; +import BackButtonService from '../../services/BackButtonService'; + +jest.mock('@joplin/lib/services/noteLock/NoteLockKeyMigration', () => ({ + startNoteLockKeyMigration: jest.fn(), + confirmAndSkipNoteLockKeyMigration: jest.fn(), +})); + +const startMock = startNoteLockKeyMigration as jest.Mock; +const skipMock = confirmAndSkipNoteLockKeyMigration as jest.Mock; + +let store: Store; +const renderScreen = () => { + const dispatch = jest.fn(); + render( + + + , + ); + fireEvent.changeText(screen.getByLabelText('Note lock password on this device'), 'local'); + fireEvent.changeText(screen.getByLabelText('Note lock password on the sync target'), 'target'); + return dispatch; +}; + +describe('NoteLockMigrationScreen', () => { + beforeEach(async () => { + await setupDatabaseAndSynchronizer(0); + await switchClient(0); + store = createMockReduxStore(); + setupGlobalStore(store); + jest.clearAllMocks(); + }); + afterEach(() => { + screen.unmount(); + }); + + test('should go back once both passwords are accepted and leave the run to the banner', async () => { + startMock.mockImplementation(async (_local: string, _target: string, _dispatch: unknown, onStarted: ()=> void) => onStarted()); + const dispatch = renderScreen(); + + fireEvent.press(screen.getByText('Migrate')); + + await waitFor(() => expect(dispatch).toHaveBeenCalledWith({ type: 'NAV_BACK' })); + expect(startMock).toHaveBeenCalledWith('local', 'target', dispatch, expect.any(Function)); + expect(skipMock).not.toHaveBeenCalled(); + }); + + test('should block going back while the passwords are checked', async () => { + startMock.mockReturnValue(new Promise(() => {})); + renderScreen(); + expect(screen.getByLabelText('Back')).toBeVisible(); + + fireEvent.press(screen.getByText('Migrate')); + + await waitFor(() => expect(screen.queryByLabelText('Back')).toBeNull()); + expect(await BackButtonService.back()).toBe(true); + }); + + test('should not show the migration banner on its own screen', async () => { + Setting.setValue('featureFlag.noteLock', true); + Setting.setValue('noteLock.conflictNoteLockKey', { noteLockKey: { id: 'target-key' }, syncMigrationId: 'lineage' }); + store = createMockReduxStore(); + setupGlobalStore(store); + renderScreen(); + + expect(screen.queryByText(/Press to migrate/)).toBeNull(); + }); + + test('should report a wrong password and stay', async () => { + startMock.mockRejectedValue(Object.assign(new Error('bad'), { name: 'OperationError' })); + const dispatch = renderScreen(); + + fireEvent.press(screen.getByText('Migrate')); + + expect(await screen.findByRole('alert')).toHaveTextContent('Invalid password'); + expect(dispatch).not.toHaveBeenCalled(); + }); + + test.each([true, false])('should leave the screen only once the skip is confirmed (%s)', async (confirmed) => { + skipMock.mockResolvedValue(confirmed); + const dispatch = renderScreen(); + + fireEvent.press(screen.getByText('Skip')); + + await waitFor(() => expect(skipMock).toHaveBeenCalledWith(dispatch)); + if (confirmed) await waitFor(() => expect(dispatch).toHaveBeenCalledWith({ type: 'NAV_BACK' })); + else expect(dispatch).not.toHaveBeenCalled(); + expect(startMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app-mobile/components/screens/NoteLockMigrationScreen.tsx b/packages/app-mobile/components/screens/NoteLockMigrationScreen.tsx new file mode 100644 index 0000000000..e1c74c7753 --- /dev/null +++ b/packages/app-mobile/components/screens/NoteLockMigrationScreen.tsx @@ -0,0 +1,132 @@ +import * as React from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; +import { View, Text, TextInput, StyleSheet } from 'react-native'; +import { connect } from 'react-redux'; +import { Dispatch } from 'redux'; +import { themeStyle } from '../global-style'; +import { _ } from '@joplin/lib/locale'; +import { confirmAndSkipNoteLockKeyMigration, startNoteLockKeyMigration } from '@joplin/lib/services/noteLock/NoteLockKeyMigration'; +import ScreenHeader from '../ScreenHeader'; +import { PrimaryButton, SecondaryButton } from '../buttons'; +import { AppState } from '../../utils/types'; +import BackButtonService from '../../services/BackButtonService'; + +interface Props { + themeId: number; + dispatch: Dispatch; +} + +export const NoteLockMigrationScreenComponent: React.FC = props => { + const [localPassword, setLocalPassword] = useState(''); + const [targetPassword, setTargetPassword] = useState(''); + const [migrating, setMigrating] = useState(false); + const [errorMessage, setErrorMessage] = useState(''); + + const theme = useMemo(() => themeStyle(props.themeId), [props.themeId]); + + const styles = useMemo(() => { + return StyleSheet.create({ + root: theme.rootStyle, + container: { + padding: theme.margin, + }, + normalText: { + fontSize: theme.fontSize, + color: theme.color, + marginBottom: 5, + }, + // Same input style as ConfigScreen/NoteLockConfig.tsx + passwordInput: { + marginTop: 5, + marginBottom: theme.itemMarginBottom, + color: theme.color, + borderWidth: 1, + borderColor: theme.dividerColor, + borderRadius: 3, + padding: 10, + }, + errorText: { + fontSize: theme.fontSize, + color: theme.colorError, + marginBottom: theme.itemMarginBottom, + }, + buttonContainer: { + marginTop: theme.itemMarginTop, + gap: theme.margin, + }, + }); + }, [theme]); + + // Leaving while the passwords are checked would let the run start and navigate from under whatever screen replaced this one. + useEffect(() => { + if (!migrating) return () => {}; + const handler = () => true; + BackButtonService.addHandler(handler); + return () => BackButtonService.removeHandler(handler); + }, [migrating]); + + // Duplicates the migrate and skip handlers in app-desktop/gui/NoteLockMigrationDialog/Dialog.tsx. + const onMigrate = useCallback(async () => { + setMigrating(true); + setErrorMessage(''); + try { + await startNoteLockKeyMigration(localPassword, targetPassword, props.dispatch, () => props.dispatch({ type: 'NAV_BACK' })); + } catch (error) { + setErrorMessage(error.name === 'OperationError' ? _('Invalid password') : error.message); + setMigrating(false); + } + }, [localPassword, targetPassword, props.dispatch]); + + const onSkip = useCallback(async () => { + if (await confirmAndSkipNoteLockKeyMigration(props.dispatch)) props.dispatch({ type: 'NAV_BACK' }); + }, [props.dispatch]); + + const localLabelId = 'note-lock-migration-local-password'; + const targetLabelId = 'note-lock-migration-target-password'; + + return ( + + + + {_('The sync target uses a different note lock key to the one on your device. Your locked notes must be re-encrypted with the synced key before synchronisation can continue. Locked notes cannot be read while they are being migrated.')} + {_('Note lock password on this device')} + + {_('Note lock password on the sync target')} + + {!!errorMessage && {errorMessage}} + + {_('Migrate')} + {_('Skip')} + + + + ); +}; + +export default connect((state: AppState) => { + return { + themeId: state.settings.theme, + }; +})(NoteLockMigrationScreenComponent); diff --git a/packages/app-mobile/root.tsx b/packages/app-mobile/root.tsx index 017f2f5f70..1bd3d665d6 100644 --- a/packages/app-mobile/root.tsx +++ b/packages/app-mobile/root.tsx @@ -9,6 +9,7 @@ import time from '@joplin/lib/time'; import Logger from '@joplin/utils/Logger'; import NoteScreen from './components/screens/Note/Note'; import UpgradeSyncTargetScreen from './components/screens/UpgradeSyncTargetScreen'; +import NoteLockMigrationScreen from './components/screens/NoteLockMigrationScreen'; import Setting, { } from '@joplin/lib/models/Setting'; import PoorManIntervals from '@joplin/lib/PoorManIntervals'; import { NotesParent, serializeNotesParent } from '@joplin/lib/reducer'; @@ -776,6 +777,7 @@ class AppComponent extends React.Component JoplinServerSamlLogin: { screen: SsoLoginScreen(new SamlShared()) }, EncryptionConfig: { screen: EncryptionConfigScreen }, UpgradeSyncTarget: { screen: UpgradeSyncTargetScreen }, + NoteLockMigration: { screen: NoteLockMigrationScreen }, ShareManager: { screen: ShareManager }, ProfileSwitcher: { screen: ProfileSwitcher }, ProfileEditor: { screen: ProfileEditor }, diff --git a/packages/lib/Synchronizer.ts b/packages/lib/Synchronizer.ts index bc147d1822..1d14333390 100644 --- a/packages/lib/Synchronizer.ts +++ b/packages/lib/Synchronizer.ts @@ -25,7 +25,9 @@ import TaskQueue from './TaskQueue'; import ItemUploader from './services/synchronizer/ItemUploader'; import { FileApi, getSupportsDeltaWithItems, isLocalServer, PaginatedList, RemoteItem, enableEnhancedBasicDeltaAlgorithm } from './file-api'; import JoplinDatabase from './JoplinDatabase'; -import { checkIfCanSync, fetchSyncInfo, checkSyncTargetIsValid, getActiveMasterKey, localSyncInfo, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyHasBeenUsed, SyncInfo, syncInfoEquals, uploadSyncInfo } from './services/synchronizer/syncInfoUtils'; +import { parkedNoteLockKeyInUse } from './services/noteLock/NoteLockKeyMigration'; +import NoteLockSession from './services/noteLock/NoteLockSession'; +import { checkIfCanSync, fetchSyncInfo, checkSyncTargetIsValid, getActiveMasterKey, localSyncInfo, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyHasBeenUsed, SyncInfo, syncInfoEquals, uploadSyncInfo, checkNoteLockKeyConflict, checkNoteLockKeyMigrationId, checkNoteLockKeyUnchanged, checkRemoteNoteLockKeyUnchanged, noteLockKeyConflict } from './services/synchronizer/syncInfoUtils'; import { getMasterPassword, setupAndDisableEncryption, setupAndEnableEncryption } from './services/e2ee/utils'; import { generateKeyPair } from './services/e2ee/ppk/ppk'; import syncDebugLog from './services/synchronizer/syncDebugLog'; @@ -494,6 +496,11 @@ export default class Synchronizer { this.api().setTempDirName(Dirnames.Temp); try { + // Checked before anything is uploaded so a key without an id never reaches the target. The CLI cannot create or reset + // a note lock key or run a migration: it adopts the target's key, and a migration it would need (its locked notes + // meeting a different key, for example after switching the sync target) stops its sync. Revisit if it gains these. + checkNoteLockKeyMigrationId(localSyncInfo(), false); + let remoteInfo = await fetchSyncInfo(this.api()); logger.info('Sync target remote info:', remoteInfo.filterSyncInfo()); eventManager.emit(EventName.SessionEstablished); @@ -514,7 +521,10 @@ export default class Synchronizer { const appVersion = shim.appVersion(); if (appVersion !== 'unknown') checkIfCanSync(remoteInfo, appVersion); - let localInfo = await localSyncInfo(); + let localInfo = localSyncInfo(); + // Read in the same tick as the key, so a reset landing mid-sync is seen whole or not at all. + const noteLockPasswordReset = Setting.value('noteLock.passwordReset'); + const noteLockKeyIdToReset = Setting.value('noteLock.keyIdToReset'); logger.info('Sync target local info:', localInfo.filterSyncInfo()); localInfo = await this.setPpkIfNotExist(localInfo, remoteInfo); @@ -523,18 +533,50 @@ export default class Synchronizer { localInfo = setMasterKeyHasBeenUsed(localInfo, localInfo.activeMasterKeyId); } + checkNoteLockKeyMigrationId(remoteInfo, true); + const hasLockedNotes = await Note.hasLockedNotes(); + // A reset only overwrites the target key it replaced, so a target reset elsewhere since then wins. + const resetPropagates = noteLockPasswordReset && (!remoteInfo.noteLockKey || remoteInfo.noteLockKey.id === noteLockKeyIdToReset); + const staleReset = noteLockPasswordReset && !resetPropagates; + const notesUseParkedKey = await parkedNoteLockKeyInUse(); + const hadParkedKey = !!noteLockKeyConflict(); + // Read again after the await: a migration may have finished meanwhile. A running one is checked in the same tick + // as the conflict, so it cannot start in between. + checkNoteLockKeyConflict(localSyncInfo(), remoteInfo, hasLockedNotes, staleReset, notesUseParkedKey || NoteLockSession.instance().isMigrating()); + // The target no longer conflicts, so a banner left by an earlier run with nothing moved has nothing to retry. + if (hadParkedKey) this.dispatch({ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: null }); + // console.info('LOCAL', localInfo); // console.info('REMOTE', remoteInfo); + let reconciledNoteLockKeyId = localInfo.noteLockKey?.id; if (!syncInfoEquals(localInfo, remoteInfo)) { - let newInfo = mergeSyncInfos(localInfo, remoteInfo); + // The merge works from localInfo, which a key change since it was read would make stale. + checkNoteLockKeyUnchanged(localInfo); + let newInfo = mergeSyncInfos(localInfo, remoteInfo, { resetPropagates, hasLocalLockedNotes: hasLockedNotes }); if (newInfo.activeMasterKeyId) newInfo = setMasterKeyHasBeenUsed(newInfo, newInfo.activeMasterKeyId); const previousE2EE = localInfo.e2ee; logger.info('Sync target info differs between local and remote - merging infos: ', newInfo.toObject()); + // The checks above let the target key replace the local one, so it is adopted before the upload: a note locked + // from now on is encrypted with the key the target holds. + if (localInfo.noteLockKey && newInfo.noteLockKey?.id !== localInfo.noteLockKey.id) { + const adoptedInfo = localSyncInfo(); + adoptedInfo.noteLockKey = localInfo.noteLockKey = newInfo.noteLockKey; + adoptedInfo.syncMigrationId = localInfo.syncMigrationId = newInfo.syncMigrationId; + saveLocalSyncInfo(adoptedInfo); + } + await this.lockHandler().acquireLock(LockType.Exclusive, this.lockClientType(), this.clientId_, { clearExistingSyncLocksFromTheSameClient: true }); + // Must stay immediately before the upload. Sync locks are disabled (the lock above does nothing), so another device can + // upload its own info.json after this sync fetched the target's, for example a password reset, which the upload below + // would revert to the old key. + checkRemoteNoteLockKeyUnchanged(remoteInfo, await fetchSyncInfo(this.api())); await uploadSyncInfo(this.api(), newInfo); + // A reset or password change on this device during the upload would be overwritten by the save below. + checkNoteLockKeyUnchanged(localInfo); await saveLocalSyncInfo(newInfo); + reconciledNoteLockKeyId = newInfo.noteLockKey?.id; await this.lockHandler().releaseLock(LockType.Exclusive, this.lockClientType(), this.clientId_); // console.info('NEW', newInfo); @@ -562,6 +604,20 @@ export default class Synchronizer { // Set it to remote anyway so that timestamps are the same // Note: that's probably not needed anymore? // await uploadSyncInfo(this.api(), remoteInfo); + + // A reset that landed mid-sync keeps its flag for the next sync. + if (noteLockPasswordReset) checkNoteLockKeyUnchanged(localInfo); + } + + // Either the reset key is on the target now or a different lineage replaced it, so the flag is spent. A reset + // that landed after the local save replaces the key the target holds now, so it keeps its flag. + if (noteLockPasswordReset) { + if (localSyncInfo().noteLockKey?.id === reconciledNoteLockKeyId) { + Setting.setValue('noteLock.passwordReset', false); + Setting.setValue('noteLock.keyIdToReset', ''); + } else { + Setting.setValue('noteLock.keyIdToReset', reconciledNoteLockKeyId ?? ''); + } } } catch (error) { if (error.code === 403) { diff --git a/packages/lib/errors.ts b/packages/lib/errors.ts index 514d6468ac..d24c5f2b2d 100644 --- a/packages/lib/errors.ts +++ b/packages/lib/errors.ts @@ -6,4 +6,5 @@ export enum ErrorCode { UnsupportedMimeType = 'unsupportedMimeType', MustUpgradeApp = 'mustUpgradeApp', DownloadLimiter = 'downloadLimiter', + NoteLockKeyConflict = 'noteLockKeyConflict', } diff --git a/packages/lib/models/Note.ts b/packages/lib/models/Note.ts index 32a8bda33b..939a63f675 100644 --- a/packages/lib/models/Note.ts +++ b/packages/lib/models/Note.ts @@ -608,6 +608,16 @@ export default class Note extends BaseItem { return r && r.total ? r.total : 0; } + // Trashed and conflict notes count too: their content still depends on the note lock key. + public static async hasLockedNotes() { + return !!(await this.db().selectOne('SELECT 1 FROM notes WHERE is_locked = 1 LIMIT 1')); + } + + public static async lockedNoteIds() { + const rows = await this.db().selectAll<{ id: string }>('SELECT id FROM notes WHERE is_locked = 1'); + return rows.map(row => row.id); + } + public static unconflictedNotes() { return this.modelSelectAll('SELECT * FROM notes WHERE is_conflict = 0'); } diff --git a/packages/lib/models/settings/builtInMetadata.ts b/packages/lib/models/settings/builtInMetadata.ts index 46a3f192df..340666b13a 100644 --- a/packages/lib/models/settings/builtInMetadata.ts +++ b/packages/lib/models/settings/builtInMetadata.ts @@ -8,6 +8,7 @@ import { AppType, SettingItemSubType, SettingItemType, SettingStorage, SyncStart import { defaultListColumns } from '../../services/plugins/api/noteListType'; import type { PluginSettings } from '../../services/plugins/PluginService'; import type { PublicPrivateKeyPair } from '../../services/e2ee/ppk/ppk'; +import type { NoteLockKeyConflict } from '../../services/synchronizer/syncInfoUtils'; import { EmptyObject } from '@joplin/utils/types'; const ObjectUtils = require('../../ObjectUtils'); import { toTitleCase } from '../../string-utils'; @@ -2357,6 +2358,24 @@ const builtInMetadata = (Setting: typeof SettingType) => { storage: SettingStorage.File, }, + 'noteLock.passwordReset': { + value: false, + type: SettingItemType.Bool, + public: false, + }, + + 'noteLock.keyIdToReset': { + value: '', + type: SettingItemType.String, + public: false, + }, + + 'noteLock.conflictNoteLockKey': { + value: {} as Partial, + type: SettingItemType.Object, + public: false, + }, + 'featureFlag.autoUpdaterServiceEnabled': { value: false, type: SettingItemType.Bool, diff --git a/packages/lib/reducer.ts b/packages/lib/reducer.ts index 516467b007..0d21d72c7a 100644 --- a/packages/lib/reducer.ts +++ b/packages/lib/reducer.ts @@ -90,6 +90,14 @@ export interface StateLastDeletion { timestamp: number; } +// The background note lock key migration, kept after it ends until its banner is dismissed. +export interface NoteLockMigrationStatus { + running: boolean; + failed: number; + skipped: number; + awaitingDecryption: number; +} + export type HighlightedWord = ComplexTerm|string; export interface WindowState { @@ -191,6 +199,7 @@ export interface State extends WindowState { isInsertingNotes: boolean; hasEncryptedItems: boolean; noteLockSessionUnlocked: boolean; + noteLockMigrationStatus: NoteLockMigrationStatus|null; needApiAuth: boolean; profileConfig: ProfileConfig; noteListRendererIds: string[]; @@ -262,6 +271,7 @@ export const defaultState: State = { isInsertingNotes: false, hasEncryptedItems: false, noteLockSessionUnlocked: false, + noteLockMigrationStatus: null, needApiAuth: false, profileConfig: null, noteListRendererIds: getListRendererIds(), @@ -1579,6 +1589,10 @@ const reducer = produce((draft: Draft = defaultState, action: any) => { draft.noteLockSessionUnlocked = action.value; break; + case 'NOTE_LOCK_MIGRATION_STATUS_SET': + draft.noteLockMigrationStatus = action.value; + break; + case 'SET_ACTIVE_NOTE_IS_UNDECRYPTABLE': { // The editor reporting this may be in a background window, so the flag follows that diff --git a/packages/lib/services/commands/stateToWhenClauseContext.ts b/packages/lib/services/commands/stateToWhenClauseContext.ts index 75de868a6d..e101f2f28a 100644 --- a/packages/lib/services/commands/stateToWhenClauseContext.ts +++ b/packages/lib/services/commands/stateToWhenClauseContext.ts @@ -45,6 +45,7 @@ export interface WhenClauseContext { noteIsLocked: boolean; noteLockContentUnavailable: boolean; noteLockSessionUnlocked: boolean; + noteLockMigrationRunning: boolean; noteIsMarkdown: boolean; noteIsReadOnly: boolean; noteIsReadOnlyShare: boolean; @@ -129,6 +130,7 @@ export default function stateToWhenClauseContext(state: State, options: WhenClau noteIsLocked, noteLockContentUnavailable, noteLockSessionUnlocked: state.noteLockSessionUnlocked, + noteLockMigrationRunning: !!state.noteLockMigrationStatus?.running, noteIsReadOnly: noteLockContentUnavailable || noteIsReadOnlyShare || noteIsLockedInShare(selectedNote), noteIsReadOnlyShare, noteIsDeleted: selectedNote ? !!selectedNote.deleted_time : false, diff --git a/packages/lib/services/e2ee/EncryptionService.ts b/packages/lib/services/e2ee/EncryptionService.ts index f75fe46ab1..2d3dbd6a78 100644 --- a/packages/lib/services/e2ee/EncryptionService.ts +++ b/packages/lib/services/e2ee/EncryptionService.ts @@ -810,9 +810,9 @@ export default class EncryptionService { return `${isNoteLock ? 'JLD' : 'JED'}01${encryptionMetadata}`; } - public async decodeHeaderString(cipherText: string) { + public async decodeHeaderString(cipherText: string, isNoteLock = false) { const source = this.stringReader_(cipherText); - return this.decodeHeaderSource_(source); + return this.decodeHeaderSource_(source, isNoteLock); } // eslint-disable-next-line @typescript-eslint/no-explicit-any -- See encryptAbstract_ source/destination diff --git a/packages/lib/services/noteLock/NoteLockKey.ts b/packages/lib/services/noteLock/NoteLockKey.ts index 8fdb6b6cfc..d366f8dbbc 100644 --- a/packages/lib/services/noteLock/NoteLockKey.ts +++ b/packages/lib/services/noteLock/NoteLockKey.ts @@ -1,4 +1,5 @@ import uuid from '../../uuid'; +import Setting from '../../models/Setting'; import EncryptionService from '../e2ee/EncryptionService'; import { MasterKeyEntity } from '../e2ee/types'; import { localSyncInfo, saveLocalSyncInfo } from '../synchronizer/syncInfoUtils'; @@ -39,6 +40,8 @@ export default class NoteLockKey { const syncInfo = localSyncInfo(); syncInfo.noteLockKey = key; + // Only a key migration replaces the sync migration id, so a reset keeps the lineage it belongs to. + if (!syncInfo.syncMigrationId) syncInfo.syncMigrationId = uuid.create(); saveLocalSyncInfo(syncInfo); return key; @@ -56,7 +59,13 @@ export default class NoteLockKey { // Rotate through NoteLockSession.reset() rather than calling this directly, so the session locks // and drops the old key as part of the rotation. public async reset(password: string) { - return this.save(await this.encryptionService_.generateMasterKey(password)); + const generated = await this.encryptionService_.generateMasterKey(password); + // Read once the key is ready, so a sync that finished meanwhile counts; a second reset before that sync keeps the first backup. + if (!Setting.value('noteLock.keyIdToReset')) Setting.setValue('noteLock.keyIdToReset', this.load()?.id ?? ''); + const key = this.save(generated); + // Lets the next sync push the new key to the target instead of adopting the old one back. + Setting.setValue('noteLock.passwordReset', true); + return key; } public async changePassword(currentPassword: string, newPassword: string) { diff --git a/packages/lib/services/noteLock/NoteLockKeyMigration.test.ts b/packages/lib/services/noteLock/NoteLockKeyMigration.test.ts new file mode 100644 index 0000000000..a54b08d532 --- /dev/null +++ b/packages/lib/services/noteLock/NoteLockKeyMigration.test.ts @@ -0,0 +1,251 @@ +import Setting from '../../models/Setting'; +import Note from '../../models/Note'; +import uuid from '../../uuid'; +import { afterAllCleanUp, db, encryptionService, setupDatabaseAndSynchronizer, switchClient } from '../../testing/test-utils'; +import EncryptionService from '../e2ee/EncryptionService'; +import { MasterKeyEntity } from '../e2ee/types'; +import { localSyncInfo, noteLockKeyConflict } from '../synchronizer/syncInfoUtils'; +import NoteLockKey, { DecryptedNoteLockKey } from './NoteLockKey'; +import NoteLockNote from './NoteLockNote'; +import NoteLockService from './NoteLockService'; +import NoteLockSession from './NoteLockSession'; +import { confirmAndSkipNoteLockKeyMigration, finishNoteLockKeyMigration, migrateLockedNotes, notMigratedMessage, startNoteLockKeyMigration } from './NoteLockKeyMigration'; +import eventManager, { EventName, NoteLockSessionChangeEvent } from '../../eventManager'; +import shim from '../../shim'; + +const localPassword = '111111'; +const targetPassword = '222222'; +const targetSyncMigrationId = '0123456789abcdef0123456789abcdef'; + +const generateKey = async (password: string): Promise => ({ ...await encryptionService().generateMasterKey(password), id: uuid.create() }); + +const decryptKey = async (key: MasterKeyEntity, password: string): Promise => ({ id: key.id, plainText: await encryptionService().decryptMasterKeyContent(key, password) }); + +// Encrypts outside the session so the fixture can use a key the session would refuse. +const noteLockedWith = async (key: DecryptedNoteLockKey, body: string) => { + const cipherText = await encryptionService().encryptString(body, { masterKeyId: key.id, decryptedMasterKey: key.plainText, isNoteLock: true }); + return Note.save({ title: body, body: cipherText, is_locked: 1 }); +}; + +const bodyDecryptedWith = async (noteId: string, key: DecryptedNoteLockKey) => (await NoteLockNote.decryptBody(await Note.load(noteId), key)).body; + +const storedBodies = async () => (await Note.all({ fields: ['id', 'body'] })).map(n => n.body).sort(); + +describe('NoteLockKeyMigration', () => { + + let targetKey: MasterKeyEntity; + let decryptedTargetKey: DecryptedNoteLockKey; + let decryptedLocalKey: DecryptedNoteLockKey; + + beforeEach(async () => { + await setupDatabaseAndSynchronizer(1); + await switchClient(1); + NoteLockService.destroyInstance(); + NoteLockSession.destroyInstance(); + NoteLockKey.destroyInstance(); + EncryptionService.instance_ = encryptionService(); + Setting.setValue('featureFlag.noteLock', true); + + await NoteLockKey.instance().create(localPassword); + await NoteLockSession.instance().unlock(localPassword); + decryptedLocalKey = await NoteLockKey.instance().decrypt(localPassword); + targetKey = await generateKey(targetPassword); + decryptedTargetKey = await decryptKey(targetKey, targetPassword); + Setting.setValue('noteLock.conflictNoteLockKey', { noteLockKey: targetKey, syncMigrationId: targetSyncMigrationId }); + }); + + afterAll(async () => { + await afterAllCleanUp(); + }); + + it('should re-encrypt the notes locked with the local key and report the rest', async () => { + const note1 = await Note.save({ title: 'one', body: 'one', is_locked: 1 }, { useNoteLock: true }); + const note2 = await Note.save({ title: 'two', body: 'two', is_locked: 1 }, { useNoteLock: true }); + const staleKey = await generateKey('333333'); + const staleNote = await noteLockedWith(await decryptKey(staleKey, '333333'), 'stale'); + const corruptCipherText = (await Note.load(note1.id)).body; + const corruptNote = await Note.save({ title: 'corrupt', body: `${corruptCipherText.slice(0, -8)}xxxxxxxx`, is_locked: 1 }); + await Note.save({ title: 'plain', body: 'plain' }); + const localKeyBefore = NoteLockKey.instance().load(); + + const result = await migrateLockedNotes(localPassword, targetPassword); + + expect(result).toEqual({ migrated: 2, skipped: 1, failed: 1, awaitingDecryption: 0 }); + expect(await bodyDecryptedWith(note1.id, decryptedTargetKey)).toBe('one'); + expect(await bodyDecryptedWith(note2.id, decryptedTargetKey)).toBe('two'); + expect((await Note.load(staleNote.id)).body).toBe(staleNote.body); + expect((await Note.load(corruptNote.id)).body).toBe(corruptNote.body); + expect((await Note.load(note1.id)).is_locked).toBe(1); + // Nothing is adopted until the user finishes, so an interrupted migration loses no key. + expect(NoteLockKey.instance().load()).toEqual(localKeyBefore); + expect(noteLockKeyConflict()).toEqual({ noteLockKey: targetKey, syncMigrationId: targetSyncMigrationId }); + }); + + it('should leave notes already under the target key alone on retry', async () => { + await Note.save({ title: 'one', body: 'one', is_locked: 1 }, { useNoteLock: true }); + const staleKey = await generateKey('333333'); + await noteLockedWith(await decryptKey(staleKey, '333333'), 'stale'); + await migrateLockedNotes(localPassword, targetPassword); + const bodiesAfterFirstRun = await storedBodies(); + + expect(await migrateLockedNotes(localPassword, targetPassword)).toEqual({ migrated: 0, skipped: 1, failed: 0, awaitingDecryption: 0 }); + expect(await storedBodies()).toEqual(bodiesAfterFirstRun); + }); + + test.each([ + ['local', 'wrong', targetPassword], + ['target', localPassword, 'wrong'], + ])('should not touch any note when the %s password is wrong', async (_which, local, target) => { + await Note.save({ title: 'one', body: 'one', is_locked: 1 }, { useNoteLock: true }); + const bodiesBefore = await storedBodies(); + + await expect(migrateLockedNotes(local, target)).rejects.toThrow(); + expect(await storedBodies()).toEqual(bodiesBefore); + }); + + it('should fail closed when note lock is disabled', async () => { + await Note.save({ title: 'one', body: 'one', is_locked: 1 }, { useNoteLock: true }); + const bodiesBefore = await storedBodies(); + Setting.setValue('featureFlag.noteLock', false); + + await expect(migrateLockedNotes(localPassword, targetPassword)).rejects.toThrow('not enabled'); + expect(await storedBodies()).toEqual(bodiesBefore); + }); + + it('should adopt the target key and lineage, clear the flags and lock the session on finish', async () => { + Setting.setValue('noteLock.passwordReset', true); + expect(NoteLockSession.instance().isUnlocked()).toBe(true); + const events: NoteLockSessionChangeEvent[] = []; + const listener = (event: NoteLockSessionChangeEvent) => events.push(event); + eventManager.on(EventName.NoteLockSessionChange, listener); + + try { + finishNoteLockKeyMigration(); + + // The lock is announced right away rather than on the next session check, so the UI follows. + expect(events).toEqual([{ unlocked: false }]); + expect(NoteLockKey.instance().load()).toEqual(targetKey); + expect(localSyncInfo().syncMigrationId).toBe(targetSyncMigrationId); + expect(noteLockKeyConflict()).toBeNull(); + expect(Setting.value('noteLock.passwordReset')).toBe(false); + await NoteLockSession.instance().unlock(targetPassword); + expect(NoteLockSession.instance().isUnlocked()).toBe(true); + } finally { + eventManager.off(EventName.NoteLockSessionChange, listener); + } + }); + + test.each([ + ['Yes', 0, true], + ['No', 1, false], + ])('should skip the migration only when Yes is chosen in the warning (answer: %s)', async (_answer, response, skipped) => { + const showMessageBox = jest.spyOn(shim, 'showMessageBox').mockResolvedValue(response); + const dispatch = jest.fn(); + + expect(await confirmAndSkipNoteLockKeyMigration(dispatch)).toBe(skipped); + + expect(showMessageBox.mock.calls[0][0]).toContain('JEX backup'); + expect(showMessageBox.mock.calls[0][1].buttons).toEqual(['Yes', 'No']); + expect(NoteLockKey.instance().load().id).toBe(skipped ? targetKey.id : decryptedLocalKey.id); + expect(dispatch).toHaveBeenCalledTimes(skipped ? 1 : 0); + showMessageBox.mockRestore(); + }); + + it('should report a background run that starts once the passwords are accepted', async () => { + const note = await Note.save({ title: 'one', body: 'one', is_locked: 1 }, { useNoteLock: true }); + const dispatch = jest.fn(); + const onStarted = jest.fn(); + + await expect(startNoteLockKeyMigration(localPassword, 'wrong', dispatch, onStarted)).rejects.toThrow(); + expect(onStarted).not.toHaveBeenCalled(); + expect(dispatch).not.toHaveBeenCalled(); + + const corruptNote = await Note.save({ title: 'corrupt', body: `${(await Note.load(note.id)).body.slice(0, -8)}xxxxxxxx`, is_locked: 1 }); + await startNoteLockKeyMigration(localPassword, targetPassword, dispatch, onStarted); + expect(onStarted).toHaveBeenCalledTimes(1); + expect(dispatch.mock.calls).toEqual([ + [{ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: { running: true, failed: 0, skipped: 0, awaitingDecryption: 0 } }], + [{ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: { running: false, failed: 1, skipped: 0, awaitingDecryption: 0 } }], + ]); + expect(noteLockKeyConflict()).not.toBeNull(); + + await Note.delete(corruptNote.id); + dispatch.mockClear(); + await startNoteLockKeyMigration(localPassword, targetPassword, dispatch, onStarted); + expect(dispatch).toHaveBeenLastCalledWith({ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: { running: false, failed: 0, skipped: 0, awaitingDecryption: 0 } }); + expect(NoteLockKey.instance().load()).toEqual(targetKey); + expect(await bodyDecryptedWith(note.id, decryptedTargetKey)).toBe('one'); + }); + + it('should cancel a run with a popup when the feature flag is turned off during it', async () => { + await Note.save({ title: 'one', body: 'one', is_locked: 1 }, { useNoteLock: true }); + const dispatch = jest.fn(); + const showErrorDialog = jest.spyOn(shim, 'showErrorDialog').mockResolvedValue(); + const save = Note.save.bind(Note); + const saveSpy = jest.spyOn(Note, 'save').mockImplementation((o, options) => { + Setting.setValue('featureFlag.noteLock', false); + return save(o, options); + }); + + try { + await startNoteLockKeyMigration(localPassword, targetPassword, dispatch, jest.fn()); + + expect(dispatch).toHaveBeenLastCalledWith({ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: null }); + expect(showErrorDialog).toHaveBeenCalledWith('The note lock key migration was cancelled because the note lock feature was turned off.'); + expect(noteLockKeyConflict()).not.toBeNull(); + } finally { + saveSpy.mockRestore(); + showErrorDialog.mockRestore(); + } + }); + + it('should keep locked notes closed and refuse a password reset only while a run goes', async () => { + await Note.save({ title: 'one', body: 'one', is_locked: 1 }, { useNoteLock: true }); + let unlockDuringRun: Promise = null; + let resetDuringRun: Promise = null; + await migrateLockedNotes(localPassword, targetPassword, () => { + expect(NoteLockSession.instance().isUnlocked()).toBe(false); + unlockDuringRun = NoteLockSession.instance().unlock(localPassword).then((): Error => null, (error: Error) => error); + resetDuringRun = NoteLockSession.instance().reset('333333').then((): Error => null, (error: Error) => error); + }); + + expect((await unlockDuringRun)?.message).toContain('note lock key migration is in progress'); + expect((await resetDuringRun)?.message).toContain('cannot be reset while a note lock key migration is in progress'); + await NoteLockSession.instance().unlock(localPassword); + expect(NoteLockSession.instance().isUnlocked()).toBe(true); + }); + + it('should refuse to migrate or finish without a parked key', async () => { + Setting.setValue('noteLock.conflictNoteLockKey', {}); + + await expect(migrateLockedNotes(localPassword, targetPassword)).rejects.toThrow('No note lock key conflict'); + expect(() => finishNoteLockKeyMigration()).toThrow('No note lock key conflict'); + expect(NoteLockKey.instance().load().id).toBe(decryptedLocalKey.id); + }); + it('should keep the date a note was last changed by the user', async () => { + const note = await Note.save({ title: 'one', body: 'one', is_locked: 1 }, { useNoteLock: true }); + await db().exec('UPDATE notes SET user_updated_time = 1000, updated_time = 1000 WHERE id = ?', [note.id]); + + expect(await migrateLockedNotes(localPassword, targetPassword)).toEqual({ migrated: 1, skipped: 0, failed: 0, awaitingDecryption: 0 }); + + const migrated = await Note.load(note.id); + expect(migrated.user_updated_time).toBe(1000); + // Still moves, so the re-encrypted note is synced. + expect(migrated.updated_time).toBeGreaterThan(1000); + }); + + it('should leave a note still waiting for sync decryption for a retry and not finish the migration', async () => { + const waiting = await Note.save({ title: 'waiting', body: 'waiting', is_locked: 1 }, { useNoteLock: true }); + // What sync leaves until the decryption worker has run: is_locked stays readable, the body is not there yet. + await db().exec('UPDATE notes SET encryption_applied = 1, body = \'\' WHERE id = ?', [waiting.id]); + const dispatch = jest.fn(); + + await startNoteLockKeyMigration(localPassword, targetPassword, dispatch, jest.fn()); + + const status = { running: false, failed: 0, skipped: 0, awaitingDecryption: 1 }; + expect(dispatch).toHaveBeenLastCalledWith({ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: status }); + expect(noteLockKeyConflict()).not.toBeNull(); + expect(notMigratedMessage(status)).toBe('1 locked note is still waiting to be decrypted and was not migrated yet.'); + }); + +}); diff --git a/packages/lib/services/noteLock/NoteLockKeyMigration.ts b/packages/lib/services/noteLock/NoteLockKeyMigration.ts new file mode 100644 index 0000000000..2e918c09f4 --- /dev/null +++ b/packages/lib/services/noteLock/NoteLockKeyMigration.ts @@ -0,0 +1,137 @@ +import Logger from '@joplin/utils/Logger'; +import { Dispatch } from 'redux'; +import { _, _n } from '../../locale'; +import type { NoteLockMigrationStatus } from '../../reducer'; +import Note from '../../models/Note'; +import time from '../../time'; +import shim, { MessageBoxType } from '../../shim'; +import EncryptionService from '../e2ee/EncryptionService'; +import { adoptNoteLockKeyConflict, noteLockKeyConflict } from '../synchronizer/syncInfoUtils'; +import isNoteLockEnabled from './isNoteLockEnabled'; +import NoteLockKey, { DecryptedNoteLockKey } from './NoteLockKey'; +import NoteLockSession from './NoteLockSession'; + +const logger = Logger.create('NoteLockKeyMigration'); + +export interface NoteLockKeyMigrationResult { + migrated: number; + // Locked with a key that is neither the local nor the sync target one, so nothing here can read them. + skipped: number; + failed: number; + // Still encrypted by sync, so their note lock body cannot be read yet. A retry once they are decrypted picks them up. + awaitingDecryption: number; +} + +// Re-encrypts the notes locked with the local key to the parked target key; notes already under it are left +// alone, so a retry only touches the rest. The local key stays until finish, so an interrupted run loses nothing. +// onStarted runs once both passwords are accepted. +export const migrateLockedNotes = async (localPassword: string, targetPassword: string, onStarted: ()=> void = null) => { + if (!isNoteLockEnabled()) throw new Error('Note lock is not enabled'); + const conflict = noteLockKeyConflict(); + if (!conflict) throw new Error('No note lock key conflict to migrate'); + const encryptionService = EncryptionService.instance(); + const localKey = await NoteLockKey.instance().decrypt(localPassword); + const targetKey: DecryptedNoteLockKey = { + id: conflict.noteLockKey.id, + plainText: await encryptionService.decryptMasterKeyContent(conflict.noteLockKey, targetPassword), + }; + const result: NoteLockKeyMigrationResult = { migrated: 0, skipped: 0, failed: 0, awaitingDecryption: 0 }; + // A sync may have replaced the parked key while the passwords were checked. From here the sync leaves it alone. + if (noteLockKeyConflict()?.noteLockKey.id !== targetKey.id) throw new Error(_('The note lock key on the sync target changed. Please try again.')); + NoteLockSession.instance().setMigrating(true); + try { + onStarted?.(); + for (const noteId of await Note.lockedNoteIds()) { + try { + const { body, encryption_applied } = await Note.load(noteId, { fields: ['id', 'body', 'encryption_applied'] }); + if (encryption_applied) { + result.awaitingDecryption++; + continue; + } + const header = await encryptionService.decodeHeaderString(body, true); + if (header.masterKeyId === targetKey.id) continue; + if (header.masterKeyId !== localKey.id) { + result.skipped++; + continue; + } + const note = await Note.load(noteId, { useNoteLock: true, noteLockKey: localKey }); + // The note keeps its own "updated" date; updated_time still moves so the re-encrypted note syncs. + await Note.save({ ...note, updated_time: time.unixMs() }, { useNoteLock: true, noteLockKey: targetKey, autoTimestamp: false }); + result.migrated++; + } catch (error) { + logger.warn(`Could not migrate note ${noteId}:`, error); + result.failed++; + } + // With the feature turned off the note was not re-encrypted, whether Note.save refused it or saved it ungated. + if (!isNoteLockEnabled()) throw new Error(_('The note lock key migration was cancelled because the note lock feature was turned off.')); + } + } finally { + NoteLockSession.instance().setMigrating(false); + } + return result; +}; + +// Locked notes stay closed while a run goes, so after a run with no failures no note is left under the local key, apart +// from an editor save that was already queued when the run started. +export const finishNoteLockKeyMigration = () => { + adoptNoteLockKeyConflict(); + NoteLockSession.instance().lock(); +}; + +export const confirmAndSkipNoteLockKeyMigration = async (dispatch: Dispatch) => { + if (await shim.showMessageBox(_('Warning: Notes still locked with the current note lock key will become permanently unreadable. Before continuing, it is recommended to first export all notes on this device as a JEX backup, which can be imported again later if the correct password is used. Are you sure you want to continue without migrating the remaining notes?'), { buttons: [_('Yes'), _('No')], type: MessageBoxType.Confirm, cancelId: 1 }) !== 0) return false; + finishNoteLockKeyMigration(); + dispatch({ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: null }); + return true; +}; + +// Runs in the background once both passwords are accepted, so the app stays usable, and reports its progress for the +// banners. A run with failures keeps the target key parked for a retry or a skip. +export const startNoteLockKeyMigration = async (localPassword: string, targetPassword: string, dispatch: Dispatch, onStarted: ()=> void) => { + let started = false; + let result: NoteLockKeyMigrationResult; + try { + result = await migrateLockedNotes(localPassword, targetPassword, () => { + started = true; + onStarted(); + dispatch({ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: { running: true, failed: 0, skipped: 0, awaitingDecryption: 0 } }); + }); + } catch (error) { + // Before the start the dialog is still open to show the error, after it the dialog is gone. + if (!started) throw error; + dispatch({ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: null }); + await shim.showErrorDialog(error.message); + return; + } + if (!result.failed && !result.awaitingDecryption) finishNoteLockKeyMigration(); + dispatch({ type: 'NOTE_LOCK_MIGRATION_STATUS_SET', value: { running: false, failed: result.failed, skipped: result.skipped, awaitingDecryption: result.awaitingDecryption } }); +}; + +// The notes a run left under the local key, so the banner offers a retry or a skip. +export const notMigratedMessage = (status: NoteLockMigrationStatus) => { + const parts = []; + if (status.failed) parts.push(_n('%d locked note could not be migrated.', '%d locked notes could not be migrated.', status.failed, status.failed)); + if (status.awaitingDecryption) parts.push(_n('%d locked note is still waiting to be decrypted and was not migrated yet.', '%d locked notes are still waiting to be decrypted and were not migrated yet.', status.awaitingDecryption, status.awaitingDecryption)); + return withSkippedCount(parts.join(' '), status); +}; + +// Notes already moved to the parked key need it, so a sync must not drop or replace it. +export const parkedNoteLockKeyInUse = async () => { + const conflict = noteLockKeyConflict(); + if (!conflict) return false; + for (const noteId of await Note.lockedNoteIds()) { + const { body, encryption_applied } = await Note.load(noteId, { fields: ['id', 'body', 'encryption_applied'] }); + if (encryption_applied) continue; + try { + if ((await EncryptionService.instance().decodeHeaderString(body, true)).masterKeyId === conflict.noteLockKey.id) return true; + } catch { + // A body that is not note lock ciphertext cannot use the parked key. + } + } + return false; +}; + +export const withSkippedCount = (message: string, status: NoteLockMigrationStatus) => { + if (!status.skipped) return message; + return `${message} ${_n('%d locked note was skipped because it is locked with another key.', '%d locked notes were skipped because they are locked with another key.', status.skipped, status.skipped)}`; +}; diff --git a/packages/lib/services/noteLock/NoteLockSession.test.ts b/packages/lib/services/noteLock/NoteLockSession.test.ts index 2da4b121da..123c3a346e 100644 --- a/packages/lib/services/noteLock/NoteLockSession.test.ts +++ b/packages/lib/services/noteLock/NoteLockSession.test.ts @@ -1,4 +1,5 @@ import { afterAllCleanUp, encryptionService, setupDatabaseAndSynchronizer, switchClient, synchronizerStart } from '../../testing/test-utils'; +import Setting from '../../models/Setting'; import EncryptionService from '../e2ee/EncryptionService'; import { localSyncInfo, saveLocalSyncInfo } from '../synchronizer/syncInfoUtils'; import NoteLockKey, { DecryptedNoteLockKey } from './NoteLockKey'; @@ -139,6 +140,23 @@ describe('NoteLockSession', () => { expect(session.isUnlocked()).toBe(false); }); + it('should keep the sync migration id and the first replaced key id through password resets', async () => { + const session = NoteLockSession.instance(); + const firstKey = await NoteLockKey.instance().create('123456'); + const syncMigrationId = localSyncInfo().syncMigrationId; + expect(syncMigrationId).toHaveLength(32); + expect(Setting.value('noteLock.passwordReset')).toBe(false); + + await session.reset('654321'); + expect(localSyncInfo().syncMigrationId).toBe(syncMigrationId); + expect(Setting.value('noteLock.passwordReset')).toBe(true); + expect(Setting.value('noteLock.keyIdToReset')).toBe(firstKey.id); + + // A second reset before a sync still has to replace the key the sync target holds. + await session.reset('999999'); + expect(Setting.value('noteLock.keyIdToReset')).toBe(firstKey.id); + }); + it('should not install a key that was reset while unlocking', async () => { const session = NoteLockSession.instance(); const originalKey = await NoteLockKey.instance().create('123456'); diff --git a/packages/lib/services/noteLock/NoteLockSession.ts b/packages/lib/services/noteLock/NoteLockSession.ts index ff12bccf6e..67ff554c7b 100644 --- a/packages/lib/services/noteLock/NoteLockSession.ts +++ b/packages/lib/services/noteLock/NoteLockSession.ts @@ -1,5 +1,7 @@ import NoteLockKey, { DecryptedNoteLockKey } from './NoteLockKey'; import eventManager, { EventName } from '../../eventManager'; +import { noteLockKeyConflict } from '../synchronizer/syncInfoUtils'; +import { _ } from '../../locale'; export default class NoteLockSession { @@ -8,6 +10,7 @@ export default class NoteLockSession { private key_: DecryptedNoteLockKey = null; private lockGeneration_ = 0; private rotating_ = false; + private migrating_ = false; private constructor(private noteLockKey_: NoteLockKey = NoteLockKey.instance()) {} @@ -29,6 +32,7 @@ export default class NoteLockSession { // check covers it. public async unlock(password: string) { if (this.rotating_) throw new Error('Cannot unlock: a note lock key reset is in progress'); + if (this.migrating_) throw new Error(_('This note cannot be read while a note lock key migration is in progress.')); const generation = this.lockGeneration_; const decrypted = await this.noteLockKey_.decrypt(password); if (this.lockGeneration_ !== generation) throw new Error('Cannot unlock: the session was locked while unlocking'); @@ -47,6 +51,8 @@ export default class NoteLockSession { // Blocks unlock during rotation so the still-persisted old key can't be unlocked before the new one is saved. public async reset(password: string) { if (this.rotating_) throw new Error('A note lock key reset is already in progress'); + // The migration would adopt the target key over the new one when it ends. + if (this.migrating_) throw new Error(_('The note lock password cannot be reset while a note lock key migration is in progress.')); this.rotating_ = true; this.lock(); try { @@ -57,6 +63,16 @@ export default class NoteLockSession { } } + // Locked notes stay closed in every window while a migration run re-encrypts them. + public setMigrating(migrating: boolean) { + this.migrating_ = migrating; + if (migrating) this.lock(); + } + + public isMigrating() { + return this.migrating_; + } + private lockIfKeyChanged_() { if (this.key_ && this.key_.id !== this.noteLockKey_.load()?.id) this.lock(); } @@ -76,7 +92,8 @@ export default class NoteLockSession { } // A lock clears key_ but not the persisted key, so this only fails on a real rotation, or one in progress. + // The key parked for migration is allowed too: migration re-encrypts to it before it is adopted. public assertCanEncryptWith(keyId: string) { - if (this.rotating_ || this.noteLockKey_.load()?.id !== keyId) throw new Error('Note lock key changed during operation'); + if (this.rotating_ || (this.noteLockKey_.load()?.id !== keyId && noteLockKeyConflict()?.noteLockKey.id !== keyId)) throw new Error('Note lock key changed during operation'); } } diff --git a/packages/lib/services/noteLock/setNoteLockState.test.ts b/packages/lib/services/noteLock/setNoteLockState.test.ts index e622491849..4a96b68911 100644 --- a/packages/lib/services/noteLock/setNoteLockState.test.ts +++ b/packages/lib/services/noteLock/setNoteLockState.test.ts @@ -84,6 +84,16 @@ describe('setNoteLockState', () => { expect((await Note.load(note.id)).is_locked).toBe(0); }); + it('should refuse to change the lock state while a note lock key migration is required', async () => { + await setUpUnlockedSession(); + const note = await Note.save({ title: 'note', body: 'secret' }); + const lockedNote = await Note.save({ title: 'locked note', body: 'secret', is_locked: 1 }, { useNoteLock: true }); + Setting.setValue('noteLock.conflictNoteLockKey', { noteLockKey: NoteLockKey.instance().load(), syncMigrationId: 'remote' }); + + await expect(enableNoteLock(note.id)).rejects.toThrow('note lock key migration is required'); + await expect(disableNoteLock(lockedNote.id)).rejects.toThrow('note lock key migration is required'); + }); + it('should throw when note lock is not enabled', async () => { await setUpUnlockedSession(); const note = await Note.save({ title: 'note', body: 'secret' }); diff --git a/packages/lib/services/noteLock/setNoteLockState.ts b/packages/lib/services/noteLock/setNoteLockState.ts index 6c9d1e063e..805a2197f3 100644 --- a/packages/lib/services/noteLock/setNoteLockState.ts +++ b/packages/lib/services/noteLock/setNoteLockState.ts @@ -8,6 +8,10 @@ import { NoteEntity } from '../database/types'; import eventManager, { EventName } from '../../eventManager'; import isNoteLockEnabled from './isNoteLockEnabled'; import NoteLockSession from './NoteLockSession'; +import { noteLockKeyConflict } from '../synchronizer/syncInfoUtils'; +import JoplinError from '../../JoplinError'; +import { ErrorCode } from '../../errors'; +import { _ } from '../../locale'; // The UI hides the enable/disable actions for these cases, but the commands can also be // invoked directly (keyboard, plugins), so the transitions fail closed here too. @@ -18,6 +22,8 @@ const checkCanChangeLockState = (note: NoteEntity, noteId: string) => { if (note.is_conflict) throw new Error('Cannot change the note lock of a conflict note'); if (itemIsReadOnlySync(ModelType.Note, ItemChange.SOURCE_UNSPECIFIED, note as ItemSlice, Setting.value('sync.userId'), BaseItem.syncShareCache)) throw new Error('Cannot change the note lock of a read-only note'); if (!NoteLockSession.instance().isUnlocked()) throw new Error('Cannot change the note lock while the session is locked'); + // The migration re-encrypts the locked notes it finds, so a note locked or unlocked meanwhile could be missed or fail it. + if (noteLockKeyConflict()) throw new JoplinError(_('Notes cannot be locked or unlocked while a note lock key migration is required.'), ErrorCode.NoteLockKeyConflict); }; const validationFields = ['id', 'is_locked', 'deleted_time', 'is_conflict', 'share_id']; diff --git a/packages/lib/services/synchronizer/Synchronizer.noteLock.test.ts b/packages/lib/services/synchronizer/Synchronizer.noteLock.test.ts new file mode 100644 index 0000000000..b6cae08ac9 --- /dev/null +++ b/packages/lib/services/synchronizer/Synchronizer.noteLock.test.ts @@ -0,0 +1,493 @@ +import Setting from '../../models/Setting'; +import Note from '../../models/Note'; +import { afterAllCleanUp, encryptionService, fileApi, loadEncryptionMasterKey, setupDatabaseAndSynchronizer, switchClient, synchronizer, synchronizerStart } from '../../testing/test-utils'; +import EncryptionService from '../e2ee/EncryptionService'; +import NoteLockKey from '../noteLock/NoteLockKey'; +import NoteLockSession from '../noteLock/NoteLockSession'; +import NoteLockService from '../noteLock/NoteLockService'; +import { fetchSyncInfo, localSyncInfo, noteLockKeyConflict, onRevisionServiceSettingsChanged, saveLocalSyncInfo } from './syncInfoUtils'; +import { ErrorCode } from '../../errors'; +import NoteLockNote from '../noteLock/NoteLockNote'; +import { finishNoteLockKeyMigration, migrateLockedNotes } from '../noteLock/NoteLockKeyMigration'; +import { MasterKeyEntity } from '../e2ee/types'; +import { setupAndEnableEncryption } from '../e2ee/utils'; + +// Duplicates the singleton reset from NoteLockSession.test.ts: the note lock singletons cache the +// encryption service, so each client switch has to rebuild them. +const switchToClient = async (id: number) => { + await switchClient(id); + NoteLockService.destroyInstance(); + NoteLockSession.destroyInstance(); + NoteLockKey.destroyInstance(); + EncryptionService.instance_ = encryptionService(); +}; + +const remoteNoteLockKeyId = async () => (await fetchSyncInfo(fileApi())).noteLockKey?.id; + +const decryptedRemoteKey = async (key: MasterKeyEntity) => ({ id: key.id, plainText: await encryptionService().decryptMasterKeyContent(key, '111111') }); + +// Lands the action inside the running sync, after it took its local snapshot and before the conflict check. +const duringTheLockedNotesCheck = (action: ()=> Promise) => { + const realHasLockedNotes = Note.hasLockedNotes.bind(Note); + const spy = jest.spyOn(Note, 'hasLockedNotes').mockImplementation(async () => { + spy.mockRestore(); + await action(); + return realHasLockedNotes(); + }); +}; + +describe('Synchronizer.noteLock', () => { + + beforeEach(async () => { + await setupDatabaseAndSynchronizer(1); + await setupDatabaseAndSynchronizer(2); + await switchToClient(1); + }); + + afterAll(async () => { + await afterAllCleanUp(); + }); + + it('should propagate a password reset to the sync target and then to other devices', async () => { + const firstKey = await NoteLockKey.instance().create('111111'); + const syncMigrationId = localSyncInfo().syncMigrationId; + await Note.save({ title: 'locked', is_locked: 1 }); + await synchronizerStart(); + + await switchToClient(2); + await synchronizerStart(); + expect(NoteLockKey.instance().load().id).toBe(firstKey.id); + const resetKey = await NoteLockSession.instance().reset('222222'); + await synchronizerStart(); + expect(await remoteNoteLockKeyId()).toBe(resetKey.id); + expect(Setting.value('noteLock.passwordReset')).toBe(false); + + // Locked notes do not stop a same-lineage adoption: a reset is meant to reach every device. + await switchToClient(1); + await synchronizerStart(); + expect(NoteLockKey.instance().load()).toEqual(resetKey); + expect(localSyncInfo().syncMigrationId).toBe(syncMigrationId); + }); + + it('should propagate the newest of repeated offline resets to the key they replaced', async () => { + const firstKey = await NoteLockKey.instance().create('111111'); + await Note.save({ title: 'locked', is_locked: 1 }); + await synchronizerStart(); + + await switchToClient(2); + await synchronizerStart(); + await NoteLockSession.instance().reset('222222'); + const lastResetKey = await NoteLockSession.instance().reset('333333'); + expect(Setting.value('noteLock.keyIdToReset')).toBe(firstKey.id); + await synchronizerStart(); + expect(await remoteNoteLockKeyId()).toBe(lastResetKey.id); + expect(Setting.value('noteLock.passwordReset')).toBe(false); + expect(Setting.value('noteLock.keyIdToReset')).toBe(''); + + await switchToClient(1); + await synchronizerStart(); + expect(NoteLockKey.instance().load()).toEqual(lastResetKey); + }); + + it('should park the sync target key instead of propagating a reset when another device reset first', async () => { + Setting.setValue('featureFlag.noteLock', true); + await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + Setting.setValue('featureFlag.noteLock', true); + await synchronizerStart(); + await NoteLockSession.instance().reset('222222'); + await NoteLockSession.instance().unlock('222222'); + const note = await Note.save({ title: 'secret', body: 'secret body', is_locked: 1 }, { useNoteLock: true }); + + // The other device resets and syncs first, so the key this reset replaced is gone from the target. + await switchToClient(1); + const winningKey = await NoteLockSession.instance().reset('333333'); + await synchronizerStart(); + expect(await remoteNoteLockKeyId()).toBe(winningKey.id); + + await switchToClient(2); + for (let i = 0; i < 2; i++) { + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toMatchObject({ code: ErrorCode.NoteLockKeyConflict }); + expect(await remoteNoteLockKeyId()).toBe(winningKey.id); + expect(Setting.value('noteLock.conflictNoteLockKey').noteLockKey).toEqual(winningKey); + } + + expect(await migrateLockedNotes('222222', '333333')).toEqual({ migrated: 1, skipped: 0, failed: 0, awaitingDecryption: 0 }); + finishNoteLockKeyMigration(); + expect(Setting.value('noteLock.passwordReset')).toBe(false); + expect(Setting.value('noteLock.keyIdToReset')).toBe(''); + await synchronizerStart(null, { throwOnError: true }); + expect(NoteLockKey.instance().load()).toEqual(winningKey); + expect(await remoteNoteLockKeyId()).toBe(winningKey.id); + const key = await NoteLockKey.instance().decrypt('333333'); + expect((await NoteLockNote.decryptBody(await Note.load(note.id), key)).body).toBe('secret body'); + }); + + it('should drop a reset that another device overtook when no note depends on it', async () => { + await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + await synchronizerStart(); + await NoteLockSession.instance().reset('222222'); + + await switchToClient(1); + const winningKey = await NoteLockSession.instance().reset('333333'); + await synchronizerStart(); + + await switchToClient(2); + await synchronizerStart(); + expect(NoteLockKey.instance().load()).toEqual(winningKey); + expect(await remoteNoteLockKeyId()).toBe(winningKey.id); + expect(Setting.value('noteLock.passwordReset')).toBe(false); + expect(Setting.value('noteLock.keyIdToReset')).toBe(''); + expect(Setting.value('noteLock.conflictNoteLockKey')).toEqual({}); + }); + + it('should stop the sync and park the remote key when a different lineage meets local locked notes', async () => { + const remoteKey = await NoteLockKey.instance().create('111111'); + const remoteSyncMigrationId = localSyncInfo().syncMigrationId; + await synchronizerStart(); + + await switchToClient(2); + const localKey = await NoteLockKey.instance().create('222222'); + const localSyncMigrationId = localSyncInfo().syncMigrationId; + await Note.save({ title: 'locked', is_locked: 1 }); + + for (let i = 0; i < 2; i++) { + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toMatchObject({ code: ErrorCode.NoteLockKeyConflict }); + expect(NoteLockKey.instance().load()).toEqual(localKey); + expect(localSyncInfo().syncMigrationId).toBe(localSyncMigrationId); + expect(await remoteNoteLockKeyId()).toBe(remoteKey.id); + expect(Setting.value('noteLock.conflictNoteLockKey')).toEqual({ noteLockKey: remoteKey, syncMigrationId: remoteSyncMigrationId }); + } + }); + + it('should drop a local key that no note depends on and adopt the sync target key', async () => { + const remoteKey = await NoteLockKey.instance().create('111111'); + const remoteSyncMigrationId = localSyncInfo().syncMigrationId; + await synchronizerStart(); + + await switchToClient(2); + await NoteLockKey.instance().create('222222'); + await synchronizerStart(); + + expect(NoteLockKey.instance().load()).toEqual(remoteKey); + expect(localSyncInfo().syncMigrationId).toBe(remoteSyncMigrationId); + expect(await remoteNoteLockKeyId()).toBe(remoteKey.id); + expect(Setting.value('noteLock.conflictNoteLockKey')).toEqual({}); + }); + + it('should keep the sync stopped and say how to migrate when note lock is off', async () => { + Setting.setValue('featureFlag.noteLock', true); + await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + Setting.setValue('featureFlag.noteLock', true); + const localKey = await NoteLockKey.instance().create('222222'); + await NoteLockSession.instance().unlock('222222'); + await Note.save({ title: 'secret', body: 'secret body', is_locked: 1 }, { useNoteLock: true }); + Setting.setValue('featureFlag.noteLock', false); + + for (let i = 0; i < 2; i++) { + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toThrow('Enable the note lock feature'); + } + expect(NoteLockKey.instance().load()).toEqual(localKey); + }); + + it('should sync again after migrating local locked notes to the sync target key', async () => { + Setting.setValue('featureFlag.noteLock', true); + const remoteKey = await NoteLockKey.instance().create('111111'); + const remoteSyncMigrationId = localSyncInfo().syncMigrationId; + await synchronizerStart(); + + await switchToClient(2); + Setting.setValue('featureFlag.noteLock', true); + await NoteLockKey.instance().create('222222'); + await NoteLockSession.instance().unlock('222222'); + const note = await Note.save({ title: 'secret', body: 'secret body', is_locked: 1 }, { useNoteLock: true }); + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toMatchObject({ code: ErrorCode.NoteLockKeyConflict }); + + expect(await migrateLockedNotes('222222', '111111')).toEqual({ migrated: 1, skipped: 0, failed: 0, awaitingDecryption: 0 }); + finishNoteLockKeyMigration(); + await synchronizerStart(null, { throwOnError: true }); + expect(NoteLockKey.instance().load()).toEqual(remoteKey); + expect(localSyncInfo().syncMigrationId).toBe(remoteSyncMigrationId); + expect(await remoteNoteLockKeyId()).toBe(remoteKey.id); + + await switchToClient(1); + await synchronizerStart(); + const key = await NoteLockKey.instance().decrypt('111111'); + expect((await NoteLockNote.decryptBody(await Note.load(note.id), key)).body).toBe('secret body'); + }); + + it('should adopt the sync target key before the upload, so a note cannot be locked with the dropped key mid-sync', async () => { + Setting.setValue('featureFlag.noteLock', true); + const remoteKey = await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + Setting.setValue('featureFlag.noteLock', true); + await NoteLockKey.instance().create('222222'); + await NoteLockSession.instance().unlock('222222'); + + // Tries to lock a note with the local key while the info.json upload is in flight. + const api = synchronizer().api(); + const realPut = api.put.bind(api); + let lateLockError: Error = null; + const putSpy = jest.spyOn(api, 'put').mockImplementation(async (path, content, options) => { + if (path === 'info.json' && !lateLockError) { + try { + await Note.save({ title: 'late', body: 'late body', is_locked: 1 }, { useNoteLock: true }); + } catch (error) { + lateLockError = error; + } + } + return realPut(path, content, options); + }); + try { + await synchronizerStart(null, { throwOnError: true }); + } finally { + putSpy.mockRestore(); + } + + expect(lateLockError?.message).toBe('Note lock session is locked'); + expect(NoteLockKey.instance().load()).toEqual(remoteKey); + expect(noteLockKeyConflict()).toBeNull(); + }); + + it('should keep a reset that lands mid-sync for the next sync instead of clearing its flag', async () => { + await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + await synchronizerStart(); + let resetKey: MasterKeyEntity = null; + duringTheLockedNotesCheck(async () => { resetKey = await NoteLockSession.instance().reset('222222'); }); + await synchronizerStart(null, { throwOnError: true }); + + expect(NoteLockKey.instance().load()).toEqual(resetKey); + expect(Setting.value('noteLock.passwordReset')).toBe(true); + await synchronizerStart(null, { throwOnError: true }); + expect(await remoteNoteLockKeyId()).toBe(resetKey.id); + expect(Setting.value('noteLock.passwordReset')).toBe(false); + }); + + it('should not spend a leftover reset flag on a key that moved mid-sync', async () => { + await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + await synchronizerStart(); + // A flag left behind by an interrupted earlier sync. + Setting.setValue('noteLock.passwordReset', true); + let resetKey: MasterKeyEntity = null; + duringTheLockedNotesCheck(async () => { resetKey = await NoteLockSession.instance().reset('222222'); }); + + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toThrow('changed on this device'); + expect(Setting.value('noteLock.passwordReset')).toBe(true); + await synchronizerStart(null, { throwOnError: true }); + expect(await remoteNoteLockKeyId()).toBe(resetKey.id); + expect(Setting.value('noteLock.passwordReset')).toBe(false); + }); + + it('should hand a reset that lands after the local save the key the sync target holds', async () => { + await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + await synchronizerStart(); + const firstResetKey = await NoteLockSession.instance().reset('222222'); + // Resets again while the sync is finishing up, after the merged info was saved locally. + const handler = synchronizer().lockHandler(); + const realReleaseLock = handler.releaseLock.bind(handler); + let secondResetKey: MasterKeyEntity = null; + const spy = jest.spyOn(handler, 'releaseLock').mockImplementation(async (...args) => { + spy.mockRestore(); + secondResetKey = await NoteLockSession.instance().reset('333333'); + return realReleaseLock(...args); + }); + await synchronizerStart(null, { throwOnError: true }); + + expect(await remoteNoteLockKeyId()).toBe(firstResetKey.id); + expect(Setting.value('noteLock.passwordReset')).toBe(true); + expect(Setting.value('noteLock.keyIdToReset')).toBe(firstResetKey.id); + await synchronizerStart(null, { throwOnError: true }); + expect(await remoteNoteLockKeyId()).toBe(secondResetKey.id); + expect(Setting.value('noteLock.passwordReset')).toBe(false); + }); + + it('should back up the key the target holds when a sync finishes while a second reset is generating its key', async () => { + await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + await synchronizerStart(); + const firstResetKey = await NoteLockSession.instance().reset('222222'); + const service = encryptionService(); + const realGenerate = service.generateMasterKey.bind(service); + const spy = jest.spyOn(service, 'generateMasterKey').mockImplementation(async (...args) => { + spy.mockRestore(); + await synchronizerStart(null, { throwOnError: true }); + return realGenerate(...args); + }); + const secondResetKey = await NoteLockSession.instance().reset('333333'); + + expect(Setting.value('noteLock.keyIdToReset')).toBe(firstResetKey.id); + await synchronizerStart(null, { throwOnError: true }); + expect(await remoteNoteLockKeyId()).toBe(secondResetKey.id); + }); + + it('should adopt the sync target key in the sync that first uses a local master key', async () => { + const remoteKey = await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + await setupAndEnableEncryption(encryptionService(), await loadEncryptionMasterKey()); + await synchronizerStart(null, { throwOnError: true }); + + expect(NoteLockKey.instance().load()).toEqual(remoteKey); + }); + + it('should keep a password change that lands during the info upload', async () => { + await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + await synchronizerStart(); + + await switchToClient(1); + await NoteLockKey.instance().changePassword('111111', '222222'); + await synchronizerStart(); + + await switchToClient(2); + const api = synchronizer().api(); + const realPut = api.put.bind(api); + const putSpy = jest.spyOn(api, 'put').mockImplementation(async (path, content, options) => { + if (path === 'info.json') { + putSpy.mockRestore(); + await NoteLockKey.instance().changePassword('111111', '333333'); + } + return realPut(path, content, options); + }); + try { + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toThrow('changed on this device'); + } finally { + putSpy.mockRestore(); + } + + await expect(NoteLockKey.instance().decrypt('333333')).resolves.toBeTruthy(); + }); + + it('should not park the key again when the migration finishes while a sync is in flight', async () => { + Setting.setValue('featureFlag.noteLock', true); + const remoteKey = await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + Setting.setValue('featureFlag.noteLock', true); + await NoteLockKey.instance().create('222222'); + await NoteLockSession.instance().unlock('222222'); + await Note.save({ title: 'secret', body: 'secret body', is_locked: 1 }, { useNoteLock: true }); + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toMatchObject({ code: ErrorCode.NoteLockKeyConflict }); + await migrateLockedNotes('222222', '111111'); + duringTheLockedNotesCheck(async () => finishNoteLockKeyMigration()); + + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toThrow('changed on this device'); + expect(noteLockKeyConflict()).toBeNull(); + expect(NoteLockKey.instance().load()).toEqual(remoteKey); + await synchronizerStart(null, { throwOnError: true }); + expect(await remoteNoteLockKeyId()).toBe(remoteKey.id); + }); + + it('should not write back a key that another device replaced after this sync fetched the target info', async () => { + await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + const infoBeforeReset = await fileApi().get('info.json'); + + await switchToClient(2); + await synchronizerStart(); + const resetKey = await NoteLockSession.instance().reset('222222'); + await synchronizerStart(); + const infoAfterReset = await fileApi().get('info.json'); + + // Client 1 reads the target before the reset lands, and has its own change to upload. + await switchToClient(1); + onRevisionServiceSettingsChanged('revisionService.ttlDays', 45); + await fileApi().put('info.json', infoBeforeReset); + duringTheLockedNotesCheck(() => fileApi().put('info.json', infoAfterReset)); + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toThrow('changed during the sync'); + expect(await remoteNoteLockKeyId()).toBe(resetKey.id); + + await synchronizerStart(null, { throwOnError: true }); + expect(await remoteNoteLockKeyId()).toBe(resetKey.id); + expect(NoteLockKey.instance().load().id).toBe(resetKey.id); + }); + + it('should stop the sync when a note lock key has no sync migration id', async () => { + await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + const remoteInfo = await fetchSyncInfo(fileApi()); + remoteInfo.syncMigrationId = ''; + await fileApi().put('info.json', remoteInfo.serialize()); + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toThrow('migration ID'); + + const localInfo = localSyncInfo(); + localInfo.syncMigrationId = ''; + saveLocalSyncInfo(localInfo); + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toThrow('migration ID'); + }); + // Sets up client 2 with a locked note and a conflict parked against client 1's key on the sync target. + const parkConflictOnClient2 = async () => { + Setting.setValue('featureFlag.noteLock', true); + const remoteKey = await NoteLockKey.instance().create('111111'); + await synchronizerStart(); + + await switchToClient(2); + Setting.setValue('featureFlag.noteLock', true); + const localKey = await NoteLockKey.instance().create('222222'); + await NoteLockSession.instance().unlock('222222'); + const note = await Note.save({ title: 'secret', body: 'secret body', is_locked: 1 }, { useNoteLock: true }); + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toMatchObject({ code: ErrorCode.NoteLockKeyConflict }); + expect(noteLockKeyConflict()?.noteLockKey.id).toBe(remoteKey.id); + return { remoteKey, localKey, note }; + }; + + it('should sync again when the sync target no longer conflicts, for example after switching to an empty one', async () => { + const { localKey } = await parkConflictOnClient2(); + + await fileApi().clearRoot(); + await synchronizerStart(null, { throwOnError: true }); + + expect(noteLockKeyConflict()).toBeNull(); + expect(NoteLockKey.instance().load()).toEqual(localKey); + expect(await remoteNoteLockKeyId()).toBe(localKey.id); + }); + + it('should keep the parked key once a migration has moved notes to it, even if the sync target changes', async () => { + const { remoteKey, note } = await parkConflictOnClient2(); + const partner = await Note.save({ title: 'partner', body: 'partner body', is_locked: 1 }, { useNoteLock: true }); + // The first note moves, the second fails, so the run stops halfway with a note already under the parked key. + const save = Note.save.bind(Note); + const spy = jest.spyOn(Note, 'save').mockImplementation(async (n, options) => { + if (n.id === partner.id) throw new Error('simulated failure'); + return save(n, options); + }); + try { + expect(await migrateLockedNotes('222222', '111111')).toMatchObject({ migrated: 1, failed: 1 }); + } finally { + spy.mockRestore(); + } + + await fileApi().clearRoot(); + await expect(synchronizerStart(null, { throwOnError: true })).rejects.toMatchObject({ code: ErrorCode.NoteLockKeyConflict }); + expect(noteLockKeyConflict()?.noteLockKey.id).toBe(remoteKey.id); + const key = await decryptedRemoteKey(remoteKey); + expect((await NoteLockNote.decryptBody(await Note.load(note.id), key)).body).toBe('secret body'); + }); + +}); diff --git a/packages/lib/services/synchronizer/syncInfoUtils.test.ts b/packages/lib/services/synchronizer/syncInfoUtils.test.ts index 526a88f41c..5d954a7115 100644 --- a/packages/lib/services/synchronizer/syncInfoUtils.test.ts +++ b/packages/lib/services/synchronizer/syncInfoUtils.test.ts @@ -1,11 +1,13 @@ import { afterAllCleanUp, setupDatabaseAndSynchronizer, logger, switchClient, encryptionService, msleep, fileApi } from '../../testing/test-utils'; import MasterKey from '../../models/MasterKey'; -import { checkIfCanSync, localSyncInfo, localSyncInfoSelector, masterKeyEnabled, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyEnabled, setMasterKeyHasBeenUsed, SyncInfo, syncInfoEquals, checkSyncTargetIsValid, fetchSyncInfo, onRevisionServiceSettingsChanged, setAppMinVersion } from './syncInfoUtils'; +import { checkIfCanSync, localSyncInfo, localSyncInfoSelector, masterKeyEnabled, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyEnabled, setMasterKeyHasBeenUsed, SyncInfo, syncInfoEquals, checkSyncTargetIsValid, fetchSyncInfo, onRevisionServiceSettingsChanged, setAppMinVersion, checkNoteLockKeyConflict, checkNoteLockKeyMigrationId } from './syncInfoUtils'; import Setting from '../../models/Setting'; import BaseItem from '../../models/BaseItem'; import BaseModel from '../../models/BaseItem'; import Logger from '@joplin/utils/Logger'; import { State } from '../../reducer'; +import { MasterKeyEntity } from '../e2ee/types'; +import { ErrorCode } from '../../errors'; describe('syncInfoUtils', () => { it('should keep selected sync info stable while the cache is unchanged', () => { @@ -150,23 +152,104 @@ describe('syncInfoUtils', () => { expect(mergeSyncInfos(syncInfo1, syncInfo2).appMinVersion).toBe('0.0.0'); }); - it('should merge sync target info and keep the latest note lock key', () => { - const syncInfo1 = new SyncInfo(); - syncInfo1.noteLockKey = { - id: '1', - content: 'content1', - updated_time: 100, - }; + const noteLockKey = (id: string, updated_time: number): MasterKeyEntity => ({ id, content: `content${id}-${updated_time}`, updated_time }); - const syncInfo2 = new SyncInfo(); - syncInfo2.noteLockKey = { - id: '2', - content: 'content2', - updated_time: 200, - }; + const syncInfoWithNoteLockKey = (key: MasterKeyEntity, syncMigrationId: string) => { + const syncInfo = new SyncInfo(); + syncInfo.noteLockKey = key; + syncInfo.syncMigrationId = syncMigrationId; + return syncInfo; + }; - expect(mergeSyncInfos(syncInfo1, syncInfo2).noteLockKey).toEqual(syncInfo2.noteLockKey); - expect(mergeSyncInfos(new SyncInfo(), syncInfo1).noteLockKey).toEqual(syncInfo1.noteLockKey); + test.each([ + ['the newest copy of the same key', noteLockKey('1', 100), 'L1', noteLockKey('1', 200), 'L1', false, 'remote'], + ['the local key when a password reset in the same lineage propagates', noteLockKey('2', 100), 'L1', noteLockKey('1', 200), 'L1', true, 'local'], + ['the remote key in the same lineage without a propagating reset', noteLockKey('2', 300), 'L1', noteLockKey('1', 200), 'L1', false, 'remote'], + ['the remote key from a different lineage even with a propagating reset', noteLockKey('2', 300), 'L2', noteLockKey('1', 200), 'L1', true, 'remote'], + ['the only key, when it is remote', null, '', noteLockKey('1', 200), 'L1', false, 'remote'], + ['the only key, when it is local', noteLockKey('1', 200), 'L1', null, '', false, 'local'], + ])('should merge sync target info and keep %s', (_description, localKey, localLineage, remoteKey, remoteLineage, resetPropagates, winner) => { + const local = syncInfoWithNoteLockKey(localKey, localLineage); + const remote = syncInfoWithNoteLockKey(remoteKey, remoteLineage); + const expected = winner === 'local' ? local : remote; + + const merged = mergeSyncInfos(local, remote, { resetPropagates }); + + expect(merged.noteLockKey).toEqual(expected.noteLockKey); + expect(merged.syncMigrationId).toBe(expected.syncMigrationId); + }); + + it('should refuse to merge a different lineage while local notes depend on the local key', () => { + const local = syncInfoWithNoteLockKey(noteLockKey('2', 300), 'L2'); + const remote = syncInfoWithNoteLockKey(noteLockKey('1', 200), 'L1'); + + expect(() => mergeSyncInfos(local, remote, { hasLocalLockedNotes: true })).toThrow('different lineage'); + }); + + it('should stop the sync and park the remote key when lineages differ and local notes are locked', () => { + const local = syncInfoWithNoteLockKey(noteLockKey('2', 300), 'L2'); + const remote = syncInfoWithNoteLockKey(noteLockKey('1', 200), 'L1'); + + expect(() => checkNoteLockKeyConflict(local, remote, true, false)).toThrow(expect.objectContaining({ code: ErrorCode.NoteLockKeyConflict })); + expect(Setting.value('noteLock.conflictNoteLockKey')).toEqual({ noteLockKey: remote.noteLockKey, syncMigrationId: 'L1' }); + }); + + test.each([ + ['lineages differ but no local note is locked', 'L2', false], + ['the keys share a lineage', 'L1', true], + ])('should let the sync continue when %s', (_description, localLineage, hasLocalLockedNotes) => { + const local = syncInfoWithNoteLockKey(noteLockKey('2', 300), localLineage); + const remote = syncInfoWithNoteLockKey(noteLockKey('1', 200), 'L1'); + + expect(() => checkNoteLockKeyConflict(local, remote, hasLocalLockedNotes, false)).not.toThrow(); + expect(Setting.value('noteLock.conflictNoteLockKey')).toEqual({}); + }); + + it('should keep rejecting syncs while notes depend on the parked key', () => { + const parked = { noteLockKey: noteLockKey('1', 200), syncMigrationId: 'L1' }; + Setting.setValue('noteLock.conflictNoteLockKey', parked); + const local = syncInfoWithNoteLockKey(noteLockKey('3', 300), 'L3'); + const remote = syncInfoWithNoteLockKey(noteLockKey('3', 300), 'L3'); + + expect(() => checkNoteLockKeyConflict(local, remote, false, false, true)).toThrow(expect.objectContaining({ code: ErrorCode.NoteLockKeyConflict })); + expect(Setting.value('noteLock.conflictNoteLockKey')).toEqual(parked); + }); + + test.each([ + ['the sync target has no key', null], + ['the sync target has the local key', noteLockKey('3', 300)], + ])('should drop a parked key that no note depends on when %s', (_description, remoteKey) => { + Setting.setValue('noteLock.conflictNoteLockKey', { noteLockKey: noteLockKey('1', 200), syncMigrationId: 'L1' }); + const local = syncInfoWithNoteLockKey(noteLockKey('3', 300), 'L3'); + const remote = syncInfoWithNoteLockKey(remoteKey, remoteKey ? 'L3' : ''); + + expect(() => checkNoteLockKeyConflict(local, remote, true, false)).not.toThrow(); + expect(Setting.value('noteLock.conflictNoteLockKey')).toEqual({}); + }); + + it('should park the new sync target key when it changed and no note depends on the old one', () => { + Setting.setValue('noteLock.conflictNoteLockKey', { noteLockKey: noteLockKey('1', 200), syncMigrationId: 'L1' }); + const local = syncInfoWithNoteLockKey(noteLockKey('3', 300), 'L3'); + const remote = syncInfoWithNoteLockKey(noteLockKey('4', 400), 'L4'); + + expect(() => checkNoteLockKeyConflict(local, remote, true, false)).toThrow(expect.objectContaining({ code: ErrorCode.NoteLockKeyConflict })); + expect(Setting.value('noteLock.conflictNoteLockKey')).toEqual({ noteLockKey: remote.noteLockKey, syncMigrationId: 'L4' }); + }); + + test.each([ + ['a key without a sync migration id', noteLockKey('1', 200), '', true], + ['a key with a sync migration id', noteLockKey('1', 200), 'L1', false], + ['no key', null, '', false], + ])('should check the sync migration id: %s', (_description, key, syncMigrationId, throws) => { + const syncInfo = syncInfoWithNoteLockKey(key, syncMigrationId); + for (const [isRemote, side] of [[false, 'on this device'], [true, 'on the sync target']] as const) { + const check = () => checkNoteLockKeyMigrationId(syncInfo, isRemote); + if (throws) { + expect(check).toThrow(side); + } else { + expect(check).not.toThrow(); + } + } }); it('should merge sync target info and takes into account usage of master key - 1', async () => { @@ -313,6 +396,7 @@ describe('syncInfoUtils', () => { 'value': '400227d2222c4d3bb7346514861c643b', }, 'appMinVersion': '0.0.0', + 'syncMigrationId': '', 'e2ee': { 'updatedTime': 0, 'value': true, diff --git a/packages/lib/services/synchronizer/syncInfoUtils.ts b/packages/lib/services/synchronizer/syncInfoUtils.ts index e11c3fffca..ce9c3a0ecf 100644 --- a/packages/lib/services/synchronizer/syncInfoUtils.ts +++ b/packages/lib/services/synchronizer/syncInfoUtils.ts @@ -12,6 +12,7 @@ import JoplinError from '../../JoplinError'; import { ErrorCode } from '../../errors'; import fastDeepEqual = require('fast-deep-equal'); import { createSelector } from 'reselect'; +import isNoteLockEnabled from '../noteLock/isNoteLockEnabled'; const logger = Logger.create('syncInfoUtils'); @@ -35,6 +36,17 @@ export interface SyncInfoValuePublicPrivateKeyPair { updatedTime: number; } +export interface MergeSyncInfosOptions { + resetPropagates?: boolean; + hasLocalLockedNotes?: boolean; +} + +// The sync target's note lock key and sync migration id, parked while a sync is stopped on a conflict. +export interface NoteLockKeyConflict { + noteLockKey: MasterKeyEntity; + syncMigrationId: string; +} + // This should be set to the client version whenever we require all the clients to be at the same // version in order to synchronise. One example is when adding support for the trash feature - if an // old client that doesn't know about this feature synchronises data with a new client, the notes @@ -269,7 +281,7 @@ const mergeActiveMasterKeys = (s1: SyncInfo, s2: SyncInfo, output: SyncInfo) => }; // If there is a distinction, s1 should be local sync info and s2 remote. -export function mergeSyncInfos(s1: SyncInfo, s2: SyncInfo): SyncInfo { +export function mergeSyncInfos(s1: SyncInfo, s2: SyncInfo, options: MergeSyncInfosOptions = {}): SyncInfo { const output: SyncInfo = new SyncInfo(); output.setWithTimestamp(s1.keyTimestamp('e2ee') > s2.keyTimestamp('e2ee') ? s1 : s2, 'e2ee'); @@ -292,15 +304,23 @@ export function mergeSyncInfos(s1: SyncInfo, s2: SyncInfo): SyncInfo { } } - const noteLockKey1 = s1.noteLockKey; - const noteLockKey2 = s2.noteLockKey; - if (!noteLockKey1) { - output.noteLockKey = noteLockKey2; - } else if (!noteLockKey2) { - output.noteLockKey = noteLockKey1; + let noteLockKeySource: SyncInfo; + if (!s2.noteLockKey) { + noteLockKeySource = s1; + } else if (!s1.noteLockKey) { + noteLockKeySource = s2; + } else if (s1.noteLockKey.id === s2.noteLockKey.id) { + noteLockKeySource = (s1.noteLockKey.updated_time || 0) >= (s2.noteLockKey.updated_time || 0) ? s1 : s2; + } else if (s1.syncMigrationId === s2.syncMigrationId) { + noteLockKeySource = options.resetPropagates ? s1 : s2; + } else if (!options.hasLocalLockedNotes) { + noteLockKeySource = s2; } else { - output.noteLockKey = (noteLockKey1.updated_time || 0) >= (noteLockKey2.updated_time || 0) ? noteLockKey1 : noteLockKey2; + // checkNoteLockKeyConflict() stops the sync before this point, so this only guards against a future caller skipping it. + throw new Error('Cannot merge a note lock key from a different lineage while local notes depend on the local key'); } + output.noteLockKey = noteLockKeySource.noteLockKey; + output.syncMigrationId = noteLockKeySource.syncMigrationId; // We use >= so that the version from s1 (local) is preferred to the version in s2 (remote). // For example, if s2 has appMinVersion 0.00 and s1 has appMinVersion 0.0.0, we choose the @@ -321,6 +341,7 @@ export class SyncInfo { private activeMasterKeyId_: SyncInfoValueString; private masterKeys_: MasterKeyEntity[] = []; private noteLockKey_: MasterKeyEntity = null; + private syncMigrationId_ = ''; private ppk_: SyncInfoValuePublicPrivateKeyPair; private appMinVersion_: string = appMinVersion_; private revisionServiceEnabled_: SyncInfoValueBoolean; @@ -343,6 +364,7 @@ export class SyncInfo { activeMasterKeyId: this.activeMasterKeyId_, masterKeys: this.masterKeys, noteLockKey: this.noteLockKey, + syncMigrationId: this.syncMigrationId, ppk: this.ppk_, appMinVersion: this.appMinVersion, revisionServiceEnabled: this.revisionServiceEnabled_, @@ -393,6 +415,7 @@ export class SyncInfo { this.activeMasterKeyId_ = 'activeMasterKeyId' in s ? s.activeMasterKeyId : { value: '', updatedTime: 0 }; this.masterKeys_ = 'masterKeys' in s ? s.masterKeys : []; this.noteLockKey_ = 'noteLockKey' in s ? s.noteLockKey : null; + this.syncMigrationId_ = 'syncMigrationId' in s ? s.syncMigrationId : ''; this.ppk_ = 'ppk' in s ? s.ppk : { value: null, updatedTime: 0 }; this.appMinVersion_ = s.appMinVersion ? s.appMinVersion : '0.0.0'; this.revisionServiceEnabled_ = 'revisionServiceEnabled' in s ? s.revisionServiceEnabled : { value: true, updatedTime: 0 }; @@ -502,6 +525,16 @@ export class SyncInfo { this.noteLockKey_ = v; } + // Identifies the note lock key lineage: created with the first key, kept through password resets and + // replaced only when the user migrates to a sync target's key. + public get syncMigrationId(): string { + return this.syncMigrationId_; + } + + public set syncMigrationId(v: string) { + this.syncMigrationId_ = v; + } + public keyTimestamp(name: string): number { const self = this as unknown as Record; if (!(`${name}_` in self)) throw new Error(`Invalid name: ${name}`); @@ -576,8 +609,7 @@ export const setMasterKeyHasBeenUsed = (s: SyncInfo, mkId: string) => { updated_time: Date.now(), }; - saveLocalSyncInfo(s); - + // Saved by the caller with the rest of the merge, so the sync never persists a candidate its checks may still reject. return s; }; @@ -604,6 +636,83 @@ export function masterKeyById(id: string) { return localSyncInfo().masterKeys.find(mk => mk.id === id); } +const noteLockKeyConflictSettingKey = 'noteLock.conflictNoteLockKey'; + +export const noteLockKeyConflict = (): NoteLockKeyConflict | null => { + const conflict = Setting.value(noteLockKeyConflictSettingKey) as Partial; + return conflict.noteLockKey ? conflict as NoteLockKeyConflict : null; +}; + +export const noteLockKeyConflictFromState = (state: State) => { + return isNoteLockEnabled() && !!(state.settings[noteLockKeyConflictSettingKey] as Partial)?.noteLockKey; +}; + +// Replaces the local key and lineage with the sync target's, so callers migrate first or warn: notes still locked +// with the local key become unreadable. A pending reset belonged to the dropped lineage, so it is cleared too. +export const adoptNoteLockKeyConflict = () => { + const conflict = noteLockKeyConflict(); + if (!conflict) throw new Error('No note lock key conflict to adopt'); + const syncInfo = localSyncInfo(); + syncInfo.noteLockKey = conflict.noteLockKey; + syncInfo.syncMigrationId = conflict.syncMigrationId; + saveLocalSyncInfo(syncInfo); + Setting.setValue(noteLockKeyConflictSettingKey, {}); + Setting.setValue('noteLock.passwordReset', false); + Setting.setValue('noteLock.keyIdToReset', ''); +}; + +// Different lineages with local notes depending on the local key stop the sync: the remote key is parked for the +// migration and keeps rejecting syncs until resolved. A stale reset (another device reset first) counts as a different lineage. +// A parked key is checked again against the current target, for example after switching to another sync target, unless a +// migration is running or has already moved notes to it (parkedKeyInUse), since those notes need it. +export const checkNoteLockKeyConflict = (local: SyncInfo, remote: SyncInfo, hasLocalLockedNotes: boolean, staleReset: boolean, parkedKeyInUse = false) => { + const parked = noteLockKeyConflict(); + if (!parked || !parkedKeyInUse) { + const keysDiffer = !!local.noteLockKey && !!remote.noteLockKey && local.noteLockKey.id !== remote.noteLockKey.id; + // The scenarios where a conflict occurs are when a different device has set up note lock for the first time (noteLockKey + // and syncMigrationId both differ) and when both remote and local have reset the password at the point of syncing (which + // makes staleReset true). staleReset is false both when a local password reset should propagate and when only a remote + // password reset was made (which silently accepts the remote key), and syncMigrationId is what tells a password reset + // apart from a key created for the first time. + if (!keysDiffer || (local.syncMigrationId === remote.syncMigrationId && !staleReset) || !hasLocalLockedNotes) { + if (parked) Setting.setValue(noteLockKeyConflictSettingKey, {}); + return; + } + const conflict: NoteLockKeyConflict = { noteLockKey: remote.noteLockKey, syncMigrationId: remote.syncMigrationId }; + if (!parked || parked.syncMigrationId !== conflict.syncMigrationId || parked.noteLockKey.id !== conflict.noteLockKey.id || parked.noteLockKey.updated_time !== conflict.noteLockKey.updated_time) { + Setting.setValue(noteLockKeyConflictSettingKey, conflict); + } + } + throw new JoplinError(isNoteLockEnabled() + ? _('Synchronisation was stopped because the sync target uses a different note lock key to the one on your device. Your locked notes must be migrated to the synced key before synchronisation can continue.') + : _('Synchronisation was stopped because a note lock key migration is required. Enable the note lock feature to migrate your locked notes.'), ErrorCode.NoteLockKeyConflict); +}; + +const sameNoteLockKey = (a: SyncInfo, b: SyncInfo) => { + return a.noteLockKey?.id === b.noteLockKey?.id && a.noteLockKey?.updated_time === b.noteLockKey?.updated_time && a.syncMigrationId === b.syncMigrationId; +}; + +// A reset, password change or migration that lands while a sync is in flight would be clobbered by the sync's +// local save, so the sync stops instead and the next one starts from the new state. +export const checkNoteLockKeyUnchanged = (snapshot: SyncInfo) => { + if (sameNoteLockKey(localSyncInfo(), snapshot)) return; + throw new Error(_('Synchronisation was stopped because the note lock key changed on this device during the sync. Please synchronise again.')); +}; + +export const checkRemoteNoteLockKeyUnchanged = (fetched: SyncInfo, latest: SyncInfo) => { + if (sameNoteLockKey(fetched, latest)) return; + throw new Error(_('Synchronisation was stopped because the note lock key on the sync target changed during the sync. Please synchronise again.')); +}; + +// Sync migration ids predate the release of the note lock feature, so a key without one comes from a +// development build and cannot take part in the reset and migration handling. +export const checkNoteLockKeyMigrationId = (s: SyncInfo, isRemote: boolean) => { + if (!s.noteLockKey || s.syncMigrationId) return; + throw new Error(isRemote + ? _('Synchronisation was stopped because the note lock key on the sync target has no sync migration ID.') + : _('Synchronisation was stopped because the note lock key on this device has no sync migration ID. Please reset the note lock password.')); +}; + export const checkIfCanSync = (s: SyncInfo, appVersion: string) => { const isForwardCompatible = () => { // Forward compatibility: This version of Joplin supports the Joplin 3.7 sync target format