Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Advanced Sync / Merge #17

Closed
mmcguill opened this issue Nov 22, 2018 · 36 comments
Closed

Advanced Sync / Merge #17

mmcguill opened this issue Nov 22, 2018 · 36 comments

Comments

@mmcguill
Copy link
Collaborator

KeePass sync involves comparing the database before writing changes back to it. Comparing records by UUID and timestamp, and then taking the latest entry, and moving the older staler entry to History. This allows for multiple editors to work on the Database and avoid sync conflicts.

It would involve comparing the XML documents, and so applies only to KeePass 2 Databases. It also depends on the History feature which needs to be implemented separately. More info:

https://keepass.info/help/v2/sync.html

@mmcguill mmcguill self-assigned this Nov 22, 2018
@jacek-1
Copy link

jacek-1 commented Feb 26, 2019

Hi Mark, first of all, great work with your KeePass oriented app for iOS/MacOS,

any predictions whether that feature could be implemented ?

that would greatly help with choosing your app as the only one for a solution with e.g. self-hosted nextcloud in order to safely sync keepass master db between multiple platforms

@mmcguill
Copy link
Collaborator Author

Thank you @jacek-1!

This feature is a bit of a tough one. I think the window for issues here is probably reasonably small for a lot of users. It is also dependent on a few features that need to be implemented first.

  1. File Metadata for all Storage Providers (Last Modified dates basically for 6 or 7 different providers like Google Drive, Dropbox, etc). This is kind of grunt work, but required.
  2. Managing Password History fully.
  3. Finally, implementing full database comparison and merging isn't trivial either, quite a chunky bit of work.

I think there are probably some other features that take precedence before this, so I think I'd really struggle to give you an ETA, but if pressed, I'd say in the next 3-6 months, hopefully sooner.

You could always open your databases in 'Read-Only' mode if you're concerned about overwriting a newer edit for the moment. This way Strongbox can/will not write back to storage.

@jacek-1
Copy link

jacek-1 commented Feb 26, 2019

perfectly fine with it, thanks for giving details.

@sn3ak
Copy link

sn3ak commented Jul 25, 2019

It would be really nice to have this. I switched to your (paid) product due to one of your competitors failing to do this.

As a forgetful guy, and app(s) without transparency it's very easy to lose passwords when things change on both sides.

I lost numerous passwords in my database do to your competitor apparently deciding to save blindly, not caring that the hosted file may have been changed. In these cases the product didn't update it's cached copy (and didn't notify that it failed) which would have included the updated passwords, and then overwrote with old data along with the one entry I added from my device.

@mmcguill
Copy link
Collaborator Author

That's fair. I'd like to get this done, but it's a little back on the roadmap at the moment, basically because it's a reasonably big dev task. Hopefully I'll be able to get to this in the next couple of months.

@sn3ak
Copy link

sn3ak commented Jul 25, 2019

That's fine, just adding my 'vote of support' and giving a use case where it might be more important to more users.

Keep up the good work!

@gitlarssondk
Copy link

I’ll become a paid customer as soon as this is implemented and working.

@Slummi
Copy link

Slummi commented Oct 19, 2019

Hey @mmcguill - Do you have any news about the schedule of this feature?

@mmcguill
Copy link
Collaborator Author

Hi @Slummi - I'm afraid not... It has slipped back on the roadmap as other problems and support issues consume resources. I would hope to have something here within the next 3-6 months, but unfortunately it is hard to put a good estimate on this because it's quite a niche feature with a heavy development load

@chessgriffin
Copy link

We are becoming more and more mobile these days so I find myself editing on mobile devices far more than traditional desktops. So being able to edit and then sync from mobile is a top 3 feature need for me. Thanks!

@Slummi
Copy link

Slummi commented Aug 17, 2020

Short question, because you recently started implementing of advanced sync: Are you still planning to support UUID sync for KeePass databses and maybe also lock files (for read-only access, if a database has been opened by another application)?

@strongbox-mark
Copy link
Member

Hi @Slummi, Yes, this is coming in the next few weeks. The first priority was supporting the ability to detect conflict situations and background syncing... This is pretty much done now barring some initial bugs/problems that need to be ironed out.

The next stage is to offer the ability to "Merge" the local/remote copies (optionally automatically without asking). This is the "UUID Sync" I assume you're speaking about. It's a big feature (complex algorithm to get right).

As to Locking Files, I don't see this as connected to the Advanced Sync feature and I'm not sure how it would work. How would Strongbox detect that the database has been opened by another app? Why would you want need this feature if you have the "Merge/UUID Sync" function?

@Slummi
Copy link

Slummi commented Aug 17, 2020

Hi @strongbox-mark Yes, merging the local/remote copies is what I mean with UUID sync. I just wanted to know, if it's still on the roadmap.

Locking files was just an idea, because it's an (optional) available function of KeePass, if you don't use the sync feature.
In this case KeePass can automatically create an additional lock file, which signals that the database file is currently used by another client and can only opened in read-only mode. But I think this is more a KeeePass 1.x feature and merging by UUID comparision is much better.

Here is some information about locking files:

Added option to use database lock files (intended for storage providers that don't lock files while writing to them, like e.g. some FTP servers); the option is turned off by default (and especially for local files and files on a network share it's recommended to leave it turned off).

Source: https://keepass.info/news/n120105_2.18.html (KeePass 2.18 change log)

And the KeePass documentation about multi-user features: https://keepass.info/help/base/multiuser.html

@strongbox-mark
Copy link
Member

Got it. Yeah, I don't think the lock file is a runner for Strongbox. With all the different storage cloud providers and permissions issues of listing/checking for the existence of a lock file it would be more hassle than I think it's worth...

Merge/Sync should have you covered in the next few weeks :)

