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

[android][expo-local-authentication] supportedAuthenticationTypesAsync returns wrong type for Pixel 4 #7838

Closed
awinograd opened this issue Apr 14, 2020 · 12 comments

Comments

@awinograd
Copy link
Contributor

🐛 Bug Report

The Pixel 4 has facial recognition, but no fingerprint sensor. LocalAuthentication.supportedAuthenticationTypesAsync returns as if the Pixel 4 had a fingerprint sensor but no facial recognition.

Environment

Expo CLI 3.17.14 environment info:
System:
OS: macOS 10.15.3
Shell: 5.7.1 - /bin/zsh
Binaries:
Node: 12.14.0 - ~/.nodenv/versions/12.14.0/bin/node
Yarn: 1.12.1 - ~/.yarn/bin/yarn
npm: 6.13.4 - ~/.nodenv/versions/12.14.0/bin/npm
IDEs:
Android Studio: 3.5 AI-191.8026.42.35.6010548
Xcode: 11.3.1/11C504 - /usr/bin/xcodebuild
npmPackages:
expo: ^37.0.0 => 37.0.3
react: 16.9.0 => 16.9.0
react-native: https://github.com/expo/react-native/archive/sdk-37.0.0.tar.gz => 0.61.4

Targeting Pixel 4 (android) standalone & expo client

Steps to Reproduce

  1. Open snack on Pixel 4: https://snack.expo.io/@ouihealth/a78fc1

Expected Behavior

Should render { supportedTypes: [2] } to the screen

Actual Behavior

Renders { supportedTypes: [1] } to the screen

Reproducible Demo

https://snack.expo.io/@ouihealth/a78fc1

@awinograd
Copy link
Contributor Author

It looks like the underlying Android library doesn't distinguish between the available authentication type. Does it make more sense to return a generic AuthenticationType.BIOMETRIC on android instead of pretending it's a fingerprint?

@byCedric
Copy link
Member

Hi @awinograd! Good catch, that's definitely something we should fix 😄 I'll put it on our list to take a closer look at once we upgrade to Android R. To me, it looks sensible to add a more abstract BIOMETRIC type. But maybe someone has another idea to keep the types compatible. Thanks for reporting this!

@bayoremit
Copy link

Forgive me if leveraging this thread for this question is a big NO. I have a semi-related question

We just updated to SDK37 and need to account for the breaking changes from expo-local-authentication. The new BiometricPrompt seems to require Android 6, but Expo's min Android version is said to be Android 5.

What will happen when this is called on Android 5? Does this mean, expo's min android version is now 6? or do we have to specially handle the Android 5 situation? If so, is there a different API we need to call?

Thanks!!

@vernondegoede
Copy link

Hi @byCedric,

I'll put it on our list to take a closer look at once we upgrade to Android R

Are you suggesting this will be fixed only in the next major release? Our Samsung Galaxy S10 has the same issue, so this isn't limited to Pixel 4.

@byCedric
Copy link
Member

Hi @vernondegoede! I'm sorry if my wording felt like we were delaying this, that's not the case. We are exploring some options at the moment to implement the best fix, preferably without breaking changes.

Just to clarify, does your S10 not have a fingerprint? Or do you have facial recognition set-up, but it's returning as fingerprint?

@byCedric byCedric self-assigned this Apr 16, 2020
@vernondegoede
Copy link

Cool!

My S10 does have the ability to use fingerprint, but it's not set up. I do have facial recognition set-up, though.

Since facial recognition is not recognized as an available local authentication method, we therefore fall back to passcode.

@tsapeta
Copy link
Member

tsapeta commented Apr 17, 2020

Hello guys! 👋

This is definitely something that we need to lean over. supportedAuthenticationTypesAsync was really easy to implement before we moved to BiometricPrompt API as with FingerprintManager (as name suggests) we could support only fingerprint as authentication type. Even if the device had others auth types such as face recognition or iris recognition they all were not provided by the platform itself. Moving to BiometricPrompt opened up the possibility to use other auth types, however I don't know of any way that could tell us which types are supported by the device - we can only determine whether any biometric method is available.

I'm generally fine with introducing separate AuthenticationType.BIOMETRIC as @awinograd suggested. Unfortunately, it has its trade-offs, in particular it would cause a discrepancy between platforms. Would BIOMETRIC be supported on iOS? If so, what would it mean there? Would it mean that the device supports at least one of finger and facial recognitions? Either way, I don't see any profit of this new type and also the entire method. LocalAuthentication.hasHardwareAsync() should be the only needed method in all cases because it means exactly what you should need. In my opinion apps shouldn't really depend on which authentication types are available on the device if only any (hardware/biometric) is available. There is also no way to choose between fingerprint/face recognition types in authenticateAsync method as it's not possible on both platforms. At this moment we can only hope it will be possible at some point.

To sum up, I'd be more for just slowly deprecating supportedAuthenticationTypesAsync or maybe have it available only on iOS. On Android we can't really get any more information that you can get now through hasHardwareAsync. I'm leaving this issue open because I still want to hear your opinions 😉

@tsapeta
Copy link
Member

tsapeta commented Apr 17, 2020

I also wanted to refer to @bayoremit question:

The new BiometricPrompt seems to require Android 6, but Expo's min Android version is said to be Android 5.
What will happen when this is called on Android 5? Does this mean, expo's min android version is now 6? or do we have to specially handle the Android 5 situation? If so, is there a different API we need to call?

BiometricPrompt API is generally available as of Android 9, however we use its wrapper from AndroidX which provides backwards compatibility for older versions of Android, where it just uses FingerprintManager under the hood. So you don't need to worry about older versions, it should work in the same way 🙂

@awinograd
Copy link
Contributor Author

Thanks for the info, @tsapeta.

I think you're right that LocalAuthentication.hasHardwareAsync() is the equivalent of AuthenticationTypes.BIOMETRIC. I dont see it adding any value now.

I do think there's value in knowing what type of sensor is available on the device to adapt the UI/text accordingly, but if Android APIs don't allow for that then is sounds like we're out of luck!

@byCedric
Copy link
Member

byCedric commented Apr 17, 2020

Thanks @tsapeta and others for taking a deep dive into this. As addition I also want to mention a couple of things:

  • Right now the Biometric API doesn't allow us to determine what type of auth is available. Luckily, we are not the only one with this problem and others have opened an issue at Google. We could try adding some additional comments why this is required there? (See this and that)
  • As for the Samsung S10 with face scan and without fingerprint; this looks like a deeper issue at Samsung itself. At the moment, these devices are blacklisted from using fingerprint with the Biometric API. I'm not sure why, but it looks like its related to some vendor specific security implications.

@beaur
Copy link
Contributor

beaur commented May 20, 2020

https://handstandsam.com/2020/01/03/android-biometrics-ux-guide-user-messaging/

This article seems to suggest it is possible to determine the sensors on Android using packageManager.hasFeature(PackageManager.FEATURE_FINGERPRINT)
packageManager.hasFeature(PackageManager.FEATURE_FACE)
packageManager.hasFeature(PackageManager.FEATURE_IRIS)

I think having these values for android under supportedAuthenticationTypesAsync would be much better than a generic "biometric" response.

@awinograd
Copy link
Contributor Author

I believe this issue is fully addressed by #8431. I'm running expo-local-auth 9.2.0 and types are returning as expected on android now

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
SDK 38
  
Done
Development

Successfully merging a pull request may close this issue.

6 participants