Skip to content

Commit 65b1a0c

Browse files
andrebispo5andrebispo5
committed
[PM-16670] Add check for 2fa status (#4542)
1 parent 0a8d1fa commit 65b1a0c

File tree

6 files changed

+79
-49
lines changed

6 files changed

+79
-49
lines changed

Diff for: ‎app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt

-1
Original file line numberDiff line numberDiff line change
@@ -1350,7 +1350,6 @@ class AuthRepositoryImpl(
13501350
}
13511351

13521352
override fun checkUserNeedsNewDeviceTwoFactorNotice(): Boolean {
1353-
vaultRepository.syncIfNecessary()
13541353
return activeUserId?.let { userId ->
13551354
val temporaryFlag = featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
13561355
val permanentFlag = featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)

Diff for: ‎app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModel.kt

+13
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,22 @@ package com.x8bit.bitwarden.ui.auth.feature.newdevicenotice
22

33
import android.os.Parcelable
44
import androidx.lifecycle.SavedStateHandle
5+
import androidx.lifecycle.viewModelScope
56
import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeDisplayStatus.CAN_ACCESS_EMAIL
67
import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeDisplayStatus.CAN_ACCESS_EMAIL_PERMANENT
78
import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeState
89
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
910
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
1011
import com.x8bit.bitwarden.data.platform.manager.model.FlagKey
12+
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
1113
import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeEmailAccessAction.ContinueClick
1214
import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeEmailAccessAction.EmailAccessToggle
1315
import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeEmailAccessAction.LearnMoreClick
1416
import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeEmailAccessEvent.NavigateToTwoFactorOptions
1517
import com.x8bit.bitwarden.ui.platform.base.BaseViewModel
1618
import dagger.hilt.android.lifecycle.HiltViewModel
1719
import kotlinx.coroutines.flow.update
20+
import kotlinx.coroutines.launch
1821
import kotlinx.parcelize.Parcelize
1922
import javax.inject.Inject
2023

@@ -26,6 +29,7 @@ private const val KEY_STATE = "state"
2629
@HiltViewModel
2730
class NewDeviceNoticeEmailAccessViewModel @Inject constructor(
2831
private val authRepository: AuthRepository,
32+
private val vaultRepository: VaultRepository,
2933
private val featureFlagManager: FeatureFlagManager,
3034
savedStateHandle: SavedStateHandle,
3135
) : BaseViewModel<
@@ -39,6 +43,15 @@ class NewDeviceNoticeEmailAccessViewModel @Inject constructor(
3943
isEmailAccessEnabled = false,
4044
),
4145
) {
46+
init {
47+
viewModelScope.launch {
48+
vaultRepository.syncForResult()
49+
if (!authRepository.checkUserNeedsNewDeviceTwoFactorNotice()) {
50+
sendEvent(NewDeviceNoticeEmailAccessEvent.NavigateBackToVault)
51+
}
52+
}
53+
}
54+
4255
override fun handleAction(action: NewDeviceNoticeEmailAccessAction) {
4356
when (action) {
4457
ContinueClick -> handleContinueClick()

Diff for: ‎app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModel.kt

+13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.x8bit.bitwarden.ui.auth.feature.newdevicenotice
22

33
import android.os.Parcelable
4+
import androidx.lifecycle.viewModelScope
45
import com.x8bit.bitwarden.R
56
import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeDisplayStatus
67
import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeState
@@ -10,6 +11,7 @@ import com.x8bit.bitwarden.data.platform.manager.model.FlagKey
1011
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
1112
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
1213
import com.x8bit.bitwarden.data.platform.repository.util.baseWebVaultUrlOrDefault
14+
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
1315
import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorAction.ChangeAccountEmailClick
1416
import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorAction.ContinueDialogClick
1517
import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorAction.DismissDialogClick
@@ -22,6 +24,7 @@ import com.x8bit.bitwarden.ui.platform.base.util.Text
2224
import com.x8bit.bitwarden.ui.platform.base.util.asText
2325
import dagger.hilt.android.lifecycle.HiltViewModel
2426
import kotlinx.coroutines.flow.update
27+
import kotlinx.coroutines.launch
2528
import kotlinx.parcelize.Parcelize
2629
import java.time.Clock
2730
import java.time.ZonedDateTime
@@ -36,6 +39,7 @@ class NewDeviceNoticeTwoFactorViewModel @Inject constructor(
3639
val environmentRepository: EnvironmentRepository,
3740
val featureFlagManager: FeatureFlagManager,
3841
val settingsRepository: SettingsRepository,
42+
val vaultRepository: VaultRepository,
3943
private val clock: Clock,
4044
) : BaseViewModel<
4145
NewDeviceNoticeTwoFactorState,
@@ -48,6 +52,15 @@ class NewDeviceNoticeTwoFactorViewModel @Inject constructor(
4852
),
4953
),
5054
) {
55+
init {
56+
viewModelScope.launch {
57+
vaultRepository.syncForResult()
58+
if (!authRepository.checkUserNeedsNewDeviceTwoFactorNotice()) {
59+
sendEvent(NewDeviceNoticeTwoFactorEvent.NavigateBackToVault)
60+
}
61+
}
62+
}
63+
5164
private val webTwoFactorUrl: String
5265
get() {
5366
val baseUrl = environmentRepository

Diff for: ‎app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt

-47
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,6 @@ class AuthRepositoryTest {
170170
private val vaultRepository: VaultRepository = mockk {
171171
every { vaultUnlockDataStateFlow } returns mutableVaultUnlockDataStateFlow
172172
every { deleteVaultData(any()) } just runs
173-
every { syncIfNecessary() } just runs
174173
}
175174
private val fakeAuthDiskSource = FakeAuthDiskSource()
176175
private val fakeEnvironmentRepository =
@@ -6543,9 +6542,6 @@ class AuthRepositoryTest {
65436542

65446543
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
65456544

6546-
verify(exactly = 1) {
6547-
vaultRepository.syncIfNecessary()
6548-
}
65496545
assertTrue(shouldShowNewDeviceNotice)
65506546
}
65516547

@@ -6568,9 +6564,6 @@ class AuthRepositoryTest {
65686564

65696565
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
65706566

6571-
verify(exactly = 1) {
6572-
vaultRepository.syncIfNecessary()
6573-
}
65746567
assertFalse(shouldShowNewDeviceNotice)
65756568
}
65766569

@@ -6592,9 +6585,6 @@ class AuthRepositoryTest {
65926585

65936586
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
65946587

6595-
verify(exactly = 1) {
6596-
vaultRepository.syncIfNecessary()
6597-
}
65986588
assertTrue(shouldShowNewDeviceNotice)
65996589
}
66006590

@@ -6613,9 +6603,6 @@ class AuthRepositoryTest {
66136603

66146604
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
66156605

6616-
verify(exactly = 1) {
6617-
vaultRepository.syncIfNecessary()
6618-
}
66196606
assertFalse(shouldShowNewDeviceNotice)
66206607
}
66216608

@@ -6636,9 +6623,6 @@ class AuthRepositoryTest {
66366623

66376624
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
66386625

6639-
verify(exactly = 1) {
6640-
vaultRepository.syncIfNecessary()
6641-
}
66426626
assertFalse(shouldShowNewDeviceNotice)
66436627
}
66446628

@@ -6654,9 +6638,6 @@ class AuthRepositoryTest {
66546638

66556639
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
66566640

6657-
verify(exactly = 1) {
6658-
vaultRepository.syncIfNecessary()
6659-
}
66606641
assertFalse(shouldShowNewDeviceNotice)
66616642
}
66626643

@@ -6682,9 +6663,6 @@ class AuthRepositoryTest {
66826663

66836664
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
66846665

6685-
verify(exactly = 1) {
6686-
vaultRepository.syncIfNecessary()
6687-
}
66886666
assertFalse(shouldShowNewDeviceNotice)
66896667
}
66906668

@@ -6708,9 +6686,6 @@ class AuthRepositoryTest {
67086686

67096687
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
67106688

6711-
verify(exactly = 1) {
6712-
vaultRepository.syncIfNecessary()
6713-
}
67146689
assertFalse(shouldShowNewDeviceNotice)
67156690
}
67166691

@@ -6734,9 +6709,6 @@ class AuthRepositoryTest {
67346709

67356710
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
67366711

6737-
verify(exactly = 1) {
6738-
vaultRepository.syncIfNecessary()
6739-
}
67406712
assertTrue(shouldShowNewDeviceNotice)
67416713
}
67426714

@@ -6760,9 +6732,6 @@ class AuthRepositoryTest {
67606732

67616733
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
67626734

6763-
verify(exactly = 1) {
6764-
vaultRepository.syncIfNecessary()
6765-
}
67666735
assertTrue(shouldShowNewDeviceNotice)
67676736
}
67686737

@@ -6786,9 +6755,6 @@ class AuthRepositoryTest {
67866755

67876756
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
67886757

6789-
verify(exactly = 1) {
6790-
vaultRepository.syncIfNecessary()
6791-
}
67926758
assertFalse(shouldShowNewDeviceNotice)
67936759
}
67946760

@@ -6817,9 +6783,6 @@ class AuthRepositoryTest {
68176783
} returns false
68186784

68196785
assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice())
6820-
verify(exactly = 2) {
6821-
vaultRepository.syncIfNecessary()
6822-
}
68236786
}
68246787

68256788
@Test
@@ -6833,9 +6796,6 @@ class AuthRepositoryTest {
68336796
fakeAuthDiskSource.userState = null
68346797

68356798
assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice())
6836-
verify(exactly = 1) {
6837-
vaultRepository.syncIfNecessary()
6838-
}
68396799
}
68406800

68416801
@Test
@@ -6857,10 +6817,6 @@ class AuthRepositoryTest {
68576817
),
68586818
)
68596819
assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice())
6860-
6861-
verify(exactly = 1) {
6862-
vaultRepository.syncIfNecessary()
6863-
}
68646820
}
68656821

