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

Commit 38a987f8 authored by burakov's avatar burakov
Browse files

[flexiglass] Improvements to lockout state management and rendering:

1. Add a new flow `failedAuthenticationAttempts`, which is updated
directly from the upstream source of truth. This will be needed for the
device wipe dialog, so needs to be independent from lockout.

2. Remove the `lockout` state and associated model, which is now
redundant; we store the lockout end timestamp and failed auth attempts
as separate and independent pieces of state.

3. Unify `setLockoutDuration` with `reportLockoutStarted`.

4. Move the lockout countdown updates to the view-model, because it is a
transient state that is only relevant for presentation (UI message) and
so we can shave off some propagation overhead and complexity.

5. Fix bug where the throttling dialog countdown is updating every
second (it should remain constant).

6. Fix bug where the lockout dialog would be shown again when switching
to a user with an active lockout.

7. Make `AuthenticationRepository` responsible for maintaining the
freshness of the `hasLockoutOccurred` state.

8. Rename `authenticationChallengeResult` to `onAuthenticationResult`,
to convey it's a flow of events rather than state (similar to our
de-facto convention elsewhere), and move it out of the repository.

9. Refactor `AuthenticationInteractorTest` to be more concise, readable
and robust.

Fix: 314962784
Fix: 316117169
Bug: 314757822
Bug: 306559035
Test: Added unit tests.
Test: Existing unit tests still pass.
Flag: ACONFIG com.android.systemui.scene_container DEVELOPMENT
Change-Id: Ic3050c4c0b117c3e61c35092f14c7c6ce32cb908
parent 1b5c9f37
Loading
Loading
Loading
Loading
+45 −17
Original line number Diff line number Diff line
@@ -35,8 +35,10 @@ import com.android.systemui.statusbar.pipeline.mobile.data.repository.FakeMobile
import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy
import com.android.systemui.user.data.repository.FakeUserRepository
import com.android.systemui.util.mockito.whenever
import com.android.systemui.util.time.FakeSystemClock
import com.google.common.truth.Truth.assertThat
import java.util.function.Function
import kotlin.time.Duration.Companion.seconds
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runCurrent
@@ -58,6 +60,7 @@ class AuthenticationRepositoryTest : SysuiTestCase() {

    private val testUtils = SceneTestUtils(this)
    private val testScope = testUtils.testScope
    private val clock = FakeSystemClock()
    private val userRepository = FakeUserRepository()
    private lateinit var mobileConnectionsRepository: FakeMobileConnectionsRepository

@@ -78,8 +81,10 @@ class AuthenticationRepositoryTest : SysuiTestCase() {
        underTest =
            AuthenticationRepositoryImpl(
                applicationScope = testScope.backgroundScope,
                getSecurityMode = getSecurityMode,
                backgroundDispatcher = testUtils.testDispatcher,
                flags = testUtils.sceneContainerFlags,
                clock = clock,
                getSecurityMode = getSecurityMode,
                userRepository = userRepository,
                lockPatternUtils = lockPatternUtils,
                broadcastDispatcher = fakeBroadcastDispatcher,
@@ -140,22 +145,6 @@ class AuthenticationRepositoryTest : SysuiTestCase() {
            assertThat(values.last()).isTrue()
        }

    @Test
    fun reportAuthenticationAttempt_emitsAuthenticationChallengeResult() =
        testScope.runTest {
            val authenticationChallengeResults by
                collectValues(underTest.authenticationChallengeResult)

            runCurrent()
            underTest.reportAuthenticationAttempt(true)
            runCurrent()
            underTest.reportAuthenticationAttempt(false)
            runCurrent()
            underTest.reportAuthenticationAttempt(true)

            assertThat(authenticationChallengeResults).isEqualTo(listOf(true, false, true))
        }

    @Test
    fun isPinEnhancedPrivacyEnabled() =
        testScope.runTest {
@@ -172,6 +161,45 @@ class AuthenticationRepositoryTest : SysuiTestCase() {
            assertThat(values.last()).isTrue()
        }

    @Test
    fun lockoutEndTimestamp() =
        testScope.runTest {
            val lockoutEndMs = clock.elapsedRealtime() + 30.seconds.inWholeMilliseconds
            whenever(lockPatternUtils.getLockoutAttemptDeadline(USER_INFOS[0].id))
                .thenReturn(lockoutEndMs)
            whenever(lockPatternUtils.getLockoutAttemptDeadline(USER_INFOS[1].id)).thenReturn(0)

            // Switch to a user who is not locked-out.
            userRepository.setSelectedUserInfo(USER_INFOS[1])
            assertThat(underTest.lockoutEndTimestamp).isNull()

            // Switch back to the locked-out user, verify the timestamp is up-to-date.
            userRepository.setSelectedUserInfo(USER_INFOS[0])
            assertThat(underTest.lockoutEndTimestamp).isEqualTo(lockoutEndMs)

            // After the lockout expires, null is returned.
            clock.setElapsedRealtime(lockoutEndMs)
            assertThat(underTest.lockoutEndTimestamp).isNull()
        }

    @Test
    fun hasLockoutOccurred() =
        testScope.runTest {
            val hasLockoutOccurred by collectLastValue(underTest.hasLockoutOccurred)
            assertThat(hasLockoutOccurred).isFalse()

            underTest.reportLockoutStarted(1000)
            assertThat(hasLockoutOccurred).isTrue()

            clock.setElapsedRealtime(clock.elapsedRealtime() + 60.seconds.inWholeMilliseconds)

            underTest.reportAuthenticationAttempt(isSuccessful = false)
            assertThat(hasLockoutOccurred).isTrue()

            underTest.reportAuthenticationAttempt(isSuccessful = true)
            assertThat(hasLockoutOccurred).isFalse()
        }

    private fun setSecurityModeAndDispatchBroadcast(
        securityMode: KeyguardSecurityModel.SecurityMode,
    ) {
+184 −187

File changed.

Preview size limit exceeded, changes collapsed.

+28 −52
Original line number Diff line number Diff line
@@ -21,15 +21,15 @@ import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.authentication.data.repository.FakeAuthenticationRepository
import com.android.systemui.authentication.domain.interactor.AuthenticationResult
import com.android.systemui.authentication.shared.model.AuthenticationLockoutModel
import com.android.systemui.authentication.shared.model.AuthenticationMethodModel
import com.android.systemui.authentication.shared.model.AuthenticationPatternCoordinate
import com.android.systemui.coroutines.collectLastValue
import com.android.systemui.coroutines.collectValues
import com.android.systemui.keyguard.domain.interactor.KeyguardFaceAuthInteractor
import com.android.systemui.res.R
import com.android.systemui.scene.SceneTestUtils
import com.google.common.truth.Truth.assertThat
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.seconds
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.advanceTimeBy
import kotlinx.coroutines.test.runCurrent
@@ -79,7 +79,7 @@ class BouncerInteractorTest : SysuiTestCase() {
            utils.authenticationRepository.setAuthenticationMethod(AuthenticationMethodModel.Pin)
            runCurrent()
            underTest.clearMessage()
            assertThat(message).isEmpty()
            assertThat(message).isNull()

            underTest.resetMessage()
            assertThat(message).isEqualTo(MESSAGE_ENTER_YOUR_PIN)
@@ -149,7 +149,7 @@ class BouncerInteractorTest : SysuiTestCase() {
            // Incomplete input.
            assertThat(underTest.authenticate(listOf(1, 2), tryAutoConfirm = true))
                .isEqualTo(AuthenticationResult.SKIPPED)
            assertThat(message).isEmpty()
            assertThat(message).isNull()

            // Correct input.
            assertThat(
@@ -159,7 +159,7 @@ class BouncerInteractorTest : SysuiTestCase() {
                    )
                )
                .isEqualTo(AuthenticationResult.SKIPPED)
            assertThat(message).isEmpty()
            assertThat(message).isNull()
        }

    @Test
@@ -246,57 +246,40 @@ class BouncerInteractorTest : SysuiTestCase() {
        }

    @Test
    fun lockout() =
    fun lockoutStarted() =
        testScope.runTest {
            val lockout by collectLastValue(underTest.lockout)
            val lockoutStartedEvents by collectValues(underTest.onLockoutStarted)
            val message by collectLastValue(underTest.message)

            utils.authenticationRepository.setAuthenticationMethod(AuthenticationMethodModel.Pin)
            assertThat(lockout).isNull()
            assertThat(lockoutStartedEvents).isEmpty()

            // Try the wrong PIN repeatedly, until lockout is triggered:
            repeat(FakeAuthenticationRepository.MAX_FAILED_AUTH_TRIES_BEFORE_LOCKOUT) { times ->
                // Wrong PIN.
                assertThat(underTest.authenticate(listOf(6, 7, 8, 9)))
                    .isEqualTo(AuthenticationResult.FAILED)
                if (times < FakeAuthenticationRepository.MAX_FAILED_AUTH_TRIES_BEFORE_LOCKOUT - 1) {
                    assertThat(message).isEqualTo(MESSAGE_WRONG_PIN)
                    assertThat(lockoutStartedEvents).isEmpty()
                    assertThat(message).isNotEmpty()
                }
            }
            assertThat(lockout)
                .isEqualTo(
                    AuthenticationLockoutModel(
                        failedAttemptCount =
                            FakeAuthenticationRepository.MAX_FAILED_AUTH_TRIES_BEFORE_LOCKOUT,
                        remainingSeconds = FakeAuthenticationRepository.LOCKOUT_DURATION_SECONDS,
                    )
                )
            assertTryAgainMessage(
                message,
                FakeAuthenticationRepository.LOCKOUT_DURATION_MS.milliseconds.inWholeSeconds.toInt()
            )

            // Correct PIN, but locked out, so doesn't change away from the bouncer scene:
            assertThat(underTest.authenticate(FakeAuthenticationRepository.DEFAULT_PIN))
                .isEqualTo(AuthenticationResult.SKIPPED)
            assertTryAgainMessage(
                message,
                FakeAuthenticationRepository.LOCKOUT_DURATION_MS.milliseconds.inWholeSeconds.toInt()
            )

            lockout?.remainingSeconds?.let { seconds ->
                repeat(seconds) { time ->
                    advanceTimeBy(1000)
                    val remainingTimeSec = seconds - time - 1
                    if (remainingTimeSec > 0) {
                        assertTryAgainMessage(message, remainingTimeSec)
                    }
            assertThat(authenticationInteractor.lockoutEndTimestamp).isNotNull()
            assertThat(lockoutStartedEvents.size).isEqualTo(1)
            assertThat(message).isNull()

            // Advance the time to finish the lockout:
            advanceTimeBy(FakeAuthenticationRepository.LOCKOUT_DURATION_SECONDS.seconds)
            assertThat(authenticationInteractor.lockoutEndTimestamp).isNull()
            assertThat(message).isNull()
            assertThat(lockoutStartedEvents.size).isEqualTo(1)

            // Trigger lockout again:
            repeat(FakeAuthenticationRepository.MAX_FAILED_AUTH_TRIES_BEFORE_LOCKOUT) {
                // Wrong PIN.
                underTest.authenticate(listOf(6, 7, 8, 9))
            }
            }
            assertThat(message).isEqualTo("")
            assertThat(lockout).isNull()

            // Correct PIN and no longer locked out so changes to the Gone scene:
            assertThat(underTest.authenticate(FakeAuthenticationRepository.DEFAULT_PIN))
                .isEqualTo(AuthenticationResult.SUCCEEDED)
            assertThat(lockout).isNull()
            assertThat(lockoutStartedEvents.size).isEqualTo(2)
        }

    @Test
@@ -326,13 +309,6 @@ class BouncerInteractorTest : SysuiTestCase() {
            verify(keyguardFaceAuthInteractor).onPrimaryBouncerUserInput()
        }

    private fun assertTryAgainMessage(
        message: String?,
        time: Int,
    ) {
        assertThat(message).isEqualTo("Try again in $time seconds.")
    }

    companion object {
        private const val MESSAGE_ENTER_YOUR_PIN = "Enter your PIN"
        private const val MESSAGE_ENTER_YOUR_PASSWORD = "Enter your password"
+51 −13
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.test.advanceTimeBy
import kotlinx.coroutines.test.currentTime
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import org.junit.Test
@@ -135,19 +136,47 @@ class BouncerViewModelTest : SysuiTestCase() {
    fun message() =
        testScope.runTest {
            val message by collectLastValue(underTest.message)
            val lockout by collectLastValue(bouncerInteractor.lockout)
            utils.authenticationRepository.setAuthenticationMethod(AuthenticationMethodModel.Pin)
            assertThat(message?.isUpdateAnimated).isTrue()

            repeat(FakeAuthenticationRepository.MAX_FAILED_AUTH_TRIES_BEFORE_LOCKOUT) {
                // Wrong PIN.
                bouncerInteractor.authenticate(listOf(3, 4, 5, 6))
                bouncerInteractor.authenticate(WRONG_PIN)
            }
            assertThat(message?.isUpdateAnimated).isFalse()

            lockout?.remainingSeconds?.let { remainingSeconds ->
                advanceTimeBy(remainingSeconds.seconds.inWholeMilliseconds)
            val lockoutEndMs = authenticationInteractor.lockoutEndTimestamp ?: 0
            advanceTimeBy(lockoutEndMs - testScope.currentTime)
            assertThat(message?.isUpdateAnimated).isTrue()
        }

    @Test
    fun lockoutMessage() =
        testScope.runTest {
            val authMethodViewModel by collectLastValue(underTest.authMethodViewModel)
            val message by collectLastValue(underTest.message)
            utils.authenticationRepository.setAuthenticationMethod(AuthenticationMethodModel.Pin)
            assertThat(utils.authenticationRepository.lockoutEndTimestamp).isNull()
            assertThat(authMethodViewModel?.lockoutMessageId).isNotNull()

            repeat(FakeAuthenticationRepository.MAX_FAILED_AUTH_TRIES_BEFORE_LOCKOUT) { times ->
                bouncerInteractor.authenticate(WRONG_PIN)
                if (times < FakeAuthenticationRepository.MAX_FAILED_AUTH_TRIES_BEFORE_LOCKOUT - 1) {
                    assertThat(message?.text).isEqualTo(bouncerInteractor.message.value)
                    assertThat(message?.isUpdateAnimated).isTrue()
                }
            }
            val lockoutSeconds = FakeAuthenticationRepository.LOCKOUT_DURATION_SECONDS
            assertTryAgainMessage(message?.text, lockoutSeconds)
            assertThat(message?.isUpdateAnimated).isFalse()

            repeat(FakeAuthenticationRepository.LOCKOUT_DURATION_SECONDS) { time ->
                advanceTimeBy(1.seconds)
                val remainingSeconds = lockoutSeconds - time - 1
                if (remainingSeconds > 0) {
                    assertTryAgainMessage(message?.text, remainingSeconds)
                }
            }
            assertThat(message?.text).isEmpty()
            assertThat(message?.isUpdateAnimated).isTrue()
        }

@@ -160,32 +189,30 @@ class BouncerViewModelTest : SysuiTestCase() {
                        authViewModel?.isInputEnabled ?: emptyFlow()
                    }
                )
            val lockout by collectLastValue(bouncerInteractor.lockout)
            utils.authenticationRepository.setAuthenticationMethod(AuthenticationMethodModel.Pin)
            assertThat(isInputEnabled).isTrue()

            repeat(FakeAuthenticationRepository.MAX_FAILED_AUTH_TRIES_BEFORE_LOCKOUT) {
                // Wrong PIN.
                bouncerInteractor.authenticate(listOf(3, 4, 5, 6))
                bouncerInteractor.authenticate(WRONG_PIN)
            }
            assertThat(isInputEnabled).isFalse()

            lockout?.remainingSeconds?.let { remainingSeconds ->
                advanceTimeBy(remainingSeconds.seconds.inWholeMilliseconds)
            }
            val lockoutEndMs = authenticationInteractor.lockoutEndTimestamp ?: 0
            advanceTimeBy(lockoutEndMs - testScope.currentTime)
            assertThat(isInputEnabled).isTrue()
        }

    @Test
    fun dialogMessage() =
        testScope.runTest {
            val authMethodViewModel by collectLastValue(underTest.authMethodViewModel)
            val dialogMessage by collectLastValue(underTest.dialogMessage)
            utils.authenticationRepository.setAuthenticationMethod(AuthenticationMethodModel.Pin)
            assertThat(authMethodViewModel?.lockoutMessageId).isNotNull()

            repeat(FakeAuthenticationRepository.MAX_FAILED_AUTH_TRIES_BEFORE_LOCKOUT) {
                // Wrong PIN.
                assertThat(dialogMessage).isNull()
                bouncerInteractor.authenticate(listOf(3, 4, 5, 6))
                bouncerInteractor.authenticate(WRONG_PIN)
            }
            assertThat(dialogMessage).isNotEmpty()

@@ -241,4 +268,15 @@ class BouncerViewModelTest : SysuiTestCase() {
            AuthenticationMethodModel.Sim,
        )
    }

    private fun assertTryAgainMessage(
        message: String?,
        time: Int,
    ) {
        assertThat(message).isEqualTo("Try again in $time seconds.")
    }

    companion object {
        private val WRONG_PIN = FakeAuthenticationRepository.DEFAULT_PIN.map { it + 1 }
    }
}
+12 −21
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ package com.android.systemui.bouncer.ui.viewmodel
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.authentication.shared.model.AuthenticationLockoutModel
import com.android.systemui.authentication.shared.model.AuthenticationMethodModel
import com.android.systemui.coroutines.collectLastValue
import com.android.systemui.coroutines.collectValues
@@ -28,6 +27,7 @@ import com.android.systemui.scene.SceneTestUtils
import com.android.systemui.scene.shared.model.SceneKey
import com.android.systemui.scene.shared.model.SceneModel
import com.google.common.truth.Truth.assertThat
import kotlin.time.Duration.Companion.seconds
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow
@@ -45,11 +45,7 @@ class PasswordBouncerViewModelTest : SysuiTestCase() {

    private val utils = SceneTestUtils(this)
    private val testScope = utils.testScope
    private val authenticationRepository = utils.authenticationRepository
    private val authenticationInteractor =
        utils.authenticationInteractor(
            repository = authenticationRepository,
        )
    private val authenticationInteractor = utils.authenticationInteractor()
    private val sceneInteractor = utils.sceneInteractor()
    private val bouncerInteractor =
        utils.bouncerInteractor(
@@ -61,12 +57,13 @@ class PasswordBouncerViewModelTest : SysuiTestCase() {
            authenticationInteractor = authenticationInteractor,
            actionButtonInteractor = utils.bouncerActionButtonInteractor(),
        )
    private val isInputEnabled = MutableStateFlow(true)

    private val underTest =
        PasswordBouncerViewModel(
            viewModelScope = testScope.backgroundScope,
            interactor = bouncerInteractor,
            isInputEnabled = MutableStateFlow(true).asStateFlow(),
            isInputEnabled.asStateFlow(),
        )

    @Before
@@ -123,8 +120,7 @@ class PasswordBouncerViewModelTest : SysuiTestCase() {
    @Test
    fun onAuthenticateKeyPressed_whenCorrect() =
        testScope.runTest {
            val authResult by
                collectLastValue(authenticationInteractor.authenticationChallengeResult)
            val authResult by collectLastValue(authenticationInteractor.onAuthenticationResult)
            lockDeviceAndOpenPasswordBouncer()

            underTest.onPasswordInputChanged("password")
@@ -169,8 +165,7 @@ class PasswordBouncerViewModelTest : SysuiTestCase() {
    @Test
    fun onAuthenticateKeyPressed_correctAfterWrong() =
        testScope.runTest {
            val authResult by
                collectLastValue(authenticationInteractor.authenticationChallengeResult)
            val authResult by collectLastValue(authenticationInteractor.onAuthenticationResult)
            val message by collectLastValue(bouncerViewModel.message)
            val password by collectLastValue(underTest.password)
            lockDeviceAndOpenPasswordBouncer()
@@ -333,19 +328,15 @@ class PasswordBouncerViewModelTest : SysuiTestCase() {
    ) {
        if (isLockedOut) {
            repeat(failedAttemptCount) {
                authenticationRepository.reportAuthenticationAttempt(false)
                utils.authenticationRepository.reportAuthenticationAttempt(false)
            }
            val remainingTimeSeconds = 30
            authenticationRepository.setLockoutDuration(remainingTimeSeconds * 1000)
            authenticationRepository.lockout.value =
                AuthenticationLockoutModel(
                    failedAttemptCount = failedAttemptCount,
                    remainingSeconds = remainingTimeSeconds,
            utils.authenticationRepository.reportLockoutStarted(
                30.seconds.inWholeMilliseconds.toInt()
            )
        } else {
            authenticationRepository.reportAuthenticationAttempt(true)
            authenticationRepository.lockout.value = null
            utils.authenticationRepository.reportAuthenticationAttempt(true)
        }
        isInputEnabled.value = !isLockedOut

        runCurrent()
    }
Loading