Add DBus support #283

didier13150 · 2017-02-10T15:25:13Z

Description

keepassxc can be basically controlled with others applications: open/close databases or exit.

Motivation and Context

React to desktop event automatically:

open database when log in or unlock screen
close all databases when log out or lock screen
exit application

How Has This Been Tested?

Tested on Debug and RelWithDebInfo build type

No DBus interface when compilation option is not set
DBus interface is present when compilation option is set, and then run following commands on a shell
qdbus org.keepassxc.MainWindow /keepassxc org.keepassxc.MainWindow.openDatabase mydb mypasswd mykeyfile
qdbus org.keepassxc.MainWindow /keepassxc org.keepassxc.MainWindow.closeAllDatabases
qdbus org.keepassxc.MainWindow /keepassxc org.keepassxc.MainWindow.appExit

Types of changes

✅ New feature (non-breaking change which adds functionality)

Checklist:

✅ I have read the CONTRIBUTING document. [REQUIRED]
✅ My code follows the code style of this project. [REQUIRED]
✅ All existing tests passed. [REQUIRED]
✅ My change requires a change to the documentation and I add a specific readme

phoerious · 2017-02-10T17:02:05Z

Thanks for your pull request. I think this is mostly implemented by #135, though, which also adds automatic lock support on Windows and OS X, not only Linux.
Maybe you could merge these two PRs and take the best parts from both?

phoerious · 2017-02-25T00:35:08Z

Any further comments on this?

didier13150 · 2017-02-27T15:51:44Z

After a lot of check , #135 seems to subscribe and react to a d-bus event (passive mode). My PR provide another dbus interface (active mode). Both are not mutually exclusive and can be merged together. So I can modify my PR to use #135 lock feature, but it's very hard to do it until #135 is merged or not.

phoerious · 2017-02-27T17:49:34Z

Okay, we'll merge #135 first then and then proceed with your PR. But #135 still needs some work.

TheZ3ro · 2017-05-17T19:11:40Z

Postponing to 2.3

droidmonkey · 2017-05-18T01:55:26Z

I recommend closing this PR and performing a smart cherry pick of the differing capability than what was introduced with #545

TheZ3ro · 2017-05-18T08:29:44Z

@droidmonkey note that #545 is a dbus client (that listens for org.freedesktop.* signals) this instead is a server interface (that expose methods for interacting with KeePassXC via dbus)

Keisial · 2017-05-19T01:25:04Z

I don't think these will be a problem, but if others are added (eg. for reasing an entry), there should be a preferences option to not allow it.

droidmonkey · 2017-05-19T03:00:37Z

Hmmmm. Could this pose a security risk?

didier13150 · 2017-05-19T06:36:37Z

Clearly yes, but outside the scope of keepassxc because you must provide a password on cli to open a database. The risk is that the password of the database can be found on bash_history. This problem is the same than all other cli tools and can be secured by using an askpass program like ksshaskpass or just not using this feature.

mauron85 · 2017-06-22T07:48:06Z

This can serve as base for another projects. Like PR #608 which has potential to replace KeePassHTTP with 'Native Messaging'. In second variant it's using proxy that sits between browser and KeePassXC. Communication is done via UDP packets, but I can imagine to replace UDP for DBUS (on Linux), which would be awesome.

cornfeedhobo · 2017-10-02T20:58:53Z

Looks like this PR is getting lost with time, so I just want to pile on and say I would LOVE to have integration with org.freedesktop.* signals and hook into the Secret Service API, triggering desktop and application actions.

phoerious · 2018-01-14T00:09:04Z

Is this PR getting some love any time soon? If not, I'll close it.

TheZ3ro · 2018-01-14T00:58:07Z

I will look into it ;)

TheZ3ro · 2018-01-15T23:53:45Z

I've rebased this, added a method for locking all DBs (fix #687) and wrote the documentation on the wiki: Using-DBus-with-KeePassXC

Ready for review and merge

TheZ3ro · 2018-01-23T00:10:54Z

src/gui/MainWindow.cpp

