buttercup / buttercup-mobile

:iphone: React-Native mobile application for Buttercup
https://buttercup.pw
GNU General Public License v3.0
399 stars 71 forks source link

Touch ID prompts for keychain access (constant) #137

Closed perry-mitchell closed 5 years ago

perry-mitchell commented 5 years ago

When unlocking with touch unlock, I am prompted twice. Once for the typical unlock of the vault, and another when it sync's the credentials to the keychain for autofilling. Any updates to the vault also result in an auth prompt. It seems as though by using the secure storage that the touch/face auth prompt is always required.

perry-mitchell commented 5 years ago

@se1exin Hey mate, any idea with this one? When I unlock on my device using touch, it immediately asks a second time for my finger. Both touching and cancelling on the second time does nothing and just returns me to the open archive.

perry-mitchell commented 5 years ago

Hmm it seems that my physical phone requires my touch ID every time I use the keychain. Could this be it? It seems to be the same as this issue.

perry-mitchell commented 5 years ago

It may actually be the entitlements, as mentioned in the react-native-keychain project.

perry-mitchell commented 5 years ago

Created an issue on the storage project: oyyq99999/react-native-secure-storage#9

se1exin commented 5 years ago

@perry-mitchell Strange! I didn't come across this in my testing. However I wasn't able to test in release on a device as I don't have a personal fully-fledged Apple Developer account handy to create dummy App IDs (you can't use a personal team with iOS AutoFill capabilities).

It could be due to the react-native-secure-storage config flags in

https://github.com/buttercup/buttercup-mobile/blob/master/source/compat/SecureStorageInterface.js#L10

or

https://github.com/buttercup/buttercup-mobile/blob/master/source/shared/touchUnlock.js#L15

or

https://github.com/buttercup/buttercup-mobile/blob/master/ios/AutoFillHelpers.m#L96

..however it was my understanding that ACCESSIBLE.WHEN_UNLOCKED prevented the need for re-authentication.

Edit: I've just installed 1.8.0 from the App Store and can confirm the double authentication, but only when going back to the Archives list after unlocking (not getting double auth on unlock like you described). iPhone X - using Touch ID.

I'll submit a PR shortly with a patch for the react-native-secure-storage flags in question.

se1exin commented 5 years ago

@perry-mitchell PR #138 is ready with a fix for the issue.

Got myself a personal fully-fledged Apple Dev account (been wanting to buy one anyone) and have tested on a real device (using dummied bundle IDs etc).

Edit: Sorry for all the empty lines in the PR diff. We can thank Xcode for that..

perry-mitchell commented 5 years ago

PR #138 is ready with a fix for the issue.

It fixed it, thank you! 🙏

Got myself a personal fully-fledged Apple Dev account (been wanting to buy one anyone) and have tested on a real device (using dummied bundle IDs etc).

Ah that's excellent! I'm sorry if you felt at all obliged due to this feature.. It's been a lot of fun testing it (works seamlessly). We could even add you to the testflight rollout for future features if you're interested.

Sorry for all the empty lines in the PR diff. We can thank Xcode for that..

Haha yeah I'm no stranger to that.. Doesn't surprise me 😅

I'll release the PR soon.. I tested and it works!

se1exin commented 5 years ago

It fixed it, thank you!

Sweet! Sorry it slipped through in the first place!

I'm sorry if you felt at all obliged due to this feature

Not at all! It was on my list anyway, was good motivation to stop being stingey with Apple taxes and get an account already.

We could even add you to the testflight rollout for future features if you're interested.

I actually PM'd you on Spectrum a day or two ago about this 😉, wasn't/not sure what the best channel is for off-issue discussion though

perry-mitchell commented 5 years ago

I actually PM'd you on Spectrum a day or two ago about this 😉, wasn't/not sure what the best channel is for off-issue discussion though

Ah, ok! I had a quick glance at Spectrum earlier and probably missed it sorry. Thanks!

se1exin commented 5 years ago

@perry-mitchell just checking if this one needs more work or if we can call it fixed?

perry-mitchell commented 5 years ago

@se1exin Yeah this is done.. Thanks for following up 😊