Skip to content

Add biometric login + credential storage to native bridge#249

Open
sauravvarma wants to merge 1 commit into
mainfrom
feat/biometric-login
Open

Add biometric login + credential storage to native bridge#249
sauravvarma wants to merge 1 commit into
mainfrom
feat/biometric-login

Conversation

@sauravvarma

Copy link
Copy Markdown
Collaborator

iOS (full implementation):

  • New BiometricAuthHandler using LocalAuthentication + Keychain with kSecAttrAccessControl(.biometryCurrentSet) so stored credentials auto- invalidate on biometry enrollment changes
  • 5 commands: authenticateBiometric, isBiometricAvailable, setBiometricCredential, getBiometricCredential, deleteBiometricCredential
  • Schemas + allowlist updates, NSFaceIDUsageDescription added to Info.plist

Android (stubs):

  • 5 @JavascriptInterface methods returning BIOMETRIC_NOT_IMPLEMENTED
  • WebEvents + allowlist parity with iOS so JS code is identical across platforms

JS bridge:

  • Constants, nativeBridge.biometric namespace, and Promise-based WebBridge wrappers (authenticate/availability/get/set/delete credential)

iOS (full implementation):
- New BiometricAuthHandler using LocalAuthentication + Keychain with
  kSecAttrAccessControl(.biometryCurrentSet) so stored credentials auto-
  invalidate on biometry enrollment changes
- 5 commands: authenticateBiometric, isBiometricAvailable,
  setBiometricCredential, getBiometricCredential, deleteBiometricCredential
- Schemas + allowlist updates, NSFaceIDUsageDescription added to Info.plist

Android (stubs):
- 5 @JavascriptInterface methods returning BIOMETRIC_NOT_IMPLEMENTED
- WebEvents + allowlist parity with iOS so JS code is identical
  across platforms

JS bridge:
- Constants, nativeBridge.biometric namespace, and Promise-based
  WebBridge wrappers (authenticate/availability/get/set/delete credential)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@deputydev-agent

Copy link
Copy Markdown

DeputyDev will no longer review pull requests automatically.To request a review, simply comment #review on your pull request—this will trigger an on-demand review whenever you need it.

@mayankmahavar1mg

Copy link
Copy Markdown
Collaborator

🔍 PR Review: Biometric Login + Credential Storage


Layer Coverage

Layer Status Notes
iOS native ✅ Full implementation BiometricAuthHandler.swift, BridgeCommandHandler, NativeBridge, plist
Android native ⚠️ Stubs only 5 @JavascriptInterface methods returning BIOMETRIC_NOT_IMPLEMENTED
JS bridge constants ✅ Complete NATIVE_COMMANDS, NATIVE_CALLBACKS, INTERFACE_CATEGORIES, BIOMETRY_TYPE, BIOMETRIC_ERROR_CODE
NativeBridge.js ✅ Complete nativeBridge.biometric.* namespace added
WebBridge.js ✅ Complete Promise wrappers via _oneShot
hooks.js / standardizedHooks.js ❌ Missing No useBiometric hook exported

Findings

# Severity Layer Issue
1 🔴 Critical hooks.js useBiometric hook missing entirely
2 🔴 Critical hooks.js Android stubs silently swallow failures without useBaseHook
3 🟡 Warning BiometricAuthHandler.swift errSecAuthFailed mapped to .userCancelled
4 🟡 Warning WebBridge.js _oneShot has no timeout — promises can hang forever
5 🟡 Warning WebBridge.js Concurrent calls tear each other's listeners
6 🟡 Warning NativeBridge.kt No origin check — any WebView JS can write/delete credentials
7 🔵 Info NativeInterfaces.js BIOMETRY_TYPE.FINGERPRINT / IRIS never emitted by iOS
8 🔵 Info AndroidManifest.xml USE_BIOMETRIC permission not added yet
9 🔵 Info Info-Release.plist Pre-existing missing trailing newline
10 🔵 Info BridgeMessageValidator.swift reason optional — generic fallback may fail App Store review

Detailed Findings + Examples


🔴 1 — useBiometric hook missing entirely (hooks.js)

Every other feature in catalyst-core ships a React hook: useCamera, useGoogleSignIn, useFilePicker, useNotificationPermission, useHapticFeedback, useNetworkStatus, useDataProtection, useSafeArea. Biometric has none.

What a developer has to write today (without the hook):

// ❌ What devs are forced to do without useBiometric
useEffect(() => {
  window.WebBridge.register('ON_BIOMETRIC_AUTH_SUCCESS', (data) => {
    const parsed = JSON.parse(data)
    setAuthResult(parsed)
    setLoading(false)
  })
  window.WebBridge.register('ON_BIOMETRIC_AUTH_ERROR', (data) => {
    setError(data) // raw native error — not standardized
    setLoading(false)
  })
  window.WebBridge.register('ON_BIOMETRIC_AUTH_CANCELLED', () => {
    setError({ message: 'Cancelled' })
    setLoading(false)
  })
  return () => {
    window.WebBridge.unregister('ON_BIOMETRIC_AUTH_SUCCESS')
    window.WebBridge.unregister('ON_BIOMETRIC_AUTH_ERROR')
    window.WebBridge.unregister('ON_BIOMETRIC_AUTH_CANCELLED')
  }
}, [])
// No loading state, no progress, no SSR guard, no dev logging, no error standardization

