Skip to content

Support remote database access using external tools - #7222

Merged
droidmonkey merged 18 commits into
keepassxreboot:developfrom
t-h-e:feature/remote-database-support
Jun 13, 2024
Merged

Support remote database access using external tools#7222
droidmonkey merged 18 commits into
keepassxreboot:developfrom
t-h-e:feature/remote-database-support

Conversation

@t-h-e

@t-h-e t-h-e commented Dec 12, 2021

Copy link
Copy Markdown
Contributor

As discussed here: #1775 (comment)

This change provides the ability to sync a database with a remote database with the help of scp. (Fixes #1775)
The RemoteSettingsDialog works similar to DatabaseSettingsDialog and can be extended in the future to store previous settings and load them again.
Although currently scp is supported, the implementation is extensible to include further programs in the future to download and upload .kdbx files to sync with them.

Feedback is very welcome as this would be my first contribution to KeePassXC

Testing strategy

Place a second database on a separate server that is accesible via ssh and have scp installed and in your PATH variable.
The second database can include keys and/or groups that the current database does not.
The new "Remote Sync ..." feature is available in the UI next to "Merge From Database...".

Regarding unit tests: Help is very welcome as I am uncertain how to mock the QProcess. I expect the rest to work similarly to TestGui::testMergeDatabase

Type of change

  • ✅ New feature (change that adds functionality)

@t-h-e
t-h-e force-pushed the feature/remote-database-support branch 2 times, most recently from 009f35b to 41f4a06 Compare December 13, 2021 21:22
@codecov-commenter

This comment was marked as outdated.

@t-h-e
t-h-e force-pushed the feature/remote-database-support branch from 5ab47e5 to b73e6e7 Compare January 3, 2022 21:48
@t-h-e

t-h-e commented Jan 6, 2022

Copy link
Copy Markdown
Contributor Author

@droidmonkey I think this PR is ready for review. As mentioned in the discussion of the issue, I'm not a C++ developer, so feedback is very welcome 😃

@droidmonkey

Copy link
Copy Markdown
Member

Excellent, will review when I get the chance

@hifi

hifi commented Feb 9, 2022

Copy link
Copy Markdown
Contributor

I'll add a note that scp is considered harmful and this should probably use sftp instead.

@t-h-e

t-h-e commented Feb 22, 2022

Copy link
Copy Markdown
Contributor Author

I'll add a note that scp is considered harmful and this should probably use sftp instead.

While OpenSSH recommends to use modern protocols like sftp or rsync, in the near future scp will be running with SFTP protocol. Since version 8.7, scp can already be used with SFTP protocol with the switch -s.
The benefit of scp over sftp is that a single file upload can be done with a single command, which makes it easier to use.

I have changes for using rsync and curl nearly finished, but I would prefer to at least get some feedback first to know if this feature is heading in the right direction. More protocols can easily be added.

@hifi

hifi commented Feb 23, 2022

Copy link
Copy Markdown
Contributor

AFAIK you can use a single command with sftp if you batch it through stdin. That's without any temporary files.

I have high personal interest in this PR so I'll be testing this as well and providing feedback once I get around to.

@hifi

hifi commented Feb 23, 2022

Copy link
Copy Markdown
Contributor

I took a quick look, it's promising but maybe it's not being utilized to its fullest potential with hardcoded commands.

I'm thinking sync over SSH is an advanced user feature so what if this would be simplified to "Sync Command" and instead provide a generic mechanism to run a command that can be templated with the database path and possibly other options? We'd only check that the return code of the command is a success and if not possibly show the code and stdout+stderr for debugging.

We could still provide presets for scp, sftp etc. for convenience. This allows the user to write script that takes the arguments from KeePassXC and does whatever with them.

@KFDCompiled

Copy link
Copy Markdown

I've contributed to the #1775 bounty and am excited to test this PR!

@t-h-e

t-h-e commented Mar 1, 2022

Copy link
Copy Markdown
Contributor Author

@hifi using templating and running arbitrary commands is a great idea. It is way more flexible. Running the sync command after unlocking as shown in your comment would definitely be a convenient feature. 👍
You seem to have way more keepassxc experience. I am not sure if I should even proceed with my PR then 😆

@droidmonkey droidmonkey added this to the v2.8.0 milestone Jun 19, 2022
@droidmonkey droidmonkey changed the title Feature/remote database support Support remote database access using external tools Jun 19, 2022
@t-h-e

t-h-e commented Sep 1, 2022

Copy link
Copy Markdown
Contributor Author

@droidmonkey @phoerious Any chance to get a review for this PR? It is nearly 10 months old now 😞

@phoerious

Copy link
Copy Markdown
Member

Yes, sorry for the delay. But first, could you please rebase it to the latest develop HEAD and fix the conflicts?

@t-h-e
t-h-e force-pushed the feature/remote-database-support branch from b73e6e7 to 4787f99 Compare September 1, 2022 08:59
@t-h-e

t-h-e commented Sep 1, 2022

Copy link
Copy Markdown
Contributor Author

Done 😃
Edit: I will check next week, why the build fails

@t-h-e
t-h-e force-pushed the feature/remote-database-support branch 3 times, most recently from 6b811dc to 2eacd0f Compare September 8, 2022 20:36
@t-h-e

t-h-e commented Sep 9, 2022

Copy link
Copy Markdown
Contributor Author

PR is rebased and conflicts are resolved. The build works mostly. The only issue seems to be that TestGui::testSaveBackup fails sometimes, but I cannot reproduce the behaviour locally.
Do you have any idea why this test might fail? It does not seem to fail for all build configurations.

@droidmonkey

Copy link
Copy Markdown
Member

I'll give this a review soon

@t-h-e
t-h-e force-pushed the feature/remote-database-support branch 3 times, most recently from 7172131 to 8b8edd5 Compare November 6, 2022 13:50
@droidmonkey

Copy link
Copy Markdown
Member

That is our beta...

@KaKi87

KaKi87 commented Aug 21, 2024

Copy link
Copy Markdown

Well, I was hoping for a build that contains everything stable + remote databases support in addition, so that other experimental things wouldn't potentially add bugs to it. :/

@segln

segln commented Aug 21, 2024

Copy link
Copy Markdown

Then how about just waiting for the next stable build?

@KaKi87

KaKi87 commented Aug 21, 2024

Copy link
Copy Markdown

This PR is almost 3 years old, the milestone is only 32% complete containing many other posts that are years old, the latest minor version increment was done 4 years ago, all signs indicating that this PR will not be released in a stable build anytime soon...

@droidmonkey

droidmonkey commented Aug 21, 2024

Copy link
Copy Markdown
Member

2.7.9, the stable version, is 95-98% the same as a snapshot build off develop branch. Just use a snapshot and don't think too hard about it. Trust me you'll be OK.

@KaKi87

KaKi87 commented Aug 21, 2024

Copy link
Copy Markdown

What's with the very worrying warning message then ? The threat of loosing passwords is a big deal.

@droidmonkey

Copy link
Copy Markdown
Member

We are toning that down, its just to keep people from using non-final builds in production.

@Maasea

Maasea commented Aug 23, 2024

Copy link
Copy Markdown

I tried the beta version and implemented data synchronization for webdav using the curl command, which is easy to use.

But the data synchronization is manual, is it possible to add a switch to automatically trigger remote synchronization for data changes, and to automatically pull remote files for updates every time the database is unlocked.

@KaKi87

KaKi87 commented Aug 23, 2024

Copy link
Copy Markdown

its just to keep people from using non-final builds in production

But that's what I would be doing 😅

@droidmonkey

droidmonkey commented Aug 23, 2024

Copy link
Copy Markdown
Member

@Maasea that can be built in, maybe on a periodic basis versus every save to avoid locking up the interface.

@KaKi87 if you are just using it yourself should be fine, don't deploy to your enterprise though. I am running the 2.7.10 beta build myself to ensure no bugs crept in

@alexp303

Copy link
Copy Markdown

Any idea when will the sync be included in the stable release?

@droidmonkey

droidmonkey commented Nov 12, 2024

Copy link
Copy Markdown
Member

This still needs a bit of trim work for UX, but you can use it now in our snapshot builds. They are rather stable, every change is well vetted. https://snapshot.keepassxc.org

@phoerious phoerious added pr: new feature Pull request adds a new feature and removed new feature labels Nov 22, 2024
@wschlich

Copy link
Copy Markdown

@droidmonkey The current solution using remote sync settings per database stored inside the database itself (I guess in the KDBX headers?) is an interesting idea, I successfully tried using it with WebDAV and 2 cURL commands.

But how should it work using AutoOpen for other WebDAV DBs as supported by both (!) KeePass and KeePass2Android?

Right now, I have a main KDBX file on a WebDAV server.
I can just open the HTTPS-URL using KeePass and KeePass2Android and specify I/O username + I/O password for WebDAV (these can be cached by both KeePass and KeePass2Android so I don't need to enter them every time when I open that main DB).

In that main KDBX file, there's an AutoOpen folder, containing entries for other KDBX databases on other WebDAV servers.
These entries have an HTTPS-URL as well as IocUserName + IocPassword String fields which magically allow KeePass to open these WebDAV DBs without needing any more information.

Something like this (plain easily handleable!) seems to be impossible with the current DB-internal remote sync solution of KeePassXC? :-(

@droidmonkey

Copy link
Copy Markdown
Member

That is not supported and likely won't be. For awareness, keepass and keepass2android use the .NET framework which has support for web dav natively. Qt does not have this support and we are not interested in adding a third party library to add it.

You could maybe achieve this in keepassxc by mounting the webdav folder in your OS and pointing your auto open to the mounted folder.

@wschlich

Copy link
Copy Markdown

@droidmonkey

That is not supported and likely won't be. For awareness, keepass and keepass2android use the .NET framework which has support for web dav natively. Qt does not have this support and we are not interested in adding a third party library to add it.

Too bad, as there's a library available: https://github.com/PikachuHy/QtWebDAV

You could maybe achieve this in keepassxc by mounting the webdav folder in your OS and pointing your auto open to the mounted folder.

Not portable across KeePass and KeePass2Android :-(

What about this idea:

As you already store DB remote sync settings inside the KDBX, what about adding support for the exact same kind of settings per AutoOpen entry as well, using String Fields, just like KeePass + KeePass2Android do with IocUserName + IocPassword, while ignoring the AutoOpen HTTPS-URL, but just using it as a {TEMP_DATABASE}?

To me, my master copy is the WebDAV remote anyway and not any local temporary/cached copy (as KeePassXC now seems to handle it).

@droidmonkey

droidmonkey commented Nov 24, 2024

Copy link
Copy Markdown
Member

You can have as many auto open entries as you want and use IfDevice. https://keepassxc.org/docs/KeePassXC_UserGuide#_automatic_database_opening

Extending this feature is likely not going to happen, it's niche to begin with and distraction from other important core features.

@wschlich

Copy link
Copy Markdown

@droidmonkey
I believe you're completely missing my point here. It's about automatically opening remote WebDAV DBs using AutoOpen entries.
Please, read my comments again :-)

@wschlich

Copy link
Copy Markdown

@droidmonkey
The current implementation of remote database support in KeePassXC kills my highly portable workflow with multiple WebDAV DBs (one master and several AutoOpen WebDAV DBs specified inside) across KeePass2Android and classic KeePass -- it's really sad as I'd love to switch to KeePassXC on my Linux box because of .NET woes with mono on Linux :-(

@droidmonkey

Copy link
Copy Markdown
Member

I'm not missing your point. I'm telling you we aren't adding more features to this aspect of the program.

@wschlich

Copy link
Copy Markdown

Very sad to read this. Thanks anyway :-)

@mazunki

mazunki commented Apr 25, 2025

Copy link
Copy Markdown

This still needs a bit of trim work for UX, but you can use it now in our snapshot builds. They are rather stable, every change is well vetted. https://snapshot.keepassxc.org/

Forgive my confusion, but does this mean it will be included in the 2.8.0 release? I suspect there's no specific ETA for that yet?

@droidmonkey

droidmonkey commented Apr 25, 2025

Copy link
Copy Markdown
Member

Yes and no eta right now.

@kschmid

kschmid commented Apr 26, 2025

Copy link
Copy Markdown

Just tried out the remote sync feature from the most recent snapshot. This looks cool. Together with curl, I was able to successfully set up WebDav transfer. I am only confused about two things.
a) The way I understand it, there is no way to have the local file at a fixed position? This confuses me. When I am traveling and have no remote access, it would be better to be able to access the most recent version at a local point.
b) it seems, the only time remote sync happens, is when explicitly called for. - While this is nice, it might be even less errorprone, if remote-sync would be connected to opening dbs and (auto-)saving them.

I hope, I got everything right. I tried to locate a corresponding user guide version, but it seems, there is not yet relevant doc for this.

@droidmonkey

droidmonkey commented Apr 26, 2025

Copy link
Copy Markdown
Member

Your local file is wherever your kdbx file is stored. Sync pulls down a temporary remote copy, performs a merge with the open copy, then saves the result locally.

Remote sync is explicit because it's not meant to be "remote access".

@kschmid

kschmid commented Apr 26, 2025

Copy link
Copy Markdown

Thanks a lot for the clarification with the merging. This escaped me. Just tried it out: very nice.
Then also the explicit sync is not as problematic. - Though would still prefer "automatic merging", but if this is the decision...
Took me time to understand that these settings are also encrypted, so passwords could be part of the commands (which would simplify a lot. Overall very nice feature; looking forward to general release! Thanks for the great work!

@droidmonkey

Copy link
Copy Markdown
Member

Thank you for the feedback, we should make these things clearer

@droidmonkey droidmonkey mentioned this pull request Apr 26, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: new feature Pull request adds a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remote database support [$276]