Skip to content

Commit

Permalink
[PM-16670] Add check for 2fa status #4542 (#4543)
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrebispo5
andrebispo5 authored Jan 9, 2025
1 parent 0a8d1fa commit 3e55256
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 49 deletions.
1 change: 0 additions & 1 deletion 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,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)
Expand Down
13 changes: 13 additions & 0 deletions ...om/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeEmailAccessViewModel.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,22 @@ 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
import com.x8bit.bitwarden.ui.auth.feature.newdevicenotice.NewDeviceNoticeEmailAccessEvent.NavigateToTwoFactorOptions
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

Expand All @@ -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<
Expand All @@ -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()
Expand Down
13 changes: 13 additions & 0 deletions .../com/x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModel.kt
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand Down
47 changes: 0 additions & 47 deletions 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,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 =
Expand Down Expand Up @@ -6543,9 +6542,6 @@ class AuthRepositoryTest {

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()

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

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

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

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

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

@Test
Expand All @@ -6857,10 +6817,6 @@ class AuthRepositoryTest {
),
)
assertFalse(repository.checkUserNeedsNewDeviceTwoFactorNotice())

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

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

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

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

private val vaultRepository = mockk<VaultRepository>(relaxed = true)

@Test
fun `initial state should be correct with email from state handle`() = runTest {
val viewModel = createViewModel()
Expand All @@ -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()
Expand Down Expand Up @@ -127,6 +151,7 @@ class NewDeviceNoticeEmailAccessViewModelTest : BaseViewModelTest() {
): NewDeviceNoticeEmailAccessViewModel = NewDeviceNoticeEmailAccessViewModel(
authRepository = authRepository,
featureFlagManager = featureFlagManager,
vaultRepository = vaultRepository,
savedStateHandle = savedStateHandle,
)
}
Expand Down
29 changes: 28 additions & 1 deletion .../x8bit/bitwarden/ui/auth/feature/newdevicenotice/NewDeviceNoticeTwoFactorViewModelTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -24,7 +25,9 @@ import java.time.ZonedDateTime

class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() {
private val environmentRepository = FakeEnvironmentRepository()
private val authRepository = mockk<AuthRepository>(relaxed = true)
private val authRepository = mockk<AuthRepository>(relaxed = true) {
every { checkUserNeedsNewDeviceTwoFactorNotice() } returns true
}

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

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

private val vaultRepository = mockk<VaultRepository>(relaxed = true)

@Test
fun `initial state should be correct with NewDevicePermanentDismiss flag false`() = runTest {
val viewModel = createViewModel()
Expand All @@ -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()
Expand Down Expand Up @@ -187,6 +213,7 @@ class NewDeviceNoticeTwoFactorViewModelTest : BaseViewModelTest() {
environmentRepository = environmentRepository,
featureFlagManager = featureFlagManager,
settingsRepository = settingsRepository,
vaultRepository = vaultRepository,
clock = FIXED_CLOCK,
)
}
Expand Down

0 comments on commit 3e55256

Please sign in to comment.