What it should look like (following useGoogleSignIn convention):

// ✅ What useBiometric should provide
const { authenticate, getCredential, setCredential, deleteCredential, isAvailable,
        loading, error, data, isNative, clear, clearError } = useBiometric()

const handleLogin = () => authenticate({ reason: 'Confirm your identity' })

The hook needs to follow the same shape as useGoogleSignIn:

  • Wrap useBaseHook("useBiometric")
  • SSR guard returning a stub object
  • useEffect with window.WebBridge.register + cleanup for all 8 biometric callbacks
  • Named methods: authenticate, isAvailable, getCredential, setCredential, deleteCredential
  • Route success through base.setDataAndComplete, errors through base.handleNativeError
  • Export from hooks.js or standardizedHooks.js

🔴 2 — Android stubs silently swallow failures without useBaseHook (hooks.js)

Android stubs return BIOMETRIC_NOT_IMPLEMENTED via ON_BIOMETRIC_AUTH_ERROR / ON_BIOMETRIC_CREDENTIAL_ERROR. Without useBaseHook, those errors hit the caller's raw .catch() with no handleNativeError translation, no setError, no progress state update, no dev-mode logging.

Scenario: App ships to Android. User taps "Pay with Face ID". Native fires ON_BIOMETRIC_CREDENTIAL_ERROR with code: BIOMETRIC_NOT_IMPLEMENTED.

// ❌ Without useBaseHook — raw error, no standardization
window.WebBridge.getBiometricCredential({ key: 'auth_token' })
  .catch(err => {
    // err.code === 'BIOMETRIC_NOT_IMPLEMENTED'
    // but err is a plain JS Error — not a StandardError
    // setError(err) — no code, no details, no user-facing message
    // loading spinner never cleared (no setLoading(false))
    // no isDevelopment() group log
    console.error(err) // only signal the dev gets
  })

// ✅ With useBaseHook inside useBiometric
// base.handleNativeError(nativeErr) translates to StandardError,
// calls setError + setLoading(false) + errorProgress() + dev logging automatically

isBiometricAvailable has no error path in _oneShot, so it resolves with { available: false } on Android — that part is fine. The issue is the other 4 operations that reject with unstandardized errors.


🟡 3 — errSecAuthFailed mapped to .userCancelled (BiometricAuthHandler.swift, getCredential)

Scenario: User fails Face ID 3 times → iOS locks biometric for 30 seconds → SecItemCopyMatching returns errSecAuthFailed → your code fires ON_BIOMETRIC_AUTH_CANCELLED → JS sees err.cancelled = true → app shows "cancelled" UI, not "too many attempts, try again later".

// ❌ Current — lockout looks like a tap-cancel
case errSecUserCanceled, errSecAuthFailed:
    completion(.failure(.userCancelled))

// ✅ Fix — split the cases
case errSecUserCanceled:
    completion(.failure(.userCancelled))
case errSecAuthFailed:
    completion(.failure(.authFailed("Biometric authentication failed")))

The LAError path in BiometricError.from(laError:) already correctly maps .authenticationFailed → .authFailed. The Keychain path should match.


🟡 4 — _oneShot has no timeout — promises hang forever (WebBridge.js)

Scenario: User taps "Authenticate" → Face ID sheet appears → incoming phone call → iOS dismisses the sheet silently without firing any biometric event → _oneShot promise never resolves or rejects → loading spinner stays on screen forever → listeners for ON_BIOMETRIC_AUTH_SUCCESS, ON_BIOMETRIC_AUTH_ERROR, ON_BIOMETRIC_AUTH_CANCELLED remain registered indefinitely → every subsequent auth attempt stacks another set of listeners.

// ❌ Current — no escape hatch
_oneShot = (invoke, { success, error, cancelled }) => {
    return new Promise((resolve, reject) => {
        const cleanup = () => { ... }
        this.register(success, (data) => { cleanup(); resolve(parse(data)) })
        // if native never fires — promise lives forever
        invoke()
    })
}

// ✅ Fix — 30s safety valve
_oneShot = (invoke, { success, error, cancelled }) => {
    return new Promise((resolve, reject) => {
        let done = false
        const cleanup = () => {
            done = true
            clearTimeout(timerId)
            this.unregister(success)
            if (error) this.unregister(error)
            if (cancelled) this.unregister(cancelled)
        }
        const timerId = setTimeout(() => {
            if (!done) { cleanup(); reject(new Error('Native call timed out')) }
        }, 30_000)

        this.register(success, (data) => { cleanup(); resolve(parse(data)) })
        // ... rest of error/cancelled handlers
        invoke()
    })
}

