feat(wia): WebView reads existing identity/documents via host secureStorage handler (SELF-3584) - #2230
feat(wia): WebView reads existing identity/documents via host secureStorage handler (SELF-3584)#2230seshanthS wants to merge 5 commits into
Conversation
Adds an optional SecureStorageStore + a secureStorage prop on SelfVerification (mirroring the existing documents injection). When a host store is provided, KeychainHandler delegates get/set/remove to it and forwards requireBiometric; otherwise the default self_sdk_ react-native- keychain behavior is unchanged. Additive/opt-in: no bridge-protocol change, KMP path (useKmpBridge intercepts secureStorage before the router) untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cureStorage (SELF-3584)
WebViewHostScreen provides a secureStorage store that maps the WebView's
self_* keys to the app's EXISTING legacy keychain (secret / documentCatalog /
document-{contentHash}) for reads and writes — no migration, no copy, legacy
stays the single biometric-gated source of truth, content-hash doc ids kept.
Identity write-guard: set(self_mnemonic/self_private_key) never overwrites an
existing secret. Reuses authProvider (new getStoredMnemonicPhrase /
restoreMnemonicPhrase exports) + passportDataProvider primitives.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…parity guard) Golden-value assertion (Hardhat account-0 key) locks @Scure derivation to the same key ethers produces at m/44'/60'/0'/0/0, guaranteeing the WebView derives the identical key as the app from the same phrase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe RN SDK now accepts injected secure storage, and the app supplies an adapter that maps WebView ChangesWebView secure storage
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: feaf9ec4d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (await deps.hasSecretStored()) { | ||
| return; | ||
| } | ||
| await deps.writeNewMnemonic(value); |
There was a problem hiding this comment.
When an existing protected secret cannot be read—for example after biometric cancellation or a transient keychain error—hasSecretStored() catches the error and returns false, so this branch treats the user as new and calls writeNewMnemonic() for the same service. That can replace the existing identity with the mnemonic generated by ensureSecret() despite this guard being intended to prevent exactly that data loss; only a definitive “item absent” result should permit the write, while read errors must reject the bridge request.
AGENTS.md reference: AGENTS.md:L36-L36
Useful? React with 👍 / 👎.
| if (await deps.hasSecretStored()) { | ||
| return; |
There was a problem hiding this comment.
For an existing user, the WebView recovery flow validates the submitted phrase against the selected document and then calls restoreSecretFromMnemonic(), but this unconditional no-op reports success without changing the legacy secret. The flow subsequently finalizes registration and displays recovery success while later proving still uses the old key, so the adapter needs an explicit authenticated recovery operation rather than silently discarding every mnemonic replacement.
AGENTS.md reference: AGENTS.md:L36-L36
Useful? React with 👍 / 👎.
| */ | ||
| export async function restoreMnemonicPhrase(phrase: string): Promise<void> { | ||
| const options = await createKeychainOptions({ requireAuth: true }); | ||
| await restoreFromMnemonic(phrase, options); |
There was a problem hiding this comment.
When Keychain.setGenericPassword() fails, restoreFromMnemonic() catches the error and resolves to false, but this wrapper discards that result and resolves successfully. Consequently a first-time secureStorage.set('self_mnemonic', ...) receives a successful bridge response even though no identity was persisted, allowing ensureSecret() and onboarding to continue until a later key read or proof fails; convert the false result into a rejection.
AGENTS.md reference: AGENTS.md:L36-L36
Useful? React with 👍 / 👎.
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e8939d34-e93d-4a1b-8d88-1a9e5d072fb6
📒 Files selected for processing (11)
app/src/providers/authProvider.tsxapp/src/providers/webViewSecureStorageAdapter.tsapp/src/screens/dev/WebViewHostScreen.tsxapp/tests/src/providers/webViewSecureStorageAdapter.test.tspackages/rn-sdk/src/SelfVerification.tsxpackages/rn-sdk/src/__tests__/KeychainHandler.test.tspackages/rn-sdk/src/handlers/KeychainHandler.tspackages/rn-sdk/src/handlers/index.tspackages/rn-sdk/src/index.tspackages/webview-app/tests/utils/secretManager.test.tsspecs/projects/sdk/workstreams/webview-in-app/plans/WIA-APP-CUTOVER.md
| async set(key, value) { | ||
| if (key === MNEMONIC_KEY) { | ||
| // Identity write-guard: never overwrite an existing identity. Only a | ||
| // genuine new user (no legacy secret) may write a fresh mnemonic. | ||
| if (await deps.hasSecretStored()) { | ||
| return; | ||
| } | ||
| await deps.writeNewMnemonic(value); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does react-native-keychain getGenericPassword throw when the user cancels the biometric prompt, even for a presence-only check?
💡 Result:
Yes, react-native-keychain's getGenericPassword method will throw an error when a user cancels the biometric prompt, regardless of whether it is a full authentication check or a presence-only check [1][2]. When biometric authentication is requested via getGenericPassword—often triggered by options requiring authentication—the native biometric prompt is presented to the user [1][3]. If the user explicitly cancels this prompt (e.g., by tapping "Cancel," pressing the back button, or clicking outside the prompt), the library rejects the promise, causing getGenericPassword to throw an error [1][2][4]. Key details regarding this behavior include: Error Handling: Because the cancellation is treated as a rejection, developers must wrap the getGenericPassword call in a try/catch block to handle the user's action gracefully [5][1]. Platform Inconsistency: Historically, error messages and codes for cancellation have varied significantly between iOS and Android, requiring developers to match on specific error messages or codes [1][2][6]. Newer versions of the library (such as via PR #762) have introduced standardized error codes (e.g., Keychain.ERROR_CODE.AUTH_CANCELED) to improve cross-platform consistency [5][7]. No Distinction for Presence-Only: The library does not inherently treat "presence-only" checks differently regarding the promise resolution. If the prompt is displayed to the user and they decline to complete the biometric interaction, the operation is treated as an authentication failure due to user cancellation [2][6]. For robust implementation, developers should check for user cancellation errors within the catch block using the standardized error codes if available, or by inspecting the error object for known cancellation patterns if using older versions of the library [5][1][2].
Citations:
- 1: https://dev.to/aoligama/biometric-login-on-react-native-with-keychain-462j
- 2: [suggestion] Document and/or improve OS keystore exceptions oblador/react-native-keychain#747
- 3: http://github.com/oblador/react-native-keychain
- 4: How to handle user cancel auth prompt on Android? oblador/react-native-keychain#480
- 5: feat: Improve Error Handling oblador/react-native-keychain#762
- 6: Inconsistent Error Messages Between iOS and Android for Cancelled Biometry Operations oblador/react-native-keychain#609
- 7: https://github.com/oblador/react-native-keychain/blob/master/AGENTS.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'webViewSecureStorageAdapter\.ts|authProvider\.ts|authProvider\.tsx|passportDataProvider\.tsx|passportDataProvider\.ts|webViewSecureStorageAdapter' || true
echo "== outlines =="
for f in $(git ls-files | rg 'webViewSecureStorageAdapter\.ts|authProvider\.ts|authProvider\.tsx|passportDataProvider\.tsx|passportDataProvider\.ts' || true); do
echo "--- $f ($(wc -l < "$f") lines) ---"
ast-grep outline "$f" --view compact || true
done
echo "== relevant snippets =="
for f in app/src/providers/webViewSecureStorageAdapter.ts app/src/providers/authProvider.tsx app/src/providers/passportDataProvider.tsx app/src/providers/authProvider.ts 2>/dev/null; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n "hasSecretStored|writeNewMnemonic|restoreFromMnemonic|getGenericPassword|setGenericPassword|MNEMONIC_KEY|self_mnemonic|requireAuth|Keychain" "$f" -C 4 || true
fi
done
echo "== package keychain versions =="
rg -n '"react-native-keychain"|keychain' package.json pnpm-lock.yaml package-lock.json yarn.lock 2>/dev/null || trueRepository: selfxyz/self
Length of output: 1083
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant snippets =="
sed -n '1,220p' app/src/providers/webViewSecureStorageAdapter.ts | cat -n
echo
sed -n '1,260p' app/src/providers/authProvider.tsx | cat -n
echo
sed -n '1,240p' app/src/providers/passportDataProvider.tsx | cat -n
echo "== keychain usages/options =="
rg -n "getGenericPassword|setGenericPassword|deleteGenericPassword|requireAuth|SERVICE_NAME|MNEMONIC_KEY|storeMnemonic|restoreFromMnemonic|writeNewMnemonic|hasSecretStored" app/src/providers/app/src/providers app/src/providers -C 5 || true
echo "== package keychain versions =="
rg -n '"react-native-keychain"|keychain' package.json pnpm-lock.yaml package-lock.json yarn.lock 2>/dev/null || trueRepository: selfxyz/self
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== authProvider hasSecretStored section =="
sed -n '503,638p' app/src/providers/authProvider.tsx | cat -n
echo "== keychain option creators =="
fd -a 'keychain.*' app/src | sed 's#^\./##' | head -20
for f in app/src/integrations/keychain.ts app/src/integrations/keychain.tsx app/src/utils/keychainErrors.ts app/src/integrations/keychain.ts 2>/dev/null; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f" | sed -n '1,260p'
fi
done
echo "== migration/restore callers for createKeychainOptions(requireAuth true) =="
rg -n "createKeychainOptions\\(\\{ requireAuth: true \\}\\)|restoreMnemonicPhrase\\(|restoreAccountFromMnemonic|resetGenericPassword\\(\\{ service: 'secret' \\}\\)|resetGenericPassword\\(\\{ service: \"secret\" \\}\\)|storeMnemonic|hasSecretStored" app/src/providers app/src -C 4 -g '*.ts' -g '*.tsx' || trueRepository: selfxyz/self
Length of output: 5172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== keychain integrations files =="
git ls-files | rg '^app/src/(integrations|types|utils)/' | rg 'keychain|react-native-keychain|keychainErrors'
echo "== keychain option creators =="
for f in app/src/integrations/keychain.ts app/src/integrations/keychain/index.ts app/src/integrations/keychain/index.tsx app/src/types/react-native-keychain.d.ts app/src/utils/keychainErrors.ts 2>/dev/null; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f" | sed -n '1,260p'
fi
done
echo "== identity store writes and guard callers =="
rg -n "createKeychainOptions\\(|requireAuth: true|SERVICE_NAME|resetGenericPassword|hasSecretStored\\(|restoreMnemonicPhrase\\(|restoreFromMnemonic|storeMnemonicPhrase|save\\(.*self_mnemonic|safeLoadDocumentCatalog" app/src/providers app/src -g '*.ts' -g '*.tsx' -C 3 2>/dev/null || trueRepository: selfxyz/self
Length of output: 380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== keychain integrations files =="
git ls-files | rg '^app/src/(integrations|types|utils)/' | rg 'keychain|react-native-keychain|keychainErrors'
for f in app/src/integrations/keychain/index.ts app/src/types/react-native-keychain.d.ts app/src/utils/keychainErrors.ts; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f" | sed -n '1,260p'
fi
done
echo "== identity store writes and guard callers =="
rg -n "createKeychainOptions\\(|requireAuth: true|SERVICE_NAME|resetGenericPassword|hasSecretStored\\(|restoreMnemonicPhrase\\(|restoreFromMnemonic|storeMnemonicPhrase|save\\(.*self_mnemonic|safeLoadDocumentCatalog" app/src/providers app/src -g '*.ts' -g '*.tsx' -C 3 2>/dev/null || true
echo "== static diff/changes around files =="
git diff -- app/src/providers/webViewSecureStorageAdapter.ts app/src/providers/authProvider.tsx app/src/providers/passportDataProvider.tsx | sed -n '1,260p' || trueRepository: selfxyz/self
Length of output: 50368
🌐 Web query:
react-native-keychain getGenericPassword accessControl user cancel biometric prompt returns false null or throws
💡 Result:
When a user cancels a biometric prompt during Keychain.getGenericPassword(), the library does not return false or null; instead, it throws an error [1]. Historically, handling these cancellations required parsing platform-specific error messages or codes [2][3]. For instance, on Android, cancellations often manifest as errors containing codes like 10 or 13 [2][4][3], while iOS typically returns a message such as "User canceled the operation" [2]. Recent updates have introduced standardized error handling to resolve these inconsistencies [5][6]. You should wrap your getGenericPassword call in a try...catch block and check for specific error codes provided by the library, such as Keychain.ERROR_CODE.AUTH_CANCELED [5]. Example of recommended handling: try { await Keychain.getGenericPassword; } catch (error) { switch (error.code) { case Keychain.ERROR_CODE.AUTH_CANCELED: console.log('Authentication was canceled'); break; case Keychain.ERROR_CODE.BIOMETRIC_LOCKOUT: console.log('Biometric authentication locked'); break; default: console.log(Keychain error: ${error.code} - ${error.message}); } } If you encounter false or null results in other scenarios, they typically indicate that no credentials were found for the specified service or storage, rather than an authentication cancellation [7][8]. Always ensure that you are using consistent service names and configurations when setting and getting credentials to avoid unexpected "not found" results [9][8].
Citations:
- 1: https://dev.to/aoligama/biometric-login-on-react-native-with-keychain-462j
- 2: Inconsistent Error Messages Between iOS and Android for Cancelled Biometry Operations oblador/react-native-keychain#609
- 3: [suggestion] Document and/or improve OS keystore exceptions oblador/react-native-keychain#747
- 4: How to handle user cancel auth prompt on Android? oblador/react-native-keychain#480
- 5: feat: Improve Error Handling oblador/react-native-keychain#762
- 6: https://github.com/oblador/react-native-keychain/blob/master/AGENTS.md
- 7: getGenericPassword result is unclear oblador/react-native-keychain#785
- 8: After app restart, getGenericPassword always returns false on Android in Version 10.0.0 oblador/react-native-keychain#778
- 9: Keychain.getGenericPassword randomly returns false oblador/react-native-keychain#594
Handle biometric/auth errors in the identity presence check.
hasSecretStored() does a plain Keychain.getGenericPassword({ service: 'secret' }) inside a catch-all that returns false. If a biometric/credential prompt is cancelled or fails for this Keychain entry, the write-guard treats the real identity as missing and allows restoreMnemonicPhrase() to overwrite it. Preserve an existing identity on any keychain/auth failure (including user cancellation) before minting a new mnemonic.
Fixes WebView App CI format/lint (long golden hex on one line). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Implements B2 (SELF-3584) — existing users' identity + documents are visible to the WebView path, with no migration/copy and without weakening or touching the legacy keychain.
Approach (per review discussion): host-provided translating secureStorage handler
Crypto is already compatible (same mnemonic → same key at
m/44'/60'/0'/0/0); only storage namespace differs. Rather than copy legacy data into the SDK'sself_sdk_*store (which uses react-native-keychain defaults — weaker, non-biometric), the app services the WebView'ssecureStoragein place from its existing biometric-gated keychain.secureStoragehandler is now host-injectable — newSecureStorageStore+ optionalsecureStorageprop on<SelfVerification>, mirroring the existingdocumentsinjection. Default staysKeychainHandler. Additive/opt-in: no bridge-protocol change; the KMP path (useKmpBridgeinterceptssecureStoragebefore the router) is untouched.WebViewHostScreenprovideswebViewSecureStorageAdaptermapping the WebView'sself_*keys → legacysecret/documentCatalog/document-{contentHash}for get and set. Content-hash doc ids kept (WebView treatsdoc.idas opaque). ReusesauthProvider(newgetStoredMnemonicPhrase/restoreMnemonicPhrase) +passportDataProviderprimitives.requireBiometricis honored, so the secret keeps strong protection.set('self_mnemonic'/'self_private_key')never overwrites an existingsecret(blocks a biometric-cancel-then-mint from clobbering the identity). Removes of identity/catalog are no-ops.Result: no migration, no copy, no drift, legacy stays the single biometric-gated source of truth, cloud backup untouched, rollback trivial (flip
IS_WIA_ENABLEDoff).Follow-up (deferred, Path A)
When
secureStoragemigrates to the KMP transport, re-inject the same translation as a hostSecureStorageProviderviaSdkProviderRegistry(the==nullhook inSelfBridgeModule.kt). The translation logic is written to be reusable there.Validation
webViewSecureStorageAdapter17 tests,WebViewHostScreen8, full apptscclean.derivePrivateKeyBIP44 parity guard (golden Hardhat key == ethers) → proves the translated key == the existing identity.IS_WIA_ENABLEDon shows same identity + docs, no re-registration, recovery works, legacy never overwritten.🤖 Generated with Claude Code
Summary by CodeRabbit