Add cross-platform session-scoped secure unlock retention#158
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Review Summary by QodoAdd cross-platform session-scoped secure unlock retention
WalkthroughsDescription• Add shared secure-unlock retention policy and session-passkey cache contracts • Implement session retention across Android, iOS, desktop, and browser platforms • Integrate browser-extension storage.session backend for retention persistence • Expose retention toggle in settings UI with cross-platform capability gating Diagramflowchart LR
A["Shared Retention Policy<br/>PROMPT_EVERY_TIME vs<br/>RETAIN_FOR_CURRENT_SESSION"] -->|"implements"| B["SessionRetentionCapableSecureSessionManager"]
B -->|"uses"| C["SessionPasskeyCache"]
C -->|"backed by"| D["Platform Backends"]
D -->|"Android/iOS/Desktop"| E["InMemorySessionPasskeyCache"]
D -->|"Browser Extension"| F["BrowserExtensionSessionPasskeyCache<br/>storage.session"]
B -->|"exposed in"| G["SettingsViewModel"]
G -->|"drives"| H["SessionRetentionCard UI"]
File Changes1. composeApp/src/commonMain/kotlin/tech/arnav/twofac/session/SecureUnlockRetentionPolicy.kt
|
Code Review by Qodo
1. Session cache write not durable
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| override fun write(passkey: String) { | ||
| inMemoryPasskey = passkey | ||
| if (!storageClient.isAvailable()) return | ||
|
|
||
| scope.launch { | ||
| runCatching { | ||
| storageClient.setItem(storageKey, passkey) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| override fun clear() { | ||
| inMemoryPasskey = null | ||
| if (!storageClient.isAvailable()) return | ||
|
|
||
| scope.launch { | ||
| runCatching { | ||
| storageClient.removeItem(storageKey) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
1. Session cache write not durable 🐞 Bug ☼ Reliability
BrowserExtensionSessionPasskeyCache.write()/clear() launch async storage.session mutations and return immediately, so retention across popup close/reopen is not guaranteed if the popup/runtime tears down before the coroutine runs/completes. Since AccountsViewModel calls SessionManager.savePasskey(passkey) after unlock, a missed write means the next popup will prompt again even when RETAIN_FOR_CURRENT_SESSION is enabled.
Agent Prompt
## Issue description
`BrowserExtensionSessionPasskeyCache` persists the retained passkey using `scope.launch { storageClient.setItem/removeItem(...) }` and does not wait for completion. In an extension popup lifecycle, this can be torn down before the async write finishes, so the passkey is not actually retained across popup reopen (the main reason for using `storage.session`).
## Issue Context
- The call site that persists the passkey after unlock is synchronous (`SessionManager.savePasskey(...)`), and common code invokes it immediately after unlocking.
- The extension storage client APIs are suspendable (Promise-backed), but the cache API is not, so the implementation currently uses fire-and-forget background work.
## Fix Focus Areas
- composeApp/src/wasmJsMain/kotlin/tech/arnav/twofac/session/BrowserExtensionSessionPasskeyCache.kt[32-69]
- composeApp/src/commonMain/kotlin/tech/arnav/twofac/session/SessionPasskeyCache.kt[9-13]
- composeApp/src/commonMain/kotlin/tech/arnav/twofac/session/SessionManager.kt[12-29]
- composeApp/src/commonMain/kotlin/tech/arnav/twofac/viewmodels/AccountsViewModel.kt[103-114]
## Implementation direction (pick one)
1) **Preferred (correctness):** make `SessionPasskeyCache.write()`/`clear()` suspend, and update the call graph so the extension path can `await` `storage.session` writes (likely requiring `SessionManager.savePasskey` to become `suspend` and updating call sites).
2) **Alternative:** keep interfaces but move the *awaited* persistence into suspend call sites that already exist (e.g., a dedicated `suspend fun persistRetainedPasskey(...)` on the extension cache that `BrowserSessionManager` calls from its suspend unlock/enroll flows), and ensure the manual-passkey path (`savePasskey`) also performs a best-effort awaited write from a lifecycle-stable scope (background/page/service worker), not a popup-tied scope.
Include a test that simulates "write then new manager instance reads" (i.e., cross-reopen) and fails without awaiting persistence.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared, cross-platform “secure unlock retention” capability that can cache a decrypted vault passkey for the current session (app session on native/desktop; browser session for extensions), and exposes a settings toggle to control that behavior.
Changes:
- Added common contracts for retention policy/scope and a session-scoped passkey cache, plus helper methods to gate cache reads/writes based on policy/support.
- Wired Android, iOS, Desktop, and Browser session managers into the shared retention flow; browser extensions use
storage.sessionvia new WASM/TS interop. - Exposed the retention toggle in shared Settings UI and added multi-platform test coverage for the new behavior.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| composeApp/src/commonMain/kotlin/tech/arnav/twofac/session/SecureUnlockRetentionPolicy.kt | Adds shared retention policy/scope enums + retention-capable session manager contract + helper functions. |
| composeApp/src/commonMain/kotlin/tech/arnav/twofac/session/SessionPasskeyCache.kt | Introduces session passkey cache abstraction and in-memory implementation. |
| composeApp/src/wasmJsMain/typescript/src/storage.mts | Adds browser-extension storage.session interop (Firefox browser.* + Chrome chrome.*). |
| composeApp/src/wasmJsMain/kotlin/tech/arnav/twofac/session/interop/WebStorageClient.kt | Bridges new TS interop into Kotlin via an async extension session storage client. |
| composeApp/src/wasmJsMain/kotlin/tech/arnav/twofac/session/BrowserExtensionSessionPasskeyCache.kt | Implements session cache backed by extension storage.session with in-memory fallback. |
| composeApp/src/wasmJsMain/kotlin/tech/arnav/twofac/session/BrowserSessionManager.kt | Adds retention policy handling and cache-first unlock path for browser secure unlock. |
| composeApp/src/androidMain/kotlin/tech/arnav/twofac/session/AndroidBiometricSessionManager.kt | Adds retention policy persistence + session cache usage to skip repeated biometric prompts. |
| composeApp/src/iosMain/kotlin/tech/arnav/twofac/session/IosBiometricSessionManager.kt | Adds retention policy persistence + session cache usage to skip repeated biometric prompts. |
| composeApp/src/desktopMain/kotlin/tech/arnav/twofac/session/DesktopBiometricSessionManager.kt | Adds retention policy persistence + session cache usage to skip repeated secure unlock prompts. |
| composeApp/src/commonMain/kotlin/tech/arnav/twofac/viewmodels/SettingsViewModel.kt | Exposes retention support/policy/scope in UI state and handles toggle changes. |
| composeApp/src/commonMain/kotlin/tech/arnav/twofac/screens/SettingsScreen.kt | Renders retention toggle card when secure unlock is enabled and retention is supported. |
| composeApp/src/commonMain/kotlin/tech/arnav/twofac/components/settings/SessionRetentionCard.kt | Adds a dedicated settings card wrapper for the retention toggle. |
| composeApp/src/commonMain/composeResources/values/strings_settings.xml | Adds user-facing copy for app-session vs browser-session retention. |
| composeApp/src/commonTest/kotlin/tech/arnav/twofac/session/SessionPasskeyCacheTest.kt | Adds common tests validating retention helper behavior and cache semantics. |
| composeApp/src/commonTest/kotlin/tech/arnav/twofac/viewmodels/SettingsViewModelTest.kt | Adds tests ensuring retention state is surfaced and toggling updates UI/manager behavior. |
| composeApp/src/wasmJsTest/kotlin/tech/arnav/twofac/session/BrowserSessionManagerTest.kt | Adds browser tests for retained-cache hit vs unsupported-retention fallback behavior. |
| composeApp/src/iosTest/kotlin/tech/arnav/twofac/session/IosBiometricSessionManagerTest.kt | Updates iOS tests to validate availability behavior and retention policy persistence. |
| composeApp/src/androidInstrumentedTest/kotlin/tech/arnav/twofac/session/AndroidBiometricSessionManagerInstrumentedTest.kt | Extends Android instrumented tests to cover retention policy defaults + round-trip. |
| .agents/plans/42-cross-platform-session-auth-retention-plan.md | Documents the design/rollout plan and acceptance criteria for the feature. |
| import kotlinx.coroutines.CoroutineScope | ||
| import kotlinx.coroutines.SupervisorJob | ||
| import kotlinx.coroutines.await | ||
| import kotlinx.coroutines.launch | ||
| import tech.arnav.twofac.session.interop.BrowserExtensionSessionStorageClient | ||
| import tech.arnav.twofac.session.interop.ExtensionSessionStorageClient |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Summary
storage.sessionTesting