Rebased 2FA code by Kyle Manna#119
Conversation
|
Wow, I was thinking to rebase this just today and this PR pops out, nice! Related to #26 Edit: I'm testing this and I think it should be good if when waiting for the Yubico challenge-response a MessageBox appear to tell the user to insert the Yubico and press it instead of failing the challenge-response. |
| clear(); | ||
|
|
||
| for (const Key* subKey : asConst(key.m_keys)) { | ||
| Q_FOREACH (const Key* subKey, key.m_keys) { |
There was a problem hiding this comment.
This should be a normal for and not a Q_FOREACH, the latter will be deprecated soon.
Is there a reason to roll back to Q_FOREACH?
| addKey(*subKey); | ||
| } | ||
|
|
||
| Q_FOREACH (const ChallengeResponseKey* subKey, key.m_challengeResponseKeys) { |
There was a problem hiding this comment.
Also this is the same as above
|
|
||
| CryptoHash cryptoHash(CryptoHash::Sha256); | ||
|
|
||
| Q_FOREACH (ChallengeResponseKey* key, m_challengeResponseKeys) { |
| * | ||
| * This operation could block if the YubiKey requires a touch to trigger. | ||
| * | ||
| * TODO: Signal to the UI that the system is waiting for challenge response |
There was a problem hiding this comment.
I think this todo must be implemented before merging.
| find_package(Qt5X11Extras 5.2) | ||
| if(PRINT_SUMMARY) | ||
| add_feature_info(libXi X11_Xi_FOUND "The X11 Xi Protocol library is required for auto-type") | ||
| add_feature_info(libXtst X11_XTest_FOUND "The X11 XTEST Protocol library is required for auto-type") |
There was a problem hiding this comment.
This is NOT a typo... it should be libXtst
|
The travis build needs to be updated to include downloading the yubikey libraries so that the tests run. Likewise, the readme needs to be updated (or wiki) for building with Yubikey. Also, if yubikey is not compiled in recommend hiding the "challenge response" entry in the database unlock widget instead of just disabling it. May cause confusion. |
|
Ping @johseg. |
|
I'm sorry for the delay, I'll be after Christmas that I'll find time for it. So feel free to clone away |
|
@TheZ3ro recommend just pushing a copy of his branch work into the keepassxc repo directly so we can all tweak it. |
* Use the C++11 range based loop as recommended from keepassxreboot#119 Signed-off-by: Kyle Manna <[email protected]>
* Use the C++11 range based loop as recommended from keepassxreboot#119 Signed-off-by: Kyle Manna <[email protected]>
4 participants
Description
This just just keepassx/keepassx#52 rebased against keepassxc. All glory belongs to Kyle Manna
Motivation and Context
Adds 2FA with yubikeys to keepassxc
#26
How Has This Been Tested?
I've been using his fork for a while. This here is pretty much untested, I use it since today ...
Screenshots (if appropriate):
Types of changes
Checklist:
keepassx/keepassx#52