[CP Staging] Integrate new native biometrics library#86310
[CP Staging] Integrate new native biometrics library#86310chuckdries merged 44 commits intoExpensify:mainfrom
Conversation
|
|
b49e604 to
85c2a02
Compare
8acc1b2 to
5b02e71
Compare
|
Let's get this ready for review 🙏 |
rafecolton
left a comment
There was a problem hiding this comment.
A few requests please:
- Merge Expensify/App
main - Add comments to your tests per https://github.com/Expensify/App/tree/main/tests#documenting-tests
- Address all CI failures
src/components/MultifactorAuthentication/biometrics/useBiometrics/index.native.ts
Outdated
Show resolved
Hide resolved
src/components/MultifactorAuthentication/biometrics/useNativeBiometricsEC256.ts
Outdated
Show resolved
Hide resolved
src/components/MultifactorAuthentication/biometrics/useNativeBiometricsHSM.ts
Show resolved
Hide resolved
src/components/MultifactorAuthentication/biometrics/useNativeBiometricsHSM.ts
Outdated
Show resolved
Hide resolved
src/components/MultifactorAuthentication/biometrics/useNativeBiometricsHSM.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsHSM/helpers.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsEC256/VALUES.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsEC256/VALUES.ts
Outdated
Show resolved
Hide resolved
dariusz-biela
left a comment
There was a problem hiding this comment.
I'm posting my 1/3 review so we can start working on it sooner
src/components/MultifactorAuthentication/biometrics/useNativeBiometricsHSM.ts
Outdated
Show resolved
Hide resolved
src/components/MultifactorAuthentication/biometrics/useNativeBiometricsHSM.ts
Outdated
Show resolved
Hide resolved
src/components/MultifactorAuthentication/biometrics/useNativeBiometricsHSM.ts
Outdated
Show resolved
Hide resolved
src/components/MultifactorAuthentication/biometrics/useNativeBiometricsHSM.ts
Show resolved
Hide resolved
| const {authenticationMethod} = useMultifactorAuthenticationState(); | ||
|
|
||
| const authType = translate(AUTH_TYPE_TRANSLATION_KEY[authenticationMethod?.name ?? SECURE_STORE_VALUES.AUTH_TYPE.UNKNOWN.NAME]); | ||
| const authType = translate(AUTH_TYPE_TRANSLATION_KEY[authenticationMethod?.name ?? NATIVE_BIOMETRICS_EC256_VALUES.AUTH_TYPE.UNKNOWN.NAME]); |
There was a problem hiding this comment.
This change means we can't switch freely between the old and new implementations.
This isn't necessarily a bad thing, provided we stop supporting the old biometrics after this PR is merged.
Otherwise, we'll have to do something about it.
src/libs/MultifactorAuthentication/NativeBiometricsHSM/helpers.ts
Outdated
Show resolved
Hide resolved
dariusz-biela
left a comment
There was a problem hiding this comment.
I've completed 2/3 of the review; I still have some tests left to go over.
src/libs/MultifactorAuthentication/NativeBiometricsHSM/helpers.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsEC256/helpers.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsHSM/helpers.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsHSM/helpers.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsEC256/helpers.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsHSM/helpers.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsHSM/helpers.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsHSM/helpers.ts
Outdated
Show resolved
Hide resolved
src/libs/MultifactorAuthentication/NativeBiometricsHSM/helpers.ts
Outdated
Show resolved
Hide resolved
|
|
||
| const doesDeviceSupportAuthenticationMethod = useCallback(() => { | ||
| const sensorResult = getSensorResult(); | ||
| return sensorResult.isDeviceSecure ?? sensorResult.available; |
There was a problem hiding this comment.
isDeviceSecure can be false, but it’s rare.
| return sensorResult.isDeviceSecure ?? sensorResult.available; | |
| return sensorResult.isDeviceSecure || sensorResult.available; |
There was a problem hiding this comment.
I believe we can actually use just the .isDeviceSecure, as it tells us whether the device has a secure lock screen set up (e.g. PIN/passcode and not just swipe).
As far as I know, it's not possible to have biometrics set up without a PIN or other device credentials, so .isDeviceSecure covers everything (while .available only tells us whether the device has biometrics enrolled).
Currently the type signature of .isDeviceSecure is misleading (boolean | undefined - however it's always a boolean, this is fixed in the next release of the library that we're waiting for anyways), so I added ?? .available for now, for the type compliance
src/components/MultifactorAuthentication/biometrics/useNativeBiometricsHSM.ts
Outdated
Show resolved
Hide resolved
… fixed one naming problem; removed unnecessary mapping function
|
I had to leave for today, will get back to resolving the remaining comments/checks in the morning |
rafecolton
left a comment
There was a problem hiding this comment.
It looks like this change breaks web. React-native-biometrics is native-only and we rely on importing AuthType for web. We need to either stop relying on AuthType and hardcode the constants or, probably better, add basic support upstream for those constants in web.
More discussion here
dariusz-biela
left a comment
There was a problem hiding this comment.
Very good job with this PR. The code looks great. My simulator tests also showed no issues.
@chuckdries I'll leave this PR to you now.
|
🚧 @chuckdries has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
| keyAlias, | ||
| data: dataToSignB64, | ||
| inputEncoding: InputEncoding.Base64, | ||
| promptTitle: translate('multifactorAuthentication.letsVerifyItsYou'), |
There was a problem hiding this comment.
I'm not going to block this PR on it, but as a follow-up we should pass the promptSubtitle parameter here. Currently, react-native-biometrics shows the android system prompt with the subtitle "Please verify your identity to generate signature", which I feel like the user doesn't need to know or care about
Rafe is OOO, I am taking over to get this over the line
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppnative.biometrics.android.mp4Android: mWeb Chromepasskeys.still.work.android.chrome.mp4iOS: HybridAppnative.biometrics.ios.mp4iOS: mWeb Safaripasskeys.still.work.ios.safari.mp4MacOS: Chrome / Safaripasskeys.still.work.mac.chrome.mp4 |
|
🚧 @chuckdries has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
…e-new-biometry-lib-PoC Integrate new native biometrics library (cherry picked from commit 8b20dd3) (cherry-picked to staging by jasperhuangg)
|
🚀 Cherry-picked to staging by https://github.com/jasperhuangg in version: 9.3.52-6 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This PR swaps the underlying native biometrics library (to I reviewed the relevant help site articles:
None of these articles reference biometric authentication, and there is no existing help article specifically about the biometrics feature. Since this PR only changes the internal implementation (library swap, HSM key management, error handling) with no impact on UI labels, settings names, or user-facing flows, no documentation updates are needed. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.52-9 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/jasperhuangg in version: 9.3.53-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. The changes in this PR are purely internal/technical — switching the native biometrics implementation from the existing keystore-based approach to a new HSM-based EC256 key approach using I searched all files under |
|
🚀 Cherry-picked to staging by https://github.com/jasperhuangg in version: 9.3.53-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.53-7 🚀
|
Explanation of Change
This PR adds
useNativeBiometricsEC256hook which uses the@sbaiahmed1/react-native-biometricslibrary for registration and authentication in compliance with theUseBiometricReturninterface.In case of need to revert to previous implementation:
useNativeBiometricsHSMwithuseNativeBiometricsin:src/components/MultifactorAuthentication/biometrics/useBiometrics/index.native.tsCONST.MULTIFACTOR_AUTHENTICATION.TYPE.BIOMETRICS_HSMwithCONST.MULTIFACTOR_AUTHENTICATION.TYPE.BIOMETRICSin:src/components/MultifactorAuthentication/config/scenarios/AuthorizeTransaction.tsxsrc/components/MultifactorAuthentication/config/scenarios/BiometricsTest.tsxsrc/components/MultifactorAuthentication/config/scenarios/ChangePIN.tsxsrc/components/MultifactorAuthentication/config/scenarios/RevealPIN.tsxsrc/components/MultifactorAuthentication/config/scenarios/SetPINOrderCard.tsxCONST.MULTIFACTOR_AUTHENTICATION.REASON.HSM.KEY_ACCESS_FAILED || result.reason === CONST.MULTIFACTOR_AUTHENTICATION.REASON.HSM.KEY_NOT_FOUNDwithCONST.MULTIFACTOR_AUTHENTICATION.REASON.KEYSTORE.REGISTRATION_REQUIREDin:src/components/MultifactorAuthentication/Context/Main.tsxNATIVE_BIOMETRICS_HSM_VALUESwithSECURE_STORE_VALUESin:src/libs/MultifactorAuthentication/shared/types.tssrc/components/MultifactorAuthentication/components/AuthenticationMethodDescription.tsxFixed Issues
$ #81912
PROPOSAL:
Tests
Testbutton next to itGot itGot itand verify that the registration succeeded - 'Biometrics (registered)' is displayed and aRevokebutton is shown.Offline tests
N/A
QA Steps
Same as Test
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-04-03.at.19.05.02.mov
Screen.Recording.2026-04-03.at.19.05.32.mov
Screen.Recording.2026-04-03.at.19.08.14.mov
Screen.Recording.2026-04-03.at.19.08.42.mov
Android: mWeb Chrome
iOS: Native
Screen.Recording.2026-04-03.at.20.32.00.mov
Screen.Recording.2026-04-03.at.20.36.12.mov
Screen.Recording.2026-04-03.at.20.37.44.mov
iOS: mWeb Safari
MacOS: Chrome / Safari