From 6b6e95aa3f6b14280d682295c5548b6bb37c6448 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bispo?= Date: Thu, 9 Jan 2025 00:25:38 +0000 Subject: [PATCH] [PM-16670] Force app to sync after 2FA notice (#4525) (#4536) --- .../auth/repository/AuthRepositoryImpl.kt | 1 + .../NewDeviceNoticeTwoFactorViewModel.kt | 6 +++ .../auth/repository/AuthRepositoryTest.kt | 48 ++++++++++++++++++- .../NewDeviceNoticeTwoFactorViewModelTest.kt | 10 ++++ 4 files changed, 64 insertions(+), 1 deletion(-) 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 0b92fc6ce4..d3378ebf3b 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,6 +1350,7 @@ 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/NewDeviceNoticeTwoFactorViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModel.kt index 67b7e9ab4d..b22664c464 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 @@ -8,6 +8,7 @@ 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.platform.repository.EnvironmentRepository +import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.util.baseWebVaultUrlOrDefault import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorAction.ChangeAccountEmailClick import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorAction.ContinueDialogClick @@ -34,6 +35,7 @@ class NewDeviceNoticeTwoFactorViewModel @Inject constructor( val authRepository: AuthRepository, val environmentRepository: EnvironmentRepository, val featureFlagManager: FeatureFlagManager, + val settingsRepository: SettingsRepository, private val clock: Clock, ) : BaseViewModel< NewDeviceNoticeTwoFactorState, @@ -91,6 +93,8 @@ class NewDeviceNoticeTwoFactorViewModel @Inject constructor( private fun handleContinueDialog() { when (state.dialogState) { is ChangeAccountEmailDialog -> { + // when the user leaves the app set sync date to null to force a sync on next unlock + settingsRepository.vaultLastSync = null sendEvent( NewDeviceNoticeTwoFactorEvent.NavigateToChangeAccountEmail(url = webAccountUrl), ) @@ -98,6 +102,8 @@ class NewDeviceNoticeTwoFactorViewModel @Inject constructor( } is TurnOnTwoFactorDialog -> { + // when the user leaves the app set sync date to null to force a sync on next unlock + settingsRepository.vaultLastSync = null sendEvent( NewDeviceNoticeTwoFactorEvent.NavigateToTurnOnTwoFactor(url = webTwoFactorUrl), ) 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 295014c384..08c168773d 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,6 +170,7 @@ 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 = @@ -6542,6 +6543,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertTrue(shouldShowNewDeviceNotice) } @@ -6564,6 +6568,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertFalse(shouldShowNewDeviceNotice) } @@ -6585,6 +6592,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertTrue(shouldShowNewDeviceNotice) } @@ -6603,6 +6613,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertFalse(shouldShowNewDeviceNotice) } @@ -6623,6 +6636,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertFalse(shouldShowNewDeviceNotice) } @@ -6638,6 +6654,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertFalse(shouldShowNewDeviceNotice) } @@ -6663,6 +6682,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertFalse(shouldShowNewDeviceNotice) } @@ -6686,6 +6708,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertFalse(shouldShowNewDeviceNotice) } @@ -6709,6 +6734,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertTrue(shouldShowNewDeviceNotice) } @@ -6732,6 +6760,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertTrue(shouldShowNewDeviceNotice) } @@ -6755,6 +6786,9 @@ class AuthRepositoryTest { val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice() + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } assertFalse(shouldShowNewDeviceNotice) } @@ -6783,6 +6817,9 @@ class AuthRepositoryTest { } returns false assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice()) + verify(exactly = 2) { + vaultRepository.syncIfNecessary() + } } @Test @@ -6796,6 +6833,9 @@ class AuthRepositoryTest { fakeAuthDiskSource.userState = null assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice()) + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } } @Test @@ -6816,8 +6856,11 @@ class AuthRepositoryTest { ), ), ) - assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice()) + + verify(exactly = 1) { + vaultRepository.syncIfNecessary() + } } @Test @@ -6841,6 +6884,9 @@ 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/NewDeviceNoticeTwoFactorViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModelTest.kt index f9ace923ea..c11558f0ce 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 @@ -6,6 +6,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.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.util.FakeEnvironmentRepository import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorDialogState.ChangeAccountEmailDialog import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorDialogState.TurnOnTwoFactorDialog @@ -30,6 +31,8 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() { every { getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss) } returns true } + private val settingsRepository = mockk(relaxed = true) + @Test fun `initial state should be correct with NewDevicePermanentDismiss flag false`() = runTest { val viewModel = createViewModel() @@ -135,6 +138,9 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() { DEFAULT_STATE, viewModel.stateFlow.value, ) + verify(exactly = 1) { + settingsRepository.vaultLastSync = null + } } } @@ -156,6 +162,9 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() { DEFAULT_STATE, viewModel.stateFlow.value, ) + verify(exactly = 1) { + settingsRepository.vaultLastSync = null + } } } @@ -177,6 +186,7 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() { authRepository = authRepository, environmentRepository = environmentRepository, featureFlagManager = featureFlagManager, + settingsRepository = settingsRepository, clock = FIXED_CLOCK, ) }