68666822
@Test
@@ -6884,9 +6840,6 @@ class AuthRepositoryTest {
68846840
)
68856841

68866842
assertTrue(repository.checkUserNeedsNewDeviceTwoFactorNotice())
6887-
verify(exactly = 1) {
6888-
vaultRepository.syncIfNecessary()
6889-
}
68906843
}
68916844

68926845
companion object {

Diff for: ‎app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModelTest.kt

+25
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.model.NewDeviceNoticeState
77
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
88
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
99
import com.x8bit.bitwarden.data.platform.manager.model.FlagKey
10+
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
1011
import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest
1112
import io.mockk.every
1213
import io.mockk.just
@@ -32,6 +33,8 @@ class NewDeviceNoticeEmailAccessViewModelTest : BaseViewModelTest() {
3233
every { getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss) } returns true
3334
}
3435

36+
private val vaultRepository = mockk<VaultRepository>(relaxed = true)
37+
3538
@Test
3639
fun `initial state should be correct with email from state handle`() = runTest {
3740
val viewModel = createViewModel()
@@ -40,6 +43,27 @@ class NewDeviceNoticeEmailAccessViewModelTest : BaseViewModelTest() {
4043
}
4144
}
4245

46+
@Test
47+
fun `Init should not send events if user needs new device notice`() = runTest {
48+
every { authRepository.checkUserNeedsNewDeviceTwoFactorNotice() } returns true
49+
val viewModel = createViewModel()
50+
viewModel.eventFlow.test {
51+
expectNoEvents()
52+
}
53+
}
54+
55+
@Test
56+
fun `Init should send NavigateBackToVault if user does not need new device notice`() = runTest {
57+
every { authRepository.checkUserNeedsNewDeviceTwoFactorNotice() } returns false
58+
val viewModel = createViewModel()
59+
viewModel.eventFlow.test {
60+
assertEquals(
61+
NewDeviceNoticeEmailAccessEvent.NavigateBackToVault,
62+
awaitItem(),
63+
)
64+
}
65+
}
66+
4367
@Test
4468
fun `EmailAccessToggle should update value of isEmailAccessEnabled`() = runTest {
4569
val viewModel = createViewModel()
@@ -127,6 +151,7 @@ class NewDeviceNoticeEmailAccessViewModelTest : BaseViewModelTest() {
127151
): NewDeviceNoticeEmailAccessViewModel = NewDeviceNoticeEmailAccessViewModel(
128152
authRepository = authRepository,
129153
featureFlagManager = featureFlagManager,
154+
vaultRepository = vaultRepository,
130155
savedStateHandle = savedStateHandle,
131156
)
132157
}

