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.

Sign up for GitHub

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

Jump to bottom

[expo-local-authentication] Refactor is/has/supported-functions #7032

Closed
LinusU opened this issue Feb 12, 2020 · 7 comments
Closed

[expo-local-authentication] Refactor is/has/supported-functions #7032

LinusU opened this issue Feb 12, 2020 · 7 comments
Labels
LocalAuthentication

Comments

@LinusU
Copy link
Contributor

LinusU commented Feb 12, 2020

Stemming from discussions here: #6962 (comment)

My proposal is something like:

  • "hardware supported level" -> 0=None, 1=Secret, 2=Biometric
  • "enrolled level" -> 0=None, 1=Secret, 2=Biometric

That way the programmer can easy decide which level is acceptable for their app ☺️

This would thus replace hasHardwareAsync, supportedAuthenticationTypesAsync and isEnrolledAsync with two new functions.

I'd be happy to submit the PR once/if we decide on an appropriate API ☺️
@LinusU
Copy link
Contributor Author

LinusU commented Feb 12, 2020

ping @tsapeta @SimenB

@LinusU LinusU mentioned this issue Feb 12, 2020
Merged
@tsapeta
Copy link
Member

tsapeta commented Feb 19, 2020 •
edited

Sounds good to me, would you like to work on this?
Also I think we may want to leave those three functions as deprecated at least for one release cycle and warn they're gonna be removed soon.

@LinusU
Copy link
Contributor Author

LinusU commented Feb 19, 2020

would you like to work on this?

Yes!

leave those three functions as deprecated at least for one release cycle and warn they're gonna be removed soon.

Sounds good 👍

Any ideas on naming? My first suggestion is below, but let's discuss it!

  • getHardwareLevelAsync(): Promise<0 | 1 | 2>
  • getEnrolledLevelAsync(): Promise<0 | 1 | 2>
  • NONE = 0
  • SECRET = 1
  • BIOMETRIC = 2

The reason I think it's good to return integers is that we can do the checks as below easily:

const enrolled = await getEnrolledLevelAsync()

if (enrolled > NONE) {
 // has secret, biometric, or a newer system
}

if (enrolled < BIOMETRIC) {
  // require biometric or stronger authentication
}

@SimenB
Copy link
Contributor

SimenB commented Feb 20, 2020

Somewhat related - https://developer.android.com/preview/features#biometric-auth

@mickamy
Copy link
Contributor

mickamy commented Jan 29, 2021

@LinusU
Thanks for your great work on this!
I'm just wondering if this feature is being working on or not, for it seems like there is no related issues nor PRs.
I am happily implement this feature if your too busy to work on this :)

@mickamy mickamy mentioned this issue Jan 29, 2021
Merged
@mickamy
Copy link
Contributor

mickamy commented Jan 29, 2021

@LinusU
I submitted a draft PR #11780.
I followed your API design proposal here.
Sorry if I offend you 🙏

lukmccall pushed a commit that referenced this issue Feb 10, 2021
@mickamy
[expo-local-authentication] Add method to know enrolled security leve…
f793f04 
…l of device (#11780)

# Why

<!-- 
Please describe the motivation for this PR, and link to relevant GitHub issues, forums posts, or feature requests. 
-->

Some apps wants to know local authentication security level on a device.

Providing this information may be helpful for those
- who wants to advise a device owner to use more secure authentication
- who wants to check if any local authentication is been set (who does not care if it is biometric auth or whatever)

# How

<!-- 
How did you build this feature or fix this bug and why? 
-->

## API design

I basically followed to @LinusU's API proposal at #7032 (comment).
`getEnrolledLevelAsync` returns enum representing security level `NONE`, `SECRET` and `BIOMETRIC`, which seems very reasonsable.
Big up to @LinusU 🎉 

## iOS

I used [`LAPolicyDeviceOwnerAuthentication`](https://developer.apple.com/documentation/localauthentication/lapolicy/lapolicydeviceownerauthentication?language=objc) to know if any authentication is available. And [`LAPolicyDeviceOwnerAuthenticationWithBiometrics`](https://developer.apple.com/documentation/localauthentication/lapolicy/lapolicydeviceownerauthenticationwithbiometrics?language=objc) for biometrics.

## Android

### for SDK_VERSION >= M

- To detect if device has an enrolled authentication, I used [`KeyguardDeviceManager#isDeviceSecure()`](https://developer.android.com/reference/android/app/KeyguardManager#isDeviceSecure()), which is the very method recommended in [BiometricPrompt documentation](https://developer.android.com/reference/androidx/biometric/BiometricPrompt.PromptInfo.Builder#setDeviceCredentialAllowed(boolean)).
- To detect if device has an enrolled biometric authentication, I used [`BiometricManager#canAuthenticate()`](https://developer.android.com/reference/androidx/biometric/BiometricManager#canAuthenticate()). It is same on prior to M.

### for SDK_VERSION < M

Sadly, `KeyguardDeviceManager#isDeviceSecure()` is not supported on devices prior to Android M. The most similar API I found was [`isKeyguardSecure()`](https://developer.android.com/reference/android/app/KeyguardManager#isKeyguardSecure()), but it counts in SIM lock state, which is not very ideal. Because SIM lock is not where [`BiometricPrompt`](https://developer.android.com/reference/androidx/biometric/BiometricPrompt) authentication falls back to.
@LinusU
Copy link
Contributor Author

LinusU commented Mar 12, 2021

Sorry if I offend you 🙏

Absolutely not, I'm happy it got implemented! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
LocalAuthentication
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@LinusU @SimenB @tsapeta @lukmccall @mickamy @cruzach