@@ -168,6 +175,17 @@ MainWindow::MainWindow()
    , m_appExiting(false)
 {
    m_ui->setupUi(this);
+
+    #ifdef WITH_XC_DBUS
+    #if defined(Q_OS_UNIX)


This probably will trigger some problem on Q_OS_MACOS.
@weslly can you test this PR on a mac? Thanks

@weslly Ok thanks, I will fix this.

@TheZ3ro Appending && !defined(Q_OS_MAC) on this line seems to solve the compilation issue, but that would mean the #undef macro you used earlier had no effect

@TheZ3ro maybe adding this on src/CMakeLists.txt would be better:

if(APPLE OR MINGW) set(WITH_XC_DBUS OFF) endif()

@weslly pushed a different fix. Now should always work

@TheZ3ro works fine now :)

TheZ3ro · 2018-01-23T20:08:32Z

@weslly when you are free can you test this again? Thanks :)

weslly · 2018-01-23T20:54:43Z

@TheZ3ro got this now:

TheZ3ro · 2018-01-24T13:22:57Z

This is now ready for review @keepassxreboot/core-developers

phoerious · 2018-01-24T16:58:54Z

CMakeLists.txt


 if(WITH_XC_HTTP)
  message(WARNING "KeePassHTTP support has been deprecated and will be removed in a future version. Please use WITH_XC_BROWSER instead!\n"
          "For enabling / disabling network access code, WITH_XC_HTTP has been replaced by WITH_XC_NETWORKING.")
  set(WITH_XC_NETWORKING ON CACHE BOOL "Include networking code (e.g. for downlading website icons)." FORCE)
 endif()

+if((NOT UNIX) OR APPLE AND WITH_XC_DBUS)


Remove redundant parentheses, use them around the whole OR expression instead.

phoerious · 2018-01-24T16:59:32Z

src/CMakeLists.txt

