Merge pull request #1757 from vector-im/feature/fga/lock_polish

LockScreen polish
This commit is contained in:
ganfra
2023-11-08 14:02:53 +01:00
committed by GitHub
29 changed files with 264 additions and 124 deletions

View File

@@ -35,8 +35,12 @@ import io.element.android.services.appnavstate.api.AppForegroundStateService
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.launch
import javax.inject.Inject
import kotlin.time.Duration
@@ -113,14 +117,23 @@ class DefaultLockScreenService @Inject constructor(
}
}
override suspend fun isSetupRequired(): Boolean {
return lockScreenConfig.isPinMandatory
&& featureFlagService.isFeatureEnabled(FeatureFlags.PinUnlock)
&& !pinCodeManager.isPinCodeAvailable()
override fun isPinSetup(): Flow<Boolean> {
return combine(
featureFlagService.isFeatureEnabledFlow(FeatureFlags.PinUnlock),
pinCodeManager.hasPinCode()
) { isEnabled, hasPinCode ->
isEnabled && hasPinCode
}
}
override fun isSetupRequired(): Flow<Boolean> {
return isPinSetup().map { isPinSetup ->
!isPinSetup && lockScreenConfig.isPinMandatory
}
}
private fun CoroutineScope.lockIfNeeded(gracePeriod: Duration = Duration.ZERO) = launch {
if (featureFlagService.isFeatureEnabled(FeatureFlags.PinUnlock) && pinCodeManager.isPinCodeAvailable()) {
if (isPinSetup().first()) {
delay(gracePeriod)
_lockScreenState.value = LockScreenLockState.Locked
}

View File

@@ -20,7 +20,6 @@ import android.os.Parcelable
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import com.bumble.appyx.core.composable.Children
import com.bumble.appyx.core.lifecycle.subscribe
import com.bumble.appyx.core.modality.BuildContext
import com.bumble.appyx.core.node.Node
import com.bumble.appyx.core.plugin.Plugin
@@ -30,8 +29,6 @@ import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import io.element.android.anvilannotations.ContributesNode
import io.element.android.features.lockscreen.api.LockScreenEntryPoint
import io.element.android.features.lockscreen.impl.pin.DefaultPinCodeManagerCallback
import io.element.android.features.lockscreen.impl.pin.PinCodeManager
import io.element.android.features.lockscreen.impl.settings.LockScreenSettingsFlowNode
import io.element.android.features.lockscreen.impl.setup.LockScreenSetupFlowNode
import io.element.android.features.lockscreen.impl.unlock.PinUnlockNode
@@ -46,7 +43,6 @@ import kotlinx.parcelize.Parcelize
class LockScreenFlowNode @AssistedInject constructor(
@Assisted buildContext: BuildContext,
@Assisted plugins: List<Plugin>,
private val pinCodeManager: PinCodeManager,
) : BackstackNode<LockScreenFlowNode.NavTarget>(
backstack = BackStack(
initialElement = plugins.filterIsInstance(Inputs::class.java).first().initialNavTarget,
@@ -71,26 +67,14 @@ class LockScreenFlowNode @AssistedInject constructor(
data object Settings : NavTarget
}
private val pinCodeManagerCallback = object : DefaultPinCodeManagerCallback() {
override fun onPinCodeCreated() {
plugins<LockScreenEntryPoint.Callback>().forEach {
it.onSetupCompleted()
private class OnSetupDoneCallback(private val plugins: List<LockScreenEntryPoint.Callback>) : LockScreenSetupFlowNode.Callback {
override fun onSetupDone() {
plugins.forEach {
it.onSetupDone()
}
}
}
override fun onBuilt() {
super.onBuilt()
lifecycle.subscribe(
onCreate = {
pinCodeManager.addCallback(pinCodeManagerCallback)
},
onDestroy = {
pinCodeManager.removeCallback(pinCodeManagerCallback)
}
)
}
override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
return when (navTarget) {
NavTarget.Unlock -> {
@@ -98,7 +82,8 @@ class LockScreenFlowNode @AssistedInject constructor(
createNode<PinUnlockNode>(buildContext, plugins = listOf(inputs))
}
NavTarget.Setup -> {
createNode<LockScreenSetupFlowNode>(buildContext)
val callback = OnSetupDoneCallback(plugins())
createNode<LockScreenSetupFlowNode>(buildContext, plugins = listOf(callback))
}
NavTarget.Settings -> {
createNode<LockScreenSettingsFlowNode>(buildContext)

View File

@@ -24,6 +24,7 @@ import androidx.core.content.ContextCompat
import androidx.fragment.app.FragmentActivity
import io.element.android.libraries.cryptography.api.EncryptionDecryptionService
import io.element.android.libraries.cryptography.api.SecretKeyRepository
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CompletableDeferred
import timber.log.Timber
import java.security.InvalidKeyException
@@ -86,7 +87,12 @@ class DefaultBiometricUnlock(
val callback = AuthenticationCallback(callbacks, deferredAuthenticationResult)
val prompt = BiometricPrompt(activity, executor, callback)
prompt.authenticate(promptInfo, cryptoObject)
return deferredAuthenticationResult.await()
return try {
deferredAuthenticationResult.await()
} catch (cancellation: CancellationException) {
prompt.cancelAuthentication()
BiometricUnlock.AuthenticationResult.Failure(cancellation)
}
}
@Throws(KeyPermanentlyInvalidatedException::class)
@@ -110,7 +116,6 @@ private class AuthenticationCallback(
override fun onAuthenticationFailed() {
super.onAuthenticationFailed()
callbacks.forEach { it.onBiometricUnlockFailed(null) }
deferredAuthenticationResult.complete(BiometricUnlock.AuthenticationResult.Failure(null))
}
override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) {

View File

@@ -23,6 +23,7 @@ import io.element.android.libraries.cryptography.api.EncryptionResult
import io.element.android.libraries.cryptography.api.SecretKeyRepository
import io.element.android.libraries.di.AppScope
import io.element.android.libraries.di.SingleIn
import kotlinx.coroutines.flow.Flow
import java.util.concurrent.CopyOnWriteArrayList
import javax.inject.Inject
@@ -46,7 +47,7 @@ class DefaultPinCodeManager @Inject constructor(
callbacks.remove(callback)
}
override suspend fun isPinCodeAvailable(): Boolean {
override fun hasPinCode(): Flow<Boolean> {
return lockScreenStore.hasPinCode()
}

View File

@@ -16,6 +16,8 @@
package io.element.android.features.lockscreen.impl.pin
import kotlinx.coroutines.flow.Flow
/**
* This interface is the main interface to manage the pin code.
* Implementation should take care of encrypting the pin code and storing it.
@@ -55,7 +57,7 @@ interface PinCodeManager {
/**
* @return true if a pin code is available.
*/
suspend fun isPinCodeAvailable(): Boolean
fun hasPinCode(): Flow<Boolean>
/**
* @return the size of the saved pin code.

View File

@@ -32,16 +32,15 @@ import com.bumble.appyx.navmodel.backstack.operation.push
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import io.element.android.anvilannotations.ContributesNode
import io.element.android.features.lockscreen.impl.biometric.BiometricUnlockManager
import io.element.android.features.lockscreen.impl.biometric.DefaultBiometricUnlockCallback
import io.element.android.features.lockscreen.impl.pin.DefaultPinCodeManagerCallback
import io.element.android.features.lockscreen.impl.pin.PinCodeManager
import io.element.android.features.lockscreen.impl.setup.LockScreenSetupFlowNode
import io.element.android.features.lockscreen.impl.setup.pin.SetupPinNode
import io.element.android.features.lockscreen.impl.unlock.PinUnlockNode
import io.element.android.libraries.architecture.BackstackNode
import io.element.android.libraries.architecture.animation.rememberDefaultTransitionHandler
import io.element.android.libraries.architecture.createNode
import io.element.android.libraries.di.SessionScope
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
import kotlinx.parcelize.Parcelize
@@ -50,7 +49,6 @@ class LockScreenSettingsFlowNode @AssistedInject constructor(
@Assisted buildContext: BuildContext,
@Assisted plugins: List<Plugin>,
private val pinCodeManager: PinCodeManager,
private val biometricUnlockManager: BiometricUnlockManager,
) : BackstackNode<LockScreenSettingsFlowNode.NavTarget>(
backstack = BackStack(
initialElement = NavTarget.Unknown,
@@ -68,44 +66,39 @@ class LockScreenSettingsFlowNode @AssistedInject constructor(
data object Unlock : NavTarget
@Parcelize
data object Setup : NavTarget
data object SetupPin : NavTarget
@Parcelize
data object Settings : NavTarget
}
private val pinCodeManagerCallback = object : DefaultPinCodeManagerCallback() {
override fun onPinCodeVerified() {
backstack.newRoot(NavTarget.Settings)
}
override fun onPinCodeRemoved() {
navigateUp()
}
}
private val biometricUnlockCallback = object : DefaultBiometricUnlockCallback() {
override fun onBiometricUnlockSuccess() {
override fun onPinCodeCreated() {
backstack.newRoot(NavTarget.Settings)
}
}
init {
override fun onBuilt() {
super.onBuilt()
lifecycleScope.launch {
if (pinCodeManager.isPinCodeAvailable()) {
val hasPinCode = pinCodeManager.hasPinCode().first()
if (hasPinCode) {
backstack.newRoot(NavTarget.Unlock)
} else {
backstack.newRoot(NavTarget.Setup)
backstack.newRoot(NavTarget.SetupPin)
}
}
lifecycle.subscribe(
onCreate = {
pinCodeManager.addCallback(pinCodeManagerCallback)
biometricUnlockManager.addCallback(biometricUnlockCallback)
},
onDestroy = {
pinCodeManager.removeCallback(pinCodeManagerCallback)
biometricUnlockManager.removeCallback(biometricUnlockCallback)
}
)
}
@@ -114,25 +107,26 @@ class LockScreenSettingsFlowNode @AssistedInject constructor(
return when (navTarget) {
NavTarget.Unlock -> {
val inputs = PinUnlockNode.Inputs(isInAppUnlock = true)
createNode<PinUnlockNode>(buildContext, plugins = listOf(inputs))
}
NavTarget.Setup -> {
val callback = object : LockScreenSetupFlowNode.Callback {
override fun onSetupDone() {
val callback = object : PinUnlockNode.Callback {
override fun onUnlock() {
backstack.newRoot(NavTarget.Settings)
}
}
createNode<LockScreenSetupFlowNode>(buildContext, plugins = listOf(callback))
createNode<PinUnlockNode>(buildContext, plugins = listOf(inputs, callback))
}
NavTarget.SetupPin -> {
createNode<SetupPinNode>(buildContext)
}
NavTarget.Settings -> {
val callback = object : LockScreenSettingsNode.Callback {
override fun onChangePinClicked() {
backstack.push(NavTarget.Setup)
backstack.push(NavTarget.SetupPin)
}
}
createNode<LockScreenSettingsNode>(buildContext, plugins = listOf(callback))
}
NavTarget.Unknown -> node(buildContext) { }
}
}

View File

@@ -17,11 +17,10 @@
package io.element.android.features.lockscreen.impl.settings
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableIntStateOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.produceState
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import io.element.android.appconfig.LockScreenConfig
@@ -43,23 +42,15 @@ class LockScreenSettingsPresenter @Inject constructor(
@Composable
override fun present(): LockScreenSettingsState {
var triggerComputation by remember {
mutableIntStateOf(0)
}
var showRemovePinOption by remember {
mutableStateOf(false)
}
var showToggleBiometric by remember {
mutableStateOf(false)
val showRemovePinOption by produceState(initialValue = false) {
pinCodeManager.hasPinCode().collect { hasPinCode ->
value = !lockScreenConfig.isPinMandatory && hasPinCode
}
}
val isBiometricEnabled by lockScreenStore.isBiometricUnlockAllowed().collectAsState(initial = false)
var showRemovePinConfirmation by remember {
mutableStateOf(false)
}
LaunchedEffect(triggerComputation) {
showRemovePinOption = !lockScreenConfig.isPinMandatory && pinCodeManager.isPinCodeAvailable()
showToggleBiometric = biometricUnlockManager.isDeviceSecured
}
fun handleEvents(event: LockScreenSettingsEvents) {
when (event) {
@@ -69,7 +60,6 @@ class LockScreenSettingsPresenter @Inject constructor(
if (showRemovePinConfirmation) {
showRemovePinConfirmation = false
pinCodeManager.deletePinCode()
triggerComputation++
}
}
}
@@ -86,7 +76,7 @@ class LockScreenSettingsPresenter @Inject constructor(
showRemovePinOption = showRemovePinOption,
isBiometricEnabled = isBiometricEnabled,
showRemovePinConfirmation = showRemovePinConfirmation,
showToggleBiometric = showToggleBiometric,
showToggleBiometric = biometricUnlockManager.isDeviceSecured,
eventSink = ::handleEvents
)
}

View File

@@ -17,6 +17,6 @@
package io.element.android.features.lockscreen.impl.setup.pin
sealed interface SetupPinEvents {
data class OnPinEntryChanged(val entryAsText: String) : SetupPinEvents
data class OnPinEntryChanged(val entryAsText: String, val fromConfirmationStep: Boolean) : SetupPinEvents
data object ClearFailure : SetupPinEvents
}

View File

@@ -32,6 +32,11 @@ import io.element.android.libraries.core.meta.BuildMeta
import kotlinx.coroutines.delay
import javax.inject.Inject
/**
* Some time for the ui to refresh before showing confirmation step.
*/
private const val DELAY_BEFORE_CONFIRMATION_STEP_IN_MILLIS = 100L
class SetupPinPresenter @Inject constructor(
private val lockScreenConfig: LockScreenConfig,
private val pinValidator: PinValidator,
@@ -60,8 +65,7 @@ class SetupPinPresenter @Inject constructor(
setupPinFailure = pinValidationResult.failure
}
PinValidator.Result.Valid -> {
// Leave some time for the ui to refresh before showing confirmation
delay(150)
delay(DELAY_BEFORE_CONFIRMATION_STEP_IN_MILLIS)
isConfirmationStep = true
}
}
@@ -81,7 +85,8 @@ class SetupPinPresenter @Inject constructor(
fun handleEvents(event: SetupPinEvents) {
when (event) {
is SetupPinEvents.OnPinEntryChanged -> {
if (isConfirmationStep) {
// Use the fromConfirmationStep flag from ui to avoid race condition.
if (event.fromConfirmationStep) {
confirmPinEntry = confirmPinEntry.fillWith(event.entryAsText)
} else {
choosePinEntry = choosePinEntry.fillWith(event.entryAsText)

View File

@@ -116,8 +116,8 @@ private fun SetupPinContent(
PinEntryTextField(
pinEntry = state.activePinEntry,
isSecured = true,
onValueChange = {
state.eventSink(SetupPinEvents.OnPinEntryChanged(it))
onValueChange = { entry ->
state.eventSink(SetupPinEvents.OnPinEntryChanged(entry, state.isConfirmationStep))
},
modifier = modifier
.focusRequester(focusRequester)

View File

@@ -16,6 +16,8 @@
package io.element.android.features.lockscreen.impl.storage
import kotlinx.coroutines.flow.Flow
/**
* Should be implemented by any class that provides access to the encrypted PIN code.
* All methods are suspending in case there are async IO operations involved.
@@ -39,5 +41,6 @@ interface EncryptedPinCodeStorage {
/**
* Returns whether the PIN code is stored or not.
*/
suspend fun hasPinCode(): Boolean
fun hasPinCode(): Flow<Boolean>
}

View File

@@ -85,10 +85,10 @@ class PreferencesLockScreenStore @Inject constructor(
}
}
override suspend fun hasPinCode(): Boolean {
override fun hasPinCode(): Flow<Boolean> {
return context.dataStore.data.map { preferences ->
preferences[pinCodeKey] != null
}.first()
}
}
override fun isBiometricUnlockAllowed(): Flow<Boolean> {

View File

@@ -0,0 +1,53 @@
/*
* Copyright (c) 2023 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.features.lockscreen.impl.unlock
import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
import io.element.android.features.lockscreen.impl.biometric.BiometricUnlockManager
import io.element.android.features.lockscreen.impl.biometric.DefaultBiometricUnlockCallback
import io.element.android.features.lockscreen.impl.pin.DefaultPinCodeManagerCallback
import io.element.android.features.lockscreen.impl.pin.PinCodeManager
import javax.inject.Inject
class PinUnlockHelper @Inject constructor(
private val biometricUnlockManager: BiometricUnlockManager,
private val pinCodeManager: PinCodeManager
) {
@Composable
fun OnUnlockEffect(onUnlock: () -> Unit) {
DisposableEffect(Unit) {
val biometricUnlockCallback = object : DefaultBiometricUnlockCallback() {
override fun onBiometricUnlockSuccess() {
onUnlock()
}
}
val pinCodeVerifiedCallback = object : DefaultPinCodeManagerCallback() {
override fun onPinCodeVerified() {
onUnlock()
}
}
biometricUnlockManager.addCallback(biometricUnlockCallback)
pinCodeManager.addCallback(pinCodeVerifiedCallback)
onDispose {
biometricUnlockManager.removeCallback(biometricUnlockCallback)
pinCodeManager.removeCallback(pinCodeVerifiedCallback)
}
}
}
}

View File

@@ -17,10 +17,12 @@
package io.element.android.features.lockscreen.impl.unlock
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.ui.Modifier
import com.bumble.appyx.core.modality.BuildContext
import com.bumble.appyx.core.node.Node
import com.bumble.appyx.core.plugin.Plugin
import com.bumble.appyx.core.plugin.plugins
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import io.element.android.anvilannotations.ContributesNode
@@ -35,15 +37,30 @@ class PinUnlockNode @AssistedInject constructor(
private val presenter: PinUnlockPresenter,
) : Node(buildContext, plugins = plugins) {
interface Callback : Plugin {
fun onUnlock()
}
data class Inputs(
val isInAppUnlock: Boolean
) : NodeInputs
private val inputs: Inputs = inputs()
private fun onUnlock() {
plugins<Callback>().forEach {
it.onUnlock()
}
}
@Composable
override fun View(modifier: Modifier) {
val state = presenter.present()
LaunchedEffect(state.isUnlocked) {
if (state.isUnlocked) {
onUnlock()
}
}
PinUnlockView(
state = state,
isInAppUnlock = inputs.isInAppUnlock,

View File

@@ -43,6 +43,7 @@ class PinUnlockPresenter @Inject constructor(
private val biometricUnlockManager: BiometricUnlockManager,
private val matrixClient: MatrixClient,
private val coroutineScope: CoroutineScope,
private val pinUnlockHelper: PinUnlockHelper,
) : Presenter<PinUnlockState> {
@Composable
@@ -66,9 +67,10 @@ class PinUnlockPresenter @Inject constructor(
var biometricUnlockResult by remember {
mutableStateOf<BiometricUnlock.AuthenticationResult?>(null)
}
val isUnlocked = remember {
mutableStateOf(false)
}
val biometricUnlock = biometricUnlockManager.rememberBiometricUnlock()
LaunchedEffect(Unit) {
suspend {
val pinCodeSize = pinCodeManager.getPinCodeSize()
@@ -94,6 +96,9 @@ class PinUnlockPresenter @Inject constructor(
showSignOutPrompt = true
}
}
pinUnlockHelper.OnUnlockEffect {
isUnlocked.value = true
}
fun handleEvents(event: PinUnlockEvents) {
when (event) {
@@ -129,6 +134,7 @@ class PinUnlockPresenter @Inject constructor(
signOutAction = signOutAction.value,
showBiometricUnlock = biometricUnlock.isActive,
biometricUnlockResult = biometricUnlockResult,
isUnlocked = isUnlocked.value,
eventSink = ::handleEvents
)
}

View File

@@ -28,6 +28,7 @@ data class PinUnlockState(
val showSignOutPrompt: Boolean,
val signOutAction: Async<String?>,
val showBiometricUnlock: Boolean,
val isUnlocked: Boolean,
val biometricUnlockResult: BiometricUnlock.AuthenticationResult?,
val eventSink: (PinUnlockEvents) -> Unit
) {

View File

@@ -41,6 +41,7 @@ fun aPinUnlockState(
showSignOutPrompt: Boolean = false,
showBiometricUnlock: Boolean = true,
biometricUnlockResult: BiometricUnlock.AuthenticationResult? = null,
isUnlocked: Boolean = false,
signOutAction: Async<String?> = Async.Uninitialized,
) = PinUnlockState(
pinEntry = Async.Success(pinEntry),
@@ -50,5 +51,6 @@ fun aPinUnlockState(
showBiometricUnlock = showBiometricUnlock,
signOutAction = signOutAction,
biometricUnlockResult = biometricUnlockResult,
isUnlocked = isUnlocked,
eventSink = {}
)