feat(security): biometric-guarded session token (Keychain/Keystore) - #2489
Closed
innolope-dev wants to merge 43 commits into
Closed
feat(security): biometric-guarded session token (Keychain/Keystore)#2489innolope-dev wants to merge 43 commits into
innolope-dev wants to merge 43 commits into
Conversation
Resolves 8 conflicts. Where main reworded copy after it was extracted, the
catalog now follows main — the exit prompts had reversed meaning (the KYC
flow saves progress now, so 'you'll have to start over' was wrong in every
locale), and the withdraw title became 'You're withdrawing'.
Drops keys orphaned by main: the StartVerificationView interstitial and the
autoStart 'lose progress' variant are both gone.
Extracts copy main added into the localized surface: the /fix-card-signature
support page and the deposit-cancel warning. That warning's noun was
interpolated ('Cancel this {noun}?'), which can't be translated — es/pt need
gender agreement — so each kind carries its own sentence.
Four cooperating defects kept a dead session resurrecting itself and the webview reloading in a flash loop that only 'clear all app data' escaped: - CapacitorUpdater.set() reloads the webview IMMEDIATELY — the comment believed it deferred to next launch. Use next(), which actually defers. - MainActivity served pre-rendered HTML from APK assets even while an OTA bundle was active, so any hard navigation (logout goes to /setup) loaded a stale export whose chunks no longer exist. Resolve HTML against the active server base path first, assets only when no bundle is live. - logout cleared the JWT before cancelling queries, so an in-flight /users/me sliding refresh re-persisted the very token being wiped. Cancel + clear the query cache first. - the sliding refresh now checks a clear-epoch counter and drops the re-minted token when the session was cleared mid-flight; the 401/404 clearAuthToken is awaited so Preferences.remove lands before teardown.
The white-panel-above-the-inset layout is identical on both platforms, but the white fill was gated to isIOSNative — Android 15 edge-to-edge painted the nav-bar inset periwinkle under the white panel, reading as a stray blue strip on the onboarding screen.
The animated-WebP mascots drop frames in the Android WebView — the same symptom the GIF fallback fixed on legacy iOS — but the fallback was gated on isLegacyWebKit, which can never match an Android UA.
The scale/opacity enter tween hitched on first-frame rasterization in the Android WebView; transform-gpu + will-change-transform pre-promote the layer. Also drops the dead max-h-[] class.
The redirect keyed on isloginClicked && user, and isloginClicked is set before the ceremony — a stale user re-appearing mid-ceremony (the OS passkey sheet blur/refocus fires refetchOnWindowFocus) pushed /home with balance and activity rendered before any authentication. Gate the redirect on loginResolved as well.
…itial A valid, completed session (hasAppAccess) that cold-starts on /setup was stopped at 'You're already signed in — Continue as <user>' whose CTA just pushes /home. Route those straight home; keep the interstitial for the half-finished-signup case it was written for.
Calls the new POST /users/logout (peanut-api-ts tokenVersion bump) with the still-valid JWT before clearing local state, so logout now kills the session on every device instead of only wiping this one's storage. Best-effort: a dead backend falls through to local logout (and the forced-logout path skips it entirely, as before).
Release builds signed the App Store configuration with App.entitlements, which carries aps-environment=development. TestFlight/App Store binaries therefore registered against the APNs sandbox while OneSignal sends via production, so every push silently reached zero devices. Point the release config at a new AppRelease.entitlements with aps-environment=production. Android had the mirror-image hole: android-release.yml skipped google-services.json when ANDROID_GOOGLE_SERVICES_JSON was unset and build.gradle swallowed the missing file, so release AABs shipped with the google-services plugin silently omitted and no FCM token registration. Both now fail the build instead of shipping dead push, and the iOS workflow verifies aps-environment in the provisioning profile and in the exported IPA.
…gging A failed OneSignal login() left the device subscription without the external_id the backend targets, so pushes resolved to no recipients with nothing in Sentry. Capture the failure, and only commit lastLinkedExternalId on success so transient errors retry on the next sync rather than latching the device into a permanently unlinked state. Add opt-in verbose SDK logging (NEXT_PUBLIC_ONESIGNAL_DEBUG, baked in via the pushDebug workflow input, or localStorage.__onesignal_debug) for diagnosing release builds where NODE_ENV-gated tooling is eliminated.
Adapter imports conflicted: main gained notification-click handling after the push-fix branch was cut. Kept both sides — click listeners and the opt-in debug logging.
The earlier hard-fail was based on a wrong premise. OneSignal's Android SDK never reads google-services.json: PushRegistratorFCM builds FirebaseOptions in code from its own FCM_DEFAULT_* constants plus the sender ID served by the OneSignal dashboard, and com.onesignal:notifications pulls firebase-messaging in transitively. The google-services plugin only wires Firebase's automatic initialization, which OneSignal bypasses. The "Push Notifications won't work" comment that motivated the guard is Capacitor's stock boilerplate for @capacitor/push-notifications, which does need the file. This app uses OneSignal, which does not. Keep validating the file when the secret is set; skip it when unset. Android push depends on the OneSignal dashboard's FCM v1 config, not on this file.
OneSignal's Android SDK registers with FCM on its own — PushRegistratorFCM builds FirebaseOptions in code from the sender ID served by the OneSignal dashboard, and firebase-messaging arrives transitively via com.onesignal:notifications. The google-services plugin and its JSON only wire Firebase's automatic initialization, which OneSignal bypasses, so they were dead weight. Drop the com.google.gms:google-services classpath, the conditional plugin apply, and the CI "Decode google-services.json" step. Android push now depends solely on the OneSignal dashboard's FCM v1 config (a service-account key, which is a different Firebase file than google-services.json).
isCoveredByDisableList only consulted ITEMS_TO_DISABLE, so routes handled via P0_TRANSFORMS — whose replacements strip force-dynamic/generateMetadata — were reported as uncovered server-only routes and failed the native build. (mobile-ui)/claim/page.tsx is the case that broke it: it lives in P0_TRANSFORMS, not ITEMS_TO_DISABLE, so the guard never saw its transform. invite survived only because it also has a dir entry in ITEMS_TO_DISABLE. Treat a route as covered if it's in either list. This regressed native Android builds since ~v1.0.20; the bug is present on main too.
Google now rejects the Play Edit commit with "Changes are sent for review automatically. The query parameter changesNotSentForReview must not be set." once the app has a reviewed base. The flag was only needed for the very first release; with a reviewed base the edit must go to review automatically. The AAB itself uploaded fine — only the commit step failed on this flag.
…ugin cap sync ios silently omits sources/SumsubCordovaIdensicMobileSdkPlugin (exit 0, no folder) when node_modules isn't fully materialized, and the committed CapApp-SPM/Package.swift then fails the archive a minute later with a cryptic SwiftPM "folder doesn't exist" error. - native-ios-postsync.js: hard-fail with an actionable message when the sources dir is missing but the plugin is still installed or still referenced by CapApp-SPM/Package.swift; only skip on genuine removal. - ios-release.yml: verify the SumSub Cordova plugin materialized right after pnpm install, catching the partial-install root cause at the point where a job re-run recovers.
A bare `vX.Y.Z` tag push only set CURRENT_PROJECT_VERSION and left MARKETING_VERSION at the project default 1.0, so tag releases shipped to TestFlight as version 1.0 (flagged outdated). Fall back to the tag name (v1.0.31 -> 1.0.31) when no explicit versionName input is given, matching android-release.yml.
The suite was added on main while the component still used hardcoded strings; the i18n branch converted it to useTranslations, so the two only break when merged. Use the same IntlWrapper pattern as the rest of the localized suites.
Three changes that together make PEANUT-UI-R5F diagnosable in the field: - Split the single direct-fetch canary into three probes (unauthenticated GET, authorized GET, POST) so reachability, CORS preflight and the GET-vs-POST asymmetry seen on-device can be told apart. Probes run in parallel and report in a fixed order. - Tag the binary version/build on every native Sentry event. With OTA the JS `release` and the installed binary diverge, which made R5F look like it came from a build it didn't. - Actively unregister service workers inside the Capacitor WebView. Builds before 2026-04 registered the PWA SW there; the native bundle ships no sw.js, so those registrations can never self-update and sit frozen in front of all GET traffic.
… to production Also updates the src/content submodule pointer to its latest commit.
…is blocked The edge in front of api.peanut.me rejects Android WebView GETs at the TLS-fingerprint level (PEANUT-UI-R5F): the block response has no CORS headers, so fetch surfaces it as an instant opaque TypeError while POSTs (passkey verify) and Sentry ingest sail through. CapacitorHttp.request() uses the OS HTTP client — no browser fingerprint, no CORS — and is registered natively regardless of the CapacitorHttp.enabled flag, so the fallback is OTA-deliverable to existing 1.0.32 binaries. fetchWithSentry retries API requests over the native transport when the WebView fetch rejects; FormData bodies keep the WebView path. Canary v3: per-probe messages (v2's identical messages were collapsed by Sentry's dedupe integration — only the first probe ever arrived), transport detection compares against the pre-wrap fetch (the auth-capture wrapper made v2 report cap-http-proxy on proxy-off binaries), plus no-cors and native-transport probes to separate edge blocks from dead networks.
Demo mode is the app-store review sandbox — synthetic data, no real backend — so the beta-feedback, connectivity, and maintenance banners don't apply there.
… fallback (OTA download broken on affected devices)
Brings 53 commits from main into the release branch ahead of the next mobile build: push-tap routing, OneSignal failure reporting, biometric app lock, fresh-passkey step-up, report-only CSP, Manteca per-rail wallet hotfix, Rhino route minimums and the Sentry noise fixes. Nine conflicts, all main-adds-behaviour vs mobile-release-adds-i18n. Resolved to keep BOTH in every case: - ClientProviders: main's AppLockGate now wraps children INSIDE mobile-release's AppIntlProvider > ContextProvider nesting. Getting this backwards 500s the whole app and unit tests don't catch it. - authContext: took main's clearStepUpToken(); dropped its duplicate query cancel/clear, which already runs earlier here — deliberately BEFORE the token wipe, or an in-flight /users/me re-persists a sliding-refresh token and logout never sticks on Android. - PhysicalCardScreen: main's null-position fix expressed through the translation layer. Needed a new onListBodyNoPosition key in all three locales rather than interpolating '#null'. - withdraw: kept main's USD-pinned validation (no token-price scaling) with mobile-release's translated error copy; dropped the now-dead tokenSelectorContext and INSUFFICIENT_BALANCE_MESSAGE imports. - ApplicationStatusScreen: main's proof-of-address props with mobile-release's COPY_KEYS lookup. - notifications page: kept the localized date-group header, dropped main's hardcoded-English formatGroupHeaderDate. - src/content: took main's production-published pointer (ca7f9ca). The only legal-copy delta is a support@ contact address; the Rain-superset prohibited-activities expansion is present in both. Two suites from main rendered components that now call useTranslations; wrapped both in NextIntlClientProvider, same pattern as the existing qr-pay and SumsubKycWrapper tests. Full suite green: 162 suites, 2133 passed.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
auth-token.ts is reachable from Server Component pages (charges.ts → [...recipient]/page.tsx), so app-lock-state.ts must stay hook-free — useSyncExternalStore in that module failed the production build on Vercel. Move the useAppLocked hook into its own client file.
… any Extends the existing global Window.Capacitor declaration — the only eslint error this branch added on top of the known-red baseline.
Match main's #2493: drop the 'locked / could not confirm' framing for a plain log-in ask ('Welcome back!' + 'Please log in to access the app.', button 'Log in').
Collaborator
Author
kushagrasarathe
requested changes
Jul 24, 2026
kushagrasarathe
left a comment
kushagrasarathe
left a comment
Contributor
There was a problem hiding this comment.
Reviewed adversarially (external contributor, security control to a release branch) — requesting changes on one code item; the design itself is sound.
This is the real fix for the cosmetic app-lock (#2461 / issue #2472), and it genuinely closes those gaps: the JWT is now stored via NativeBiometric.setCredentials({ accessControl: BIOMETRY_CURRENT_SET }) (Keychain/Keystore, biometric-gated) rather than plain Preferences; the gate fails closed (auth state derives from the presence of the guarded token, so deleting a pref signs the user out, never opens the app); and the session is paused while locked (no /users/me polling with a live token). @capgo/capacitor-native-biometric@8.6.0 clears the 14-day supply-chain floor and its integrity matches npm. No backdoor.
Requested change (code):
onramp-quote.ts:39skips the gate — addawait authReady()before it fires so the one un-gated caller can't run mid-lock.
Merge-gate conditions (not code, but required before this rolls to the store):
- Run the on-device iOS + Android matrix — CI cannot prove the native biometric binding actually holds; this must be verified on hardware.
- Explicit sign-off on the two accepted residuals: (a) the migration-window plaintext copy kept until the first successful guarded unlock, and (b) the legacy plain-mode fail-open for older binaries / no enrolled biometric (those sessions have no guarded token to protect, so it's by-design — but call it out and accept it consciously).
Once authReady() lands and the device matrix + residual sign-off are done, this is good to ship.
Opening the app and viewing the balance is not treated as a critical vulnerability — matching the web app, where the same read-only view is ungated. Money movement stays passkey-gated at the transaction layer, independently of this flag. - add OPEN_GATED flag (NEXT_PUBLIC_APP_OPEN_GATED, default false) - AppLockGate renders children straight through when the flag is off - guarded-storage use in auth-token gated via guardedModeEnabled(), so the JWT stays in plain Preferences and the session remains readable without a biometric; the guarded infrastructure stays intact for when it is enabled - onramp-quote: await authReady() before building auth headers so the one un-gated caller parks instead of firing unauthenticated mid-lock - app-lock copy: 'Log in' -> 'Unlock' (en/es-419/pt-BR) — the ceremony is a biometric unlock of an existing session, not a login Adds tests covering guarded mode staying dormant when the flag is off.
Collaborator
Author
|
Pushed Product change — app-open is no longer gated by default. We decided that opening the app and seeing the balance isn't a critical vulnerability (the web app doesn't gate it either); money movement stays passkey-gated at the transaction layer. Rather than drop the work, it's behind a new
Requested code change: Copy nit: Added tests covering guarded mode staying dormant when the flag is off; The device-matrix + residual sign-off gates only apply when |
Contributor
|
Re-reviewed What's addressed:
Blocker — CI is red (
Two things to flag for the merge decision (not blockers, but be aware):
Once the test is green, I'm good to approve. |
…-through The app-open lock is now dormant behind OPEN_GATED (default off), so the guarded-mode lock tests failed — the effect returns early and never locks. Mock the flag on for those cases and add a case asserting a guarded session opens straight through when the flag is off.
Mirror production, where OPEN_GATED is off when NEXT_PUBLIC_APP_OPEN_GATED is unset. The gate-exercising cases enable it explicitly.
Collaborator
Author
|
Tests are green as of 3837f2a + 27bf61e — Note: this PR is not included in the v1.0.41 build going out now — it stays queued for the next release after your re-review, shipping dormant ( |
innolope-dev
added a commit
that referenced
this pull request
Jul 29, 2026
Biometric-guarded session token (Keychain/Keystore via NativeBiometric), dormant behind OPEN_GATED (default off). Supersedes the release-side stopgap that backported the flag onto the cosmetic app lock (6196d13): AppLock and app-lock.consts.ts taken from the branch; card.ts keeps the release-side apiFetch refactor, which awaits authReady() internally; lock copy uses the branch's 'Unlock' wording to match the biometric action.
Collaborator
Author
|
Superseded by #2568 (same content, transplanted onto What happened here:
@kushagrasarathe your review carries over — the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2472 — the follow-up to #2461, which shipped the app lock as a privacy screen only.
What changes
Three session modes on native, detected once per launch (
src/utils/auth-token.ts):@capgo/capacitor-native-biometric(pinned 8.6.0, clears the 14-day dependency floor) withAccessControl.BIOMETRY_CURRENT_SET: KeychainSecAccessControlon iOS, aBiometricPrompt.CryptoObject-bound Keystore key on Android. The unlock ceremony IS the token read — one OS biometric prompt both proves presence and releases the credential. No separate WebAuthn assertion.Fail closed: the lock decision derives from a non-secret presence marker (
guarded-token-present), never from the user query. Deleting the marker or any local pref yields a signed-out app — never an open session — because the JWT itself is unreadable without a biometric.Session paused while locked:
authReady()parks every API caller (a locked app never emits an unauthenticated request that would 401 →clearAuthToken()); the[USER]query is disabled (which also killsrefetchOnWindowFocuson resume); the auto-refresh poller skips its tick; a sliding-refresh token landing after suspension is dropped. On unlock, react-query's stale refetch doubles as the post-unlock refresh.Re-enrollment = session expired, never a stuck lock:
BIOMETRY_CURRENT_SETinvalidates the item when biometrics change (by design). Both platforms surface this as not-found → clean logout to /setup.card.ts fix (from the #2463 re-review):
services/card.tsread thejwt-tokenweb cookie directly, which never worked on native — now routed throughauthReady()+getAuthHeaders().Android write-prompt nuance
Keystore writes prompt outside a post-auth window (per-op auth keys). Handled with
authValidityDuration: 60: the unlock read opens a 60s window that silently covers the re-mint/users/meships right after unlock; writes outside the window are skipped (memory-only, server re-mints later). iOS Keychain writes never prompt. Consequence: on Android, login and one-time migration each show one extra 'Protect Credentials' prompt.Deliberate trade-offs
skipBackendCall): there's no token in memory to authenticate the revocation POST. tokenVersion isn't bumped — acceptable since the attacker can't extract the guarded JWT.BIOMETRY_CURRENT_SET/BIOMETRY_ANYon iOS andAUTH_BIOMETRIC_STRONGkeys on Android. After an OS biometric lockout the escape hatch is Log out → fresh login.Testing
setAuthTokenno-op, silent-window write policy, migration round-trip lifecycle, plugin error-code mapping, lock registry, and a D7 regression test (gate locks without waiting for the user query — the permanent-white-screen trap).Plugin gate verified by code-read
iOS:
SecAccessControlCreateWithFlags(kSecAttrAccessibleWhenPasscodeSetThisDeviceOnly, .biometryCurrentSet)+SecItemCopyMatchingwithLAContext. Android: Keystore AES-GCM withsetUserAuthenticationRequired(true),setUserAuthenticationValidityDurationSeconds(-1)(per-op),setInvalidatedByBiometricEnrollment(true), reads/writes throughBiometricPrompt.CryptoObjectciphers. The ungatedgetCredentials()/verifyIdentity()paths are never used.