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: App crash on app unlock with fingerprint (WPB-5110) (WPB-5108) #2348

Merged
merged 2 commits into from
Oct 20, 2023

Conversation

ohassine
Copy link
Member

@ohassine ohassine commented Oct 19, 2023

BugWPB-5110 [Android] Applock - Fail applock biometrics more than 3 times , Crash


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

  1. App crash when unlock with biometrics fails due to too many failed attempts
  2. Black screen shown without showing biometrics prompt dialog

Causes (Optional)

  1. After 5 failed attempts of auth with biometrics, the system will lock out the biometrics API and will return an error which we were not handling it.

  2. The app was emitting the screen to go when it's in the background causing a failure when showing biometrics prompt.
    Unable to start authentication. Called after onSaveInstanceState()

Solutions

  1. handle that error by navigating the user to passcode screen

  2. Make flow observation lifecycle aware so the app will stop emitting/observing when it's in background. Also this way the app will prevent resource wasting.

Needs releases with:

  • GitHub link to other pull request

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

  • Try to authenticate with wrong biometrics multiple time so the API get locked out.
  • Open the app again
  • The app should not crash
  • Authenticate with biometrics
  • Close the app and let it in background for 1min
  • Open the app again, you should see biometrics dialog

PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

@ohassine ohassine changed the title App crash on app unlock with fingerprint (WPB-5110) (WPB-5108) fix: App crash on app unlock with fingerprint (WPB-5110) (WPB-5108) Oct 19, 2023
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #2348 (96ac249) into develop (44854bb) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

@@              Coverage Diff              @@
##             develop    #2348      +/-   ##
=============================================
- Coverage      41.21%   41.19%   -0.03%     
  Complexity      1058     1058              
=============================================
  Files            335      335              
  Lines          12059    12065       +6     
  Branches        1595     1595              
=============================================
  Hits            4970     4970              
- Misses          6614     6620       +6     
  Partials         475      475              
Files Coverage Δ
...com/wire/android/biomitric/BiometricPromptUtils.kt 0.00% <0.00%> (ø)
...rc/main/kotlin/com/wire/android/ui/WireActivity.kt 0.00% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44854bb...96ac249. Read the comment docs.

@github-actions
Copy link
Contributor

Test Results

649 tests  ±0   649 ✔️ ±0   7m 1s ⏱️ - 2m 41s
  94 suites ±0       0 💤 ±0 
  94 files   ±0       0 ±0 

Results for commit 96ac249. ± Comparison against base commit 44854bb.

@github-actions
Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@ohassine ohassine requested review from a team, typfel, yamilmedina, alexandreferris, MohamadJaara and saleniuk and removed request for a team October 19, 2023 12:30
@AndroidBob
Copy link
Collaborator

Build 1428 succeeded.

The build produced the following APK's:

Copy link
Contributor

@yamilmedina yamilmedina left a comment

Choose a reason for hiding this comment

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

🙌

@ohassine ohassine added this pull request to the merge queue Oct 20, 2023
Merged via the queue into develop with commit 37ec8f1 Oct 20, 2023
21 of 22 checks passed
@ohassine ohassine deleted the app_crash_on_app_unlock_with_fingerprint branch October 20, 2023 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants