Hide remember me

[owencraston]

Description

• Remove remember me as an option for new users
• The key here is new users. Users who have already enabled remember me should not be affected by this change.
• This simply hides the front end that allows for remember me to be toggled on.
• Based on the feedback below, remember me will be something the user can turn on in settings and is now intended more for developers
• I also made a LoginOptionsSwitch that handles if what login option should be rendered into its own component since it was repeated in several files
• biometrics is always going to be the higher priority option for login. we will only show remember me as an option if its enabled in settings and biometrics are disabled

Future improvements

• Once the auth refactor ships, we can move ALL of the biometric logic (state on weather to show biometric or not, handle errors etch) into this component.
• The main blocker from doing this now is this handleRejectedOsBiometricPrompt which calls updateBiometryChoice
• the updateBiometryChoice function is repeated in each of these files

Checklist

• [x] There is a related GitHub issue
• [x] Tests are included if applicable
• [x] Any added code is fully documented

https://github.com/MetaMask/metamask-mobile/issues/4194 Progresses https://github.com/MetaMask/metamask-mobile/issues/4194

[github-actions[bot]]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[gantunesr]

Some general notes on the PR,

• If this has an impact in the UI/UX, you should add a screenshot/recording of the before and after for QA and future references
• Don't use the keyword Resolves, GH closes the issue related to the PR when it is merged, and this can lead to some confusion on our product boards. Use Progresses instead.

Regarding the code, it looks good to me in the scope of my knowledge about the "Remember Me" epic. I'm not sure if you want @sethkfman to review it since he has more context on the Auth Refactor. If not, and @blackdevelopa agrees, we can approve it and pass it to QA after the failing test (LoginWithBiometricsSwitch) is solved.

[AlexJupiter]

Suggested route forward to still allow this feature to be used by developers.

1. Still hide this feature for new users, but provide the ability for someone to turn Remember Me on via a toggle on the Security & Privacy settings section of the app.
2. This toggle should be placed underneath the "Unlock with Face ID?" toggle and be named "Unlock with Remember Me?". It should also have the subtitle "Turn on at your own risk, since this feature gives anyone with access to your phone also access to MetaMask without any additional security."
3. This toggle should be turned off by default on new installations
4. We should record an event when this toggle is turned on Remember Me Toggled On and an event for when the toggle is turned off Remember Me Toggled Off

@jakehaugen could you briefly check my copy suggestion please? Thank you :).

cc. @plasmacorral @gantunesr @sethkfman @owencraston

[AlexJupiter]

@owencraston slight improvement to the copy @jakehaugen and I have discussed: Title: Allow "Remember me" Subtitle: Turn on at your own risk. When "Remember me" is on, it gives anyone access to your MetaMask if they can access your phone.

Since this should just enable the checkbox again, and not actually turn it on, we think this copy improvement is necessary. Any thought let us know!

[owencraston]

@gantunesr thanks for the comments, as requested I have went ahead and documented these new patterns into proposals.

Typed redux actions

• proposal

Components that return null

• proposal

[plasmacorral]

Observation M1

Getting an error when attempting to change the password: "View:Root TypeError: null is not an object (evaluating 'shouldRenderBiometricOption.toLowerCase')"

https://recordit.co/5SPXcE2iIL

Reproduction steps:

1. Configure account with v5.2 build 913, import an account, bind Keystone hardware wallet and enable remember me
2. Update to the branch under test
3. launch MetaMask mobile
4. tap the burger menu
5. tap settings
6. tap security and privacy
7. tap change password
8. observe error

[owencraston]

@plasmacorral With my latest commit, I have addressed observation M1.

[gantunesr]

Some issues I noticed while testing the branch,

1. The "Biometrics" is getting triggered when I try to access the Security & Privacy settings
2. When I enable "Unlock with Face ID" it triggers the Biometrics two times
3. Pressing outside of the password modal throws an error. The error is TypeError: trigger.current is not a function. (In 'trigger.current()', 'trigger.current' is an instance of Class). I see that this is a reference error regarding the ReusableModal.

cc @plasmacorral

[owencraston]

@plasmacorral I am ready for you to take another look 👀 !

[plasmacorral]

I have read the CLA Document and I hereby sign the CLA

[plasmacorral]

I have completed another pass of feature QA and have the following observations:

MB2: Password entry not obfuscated in the prompt after attempting to disable RM enable toggle. Owen mentioned some remaining UI work, but this must be completed prior to merging for RC.