@strongbox-mark strongbox-mark changed the title Support Advanced KeePass Sync Advanced KeePass Sync / Merge Nov 8, 2020
@strongbox-mark strongbox-mark changed the title Advanced KeePass Sync / Merge Advanced Sync / Merge Nov 8, 2020
@Slummi
Copy link

Slummi commented Jan 15, 2021

Hi @strongbox-mark,

Is it intended that, when comparing a KeePass 2 4.0 and a KeePass 2 3.1 database, the message comes that the comparison is only partially supported?
I'm not sure, but I believe that KeePass 2 3.1 also supports the detection of moves, deletions and group edits. If not, the note should also include the version of the format and not only "KeePass 2".

In the current test scenario (comparison of a 3.1 DB with a 4.0 DB) the simple comparison says that the database is 97.2% different (completely different databases compared), while the comparison for merging says that the databases are identical.

@Slummi
Copy link

Slummi commented Jan 15, 2021

The "Compare Partially Supported" message is also displayed when you compare two KeePass 2 4.0 databases that have different settings (e.g. encryption, key derivation). I guess that you check too many database attributes. Format version should be enough.

@strongbox-mark
Copy link
Member

Hi @Slummi - This looks to be a bug alright, it should only happen if you use different master credentials for the databases. Do you have a different master passwords on these databases? If so, you could test by setting the second master password to the same as the first.

To any other followers of this issue, you can now start using the Advanced Sync feature in 1.51.2 available on the App Store.

@Slummi
Copy link

Slummi commented Jan 15, 2021

Hi @strongbox-mark - You're right. That message is displayed when the master credentials are different. But what do the master credentials have to do with the KeePass format.

During this test I found two other bugs.

  1. I can't change the master credentials of a database. When I go to that menu, enter the new master key and press 'Done', I get the message that the credentials have been changed, but I can't open the database with the new credentials. The old credentials are still valid.
  2. I don't have to enter the master key for the second database even if the master passwords of the databases are different. After tapping "Select second database" the comparison immediately starts. How's this possible? I used local databases without Face ID for testing.

@strongbox-mark
Copy link
Member

Yeah, long story but your first question and 2nd point are related.

Your 1st point sounds like a bug though too...

@JLWFuQrioea69ugsykvQcg
Copy link

I’m comparing a database to a copy of the database with only a change in a single record. The results are unexpected showing a 52%+ (700+records) delta. There should only be one record that appears in the results. When I look at one of the other entries to observe the difference it shows something related to the auto type. I’ve included a screenshot of it.
Image

@strongbox-mark
Copy link
Member

That's very interesting @Shad0wHawk. Some questions:

  1. Version of Strongbox you're using?
  2. Do these databases have different master credentials?
  3. Are all the spurious differences related to "AutoType"?
  4. Has Strongbox correctly detected the difference in the single record you expect?

@JLWFuQrioea69ugsykvQcg
Copy link

JLWFuQrioea69ugsykvQcg commented Jan 15, 2021

If I copy the database without saving and compare then they are identical. If I make any changes that result in a save, this issue occurs where it detects differences in the auto type value. I’ve noticed it also states the attachments are different even though they haven’t been changed either. This only applies to entries with an attachment. The auto type discrepancy only applies to entries with an auto type string.

Image

I’m using 1.51.2 so the latest version.

@JLWFuQrioea69ugsykvQcg
Copy link

JLWFuQrioea69ugsykvQcg commented Jan 15, 2021

