Option to select extensions compile-time. Fix #50 #123 by TheZ3ro · Pull Request #125 · keepassxreboot/keepassxc

TheZ3ro · 2016-12-04T19:13:50Z

Description

This PR add Options to select extensions at compile time to let user and distributors to select with component add to KeePassXC.

The extensions/components supported right now are:

Autotype
KeePassHTTP
Yubikey (still not working, waiting for Rebased 2FA code by Kyle Manna #119)

You can enable (ON) or disable (OFF) extensions with the following cmake flag:

-DWITH_XC_HTTP=ON for KeePassHTTP
-DWITH_XC_AUTOTYPE=ON for Autotype
-DWITH_XC_YUBIKEY=ON for Yubikey

At the end of the cmake command, enabled and disabled extensions will be listed:

-- Enabled features:
 * Autotype , Auto-type passwords in Input fields

-- Disabled features:
 * KeePassHTTP , KeePassHTTP support for ChromeIPass and PassIFox

Right now, by default no extension is enabled. (We can discuss about this)

Motivation and Context

This fixes #50 and #123

How Has This Been Tested?

Manually tested the enabled and disabled option for HTTP and Autotype.
NEED an update to the travis file to set all the extension to ON so it will test them
(or maybe building a matrix with different ON/OFF combination?)

Types of changes

❎ Bug fix (non-breaking change which fixes an issue)
✅ New feature (non-breaking change which adds functionality)
✅ Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

✅ I have read the CONTRIBUTING document. [REQUIRED]
✅ My code follows the code style of this project. [REQUIRED]
✅ All new and existing tests passed. [REQUIRED]
✅ My change requires a change to the documentation.
❎ I have updated the documentation accordingly.
❎ I have added tests to cover my changes.

TheZ3ro · 2016-12-04T20:02:56Z

Seems that cutting of the autotype cmake don't disable autotype at all.
Fixing.

droidmonkey · 2016-12-04T23:22:44Z

Excellent, I'll see if I can add the about dialog feature i mentioned.

As for the HTTP build, I think its smart to make it a static link library, perhaps we can make a fresh CMakeLists.txt in the http folder to encapsulate the build. I foresee a future where we have 'src/plugins/[http|autotype|yubikey]' which build either static or dynamic libraries.

TheZ3ro · 2016-12-05T00:01:48Z

It's already been build into a static library that it's later linked in the main executable

Implementing the AboutDialog should be easy with the WITH_XC_* ifdef flags

Anyway, the Autotype plugin works even if you cut off the src/autotype/Cmake.
I think it need a static library like the http as well

phoerious · 2016-12-05T01:54:21Z

tests/config-keepassx-tests.h.cmake


 #define KEEPASSX_TEST_DATA_DIR "${KEEPASSX_TEST_DATA_DIR}"

+#cmakedefine WITH_XC_HTTP


Are these defines used anywhere or is this just for when we have proper HTTP tests?

Yes they are not used rigth now, but I think it's usefull for future development

Alright, seems fair. Code's looking okay to me.

droidmonkey · 2016-12-08T03:42:44Z

I implemented the about dialog extensions view. It's not scalable with #ifdef's, there might be a better way to do it in the future when we define an actual plugin interface (maybe).

TheZ3ro · 2016-12-08T14:49:43Z

I think it's the best option we have right now

TheZ3ro · 2016-12-18T15:27:44Z

I think this can be merged @droidmonkey what do you think?

droidmonkey · 2016-12-19T22:58:48Z

I might be able to look at it this week, took some time off from the project recently.

droidmonkey · 2016-12-28T03:25:44Z

I made a bunch of changes to the autotype lib building. It did not work at all on Windows, a bunch of dual definitions and the EXPORT/IMPORT was backwards (this was an error from many moons ago). I tested on Fedora and everything seemed ok, this needs to be tested on OpenSuse and Apple.

TheZ3ro · 2016-12-30T14:05:39Z

Tested on macOS Sierra, compile fine and works fine but testgui give some error (maybe unrelated)

********* Start testing of TestGui *********
Config: Using QtTest library 5.7.0, Qt 5.7.0 (x86_64-little_endian-lp64 shared (dynamic) release build; by Clang 8.0.0 (clang-800.0.38) (Apple))
QWARN  : TestGui::initTestCase() FilePath::DataPath: can't find data dir
QWARN  : TestGui::initTestCase() Unable to load auto-type plugin:
Cannot load library /Applications/KeePassXC.app/Contents/PlugIns/libkeepassx-autotype-cocoa.so: (dlopen(/Applications/KeePassXC.app/Contents/PlugIns/libkeepassx-autotype-cocoa.so, 6): Library not loaded: @executable_path/../Frameworks/QtGui.framework/Versions/5/QtGui
  Referenced from: /Applications/KeePassXC.app/Contents/Frameworks/QtWidgets.framework/Versions/5/QtWidgets
  Reason: image not found)
PASS   : TestGui::initTestCase()
QWARN  : TestGui::testMergeDatabase() QSignalSpy: Unable to handle parameter 'mergedDb' of type 'Database*' of method 'databaseMerged', use qRegisterMetaType to register it.
PASS   : TestGui::testMergeDatabase()
FAIL!  : TestGui::testAutoreloadDatabase() Compared values are not the same
   Actual   (m_db->rootGroup()->findChildByName("General")->entries().size()): 0
   Expected (1)                                                              : 1
   Loc: [/Users/thezero/keepassxc/tests/gui/TestGui.cpp(166)]
PASS   : TestGui::testTabs()
PASS   : TestGui::testEditEntry()
PASS   : TestGui::testAddEntry()
PASS   : TestGui::testEntryEntropy()
PASS   : TestGui::testSearch()
PASS   : TestGui::testDeleteEntry()
PASS   : TestGui::testCloneEntry()
PASS   : TestGui::testDragAndDropEntry()
PASS   : TestGui::testDragAndDropGroup()
PASS   : TestGui::testSaveAs()
PASS   : TestGui::testSave()
PASS   : TestGui::testDatabaseSettings()
PASS   : TestGui::testKeePass1Import()
PASS   : TestGui::testDatabaseLocking()
PASS   : TestGui::cleanupTestCase()
Totals: 17 passed, 1 failed, 0 skipped, 0 blacklisted, 11150ms
********* Finished testing of TestGui *********

droidmonkey · 2016-12-30T16:51:11Z

The autoreload tests are susceptible to timing issues since they wait an arbitrary 1.5 seconds for the auto-detection of the file change to work. This can likely be enhanced in the future, but it is definitely unrelated to this PR.

TheZ3ro · 2016-12-30T20:52:14Z

Really it's the autoreload that fails

TheZ3ro · 2017-01-02T15:37:56Z

Ping @droidmonkey

droidmonkey · 2017-01-03T03:33:45Z

Thanks. This pull request has been integrated manually.

droidmonkey · 2017-01-03T03:34:23Z

Merged using multiple squash commits to preserve authorship.

dannysu · 2017-01-04T05:52:54Z

I think it'd also be good to update the Wiki to reflect the additional flags.
Or how about in the message that shows up, give instruction on how to do so?

E.g.

-- Disabled features:
 * KeePassHTTP , KeePassHTTP support for ChromeIPass and PassIFox (Enable by -DWITH_XC_HTTP=ON)

TheZ3ro · 2017-01-04T10:10:56Z

I like the idea of showing up in the Cmake message, but it will show up also when enabled.

I will update the wiki asap (now)
Edit: done wiki/Install-Instruction-from-Source#building-extension tell me if it's good

Option to select extension compile-time. Fix #50 #123

65992ba

TheZ3ro mentioned this pull request Dec 4, 2016

CMake notifications of features build status #123

Closed

phoerious reviewed Dec 5, 2016

View reviewed changes

phoerious approved these changes Dec 5, 2016

View reviewed changes

Added extensions section in about dialog; fixed url as well

fa891ed

droidmonkey self-requested a review December 8, 2016 03:40

droidmonkey added this to the v2.1.0 milestone Dec 8, 2016

TheZ3ro added 4 commits December 13, 2016 17:31

Added Autotype switch + updated Travis to always test all extension

7253cea

Merge branch 'develop' into feature/cmake-fix-#50-#123

58a0975

Add autotype library linking

fe3e55d

Fix travis tests

438265e

TheZ3ro and others added 2 commits December 24, 2016 17:19

Merge branch 'develop' into feature/cmake-fix-#50-#123

5fbe7b3

Corrected autotype lib building for Windows

8109b75

TheZ3ro mentioned this pull request Jan 2, 2017

Add Yubikey 2FA for unlocking databases #127

Merged

8 tasks

Merge branch 'develop' into feature/cmake-fix-#50-#123

3bebc48

droidmonkey closed this Jan 3, 2017

droidmonkey deleted the feature/cmake-fix-#50-#123 branch January 3, 2017 03:34

TheZ3ro mentioned this pull request May 10, 2024

Debian No-Feature KeePassXC Package #10725

Closed


		#define KEEPASSX_TEST_DATA_DIR "${KEEPASSX_TEST_DATA_DIR}"

		#cmakedefine WITH_XC_HTTP

Uh oh!

Conversation

TheZ3ro commented Dec 4, 2016

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

TheZ3ro commented Dec 4, 2016

Uh oh!

droidmonkey commented Dec 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheZ3ro commented Dec 5, 2016

Uh oh!

phoerious Dec 5, 2016

Choose a reason for hiding this comment

Uh oh!

TheZ3ro Dec 5, 2016

Choose a reason for hiding this comment

Uh oh!

phoerious Dec 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

droidmonkey commented Dec 8, 2016

Uh oh!

TheZ3ro commented Dec 8, 2016

Uh oh!

TheZ3ro commented Dec 18, 2016

Uh oh!

droidmonkey commented Dec 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

droidmonkey commented Dec 28, 2016

Uh oh!

TheZ3ro commented Dec 30, 2016

Uh oh!

droidmonkey commented Dec 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheZ3ro commented Dec 30, 2016

Uh oh!

TheZ3ro commented Jan 2, 2017

Uh oh!

droidmonkey commented Jan 3, 2017

Uh oh!

droidmonkey commented Jan 3, 2017

Uh oh!

dannysu commented Jan 4, 2017

Uh oh!

TheZ3ro commented Jan 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

droidmonkey commented Dec 4, 2016 •

edited

Loading

phoerious Dec 5, 2016 •

edited

Loading

droidmonkey commented Dec 19, 2016 •

edited

Loading

droidmonkey commented Dec 30, 2016 •

edited

Loading

TheZ3ro commented Jan 4, 2017 •

edited

Loading