status-im / status-mobile

a free (libre) open source, mobile OS for Ethereum
https://status.app
Mozilla Public License 2.0
3.85k stars 982 forks source link

Enable biometrics from settings without password #19084

Open clauxx opened 3 months ago

clauxx commented 3 months ago

Bug Report

Problem

Follow up to https://github.com/status-im/status-mobile/pull/18258

Currently, when enabling biometrics from the password settings, the user is prompted to enter their password, which is different from the figma user flows.

This is necessary due to us using having to store the password in the keychain so that during the log-in flow, the successful biometrics check would log the user in with the keychain password. One way to avoid that is to always store the password during onboarding, not only if the user enables biometrics, and never delete the password from the keychain (currently we delete it when disabling biometrics and when logging out).

This could have security implications, so it should be discussed with the design team and the CCs (in the discord #biometrics channel).

Expected behavior

The password is not prompted when the user enables biometrics in the password settings

Actual behavior

The password is prompted when the user enables biometrics in the password settings

flexsurfer commented 1 month ago

@clauxx could you please elaborate, i see the password in figma

image

https://www.figma.com/file/JlpPhJp0SMnEyBQy4nOZHo/Settings-for-Mobile?type=design&node-id=7212%3A149947&mode=design&t=JDdq7AtuF6AICoJX-1

clauxx commented 1 month ago

@flexsurfer we adapted the designs to include the password cause it was easier/quicker at the time, but originally it wasn't there. It was needed because the password was necessary for enabling the biometrics and we didn't want to make all users store it in the keychain, hence we're prompting the user every time. Here we should find a way to do it without (i guess storing the hashed password in the keychain when the user adds the password during onboarding)

CC @cammellos

flexsurfer commented 1 month ago

thank you, then we should descope it from release I guess

flexsurfer commented 1 month ago

This is not exactly a very frequent action; usually biometric is enabled in onboarding so I would keep password for enabling onboarding from settings for security reasons

flexsurfer commented 1 month ago

@churik should we remove this from 2.29?

cammellos commented 1 month ago

removing from milestone