From 3e552564dc2095ca51c84cd64a42634b2c4ddbb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bispo?= Date: Thu, 9 Jan 2025 22:44:06 +0000 Subject: [PATCH] [PM-16670] Add check for 2fa status #4542 (#4543) --- .../auth/repository/AuthRepositoryImpl.kt | 1 - .../NewDeviceNoticeEmailAccessViewModel.kt | 13 +++++ .../NewDeviceNoticeTwoFactorViewModel.kt | 13 +++++ .../auth/repository/AuthRepositoryTest.kt | 47 ------------------- ...NewDeviceNoticeEmailAccessViewModelTest.kt | 25 ++++++++++ .../NewDeviceNoticeTwoFactorViewModelTest.kt | 29 +++++++++++- 6 files changed, 79 insertions(+), 49 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index d3378ebf3b..0b92fc6ce4 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -1350,7 +1350,6 @@ class AuthRepositoryImpl( } override fun checkUserNeedsNewDeviceTwoFactorNotice(): Boolean { - vaultRepository.syncIfNecessary() return activeUserId?.let { userId -> val temporaryFlag = featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss) val permanentFlag = featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModel.kt index d704a79543..eb93525e63 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModel.kt @@ -2,12 +2,14 @@ package com.x8bit.bitwarden.ui.auth.feature.newdevicenotice import android.os.Parcelable import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.viewModelScope import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeDisplayStatus.CAN_ACCESS_EMAIL import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeDisplayStatus.CAN_ACCESS_EMAIL_PERMANENT import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeState import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.model.FlagKey +import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeEmailAccessAction.ContinueClick import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeEmailAccessAction.EmailAccessToggle import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeEmailAccessAction.LearnMoreClick @@ -15,6 +17,7 @@ import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeEmailA import com.x8bit.bitwarden.ui.platform.base.BaseViewModel import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch import kotlinx.parcelize.Parcelize import javax.inject.Inject @@ -26,6 +29,7 @@ private const val KEY_STATE = "state" @HiltViewModel class NewDeviceNoticeEmailAccessViewModel @Inject constructor( private val authRepository: AuthRepository, + private val vaultRepository: VaultRepository, private val featureFlagManager: FeatureFlagManager, savedStateHandle: SavedStateHandle, ) : BaseViewModel< @@ -39,6 +43,15 @@ class NewDeviceNoticeEmailAccessViewModel @Inject constructor( isEmailAccessEnabled = false, ), ) { + init { + viewModelScope.launch { + vaultRepository.syncForResult() + if (!authRepository.checkUserNeedsNewDeviceTwoFactorNotice()) { + sendEvent(NewDeviceNoticeEmailAccessEvent.NavigateBackToVault) + } + } + } + override fun handleAction(action: NewDeviceNoticeEmailAccessAction) { when (action) { ContinueClick -> handleContinueClick() diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModel.kt index b22664c464..c9eecd6f2f 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModel.kt @@ -1,6 +1,7 @@ package com.x8bit.bitwarden.ui.auth.feature.newdevicenotice import android.os.Parcelable +import androidx.lifecycle.viewModelScope import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeDisplayStatus import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeState @@ -10,6 +11,7 @@ import com.x8bit.bitwarden.data.platform.manager.model.FlagKey import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.util.baseWebVaultUrlOrDefault +import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorAction.ChangeAccountEmailClick import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorAction.ContinueDialogClick import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorAction.DismissDialogClick @@ -22,6 +24,7 @@ import com.x8bit.bitwarden.ui.platform.base.util.Text import com.x8bit.bitwarden.ui.platform.base.util.asText import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch import kotlinx.parcelize.Parcelize import java.time.Clock import java.time.ZonedDateTime @@ -36,6 +39,7 @@ class NewDeviceNoticeTwoFactorViewModel @Inject constructor( val environmentRepository: EnvironmentRepository, val featureFlagManager: FeatureFlagManager, val settingsRepository: SettingsRepository, + val vaultRepository: VaultRepository, private val clock: Clock, ) : BaseViewModel< NewDeviceNoticeTwoFactorState, @@ -48,6 +52,15 @@ class NewDeviceNoticeTwoFactorViewModel @Inject constructor( ), ), ) { + init { + viewModelScope.launch { + vaultRepository.syncForResult() + if (!authRepository.checkUserNeedsNewDeviceTwoFactorNotice()) { + sendEvent(NewDeviceNoticeTwoFactorEvent.NavigateBackToVault) + } + } + } + private val webTwoFactorUrl: String get() { val baseUrl = environmentRepository diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index 08c168773d..631868036b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -170,7 +170,6 @@ class AuthRepositoryTest { private val vaultRepository: VaultRepository = mockk { every { vaultUnlockDataStateFlow } returns mutableVaultUnlockDataStateFlow every { deleteVaultData(any()) } just runs - every { syncIfNecessary() } just runs } private val fakeAuthDiskSource = FakeAuthDiskSource() private val fakeEnvironmentRepository = @@ -6543,9 +6542,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertTrue(shouldShowNewDeviceNotice) } @@ -6568,9 +6564,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertFalse(shouldShowNewDeviceNotice) } @@ -6592,9 +6585,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertTrue(shouldShowNewDeviceNotice) } @@ -6613,9 +6603,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertFalse(shouldShowNewDeviceNotice) } @@ -6636,9 +6623,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertFalse(shouldShowNewDeviceNotice) } @@ -6654,9 +6638,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertFalse(shouldShowNewDeviceNotice) } @@ -6682,9 +6663,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertFalse(shouldShowNewDeviceNotice) } @@ -6708,9 +6686,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertFalse(shouldShowNewDeviceNotice) } @@ -6734,9 +6709,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertTrue(shouldShowNewDeviceNotice) } @@ -6760,9 +6732,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertTrue(shouldShowNewDeviceNotice) } @@ -6786,9 +6755,6 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } assertFalse(shouldShowNewDeviceNotice) } @@ -6817,9 +6783,6 @@ class AuthRepositoryTest { } returns false assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice()) - verify(exactly = 2) { - vaultRepository.syncIfNecessary() - } } @Test @@ -6833,9 +6796,6 @@ class AuthRepositoryTest { fakeAuthDiskSource.userState = null assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice()) - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } } @Test @@ -6857,10 +6817,6 @@ class AuthRepositoryTest { ), ) assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice()) - - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } } @Test @@ -6884,9 +6840,6 @@ class AuthRepositoryTest { ) assertTrue(repository.checkUserNeedsNewDeviceTwoFactorNotice()) - verify(exactly = 1) { - vaultRepository.syncIfNecessary() - } } companion object { diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModelTest.kt index 23134a86cb..0b431cbea1 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModelTest.kt @@ -7,6 +7,7 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeState import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.model.FlagKey +import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest import io.mockk.every import io.mockk.just @@ -32,6 +33,8 @@ class NewDeviceNoticeEmailAccessViewModelTest : BaseViewModelTest() { every { getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss) } returns true } + private val vaultRepository = mockk(relaxed = true) + @Test fun `initial state should be correct with email from state handle`() = runTest { val viewModel = createViewModel() @@ -40,6 +43,27 @@ class NewDeviceNoticeEmailAccessViewModelTest : BaseViewModelTest() { } } + @Test + fun `Init should not send events if user needs new device notice`() = runTest { + every { authRepository.checkUserNeedsNewDeviceTwoFactorNotice() } returns true + val viewModel = createViewModel() + viewModel.eventFlow.test { + expectNoEvents() + } + } + + @Test + fun `Init should send NavigateBackToVault if user does not need new device notice`() = runTest { + every { authRepository.checkUserNeedsNewDeviceTwoFactorNotice() } returns false + val viewModel = createViewModel() + viewModel.eventFlow.test { + assertEquals( + NewDeviceNoticeEmailAccessEvent.NavigateBackToVault, + awaitItem(), + ) + } + } + @Test fun `EmailAccessToggle should update value of isEmailAccessEnabled`() = runTest { val viewModel = createViewModel() @@ -127,6 +151,7 @@ class NewDeviceNoticeEmailAccessViewModelTest : BaseViewModelTest() { ): NewDeviceNoticeEmailAccessViewModel = NewDeviceNoticeEmailAccessViewModel( authRepository = authRepository, featureFlagManager = featureFlagManager, + vaultRepository = vaultRepository, savedStateHandle = savedStateHandle, ) } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModelTest.kt index c11558f0ce..2ee48fa918 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModelTest.kt @@ -8,6 +8,7 @@ import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.model.FlagKey import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.util.FakeEnvironmentRepository +import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorDialogState.ChangeAccountEmailDialog import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorDialogState.TurnOnTwoFactorDialog import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest @@ -24,7 +25,9 @@ import java.time.ZonedDateTime class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() { private val environmentRepository = FakeEnvironmentRepository() - private val authRepository = mockk(relaxed = true) + private val authRepository = mockk(relaxed = true) { + every { checkUserNeedsNewDeviceTwoFactorNotice() } returns true + } private val featureFlagManager = mockk(relaxed = true) { every { getFeatureFlag(FlagKey.NewDevicePermanentDismiss) } returns false @@ -33,6 +36,8 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() { private val settingsRepository = mockk(relaxed = true) + private val vaultRepository = mockk(relaxed = true) + @Test fun `initial state should be correct with NewDevicePermanentDismiss flag false`() = runTest { val viewModel = createViewModel() @@ -53,6 +58,27 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() { } } + @Test + fun `Init should not send events if user needs new device notice`() = runTest { + every { authRepository.checkUserNeedsNewDeviceTwoFactorNotice() } returns true + val viewModel = createViewModel() + viewModel.eventFlow.test { + expectNoEvents() + } + } + + @Test + fun `Init should send NavigateBackToVault if user does not need new device notice`() = runTest { + every { authRepository.checkUserNeedsNewDeviceTwoFactorNotice() } returns false + val viewModel = createViewModel() + viewModel.eventFlow.test { + assertEquals( + NewDeviceNoticeTwoFactorEvent.NavigateBackToVault, + awaitItem(), + ) + } + } + @Test fun `initial state should be correct with email from state handle`() = runTest { val viewModel = createViewModel() @@ -187,6 +213,7 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() { environmentRepository = environmentRepository, featureFlagManager = featureFlagManager, settingsRepository = settingsRepository, + vaultRepository = vaultRepository, clock = FIXED_CLOCK, ) }