Fixed
Status Update
Comments
py...@gmail.com <py...@gmail.com> #2
Removing from wednesday's list for now since there have been no reports.
rb...@unwire.com <rb...@unwire.com> #3
Project: platform/frameworks/support
Branch: androidx-master-dev
commit d02b8afc951f6a9e0fc8e24e19e56a06f1f60fac
Author: Curtis Belmonte <curtislb@google.com>
Date: Wed Sep 11 14:33:54 2019
Tolerate null result in BiometricPrompt fragments
The AndroidX BiometricPrompt API for onAuthenticationSucceeded requires
a non-null AuthenticationResult, so we construct one from the platform
BiometricPrompt or FingerprintManagerCompat result in BiometricFragment
and FingerprintHelperFragment, respectively, when calling this method.
However, the platform API does not guarantee that the result it provides
will be non-null, in which case the current implementation would crash.
This commit fixes the issue by creating an AuthenticationResult with
null crypto when handling a null result in either of these places. It
also adds unit tests to verify the new behavior.
Test: ./gradlew biometric:connectedAndroidTest
Fixes: 138862251
Change-Id: I540dd3b4ebdf100553b4fac609f3d928ec69ebfb
A biometric/src/androidTest/java/androidx/biometric/BiometricFragmentTest.java
A biometric/src/androidTest/java/androidx/biometric/FingerprintHelperFragmentTest.java
M biometric/src/main/java/androidx/biometric/BiometricFragment.java
M biometric/src/main/java/androidx/biometric/FingerprintHelperFragment.java
https://android-review.googlesource.com/1122743
https://goto.google.com/android-sha1/d02b8afc951f6a9e0fc8e24e19e56a06f1f60fac
Branch: androidx-master-dev
commit d02b8afc951f6a9e0fc8e24e19e56a06f1f60fac
Author: Curtis Belmonte <curtislb@google.com>
Date: Wed Sep 11 14:33:54 2019
Tolerate null result in BiometricPrompt fragments
The AndroidX BiometricPrompt API for onAuthenticationSucceeded requires
a non-null AuthenticationResult, so we construct one from the platform
BiometricPrompt or FingerprintManagerCompat result in BiometricFragment
and FingerprintHelperFragment, respectively, when calling this method.
However, the platform API does not guarantee that the result it provides
will be non-null, in which case the current implementation would crash.
This commit fixes the issue by creating an AuthenticationResult with
null crypto when handling a null result in either of these places. It
also adds unit tests to verify the new behavior.
Test: ./gradlew biometric:connectedAndroidTest
Fixes: 138862251
Change-Id: I540dd3b4ebdf100553b4fac609f3d928ec69ebfb
A biometric/src/androidTest/java/androidx/biometric/BiometricFragmentTest.java
A biometric/src/androidTest/java/androidx/biometric/FingerprintHelperFragmentTest.java
M biometric/src/main/java/androidx/biometric/BiometricFragment.java
M biometric/src/main/java/androidx/biometric/FingerprintHelperFragment.java
ca...@gmail.com <ca...@gmail.com> #4
Can confirm that it still happens in 1.1.0-beta01
cu...@google.com <cu...@google.com>
ap...@google.com <ap...@google.com> #5
Project: platform/frameworks/support
Branch: androidx-main
commit fcbbd9368d75b736de4b92d067be84421376b294
Author: Curtis Belmonte <curtislb@google.com>
Date: Wed Jan 20 15:54:34 2021
Reset BiometricViewModel callback in Fragment#onDestroy()
Currently, the client callback reference held by BiometricViewModel is
retained for the lifecycle of the hosting activity. In the (likely) case
that a client is using a fragment within that activity to host
BiometricPrompt and passes an AuthenticationCallback with a reference to
that fragment, this will cause the fragment to leak.
This commit applies a minimal fix for the issue by resetting the
callback reference held by the view model when the host fragment is
destroyed (via a LifecycleObserver). This shouldn't affect the prompt's
behavior across configuration changes such as device rotation, since the
callback should be reinitialized by the client in onCreate() or similar.
Test: Biometric integration test app on API 27-30.
Test: ./gradlew biometric:biometric:test
Test: ./gradlew biometric:biometric:connectedAndroidTest
Test: ./gradlew biometric:integration-tests:testapp:connectedAndroidTest
Bug: 167014923
Change-Id: I7086460fac3921a490f4e2abf0671adec5c146bd
M biometric/biometric/src/main/java/androidx/biometric/BiometricPrompt.java
M biometric/biometric/src/main/java/androidx/biometric/BiometricViewModel.java
https://android-review.googlesource.com/1555701
Branch: androidx-main
commit fcbbd9368d75b736de4b92d067be84421376b294
Author: Curtis Belmonte <curtislb@google.com>
Date: Wed Jan 20 15:54:34 2021
Reset BiometricViewModel callback in Fragment#onDestroy()
Currently, the client callback reference held by BiometricViewModel is
retained for the lifecycle of the hosting activity. In the (likely) case
that a client is using a fragment within that activity to host
BiometricPrompt and passes an AuthenticationCallback with a reference to
that fragment, this will cause the fragment to leak.
This commit applies a minimal fix for the issue by resetting the
callback reference held by the view model when the host fragment is
destroyed (via a LifecycleObserver). This shouldn't affect the prompt's
behavior across configuration changes such as device rotation, since the
callback should be reinitialized by the client in onCreate() or similar.
Test: Biometric integration test app on API 27-30.
Test: ./gradlew biometric:biometric:test
Test: ./gradlew biometric:biometric:connectedAndroidTest
Test: ./gradlew biometric:integration-tests:testapp:connectedAndroidTest
Bug: 167014923
Change-Id: I7086460fac3921a490f4e2abf0671adec5c146bd
M biometric/biometric/src/main/java/androidx/biometric/BiometricPrompt.java
M biometric/biometric/src/main/java/androidx/biometric/BiometricViewModel.java
cu...@google.com <cu...@google.com>
di...@gmail.com <di...@gmail.com> #6
I don't use Fragments and use a View-based solutions instead. I create prompt like this:
BiometricPrompt(requireActivity, myAuthCallback)
So it is tied to the activity and it still leaks my view-based screens (1.2.0-alpha02). Ideally there would be a way to reset this callback. Latest commit fixes this leak only for Fragments (sadly).
Is there any workaround maybe? Dirty (but working) ones would do too ;)
ja...@sharesies.co.nz <ja...@sharesies.co.nz> #7
Ditto with the above comment. We're a compose-only app, and use `BiometricPrompt` with the `Activity` context as opposed to passing in a Fragment. This results in leaks, as the callback held by `BiometricViewModel` is never cleared.
An API allowing developers to pass in their own `LifecycleOwner` to set up the `ResetCallbackObserver` observer would be a relatively quick and easy fix for this. I'm more than happy to draft a change for this and submit it via GitHub!
An API allowing developers to pass in their own `LifecycleOwner` to set up the `ResetCallbackObserver` observer would be a relatively quick and easy fix for this. I'm more than happy to draft a change for this and submit it via GitHub!
te...@gmail.com <te...@gmail.com> #8
Any updates on this?
Description
Biometric library version:
1.1.0-alpha01
This bug was reported in Mozilla Fenix, but it's actually a leak in BiometricViewModel.
Mozilla Fenix bug report:https://github.com/mozilla-mobile/fenix/issues/13477
To reproduce, follow the official Android X sample code and add the sample code to a fragment. Display the login prompt, get the fragment destroyed, the fragment is now leaking
As soon as the fragment is destroyed, the leak triggers as the fragment cannot be garbage collected because the AuthenticationCallback implementation is retained.
Looking at the Android X sources , the callback is set in
BiometricPrompt
BiometricPrompt.init()
which is called from theBiometricPrompt
constructor and never unset.So, as it stands, there is no way to clear the callback, and
BiometricViewModel
is always created with an activity lifecycle, so the callback always has the lifecycle of the activity. This means that currently the library will leak any fragment it's used from.