Diff for: ‎app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModelTest.kt

+28-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
88
import com.x8bit.bitwarden.data.platform.manager.model.FlagKey
99
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
1010
import com.x8bit.bitwarden.data.platform.repository.util.FakeEnvironmentRepository
11+
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
1112
import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorDialogState.ChangeAccountEmailDialog
1213
import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeTwoFactorDialogState.TurnOnTwoFactorDialog
1314
import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest
@@ -24,7 +25,9 @@ import java.time.ZonedDateTime
2425

2526
class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() {
2627
private val environmentRepository = FakeEnvironmentRepository()
27-
private val authRepository = mockk<AuthRepository>(relaxed = true)
28+
private val authRepository = mockk<AuthRepository>(relaxed = true) {
29+
every { checkUserNeedsNewDeviceTwoFactorNotice() } returns true
30+
}
2831

2932
private val featureFlagManager = mockk<FeatureFlagManager>(relaxed = true) {
3033
every { getFeatureFlag(FlagKey.NewDevicePermanentDismiss) } returns false
@@ -33,6 +36,8 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() {
3336

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

39+
private val vaultRepository = mockk<VaultRepository>(relaxed = true)
40+
3641
@Test
3742
fun `initial state should be correct with NewDevicePermanentDismiss flag false`() = runTest {
3843
val viewModel = createViewModel()
@@ -53,6 +58,27 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() {
5358
}
5459
}
5560

61+
@Test
62+
fun `Init should not send events if user needs new device notice`() = runTest {
63+
every { authRepository.checkUserNeedsNewDeviceTwoFactorNotice() } returns true
64+
val viewModel = createViewModel()
65+
viewModel.eventFlow.test {
66+
expectNoEvents()
67+
}
68+
}
69+
70+
@Test
71+
fun `Init should send NavigateBackToVault if user does not need new device notice`() = runTest {
72+
every { authRepository.checkUserNeedsNewDeviceTwoFactorNotice() } returns false
73+
val viewModel = createViewModel()
74+
viewModel.eventFlow.test {
75+
assertEquals(
76+
NewDeviceNoticeTwoFactorEvent.NavigateBackToVault,
77+
awaitItem(),
78+
)
79+
}
80+
}
81+
5682
@Test
5783
fun `initial state should be correct with email from state handle`() = runTest {
5884
val viewModel = createViewModel()
@@ -187,6 +213,7 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() {
187213
environmentRepository = environmentRepository,
188214
featureFlagManager = featureFlagManager,
189215
settingsRepository = settingsRepository,
216+
vaultRepository = vaultRepository,
190217
clock = FIXED_CLOCK,
191218
)
192219
}

0 commit comments

Comments
 (0)