BIT-778: Account recovery (#905)

This commit is contained in:
Shannon Draeker 2024-01-31 12:37:47 -07:00 committed by Álison Fernandes
parent 10471a7ea6
commit 608779ba68
14 changed files with 308 additions and 95 deletions

View file

@ -15,6 +15,12 @@ interface AuthenticatedAccountsApi {
@HTTP(method = "DELETE", path = "/accounts", hasBody = true)
suspend fun deleteAccount(@Body body: DeleteAccountRequestJson): Result<Unit>
/**
* Resets the temporary password.
*/
@HTTP(method = "PUT", path = "/accounts/update-temp-password", hasBody = true)
suspend fun resetTempPassword(@Body body: ResetPasswordRequestJson): Result<Unit>
/**
* Resets the password.
*/

View file

@ -14,7 +14,7 @@ import kotlinx.serialization.Serializable
@Serializable
data class ResetPasswordRequestJson(
@SerialName("masterPasswordHash")
val currentPasswordHash: String,
val currentPasswordHash: String?,
@SerialName("newMasterPasswordHash")
val newPasswordHash: String,

View file

@ -64,6 +64,11 @@ class AccountsServiceImpl constructor(
override suspend fun resendVerificationCodeEmail(body: ResendEmailRequestJson): Result<Unit> =
accountsApi.resendVerificationCodeEmail(body = body)
override suspend fun resetPassword(body: ResetPasswordRequestJson): Result<Unit> =
authenticatedAccountsApi.resetPassword(body = body)
override suspend fun resetPassword(body: ResetPasswordRequestJson): Result<Unit> {
return if (body.currentPasswordHash == null) {
authenticatedAccountsApi.resetTempPassword(body = body)
} else {
authenticatedAccountsApi.resetPassword(body = body)
}
}
}

View file

@ -1,5 +1,6 @@
package com.x8bit.bitwarden.data.auth.repository
import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason
import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.TwoFactorDataModel
import com.x8bit.bitwarden.data.auth.repository.model.AuthRequest
@ -90,6 +91,11 @@ interface AuthRepository : AuthenticatorProvider {
*/
val hasExportVaultPoliciesEnabled: Boolean
/**
* The reason for resetting the password.
*/
val passwordResetReason: ForcePasswordResetReason?
/**
* Clears the pending deletion state that occurs when the an account is successfully deleted.
*/
@ -172,11 +178,11 @@ interface AuthRepository : AuthenticatorProvider {
): PasswordHintResult
/**
* Resets the users password from the [currentPassword] to the [newPassword] and
* optional [passwordHint].
* Resets the users password from the [currentPassword] (or null for account recovery resets),
* to the [newPassword] and optional [passwordHint].
*/
suspend fun resetPassword(
currentPassword: String,
currentPassword: String?,
newPassword: String,
passwordHint: String?,
): ResetPasswordResult

View file

@ -248,6 +248,13 @@ class AuthRepositoryImpl(
?: false
} ?: false
override val passwordResetReason: ForcePasswordResetReason?
get() = authDiskSource
.userState
?.activeAccount
?.profile
?.forcePasswordResetReason
init {
pushManager
.syncOrgKeysFlow
@ -267,6 +274,13 @@ class AuthRepositoryImpl(
authDiskSource.currentUserPoliciesListFlow
.onEach { policies ->
val userId = activeUserId ?: return@onEach
// If the password already has to be reset for some other reason, there's no
// need to check the password policies.
if (passwordResetReason != null) return@onEach
// Otherwise check the user's password against the policies and set or
// clear the force reset reason accordingly.
storeUserResetPasswordReason(
userId = userId,
reason = ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN
@ -659,7 +673,7 @@ class AuthRepositoryImpl(
@Suppress("ReturnCount")
override suspend fun resetPassword(
currentPassword: String,
currentPassword: String?,
newPassword: String,
passwordHint: String?,
): ResetPasswordResult {
@ -667,17 +681,19 @@ class AuthRepositoryImpl(
.userState
?.activeAccount
?: return ResetPasswordResult.Error
val currentPasswordHash = authSdkSource
.hashPassword(
email = activeAccount.profile.email,
password = currentPassword,
kdf = activeAccount.profile.toSdkParams(),
purpose = HashPurpose.SERVER_AUTHORIZATION,
)
.fold(
onFailure = { return ResetPasswordResult.Error },
onSuccess = { it },
)
val currentPasswordHash = currentPassword?.let {
authSdkSource
.hashPassword(
email = activeAccount.profile.email,
password = it,
kdf = activeAccount.profile.toSdkParams(),
purpose = HashPurpose.SERVER_AUTHORIZATION,
)
.fold(
onFailure = { return ResetPasswordResult.Error },
onSuccess = { it },
)
}
return vaultSdkSource
.updatePassword(
userId = activeAccount.profile.userId,

View file

@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.auth.repository.util
import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountJson
import com.x8bit.bitwarden.data.auth.datasource.disk.model.EnvironmentUrlDataJson
import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason
import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson
@ -33,7 +34,11 @@ fun GetTokenResponseJson.Success.toUserState(
organizationId = null,
avatarColorHex = null,
hasPremium = jwtTokenData.hasPremium,
forcePasswordResetReason = null,
forcePasswordResetReason = if (this.shouldForcePasswordReset) {
ForcePasswordResetReason.ADMIN_FORCE_PASSWORD_RESET
} else {
null
},
kdfType = this.kdfType,
kdfIterations = this.kdfIterations,
kdfMemory = this.kdfMemory,

View file

@ -58,6 +58,9 @@ fun UserStateJson.toUserState(
.values
.map { accountJson ->
val userId = accountJson.profile.userId
val vaultUnlocked = vaultState.statusFor(userId) == VaultUnlockData.Status.UNLOCKED
val needsPasswordReset = accountJson.profile.forcePasswordResetReason != null
UserState.Account(
userId = accountJson.profile.userId,
name = accountJson.profile.name,
@ -70,9 +73,8 @@ fun UserStateJson.toUserState(
.toEnvironmentUrlsOrDefault(),
isPremium = accountJson.profile.hasPremium == true,
isLoggedIn = accountJson.isLoggedIn,
isVaultUnlocked = vaultState.statusFor(userId) ==
VaultUnlockData.Status.UNLOCKED,
needsPasswordReset = accountJson.profile.forcePasswordResetReason != null,
isVaultUnlocked = vaultUnlocked && !needsPasswordReset,
needsPasswordReset = needsPasswordReset,
organizations = userOrganizationsList
.find { it.userId == userId }
?.organizations

View file

@ -34,6 +34,7 @@ import androidx.compose.ui.unit.dp
import androidx.hilt.navigation.compose.hiltViewModel
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason
import com.x8bit.bitwarden.ui.platform.base.util.asText
import com.x8bit.bitwarden.ui.platform.components.BasicDialogState
import com.x8bit.bitwarden.ui.platform.components.BitwardenBasicDialog
@ -164,8 +165,14 @@ private fun ResetPasswordScreeContent(
.verticalScroll(rememberScrollState()),
) {
val instructionsTextId =
if (state.resetReason == ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN) {
R.string.update_weak_master_password_warning
} else {
R.string.update_master_password_warning
}
Text(
text = stringResource(id = R.string.update_weak_master_password_warning),
text = stringResource(id = instructionsTextId),
textAlign = TextAlign.Start,
style = MaterialTheme.typography.bodyMedium,
color = MaterialTheme.colorScheme.onSurfaceVariant,
@ -182,28 +189,29 @@ private fun ResetPasswordScreeContent(
Spacer(modifier = Modifier.height(16.dp))
val passwordPolicyContent = listOf(
stringResource(id = R.string.master_password_policy_in_effect),
)
.plus(state.policies.map { it() })
.joinToString("\n")
Text(
text = passwordPolicyContent,
textAlign = TextAlign.Start,
style = MaterialTheme.typography.bodyMedium,
color = MaterialTheme.colorScheme.onSurfaceVariant,
modifier = Modifier
.padding(horizontal = 16.dp)
.border(
width = 1.dp,
color = MaterialTheme.colorScheme.primary,
shape = RoundedCornerShape(4.dp),
)
.padding(16.dp)
.fillMaxWidth(),
)
if (state.resetReason == ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN) {
val passwordPolicyContent = listOf(
stringResource(id = R.string.master_password_policy_in_effect),
)
.plus(state.policies.map { it() })
.joinToString("\n")
Text(
text = passwordPolicyContent,
textAlign = TextAlign.Start,
style = MaterialTheme.typography.bodyMedium,
color = MaterialTheme.colorScheme.onSurfaceVariant,
modifier = Modifier
.padding(horizontal = 16.dp)
.border(
width = 1.dp,
color = MaterialTheme.colorScheme.primary,
shape = RoundedCornerShape(4.dp),
)
.padding(16.dp)
.fillMaxWidth(),
)
Spacer(modifier = Modifier.height(16.dp))
Spacer(modifier = Modifier.height(16.dp))
BitwardenPasswordField(
label = stringResource(id = R.string.current_master_password),
@ -215,7 +223,8 @@ private fun ResetPasswordScreeContent(
.fillMaxWidth(),
)
Spacer(modifier = Modifier.height(16.dp))
Spacer(modifier = Modifier.height(16.dp))
}
BitwardenPasswordField(
label = stringResource(id = R.string.master_password),

View file

@ -4,6 +4,7 @@ import android.os.Parcelable
import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.viewModelScope
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.ResetPasswordResult
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
@ -11,6 +12,7 @@ import com.x8bit.bitwarden.ui.auth.feature.resetpassword.util.toDisplayLabels
import com.x8bit.bitwarden.ui.platform.base.BaseViewModel
import com.x8bit.bitwarden.ui.platform.base.util.Text
import com.x8bit.bitwarden.ui.platform.base.util.asText
import com.x8bit.bitwarden.ui.platform.base.util.orNullIfBlank
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
@ -20,6 +22,7 @@ import kotlinx.parcelize.Parcelize
import javax.inject.Inject
private const val KEY_STATE = "state"
private const val MIN_PASSWORD_LENGTH = 12
/**
* Manages application state for the Reset Password screen.
@ -33,6 +36,7 @@ class ResetPasswordViewModel @Inject constructor(
initialState = savedStateHandle[KEY_STATE]
?: ResetPasswordState(
policies = authRepository.passwordPolicies.toDisplayLabels(),
resetReason = authRepository.passwordResetReason,
dialogState = null,
currentPasswordInput = "",
passwordInput = "",
@ -93,8 +97,7 @@ class ResetPasswordViewModel @Inject constructor(
*/
private fun handleSubmitClicked() {
// Display an error dialog if the new password field is blank.
val password = state.passwordInput
if (password.isBlank()) {
if (state.passwordInput.isBlank()) {
mutableStateFlow.update {
it.copy(
dialogState = ResetPasswordState.DialogState.Error(
@ -107,12 +110,35 @@ class ResetPasswordViewModel @Inject constructor(
return
}
// Check if the new password meets the policy requirements.
viewModelScope.launch {
val result = authRepository.validatePasswordAgainstPolicies(password)
sendAction(
ResetPasswordAction.Internal.ReceiveValidatePasswordAgainstPoliciesResult(result),
)
// Check if the new password meets the policy requirements, if applicable.
if (state.resetReason == ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN) {
viewModelScope.launch {
val result = authRepository.validatePasswordAgainstPolicies(state.passwordInput)
sendAction(
ResetPasswordAction.Internal.ReceiveValidatePasswordAgainstPoliciesResult(
result,
),
)
}
} else {
// Otherwise, simply verify that the password meets the minimum length requirement.
if (state.passwordInput.length < MIN_PASSWORD_LENGTH) {
mutableStateFlow.update {
it.copy(
dialogState = ResetPasswordState.DialogState.Error(
title = null,
message = R.string.master_password_length_val_message_x
.asText(MIN_PASSWORD_LENGTH),
),
)
}
} else {
// Check that the re-typed password matches.
if (!checkRetypedPassword()) return
// Otherwise, if the password checks out, attempt to reset it.
resetPassword()
}
}
}
@ -236,24 +262,7 @@ class ResetPasswordViewModel @Inject constructor(
)
}
} else {
// Show the loading dialog.
mutableStateFlow.update {
it.copy(
dialogState = ResetPasswordState.DialogState.Loading(
message = R.string.updating_password.asText(),
),
)
}
viewModelScope.launch {
val result = authRepository.resetPassword(
currentPassword = state.currentPasswordInput,
newPassword = state.passwordInput,
passwordHint = state.passwordHintInput,
)
trySendAction(
ResetPasswordAction.Internal.ReceiveResetPasswordResult(result),
)
}
resetPassword()
}
}
}
@ -279,18 +288,8 @@ class ResetPasswordViewModel @Inject constructor(
return
}
// Display an error alert if the re-typed password doesn't match the new password.
if (state.passwordInput != state.retypePasswordInput) {
mutableStateFlow.update {
it.copy(
dialogState = ResetPasswordState.DialogState.Error(
title = null,
message = R.string.master_password_confirmation_val_message.asText(),
),
)
}
return
}
// Check that the re-typed password matches.
if (!checkRetypedPassword()) return
// Check that the entered current password is correct.
viewModelScope.launch {
@ -299,6 +298,48 @@ class ResetPasswordViewModel @Inject constructor(
trySendAction(ResetPasswordAction.Internal.ReceiveValidatePasswordResult(result))
}
}
/**
* A helper function to determine if the re-typed password matches and
* display an alert if not. Returns true if the passwords match.
*/
private fun checkRetypedPassword(): Boolean {
if (state.passwordInput == state.retypePasswordInput) return true
mutableStateFlow.update {
it.copy(
dialogState = ResetPasswordState.DialogState.Error(
title = null,
message = R.string.master_password_confirmation_val_message.asText(),
),
)
}
return false
}
/**
* A helper function to launch the reset password request.
*/
private fun resetPassword() {
// Show the loading dialog.
mutableStateFlow.update {
it.copy(
dialogState = ResetPasswordState.DialogState.Loading(
message = R.string.updating_password.asText(),
),
)
}
viewModelScope.launch {
val result = authRepository.resetPassword(
currentPassword = state.currentPasswordInput.orNullIfBlank(),
newPassword = state.passwordInput,
passwordHint = state.passwordHintInput,
)
trySendAction(
ResetPasswordAction.Internal.ReceiveResetPasswordResult(result),
)
}
}
}
/**
@ -307,6 +348,7 @@ class ResetPasswordViewModel @Inject constructor(
@Parcelize
data class ResetPasswordState(
val policies: List<Text>,
val resetReason: ForcePasswordResetReason?,
val dialogState: DialogState?,
val currentPasswordInput: String,
val passwordInput: String,

View file

@ -243,6 +243,21 @@ class AccountsServiceTest : BaseServiceTest() {
assertTrue(result.isSuccess)
}
@Test
fun `resetPassword with empty response and null current password is success`() = runTest {
val response = MockResponse().setBody("")
server.enqueue(response)
val result = service.resetPassword(
body = ResetPasswordRequestJson(
currentPasswordHash = null,
newPasswordHash = "",
passwordHint = null,
key = "",
),
)
assertTrue(result.isSuccess)
}
companion object {
private const val EMAIL = "email"
private val registerRequestBody = RegisterRequestJson(

View file

@ -540,6 +540,25 @@ class AuthRepositoryTest {
assertTrue(repository.hasExportVaultPoliciesEnabled)
}
@Test
fun `passwordResetReason should pull from the user's profile in AuthDiskSource`() = runTest {
val updatedProfile = ACCOUNT_1.profile.copy(
forcePasswordResetReason = ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN,
)
fakeAuthDiskSource.userState = UserStateJson(
activeUserId = USER_ID_1,
accounts = mapOf(
USER_ID_1 to ACCOUNT_1.copy(
profile = updatedProfile,
),
),
)
assertEquals(
ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN,
repository.passwordResetReason,
)
}
@Test
fun `clear Pending Account Deletion should unblock userState updates`() = runTest {
val masterPassword = "hello world"

View file

@ -79,7 +79,7 @@ private val GET_TOKEN_RESPONSE_SUCCESS = GetTokenResponseJson.Success(
kdfMemory = 16,
kdfParallelism = 4,
privateKey = "privateKey",
shouldForcePasswordReset = true,
shouldForcePasswordReset = false,
shouldResetMasterPassword = true,
twoFactorToken = null,
masterPasswordPolicyOptions = null,

View file

@ -8,6 +8,7 @@ import androidx.compose.ui.test.isDisplayed
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performTextInput
import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason
import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow
import com.x8bit.bitwarden.ui.auth.feature.resetpassword.ResetPasswordAction
import com.x8bit.bitwarden.ui.auth.feature.resetpassword.ResetPasswordEvent
@ -109,10 +110,37 @@ class ResetPasswordScreenTest : BaseComposeTest() {
}
@Test
@Suppress("MaxLineLength")
fun `password instructions should update according to state`() {
val baseString =
"One or more organization policies require your master password to meet the following requirements:"
fun `instructions text should update according to state`() {
val weakPasswordString = "Your master password does not meet one or more " +
"of your organization policies. In order to access the vault, you must " +
"update your master password now. Proceeding will log you out of your " +
"current session, requiring you to log back in. Active sessions on other " +
"devices may continue to remain active for up to one hour."
composeTestRule
.onNodeWithText(weakPasswordString)
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(
resetReason = ForcePasswordResetReason.ADMIN_FORCE_PASSWORD_RESET,
)
}
val adminChangeString =
"Your master password was recently changed by an administrator in " +
"your organization. In order to access the vault, you must update your master " +
"password now. Proceeding will log you out of your current session, " +
"requiring you to log back in. Active sessions on other devices may continue " +
"to remain active for up to one hour."
composeTestRule
.onNodeWithText(adminChangeString)
.assertIsDisplayed()
}
@Test
fun `detailed instructions should update according to state`() {
val baseString = "One or more organization policies require your master password to " +
"meet the following requirements:"
composeTestRule
.onNodeWithText(baseString)
.assertIsDisplayed()
@ -131,6 +159,16 @@ class ResetPasswordScreenTest : BaseComposeTest() {
composeTestRule
.onNodeWithText(updatedString)
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(
resetReason = ForcePasswordResetReason.ADMIN_FORCE_PASSWORD_RESET,
)
}
composeTestRule
.onNodeWithText(baseString)
.assertDoesNotExist()
}
@Test
@ -142,6 +180,23 @@ class ResetPasswordScreenTest : BaseComposeTest() {
}
}
@Test
fun `current password field should update according to state`() {
composeTestRule
.onNodeWithText("Current master password")
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(
resetReason = ForcePasswordResetReason.ADMIN_FORCE_PASSWORD_RESET,
)
}
composeTestRule
.onNodeWithText("Current master password")
.assertDoesNotExist()
}
@Test
fun `password input change should send PasswordInputChange action`() {
val input = "Test123"
@ -172,6 +227,7 @@ class ResetPasswordScreenTest : BaseComposeTest() {
private val DEFAULT_STATE = ResetPasswordState(
policies = emptyList(),
resetReason = ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN,
dialogState = null,
currentPasswordInput = "",
passwordInput = "",

View file

@ -3,6 +3,7 @@ package com.x8bit.bitwarden.ui.auth.feature.resetPassword
import androidx.lifecycle.SavedStateHandle
import app.cash.turbine.test
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.ResetPasswordResult
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
@ -23,8 +24,9 @@ import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
class ResetPasswordViewModelTest : BaseViewModelTest() {
private val authRepository: AuthRepository = mockk() {
private val authRepository: AuthRepository = mockk {
every { passwordPolicies } returns emptyList()
every { passwordResetReason } returns ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN
}
private val savedStateHandle = SavedStateHandle()
@ -82,11 +84,37 @@ class ResetPasswordViewModelTest : BaseViewModelTest() {
}
@Test
fun `SubmitClicked with invalid password shows error alert`() = runTest {
fun `SubmitClicked with invalid password shows error alert for weak password reason`() =
runTest {
val password = "Test123"
coEvery {
authRepository.validatePasswordAgainstPolicies(password)
} returns false
val viewModel = createViewModel()
viewModel.trySendAction(ResetPasswordAction.PasswordInputChanged(password))
viewModel.eventFlow.test {
viewModel.trySendAction(ResetPasswordAction.SubmitClick)
assertEquals(
DEFAULT_STATE.copy(
dialogState = ResetPasswordState.DialogState.Error(
title = R.string.master_password_policy_validation_title.asText(),
message = R.string.master_password_policy_validation_message.asText(),
),
passwordInput = password,
),
viewModel.stateFlow.value,
)
}
}
@Test
fun `SubmitClicked with invalid password shows error alert for admin reset reason`() = runTest {
val password = "Test123"
coEvery {
authRepository.validatePasswordAgainstPolicies(password)
} returns false
every {
authRepository.passwordResetReason
} returns ForcePasswordResetReason.ADMIN_FORCE_PASSWORD_RESET
val viewModel = createViewModel()
viewModel.trySendAction(ResetPasswordAction.PasswordInputChanged(password))
@ -95,9 +123,11 @@ class ResetPasswordViewModelTest : BaseViewModelTest() {
assertEquals(
DEFAULT_STATE.copy(
resetReason = ForcePasswordResetReason.ADMIN_FORCE_PASSWORD_RESET,
dialogState = ResetPasswordState.DialogState.Error(
title = R.string.master_password_policy_validation_title.asText(),
message = R.string.master_password_policy_validation_message.asText(),
title = null,
message = R.string.master_password_length_val_message_x
.asText(MIN_PASSWORD_LENGTH),
),
passwordInput = password,
),
@ -311,8 +341,10 @@ class ResetPasswordViewModelTest : BaseViewModelTest() {
)
}
private const val MIN_PASSWORD_LENGTH = 12
private val DEFAULT_STATE = ResetPasswordState(
policies = emptyList(),
resetReason = ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN,
dialogState = null,
currentPasswordInput = "",
passwordInput = "",