From bd20ecff78191baa673608ba686b21090d0a4b70 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Wed, 13 Dec 2017 22:53:20 +0000 Subject: [PATCH] All: Handle conflict for encrypted notes --- CliClient/tests/synchronizer.js | 53 +++++++++++++++++++++------- ReactNativeClient/lib/models/note.js | 4 +++ 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/CliClient/tests/synchronizer.js b/CliClient/tests/synchronizer.js index 87c715481..4b124a603 100644 --- a/CliClient/tests/synchronizer.js +++ b/CliClient/tests/synchronizer.js @@ -570,13 +570,11 @@ describe('Synchronizer', function() { done(); }); - it('should not consider it is a conflict if neither the title nor body of the note have changed', async (done) => { - // That was previously a common conflict: - // - Client 1 mark todo as "done", and sync - // - Client 2 doesn't sync, mark todo as "done" todo. Then sync. - // In theory it is a conflict because the todo_completed dates are different - // but in practice it doesn't matter, we can just take the date when the - // todo was marked as "done" the first time. + async function ignorableConflictTest(withEncryption) { + if (withEncryption) { + Setting.setValue('encryption.enabled', true); + await loadEncryptionMasterKey(); + } let folder1 = await Folder.save({ title: "folder1" }); let note1 = await Note.save({ title: "un", is_todo: 1, parent_id: folder1.id }); @@ -599,13 +597,36 @@ describe('Synchronizer', function() { note2conf = await Note.load(note1.id); await synchronizer().start(); - let conflictedNotes = await Note.conflictedNotes(); - expect(conflictedNotes.length).toBe(0); + if (!withEncryption) { + // That was previously a common conflict: + // - Client 1 mark todo as "done", and sync + // - Client 2 doesn't sync, mark todo as "done" todo. Then sync. + // In theory it is a conflict because the todo_completed dates are different + // but in practice it doesn't matter, we can just take the date when the + // todo was marked as "done" the first time. - let notes = await Note.all(); - expect(notes.length).toBe(1); - expect(notes[0].id).toBe(note1.id); - expect(notes[0].todo_completed).toBe(note2.todo_completed); + let conflictedNotes = await Note.conflictedNotes(); + expect(conflictedNotes.length).toBe(0); + + let notes = await Note.all(); + expect(notes.length).toBe(1); + expect(notes[0].id).toBe(note1.id); + expect(notes[0].todo_completed).toBe(note2.todo_completed); + } else { + // If the notes are encrypted however it's not possible to do this kind of + // smart conflict resolving since we don't know the content, so in that + // case it's handled as a regular conflict. + + let conflictedNotes = await Note.conflictedNotes(); + expect(conflictedNotes.length).toBe(1); + + let notes = await Note.all(); + expect(notes.length).toBe(2); + } + } + + it('should not consider it is a conflict if neither the title nor body of the note have changed', async (done) => { + await ignorableConflictTest(false); done(); }); @@ -702,6 +723,12 @@ describe('Synchronizer', function() { done(); }); + it('should always handle conflict if local or remote are encrypted', async (done) => { + await ignorableConflictTest(true); + + done(); + }); + // TODO: test tags // TODO: test resources diff --git a/ReactNativeClient/lib/models/note.js b/ReactNativeClient/lib/models/note.js index f5359fbdc..144dbd886 100644 --- a/ReactNativeClient/lib/models/note.js +++ b/ReactNativeClient/lib/models/note.js @@ -438,6 +438,10 @@ class Note extends BaseItem { // That shouldn't happen so throw an exception if (localNote.id !== remoteNote.id) throw new Error('Cannot handle conflict for two different notes'); + // For encrypted notes the conflict must always be handled + if (localNote.encryption_cipher_text || remoteNote.encryption_cipher_text) return true; + + // Otherwise only handle the conflict if there's a different on the title or body if (localNote.title !== remoteNote.title) return true; if (localNote.body !== remoteNote.body) return true;