I didn’t answer your last question in my last post. The compare correctly detected the attribute that changed for the entry that was changed except it also included the auto type change which is not accurate.

@strongbox-mark
Copy link
Member

Can you tell me:

  1. Are you using any other KeePass apps with this database?
  2. What was the last App to change this database?
  3. How or where was the "Copy" made in your initial post above?
  4. Same questions 1 and 2 for the "Copy"?
  5. Do you use this AutoType feature at all?

@JLWFuQrioea69ugsykvQcg
Copy link

JLWFuQrioea69ugsykvQcg commented Jan 15, 2021

In general, I use the Windows Keepass app, iOS Strongbox and MacOS Strongbox apps to update the database.

This test database was last updated in Strongbox on iOS to test the comparison feature. Prior to that it was updated using Keepass 2.47 on Windows. To create a local copy, I long pressed the source database in Strongbox and made a copy. If I compare at this point, they are the same. If I then update one of the databases (source or target), I get the large delta I mentioned previously.

I do use the auto type feature in Windows, MacOS and iOS.

Both databases use the same credentials.

If I open, edit, and save both the original database and the copy on Strongbox, the compare doesn’t show the auto type and attachment deltas. The auto type and attachment deltas seem to occur if I compare a database that was edited in Strongbox with a database that was last saved in Keepass for Windows.

@strongbox-mark
Copy link
Member

That's "weird"...

Your answer to question 2 is very unexpected and so I just want to be absolutely 100% definite about it.

You are 100% sure that your original database (before your test edit) was last saved/edited by Strongbox?

@JLWFuQrioea69ugsykvQcg
Copy link

I updated my last comment with additional info. It was last saved using Keepass 2.47 for Windows. My database resides in OneDrive and syncs between applications and devices. Sometimes I use Keepass to edit and save, other times Strongbox. I’ve never had any issues with this method until now.

@strongbox-mark
Copy link
Member

Yeah, that's exactly what I thought. This looks like a slight difference between how KeePass on Windows serializes out and how Strongbox does.

I should be able to reproduce this and eliminate the spurious diff.

Thanks @Shad0wHawk

@JLWFuQrioea69ugsykvQcg
Copy link

I hope that helps. Hopefully you get it figured out. I really like the compare functionality and plan on using it once this issue is resolved. Thanks for all of your hard work implementing this feature.

@strongbox-mark
Copy link
Member

For sure, very helpful. There's alway some teething issues like this with new features, especially one this complex. We'll get there.

@JLWFuQrioea69ugsykvQcg
Copy link

JLWFuQrioea69ugsykvQcg commented Jan 15, 2021

I just realized these changes show up under the “History will Change” section if I select Compare and Merge instead of Simple Compare. So maybe this only impacts the historical entries in some way. Or maybe that means the a historical item will be created for those entries.

It’s interesting that I get substantially different results if I use Simple Compare (59.7% delta—800+ records delta) versus Compare and Merge (52.3% delta—700+ records delta).

Another interesting tidbit is that if I compare and merge the two databases and then recompare, only the true changes (1 record) merged, with those auto type entries still showing a delta that isn’t resolved.

@strongbox-mark
Copy link
Member

@Slummi - Your issues should be fixed now with the release of 1.51.4 - could you verify?

@Shad0wHawk - Your issues should also be fixed with that release - could you check?

@Slummi
Copy link

Slummi commented Jan 18, 2021

Hi @strongbox-mark,

Looks good! Changing the master credentials works again in 1.51.4 and you have to enter the credentials for the second database if they are different. There's also no message that comparision is partially supported if the database format is the same, but the credentials are different.

One suggestion for improvment:
It would be nice to have an option to go back to the screen, where you can choose the comparision type. Then you don't have to select and open both databases again when you want to do a simple comparison first and after that a comparison for merge.

@strongbox-mark
Copy link
Member

Great, yeah for sure, I agree with you on the improvement suggestion, keep an eye out for that in the next release.

@JLWFuQrioea69ugsykvQcg
Copy link

JLWFuQrioea69ugsykvQcg commented Jan 18, 2021

I just tested it and it works as expected. The false positives are no longer showing. Nice job fixing that so quickly. I agree with @Slummi about being able to go back a screen in the comparison. I noticed that as well and had the same thought.

@strongbox-mark
Copy link
Member

strongbox-mark commented Jan 18, 2021

That's great thanks @Shad0wHawk - I'm going to close this one out, please open new issues if necessary.

Edit: Blog Post up here: https://strongboxsafe.com/updates/advanced-sync-and-auto-merge-ios/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants