search

NiklasMerz / cordova-plugin-fingerprint-aio

Cordova Plugin for fingerprint sensors (and FaceID) with Android and iOS support
https://www.npmjs.com/package/cordova-plugin-fingerprint-aio
MIT License
320 stars 189 forks source link

FingerprintManager deprecated: Plans to use BiometricPrompt? #129

Closed TomSeldon closed 4 years ago

TomSeldon commented 5 years ago

Feature Request

Feature Description

FingerprintManager is deprecated as of API 28 (https://developer.android.com/reference/android/hardware/fingerprint/FingerprintManager), and it looks like BiometricPrompt should be used instead.

Are there plans to support this?

Sorry I can't be much more help, I'm not a Java dev. :)

NiklasMerz commented 5 years ago

Thanks for bringing this up. Help for this from anyone who wants to help is really appreciated.

NiklasMerz commented 5 years ago

This API change should be the best opportunity to rewrite the Android part from scratch. Since this is a big change any help is appreciated. It may take a while for me to start working on this.

NiklasMerz commented 5 years ago

This could be starting point: https://github.com/Kieun/android-biometricprompt

Compatability with older versions of Android via the support library must be considered.

greaterking commented 5 years ago

I will be able to work on this. I've been using/reviewing this plugin for the last few days in development and notice this same issue as well as a few others. The other being the inconsistent way errors are reported for ios/android including the isAvailable() check. For example if finger print auth fails on android it always returns "Cancelled" which isn't helpful and misleading. I see six other un-merged pull request already one of which pertains to the error messages from isAvailable method https://github.com/NiklasMerz/cordova-plugin-fingerprint-aio/pull/173 ...is this AIO still active? Need to know that if I submit changes they'll be reviewed and merged in a timely manner if code checks out...unless you are already working on it. As per other commenters we don't want to use forks which is cordova plugin eco systems biggest problem.

I'd like to

  1. upgrade to BiometricPrompt
  2. use json for call backs with proper success/error codes

Yes changes will be breaking...but thats sometimes unavoidable with major changes.

Please respond soon. Thank you for your work thus far!

NiklasMerz commented 5 years ago

Hi @greaterking. Sorry for the late response but I was on vacation the last few days. Yes this plugin is still actively maintained, but I couldn`t invest the time to test everything recently.

I would like to merge #173 soon.

I would love to see the change to Biometric prompt done, but I won`t do it in the next time. I would love if you want to do the change. This could be a version 3 with more breaking changes and I am ok with that.

Contact me if you need more information.

greaterking commented 5 years ago

Ok great. My approach was going to be...

Does this make sense? I'll post some code soon.

NiklasMerz commented 5 years ago

You approach sounds great! I would do that. Feel free to cleanup the android part a bit since it is hard to read.

For older API versions there is a compat lib/class, right?

This is going to be in version 3.

greaterking commented 5 years ago

Yes I’m using the compact lib and yes it will be version 3... I accidentally named my working development branch 2.0 but I will rename it. Yes and cleaning up Android as well...so far everything is working.

NiklasMerz commented 5 years ago

Sounds great. The branch name doesnt matter to me. You could create a draft PR to discuss the changes.

Your branch already does look better than the old stuff.

greaterking commented 5 years ago

Lol I was trying to salvage code but 😅. I have it working .. just have to refine somethings in Fingerprint.java. Additionally I know there’s various language support so will try to work this in as well .. I’m away for the weekend but will create the draft PR soon.

NiklasMerz commented 5 years ago

No hurry! This could take a while I guess :-). Testing will be tricky, too.

I just released version 2. The new isAvailable returns should be part of the version, too.

greaterking commented 5 years ago

Yes saw that will look at that as well. Yes I think testing will be interesting as well but we'll cross that bridge soon ...I'm sure it will all work out in the end.

greaterking commented 5 years ago

Hey @NiklasMerz I've got everything working well at this point and want to know how would I go about creating a branch to update the ionic native wrapper for testing so I can put up the PR draft? I saw this https://github.com/ionic-team/ionic-native/blob/master/DEVELOPER.md so let me know. I still need to update the readme a bit and I need to update the .js interface as well

NiklasMerz commented 5 years ago

The ionic native wrapper does not belong to this plugin. You could test this with the standard javascript methods. Did you change the public methods of this plugin much? Also consider updating the manual and automatic tests. If you create the PR and select "Allo edits from maintainers" I could help.

Ionic native should be addressed afterwards. You must change this file to update ionic native: https://github.com/ionic-team/ionic-native/blob/master/src/@ionic-native/plugins/fingerprint-aio/index.ts But you should wait before submitting this as PR to ionic native until the new plugin version is released and the API is final.

greaterking commented 5 years ago

Agreed will work on this today.