Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TouchID not working (in 2.4.0 Beta 2) #2720

Closed
bhavers opened this issue Feb 20, 2019 · 13 comments
Closed

TouchID not working (in 2.4.0 Beta 2) #2720

bhavers opened this issue Feb 20, 2019 · 13 comments

Comments

@bhavers
Copy link

bhavers commented Feb 20, 2019

Hi, i tried 2.4 Beta 2 on my MBP 2017 (Mojave 10.14.3) but can't get TouchID to work.
This bug report belongs to feature request for TouchID.

Problem:
When i open KeepassXC i do see the TouchID option (see screenshot below).
I enter my password and open my database file. Than, when the session expires, i can't reopen it with TouchID. I have to provide the password again.
If i just check TouchID and select OK it is unable to open the database.

Expected Behavior

[I expected that i had to login once with my password and that, when the regular inactivity timer would expire, i could login with touchid (within the time period specified in Security settings as "Forget TouchID after inactivity of x min").

Current Behavior

When the regular inactivity timeout triggers, i see the login screen. And i am not able to use TouchID to log in.

Steps to Reproduce

  1. Login with password (also check TouchID at login screen)
  2. Wait for the inactivity timer to bring Keepass back to the login screen
  3. Try to login with TouchID --> nothing happens (would expect a screen/instruction to tell me to put my finger on the touchid sensor).

Debug Info

KeePassXC - Version 2.4.0-beta2
Build Type: PreRelease
Revision: 9bc20f0

Libraries:

  • Qt 5.12.0
  • libgcrypt 1.8.4

Operating system: macOS 10.14
CPU architecture: x86_64
Kernel: darwin 18.2.0

Enabled extensions:

  • Auto-Type
  • Browser Integration
  • SSH Agent
  • KeeShare (only unsigned sharing)
  • YubiKey
  • TouchID

See screenshots in previous bug report for beta 1, which is identical.

@droidmonkey
Copy link
Member

@weslly @louib can you confirm it is NOT working? I cannot test this.

@k6nmx
Copy link
Contributor

k6nmx commented Feb 24, 2019

I can confirm it is not working with the current beta. Here's a quick fix for DatabaseOpenWidget.cpp:databaseKey()#L252 (here)

#ifdef WITH_XC_TOUCHID
    // check if TouchID is available and enabled for unlocking the database and no password was entered
    if (m_ui->checkTouchID->isChecked() && TouchID::getInstance().isAvailable() && m_ui->editPassword->text().isEmpty()) {
        // clear empty password from composite key
        masterKey->clear();

        // try to get, decrypt and use PasswordKey
        QSharedPointer<QByteArray> passwordKey = TouchID::getInstance().getKey(m_filename);

Background: When I originally implemented the TouchID support, KeepassXC reset the password checkbox to 'unchecked' when nothing was entered. I used that to determine that no password was entered and then checked if the password was stored with TouchID/Keychain. Apparently this has changed now, though (password checkbox checked even if nothing is entered). Now, I simply check if the password field was empty.

Please tell me if I should open a pull request for this or if you just want to do it yourself :)

@droidmonkey
Copy link
Member

I will incorporate in my existing PR, thank you!!

@tijme
Copy link

tijme commented Feb 26, 2019

I can confirm it is not working with the current beta. Here's a quick fix for DatabaseOpenWidget.cpp:databaseKey()#L252 (here)

#ifdef WITH_XC_TOUCHID
    // check if TouchID is available and enabled for unlocking the database and no password was entered
    if (m_ui->checkTouchID->isChecked() && TouchID::getInstance().isAvailable() && m_ui->editPassword->text().isEmpty()) {
        // clear empty password from composite key
        masterKey->clear();

        // try to get, decrypt and use PasswordKey
        QSharedPointer<QByteArray> passwordKey = TouchID::getInstance().getKey(m_filename);

Background: When I originally implemented the TouchID support, KeepassXC reset the password checkbox to 'unchecked' when nothing was entered. I used that to determine that no password was entered and then checked if the password was stored with TouchID/Keychain. Apparently this has changed now, though (password checkbox checked even if nothing is entered). Now, I simply check if the password field was empty.

Please tell me if I should open a pull request for this or if you just want to do it yourself :)

Could you elaborate on this a little bit more? If I understand you correctly, one needs to press unlock without entering a password, which would then trigger the TouchID popup. Is that correct?

I somehow thought KeepassXC would automatically show the TouchID popup if TouchID was enabled.

@k6nmx
Copy link
Contributor

k6nmx commented Feb 26, 2019

Hi @tijme, yes you are right in your assumption, you just leave the password field empty when trying to unlock (press unlock or press enter) and then the TouchID popup comes up (i.e. it currently needs a trigger). Also see original pull request.

How would you suggest implementing your proposal of "automatically showing the TouchID popup", to be more precise on which kind of trigger should we ask for the user's fingerprint (do you know how other password managers do this)?

@droidmonkey
Copy link
Member

droidmonkey commented Feb 26, 2019

You could simply display a persistent notice when quick unlock is available to touch to unlock. The unlock form should default to quick unlock with a "cancel" that resets everything.

@vraravam
Copy link
Contributor

I am also facing the same issue. Tried the same steps - but, the touchID support is not working. im also on the same 2.4.0 beta2.

@droidmonkey
Copy link
Member

The fix came after beta 2. It'll be in final

@vraravam
Copy link
Contributor

Maybe i'm missing something - please help!
I have just installed the final 2.4.0 on Mac with touch id - but, i'm still unable to get it to work.

@k6nmx
Copy link
Contributor

k6nmx commented Mar 26, 2019

Hmh I just installed it as well and it works fine for me. What are the steps you took to try it out?

Check out this comment and the original pull request (#1851) for instructions how to currently use it. In #2865 you find proposed next steps for making TouchID for KeePassXC more intuitive to use.

@vraravam
Copy link
Contributor

tx @mxk6n . i'll check it out in a few hours.

@bhavers
Copy link
Author

bhavers commented Mar 26, 2019

I can confirm that it works. Thanks for the great work!

@gsluthra
Copy link

Just writing this in case someone else also struggles with triggering touchid like I did.

I am using KeepassXC 2.4.3, and realised that to trigger TouchID on mac, I needed to leave the password field blank, and press the enter key. Then I see the "TouchID" dialog and am able to unlock with my fingerprint. Of course, this happens only on "lock" of an open database. First time login into a database still requires a password (which is good).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants