From f4fceb175a6611ad4cf13b9c18b6a112075a0e53 Mon Sep 17 00:00:00 2001 From: Brian Yencho Date: Mon, 22 Jan 2024 17:29:51 -0600 Subject: [PATCH] Add checks for locked vault to autofill (#703) --- .../autofill/builder/FilledDataBuilderImpl.kt | 3 +- .../data/autofill/di/AutofillModule.kt | 11 +- .../provider/AutofillCipherProvider.kt | 6 + .../provider/AutofillCipherProviderImpl.kt | 24 +++- .../autofill/builder/FilledDataBuilderTest.kt | 4 +- .../processor/AutofillCipherProviderTest.kt | 124 +++++++++++++++++- 6 files changed, 160 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt index fb7f8d0bd2..84ba699290 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt @@ -18,8 +18,7 @@ class FilledDataBuilderImpl( private val autofillCipherProvider: AutofillCipherProvider, ) : FilledDataBuilder { override suspend fun build(autofillRequest: AutofillRequest.Fillable): FilledData { - // TODO: determine whether or not the vault is locked (BIT-1296) - val isVaultLocked = false + val isVaultLocked = autofillCipherProvider.isVaultLocked() // Subtract one to make sure there is space for the vault item. val maxCipherInlineSuggestionsCount = autofillRequest.maxInlineSuggestionsCount - 1 diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt index 89469c547f..0faa030e28 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt @@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.autofill.di import android.content.Context import android.view.autofill.AutofillManager +import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.autofill.builder.FillResponseBuilder import com.x8bit.bitwarden.data.autofill.builder.FillResponseBuilderImpl import com.x8bit.bitwarden.data.autofill.builder.FilledDataBuilder @@ -14,6 +15,7 @@ import com.x8bit.bitwarden.data.autofill.provider.AutofillCipherProvider import com.x8bit.bitwarden.data.autofill.provider.AutofillCipherProviderImpl import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import com.x8bit.bitwarden.data.platform.repository.SettingsRepository +import com.x8bit.bitwarden.data.vault.repository.VaultRepository import dagger.Module import dagger.Provides import dagger.hilt.InstallIn @@ -43,7 +45,14 @@ object AutofillModule { ) @Provides - fun providesAutofillCipherProvider(): AutofillCipherProvider = AutofillCipherProviderImpl() + fun providesAutofillCipherProvider( + authRepository: AuthRepository, + vaultRepository: VaultRepository, + ): AutofillCipherProvider = + AutofillCipherProviderImpl( + authRepository = authRepository, + vaultRepository = vaultRepository, + ) @Provides fun providesAutofillProcessor( diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/provider/AutofillCipherProvider.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/provider/AutofillCipherProvider.kt index 1241dec012..b81e68b490 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/provider/AutofillCipherProvider.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/provider/AutofillCipherProvider.kt @@ -6,6 +6,12 @@ import com.x8bit.bitwarden.data.autofill.model.AutofillCipher * A service for getting [AutofillCipher]s. */ interface AutofillCipherProvider { + /** + * Returns `true` if the vault for the current user is locked. This suspends in order to return + * a value only after any unlocking vaults have fully unlocked (or failed to do so). + */ + suspend fun isVaultLocked(): Boolean + /** * Get all [AutofillCipher.Card]s for the current user. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/provider/AutofillCipherProviderImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/provider/AutofillCipherProviderImpl.kt index 387f15da30..e359a87c44 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/provider/AutofillCipherProviderImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/provider/AutofillCipherProviderImpl.kt @@ -1,22 +1,40 @@ package com.x8bit.bitwarden.data.autofill.provider +import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.autofill.model.AutofillCipher +import com.x8bit.bitwarden.data.vault.repository.VaultRepository +import kotlinx.coroutines.flow.first /** * The default [AutofillCipherProvider] implementation. This service is used for getting currrent * [AutofillCipher]s. */ -class AutofillCipherProviderImpl : AutofillCipherProvider { +class AutofillCipherProviderImpl( + private val authRepository: AuthRepository, + private val vaultRepository: VaultRepository, +) : AutofillCipherProvider { + private val activeUserId: String? get() = authRepository.activeUserId + + override suspend fun isVaultLocked(): Boolean { + val userId = activeUserId ?: return true + + // Wait for any unlocking actions to finish. This can be relevant on startup for Never lock + // accounts. + vaultRepository.vaultStateFlow.first { userId !in it.unlockingVaultUserIds } + + return !vaultRepository.isVaultUnlocked(userId = userId) + } + override suspend fun getCardAutofillCiphers(): List { // TODO: fulfill with real ciphers (BIT-1294) - return cardCiphers + return if (isVaultLocked()) emptyList() else cardCiphers } override suspend fun getLoginAutofillCiphers( uri: String, ): List { // TODO: fulfill with real ciphers (BIT-1294) - return loginCiphers + return if (isVaultLocked()) emptyList() else loginCiphers } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt index 784c070ce5..e1576ef1bc 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt @@ -28,7 +28,9 @@ import org.junit.jupiter.api.Test class FilledDataBuilderTest { private lateinit var filledDataBuilder: FilledDataBuilder - private val autofillCipherProvider: AutofillCipherProvider = mockk() + private val autofillCipherProvider: AutofillCipherProvider = mockk { + coEvery { isVaultLocked() } returns false + } private val autofillId: AutofillId = mockk() private val autofillViewData = AutofillView.Data( diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillCipherProviderTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillCipherProviderTest.kt index 5f11dc8014..da73704f93 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillCipherProviderTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillCipherProviderTest.kt @@ -1,40 +1,154 @@ package com.x8bit.bitwarden.data.autofill.processor +import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.autofill.model.AutofillCipher import com.x8bit.bitwarden.data.autofill.provider.AutofillCipherProvider import com.x8bit.bitwarden.data.autofill.provider.AutofillCipherProviderImpl +import com.x8bit.bitwarden.data.vault.repository.VaultRepository +import com.x8bit.bitwarden.data.vault.repository.model.VaultState +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.async +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test class AutofillCipherProviderTest { + + private val authRepository: AuthRepository = mockk { + every { activeUserId } returns ACTIVE_USER_ID + } + private val mutableVaultStateFlow = MutableStateFlow( + VaultState( + unlockingVaultUserIds = emptySet(), + unlockedVaultUserIds = emptySet(), + ), + ) + private val vaultRepository: VaultRepository = mockk { + every { vaultStateFlow } returns mutableVaultStateFlow + every { isVaultUnlocked(ACTIVE_USER_ID) } answers { + ACTIVE_USER_ID in mutableVaultStateFlow.value.unlockedVaultUserIds + } + } + private lateinit var autofillCipherProvider: AutofillCipherProvider @BeforeEach fun setup() { - autofillCipherProvider = AutofillCipherProviderImpl() + autofillCipherProvider = AutofillCipherProviderImpl( + authRepository = authRepository, + vaultRepository = vaultRepository, + ) } + @Suppress("MaxLineLength") @Test - fun `getCardAutofillCiphers should return default list of card ciphers`() = runTest { + fun `isVaultLocked when there is no active user should return true`() = + runTest { + every { authRepository.activeUserId } returns null + + val result = async { + autofillCipherProvider.isVaultLocked() + } + + testScheduler.advanceUntilIdle() + assertTrue(result.isCompleted) + assertTrue(result.await()) + } + + @Suppress("MaxLineLength") + @Test + fun `isVaultLocked when there is an active user should wait for pending unlocking to finish and return the locked state for that user`() = + runTest { + every { authRepository.activeUserId } returns ACTIVE_USER_ID + mutableVaultStateFlow.value = VaultState( + unlockedVaultUserIds = emptySet(), + unlockingVaultUserIds = setOf(ACTIVE_USER_ID), + ) + + val result = async { + autofillCipherProvider.isVaultLocked() + } + + testScheduler.advanceUntilIdle() + assertFalse(result.isCompleted) + + mutableVaultStateFlow.value = VaultState( + unlockedVaultUserIds = setOf(ACTIVE_USER_ID), + unlockingVaultUserIds = emptySet(), + ) + + testScheduler.advanceUntilIdle() + assertTrue(result.isCompleted) + + assertFalse(result.await()) + } + + @Test + fun `getCardAutofillCiphers when unlocked should return default list of card ciphers`() = + runTest { + mutableVaultStateFlow.value = VaultState( + unlockedVaultUserIds = setOf(ACTIVE_USER_ID), + unlockingVaultUserIds = emptySet(), + ) + + // Test & Verify + val actual = autofillCipherProvider.getCardAutofillCiphers() + + assertEquals(CARD_CIPHERS, actual) + } + + @Test + fun `getCardAutofillCiphers when locked should return an empty list`() = runTest { + mutableVaultStateFlow.value = VaultState( + unlockedVaultUserIds = emptySet(), + unlockingVaultUserIds = emptySet(), + ) + // Test & Verify val actual = autofillCipherProvider.getCardAutofillCiphers() - assertEquals(CARD_CIPHERS, actual) + assertEquals(emptyList(), actual) } @Test - fun `getLoginAutofillCiphers should return default list of login ciphers`() = runTest { + fun `getLoginAutofillCiphers when unlocked should return default list of login ciphers`() = + runTest { + mutableVaultStateFlow.value = VaultState( + unlockedVaultUserIds = setOf(ACTIVE_USER_ID), + unlockingVaultUserIds = emptySet(), + ) + + // Test & Verify + val actual = autofillCipherProvider.getLoginAutofillCiphers( + uri = URI, + ) + + assertEquals(LOGIN_CIPHERS, actual) + } + + @Test + fun `getLoginAutofillCiphers when locked should return an empty list`() = runTest { + mutableVaultStateFlow.value = VaultState( + unlockedVaultUserIds = emptySet(), + unlockingVaultUserIds = emptySet(), + ) + // Test & Verify val actual = autofillCipherProvider.getLoginAutofillCiphers( uri = URI, ) - assertEquals(LOGIN_CIPHERS, actual) + assertEquals(emptyList(), actual) } } +private const val ACTIVE_USER_ID = "activeUserId" + private val CARD_CIPHERS = listOf( AutofillCipher.Card( cardholderName = "John",