LN-Zap / zap-android

Zap Wallet - Native android lightning wallet for node operators focused on user experience and ease of use ⚡️
MIT License
165 stars 49 forks source link

Do not require PIN #31

Closed Kixunil closed 4 years ago

Kixunil commented 5 years ago

Allow the user to disable PIN. While this seems like decreasing security, additional PIN is just annoyance in case the user has the phone locked with strong passphrase already. If the attacker can somehow interact with the application using malware, it's highly likely that the PIN won't help.

Also in practice (and we have a lot of practical experience with users in Paralelna Polis) the users are very likely to lose access to funds by locking themselves out by forgetting PIN/password.

Ideally detect that whole phone has passphrase set up and automatically skip PIN. If that is impossible, notify the user about the importance of having phone locked.

michaelWuensch commented 5 years ago

Thanks for your input!

People will not lose access to their funds if they forget the PIN. They can still recover their funds using the Mnemonic Seed.

Even if we can detect if the phone is secured by a passphrase or something similar, we can not detect if that one is safe or if for example "0000" is used.

I think though we could make it like this: On the screen where you have to create the PIN there is an additional button "SKIP". If pressed, there will be a notification notifying the user about the importance of having phone locked. Later in the settings a PIN can be added or removed.

Would that work for you?

Kixunil commented 5 years ago

Yes, that sounds reasonable.

zechendorf commented 5 years ago

Perfect solution 👌

Would love that feature too!

michaelWuensch commented 5 years ago

An update just got released that supports unlocking with biometrics (Fingerprint, Face recognition or Iris Scan). This should reduce the friction a lot and add convenience to those who want to use it.

We decided against the possibility to completely remove protection, though.

Kixunil commented 5 years ago

This sounded very good and I proceeded to try this out. I lost excitement when I realized it's impossible to enable fingerprint authentication for Zap without enabling fingerprint authentication for the lock screen too. I definitely don't want to have fingerprint enabled for lock screen, but having another PIN for Zap seems unnecessary.

Would you reconsider your decision with this information in mind? I suspect it might be impossible for Zap to add its own fingerprint, but maybe there's a way to do it?

michaelWuensch commented 5 years ago

Hi, thanks for your feedback. I will keep the issue open and see what we can do.

JimmyMow commented 5 years ago

This has also been heavily requested for iOS. We should maybe prioritize this one

michaelWuensch commented 5 years ago

Ok. Basically there would be 3 ways to do this.

  1. We still require a PIN setup during onboarding. Later the PIN can be turned off in the settings. This would be "opt out" and basic sentiment is safe your wallet!
  2. We suggest to protect it with a PIN during setup. But the step is skippable.
  3. We do not ask to create a PIN at all during onboarding. A PIN can be added later in the settings.

Any thoughts on what to prefer?

Kixunil commented 5 years ago

From a year of experience doing "cryptoapostle" (a mentor for setting up wallets basically) in Paralelna Polis:

  1. People forget it immediately, turning it off later doesn't solve the problem
  2. If someone doesn't recommend skipping it, people don't skip and then forget it
  3. Sounds good, but might be unsecure if the user didn't set lock screen

My suggestion: tell users that locking whole phone is strongly recommended. If the presence of protection can be detected, the message could be skipped (but again the 0000 problem, one-time warning shouldn't be too annoying anyway). Optionally the user can set PIN in menu.

michaelWuensch commented 5 years ago

@Kixunil @JimmyMow I have finally made a PR for this one. PIN is now completely removed from setup and can optionally be added in the settings.