MB3: faceID/fingerprint authentication is turned on during a password change, regardless of user selection during the password change or configuration prior to password change

• faceID turned on in pw change causing device to freeze on orange diamond when faceID perms are not granted or have been previously disabled https://recordit.co/qDk2SKyemb
• faceID turned on in pw change when permissions have previously been granted https://recordit.co/z9mXoPCQ5a
• https://recordit.co/IPGozRiBYc faceID toggle within password change is: - 16s- Showing the toggle set to off when faceID auth is enabled - 45s- faceID remains enabled if user engages the toggle to on - 1m5s- Only turns off faceID if the user taps the toggle on then off.
• This also happens when Device passcode is the authentication factor - https://recordit.co/EcUzoY1h7C - 5s login and toggle RM feature to on - 20s enable iPhone passcode auth - 25s use iPhone passcode auth for access - 28s accessing Security & Privacy requires device passcode - 35s confirm password for a pw change with Device passcode auth enabled - 42s enter and confirm new password, and do NOT enable "unlock with Face ID"

MB4: https://recordit.co/EcUzoY1h7C in the device passcode video at 28s, you can see an example of the device passcode being prompted to allow the user into the Security & Privacy menu. - This is not the case today in v5.3 build 927, where a user with device passcode authentication is able to open the settings page without having to provide the device passcode. https://recordit.co/X23ksVCcnh

MB5: Revisiting a recording from MB3 https://recordit.co/IPGozRiBYc. The Biometric/faceID toggle within the change password flow does not match current user settings - If biometrics or device passcode are enabled in Production build 927 v5.3, this toggle defaults to ON within the password change flow and matches the current user settings.

MB6: When tapping "cancel" on faceID failure, toggle at login page looks like faceID is off...but faceID actually remains enabled. Cannot use password auth as an alternative to faceID until turning faceID on, then back off- bug? - https://recordit.co/qWOrEsplxt - https://recordit.co/Y5FqxH9aWt

MB7: Device passcode authentication enables RM enable toggle https://recordit.co/B5FscoUN5g
- @40s- Turning on device passcode auth turns the enable remember me toggle to ON. Should these two setting toggles be independent?

MB8: from the same Device passcode authentication video https://recordit.co/B5FscoUN5g
- @41s- When device passcode is auth method, passcode is asked after every keystroke when trying to disable RM.

MB9: When biometrics are present the faceID toggle in the import from seed view should default to the ON position. - It does default to on in production build 927, but not in this branch.

[owencraston]

@holantonela I roughly implemented your design requests. Does this look good to you?

[holantonela]

Thanks for this iteration @owencraston. it looks LGTM!

We will need a password default text for the input field. We can do it now if a reusable component exists or open a ticket for the design system team to make one. cc @georgewrmarshall

[holantonela]

UPDATED

Ay, Corey revisited the copy. We should have

Enter your password to turn off the Remember me option If you turn this option off, you'll need your password to unlock MetaMask from now on.

REF https://www.figma.com/file/WJF8Tdwysm6dzabGkMYpJK?node-id=301:198#235957544

[plasmacorral]

MB 10: Just noticed in the screenshots from observation MB9 that the actual toggle is moved from the right side of the screen and in this branch the toggle appears underneath "Unlock with FaceID" on the left side.

Should the toggle remain on the right side of the screen?

[georgewrmarshall]

Thanks for this iteration @owencraston. it looks LGTM!

We will need a password default text for the input field. We can do it now if a reusable component exists or open a ticket for the design system team to make one. cc @georgewrmarshall

@holantonela I think @owencraston just needs to add a placeholder prop to OutlinedTextField component? Not super familiar with that component but looks like it accepts it?

[owencraston]

@plasmacorral regarding MB6: This is the 5.4.0 build and it appears to be doing the same behaviour. Do you spot any differences?

https://user-images.githubusercontent.com/22918444/181635230-16a3063a-695c-46d7-8766-edbd8a9ce09e.MP4

[plasmacorral]

@plasmacorral regarding MB6: This is the 5.4.0 build and it appears to be doing the same behaviour. Do you spot any differences? Image.from.iOS.MP4

Your video looks like it addresses MB6.

[owencraston]

@holantonela @georgewrmarshall

Updated designs with new copy and placeholder

[owencraston]

I made a separate PR for the strings so we could get the translations in ASAP. I will rebase once this is merged

[owencraston]

I ripped the translations out of this crowdin PR