feat(security): biometric app lock for the native app - #2461
Conversation
Sessions persist for weeks, so an unlocked phone was enough to reach a funded account. Gate the native app behind a local WebAuthn assertion on cold start and after LOCK_AFTER_BACKGROUND_MS in the background. The assertion is checked locally, not proven to the API: the threat is physical device access, and the OS will not produce one without the user's biometric or passcode. Server-side proof of freshness belongs to step-up auth on sensitive endpoints, which is a separate change. Fails open when there is no credential to prompt against or the platform has no WebAuthn — a lock that cannot be lifted would strand the user.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughChangesThe PR adds a native Capacitor app lock using local WebAuthn presence verification, including startup and long-background relocking. The gate is mounted globally, and utility tests cover supported, unsupported, dismissed, and successful authentication outcomes. Native app lock
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Capacitor
participant AppLockGate
participant PresenceCheck
participant WebAuthn
Capacitor->>AppLockGate: startup or appStateChange
AppLockGate->>PresenceCheck: requestLocalUserPresence(credentialId)
PresenceCheck->>WebAuthn: credentials.get with required verification
WebAuthn-->>PresenceCheck: assertion or error
PresenceCheck-->>AppLockGate: UnlockOutcome
AppLockGate-->>Capacitor: render children or lock overlay
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code-analysis diffPainscore total: 6212.38 → 6227.37 (+14.99) 🆕 New findings (9)
✅ Resolved (1)
📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![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
🧹 Nitpick comments (1)
src/utils/__tests__/app-lock.test.ts (1)
61-63: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert the pinned credential ID bytes.
Checking only the descriptor count would still pass if the stored credential were replaced with a different one. Compare
allowCredentials[0].idwithbase64URLToBytes(CREDENTIAL_ID).Proposed test strengthening
+import { base64URLToBytes } from '../native-webauthn' + ... expect(received?.userVerification).toBe('required') expect(received?.allowCredentials).toHaveLength(1) + expect(received?.allowCredentials?.[0]?.id).toEqual(base64URLToBytes(CREDENTIAL_ID)) expect(new Uint8Array(received!.challenge as ArrayBufferView['buffer'])).not.toHaveLength(0)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/__tests__/app-lock.test.ts` around lines 61 - 63, Strengthen the assertions in the app-lock test by comparing the first allowCredentials entry’s id with base64URLToBytes(CREDENTIAL_ID), while retaining the existing length check and other assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Global/AppLock/index.tsx`:
- Around line 23-26: Update AppLockGate in
src/components/Global/AppLock/index.tsx at lines 23-26 to initialize native
protected content as pending/blocked until support, credential, and presence
checks complete; at lines 101-120, trap focus and remove or inert all protected
content while locked. In src/app/ClientProviders.tsx lines 59-61, wrap the
protected provider/content branch with AppLockGate instead of mounting the gate
beside it.
---
Nitpick comments:
In `@src/utils/__tests__/app-lock.test.ts`:
- Around line 61-63: Strengthen the assertions in the app-lock test by comparing
the first allowCredentials entry’s id with base64URLToBytes(CREDENTIAL_ID),
while retaining the existing length check and other assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b221adf-6f00-40e2-bea8-942e06555894
📒 Files selected for processing (4)
src/app/ClientProviders.tsxsrc/components/Global/AppLock/index.tsxsrc/utils/__tests__/app-lock.test.tssrc/utils/app-lock.ts
The gate started unlocked and was mounted beside the page, so protected content could paint before the lock engaged and stayed focusable behind the overlay. It now wraps children and closes on the first client paint on native — before the user query can resolve and render balances. A 'pending' state covers the window where auth hasn't settled yet, resolving to 'open' for a signed-out user or one with no usable credential. While locked, the protected tree is not rendered at all, so nothing sits behind the lock screen for tab or screen reader to reach. Tradeoff: unlocking remounts, losing in-flight component state. The lock only fires on cold start (no state yet) or after five minutes background, where iOS has usually discarded the webview anyway. Also assert the pinned credential's actual bytes — a descriptor for the wrong credential passed the old length-only check.
There was a problem hiding this comment.
Requesting changes — the lock is cosmetic; don't ship it as account protection
The code is clean, native-scoped, no new dep. But it gates rendering only and does not protect the credential it claims to:
- The session JWT stays in plain
localStorage(auth-token.ts), untouched by this PR, and/users/mekeeps polling with that Bearer token while the gate is up. - Bypasses on an unlocked/filesystem-accessible device: (a) read
localStorage['jwt-token']via a webview inspector / backup → full API access, biometric never involved; (b) delete${userId}:user-preferences→credentialIdbecomes undefined → the gate resolves straight toopen. - Fails open on no-credential and on
NotSupportedError.
Required before merge:
- Re-scope the claim — describe it as a deterrent / privacy screen, not account protection. Do not tell users (or the "session-security series") that it protects a funded account on a lost device; that's false.
- Fix the fail-open doc comment — the
catchblock returnsunsupportedonNotSupportedError, whichattemptUnlocktreats as unlock, so the fail-open surface is larger than "only the pre-flight checks." - Native smoke test (cold start, 6-min background resume, cancel prompt, log-out escape) — never run on a device.
Follow-up to make it a genuine control: keep the JWT in biometric-guarded Keychain/Keystore and materialize it only after a successful assertion.
The catch block's NotSupportedError path also resolves to unsupported (which the gate treats as unlock), so 'only the pre-flight checks fail open' undersold it. Also spell out in the module header that this is a privacy screen, not account protection — the session token is untouched by this PR and the gate disappears if the stored credential id is stripped.
|
Accepted in full — the "cosmetic" framing is accurate, and the PR now says so out loud. Claim re-scoped. The PR description has been rewritten: this is a privacy screen and deterrent, explicitly not account protection. It now spells out the three bypasses you listed (token untouched in webview storage, polling continues behind the gate, deleting the stored Fail-open doc fixed (same commit). The docstring and catch comment now state the full fail-open surface including the Native smoke test — still outstanding, listed as an unchecked box in the description; needs a device (cold start, 6-min background resume, cancel prompt, log-out escape, iOS + Android). Same device session should cover #2463's passkey path. The real control (JWT into biometric-guarded Keychain/Keystore, materialized only after assertion) — agreed as the follow-up; it dovetails with the native transport/header-auth PR that's queued behind #1195, since that's the change that makes JS/native token custody explicit. One judgment call to flag: your review says the gate "fails open on no-credential and NotSupportedError" as a defect. That behavior is retained deliberately — a lock that can't be lifted strands the user in their own app, and given the gate is now honestly scoped as a deterrent, fail-open on can't-prompt seems like the right trade. If you'd rather it fail closed with a log-out-only escape, say so and it's a small change. |
kushagrasarathe
left a comment
kushagrasarathe
left a comment
There was a problem hiding this comment.
Re-reviewed. The honesty condition IS met — the doc now accurately states it is a privacy screen / deterrent, NOT account protection, and names the full fail-open surface. The commits also made it a real render boundary (gates the subtree) and fixed the credential byte-equality check. But it is still fundamentally cosmetic as a security control: the JWT is still in plain localStorage, useUserQuery/sliding-refresh still run while locked (the gate is a child of the auth context, not a parent), and both bypasses remain open by design — reading the token off-device, and deleting ${userId}:user-preferences resolves the gate straight to open. It does no harm and fail-open is the right UX direction. Decision for you/PM: merge it as a deterrent (fine), but it does NOT close the "unlocked phone reaches a funded account" risk — that needs the JWT moved into biometric-guarded Keychain/Keystore (explicitly deferred here). Not clearing my change-request until that framing is acknowledged.
kushagrasarathe
left a comment
kushagrasarathe
left a comment
There was a problem hiding this comment.
Clearing my change request — the framing concern is already fully handled in the PR (the "What this is — and is not" section explicitly states it's a privacy screen / deterrent, NOT account protection, and lists its own bypasses). Approving to ship as an honest deterrent.
The real control (biometric-guarded token storage) is filed as a follow-up: #2472. One non-engineering note: user-facing release/store copy must not upgrade this to "protects your account if your phone is lost."
2 participants
Why
Peanut sessions persist for weeks, so an unlocked phone shows a funded account to whoever is holding it. Every peer consumer-fintech app gates the native shell behind a device biometric; we had nothing.
This is item C of the session-security series. Companion PRs cover step-up auth on sensitive endpoints (the actual server-side control), session lifetime bounds, and a report-only CSP.
What this is — and is not
This is a privacy screen and a deterrent: it stops a person holding the phone from seeing or navigating the app without the owner's biometric. It is not account protection, and it must not be described to users as such:
credentialIdpreference disables the gate entirely.Real account protection on a lost device is server-side (step-up auth, #2463/#1217) plus, as a follow-up, moving the token into biometric-guarded Keychain/Keystore so it only materializes after a successful assertion.
What
AppLockGatecovers the app on cold start and whenever it returns from more than 5 minutes in the background.ClientProviders. Renders children directly on web.No new native dependency:
@capacitor/appand@capgo/capacitor-passkeyare already in the binary, so this ships over Capgo rather than needing a store release.The local-check decision
The assertion is not sent to the API for verification. The threat model here is physical — someone holding an already-unlocked phone — and the OS will not mint an assertion without user verification, which is the property we want. Proving assertion freshness to the server is a different control (step-up auth on card/withdrawal endpoints) and is deliberately not conflated with a UI lock.
Failing open, on purpose
A lock that cannot be lifted is worse than no lock — it strands users in their own app. The full fail-open surface, explicitly: no stored credential, no WebAuthn on the platform, and the authenticator rejecting the ceremony with
NotSupportedErrorall resolve tounsupported, which the gate treats as "do not lock". A cancelled prompt (NotAllowedErroror any other error) keeps the gate up with a retry and a log-out escape. This asymmetry is acceptable precisely because the gate is a deterrent, not a security boundary — see "What this is — and is not".Testing
src/utils/__tests__/app-lock.test.tspins which failure modes fall open vs. closed, credential pinning including the exact credential-id bytes, anduserVerification: 'required'. Typecheck clean.Summary by CodeRabbit