🟡 5 — Concurrent calls tear each other's listeners (WebBridge.js)

Scenario: User double-taps "View saved card" button (no debounce on the button). Two getBiometricCredential() calls fire before either resolves. Both register on ON_BIOMETRIC_CREDENTIAL_GET. First call's cleanup() unregisters that event. Face ID succeeds — native fires ON_BIOMETRIC_CREDENTIAL_GET — but the second call's listener is already gone. Second promise hangs forever.

// ❌ Current — second call's listener silently removed by first call's cleanup
const call1 = window.WebBridge.getBiometricCredential({ key: 'token' }) // registers listener
const call2 = window.WebBridge.getBiometricCredential({ key: 'token' }) // registers same listener slot
// call1 resolves → cleanup() → unregisters listener
// call2 now has no listener → hangs forever

// ✅ Option A — in-flight guard in useBiometric hook
const getCredential = useCallback(async (options) => {
    if (inFlightRef.current) return // silently drop or throw
    inFlightRef.current = true
    try {
        return await window.WebBridge.getBiometricCredential(options)
    } finally {
        inFlightRef.current = false
    }
}, [])

// ✅ Option B — at minimum, document it
// JSDoc on _oneShot: "Do not call concurrently on the same command — concurrent
// calls share one listener slot; the second call will hang."

🟡 6 — No origin check — any WebView JS can write/delete credentials without biometric gate (NativeBridge.kt)

Scenario: WebView loads a third-party ad iframe (common in e-commerce apps). Injected script runs:

// Malicious script in an ad iframe — no user interaction, no biometric prompt
window.NativeBridge.deleteBiometricCredential(JSON.stringify({ key: 'auth_token' }))
// → credential silently deleted
// → user gets logged out on next app open with no explanation

window.NativeBridge.setBiometricCredential(JSON.stringify({ key: 'auth_token', value: 'attacker_token' }))
// → attacker's token stored behind Face ID
// → next getBiometricCredential returns attacker token

getBiometricCredential at least triggers a Face ID prompt the user sees. set and delete run silently.

This matters specifically for catalyst-core because it is a universal framework — the host app controls what the WebView loads, and not all host apps will be first-party-only. Recommend either:

  1. Document explicitly: "This bridge assumes a trusted first-party WebView. Do not use if WebView loads third-party content."
  2. Or add a biometric gate to setBiometricCredential (match the getCredential behavior).

Convention Comparison (vs useCamera, useGoogleSignIn, useBaseHook)

Convention Other hooks This PR
Exported React hook (use*) useCamera, useGoogleSignIn, useFilePicker, etc. ❌ Missing — no useBiometric
Wraps useBaseHook All hooks ❌ Not applicable (hook missing)
executeOperation pattern useCamera, useGoogleSignIn ❌ Not applicable (hook missing)
useEffect listener registration + cleanup All hooks ❌ Not applicable (hook missing)
handleNativeError for error standardization All hooks ❌ Not applicable (hook missing)
setDataAndComplete on success All hooks ❌ Not applicable (hook missing)
isDevelopment() debug logging All hooks ❌ Not applicable (hook missing)
SSR guard (typeof window === "undefined") All hooks ❌ Not applicable (hook missing)
Exported from hooks.js or standardizedHooks.js All features ❌ Not present

What's Done Well ✅

  • Wire parity across iOS, Android, and JS is solid — allowlists, event enum values, and callback names match across all three layers
  • kSecAttrAccessControl(.biometryCurrentSet) is the correct flag — credentials auto-invalidate on enrollment change (new fingerprint added)
  • NSFaceIDUsageDescription present in both Info.plist and Info-Release.plist (commonly missed, handled correctly here)
  • Keychain operations that block on biometric prompt dispatched off main queue, results delivered back on main
  • deleteCredentialRaw called before SecItemAdd in setCredential — avoids errSecDuplicateItem
  • @unknown default in biometryType switch — forward-compatible with future Apple biometry types
  • Android stubs emit event-compatible payloads — JS symmetry maintained before Android is built out
  • INTERFACE_CATEGORIES.BIOMETRIC block correctly groups commands + callbacks for capability detection

Bottom Line

The Android stub approach is fine — stubs emit the right events so JS is platform-symmetric. The native iOS implementation is solid. The main gaps before merge:

Must fix:

  1. Add useBiometric hook to hooks.js following the useGoogleSignIn pattern
  2. Fix errSecAuthFailed → split from errSecUserCanceled in the Keychain path

Strongly recommended:
3. Add 30s timeout to _oneShot
4. Add in-flight guard for concurrent calls (or document the limitation)
5. Document the trusted-WebView assumption for set/delete credential

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants