Skip to content

Commit

Permalink
[PM-16670] Force app to sync after 2FA notice (#4525)
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrebispo5
andrebispo5 authored Jan 8, 2025
1 parent 8310a03 commit 08d173e
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 1 deletion.
1 change: 1 addition & 0 deletions app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions .../com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModel.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -91,13 +93,17 @@ 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),
)
updateDialogState(newState = null)
}

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),
)
Expand Down
48 changes: 47 additions & 1 deletion app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -6542,6 +6543,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertTrue(shouldShowNewDeviceNotice)
}

Expand All @@ -6564,6 +6568,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertFalse(shouldShowNewDeviceNotice)
}

Expand All @@ -6585,6 +6592,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertTrue(shouldShowNewDeviceNotice)
}

Expand All @@ -6603,6 +6613,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertFalse(shouldShowNewDeviceNotice)
}

Expand All @@ -6623,6 +6636,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertFalse(shouldShowNewDeviceNotice)
}

Expand All @@ -6638,6 +6654,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertFalse(shouldShowNewDeviceNotice)
}

Expand All @@ -6663,6 +6682,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertFalse(shouldShowNewDeviceNotice)
}

Expand All @@ -6686,6 +6708,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertFalse(shouldShowNewDeviceNotice)
}

Expand All @@ -6709,6 +6734,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertTrue(shouldShowNewDeviceNotice)
}

Expand All @@ -6732,6 +6760,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertTrue(shouldShowNewDeviceNotice)
}

Expand All @@ -6755,6 +6786,9 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
assertFalse(shouldShowNewDeviceNotice)
}

Expand Down Expand Up @@ -6783,6 +6817,9 @@ class AuthRepositoryTest {
} returns false

assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice())
verify(exactly = 2) {
vaultRepository.syncIfNecessary()
}
}

@Test
Expand All @@ -6796,6 +6833,9 @@ class AuthRepositoryTest {
fakeAuthDiskSource.userState = null

assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice())
verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
}

@Test
Expand All @@ -6816,8 +6856,11 @@ class AuthRepositoryTest {
),
),
)

assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice())

verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
}

@Test
Expand All @@ -6841,6 +6884,9 @@ class AuthRepositoryTest {
)

assertTrue(repository.checkUserNeedsNewDeviceTwoFactorNotice())
verify(exactly = 1) {
vaultRepository.syncIfNecessary()
}
}

companion object {
Expand Down
10 changes: 10 additions & 0 deletions .../x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModelTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -30,6 +31,8 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() {
every { getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss) } returns true
}

private val settingsRepository = mockk<SettingsRepository>(relaxed = true)

@Test
fun `initial state should be correct with NewDevicePermanentDismiss flag false`() = runTest {
val viewModel = createViewModel()
Expand Down Expand Up @@ -135,6 +138,9 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() {
DEFAULT_STATE,
viewModel.stateFlow.value,
)
verify(exactly = 1) {
settingsRepository.vaultLastSync = null
}
}
}

Expand All @@ -156,6 +162,9 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() {
DEFAULT_STATE,
viewModel.stateFlow.value,
)
verify(exactly = 1) {
settingsRepository.vaultLastSync = null
}
}
}

Expand All @@ -177,6 +186,7 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() {
authRepository = authRepository,
environmentRepository = environmentRepository,
featureFlagManager = featureFlagManager,
settingsRepository = settingsRepository,
clock = FIXED_CLOCK,
)
}
Expand Down

0 comments on commit 08d173e

Please sign in to comment.