ACINQ / phoenix

Phoenix is a self-custodial Bitcoin wallet using Lightning to send/receive payments.
https://phoenix.acinq.co
Apache License 2.0
616 stars 93 forks source link

(ios) Custom PIN as an option for “app access” #560

Closed robbiehanson closed 1 month ago

robbiehanson commented 1 month ago

We've had many requests for a "custom PIN" option. That is, the ability to set a PIN/passcode that's specific to Phoenix, and possibly different than the system PIN/passcode.

This PR allows us to explore how that feature would operate in practice.

On the surface it's quite simple:

A new PIN option is added to the "app access" section. The user can choose any of the following options:

The custom PIN length is 6 digits. (This matches the iOS default PIN length.)

And the PIN will be presented to the user from within the Lock screen:

  

The usual suspects are there:

However, there are some iOS specific "things to consider":

The "system PIN fallback" was added in PR #478. Recall that when we prompt for "system authentication" we basically only have 3 options on iOS:

In other words, prompting for the system PIN is not an option. It's only available as a fallback option (after biometrics fails).

So the "system PIN fallback" option was primarily motivated by user expectations. But was also partly motivated by user reports of broken Face ID (due to broken hardware). It was thought that, if Face ID breaks, then users would still be able to access Phoenix via the system PIN. However, recent reports have cast doubt on that assumption.

When I go to Settings -> Face ID & code, I get the message "A problem was detected with the TrueDepth camera. Face ID has been deactivated"

In other words, in certain situations when Face ID breaks, the system automatically disables it on a system level. Making it no longer accessible from within apps. Meaning even with the "system PIN fallback" users would be locked out of Phoenix. To re-access their wallet, they would need to reinstall & restore their wallet. The custom PIN option does present an alternative here.

robbiehanson commented 1 month ago

Is issue #441 we said:

In all cases, the seed should be backed up before enabling any of these [app access] options. Or at least show a visible warning.

We could add that here, or make it a separate PR.

dpad85 commented 1 month ago

We could add that here, or make it a separate PR

I think it's fine in this PR (it should not be a large change?)

robbiehanson commented 1 month ago

All options are now disabled until the user performs a backup: