Skip to content
This repository has been archived by the owner on Dec 9, 2021. It is now read-only.

Automatic reload when the database get externally modified #56

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

testcocoon
Copy link

Feature: if the .kdbx database get modified by an other instance of Keepassx, Keepassx detect it and reload the database automatically.
The detection works also on network share and so permits that all user which have the same database open are not working on outdated databases.

This does not handle concurrent edition of the same .kdbx file. In this case a lock file or a merge mechanism is also necessary.

Sebastien Fricker added 3 commits May 3, 2014 16:59
When the database if modified by an other instance of KeePassX, KeePassX
detect it and reload automatically the database.
Conflicts:
	src/gui/DatabaseWidget.cpp
@jmena
Copy link

jmena commented Jun 9, 2014

Nice feature. I have my password file in dropbox, what happens if modified the file externally and locally? would it ignore my local changes and just reload the new file?

@testcocoon
Copy link
Author

The FileSystemWatcher class is a wrapper to QFileSystemWatcher (http://qt-project.org/doc/qt-4.8/qfilesystemwatcher.html#details). It works only on mounted file system, and I'm not sure it is works on other shared file system that NFS or Samba.

Wrapping FileSystemWatcher was necessary to handle the deletion and creation of .kdbx files.

If you use dropbox, your remote modifications will be ignored until you do a local update. This is what I'm supposing, but I'm not a dropbox user.

@airdrummingfool
Copy link

This appears to be similar (at least in spirit) to #12. Getting one of these two merged would be most excellent.

@testcocoon: What happens if the local db has been modified, but not saved, when the database on disk is changed? Does the user lose their modifications when the file is automatically reloaded?

@jmena: if the file was modified locally and elsewhere via Dropbox simultaneously, it would be up to Dropbox to handle the conflict (it will choose one to rename to [original filename] [user]'s Conflicted Copy [date]). If that were to happen, there would still only be one update to the file that the FileSystemWatcher is watching and everything would work as intended.

@testcocoon
Copy link
Author

This is definitely not the same as #12 because the reload button is a manual operation. Here keepassx detect that a file is modified and reload it automatically.

Actually, the user changes get lost when the databse get modified and automatically reloaded.
So I think that this option should depend from the "Automatically save on each change".
And a message box should inform the user that the current of this situation.

But better would be to merge the database reloaded with the current version. Is this possible?

@francoisferrand
Copy link

#12 does not add a reload button: it automatically detects background changes, and then, depending on the settings and the status of the database (i.e. some pending modifications), will either:

  • ignore the change
  • reload the database
  • ask the user what to do

@testcocoon
Copy link
Author

I was reading the thread too fast.
The implementation of #12 is better than mine with the following exception: it uses directly QFileSystemWatcher.
I wrote an own wrapper for QFileSystemWatcher because when the file get deleted, the watch is also silently removed.
My class FileSystemWatcher handle this, by setting a watch to the directory of the file watched.
Specially it handle the following use case:

  1. an external tool delete the .kdbx databse
  2. and then replace it by an own version

I pick this wrapper from the testcocoon project. But since I'm the author of it, don't be worry about any copyright issues.

@jleclanche
Copy link
Contributor

  1. an external tool delete the .kdbx databse
  2. and then replace it by an own version

This sounds like the sort of stuff you should not handle, tbh. If a tool does that, it's doing it wrong.

@testcocoon
Copy link
Author

This sounds like the sort of stuff you should not handle, tbh. If a tool does that, it's doing it wrong.

This is a wrong assumption, some tools are processing as followings:

  1. Download to a temporary file
  2. Swap the file once the transfer is finished:
    1. Delete the old file
    2. Rename the downloaded temporary file
This is a really safe method to not get corrupt files after a partial download. As far I know rsync is doing so and may be unison.

@andrewleech
Copy link

I'd like to add my +1 to get this merged in, a very valuable feature for people syncing a database across many systems (fwiw I use btsync).

One little note, the setting it exposes has a spelling mistake: "... is expernally modified" <<< p instead of t

<item row="5" column="0">
<widget class="QCheckBox" name="autoReloadOnChangeCheckBox">
<property name="text">
<string>Automatically reload when the database is expernally modified</string>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the line I meant, should be:
Automatically reload when the database is externally modified

@alerque
Copy link

alerque commented Aug 11, 2014

@jleclanche Replacing an existing file with a different object rather than opening for writing and just touching the changed bits is actually a common pattern and a proper way to handle files on *nix systems. Besides the already mentioned rsync and unison, git will also do this sometimes when switching branches as do many other sync systems like syncthing or btsync.

👍 for this or some variation thereof as a feature for KeePassX, this would be quite useful for those of us syncing password databases between systems.

@testcocoon
Copy link
Author

@alerque As mentioned before the implementation #12 is nicer, but they should use my own wrapper off QFileSystemWatcher to handle correctly deletions. I can do it, but I would like to know the opinion of the author of #12 before.

@alerque
Copy link

alerque commented Aug 11, 2014

@testcocoon Understood. If they are favorable you could submit the wrapper bit as a PR to their branch to be merged prior to their PR to the upstream project being accepted.

@Tklaversma
Copy link

Does anyone know the current status of this feature, also known as issue #73? Can't wait for it to be merged into KeePass..

nandhp added a commit to nandhp/keepassx that referenced this pull request Oct 12, 2015
nandhp added a commit to nandhp/keepassx that referenced this pull request Oct 12, 2015
nandhp added a commit to nandhp/keepassx that referenced this pull request Oct 12, 2015
@Tklaversma
Copy link

Why hasn't this been accepted yet? It's suggested since 2014 and one of the most important things???

@Vmadmax
Copy link

Vmadmax commented Mar 3, 2016

Any news to this pr?

@Tklaversma
Copy link

For as far as I know, still not in progress as you can see here unfortunately :(

@jytou
Copy link

jytou commented Apr 12, 2016

This would be absolutely necessary in my opinion. People are using cloud services to sync their different devices nowadays... or so I was told. :P

@phoerious
Copy link

phoerious commented Jan 14, 2017

This is implemented in the upcoming KeePassXC version 2.1.0, also see pull request #12 .

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet