smstuebe / xamarin-fingerprint

Xamarin and MvvMCross plugin for authenticate a user via fingerprint sensor
Microsoft Public License
494 stars 118 forks source link

Fix for the LivecycleObserver #200

Closed albertruff closed 3 years ago

albertruff commented 3 years ago

When a new instance of BiometricPrompt class is created, it adds an LifecycleObserver to the Lifecycle of the given FragmentActivity but never removes it. So when you call NativAuthentication multiple times, multiple LifecycleObserver will be added to CurrentActivity. This produces the following issue: When you have a device, who have only Pin, Pattern or Password enabled and calling CrossFingerprint.Current.AuthenticateAsync (with AllowAlternativeAuthentication = true) twice, on the second call you never gets the Task fineshed. The problem is that all operations performs on the first LifcycleObserver that was added. This problem exist in Android 9 and lower. Tested on diffrent Devices with Android 7 and 9.

smstuebe commented 3 years ago

Thanks for making me aware of this issue. I was able to reproduce it on a simulator. Your solution looks a bit hacky to me (no offense) :). It might fix the issue, but would expect, that these resource should be properly cleaned up after authentication by the framework itself. I'm currently having a look at the android-x code (maybe I'm just missing a dispose or release call). If you know more, let me know. If I can't find anything I will merge and release your solution.

albertruff commented 3 years ago

My Solution is based on the first Comment from here. I explore the AndroidX-Code to verify this behavior and in line 595, in the BiometricPrompt-Constructor it adds an LivecycleObserver to mFragmentActivity. I have searched in the code for the removing but I couldn't find anything. I think google doesen't care about removing, because googles recomendation is to create an BiometricPrompt, once, in onCreate of an Activity and to add an clickListener to a Button with an authenticate-call.

smstuebe commented 3 years ago

Ahh cool. Thanks for the details. I searched the wrong branch ^^

I think google doesen't care about removing, because googles recomendation is to create an BiometricPrompt, once, in onCreate of an Activity and to add an clickListener to a Button with an authenticate-call.

That's very odd 🙄 but ok.

albertruff commented 3 years ago

Is It possible to go the same way? To create the BiometricPrompt once and call evry time the authenticate. The other problem is that the last release of AndroidX, i think its not ported yet to xamarin, set the isDeviceCredentialAllowed [deprecated](https://developer.android.com/reference/androidx/biometric/BiometricPrompt.PromptInfo#isDeviceCredentialAllowed()). The new implementation (Line 817) don't adds an LifecycleObserver.

smstuebe commented 3 years ago

I've made the cleanup more defensive and I cleanup the lifecycle directly after the authentication finished. Can you have a look? What do you think? Do you see any problems?

Is It possible to go the same way? To create the BiometricPrompt once and call every time the authenticate.

No, because the plugin doesn't know if it's always the same activity. We would have to keep a list of activities and prompts. This is state. and I hate state 😅

albertruff commented 3 years ago

O a nice and clean solution, I don't see any problem jet

smstuebe commented 3 years ago

@albertruff 2.1.3 deployed :)