BIT-853: Implement account switching (#316)

This commit is contained in:
Brian Yencho 2023-12-04 18:40:57 -06:00 committed by Álison Fernandes
parent f8fefae3b8
commit 58aa38ceea
9 changed files with 312 additions and 76 deletions

View file

@ -6,6 +6,7 @@ import com.x8bit.bitwarden.data.auth.repository.model.DeleteAccountResult
import com.x8bit.bitwarden.data.auth.repository.model.LoginResult
import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult
import com.x8bit.bitwarden.data.auth.repository.model.RegisterResult
import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.auth.repository.util.CaptchaCallbackTokenResult
import com.x8bit.bitwarden.data.platform.datasource.network.authenticator.AuthenticatorProvider
@ -66,6 +67,11 @@ interface AuthRepository : AuthenticatorProvider {
*/
fun logout()
/**
* Switches to the account corresponding to the given [userId] if possible.
*/
fun switchAccount(userId: String): SwitchAccountResult
/**
* Attempt to register a new account with the given parameters.
*/

View file

@ -20,6 +20,7 @@ import com.x8bit.bitwarden.data.auth.repository.model.DeleteAccountResult
import com.x8bit.bitwarden.data.auth.repository.model.LoginResult
import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult
import com.x8bit.bitwarden.data.auth.repository.model.RegisterResult
import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.auth.repository.util.CaptchaCallbackTokenResult
import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams
@ -258,6 +259,31 @@ class AuthRepositoryImpl constructor(
if (wasActiveUser) vaultRepository.clearUnlockedData()
}
@Suppress("ReturnCount")
override fun switchAccount(userId: String): SwitchAccountResult {
val currentUserState = authDiskSource.userState
?: return SwitchAccountResult.NoChange
val previousActiveUserId = currentUserState.activeUserId
if (userId == previousActiveUserId) {
// Nothing to do
return SwitchAccountResult.NoChange
}
if (userId !in currentUserState.accounts.keys) {
// The requested user is not currently stored
return SwitchAccountResult.NoChange
}
// Switch to the new user
authDiskSource.userState = currentUserState.copy(activeUserId = userId)
// Lock and clear data for the previous user
vaultRepository.lockVaultIfNecessary(previousActiveUserId)
vaultRepository.clearUnlockedData()
return SwitchAccountResult.AccountSwitched
}
@Suppress("ReturnCount", "LongMethod")
override suspend fun register(
email: String,

View file

@ -0,0 +1,17 @@
package com.x8bit.bitwarden.data.auth.repository.model
/**
* Describes the result of attempting to switch user accounts locally.
*/
sealed class SwitchAccountResult {
/**
* The user account was switched successfully.
*/
data object AccountSwitched : SwitchAccountResult()
/**
* There was no change in accounts when attempting to switch users.
*/
data object NoChange : SwitchAccountResult()
}

View file

@ -111,8 +111,7 @@ class VaultUnlockViewModel @Inject constructor(
}
private fun handleSwitchAccountClick(action: VaultUnlockAction.SwitchAccountClick) {
// TODO: Handle switching accounts (BIT-853)
sendEvent(VaultUnlockEvent.ShowToast("Not yet implemented.".asText()))
authRepository.switchAccount(userId = action.accountSummary.userId)
}
private fun handleUnlockClick() {

View file

@ -98,13 +98,6 @@ fun VaultScreen(
.show()
}
VaultEvent.NavigateToVaultUnlockScreen -> {
// TODO: Handle unlocking accounts (BIT-853)
Toast
.makeText(context, "Not yet implemented.", Toast.LENGTH_SHORT)
.show()
}
is VaultEvent.ShowToast -> {
Toast
.makeText(context, event.message, Toast.LENGTH_SHORT)

View file

@ -6,6 +6,7 @@ import androidx.compose.ui.graphics.Color
import androidx.lifecycle.viewModelScope
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.platform.repository.model.DataState
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
@ -121,20 +122,13 @@ class VaultViewModel @Inject constructor(
}
private fun handleAccountSwitchClick(action: VaultAction.AccountSwitchClick) {
when (action.accountSummary.status) {
AccountSummary.Status.ACTIVE -> {
// Nothing to do for the active account
}
AccountSummary.Status.LOCKED -> {
// TODO: Handle switching accounts (BIT-853)
sendEvent(VaultEvent.NavigateToVaultUnlockScreen)
}
AccountSummary.Status.UNLOCKED -> {
// TODO: Handle switching accounts (BIT-853)
sendEvent(VaultEvent.ShowToast(message = "Not yet implemented."))
val isSwitchingAccounts =
when (authRepository.switchAccount(userId = action.accountSummary.userId)) {
SwitchAccountResult.AccountSwitched -> true
SwitchAccountResult.NoChange -> false
}
mutableStateFlow.update {
it.copy(isSwitchingAccounts = isSwitchingAccounts)
}
}
@ -159,6 +153,10 @@ class VaultViewModel @Inject constructor(
// out.
val userState = action.userState ?: return
// Avoid updating the UI if we are actively switching users to avoid changes while
// navigating.
if (state.isSwitchingAccounts) return
mutableStateFlow.update {
val accountSummaries = userState.toAccountSummaries()
val activeAccountSummary = userState.toActiveAccountSummary()
@ -171,6 +169,10 @@ class VaultViewModel @Inject constructor(
}
private fun handleVaultDataReceive(action: VaultAction.Internal.VaultDataReceive) {
// Avoid updating the UI if we are actively switching users to avoid changes while
// navigating.
if (state.isSwitchingAccounts) return
when (val vaultData = action.vaultData) {
is DataState.Error -> vaultErrorReceive(vaultData = vaultData)
is DataState.Loaded -> vaultLoadedReceive(vaultData = vaultData)
@ -213,7 +215,9 @@ class VaultViewModel @Inject constructor(
*
* @property avatarColorString The color of the avatar in HEX format.
* @property initials The initials to be displayed on the avatar.
* @property accountSummaries List of all the current accounts.
* @property viewState The specific view state representing loading, no items, or content state.
* @property isSwitchingAccounts Whether or not we are actively switching accounts.
*/
@Parcelize
data class VaultState(
@ -221,6 +225,8 @@ data class VaultState(
val initials: String,
val accountSummaries: List<AccountSummary>,
val viewState: ViewState,
// Internal-use properties
val isSwitchingAccounts: Boolean = false,
) : Parcelable {
/**
@ -445,11 +451,6 @@ sealed class VaultEvent {
*/
data object NavigateToSecureNotesGroup : VaultEvent()
/**
* Navigate to the vault unlock screen.
*/
data object NavigateToVaultUnlockScreen : VaultEvent()
/**
* Show a toast with the given [message].
*/

View file

@ -30,6 +30,7 @@ import com.x8bit.bitwarden.data.auth.repository.model.DeleteAccountResult
import com.x8bit.bitwarden.data.auth.repository.model.LoginResult
import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult
import com.x8bit.bitwarden.data.auth.repository.model.RegisterResult
import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.auth.repository.util.CaptchaCallbackTokenResult
import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams
@ -1079,6 +1080,106 @@ class AuthRepositoryTest {
}
}
@Test
fun `switchAccount when there is no saved UserState should do nothing`() {
val updatedUserId = USER_ID_2
fakeAuthDiskSource.userState = null
assertNull(repository.userStateFlow.value)
assertEquals(
SwitchAccountResult.NoChange,
repository.switchAccount(userId = updatedUserId),
)
assertNull(repository.userStateFlow.value)
verify(exactly = 0) { vaultRepository.lockVaultIfNecessary(any()) }
verify(exactly = 0) { vaultRepository.clearUnlockedData() }
}
@Suppress("MaxLineLength")
@Test
fun `switchAccount when the given userId is the same as the current activeUserId should do nothing`() {
val originalUserId = USER_ID_1
val originalUserState = SINGLE_USER_STATE_1.toUserState(
vaultState = VAULT_STATE,
specialCircumstance = null,
)
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
assertEquals(
originalUserState,
repository.userStateFlow.value,
)
assertEquals(
SwitchAccountResult.NoChange,
repository.switchAccount(userId = originalUserId),
)
assertEquals(
originalUserState,
repository.userStateFlow.value,
)
verify(exactly = 0) { vaultRepository.lockVaultIfNecessary(originalUserId) }
verify(exactly = 0) { vaultRepository.clearUnlockedData() }
}
@Suppress("MaxLineLength")
@Test
fun `switchAccount when the given userId does not correspond to a saved account should do nothing`() {
val originalUserId = USER_ID_1
val invalidId = "invalidId"
val originalUserState = SINGLE_USER_STATE_1.toUserState(
vaultState = VAULT_STATE,
specialCircumstance = null,
)
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
assertEquals(
originalUserState,
repository.userStateFlow.value,
)
assertEquals(
SwitchAccountResult.NoChange,
repository.switchAccount(userId = invalidId),
)
assertEquals(
originalUserState,
repository.userStateFlow.value,
)
verify(exactly = 0) { vaultRepository.lockVaultIfNecessary(originalUserId) }
verify(exactly = 0) { vaultRepository.clearUnlockedData() }
}
@Suppress("MaxLineLength")
@Test
fun `switchAccount when the userId is valid should update the current UserState, lock the vault of the previous active user, and clear the previously unlocked data`() {
val originalUserId = USER_ID_1
val updatedUserId = USER_ID_2
val originalUserState = MULTI_USER_STATE.toUserState(
vaultState = VAULT_STATE,
specialCircumstance = null,
)
fakeAuthDiskSource.userState = MULTI_USER_STATE
assertEquals(
originalUserState,
repository.userStateFlow.value,
)
assertEquals(
SwitchAccountResult.AccountSwitched,
repository.switchAccount(userId = updatedUserId),
)
assertEquals(
originalUserState.copy(activeUserId = updatedUserId),
repository.userStateFlow.value,
)
verify { vaultRepository.lockVaultIfNecessary(originalUserId) }
verify { vaultRepository.clearUnlockedData() }
}
@Test
fun `getPasswordBreachCount should return failure when service returns failure`() = runTest {
val password = "password"

View file

@ -1,10 +1,10 @@
package com.x8bit.bitwarden.ui.auth.feature.vaultunlock
import androidx.lifecycle.SavedStateHandle
import app.cash.turbine.test
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.datasource.disk.model.EnvironmentUrlDataJson
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.auth.repository.model.UserState.SpecialCircumstance
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
@ -36,6 +36,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
every { specialCircumstance } returns null
every { specialCircumstance = any() } just runs
every { logout() } just runs
every { switchAccount(any()) } returns SwitchAccountResult.AccountSwitched
}
private val vaultRepository = mockk<VaultRepository>()
@ -163,15 +164,17 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
}
@Test
fun `on SwitchAccountClick should emit ShowToast`() = runTest {
fun `on SwitchAccountClick should switch to the given account`() = runTest {
val viewModel = createViewModel()
val accountSummary = mockk<AccountSummary> {
every { status } returns AccountSummary.Status.ACTIVE
}
viewModel.eventFlow.test {
viewModel.trySendAction(VaultUnlockAction.SwitchAccountClick(accountSummary))
assertEquals(VaultUnlockEvent.ShowToast("Not yet implemented.".asText()), awaitItem())
}
val updatedUserId = "updatedUserId"
viewModel.trySendAction(
VaultUnlockAction.SwitchAccountClick(
accountSummary = mockk {
every { userId } returns updatedUserId
},
),
)
verify { authRepository.switchAccount(userId = updatedUserId) }
}
@Test

View file

@ -2,6 +2,7 @@ package com.x8bit.bitwarden.ui.vault.feature.vault
import app.cash.turbine.test
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.auth.repository.model.UserState.SpecialCircumstance
import com.x8bit.bitwarden.data.platform.repository.model.DataState
@ -30,11 +31,14 @@ class VaultViewModelTest : BaseViewModelTest() {
private val mutableVaultDataStateFlow =
MutableStateFlow<DataState<VaultData>>(DataState.Loading)
private var switchAccountResult: SwitchAccountResult = SwitchAccountResult.NoChange
private val authRepository: AuthRepository =
mockk {
every { userStateFlow } returns mutableUserStateFlow
every { specialCircumstance } returns null
every { specialCircumstance = any() } just runs
every { switchAccount(any()) } answers { switchAccountResult }
}
private val vaultRepository: VaultRepository =
@ -52,19 +56,59 @@ class VaultViewModelTest : BaseViewModelTest() {
@Test
fun `UserState updates with a null value should do nothing`() {
val viewModel = createViewModel()
assertEquals(DEFAULT_STATE, viewModel.stateFlow.value)
assertEquals(
DEFAULT_STATE,
viewModel.stateFlow.value,
)
mutableUserStateFlow.value = null
assertEquals(DEFAULT_STATE, viewModel.stateFlow.value)
assertEquals(
DEFAULT_STATE,
viewModel.stateFlow.value,
)
}
@Suppress("MaxLineLength")
@Test
fun `UserState updates with a non-null value update the account information in the state`() {
fun `UserState updates with a non-null value when switching accounts should do nothing`() {
val viewModel = createViewModel()
assertEquals(DEFAULT_STATE, viewModel.stateFlow.value)
mutableUserStateFlow.value = DEFAULT_USER_STATE.copy(
// Ensure we are currently switching accounts
val initialState = DEFAULT_STATE.copy(isSwitchingAccounts = true)
switchAccountResult = SwitchAccountResult.AccountSwitched
val updatedUserId = "lockedUserId"
viewModel.trySendAction(
VaultAction.AccountSwitchClick(
accountSummary = mockk() {
every { userId } returns updatedUserId
},
),
)
assertEquals(
initialState,
viewModel.stateFlow.value,
)
mutableUserStateFlow.value = DEFAULT_USER_STATE.copy(activeUserId = updatedUserId)
assertEquals(
initialState,
viewModel.stateFlow.value,
)
}
@Suppress("MaxLineLength")
@Test
fun `UserState updates with a non-null value when not switching accounts should update the account information in the state`() {
val viewModel = createViewModel()
assertEquals(
DEFAULT_STATE,
viewModel.stateFlow.value,
)
mutableUserStateFlow.value =
DEFAULT_USER_STATE.copy(
accounts = listOf(
UserState.Account(
userId = "activeUserId",
@ -95,49 +139,46 @@ class VaultViewModelTest : BaseViewModelTest() {
)
}
@Suppress("MaxLineLength")
@Test
fun `on AccountSwitchClick for the active account should do nothing`() = runTest {
fun `on AccountSwitchClick when result is NoChange should try to switch to the given account and set isSwitchingAccounts to false`() =
runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
switchAccountResult = SwitchAccountResult.NoChange
val updatedUserId = "lockedUserId"
viewModel.trySendAction(
VaultAction.AccountSwitchClick(
accountSummary = mockk {
every { status } returns AccountSummary.Status.ACTIVE
every { userId } returns updatedUserId
},
),
)
expectNoEvents()
}
verify { authRepository.switchAccount(userId = updatedUserId) }
assertEquals(
DEFAULT_STATE.copy(isSwitchingAccounts = false),
viewModel.stateFlow.value,
)
}
@Suppress("MaxLineLength")
@Test
fun `on AccountSwitchClick for a locked account emit NavigateToVaultUnlockScreen`() = runTest {
fun `on AccountSwitchClick when result is AccountSwitched should switch to the given account and set isSwitchingAccounts to true`() =
runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
switchAccountResult = SwitchAccountResult.AccountSwitched
val updatedUserId = "lockedUserId"
viewModel.trySendAction(
VaultAction.AccountSwitchClick(
accountSummary = mockk {
every { status } returns AccountSummary.Status.LOCKED
every { userId } returns updatedUserId
},
),
)
assertEquals(VaultEvent.NavigateToVaultUnlockScreen, awaitItem())
}
}
@Test
fun `on AccountSwitchClick for an unlocked account emit ShowToast`() = runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(
VaultAction.AccountSwitchClick(
accountSummary = mockk {
every { status } returns AccountSummary.Status.UNLOCKED
},
),
verify { authRepository.switchAccount(userId = updatedUserId) }
assertEquals(
DEFAULT_STATE.copy(isSwitchingAccounts = true),
viewModel.stateFlow.value,
)
assertEquals(VaultEvent.ShowToast("Not yet implemented."), awaitItem())
}
}
@Suppress("MaxLineLength")
@ -257,6 +298,39 @@ class VaultViewModelTest : BaseViewModelTest() {
)
}
@Test
fun `vaultDataStateFlow updates should do nothing when switching accounts`() {
val viewModel = createViewModel()
// Ensure we are currently switching accounts
val initialState = DEFAULT_STATE.copy(isSwitchingAccounts = true)
switchAccountResult = SwitchAccountResult.AccountSwitched
val updatedUserId = "lockedUserId"
viewModel.trySendAction(
VaultAction.AccountSwitchClick(
accountSummary = mockk() {
every { userId } returns updatedUserId
},
),
)
assertEquals(
initialState,
viewModel.stateFlow.value,
)
mutableVaultDataStateFlow.value = DataState.Loaded(
data = VaultData(
cipherViewList = emptyList(),
folderViewList = emptyList(),
),
)
assertEquals(
initialState,
viewModel.stateFlow.value,
)
}
@Test
fun `AddItemClick should emit NavigateToAddItemScreen`() = runTest {
val viewModel = createViewModel()
@ -367,6 +441,14 @@ private val DEFAULT_USER_STATE = UserState(
isPremium = true,
isVaultUnlocked = true,
),
UserState.Account(
userId = "lockedUserId",
name = "Locked User",
email = "locked@bitwarden.com",
avatarColorHex = "#00aaaa",
isPremium = false,
isVaultUnlocked = false,
),
),
)
@ -382,6 +464,14 @@ private fun createMockVaultState(viewState: VaultState.ViewState): VaultState =
avatarColorHex = "#aa00aa",
status = AccountSummary.Status.ACTIVE,
),
AccountSummary(
userId = "lockedUserId",
name = "Locked User",
email = "locked@bitwarden.com",
avatarColorHex = "#00aaaa",
status = AccountSummary.Status.LOCKED,
),
),
viewState = viewState,
isSwitchingAccounts = false,
)