Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 426a11d7 authored by burakov's avatar burakov Committed by Danny Burakov
Browse files

[flexiglass] Bouncer refactoring, improvements and fixes.

1. Store `AuthMethodBouncerViewModel`s polymorphically.

2. Extract most of the common logic of the bouncer sub-view-models into
the parent `AuthMethodBouncerViewModel`, to ensure behavior consistency
and remove some code duplication. This fixes some bugs like the error
message not always being cleared when the input is cleared.

3. Represent the authentication attempt result more explicitly via a
dedicated enum, to prevent confusion when handling a `null` vs `false`
result.

4. Fix a memory leak when switching the authentication method.

5. Assign child view models of `BouncerViewModel` a temporary coroutine
scope, so we can dispose of them properly when the authentication method
is switched.

6. Move the "skip-authentication" business logic from `BouncerViewModel`
to `BouncerInteractor`.

7. Remove `minPatternLength` from `BouncerViewModel` and
`BouncerInteractor`.

8. Make view-model unit tests more concise by extracting the setup code
(setting the authentication method, locking the device and opening the
bouncer) to a helper function.

Fix: 300441917
Test: Unit tests still pass.
Test: Manually tested the 3 different bouncer auth methods and verified
 they still work as expected.
Change-Id: I711df14547d8b10caf275890b4de6cbdd3177c36
parent 3d505196
Loading
Loading
Loading
Loading
+3 −2
Original line number Diff line number Diff line
@@ -48,13 +48,13 @@ import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.dp
import com.android.compose.animation.scene.ElementKey
import com.android.compose.animation.scene.SceneScope
import com.android.systemui.res.R
import com.android.systemui.bouncer.ui.viewmodel.AuthMethodBouncerViewModel
import com.android.systemui.bouncer.ui.viewmodel.BouncerViewModel
import com.android.systemui.bouncer.ui.viewmodel.PasswordBouncerViewModel
import com.android.systemui.bouncer.ui.viewmodel.PatternBouncerViewModel
import com.android.systemui.bouncer.ui.viewmodel.PinBouncerViewModel
import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.res.R
import com.android.systemui.scene.shared.model.Direction
import com.android.systemui.scene.shared.model.SceneKey
import com.android.systemui.scene.shared.model.SceneModel
@@ -104,7 +104,8 @@ private fun SceneScope.BouncerScene(
    modifier: Modifier = Modifier,
) {
    val message: BouncerViewModel.MessageViewModel by viewModel.message.collectAsState()
    val authMethodViewModel: AuthMethodBouncerViewModel? by viewModel.authMethod.collectAsState()
    val authMethodViewModel: AuthMethodBouncerViewModel? by
        viewModel.authMethodViewModel.collectAsState()
    val dialogMessage: String? by viewModel.throttlingDialogMessage.collectAsState()
    var dialog: Dialog? by remember { mutableStateOf(null) }
    val backgroundColor = MaterialTheme.colorScheme.surface
+34 −13
Original line number Diff line number Diff line
@@ -185,9 +185,6 @@ constructor(
    /** Whether the pattern should be visible for the currently-selected user. */
    val isPatternVisible: StateFlow<Boolean> = repository.isPatternVisible

    /** The minimal length of a pattern. */
    val minPatternLength: Int = repository.minPatternLength

    private var throttlingCountdownJob: Job? = null

    init {
@@ -243,39 +240,46 @@ constructor(
     * Attempts to authenticate the user and unlock the device.
     *
     * If [tryAutoConfirm] is `true`, authentication is attempted if and only if the auth method
     * supports auto-confirming, and the input's length is at least the code's length. Otherwise,
     * `null` is returned.
     * supports auto-confirming, and the input's length is at least the required length. Otherwise,
     * `AuthenticationResult.SKIPPED` is returned.
     *
     * @param input The input from the user to try to authenticate with. This can be a list of
     *   different things, based on the current authentication method.
     * @param tryAutoConfirm `true` if called while the user inputs the code, without an explicit
     *   request to validate.
     * @return `true` if the authentication succeeded and the device is now unlocked; `false` when
     *   authentication failed, `null` if the check was not performed.
     * @return The result of this authentication attempt.
     */
    suspend fun authenticate(input: List<Any>, tryAutoConfirm: Boolean = false): Boolean? {
    suspend fun authenticate(
        input: List<Any>,
        tryAutoConfirm: Boolean = false
    ): AuthenticationResult {
        if (input.isEmpty()) {
            throw IllegalArgumentException("Input was empty!")
        }

        val authMethod = getAuthenticationMethod()
        val skipCheck =
            when {
                // We're being throttled, the UI layer should not have called this; skip the
                // attempt.
                isThrottled.value -> true
                // The pattern is too short; skip the attempt.
                authMethod == DomainLayerAuthenticationMethodModel.Pattern &&
                    input.size < repository.minPatternLength -> true
                // Auto-confirm attempt when the feature is not enabled; skip the attempt.
                tryAutoConfirm && !isAutoConfirmEnabled.value -> true
                // Auto-confirm should skip the attempt if the pin entered is too short.
                tryAutoConfirm && input.size < repository.getPinLength() -> true
                tryAutoConfirm &&
                    authMethod == DomainLayerAuthenticationMethodModel.Pin &&
                    input.size < repository.getPinLength() -> true
                else -> false
            }
        if (skipCheck) {
            return null
            return AuthenticationResult.SKIPPED
        }

        // Attempt to authenticate:
        val authMethod = getAuthenticationMethod()
        val credential = authMethod.createCredential(input) ?: return null
        val credential = authMethod.createCredential(input) ?: return AuthenticationResult.SKIPPED
        val authenticationResult = repository.checkCredential(credential)
        credential.zeroize()

@@ -299,7 +303,11 @@ constructor(
            refreshThrottling()
        }

        return authenticationResult.isSuccessful
        return if (authenticationResult.isSuccessful) {
            AuthenticationResult.SUCCEEDED
        } else {
            AuthenticationResult.FAILED
        }
    }

    /** Starts refreshing the throttling state every second. */
@@ -383,3 +391,16 @@ constructor(
        }
    }
}

/** Result of a user authentication attempt. */
enum class AuthenticationResult {
    /** Authentication succeeded and the device is now unlocked. */
    SUCCEEDED,
    /** Authentication failed and the device remains unlocked. */
    FAILED,
    /**
     * Authentication was not performed, e.g. due to insufficient input, and the device remains
     * unlocked.
     */
    SKIPPED,
}
+32 −22
Original line number Diff line number Diff line
@@ -17,8 +17,8 @@
package com.android.systemui.bouncer.domain.interactor

import android.content.Context
import com.android.systemui.res.R
import com.android.systemui.authentication.domain.interactor.AuthenticationInteractor
import com.android.systemui.authentication.domain.interactor.AuthenticationResult
import com.android.systemui.authentication.domain.model.AuthenticationMethodModel
import com.android.systemui.authentication.shared.model.AuthenticationThrottlingModel
import com.android.systemui.bouncer.data.repository.BouncerRepository
@@ -26,6 +26,7 @@ import com.android.systemui.classifier.FalsingClassifier
import com.android.systemui.classifier.domain.interactor.FalsingInteractor
import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.dagger.qualifiers.Application
import com.android.systemui.res.R
import com.android.systemui.scene.domain.interactor.SceneInteractor
import com.android.systemui.scene.shared.flag.SceneContainerFlags
import com.android.systemui.scene.shared.model.SceneKey
@@ -34,6 +35,7 @@ import com.android.systemui.util.kotlin.pairwise
import javax.inject.Inject
import kotlin.time.Duration.Companion.milliseconds
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.async
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
@@ -92,9 +94,6 @@ constructor(
    /** Whether the pattern should be visible for the currently-selected user. */
    val isPatternVisible: StateFlow<Boolean> = authenticationInteractor.isPatternVisible

    /** The minimal length of a pattern. */
    val minPatternLength = authenticationInteractor.minPatternLength

    init {
        if (flags.isEnabled()) {
            // Clear the message if moved from throttling to no-longer throttling.
@@ -184,33 +183,44 @@ constructor(
     * dismissed and hidden.
     *
     * If [tryAutoConfirm] is `true`, authentication is attempted if and only if the auth method
     * supports auto-confirming, and the input's length is at least the code's length. Otherwise,
     * `null` is returned.
     * supports auto-confirming, and the input's length is at least the required length. Otherwise,
     * `AuthenticationResult.SKIPPED` is returned.
     *
     * @param input The input from the user to try to authenticate with. This can be a list of
     *   different things, based on the current authentication method.
     * @param tryAutoConfirm `true` if called while the user inputs the code, without an explicit
     *   request to validate.
     * @return `true` if the authentication succeeded and the device is now unlocked; `false` when
     *   authentication failed, `null` if the check was not performed.
     * @return The result of this authentication attempt.
     */
    suspend fun authenticate(
        input: List<Any>,
        tryAutoConfirm: Boolean = false,
    ): Boolean? {
        val isAuthenticated =
            authenticationInteractor.authenticate(input, tryAutoConfirm) ?: return null

        if (isAuthenticated) {
    ): AuthenticationResult {
        if (input.isEmpty()) {
            return AuthenticationResult.SKIPPED
        }
        // Switching to the application scope here since this method is often called from
        // view-models, whose lifecycle (and thus scope) is shorter than this interactor.
        // This allows the task to continue running properly even when the calling scope has been
        // cancelled.
        return applicationScope
            .async {
                val authResult = authenticationInteractor.authenticate(input, tryAutoConfirm)
                when (authResult) {
                    // Authentication succeeded.
                    AuthenticationResult.SUCCEEDED ->
                        sceneInteractor.changeScene(
                            scene = SceneModel(SceneKey.Gone),
                            loggingReason = "successful authentication",
                        )
        } else {
            showErrorMessage()
                    // Authentication failed.
                    AuthenticationResult.FAILED -> showErrorMessage()
                    // Authentication skipped.
                    AuthenticationResult.SKIPPED -> if (!tryAutoConfirm) showErrorMessage()
                }

        return isAuthenticated
                authResult
            }
            .await()
    }

    /**
@@ -221,7 +231,7 @@ constructor(
     * For example, if the user entered a pattern that's too short, the system can show the error
     * message without having the attempt trigger throttling.
     */
    suspend fun showErrorMessage() {
    private suspend fun showErrorMessage() {
        repository.setMessage(errorMessage(authenticationInteractor.getAuthenticationMethod()))
    }

+60 −8
Original line number Diff line number Diff line
@@ -16,12 +16,21 @@

package com.android.systemui.bouncer.ui.viewmodel

import android.annotation.StringRes
import android.util.Log
import com.android.systemui.authentication.domain.interactor.AuthenticationResult
import com.android.systemui.authentication.domain.model.AuthenticationMethodModel
import com.android.systemui.bouncer.domain.interactor.BouncerInteractor
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.launch

sealed class AuthMethodBouncerViewModel(
    protected val viewModelScope: CoroutineScope,
    protected val interactor: BouncerInteractor,

    /**
     * Whether user input is enabled.
     *
@@ -29,7 +38,6 @@ sealed class AuthMethodBouncerViewModel(
     * being able to attempt to unlock the device.
     */
    val isInputEnabled: StateFlow<Boolean>,
    private val interactor: BouncerInteractor,
) {

    private val _animateFailure = MutableStateFlow(false)
@@ -42,12 +50,26 @@ sealed class AuthMethodBouncerViewModel(
    /** Whether the input method editor (for example, the software keyboard) is visible. */
    private var isImeVisible: Boolean = false

    /** The authentication method that corresponds to this view model. */
    abstract val authenticationMethod: AuthenticationMethodModel

    /**
     * Notifies that the failure animation has been shown. This should be called to consume a `true`
     * value in [animateFailure].
     * String resource ID of the failure message to be shown during throttling.
     *
     * The message must include 2 number parameters: the first one indicating how many unsuccessful
     * attempts were made, and the second one indicating in how many seconds throttling will expire.
     */
    fun onFailureAnimationShown() {
        _animateFailure.value = false
    @get:StringRes abstract val throttlingMessageId: Int

    /** Notifies that the UI has been shown to the user. */
    fun onShown() {
        clearInput()
        interactor.resetMessage()
    }

    /** Notifies that the user has placed down a pointer. */
    fun onDown() {
        interactor.onDown()
    }

    /**
@@ -65,8 +87,38 @@ sealed class AuthMethodBouncerViewModel(
        isImeVisible = isVisible
    }

    /** Ask the UI to show the failure animation. */
    protected fun showFailureAnimation() {
        _animateFailure.value = true
    /**
     * Notifies that the failure animation has been shown. This should be called to consume a `true`
     * value in [animateFailure].
     */
    fun onFailureAnimationShown() {
        _animateFailure.value = false
    }

    /** Clears any previously-entered input. */
    protected abstract fun clearInput()

    /** Returns the input entered so far. */
    protected abstract fun getInput(): List<Any>

    /**
     * Attempts to authenticate the user using the current input value.
     *
     * @see BouncerInteractor.authenticate
     */
    protected fun tryAuthenticate(useAutoConfirm: Boolean = false) {
        viewModelScope.launch {
            Log.d("Danny", "tryAuthenticate(useAutoConfirm=$useAutoConfirm)")
            val authenticationResult = interactor.authenticate(getInput(), useAutoConfirm)
            Log.d("Danny", "result = $authenticationResult")
            if (authenticationResult == AuthenticationResult.SKIPPED && useAutoConfirm) {
                return@launch
            }
            _animateFailure.value = authenticationResult != AuthenticationResult.SUCCEEDED

            // TODO(b/291528545): On success, this should only be cleared after the view is animated
            //  away).
            clearInput()
        }
    }
}
+68 −65
Original line number Diff line number Diff line
@@ -17,16 +17,19 @@
package com.android.systemui.bouncer.ui.viewmodel

import android.content.Context
import com.android.systemui.res.R
import com.android.systemui.authentication.domain.interactor.AuthenticationInteractor
import com.android.systemui.authentication.domain.model.AuthenticationMethodModel
import com.android.systemui.bouncer.domain.interactor.BouncerInteractor
import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.dagger.qualifiers.Application
import com.android.systemui.dagger.qualifiers.Main
import com.android.systemui.scene.shared.flag.SceneContainerFlags
import javax.inject.Inject
import kotlin.math.ceil
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
@@ -35,6 +38,7 @@ import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.job
import kotlinx.coroutines.launch

/** Holds UI state and handles user input on bouncer UIs. */
@@ -44,8 +48,9 @@ class BouncerViewModel
constructor(
    @Application private val applicationContext: Context,
    @Application private val applicationScope: CoroutineScope,
    @Main private val mainDispatcher: CoroutineDispatcher,
    private val bouncerInteractor: BouncerInteractor,
    private val authenticationInteractor: AuthenticationInteractor,
    authenticationInteractor: AuthenticationInteractor,
    flags: SceneContainerFlags,
) {
    private val isInputEnabled: StateFlow<Boolean> =
@@ -57,91 +62,45 @@ constructor(
                initialValue = !bouncerInteractor.isThrottled.value,
            )

    private val pin: PinBouncerViewModel by lazy {
        PinBouncerViewModel(
            applicationContext = applicationContext,
            applicationScope = applicationScope,
            interactor = bouncerInteractor,
            isInputEnabled = isInputEnabled,
        )
    }

    private val password: PasswordBouncerViewModel by lazy {
        PasswordBouncerViewModel(
            applicationScope = applicationScope,
            interactor = bouncerInteractor,
            isInputEnabled = isInputEnabled,
        )
    }

    private val pattern: PatternBouncerViewModel by lazy {
        PatternBouncerViewModel(
            applicationContext = applicationContext,
            applicationScope = applicationScope,
            interactor = bouncerInteractor,
            isInputEnabled = isInputEnabled,
        )
    }

    /** View-model for the current UI, based on the current authentication method. */
    val authMethod: StateFlow<AuthMethodBouncerViewModel?> =
    val authMethodViewModel: StateFlow<AuthMethodBouncerViewModel?> =
        authenticationInteractor.authenticationMethod
            .map { authenticationMethod ->
                when (authenticationMethod) {
                    is AuthenticationMethodModel.Pin -> pin
                    is AuthenticationMethodModel.Password -> password
                    is AuthenticationMethodModel.Pattern -> pattern
                    else -> null
                }
            }
            .map(::getChildViewModel)
            .stateIn(
                scope = applicationScope,
                started = SharingStarted.WhileSubscribed(),
                initialValue = null,
            )

    // Handle to the scope of the child ViewModel (stored in [authMethod]).
    private var childViewModelScope: CoroutineScope? = null

    init {
        if (flags.isEnabled()) {
            applicationScope.launch {
                bouncerInteractor.isThrottled
                    .map { isThrottled ->
                        if (isThrottled) {
                            when (authenticationInteractor.getAuthenticationMethod()) {
                                is AuthenticationMethodModel.Pin ->
                                    R.string.kg_too_many_failed_pin_attempts_dialog_message
                                is AuthenticationMethodModel.Password ->
                                    R.string.kg_too_many_failed_password_attempts_dialog_message
                                is AuthenticationMethodModel.Pattern ->
                                    R.string.kg_too_many_failed_pattern_attempts_dialog_message
                                else -> null
                            }?.let { stringResourceId ->
                combine(bouncerInteractor.isThrottled, authMethodViewModel) {
                        isThrottled,
                        authMethodViewModel ->
                        if (isThrottled && authMethodViewModel != null) {
                            applicationContext.getString(
                                    stringResourceId,
                                authMethodViewModel.throttlingMessageId,
                                bouncerInteractor.throttling.value.failedAttemptCount,
                                ceil(bouncerInteractor.throttling.value.remainingMs / 1000f)
                                    .toInt(),
                            )
                            }
                        } else {
                            null
                        }
                    }
                    .distinctUntilChanged()
                    .collect { dialogMessageOrNull ->
                        if (dialogMessageOrNull != null) {
                            _throttlingDialogMessage.value = dialogMessageOrNull
                        }
                    }
                    .collect { dialogMessage -> _throttlingDialogMessage.value = dialogMessage }
            }
        }
    }

    /** The user-facing message to show in the bouncer. */
    val message: StateFlow<MessageViewModel> =
        combine(
                bouncerInteractor.message,
                bouncerInteractor.isThrottled,
            ) { message, isThrottled ->
        combine(bouncerInteractor.message, bouncerInteractor.isThrottled) { message, isThrottled ->
                toMessageViewModel(message, isThrottled)
            }
            .stateIn(
@@ -186,6 +145,50 @@ constructor(
        )
    }

    private fun getChildViewModel(
        authenticationMethod: AuthenticationMethodModel,
    ): AuthMethodBouncerViewModel? {
        // If the current child view-model matches the authentication method, reuse it instead of
        // creating a new instance.
        val childViewModel = authMethodViewModel.value
        if (authenticationMethod == childViewModel?.authenticationMethod) {
            return childViewModel
        }

        childViewModelScope?.cancel()
        val newViewModelScope = createChildCoroutineScope(applicationScope)
        childViewModelScope = newViewModelScope
        return when (authenticationMethod) {
            is AuthenticationMethodModel.Pin ->
                PinBouncerViewModel(
                    applicationContext = applicationContext,
                    viewModelScope = newViewModelScope,
                    interactor = bouncerInteractor,
                    isInputEnabled = isInputEnabled,
                )
            is AuthenticationMethodModel.Password ->
                PasswordBouncerViewModel(
                    viewModelScope = newViewModelScope,
                    interactor = bouncerInteractor,
                    isInputEnabled = isInputEnabled,
                )
            is AuthenticationMethodModel.Pattern ->
                PatternBouncerViewModel(
                    applicationContext = applicationContext,
                    viewModelScope = newViewModelScope,
                    interactor = bouncerInteractor,
                    isInputEnabled = isInputEnabled,
                )
            else -> null
        }
    }

    private fun createChildCoroutineScope(parentScope: CoroutineScope): CoroutineScope {
        return CoroutineScope(
            SupervisorJob(parent = parentScope.coroutineContext.job) + mainDispatcher
        )
    }

    data class MessageViewModel(
        val text: String,

Loading