fix: face unlock on devices that do not consider face to be level 3 #62
Conversation
WalkthroughReplaces biometry-type filtering with a requiredStrength model across platforms, adds STRONG/WEAK/NONE authentication detection and fallback handling, maps native BiometricManager/LocalAuth errors to plugin error codes, and updates examples, web shim, types, and docs to use AuthenticationStrength and BiometricAuthError. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Example App / UI
participant JS as Plugin JS
participant Native as NativeBiometric (Android/iOS)
participant BM as BiometricManager / LocalAuth
App->>JS: isAvailable({ useFallback })
JS->>Native: isAvailable(useFallback)
activate Native
Native->>BM: canAuthenticate(BIOMETRIC_STRONG)
alt Strong available
BM-->>Native: SUCCESS
Native-->>JS: { isAvailable: true, authenticationStrength: STRONG }
else Strong unavailable
BM-->>Native: FAIL/ERROR
alt useFallback == true
Native->>BM: canAuthenticate(BIOMETRIC_WEAK)
alt Weak available
BM-->>Native: SUCCESS
Native-->>JS: { isAvailable: true, authenticationStrength: WEAK }
else Weak unavailable
Native->>BM: canAuthenticate(DEVICE_CREDENTIAL)
BM-->>Native: SUCCESS/FAIL/ERROR
Native-->>JS: { isAvailable: maybe, authenticationStrength: NONE/WEAK, errorCode }
end
else
Native-->>JS: { isAvailable: false, authenticationStrength: NONE, errorCode }
end
end
deactivate Native
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-02-13T20:05:24.035ZApplied to files:
🧬 Code graph analysis (1)android/src/main/java/ee/forgr/biometric/NativeBiometric.java (2)
🪛 GitHub Actions: Build source code and test itandroid/src/main/java/ee/forgr/biometric/NativeBiometric.java[error] 467-467: cannot find symbol: BiometricManager.BIOMETRIC_ERROR_LOCKOUT_PERMANENT [error] 473-473: cannot find symbol: BiometricManager.BIOMETRIC_ERROR_LOCKOUT 🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (1)
203-211: Unsafe cast from JSArray to int can crash.allowedBiometryTypes items from JS arrive as Number (Double) or String.
(int) allowedTypes.toList().get(i)can throw ClassCastException.Apply:
- for (int i = 0; i < allowedTypes.length(); i++) { - types[i] = (int) allowedTypes.toList().get(i); - } + for (int i = 0; i < allowedTypes.length(); i++) { + Object v = allowedTypes.toList().get(i); + if (v instanceof Number) { + types[i] = ((Number) v).intValue(); + } else if (v instanceof String) { + try { + types[i] = Integer.parseInt((String) v); + } catch (NumberFormatException ignore) { + // skip invalid entry + } + } + }
🧹 Nitpick comments (5)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (3)
136-153: Availability flow is reasonable; consider not overriding BiometricManager’s error.When useFallback is true but the device has no credentials, you force canAuthenticateResult = BIOMETRIC_ERROR_HW_UNAVAILABLE. That can mask more accurate statuses (e.g., NONE_ENROLLED).
Suggestion: keep canAuthenticateResult as returned by BiometricManager and use the separate fallbackAvailable flag to compute isAvailable. This preserves error fidelity.
If you prefer a distinct “no device credential” signal, introduce a plugin‑side code (e.g., 14) for this path to mirror BiometricPrompt.ERROR_NO_DEVICE_CREDENTIAL semantics.
169-171: Error-code mapping: verify SECURITY_UPDATE_REQUIRED and add UNKNOWN mapping.
- SECURITY_UPDATE_REQUIRED mapped to USER_LOCKOUT (2) seems semantically wrong; consider BIOMETRICS_UNAVAILABLE or a dedicated “security update required” plugin code.
- Add a case for BIOMETRIC_STATUS_UNKNOWN to return UNKNOWN_ERROR (0) explicitly.
Would you like me to align these mappings with the README error table and iOS parity?
Also applies to: 471-489
173-175: Type reporting may say “Biometrics Available (None)” in fallback-only cases.When only device credentials are available, isAvailable is true but biometryType is NONE. Consider exposing a second flag like hasBiometrics to avoid confusing UIs.
example-app/src/js/biometric-tester.js (1)
244-261: Clarify UI when only device credentials are available.Avoid “Biometrics Available (None)” wording in fallback‑only cases.
Apply:
- if (result.isAvailable) { - elements.biometricStatus.textContent = `✅ Biometrics Available (${getBiometryTypeName(result.biometryType)})`; + if (result.isAvailable) { + const onlyDc = result.biometryType === 0 && useFallback; + const label = onlyDc ? 'Device Credential Only' : getBiometryTypeName(result.biometryType); + elements.biometricStatus.textContent = `✅ ${onlyDc ? 'Authentication Available' : 'Biometrics Available'} (${label})`; elements.biometricStatus.className = 'status available'; elements.biometricInfo.style.display = 'block'; elements.biometricInfo.innerHTML = `<strong>Biometry Type:</strong> ${getBiometryTypeName(result.biometryType)} (${result.biometryType})`; } else {android/src/main/java/ee/forgr/biometric/AuthActivity.java (1)
159-179: FACE → STRONG|WEAK approach is verified; consider optional DEVICE_CREDENTIAL fallback logic.AndroidX Biometric 1.1.0 supports
BiometricManager.Authenticators.BIOMETRIC_WEAKinsetAllowedAuthenticators()andcanAuthenticate(int). The OR'ing of BIOMETRIC_STRONG with BIOMETRIC_WEAK for face authentication is a valid approach for Samsung weak-face device compatibility.Optional: If you want face-only authentication with device credential fallback when
useFallbackis true, consider adding logic to ORDEVICE_CREDENTIALwhenallowedTypesis provided, rather than always relying on the fallback parameter alone.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
android/src/main/java/ee/forgr/biometric/AuthActivity.java(2 hunks)android/src/main/java/ee/forgr/biometric/NativeBiometric.java(4 hunks)example-app/src/js/biometric-tester.js(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: below43
PR: Cap-go/capacitor-native-biometric#41
File: android/src/main/java/ee/forgr/biometric/AuthActivity.java:75-82
Timestamp: 2025-02-13T20:05:24.035Z
Learning: In android/src/main/java/ee/forgr/biometric/AuthActivity.java, the negativeButtonText must be set regardless of the useFallback value to prevent Android errors when negativeButtonText is null.
📚 Learning: 2025-02-13T20:05:24.035Z
Learnt from: below43
PR: Cap-go/capacitor-native-biometric#41
File: android/src/main/java/ee/forgr/biometric/AuthActivity.java:75-82
Timestamp: 2025-02-13T20:05:24.035Z
Learning: In android/src/main/java/ee/forgr/biometric/AuthActivity.java, the negativeButtonText must be set regardless of the useFallback value to prevent Android errors when negativeButtonText is null.
Applied to files:
android/src/main/java/ee/forgr/biometric/AuthActivity.java
🧬 Code graph analysis (1)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (1)
src/web.ts (1)
isAvailable(17-19)
🔇 Additional comments (5)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (1)
73-79: Signature change looks good.Passing useFallback into getAvailableFeature keeps availability/type detection consistent with the new flow.
android/src/main/java/ee/forgr/biometric/AuthActivity.java (2)
48-57: Good separation of authenticators.leftAuthenticators correctly folds in DEVICE_CREDENTIAL only when requested and applies allowed types filtering before building the prompt.
58-65: Negative button handling is correct.Setting negative text only when DEVICE_CREDENTIAL is not allowed avoids builder exceptions and ensures a non‑null label. This also aligns with our prior learning about null negativeButtonText causing Android errors. Based on learnings.
Please confirm no paths build PromptInfo without a negative button when DEVICE_CREDENTIAL is absent.
example-app/src/js/biometric-tester.js (2)
117-123: Nice UX addition for fallback toggle.The checkbox wiring cleanly exercises the new isAvailable({ useFallback }) path.
235-237: Face-only path is correctly limited.Passing [BiometryType.FACE_AUTHENTICATION] ensures Android uses face (weak or strong) where available.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (1)
97-114: Face detection remains incorrectly classified (critical issue from previous review).This is the same issue flagged in the previous review. Checking
canAuthenticate(BIOMETRIC_WEAK)will returnBIOMETRIC_SUCCESSon any device with strong fingerprint support, because strong biometrics satisfy weak requirements. This causes fingerprint-only devices to incorrectly report face authentication support.Face presence must be determined exclusively by
PackageManager.FEATURE_FACE. Remove lines 104-114.Apply this diff:
- // If fallback is enabled, also check for weak face authentication (like Samsung) - if (useFallback && !hasFace) { - try { - int faceWeakTest = biometricManager.canAuthenticate(BiometricManager.Authenticators.BIOMETRIC_WEAK); - if (faceWeakTest == BiometricManager.BIOMETRIC_SUCCESS) { - hasFace = true; - } - } catch (Exception e) { - System.out.println("Error checking face authentication: " + e.getMessage()); - } - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (1)
src/web.ts (1)
isAvailable(17-19)
🔇 Additional comments (2)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (2)
146-154: Weak biometric fallback logic looks sound for capability detection.The logic here correctly checks whether the device can authenticate using weak biometrics and device credentials. This is appropriate for determining authentication capability (as opposed to determining which specific biometric types are available).
491-509: Verify the SECURITY_UPDATE_REQUIRED error mapping.The mapping of
BIOMETRIC_ERROR_SECURITY_UPDATE_REQUIREDto error code2(USER_LOCKOUT) seems semantically incorrect. A security update requirement is a different condition than a user lockout.Consider mapping this to
BIOMETRICS_UNAVAILABLE(1) instead, or introducing a distinct error code if the plugin supports it.Can you verify the intended error mapping for security update requirements? You may want to check the plugin's error code definitions and client expectations.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (3)
75-86: Fallback logic still conflates capability with modality detection.Using
canAuthenticate(BIOMETRIC_WEAK)to check availability is reasonable, but this result then flows into type detection logic (lines 88-126) where it influenceshasFace,hasFinger, etc. The previous review correctly identified that capability checks (can authenticate?) should be separated from modality detection (which biometric types are present?).Modality should rely exclusively on
PackageManager.hasSystemFeatureflags, not oncanAuthenticateresults.
97-114: Face detection is still misclassified when any weak biometric is available.Lines 104-114 set
hasFace = truewhencanAuthenticate(BIOMETRIC_WEAK)returns SUCCESS. However, BIOMETRIC_WEAK succeeds on devices with strong fingerprint (since strong biometrics satisfy weak checks). This causes fingerprint-only devices to be incorrectly reported as having face authentication.The previous review identified this exact issue. Biometric type must be determined exclusively by system feature flags (
PackageManager.FEATURE_FACE), not by capability checks.Remove the fallback block that infers face from BIOMETRIC_WEAK:
- // If fallback is enabled, also check for weak face authentication (like Samsung) - if (useFallback && !hasFace) { - try { - int faceWeakTest = biometricManager.canAuthenticate(BiometricManager.Authenticators.BIOMETRIC_WEAK); - if (faceWeakTest == BiometricManager.BIOMETRIC_SUCCESS) { - hasFace = true; - } - } catch (Exception e) { - System.out.println("Error checking face authentication: " + e.getMessage()); - } - }
175-175: Biometry type will be incorrect due to flawed face detection.This line returns the result from
getAvailableFeature(useFallback), which contains flawed face-detection logic (lines 104-114) that misclassifies fingerprint-only devices as having face authentication.Once lines 104-114 are corrected to detect face exclusively via
FEATURE_FACE, this line will return accurate biometry types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-13T20:05:24.035Z
Learnt from: below43
PR: Cap-go/capacitor-native-biometric#41
File: android/src/main/java/ee/forgr/biometric/AuthActivity.java:75-82
Timestamp: 2025-02-13T20:05:24.035Z
Learning: In android/src/main/java/ee/forgr/biometric/AuthActivity.java, the negativeButtonText must be set regardless of the useFallback value to prevent Android errors when negativeButtonText is null.
Applied to files:
android/src/main/java/ee/forgr/biometric/NativeBiometric.java
🧬 Code graph analysis (1)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (1)
ios/Sources/NativeBiometricPlugin/NativeBiometricPlugin.swift (1)
isAvailable(38-72)
🔇 Additional comments (3)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (3)
146-154: Availability check is reasonable but type detection remains flawed.Using
BIOMETRIC_WEAKto determine availability (can any biometric authenticate?) is appropriate. However, at line 175,biometryTypeis computed usinggetAvailableFeature(useFallback), which contains flawed face-detection logic (lines 104-114) that will misreport types.Once the face detection logic is corrected to rely solely on
FEATURE_FACE, this availability check will work correctly.
161-164: LGTM: Error handling for unavailable fallback.The logic correctly marks authentication as unavailable when fallback is requested but not available and no biometrics are present.
170-172: Good refactor: Centralized error mapping.Extracting error code conversion into a dedicated helper method improves maintainability.
…apacitor-native-biometric into fix_face_unlock_on_weak_devices
|
@coderabbitai - can you check the comments you posted and see if they still apply after the latest commit? |
🧩 Analysis chain🏁 Script executed: Length of output: 15 🏁 Script executed: Length of output: 692 🏁 Script executed: Length of output: 84 @WcaleNieWolny, I checked the PR and found that I haven't posted any comments or reviews on PR #62 yet. There are no previous comments from me to verify against the latest commit. Would you like me to conduct a fresh review of the changes in this PR? If so, just let me know and I'll review the code changes, or you can use the `` tag to trigger an automatic review. ((\ ✅ Actions performedReview triggered.
|
|
@coderabbitai - fine, rereview this PR :) |
|
@WcaleNieWolny, I'll conduct a complete review of this PR for you! Let me go through all the changes related to the face unlock fix on devices that don't consider face authentication as level 3 biometric. 🔍 ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ios/Sources/NativeBiometricPlugin/NativeBiometricPlugin.swift (1)
55-80: Fallback error is dropped; wrong errorCode returned.If
useFallbackis true and the passcode/PIN check fails (e.g.,LAError.passcodeNotSet),deviceErrorcaptures that failure but we continue to surface the earlier biometric error. The promise resolves witherrorCodedescribing biometrics instead of the missing credentials, breaking the newBiometricAuthError.PASSCODE_NOT_SETcontract.Please carry the fallback error forward before the final guard.
- if useFallback { - let devicePolicy = LAPolicy.deviceOwnerAuthentication - var deviceError: NSError? - hasDeviceCredentials = context.canEvaluatePolicy(devicePolicy, error: &deviceError) - } + if useFallback { + let devicePolicy = LAPolicy.deviceOwnerAuthentication + var deviceError: NSError? + hasDeviceCredentials = context.canEvaluatePolicy(devicePolicy, error: &deviceError) + if !hasDeviceCredentials, let deviceError { + error = deviceError + } + }README.md (1)
22-46: Update sample to useauthenticationStrength.The top-level example still imports
BiometryTypeand readsresult.biometryType, which no longer exists after this PR. Anyone copying the snippet will hit a TS error at build time.Please switch the import/usage to
AuthenticationStrengthandresult.authenticationStrength(with the new helper) to keep the README accurate.
♻️ Duplicate comments (2)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (2)
109-118: Stop inferring face from generic WEAK success.This reintroduces the misclassification we discussed earlier:
BIOMETRIC_STRONGsensors also satisfyBIOMETRIC_WEAK, so fingerprint‑only devices will now be reported as having face auth onceuseFallbackis true.Please drop the weak-canAuthenticate fallback when determining
hasFace; rely solely onFEATURE_FACE.- if (useFallback && !hasFace) { - try { - int faceWeakTest = biometricManager.canAuthenticate(BiometricManager.Authenticators.BIOMETRIC_WEAK); - if (faceWeakTest == BiometricManager.BIOMETRIC_SUCCESS) { - hasFace = true; - } - } catch (Exception e) { - System.out.println("Error checking face authentication: " + e.getMessage()); - } - } + // Keep face detection bound to the platform feature flag only.
523-525: Map SECURITY_UPDATE_REQUIRED to biometrics unavailable.
BIOMETRIC_ERROR_SECURITY_UPDATE_REQUIREDmeans the sensor is disabled until a security patch lands; it is not a user lockout. Returning 2 misleads consumers about why auth failed.Switch this case to return 1 (
BIOMETRICS_UNAVAILABLE).- case BiometricManager.BIOMETRIC_ERROR_SECURITY_UPDATE_REQUIRED: - return 2; // USER_LOCKOUT + case BiometricManager.BIOMETRIC_ERROR_SECURITY_UPDATE_REQUIRED: + return 1; // BIOMETRICS_UNAVAILABLE
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(3 hunks)android/src/main/java/ee/forgr/biometric/AuthActivity.java(1 hunks)android/src/main/java/ee/forgr/biometric/NativeBiometric.java(6 hunks)example-app/.gitignore(1 hunks)example-app/simple-test.html(3 hunks)example-app/src/js/biometric-tester.js(9 hunks)ios/Sources/NativeBiometricPlugin/NativeBiometricPlugin.swift(2 hunks)src/definitions.ts(4 hunks)src/web.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- example-app/.gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- android/src/main/java/ee/forgr/biometric/AuthActivity.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: below43
Repo: Cap-go/capacitor-native-biometric PR: 41
File: android/src/main/java/ee/forgr/biometric/AuthActivity.java:75-82
Timestamp: 2025-02-13T20:05:24.035Z
Learning: In android/src/main/java/ee/forgr/biometric/AuthActivity.java, the negativeButtonText must be set regardless of the useFallback value to prevent Android errors when negativeButtonText is null.
📚 Learning: 2025-02-13T20:05:24.035Z
Learnt from: below43
Repo: Cap-go/capacitor-native-biometric PR: 41
File: android/src/main/java/ee/forgr/biometric/AuthActivity.java:75-82
Timestamp: 2025-02-13T20:05:24.035Z
Learning: In android/src/main/java/ee/forgr/biometric/AuthActivity.java, the negativeButtonText must be set regardless of the useFallback value to prevent Android errors when negativeButtonText is null.
Applied to files:
android/src/main/java/ee/forgr/biometric/NativeBiometric.java
🧬 Code graph analysis (2)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (2)
ios/Sources/NativeBiometricPlugin/NativeBiometricPlugin.swift (1)
isAvailable(38-83)src/web.ts (1)
isAvailable(19-24)
example-app/src/js/biometric-tester.js (2)
ios/Sources/NativeBiometricPlugin/NativeBiometricPlugin.swift (1)
verifyIdentity(85-125)src/web.ts (1)
verifyIdentity(26-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_android
🔇 Additional comments (1)
example-app/src/js/biometric-tester.js (1)
271-287: Strength-aware verification flow looks correct.
Conditionally addingrequiredStrengthonly when the user picks a strength keeps the default verification behavior unchanged while letting us exercise the new weak/strong paths in the tester. Nice balance between backwards compatibility and new coverage.
…apacitor-native-biometric into fix_face_unlock_on_weak_devices
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (1)
109-119: Face detection is still misclassified when any weak biometric is available.This is the same critical issue flagged in previous reviews. Setting
hasFace = truebecausecanAuthenticate(BIOMETRIC_WEAK) == SUCCESSwill incorrectly report face on fingerprint-only devices, since strong fingerprint also satisfies the weak check.Per previous review guidance, rely exclusively on
FEATURE_FACEfor face detection:- // If fallback is enabled, also check for weak face authentication (like Samsung) - if (useFallback && !hasFace) { - try { - int faceWeakTest = biometricManager.canAuthenticate(BiometricManager.Authenticators.BIOMETRIC_WEAK); - if (faceWeakTest == BiometricManager.BIOMETRIC_SUCCESS) { - hasFace = true; - } - } catch (Exception e) { - System.out.println("Error checking face authentication: " + e.getMessage()); - } - } + // Do not infer face presence from BIOMETRIC_WEAK; fingerprint (STRONG) also satisfies WEAK. + // Only trust the system feature flag FEATURE_FACE for face availability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: below43
Repo: Cap-go/capacitor-native-biometric PR: 41
File: android/src/main/java/ee/forgr/biometric/AuthActivity.java:75-82
Timestamp: 2025-02-13T20:05:24.035Z
Learning: In android/src/main/java/ee/forgr/biometric/AuthActivity.java, the negativeButtonText must be set regardless of the useFallback value to prevent Android errors when negativeButtonText is null.
📚 Learning: 2025-02-13T20:05:24.035Z
Learnt from: below43
Repo: Cap-go/capacitor-native-biometric PR: 41
File: android/src/main/java/ee/forgr/biometric/AuthActivity.java:75-82
Timestamp: 2025-02-13T20:05:24.035Z
Learning: In android/src/main/java/ee/forgr/biometric/AuthActivity.java, the negativeButtonText must be set regardless of the useFallback value to prevent Android errors when negativeButtonText is null.
Applied to files:
android/src/main/java/ee/forgr/biometric/NativeBiometric.java
🧬 Code graph analysis (1)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (2)
src/web.ts (1)
isAvailable(19-24)ios/Sources/NativeBiometricPlugin/NativeBiometricPlugin.swift (1)
isAvailable(38-83)
🪛 GitHub Actions: Build source code and test it
android/src/main/java/ee/forgr/biometric/NativeBiometric.java
[error] 523-523: cannot find symbol: BiometricManager.BIOMETRIC_ERROR_LOCKOUT_PERMANENT
[error] 529-529: cannot find symbol: BiometricManager.BIOMETRIC_ERROR_LOCKOUT
🔇 Additional comments (4)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (4)
64-67: LGTM! Authentication strength constants align with cross-platform API.The constants are clear and consistent with the iOS implementation.
144-174: Well-designed separation of strong/weak biometric detection.The logic correctly checks strong and weak biometrics independently, then determines authentication strength based on availability. The distinction between biometric-backed (STRONG/WEAK) and credential-based (WEAK) authentication is clear.
230-236: LGTM! Required strength parameter properly wired.The parameter is correctly passed to AuthActivity with appropriate null handling.
176-196: Error handling logic is sound.The error prioritization (strong → weak → default) is correct. However, ensure the
convertBiometricManagerErrorToPluginErrormethod is fixed first to properly map error codes.
|
@WcaleNieWolny, I'll conduct a complete review of all the changes in this PR! 🔍 ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/Sources/NativeBiometricPlugin/NativeBiometricPlugin.swift (1)
53-81: Propagate fallback errors instead of always reporting biometric failuresWhen
useFallbackis true and the device lacks both enrolled biometrics and a passcode/PIN,context.canEvaluatePolicyfor biometrics fails first (settingerrorto something likeLAError.biometryNotAvailable). We then check device credentials, but the localdeviceErrorgoes out of scope and we ignore it. The final branch therefore always reports the biometric error code, masking situations such asLAError.passcodeNotSet, so callers never learn why fallback failed. Please keep the fallback error around and prefer it when present.- if useFallback { + var fallbackError: NSError? + if useFallback { let devicePolicy = LAPolicy.deviceOwnerAuthentication - var deviceError: NSError? - hasDeviceCredentials = context.canEvaluatePolicy(devicePolicy, error: &deviceError) + hasDeviceCredentials = context.canEvaluatePolicy(devicePolicy, error: &fallbackError) } … - guard let authError = error else { + let finalError = fallbackError ?? error + guard let authError = finalError else { obj["errorCode"] = 0 call.resolve(obj) return } - var pluginErrorCode = convertToPluginErrorCode(authError.code) - obj["errorCode"] = pluginErrorCode + let pluginErrorCode = convertToPluginErrorCode(authError.code) + obj["errorCode"] = pluginErrorCode
♻️ Duplicate comments (2)
android/src/main/java/ee/forgr/biometric/NativeBiometric.java (2)
109-119: Face detection is still incorrectly inferred from weak biometric availability.This issue was previously flagged but the problematic code remains. Setting
hasFace = truewhencanAuthenticate(BIOMETRIC_WEAK)succeeds will misclassify fingerprint-only devices as having face authentication, since strong fingerprint also satisfies the weak biometric requirement.Remove the fallback block that infers face presence from
BIOMETRIC_WEAK. Face availability should be determined exclusively byPackageManager.FEATURE_FACE:- // If fallback is enabled, also check for weak face authentication (like Samsung) - if (useFallback && !hasFace) { - try { - int faceWeakTest = biometricManager.canAuthenticate(BiometricManager.Authenticators.BIOMETRIC_WEAK); - if (faceWeakTest == BiometricManager.BIOMETRIC_SUCCESS) { - hasFace = true; - } - } catch (Exception e) { - System.out.println("Error checking face authentication: " + e.getMessage()); - } - }
514-536: Compilation failure: BiometricManager lockout constants don't exist.The pipeline confirms that
BIOMETRIC_ERROR_LOCKOUTandBIOMETRIC_ERROR_LOCKOUT_PERMANENTareBiometricPromptconstants, notBiometricManagerconstants.BiometricManager.canAuthenticate()never returns these lockout codes—they are only reported during authentication viaBiometricPromptcallbacks inAuthActivity.Additionally, mapping
SECURITY_UPDATE_REQUIREDtoUSER_LOCKOUTis semantically incorrect. This error indicates the biometric sensor is blocked due to a security vulnerability, not user lockout.Apply this fix:
private int convertBiometricManagerErrorToPluginError(int errorCode) { switch (errorCode) { case BiometricManager.BIOMETRIC_ERROR_HW_UNAVAILABLE: case BiometricManager.BIOMETRIC_ERROR_NO_HARDWARE: return 1; // BIOMETRICS_UNAVAILABLE - case BiometricManager.BIOMETRIC_ERROR_LOCKOUT_PERMANENT: - return 2; // USER_LOCKOUT (permanent lockout) case BiometricManager.BIOMETRIC_ERROR_SECURITY_UPDATE_REQUIRED: - return 2; // USER_LOCKOUT + return 1; // BIOMETRICS_UNAVAILABLE case BiometricManager.BIOMETRIC_ERROR_NONE_ENROLLED: return 3; // BIOMETRICS_NOT_ENROLLED - case BiometricManager.BIOMETRIC_ERROR_LOCKOUT: - return 4; // USER_TEMPORARY_LOCKOUT (temporary lockout, 30 seconds) case BiometricManager.BIOMETRIC_ERROR_UNSUPPORTED: return 1; // BIOMETRICS_UNAVAILABLE default: return 0; // UNKNOWN_ERROR } }Note: Lockout states must be handled in
AuthActivity'sBiometricPrompt.AuthenticationCallback, where these error codes are actually produced.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(3 hunks)android/src/main/java/ee/forgr/biometric/AuthActivity.java(1 hunks)android/src/main/java/ee/forgr/biometric/NativeBiometric.java(6 hunks)example-app/.gitignore(1 hunks)example-app/simple-test.html(3 hunks)example-app/src/js/biometric-tester.js(9 hunks)ios/Sources/NativeBiometricPlugin/NativeBiometricPlugin.swift(2 hunks)src/definitions.ts(4 hunks)src/web.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: below43
Repo: Cap-go/capacitor-native-biometric PR: 41
File: android/src/main/java/ee/forgr/biometric/AuthActivity.java:75-82
Timestamp: 2025-02-13T20:05:24.035Z
Learning: In android/src/main/java/ee/forgr/biometric/AuthActivity.java, the negativeButtonText must be set regardless of the useFallback value to prevent Android errors when negativeButtonText is null.
📚 Learning: 2025-02-13T20:05:24.035Z
Learnt from: below43
Repo: Cap-go/capacitor-native-biometric PR: 41
File: android/src/main/java/ee/forgr/biometric/AuthActivity.java:75-82
Timestamp: 2025-02-13T20:05:24.035Z
Learning: In android/src/main/java/ee/forgr/biometric/AuthActivity.java, the negativeButtonText must be set regardless of the useFallback value to prevent Android errors when negativeButtonText is null.
Applied to files:
android/src/main/java/ee/forgr/biometric/NativeBiometric.javaandroid/src/main/java/ee/forgr/biometric/AuthActivity.java
🪛 GitHub Actions: Build source code and test it
android/src/main/java/ee/forgr/biometric/NativeBiometric.java
[error] 523-523: Compilation error: cannot find symbol BiometricManager.BIOMETRIC_ERROR_LOCKOUT_PERMANENT
[error] 529-529: Compilation error: cannot find symbol BiometricManager.BIOMETRIC_ERROR_LOCKOUT
[warning] 1-1: Gradle build produced warnings about obsolete Java 8 source/target compatibility; consider configuring Java toolchain or upgrading source/target compatibility.
android/src/main/java/ee/forgr/biometric/AuthActivity.java
[warning] 1-1: Gradle build produced warnings about obsolete Java 8 source/target compatibility; consider configuring Java toolchain or upgrading source/target compatibility.
🔇 Additional comments (8)
example-app/.gitignore (1)
1-8: Clean and comprehensive ignore patterns.The .gitignore file covers all standard categories for a Node.js/web project: IDE/editor settings, dependencies, build output, system files, and source maps. The patterns are appropriate and consistent with best practices.
src/definitions.ts (4)
18-33: LGTM! Well-documented authentication strength enum.The
AuthenticationStrengthenum is clearly defined with comprehensive documentation explaining the distinction between STRONG, WEAK, and NONE levels. The clarification that PIN/pattern/password is always WEAK (never STRONG) is particularly helpful for API consumers.
47-65: LGTM! Clear interface changes align with strength-based model.The
AvailableResultinterface properly replaces the previous biometry-type approach with the newauthenticationStrengthfield and uses theBiometricAuthErrorenum for better error handling.
89-95: LGTM! Clear documentation for the new strength-based option.The replacement of
allowedBiometryTypeswithrequiredStrengthis well-documented and aligns with the PR's objective to support devices that classify face authentication differently.
127-192: LGTM! Comprehensive error code documentation.The
BiometricAuthErrorenum provides detailed documentation for each error code, including platform availability. This will significantly improve the developer experience when handling authentication errors.android/src/main/java/ee/forgr/biometric/NativeBiometric.java (3)
64-67: LGTM! Constants align with TypeScript enum.The authentication strength constants are correctly defined and match the
AuthenticationStrengthenum values fromdefinitions.ts.
137-202: LGTM! Well-structured strength-based availability check.The refactored
isAvailablemethod properly:
- Separates strong and weak biometric checks
- Correctly determines authentication strength (STRONG for strong biometrics, WEAK for weak biometrics or PIN/pattern/password)
- Handles error codes appropriately when authentication is unavailable
- Prioritizes strong biometric errors over weak biometric errors in error reporting
This is a solid implementation of the strength-based authentication model.
230-236: LGTM! Proper replacement of biometry type filtering with strength-based filtering.The code correctly passes
requiredStrengthtoAuthActivityand handles the nullable integer appropriately. This aligns with the PR's objective to support strength-based authentication.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Documentation