@@ -193,7 +193,7 @@ add_feature_info(KeePassXC-Browser WITH_XC_BROWSER "Browser integration with Kee
 add_feature_info(KeePassHTTP WITH_XC_HTTP "Browser integration compatible with ChromeIPass and PassIFox (deprecated, implies Networking)")
 add_feature_info(SSHAgent WITH_XC_SSHAGENT "SSH agent integration compatible with KeeAgent")
 add_feature_info(YubiKey WITH_XC_YUBIKEY "YubiKey HMAC-SHA1 challenge-response")
-
+add_feature_info(DBus WITH_XC_DBUS "Take keepassxc control by DBus")


"Control KeePassXC via DBus"

But TBH, I don't think we need this flag. No matter what's selected here, DBus will always be compiled in on Linux, because we need it for detecting screen locking and for the stupid Unity menu bug workaround.

phoerious · 2018-01-24T17:00:06Z

src/CMakeLists.txt

+if(WITH_XC_DBUS)
+    set(keepassx_SOURCES
+        ${keepassx_SOURCES}
+        gui/MainWindowAdaptor.cpp


This can be written on a single line.

phoerious · 2018-01-24T17:03:06Z

src/gui/MainWindow.cpp

+    new MainWindowAdaptor(this);
+    QDBusConnection dbus = QDBusConnection::sessionBus();
+    dbus.registerObject("/keepassxc", this);
+    dbus.registerService("org.keepassxc.MainWindow");


This should be org.keepassxc.KeePassXC.MainWindow.

Seems redundant to me using the name 2 times

That's how the standard works.

phoerious · 2018-01-24T17:03:32Z

src/gui/MainWindow.h

@@ -37,6 +37,10 @@ class InactivityTimer;
 class MainWindow : public QMainWindow
 {
    Q_OBJECT
+
+#if defined(WITH_XC_DBUS) && !defined(QT_NO_DBUS)
+    Q_CLASSINFO("D-Bus Interface", "org.keepassxc.MainWindow")


phoerious · 2018-01-24T17:03:55Z

src/gui/MainWindowAdaptor.cpp

+MainWindowAdaptor::MainWindowAdaptor(QObject *parent)
+    : QDBusAbstractAdaptor(parent)
+{
+    // constructor


Remove comment.

phoerious · 2018-01-24T17:04:06Z

src/gui/MainWindowAdaptor.cpp

+
+MainWindowAdaptor::~MainWindowAdaptor()
+{
+    // destructor


phoerious · 2018-01-24T17:04:29Z

src/gui/MainWindowAdaptor.cpp

+
+void MainWindowAdaptor::appExit()
+{
+    // handle method call org.keepassxc.MainWindow.appExit


All these comments should be doc comments above the method signature.

This file is auto-generated, the next time it will be generated it will have same comment as now

I know, but it's so short, I don't even know if it's worth maintaining an XML file for it.

phoerious · 2018-01-24T17:05:40Z

src/gui/MainWindowAdaptor.h

+    void openDatabase(const QString &fileName);
+    void openDatabase(const QString &fileName, const QString &pw);
+    void openDatabase(const QString &fileName, const QString &pw, const QString &keyFile);
+Q_SIGNALS: // SIGNALS


Replace Q_SLOTS and Q_SIGNALS with keywords.

phoerious · 2018-01-24T17:05:54Z

src/gui/MainWindowAdaptor.h

@@ -0,0 +1,67 @@
+/*
+ * This file was generated by qdbusxml2cpp version 0.8


File should have the usual header, maybe with additional notices.

Sincerely, I would feel silly for adding a copyright notice to an autogenerated file

Shouldn't this file be generated by the build system then (add_custom_target or some shit)?

If we can execute external programs in cmake I would say yes.

qdbusxml2cpp comes in qtbase5-dev-tools package on debian, so it's another dependency to add on linux.

Probably this file won't be edited for a long time, we should leave this here and don't bother too much, we just need to remember that's autogenerated and the header will come in handy

phoerious · 2018-01-25T20:05:00Z

src/gui/org.keepassxc.KeePassXC.MainWindow.xml

@@ -0,0 +1,23 @@
+<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN" "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
+<node>
+  <interface name="org.keepassxc.MainWindow">


This still needs to be changed to org.keepassxc.KeePassXC.MainWindow.

phoerious · 2018-01-25T20:05:35Z

src/config-keepassx.h.cmake

@@ -18,6 +18,7 @@
 #cmakedefine WITH_XC_HTTP
 #cmakedefine WITH_XC_YUBIKEY
 #cmakedefine WITH_XC_SSHAGENT
+#cmakedefine WITH_XC_DBUS


Option does not exist anymore.

Fixed and rebased

…when log in, close all databases when log out)

- Add support for KDBX 4.0, Argon2 and ChaCha20 [#148, #1179, #1230, #1494] - Add SSH Agent feature [#1098, #1450, #1463] - Add preview panel with details of the selected entry [#879, #1338] - Add more and configurable columns to entry table and allow copying of values by double click [#1305] - Add KeePassXC-Browser API as a replacement for KeePassHTTP [#608] - Deprecate KeePassHTTP [#1392] - Add support for Steam one-time passwords [#1206] - Add support for multiple Auto-Type sequences for a single entry [#1390] - Adjust YubiKey HMAC-SHA1 challenge-response key generation for KDBX 4.0 [#1060] - Replace qHttp with cURL for website icon downloads [#1460] - Remove lock file [#1231] - Add option to create backup file before saving [#1385] - Ask to save a generated password before closing the entry password generator [#1499] - Resolve placeholders recursively [#1078] - Add Auto-Type button to the toolbar [#1056] - Improve window focus handling for Auto-Type dialogs [#1204, #1490] - Auto-Type dialog and password generator can now be exited with ESC [#1252, #1412] - Add optional dark tray icon [#1154] - Add new "Unsafe saving" option to work around saving problems with file sync services [#1385] - Add IBus support to AppImage and additional image formats to Windows builds [#1534, #1537] - Add diceware password generator to CLI [#1406] - Add --key-file option to CLI [#816, #824] - Add DBus interface for opening and closing KeePassXC databases [#283] - Add KDBX compression options to database settings [#1419] - Discourage use of old fixed-length key files in favor of arbitrary files [#1326, #1327] - Correct reference resolution in entry fields [#1486] - Fix window state and recent databases not being remembered on exit [#1453] - Correct history item generation when configuring TOTP for an entry [#1446] - Correct multiple TOTP bugs [#1414] - Automatic saving after every change is now a default [#279] - Allow creation of new entries during search [#1398] - Correct menu issues on macOS [#1335] - Allow compilation on OpenBSD [#1328] - Improve entry attachments view [#1139, #1298] - Fix auto lock for Gnome and Xfce [#910, #1249] - Don't remember key files in file dialogs when the setting is disabled [#1188] - Improve database merging and conflict resolution [#807, #1165] - Fix macOS pasteboard issues [#1202] - Improve startup times on some platforms [#1205] - Hide the notes field by default [#1124] - Toggle main window by clicking tray icon with the middle mouse button [#992] - Fix custom icons not copied over when databases are merged [#1008] - Allow use of DEL key to delete entries [#914] - Correct intermittent crash due to stale history items [#1527] - Sanitize newline characters in title, username and URL fields [#1502] - Reopen previously opened databases in correct order [#774] - Use system's zxcvbn library if available [#701] - Implement various i18n improvements [#690, #875, #1436]

droidmonkey added platform: Linux new feature labels Apr 16, 2017

droidmonkey added this to the v2.2.0 milestone Apr 16, 2017

droidmonkey added the needs work label May 7, 2017

TheZ3ro modified the milestones: v2.3.0, v2.2.0 May 17, 2017

droidmonkey added the in progress label Oct 11, 2017

droidmonkey removed the in progress label Oct 19, 2017

TheZ3ro self-assigned this Jan 15, 2018

TheZ3ro force-pushed the feature/dbus branch from e357af7 to ed82f03 Compare January 15, 2018 22:14

TheZ3ro removed the needs work label Jan 15, 2018

TheZ3ro requested a review from a team January 16, 2018 21:13

TheZ3ro force-pushed the feature/dbus branch from 3438268 to 8084ea0 Compare January 23, 2018 00:05

TheZ3ro reviewed Jan 23, 2018

View reviewed changes

TheZ3ro force-pushed the feature/dbus branch 2 times, most recently from fe66a11 to 1af06af Compare January 24, 2018 13:13

phoerious requested changes Jan 24, 2018

View reviewed changes

TheZ3ro force-pushed the feature/dbus branch 2 times, most recently from 7c0673d to 0edd9aa Compare January 24, 2018 22:34

phoerious reviewed Jan 25, 2018

View reviewed changes

didier13150 and others added 2 commits January 25, 2018 23:48

Add DBus support: control keepassxc on desktop events (load database …

e560650

…when log in, close all databases when log out)

add function for locking DBs from DBus, fix keepassxreboot#687

98a89e4

TheZ3ro force-pushed the feature/dbus branch from 0edd9aa to 98a89e4 Compare January 25, 2018 22:49

droidmonkey approved these changes Jan 27, 2018

View reviewed changes

phoerious approved these changes Jan 28, 2018

View reviewed changes

phoerious merged commit 2849e19 into keepassxreboot:develop Jan 28, 2018

TheZ3ro mentioned this pull request Feb 28, 2018

Fix build on NetBSD. #1557

Merged

yan12125 mentioned this pull request Sep 4, 2020

DBus interface name does not match XML definition file #5355

Closed

albinou mentioned this pull request Oct 31, 2020

keepassxc: issues with browser extension and tray icon (dbus) netblue30/firejail#3713

Closed

5 tasks

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

		@@ -0,0 +1,67 @@
		/*
		* This file was generated by qdbusxml2cpp version 0.8

Add DBus support #283

Add DBus support #283

Conversation

didier13150 commented Feb 10, 2017

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

phoerious commented Feb 10, 2017 • edited Loading

phoerious commented Feb 25, 2017

didier13150 commented Feb 27, 2017

phoerious commented Feb 27, 2017

TheZ3ro commented May 17, 2017 • edited Loading

droidmonkey commented May 18, 2017

TheZ3ro commented May 18, 2017

Keisial commented May 19, 2017

droidmonkey commented May 19, 2017

didier13150 commented May 19, 2017

mauron85 commented Jun 22, 2017 • edited Loading

cornfeedhobo commented Oct 2, 2017

phoerious commented Jan 14, 2018

TheZ3ro commented Jan 14, 2018

TheZ3ro commented Jan 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheZ3ro commented Jan 23, 2018

weslly commented Jan 23, 2018

TheZ3ro commented Jan 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheZ3ro Jan 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phoerious Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phoerious commented Feb 10, 2017 •

edited

Loading

TheZ3ro commented May 17, 2017 •

edited

Loading

mauron85 commented Jun 22, 2017 •

edited

Loading

TheZ3ro Jan 24, 2018 •

edited

Loading

phoerious Jan 25, 2018 •

edited

Loading