metamask-mobile
metamask-mobile copied to clipboard
[IMPROVEMENT] Authentication Refactor
Description
The goal of this refactor was to be able to remove logic from our UI components that handle authentication(login, logout, update password, update biometric) and centralize them in a single service.
This work is preparation to make it easier for additional refactors and improvements. There is still more opportunity to refactor vault creation but that will be completed in future work.
Checklist
- [ ] There is a related GitHub issue
- [x] Tests are included if applicable
- [x] Any added code is fully documented
Issue
Resolves #???
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.
@owencraston and myself paired and dev tested on this PR. We are feeling pretty good about the update but need review from other devs. Thx
I am 35% done with testing.
Issue 8
When i tried importing an account I am prompted to log in with faceID biometrics. It seems as if faceID biometrics is stuck in a loop for a brief moment then i am taken back to the onboarding view:
To reproduce: import your wallet with “unlock with biometrics” on open the account lists tap on “import account” enter your private key notice after you import the account, you are asked to use your biometrics again and again until you are finally kicked back to the onboarding view.
I also see this error undefined is not an object (evaluating address.toLowerCase)
see here: http://recordit.co/1cgeuKBpVn
Issue 9
While going through the backup SRP flow I am prompted to enter my password and when i do enter my password I get an error Couldn’t unlock your account. Please try again”
to reproduce:
creating your wallet with remember me/ unlock with biometrics toggled on
land on the wallet view then kill the app
relaunch the app
now while on the wallet view, tap on the text “Protect wallet” in the back up alert
notice you are taken to step 1 of the backup SRP page.
proceed to step 2 “Secure wallet”.
Here you would be asked to enter your password. If your device has biometrics enabled deny it. That way you can enter your password to proceed.
enter your password.
Notice the error Couldn’t unlock your account. Please try again
see here: http://recordit.co/p8f1UpzA0A
Issue 10
This one is sometimes-ish meaning it is not a consistent bug to reproduce. When I attempt to deep link to polygon send flow i get: [TypeError: undefined is not an object (evaluating 'identities[selectedAddress].name')]
to reproduce: add polygon to your list of networks kill the app tap on https://metamask.app.link/send/0x0000000000000000000000000000000000001010@137/transfer?address=0xC5b2b5ae370876c0122910F92a13bef85A133E56&uint256=3e18
the error should appear.
Update on Issue 1B
I am still have to authenticate two times before unlocking the app. see here http://recordit.co/grrcMhTtaJ
Update on Issue 2: I am unable to disabled biometrics. The app logs me out, tries to authenticate using FaceID then prompts me with an error alert “invalid password”
see here: http://recordit.co/x6S1FYX1Nl
I am 75% done with testing
@cortisiko I made progress on most of the items but still have to reply to @owencraston feedback. I think this will need to wait until May 24th. Unless @owencraston ends up with some free time. No pressure @owencraston ;-)
- Outstanding issues:
- Issue 2
- New issue 11: If you lock the wallet (biometrics enabled), kill the app, then start the app, in the login view the biometric button on the password window appears but doesn't work (it should be hidden)
Hey @sethkfman I tested the fixes on your latest commit. Most of the issues are fixed.
Issue 9 is fixed but we have a cosmetic issue. The color of the masked password differs from that of prod. On your PR the color is black while in prod it is white.
I do have another issue to report. Call this one Issue 12.
Issue 12
As a new user when I declined to authenticate using FaceID (here visual representation of what I mean) I get into a broken state. Meaning I am stuck on the Create your wallet view.
To reproduce: Create a new wallet You should be prompted with an alert message asking you to Allow or disallow Face ID (this is what I mean.) Tap on “don’t allow” Notice you are in a broken state.
See here
I will be OOO until June 6th. If you are not too swamped @plasmacorral can you keep things rolling on this? Thanks!
Break out onLogin to catch separate issues: https://sentry.io/organizations/metamask/issues/2034834647/?project=2299799&query=is%3Aunresolved+release.version%3A5.3.0+release.build%3A927&sort=freq&statsPeriod=30d
Close PR due to inactivity