BIT-1110: Allow account addition via the account switcher (#305)

This commit is contained in:
Brian Yencho 2023-11-30 14:03:46 -06:00 committed by Álison Fernandes
parent da6d40790e
commit c377376835
13 changed files with 196 additions and 37 deletions

View file

@ -36,6 +36,15 @@ interface AuthRepository : AuthenticatorProvider {
*/
var rememberedEmailAddress: String?
/**
* Any special account circumstances that may be relevant (ex: pending multi-user account
* additions).
*
* This allows a direct view into and modification of [UserState.specialCircumstance].
* Note that this call has no effect when there is no [UserState] information available.
*/
var specialCircumstance: UserState.SpecialCircumstance?
/**
* Attempt to delete the current account and logout them out upon success.
*/

View file

@ -34,6 +34,7 @@ import com.x8bit.bitwarden.data.vault.repository.VaultRepository
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asSharedFlow
@ -57,6 +58,8 @@ class AuthRepositoryImpl constructor(
private val vaultRepository: VaultRepository,
dispatcherManager: DispatcherManager,
) : AuthRepository {
private val mutableSpecialCircumstanceStateFlow =
MutableStateFlow<UserState.SpecialCircumstance?>(null)
private val scope = CoroutineScope(dispatcherManager.io)
override val activeUserId: String? get() = authDiskSource.userState?.activeUserId
@ -84,8 +87,13 @@ class AuthRepositoryImpl constructor(
override val userStateFlow: StateFlow<UserState?> = combine(
authDiskSource.userStateFlow,
vaultRepository.vaultStateFlow,
) { userStateJson, vaultState ->
userStateJson?.toUserState(vaultState = vaultState)
mutableSpecialCircumstanceStateFlow,
) { userStateJson, vaultState, specialCircumstance ->
userStateJson
?.toUserState(
vaultState = vaultState,
specialCircumstance = specialCircumstance,
)
}
.stateIn(
scope = scope,
@ -94,6 +102,7 @@ class AuthRepositoryImpl constructor(
.userState
?.toUserState(
vaultState = vaultRepository.vaultStateFlow.value,
specialCircumstance = mutableSpecialCircumstanceStateFlow.value,
),
)
@ -104,6 +113,9 @@ class AuthRepositoryImpl constructor(
override var rememberedEmailAddress: String? by authDiskSource::rememberedEmailAddress
override var specialCircumstance: UserState.SpecialCircumstance?
by mutableSpecialCircumstanceStateFlow::value
override suspend fun deleteAccount(password: String): DeleteAccountResult {
val profile = authDiskSource.userState?.activeAccount?.profile
?: return DeleteAccountResult.Error
@ -121,6 +133,7 @@ class AuthRepositoryImpl constructor(
)
}
@Suppress("LongMethod")
override suspend fun login(
email: String,
password: String,
@ -148,12 +161,16 @@ class AuthRepositoryImpl constructor(
when (loginResponse) {
is CaptchaRequired -> LoginResult.CaptchaRequired(loginResponse.captchaKey)
is Success -> {
activeUserId?.let { previousActiveUserId ->
vaultRepository.lockVaultIfNecessary(userId = previousActiveUserId)
}
val userStateJson = loginResponse.toUserState(
previousUserState = authDiskSource.userState,
environmentUrlData = environmentRepository
.environment
.environmentUrlData,
)
vaultRepository.clearUnlockedData()
vaultRepository.unlockVault(
userId = userStateJson.activeUserId,
email = userStateJson.activeAccount.profile.email,
@ -174,6 +191,7 @@ class AuthRepositoryImpl constructor(
privateKey = loginResponse.privateKey,
)
vaultRepository.sync()
specialCircumstance = null
LoginResult.Success
}

View file

@ -9,10 +9,12 @@ import com.x8bit.bitwarden.data.auth.repository.model.UserState.Account
* @property activeUserId The ID of the current active user.
* @property accounts A mapping between user IDs and the [Account] information associated with
* that user.
* @property specialCircumstance A special circumstance (if any) that may be present.
*/
data class UserState(
val activeUserId: String,
val accounts: List<Account>,
val specialCircumstance: SpecialCircumstance? = null,
) {
init {
require(accounts.any { it.userId == activeUserId })
@ -24,6 +26,12 @@ data class UserState(
val activeAccount: Account
get() = accounts.first { it.userId == activeUserId }
/**
* Returns `true` if a new user is in the process of being added, `false` otherwise.
*/
val hasPendingAccountAddition: Boolean
get() = specialCircumstance == SpecialCircumstance.PendingAccountAddition
/**
* Basic account information about a given user.
*
@ -42,4 +50,16 @@ data class UserState(
val isPremium: Boolean,
val isVaultUnlocked: Boolean,
)
/**
* Represents a special account-related circumstance.
*/
sealed class SpecialCircumstance {
/**
* There is an additional account that is pending login/registration in order to have
* multiple accounts available.
*/
data object PendingAccountAddition : SpecialCircumstance()
}
}

View file

@ -36,10 +36,12 @@ fun UserStateJson.toUpdatedUserStateJson(
}
/**
* Converts the given [UserStateJson] to a [UserState] using the given [vaultState].
* Converts the given [UserStateJson] to a [UserState] using the given [vaultState] and
* [specialCircumstance].
*/
fun UserStateJson.toUserState(
vaultState: VaultState,
specialCircumstance: UserState.SpecialCircumstance?,
): UserState =
UserState(
activeUserId = this.activeUserId,
@ -58,4 +60,5 @@ fun UserStateJson.toUserState(
isVaultUnlocked = userId in vaultState.unlockedVaultUserIds,
)
},
specialCircumstance = specialCircumstance,
)

View file

@ -65,11 +65,6 @@ fun VaultUnlockScreen(
is VaultUnlockEvent.ShowToast -> {
Toast.makeText(context, event.text(resources), Toast.LENGTH_SHORT).show()
}
VaultUnlockEvent.NavigateToLoginScreen -> {
// TODO: Handle adding accounts (BIT-853)
Toast.makeText(context, "Not yet implemented.", Toast.LENGTH_SHORT).show()
}
}
}

View file

@ -93,7 +93,7 @@ class VaultUnlockViewModel @Inject constructor(
}
private fun handleAddAccountClick() {
sendEvent(VaultUnlockEvent.NavigateToLoginScreen)
authRepository.specialCircumstance = UserState.SpecialCircumstance.PendingAccountAddition
}
private fun handleDismissDialog() {
@ -220,11 +220,6 @@ data class VaultUnlockState(
* Models events for the vault unlock screen.
*/
sealed class VaultUnlockEvent {
/**
* Navigates to the login flow.
*/
data object NavigateToLoginScreen : VaultUnlockEvent()
/**
* Displays a toast to the user.
*/

View file

@ -41,7 +41,7 @@ class RootNavViewModel @Inject constructor(
) {
val userState = action.userState
val updatedRootNavState = when {
userState == null -> RootNavState.Auth
userState == null || userState.hasPendingAccountAddition -> RootNavState.Auth
userState.activeAccount.isVaultUnlocked -> RootNavState.VaultUnlocked
else -> RootNavState.VaultLocked
}

View file

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

View file

@ -35,7 +35,7 @@ import javax.inject.Inject
@Suppress("TooManyFunctions")
@HiltViewModel
class VaultViewModel @Inject constructor(
authRepository: AuthRepository,
private val authRepository: AuthRepository,
vaultRepository: VaultRepository,
) : BaseViewModel<VaultState, VaultEvent, VaultAction>(
initialState = run {
@ -139,7 +139,7 @@ class VaultViewModel @Inject constructor(
}
private fun handleAddAccountClick() {
sendEvent(VaultEvent.NavigateToLoginScreen)
authRepository.specialCircumstance = UserState.SpecialCircumstance.PendingAccountAddition
}
private fun handleTrashClick() {
@ -445,11 +445,6 @@ sealed class VaultEvent {
*/
data object NavigateToSecureNotesGroup : VaultEvent()
/**
* Navigate to the login flow for an additional account.
*/
data object NavigateToLoginScreen : VaultEvent()
/**
* Navigate to the vault unlock screen.
*/

View file

@ -29,6 +29,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.UserState
import com.x8bit.bitwarden.data.auth.repository.util.CaptchaCallbackTokenResult
import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams
import com.x8bit.bitwarden.data.auth.repository.util.toUserState
@ -148,6 +149,7 @@ class AuthRepositoryTest {
assertEquals(
SINGLE_USER_STATE_1.toUserState(
vaultState = VAULT_STATE,
specialCircumstance = null,
),
repository.userStateFlow.value,
)
@ -156,6 +158,7 @@ class AuthRepositoryTest {
assertEquals(
MULTI_USER_STATE.toUserState(
vaultState = VAULT_STATE,
specialCircumstance = null,
),
repository.userStateFlow.value,
)
@ -165,6 +168,7 @@ class AuthRepositoryTest {
assertEquals(
MULTI_USER_STATE.toUserState(
vaultState = emptyVaultState,
specialCircumstance = null,
),
repository.userStateFlow.value,
)
@ -185,6 +189,31 @@ class AuthRepositoryTest {
assertNull(repository.rememberedEmailAddress)
}
@Test
fun `specialCircumstance update should trigger a change in UserState`() {
// Populate the initial UserState
assertNull(repository.specialCircumstance)
val initialUserState = SINGLE_USER_STATE_1.toUserState(
vaultState = VAULT_STATE,
specialCircumstance = null,
)
mutableVaultStateFlow.value = VAULT_STATE
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
assertEquals(
initialUserState,
repository.userStateFlow.value,
)
repository.specialCircumstance = UserState.SpecialCircumstance.PendingAccountAddition
assertEquals(
initialUserState.copy(
specialCircumstance = UserState.SpecialCircumstance.PendingAccountAddition,
),
repository.userStateFlow.value,
)
}
@Test
fun `delete account fails if not logged in`() = runTest {
val masterPassword = "hello world"
@ -439,6 +468,94 @@ class AuthRepositoryTest {
)
vaultRepository.sync()
}
assertEquals(
SINGLE_USER_STATE_1,
fakeAuthDiskSource.userState,
)
assertNull(repository.specialCircumstance)
verify(exactly = 0) { vaultRepository.lockVaultIfNecessary(any()) }
verify { vaultRepository.clearUnlockedData() }
}
@Suppress("MaxLineLength")
@Test
fun `login get token succeeds when there is an existing user should switch to the new logged in user and lock the old user's vault`() =
runTest {
// Ensure the initial state for User 2 with a account addition
fakeAuthDiskSource.userState = SINGLE_USER_STATE_2
repository.specialCircumstance = UserState.SpecialCircumstance.PendingAccountAddition
// Set up login for User 1
val successResponse = GET_TOKEN_RESPONSE_SUCCESS
coEvery {
accountsService.preLogin(email = EMAIL)
} returns Result.success(PRE_LOGIN_SUCCESS)
coEvery {
identityService.getToken(
email = EMAIL,
passwordHash = PASSWORD_HASH,
captchaToken = null,
uniqueAppId = UNIQUE_APP_ID,
)
}
.returns(Result.success(successResponse))
coEvery {
vaultRepository.unlockVault(
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key,
privateKey = successResponse.privateKey,
organizationalKeys = emptyMap(),
masterPassword = PASSWORD,
)
} returns VaultUnlockResult.Success
coEvery { vaultRepository.sync() } just runs
every {
GET_TOKEN_RESPONSE_SUCCESS.toUserState(
previousUserState = SINGLE_USER_STATE_2,
environmentUrlData = EnvironmentUrlDataJson.DEFAULT_US,
)
} returns MULTI_USER_STATE
val result = repository.login(email = EMAIL, password = PASSWORD, captchaToken = null)
assertEquals(LoginResult.Success, result)
assertEquals(AuthState.Authenticated(ACCESS_TOKEN), repository.authStateFlow.value)
coVerify { accountsService.preLogin(email = EMAIL) }
fakeAuthDiskSource.assertPrivateKey(
userId = USER_ID_1,
privateKey = "privateKey",
)
fakeAuthDiskSource.assertUserKey(
userId = USER_ID_1,
userKey = "key",
)
coVerify {
identityService.getToken(
email = EMAIL,
passwordHash = PASSWORD_HASH,
captchaToken = null,
uniqueAppId = UNIQUE_APP_ID,
)
vaultRepository.unlockVault(
userId = USER_ID_1,
email = EMAIL,
kdf = ACCOUNT_1.profile.toSdkParams(),
userKey = successResponse.key,
privateKey = successResponse.privateKey,
organizationalKeys = emptyMap(),
masterPassword = PASSWORD,
)
vaultRepository.sync()
}
assertEquals(
MULTI_USER_STATE,
fakeAuthDiskSource.userState,
)
assertNull(repository.specialCircumstance)
verify { vaultRepository.lockVaultIfNecessary(userId = USER_ID_2) }
verify { vaultRepository.clearUnlockedData() }
}
@Test

View file

@ -121,6 +121,7 @@ class UserStateJsonExtensionsTest {
vaultState = VaultState(
unlockedVaultUserIds = setOf("activeUserId"),
),
specialCircumstance = null,
),
)
}
@ -141,6 +142,7 @@ class UserStateJsonExtensionsTest {
isVaultUnlocked = false,
),
),
specialCircumstance = UserState.SpecialCircumstance.PendingAccountAddition,
),
UserStateJson(
activeUserId = "activeUserId",
@ -162,6 +164,7 @@ class UserStateJsonExtensionsTest {
vaultState = VaultState(
unlockedVaultUserIds = emptySet(),
),
specialCircumstance = UserState.SpecialCircumstance.PendingAccountAddition,
),
)
}

View file

@ -6,6 +6,7 @@ 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.UserState
import com.x8bit.bitwarden.data.auth.repository.model.UserState.SpecialCircumstance
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
import com.x8bit.bitwarden.data.platform.repository.model.Environment
import com.x8bit.bitwarden.data.platform.repository.util.FakeEnvironmentRepository
@ -32,6 +33,8 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
private val environmentRepository = FakeEnvironmentRepository()
private val authRepository = mockk<AuthRepository>() {
every { userStateFlow } returns mutableUserStateFlow
every { specialCircumstance } returns null
every { specialCircumstance = any() } just runs
every { logout() } just runs
}
private val vaultRepository = mockk<VaultRepository>()
@ -120,12 +123,13 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
)
}
@Suppress("MaxLineLength")
@Test
fun `on AddAccountClick should emit NavigateToLoginScreen`() = runTest {
fun `on AddAccountClick should update the SpecialCircumstance of the AuthRepository to PendingAccountAddition`() {
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(VaultUnlockAction.AddAccountClick)
assertEquals(VaultUnlockEvent.NavigateToLoginScreen, awaitItem())
verify {
authRepository.specialCircumstance = SpecialCircumstance.PendingAccountAddition
}
}

View file

@ -3,6 +3,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.UserState
import com.x8bit.bitwarden.data.auth.repository.model.UserState.SpecialCircumstance
import com.x8bit.bitwarden.data.platform.repository.model.DataState
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCipherView
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockFolderView
@ -12,7 +13,10 @@ import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest
import com.x8bit.bitwarden.ui.platform.base.util.asText
import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.runs
import io.mockk.verify
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.Assertions.assertEquals
@ -29,6 +33,8 @@ class VaultViewModelTest : BaseViewModelTest() {
private val authRepository: AuthRepository =
mockk {
every { userStateFlow } returns mutableUserStateFlow
every { specialCircumstance } returns null
every { specialCircumstance = any() } just runs
}
private val vaultRepository: VaultRepository =
@ -134,12 +140,13 @@ class VaultViewModelTest : BaseViewModelTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `on AddAccountClick should emit NavigateToLoginScreen`() = runTest {
fun `on AddAccountClick should update the SpecialCircumstance of the AuthRepository to PendingAccountAddition`() {
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(VaultAction.AddAccountClick)
assertEquals(VaultEvent.NavigateToLoginScreen, awaitItem())
verify {
authRepository.specialCircumstance = SpecialCircumstance.PendingAccountAddition
}
}