Merge branch 'main' into perf-mailbox-message-loading
This commit is contained in:
@@ -3,6 +3,7 @@ package org.libremail.ui.settings
|
||||
|
||||
import androidx.activity.ComponentActivity
|
||||
import androidx.compose.ui.test.junit4.createAndroidComposeRule
|
||||
import androidx.compose.ui.test.onAllNodesWithText
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.compose.ui.test.performScrollTo
|
||||
@@ -14,7 +15,6 @@ import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremail.R
|
||||
import org.libremail.data.security.AppLockAvailability
|
||||
import org.libremail.data.security.AppLockManager
|
||||
import org.libremail.data.security.DatabaseKeyCipher
|
||||
import org.libremail.data.security.DatabaseKeyStore
|
||||
@@ -29,8 +29,10 @@ import org.libremail.ui.theme.LibreMailTheme
|
||||
import javax.inject.Provider
|
||||
|
||||
/**
|
||||
* End-to-end test for the top-level message-downloading setting: tapping a policy must round-trip
|
||||
* through the real [SettingsRepository] (DataStore), proving the UI → ViewModel → persistence wiring.
|
||||
* End-to-end tests for the global settings screen. Tapping a policy must round-trip through the real
|
||||
* [SettingsRepository] (DataStore), proving the UI → ViewModel → persistence wiring; and enabling
|
||||
* app-lock on a device with no secure lock must surface the rejection message via a snackbar — now
|
||||
* visible to Compose semantics, unlike the former Toast it replaced.
|
||||
*/
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class SettingsScreenTest {
|
||||
@@ -38,27 +40,28 @@ class SettingsScreenTest {
|
||||
@get:Rule
|
||||
val composeTestRule = createAndroidComposeRule<ComponentActivity>()
|
||||
|
||||
private val context = InstrumentationRegistry.getInstrumentation().targetContext.applicationContext
|
||||
|
||||
// Device reports no secure lock, so enabling app-lock is rejected with a message.
|
||||
private val insecureDevice = object : AppLockManager {
|
||||
override fun isDeviceSecure() = false
|
||||
}
|
||||
|
||||
private fun string(resId: Int) = composeTestRule.activity.getString(resId)
|
||||
|
||||
@Test
|
||||
fun selectingFetchPolicy_persistsThroughTheRepository() {
|
||||
val context = InstrumentationRegistry.getInstrumentation().targetContext.applicationContext
|
||||
val settingsRepository = SettingsRepository(context)
|
||||
runBlocking { settingsRepository.setFetchPolicy(FetchPolicy.ALWAYS) } // known starting state
|
||||
val appLockManager = object : AppLockManager {
|
||||
override fun isDeviceSecure() = false
|
||||
override fun availability() = AppLockAvailability.NONE_ENROLLED
|
||||
}
|
||||
private fun settingsViewModel(settingsRepository: SettingsRepository): SettingsViewModel {
|
||||
val keyStore = DatabaseKeyStore(context, KeystoreCrypto(), DatabaseKeyCipher(), PassphraseSession())
|
||||
val viewModel = SettingsViewModel(
|
||||
return SettingsViewModel(
|
||||
FakeAccountRepository(),
|
||||
settingsRepository,
|
||||
appLockManager,
|
||||
insecureDevice,
|
||||
keyStore,
|
||||
BatteryOptimizationManager(context),
|
||||
SyncScheduler(Provider { WorkManager.getInstance(context) }),
|
||||
)
|
||||
}
|
||||
|
||||
private fun setContent(viewModel: SettingsViewModel) {
|
||||
composeTestRule.setContent {
|
||||
LibreMailTheme(darkTheme = false, dynamicColor = false) {
|
||||
SettingsScreen(
|
||||
@@ -70,6 +73,13 @@ class SettingsScreenTest {
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun selectingFetchPolicy_persistsThroughTheRepository() {
|
||||
val settingsRepository = SettingsRepository(context)
|
||||
runBlocking { settingsRepository.setFetchPolicy(FetchPolicy.ALWAYS) } // known starting state
|
||||
setContent(settingsViewModel(settingsRepository))
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.fetch_on_demand)).performScrollTo().performClick()
|
||||
|
||||
@@ -77,4 +87,20 @@ class SettingsScreenTest {
|
||||
runBlocking { settingsRepository.fetchPolicy() } == FetchPolicy.ON_DEMAND
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun enablingAppLockWithoutSecureDevice_showsRejectionSnackbar() {
|
||||
val settingsRepository = SettingsRepository(context)
|
||||
runBlocking { settingsRepository.setAppLock(false) } // known starting state: off
|
||||
setContent(settingsViewModel(settingsRepository))
|
||||
|
||||
// App-lock lives under the collapsed "Advanced" section: expand it, then toggle the switch on.
|
||||
composeTestRule.onNodeWithText(string(R.string.settings_advanced)).performScrollTo().performClick()
|
||||
composeTestRule.onNodeWithText(string(R.string.settings_adv_app_lock)).performScrollTo().performClick()
|
||||
|
||||
val message = string(R.string.app_lock_needs_device_lock)
|
||||
composeTestRule.waitUntil(5_000) {
|
||||
composeTestRule.onAllNodesWithText(message).fetchSemanticsNodes().isNotEmpty()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,40 +3,21 @@ package org.libremail.data.security
|
||||
|
||||
import android.app.KeyguardManager
|
||||
import android.content.Context
|
||||
import androidx.biometric.BiometricManager
|
||||
import androidx.biometric.BiometricManager.Authenticators.BIOMETRIC_STRONG
|
||||
import androidx.biometric.BiometricManager.Authenticators.DEVICE_CREDENTIAL
|
||||
import dagger.hilt.android.qualifiers.ApplicationContext
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
/** Whether a usable device authenticator (strong biometric or device credential) is available. */
|
||||
enum class AppLockAvailability {
|
||||
/** A biometric or device credential can be presented right now. */
|
||||
AVAILABLE,
|
||||
|
||||
/** Hardware exists but nothing (biometric or PIN/pattern/password) is enrolled. */
|
||||
NONE_ENROLLED,
|
||||
|
||||
/** No suitable authentication hardware, or it is temporarily unavailable. */
|
||||
NO_HARDWARE,
|
||||
|
||||
/** Availability could not be determined (unknown status / security update required). */
|
||||
UNAVAILABLE,
|
||||
}
|
||||
|
||||
/**
|
||||
* Queries device-lock and biometric availability for the app-lock feature. Real behavior depends on
|
||||
* the device keyguard and BiometricManager, so it is validated on-device, not in JVM tests.
|
||||
* Reports whether the device has a secure lock, for the app-lock feature. Real behavior depends on
|
||||
* the device keyguard, so it is validated on-device, not in JVM tests.
|
||||
*/
|
||||
interface AppLockManager {
|
||||
|
||||
/** True when the device has a secure lock screen (PIN, pattern, password, or biometric). */
|
||||
fun isDeviceSecure(): Boolean
|
||||
|
||||
/** Whether a biometric or device-credential prompt can currently be shown. */
|
||||
fun availability(): AppLockAvailability
|
||||
|
||||
companion object {
|
||||
/** Accept a strong biometric OR the device credential (PIN/pattern/password) as fallback. */
|
||||
const val AUTHENTICATORS: Int = BIOMETRIC_STRONG or DEVICE_CREDENTIAL
|
||||
@@ -50,14 +31,4 @@ class AndroidAppLockManager @Inject constructor(@ApplicationContext private val
|
||||
get() = context.getSystemService(Context.KEYGUARD_SERVICE) as? KeyguardManager
|
||||
|
||||
override fun isDeviceSecure(): Boolean = keyguardManager?.isDeviceSecure == true
|
||||
|
||||
override fun availability(): AppLockAvailability =
|
||||
when (BiometricManager.from(context).canAuthenticate(AppLockManager.AUTHENTICATORS)) {
|
||||
BiometricManager.BIOMETRIC_SUCCESS -> AppLockAvailability.AVAILABLE
|
||||
BiometricManager.BIOMETRIC_ERROR_NONE_ENROLLED -> AppLockAvailability.NONE_ENROLLED
|
||||
BiometricManager.BIOMETRIC_ERROR_NO_HARDWARE,
|
||||
BiometricManager.BIOMETRIC_ERROR_HW_UNAVAILABLE,
|
||||
-> AppLockAvailability.NO_HARDWARE
|
||||
else -> AppLockAvailability.UNAVAILABLE
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,13 +1,13 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui.lock
|
||||
|
||||
import androidx.activity.compose.LocalActivity
|
||||
import androidx.biometric.BiometricPrompt
|
||||
import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.foundation.layout.fillMaxSize
|
||||
import androidx.compose.material3.MaterialTheme
|
||||
import androidx.compose.material3.Surface
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.DisposableEffect
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
@@ -15,14 +15,12 @@ import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.input.pointer.pointerInput
|
||||
import androidx.compose.ui.platform.LocalContext
|
||||
import androidx.compose.ui.res.stringResource
|
||||
import androidx.core.content.ContextCompat
|
||||
import androidx.fragment.app.FragmentActivity
|
||||
import androidx.hilt.navigation.compose.hiltViewModel
|
||||
import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.LifecycleEventObserver
|
||||
import androidx.lifecycle.compose.LocalLifecycleOwner
|
||||
import androidx.lifecycle.compose.LifecycleEventEffect
|
||||
import androidx.lifecycle.compose.collectAsStateWithLifecycle
|
||||
import org.libremail.R
|
||||
import org.libremail.data.security.AppLockManager
|
||||
@@ -38,35 +36,29 @@ import org.libremail.data.security.AppLockManager
|
||||
@Composable
|
||||
fun AppLockGateHost(viewModel: AppLockViewModel = hiltViewModel(), content: @Composable () -> Unit) {
|
||||
val uiState by viewModel.uiState.collectAsStateWithLifecycle()
|
||||
val lifecycleOwner = LocalLifecycleOwner.current
|
||||
val context = LocalContext.current
|
||||
val activity = context.findFragmentActivity()
|
||||
|
||||
DisposableEffect(lifecycleOwner) {
|
||||
val observer = LifecycleEventObserver { _, event ->
|
||||
when (event) {
|
||||
Lifecycle.Event.ON_START -> viewModel.onForeground()
|
||||
Lifecycle.Event.ON_STOP -> viewModel.onBackground()
|
||||
else -> Unit
|
||||
}
|
||||
}
|
||||
lifecycleOwner.lifecycle.addObserver(observer)
|
||||
onDispose { lifecycleOwner.lifecycle.removeObserver(observer) }
|
||||
}
|
||||
LifecycleEventEffect(Lifecycle.Event.ON_START) { viewModel.onForeground() }
|
||||
LifecycleEventEffect(Lifecycle.Event.ON_STOP) { viewModel.onBackground() }
|
||||
|
||||
// BiometricPrompt must be hosted by a FragmentActivity; LocalActivity is the hosting Activity.
|
||||
val activity = LocalActivity.current
|
||||
val fragmentActivity = remember(activity) { activity as? FragmentActivity }
|
||||
|
||||
val promptTitle = stringResource(R.string.app_lock_prompt_title)
|
||||
val promptSubtitle = stringResource(R.string.app_lock_prompt_subtitle)
|
||||
val authenticate: () -> Unit = authenticate@{
|
||||
val host = activity ?: run {
|
||||
viewModel.onAuthError(null)
|
||||
return@authenticate
|
||||
val authenticate: () -> Unit = remember(fragmentActivity, viewModel, promptTitle, promptSubtitle) {
|
||||
authenticate@{
|
||||
val host = fragmentActivity ?: run {
|
||||
viewModel.onAuthError(null)
|
||||
return@authenticate
|
||||
}
|
||||
host.showAppLockPrompt(
|
||||
title = promptTitle,
|
||||
subtitle = promptSubtitle,
|
||||
onSuccess = viewModel::onAuthenticated,
|
||||
onError = { message -> viewModel.onAuthError(message) },
|
||||
)
|
||||
}
|
||||
host.showAppLockPrompt(
|
||||
title = promptTitle,
|
||||
subtitle = promptSubtitle,
|
||||
onSuccess = viewModel::onAuthenticated,
|
||||
onError = { message -> viewModel.onAuthError(message) },
|
||||
)
|
||||
}
|
||||
|
||||
// Once unlocked, keep [content] in the composition across later re-locks so its state (navigation
|
||||
@@ -149,9 +141,3 @@ private fun FragmentActivity.showAppLockPrompt(
|
||||
.build()
|
||||
prompt.authenticate(info)
|
||||
}
|
||||
|
||||
private tailrec fun android.content.Context.findFragmentActivity(): FragmentActivity? = when (this) {
|
||||
is FragmentActivity -> this
|
||||
is android.content.ContextWrapper -> baseContext.findFragmentActivity()
|
||||
else -> null
|
||||
}
|
||||
|
||||
@@ -97,8 +97,18 @@ class AppLockViewModel @Inject constructor(
|
||||
}
|
||||
}
|
||||
|
||||
private fun emitLocked(error: String? = null) {
|
||||
_uiState.value = AppLockUiState.Locked(error, ++lockSeq)
|
||||
/**
|
||||
* Single writer for the gated UI state: derive it from [AppLockGate.state] — the source of truth
|
||||
* for whether the session is locked — instead of hand-mirroring the gate at each call site. [error]
|
||||
* is a human-readable reason the previous unlock attempt failed. The transient
|
||||
* [AppLockUiState.Checking] cover and the "app-lock off / just disabled" unlocked states are driven
|
||||
* by settings/lifecycle rather than the session gate, so their sites set [_uiState] directly.
|
||||
*/
|
||||
private fun publish(error: String? = null) {
|
||||
_uiState.value = when (gate.state) {
|
||||
LockState.UNLOCKED -> AppLockUiState.Unlocked
|
||||
LockState.LOCKED -> AppLockUiState.Locked(error, ++lockSeq)
|
||||
}
|
||||
}
|
||||
|
||||
/** Recompute the lock state when the app comes to the foreground (lifecycle ON_START). */
|
||||
@@ -136,18 +146,15 @@ class AppLockViewModel @Inject constructor(
|
||||
LockAction.CLEAR_AND_REQUIRE_AUTH -> clearCacheAndRestart(disableAppLock = false)
|
||||
|
||||
LockAction.REQUIRE_AUTH -> {
|
||||
val state = gate.onForeground(foregroundAt, appLockEnabled = true)
|
||||
if (state == LockState.UNLOCKED) {
|
||||
_uiState.value = AppLockUiState.Unlocked
|
||||
} else {
|
||||
// Re-lock on grace expiry (timeout). The SQLCipher passphrase is intentionally
|
||||
// NOT evicted here: PassphraseSession is the only separately-held copy, but
|
||||
// clearing it flips EncryptedCacheGuard to "locked" and would stall background
|
||||
// sync/push while locked, and the already-open Room handle keeps the key
|
||||
// resident regardless. Full eviction needs the DB close/reopen owned by #93 /
|
||||
// #111 — see PassphraseSession's KDoc.
|
||||
emitLocked()
|
||||
}
|
||||
// Advance the gate for this foreground pass, then publish its decision (UNLOCKED
|
||||
// within the grace window, otherwise LOCKED). Re-locking on grace expiry
|
||||
// intentionally does NOT evict the SQLCipher passphrase: PassphraseSession is the
|
||||
// only separately-held copy, but clearing it flips EncryptedCacheGuard to "locked"
|
||||
// and would stall background sync/push while locked, and the already-open Room handle
|
||||
// keeps the key resident regardless. Full eviction needs the DB close/reopen owned by
|
||||
// #93 / #111 — see PassphraseSession's KDoc.
|
||||
gate.onForeground(foregroundAt, appLockEnabled = true)
|
||||
publish()
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -169,7 +176,7 @@ class AppLockViewModel @Inject constructor(
|
||||
when (withContext(Dispatchers.Default) { unlockOrArm() }) {
|
||||
UnlockResult.OK -> {
|
||||
gate.onAuthenticated()
|
||||
_uiState.value = AppLockUiState.Unlocked
|
||||
publish()
|
||||
}
|
||||
|
||||
UnlockResult.UNRECOVERABLE -> {
|
||||
@@ -181,7 +188,7 @@ class AppLockViewModel @Inject constructor(
|
||||
|
||||
UnlockResult.RETRY -> {
|
||||
gate.lock()
|
||||
emitLocked(context.getString(R.string.app_lock_unlock_failed))
|
||||
publish(context.getString(R.string.app_lock_unlock_failed))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -190,7 +197,7 @@ class AppLockViewModel @Inject constructor(
|
||||
/** Called by the host when the prompt is cancelled or errors. */
|
||||
fun onAuthError(message: String?) {
|
||||
gate.lock()
|
||||
emitLocked(message)
|
||||
publish(message)
|
||||
}
|
||||
|
||||
/** Outcome of unwrapping/arming the auth-bound passphrase after a successful device auth. */
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui.settings
|
||||
|
||||
import android.widget.Toast
|
||||
import androidx.compose.animation.AnimatedVisibility
|
||||
import androidx.compose.foundation.clickable
|
||||
import androidx.compose.foundation.layout.Column
|
||||
@@ -18,11 +17,14 @@ import androidx.compose.material3.HorizontalDivider
|
||||
import androidx.compose.material3.Icon
|
||||
import androidx.compose.material3.MaterialTheme
|
||||
import androidx.compose.material3.Scaffold
|
||||
import androidx.compose.material3.SnackbarHost
|
||||
import androidx.compose.material3.SnackbarHostState
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.material3.TopAppBar
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.draw.rotate
|
||||
@@ -55,10 +57,14 @@ fun SettingsScreen(
|
||||
val batteryUnrestricted by viewModel.batteryUnrestricted.collectAsStateWithLifecycle()
|
||||
val context = LocalContext.current
|
||||
val resources = LocalResources.current
|
||||
val snackbarHostState = remember { SnackbarHostState() }
|
||||
|
||||
// Surface a rejected app-lock toggle via the canonical snackbar pattern (matches MailboxScreen).
|
||||
// The ViewModel holds the @StringRes id; resolve it here via LocalResources (so it re-resolves on
|
||||
// configuration changes) at the display boundary, then clear it.
|
||||
LaunchedEffect(appLockMessage) {
|
||||
appLockMessage?.let {
|
||||
Toast.makeText(context, resources.getString(it), Toast.LENGTH_LONG).show()
|
||||
appLockMessage?.let { messageId ->
|
||||
snackbarHostState.showSnackbar(resources.getString(messageId))
|
||||
viewModel.clearAppLockMessage()
|
||||
}
|
||||
}
|
||||
@@ -69,6 +75,7 @@ fun SettingsScreen(
|
||||
Scaffold(
|
||||
topBar = { TopAppBar(title = { Text(stringResource(R.string.title_settings)) }) },
|
||||
bottomBar = { LibreMailBottomBar(current = TopDest.SETTINGS, onSelect = onSelectTab) },
|
||||
snackbarHost = { SnackbarHost(snackbarHostState) },
|
||||
) { padding ->
|
||||
Column(
|
||||
modifier = Modifier
|
||||
|
||||
Reference in New Issue
Block a user