AndroidX biometric manager #260
Conversation
|
@OleksandrKucherenko - you haven't modified the example to work on iOS, right? |
|
@compojoom No, only android right now... PR is not finished yet. It's earlier preview. in plans:
|
|
I can do the iOS updates if you want. I wanted to test the example, but it is not working with 0.57 and xcode 11, so I'll have to update it anyway. |
|
@compojoom great! any help is appreciated! Thanks |
|
After fighting with it for couple of hours I got it to work. Partially. I guess that I'll need to update the js code as well. I have a question - I was running into a lot of messages similar to this one: And it turned out that once I deleted the node_modules/react-native-keychain/node_modules it worked. It seems that since the example install react-native-keychain from ..file if the user runs yarn in the main repository all node files will be also copied to the keychainExample/node_modules/react-native-keychain folder. I can remove the node_modules in the postinstall_cleanup, but I wonder if that is correct. |
The example project is a major test app, that is why I use the |
I have no idea why that message shown... maybe a RN 0.6x after migration thing |
|
here are my changes: I created a react-native.config.js for autolinking and fixed the keychainExampel - the app runs fine on iOS and on Android. Would you please merge my changes in your branch? |
|
Do we really need a dynamic switch between androidX and the android support library? |
|
I tested on an ulefon without any security turned on and I managed to save and load credentials. The phone has faceid support, I turned on this one, but the library was not detecting any faceid. I looked at the source code and it seems that there is no faceid on android: Then I tested on a huawei device with fingerprint. I am able to save the password (withot a prompt), but when I try to read I'm presented with the fingerprint input ui. Once I scan the finger however I get an error "could not load credentials. Error: Must be called from main thread of fragment host" I don't understand however how this is supposed to work cause iOS and android behave slightly different. on iOS if I select passcode, password, touchId or fingerprint. I have to give permission for writing to the keychain. |
Nice catch. I saw that error before, but was not able to catch a clear exception stack trace. Thanks for details. Now I can fix it. It's a 1-2 lines of code. Several commits from my side should be today to this PR. I almost solve the |
Yes, true its confusing me too. But that's how its done on android. RSA public key used for encryption, and RSA private key used for decryption. For save operation not needed any "unlock" or "permissions" - just encrypt and save. But for decrypt operation we should allow system to extract private part of the key and its protected by 'fingerprint' scanning process. In other words "storage" is a SharedPreferences, that is unprotected, it's and XML file inside application folder. But each record inside preferences is encrypted. FaceId for android can be implemented too, but that will be subject of another PR. |
|
1-2 lines of code! Damn this is when you know java :) Let me guess - something with new Handler? I've been googling for the past 20mins :) In the meantime I found a new problem. I'm trying to simulate a fingerprint in the emulator. when I try to save credentials I get: stack trace is |
/** trigger interactive authentication. */
public void startAuthentication() {
final FragmentActivity activity = (FragmentActivity) getCurrentActivity();
if (null == activity) throw new NullPointerException("Not assigned current activity");
// code can be executed only from MAIN thread
if(Thread.currentThread() != Looper.getMainLooper().getThread()){
activity.runOnUiThread(this::startAuthentication);
return;
}
final BiometricPrompt prompt = new BiometricPrompt(activity, executor, this);
final BiometricPrompt.PromptInfo info = new BiometricPrompt.PromptInfo.Builder()
.setTitle("Authentication required")
.setNegativeButtonText("Cancel")
.setSubtitle("Please use biometric authentication to unlock the app")
.build();
prompt.authenticate(info);
}
} |
You should try to test on real device. Emulators unfortunately are quite bad with fingerprint emulation. You should check API level (should be 23 or higher), and required real crypto libraries (that are often not a part of emulator OR limited in scope) |
|
Ok, ordered a cheap phone with fingerprint reader. I got another funny error on samsung galaxy s9. I was able to store the credentials, but trying to read them resulted in: |
ups... that is my issue, result of quick fix with main thread... lib should not release the caller thread untill recieve the result of user interaction. |
|
Another thing that is strage. With your changes when I try to sync gradle I get: I had to either comment this line out or add: to the build.gradle file. Why is it working at your end? |
|
Just tested your changes and now it seems to work fine! The foldable emulator with API 29 didn't work, but Pixel2 emuulator with API 28 worked as well. |
fixed... but RN59 example is not finished yet |
|
|
|
and now RN59
|
|
@oblador - I think PR is ready for merge. Required only testing on iOS - @compojoom |
|
Hi, thanks for the big effort in building fingerprint support for Android into the library! The PR seems to be quite large, and in many ways overlapping with #195. I'm wondering wether it would be more beneficial to first focus on getting the main fingerprint functionality to work through the #195 PR, and then build on top of that with separate PRs to introduce the changes proposed here. I could at least see some of the changes be separated into separate PRs for easier review:
|
I agree that it probably doesn't make sense to provide backwards compativility to android support library, as release 4.0.0 already moved the library to AndroidX from android support library. |
it's done. Please review and approve it. |
|
@OleksandrKucherenko - I haven't tested the lateste changes yet, but just from the review - your KeychainExample is the old version - you haven't applied my changes to it and it was not working yesterday. So I doubt that it will work today. Or am I missing something? |
|
PR is temporary not usable... I'm working on gradle fix. workaround: # cd KeychainExample/android
./gradlew preBuild
./graldew assembleDebug |
|
Just weighing in on the backwards compatibility parts; since Google has a requirement for people to migrate towards AndroidX and RN 0.60 is necessary to do so, I don't see the point in keeping backwards compatibility with 0.59. I'm fine with having 0.61 as a requirement even, just have to follow semver and release it as a new major version. |
OleksandrKucherenko
force-pushed
the
android-biometric-manager
branch
from
55e0fde to
9741208
Compare
added strings for copy/paste fixed anchors cleanup the code iOS fix, documentation become more accurate now improved ios part last minute changes updated list of ignored files Sync api method changes cleanup
OleksandrKucherenko
force-pushed
the
android-biometric-manager
branch
from
9741208 to
fa43f37
Compare
|
Wohooo 😄 🍾 🎉 |
|
i installed verison 7.0.0 but Android not authenticate. |
implementation of Biometric with fingerprint scan interaction.
Major changes: