Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(BiometricLogin): Move to regular login page when biometric login fails with error #14860

Conversation

alexjba
Copy link
Contributor

@alexjba alexjba commented May 20, 2024

What does the PR do

Closes #13046

Moving to regular login whenever the biometric login fails

Affected areas

Onboarding login flow

Screenshot of functionality (including design for comparison)

Screen.Recording.2024-05-20.at.15.44.37.mov

@status-im-auto
Copy link
Member

status-im-auto commented May 20, 2024

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 273aceb #1 2024-05-20 13:02:07 ~7 min macos/aarch64 🍎dmg
✔️ 273aceb #1 2024-05-20 13:03:50 ~9 min tests/nim 📄log
✔️ 273aceb #1 2024-05-20 13:07:21 ~12 min macos/x86_64 🍎dmg
✔️ 273aceb #1 2024-05-20 13:13:04 ~18 min tests/ui 📄log
✔️ 273aceb #1 2024-05-20 13:17:42 ~23 min linux/x86_64 📦tgz
✔️ 273aceb #1 2024-05-20 13:30:06 ~35 min windows/x86_64 💿exe

Copy link
Member

@micieslak micieslak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flow here is not clear for me.

From the issue description I understand that the problem here is invisible error message in case of failed biometric login. But nothing about automatic switch to password mode. Isn't it confusing for potential user to switch automatically with no chance to do another attempt with biometrics and only optionally explicitly switch to password mode by clicking button?

@alexjba
Copy link
Contributor Author

alexjba commented May 20, 2024

The flow here is not clear for me.

From the issue description I understand that the problem here is invisible error message in case of failed biometric login. But nothing about automatic switch to password mode. Isn't it confusing for potential user to switch automatically with no chance to do another attempt with biometrics and only optionally explicitly switch to password mode by clicking button?

Currently there is no option to retry the biometrics login. Once it fails the only option is to use the regular login. Not sure if it's something we're missing here.

@benjthayer could you help in reviewing the login flows when biometrics is used?

@micieslak
Copy link
Member

The flow here is not clear for me.
From the issue description I understand that the problem here is invisible error message in case of failed biometric login. But nothing about automatic switch to password mode. Isn't it confusing for potential user to switch automatically with no chance to do another attempt with biometrics and only optionally explicitly switch to password mode by clicking button?

Currently there is no option to retry the biometrics login. Once it fails the only option is to use the regular login. Not sure if it's something we're missing here.

@benjthayer could you help in reviewing the login flows when biometrics is used?

For me it's a bit strange to see biometrics login error here in the password mode:

Screenshot from 2024-05-20 16-38-14

But maybe it's ok 🤷‍♂️

@alexjba
Copy link
Contributor Author

alexjba commented May 20, 2024

@micieslak Maybe we should have a complete biometrics flow with error field and retry button. Although probably retying biometrics in case of error doesn't make sense because there's nothing the user can do to mitigate the error.

@saledjenic
Copy link
Contributor

The error FOREIGN_KEYS PRAGMA doesn't look like keychain error.
Also keychain errors are handled differently.

We don't have a button on the UI login page like "run biometrics again", also not sure if we need it, but @benjthayer will confirm that.

After a user start the app the flow is:

  • if the pass/pin is stored to a Keychain we should the OS' popup for the Keychain
    • if the user cancels the OS' popup for the Keychain, we automatically display an appropriate input for entering pass/pin
    • if the user successfully accesses the Keychain:
      • if the stored pass/pin is the correct one, the user is logged in
      • if the stored pass/pin is wrong, we should display a message about it and display an appropriate input for entering pass/pin (we can discuss this option, but I wouldn't offer user at this stage to update the pass/pin stored to the Keychain, but rather let him enter it and silently disable biometrics in the backed, that way when he wants to enable it again, will need to provide the correct pin/pass)
  • if the pass/pin is not stored we should display an appropriate input for entering pass/pin

@alexjba alexjba merged commit f944e83 into master May 21, 2024
8 checks passed
@alexjba alexjba deleted the 13046-login-error-is-not-visible-when-authenticating-with-touchid branch May 21, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Login error is not visible when authenticating with TouchID
4 participants