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

WinHelloUnlock stops working when update is available #25

Closed
sambul13 opened this issue Aug 25, 2019 · 12 comments
Closed

WinHelloUnlock stops working when update is available #25

sambul13 opened this issue Aug 25, 2019 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@sambul13
Copy link

sambul13 commented Aug 25, 2019

Describe the bug
When KeePass shows a plugin & package Update Prompt at launch, and user closes it, WinHelloUnlock doesn't show Scan Prompt after that and fails to work

To Reproduce
Steps to reproduce the behavior:

  1. Open KeePass
  2. KeePass Prompt appears "Updates available" if there are any
  3. You can't close the prompt and scan your finger anymore until the update is installed, and KeePass restarted, if KeePass update notifications remain On

Expected behavior
WinHelloUnlock should show Scan Prompt and allow user to scan fingerprint and enter KeePass once KeePass Update Prompt is closed by the user.

@sambul13 sambul13 changed the title WinHelloUnlock stops working when new release is available WinHelloUnlock stops working when update is available Aug 25, 2019
@Angelelz
Copy link
Owner

Yes, I am aware of the issue, it was introduced when closing #12.
After attempting to open the database, the plugin checks if the file was actually opened, If it was not, then the master key might have changed (Hence, the prompt from the plugin). If KeePass is checking for updates and finds any, it doesn't open the database right away but shows the obsolete components, that's why it fails.
I am testing another way of closing #12 that doesn't cause this issue.

@Angelelz Angelelz self-assigned this Aug 25, 2019
@Angelelz Angelelz added the bug Something isn't working label Aug 25, 2019
@Angelelz
Copy link
Owner

It would be very helpful if you could test this
WinHelloUnlock.zip build from EscButtonBugFix branch with my attempt to fix this issue.
I am testing but even though I downgraded some plugins, I can't get KeePass to check for update when I'm trying to unlock a database.

@sambul13
Copy link
Author

sambul13 commented Aug 25, 2019

Just tested. Still shows KeePass warning prompt at pressing Esc "Error decrypting the data. The request was cancelled by the user". Pressing OK in that prompt shows "Enter Password" prompt.

It sounds like you submit the request to decrypt data BEFORE finger scan is done and processed, instead of AFTER that. Esc works well with KeePassWinHello plugin.

Mark KeePass - Options - Advanced - Check for updates at KeePass startup. Then downgrade your plugin version, and re-launch KeePass.

@Angelelz
Copy link
Owner

Just tested. Still shows KeePass warning prompt at pressing Esc "Error decrypting the data. The request was cancelled by the user". Pressing OK in that prompt shows "Enter Password" prompt.

That is actually expected behaviour! Esc key should be equivalent to clicking cancel button!

Mark KeePass - Options - Advanced - Check for updates at KeePass startup. Then downgrade your plugin version, and re-launch KeePass.

I did that but for some reason it doesn't work.

Anyway, I am asking about the issue in this thread. Is WinHelloUnlock working when it checks for updates?

Angelelz added a commit that referenced this issue Aug 26, 2019
Fix #24 and #25. Added automatic detection of MasterKey Change.
@sambul13
Copy link
Author

sambul13 commented Aug 26, 2019

Is WinHelloUnlock working when it checks for updates?

For me it doesn't, and for some reason you close bugs without testing. I don't mind though, its your plugin. What happens now:

  1. When KeePass shows updates available, WinHelloUnlock prompt appears. If you scan the finger, Update prompt comes to front. Once you Close it, KeePass doesn't open any file, and stays empty.

  2. When there are no updates (or KeePass just waits a few days without reminders after showing once an update prompt), WinHelloUnlock prompt appears. If you Esc it, KeePass Error Prompt appears. You said this is expected behavior. But I don't expect such prompt, and KeePassWinHello plugin at Esc doesn't cause such error prompt.

The rest is up to you. I'm just trying to help by reporting errors to make it more convenient to use. If its enough for you to say "its expected" - this is your plugin. But some folks would move back to competitor. You also need to allow at least 48h for users to respond to your questions before closing any bug. :)

@Angelelz
Copy link
Owner

Thanks for reporting! I closed because I could get Keepass to check for updates and the database was successfully opened, and don't worry, we can just reopen if needed. Are you using latest v1.3 release?

Also, let me understand correctly, do you push Esc key on the keyboard expecting to Unlock the database?

@sambul13
Copy link
Author

sambul13 commented Aug 26, 2019

Sorry, I was using previous build. With latest build, there is no update anymore unless you post a new test build. So I can't test the update behavior.

When Esc is pushed, I expect hidden behind KeePass Enter Password form to appear, not KeePass Error popup, it must proceed quietly with errors suppressed.

@Angelelz
Copy link
Owner

To test this issue, downgrade a different plugin and test with latest WinHelloUnlock.
As for the other, I understand now what you mean, you just want the plugin to not report that the credentials were not retrieved because the user canceled! I fixed the plugin showing the error twice. I could add a setting for it to not show at all. Please let's continue this discussion in that issue thread

@sambul13
Copy link
Author

sambul13 commented Aug 27, 2019

Hope you can finish the fix without further help. :)

@sambul13
Copy link
Author

sambul13 commented Aug 27, 2019

Ooops, your competitor just got a new version, and it doesn't show KeePass window until finger is scanned, which is more logical, and shouldn't lead to problems like this bug.

KeePass shown Update prompt again. WinHelloUnlock finger scan worked, but after closing KeePass Update prompt, the DB remained locked. So your fix didn't work.

If KeePassWinHello plugin devs borrowed your method and code of storing password, they should reference you in the new release as per Github best practices.

@Angelelz
Copy link
Owner

I don't have a competitor, I wrote this plugin for me based on KeePassWinHello to fit my needs for permanent key storage and I decided to share it for free. You are asking for a removal of a warning I feel it should stay, informing the user that the credential retrieval was canceled by himself, you just need to push Enter, Esc or click the button to get the regular Key Prompt. Remember, according to the license, this software is offered as is and you are free to use whichever plugin fit your needs better. You can also just fork the repository and make the changes you want, as I did myself with this plugin.

@Angelelz
Copy link
Owner

As for this issue, you are right, it is not fixed! thanks for pointing that out.

@Angelelz Angelelz reopened this Aug 28, 2019
Angelelz added a commit that referenced this issue Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants