mirror of
https://github.com/bitwarden/android.git
synced 2025-02-17 04:19:54 +03:00
Move validatePassword to AuthRepository and ensure errors are caught (#841)
This commit is contained in:
parent
474025b893
commit
2623fc3cbe
12 changed files with 227 additions and 82 deletions
|
@ -18,6 +18,7 @@ import com.x8bit.bitwarden.data.auth.repository.model.RegisterResult
|
|||
import com.x8bit.bitwarden.data.auth.repository.model.ResendEmailResult
|
||||
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.ValidatePasswordResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.CaptchaCallbackTokenResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.SsoCallbackResult
|
||||
import com.x8bit.bitwarden.data.platform.datasource.network.authenticator.AuthenticatorProvider
|
||||
|
@ -221,4 +222,9 @@ interface AuthRepository : AuthenticatorProvider {
|
|||
* Get the password strength for the given [email] and [password] combo.
|
||||
*/
|
||||
suspend fun getPasswordStrength(email: String, password: String): PasswordStrengthResult
|
||||
|
||||
/**
|
||||
* Validates the master password for the current logged in user.
|
||||
*/
|
||||
suspend fun validatePassword(password: String): ValidatePasswordResult
|
||||
}
|
||||
|
|
|
@ -43,6 +43,7 @@ import com.x8bit.bitwarden.data.auth.repository.model.ResendEmailResult
|
|||
import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.UserFingerprintResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.UserState
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.CaptchaCallbackTokenResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.SsoCallbackResult
|
||||
|
@ -777,6 +778,27 @@ class AuthRepositoryImpl(
|
|||
},
|
||||
)
|
||||
|
||||
@Suppress("ReturnCount")
|
||||
override suspend fun validatePassword(password: String): ValidatePasswordResult {
|
||||
val userId = activeUserId ?: return ValidatePasswordResult.Error
|
||||
val masterPasswordHash = authDiskSource.getMasterPasswordHash(userId = userId)
|
||||
?: return ValidatePasswordResult.Error
|
||||
return vaultSdkSource
|
||||
.validatePassword(
|
||||
userId = userId,
|
||||
password = password,
|
||||
passwordHash = masterPasswordHash,
|
||||
)
|
||||
.fold(
|
||||
onSuccess = {
|
||||
ValidatePasswordResult.Success(isValid = it)
|
||||
},
|
||||
onFailure = {
|
||||
ValidatePasswordResult.Error
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
private suspend fun getFingerprintPhrase(
|
||||
publicKey: String,
|
||||
): UserFingerprintResult {
|
||||
|
|
|
@ -0,0 +1,19 @@
|
|||
package com.x8bit.bitwarden.data.auth.repository.model
|
||||
|
||||
/**
|
||||
* Models result of determining if a password is valid.
|
||||
*/
|
||||
sealed class ValidatePasswordResult {
|
||||
|
||||
/**
|
||||
* The validity of the password was checked successfully and [isValid].
|
||||
*/
|
||||
data class Success(
|
||||
val isValid: Boolean,
|
||||
) : ValidatePasswordResult()
|
||||
|
||||
/**
|
||||
* There was an error determining if the validity of the password.
|
||||
*/
|
||||
data object Error : ValidatePasswordResult()
|
||||
}
|
|
@ -197,9 +197,4 @@ interface SettingsRepository {
|
|||
* Clears any previously set unlock PIN for the current user.
|
||||
*/
|
||||
fun clearUnlockPin()
|
||||
|
||||
/**
|
||||
* Validate the master password.
|
||||
*/
|
||||
suspend fun validatePassword(password: String): Boolean
|
||||
}
|
||||
|
|
|
@ -430,21 +430,6 @@ class SettingsRepositoryImpl(
|
|||
}
|
||||
.launchIn(unconfinedScope)
|
||||
}
|
||||
|
||||
override suspend fun validatePassword(password: String): Boolean =
|
||||
activeUserId
|
||||
?.let { userId ->
|
||||
authDiskSource
|
||||
.getMasterPasswordHash(userId)
|
||||
?.let { passwordHash ->
|
||||
vaultSdkSource.validatePassword(
|
||||
userId = userId,
|
||||
password = password,
|
||||
passwordHash = passwordHash,
|
||||
)
|
||||
}
|
||||
}
|
||||
?: false
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -311,5 +311,5 @@ interface VaultSdkSource {
|
|||
userId: String,
|
||||
password: String,
|
||||
passwordHash: String,
|
||||
): Boolean
|
||||
): Result<Boolean>
|
||||
}
|
||||
|
|
|
@ -320,13 +320,14 @@ class VaultSdkSourceImpl(
|
|||
userId: String,
|
||||
password: String,
|
||||
passwordHash: String,
|
||||
): Boolean =
|
||||
): Result<Boolean> = runCatching {
|
||||
getClient(userId = userId)
|
||||
.auth()
|
||||
.validatePassword(
|
||||
password = password,
|
||||
passwordHash = passwordHash,
|
||||
)
|
||||
}
|
||||
|
||||
private fun getClient(
|
||||
userId: String,
|
||||
|
|
|
@ -4,7 +4,8 @@ import android.os.Parcelable
|
|||
import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.x8bit.bitwarden.R
|
||||
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
|
||||
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
|
||||
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
|
||||
|
@ -24,7 +25,7 @@ private const val KEY_STATE = "state"
|
|||
*/
|
||||
@HiltViewModel
|
||||
class ExportVaultViewModel @Inject constructor(
|
||||
private val settingsRepository: SettingsRepository,
|
||||
private val authRepository: AuthRepository,
|
||||
savedStateHandle: SavedStateHandle,
|
||||
) : BaseViewModel<ExportVaultState, ExportVaultEvent, ExportVaultAction>(
|
||||
initialState = savedStateHandle[KEY_STATE]
|
||||
|
@ -85,7 +86,7 @@ class ExportVaultViewModel @Inject constructor(
|
|||
viewModelScope.launch {
|
||||
sendAction(
|
||||
ExportVaultAction.Internal.ReceiveValidatePasswordResult(
|
||||
isPasswordValid = settingsRepository.validatePassword(
|
||||
result = authRepository.validatePassword(
|
||||
password = mutableStateFlow.value.passwordInput,
|
||||
),
|
||||
),
|
||||
|
@ -124,19 +125,34 @@ class ExportVaultViewModel @Inject constructor(
|
|||
private fun handleReceiveValidatePasswordResult(
|
||||
action: ExportVaultAction.Internal.ReceiveValidatePasswordResult,
|
||||
) {
|
||||
// Display an error dialog if the password is invalid.
|
||||
if (!action.isPasswordValid) {
|
||||
mutableStateFlow.update {
|
||||
it.copy(
|
||||
dialogState = ExportVaultState.DialogState.Error(
|
||||
title = null,
|
||||
message = R.string.invalid_master_password.asText(),
|
||||
),
|
||||
)
|
||||
when (action.result) {
|
||||
ValidatePasswordResult.Error -> {
|
||||
mutableStateFlow.update {
|
||||
it.copy(
|
||||
dialogState = ExportVaultState.DialogState.Error(
|
||||
title = null,
|
||||
message = R.string.generic_error_message.asText(),
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
is ValidatePasswordResult.Success -> {
|
||||
// Display an error dialog if the password is invalid.
|
||||
if (!action.result.isValid) {
|
||||
mutableStateFlow.update {
|
||||
it.copy(
|
||||
dialogState = ExportVaultState.DialogState.Error(
|
||||
title = null,
|
||||
message = R.string.invalid_master_password.asText(),
|
||||
),
|
||||
)
|
||||
}
|
||||
} else {
|
||||
// TODO: BIT-1274, BIT-1275, and BIT-1276
|
||||
sendEvent(ExportVaultEvent.ShowToast("Not yet implemented".asText()))
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// TODO: BIT-1274, BIT-1275, and BIT-1276
|
||||
sendEvent(ExportVaultEvent.ShowToast("Not yet implemented".asText()))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -226,7 +242,7 @@ sealed class ExportVaultAction {
|
|||
* Indicates that a validate password result has been received.
|
||||
*/
|
||||
data class ReceiveValidatePasswordResult(
|
||||
val isPasswordValid: Boolean,
|
||||
val result: ValidatePasswordResult,
|
||||
) : Internal()
|
||||
}
|
||||
}
|
||||
|
|
|
@ -54,6 +54,7 @@ import com.x8bit.bitwarden.data.auth.repository.model.RegisterResult
|
|||
import com.x8bit.bitwarden.data.auth.repository.model.ResendEmailResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.SwitchAccountResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.UserOrganizations
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.CaptchaCallbackTokenResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.SsoCallbackResult
|
||||
|
@ -2668,6 +2669,110 @@ class AuthRepositoryTest {
|
|||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `validatePassword with no current user returns ValidatePasswordResult Error`() = runTest {
|
||||
val userId = "userId"
|
||||
val password = "password"
|
||||
val passwordHash = "passwordHash"
|
||||
fakeAuthDiskSource.userState = null
|
||||
coEvery {
|
||||
vaultSdkSource.validatePassword(
|
||||
userId = userId,
|
||||
password = password,
|
||||
passwordHash = passwordHash,
|
||||
)
|
||||
} returns true.asSuccess()
|
||||
|
||||
val result = repository
|
||||
.validatePassword(
|
||||
password = password,
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
ValidatePasswordResult.Error,
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `validatePassword with no stored password hash returns ValidatePasswordResult Error`() =
|
||||
runTest {
|
||||
val userId = USER_ID_1
|
||||
val password = "password"
|
||||
val passwordHash = "passwordHash"
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
|
||||
coEvery {
|
||||
vaultSdkSource.validatePassword(
|
||||
userId = userId,
|
||||
password = password,
|
||||
passwordHash = passwordHash,
|
||||
)
|
||||
} returns true.asSuccess()
|
||||
|
||||
val result = repository
|
||||
.validatePassword(
|
||||
password = password,
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
ValidatePasswordResult.Error,
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `validatePassword with sdk failure returns a ValidatePasswordResult Error`() = runTest {
|
||||
val userId = USER_ID_1
|
||||
val password = "password"
|
||||
val passwordHash = "passwordHash"
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
|
||||
fakeAuthDiskSource.storeMasterPasswordHash(userId = userId, passwordHash = passwordHash)
|
||||
coEvery {
|
||||
vaultSdkSource.validatePassword(
|
||||
userId = userId,
|
||||
password = password,
|
||||
passwordHash = passwordHash,
|
||||
)
|
||||
} returns Throwable().asFailure()
|
||||
|
||||
val result = repository
|
||||
.validatePassword(
|
||||
password = password,
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
ValidatePasswordResult.Error,
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `validatePassword with sdk success returns a ValidatePasswordResult Success`() = runTest {
|
||||
val userId = USER_ID_1
|
||||
val password = "password"
|
||||
val passwordHash = "passwordHash"
|
||||
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
|
||||
fakeAuthDiskSource.storeMasterPasswordHash(userId = userId, passwordHash = passwordHash)
|
||||
coEvery {
|
||||
vaultSdkSource.validatePassword(
|
||||
userId = userId,
|
||||
password = password,
|
||||
passwordHash = passwordHash,
|
||||
)
|
||||
} returns true.asSuccess()
|
||||
|
||||
val result = repository
|
||||
.validatePassword(
|
||||
password = password,
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
ValidatePasswordResult.Success(isValid = true),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
companion object {
|
||||
private const val UNIQUE_APP_ID = "testUniqueAppId"
|
||||
private const val EMAIL = "test@bitwarden.com"
|
||||
|
|
|
@ -884,40 +884,6 @@ class SettingsRepositoryTest {
|
|||
assertEquals(false, fakeSettingsDiskSource.getScreenCaptureAllowed(userId))
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `validatePassword returns the validate password result`() = runTest {
|
||||
val userId = "userId"
|
||||
val password = "password"
|
||||
val passwordHash = "passwordHash"
|
||||
fakeAuthDiskSource.userState = MOCK_USER_STATE
|
||||
fakeAuthDiskSource.storeMasterPasswordHash(userId = userId, passwordHash = passwordHash)
|
||||
coEvery {
|
||||
vaultSdkSource.validatePassword(
|
||||
userId = userId,
|
||||
password = password,
|
||||
passwordHash = passwordHash,
|
||||
)
|
||||
} returns true
|
||||
|
||||
val result = settingsRepository
|
||||
.validatePassword(
|
||||
password = password,
|
||||
)
|
||||
assertTrue(result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `validatePassword returns false if there's no stored password hash`() = runTest {
|
||||
fakeAuthDiskSource.userState = MOCK_USER_STATE
|
||||
val password = "password"
|
||||
|
||||
val result = settingsRepository
|
||||
.validatePassword(
|
||||
password = password,
|
||||
)
|
||||
assertFalse(result)
|
||||
}
|
||||
}
|
||||
|
||||
private val MOCK_USER_STATE =
|
||||
|
|
|
@ -39,7 +39,6 @@ import io.mockk.verify
|
|||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.Assertions.assertTrue
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
@Suppress("LargeClass")
|
||||
|
@ -742,7 +741,7 @@ class VaultSdkSourceTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
fun `validatePassword should call SDK and return the expected Boolean`() = runTest {
|
||||
fun `validatePassword should call SDK and a Result with correct data`() = runTest {
|
||||
val userId = "userId"
|
||||
val password = "password"
|
||||
val passwordHash = "passwordHash"
|
||||
|
@ -758,6 +757,9 @@ class VaultSdkSourceTest {
|
|||
password = password,
|
||||
passwordHash = passwordHash,
|
||||
)
|
||||
assertTrue(result)
|
||||
assertEquals(
|
||||
true.asSuccess(),
|
||||
result,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
@ -3,7 +3,8 @@ package com.x8bit.bitwarden.ui.platform.feature.settings.exportvault
|
|||
import androidx.lifecycle.SavedStateHandle
|
||||
import app.cash.turbine.test
|
||||
import com.x8bit.bitwarden.R
|
||||
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
|
||||
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.ValidatePasswordResult
|
||||
import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest
|
||||
import com.x8bit.bitwarden.ui.platform.base.util.asText
|
||||
import com.x8bit.bitwarden.ui.platform.feature.settings.exportvault.model.ExportVaultFormat
|
||||
|
@ -14,7 +15,7 @@ import org.junit.jupiter.api.Assertions.assertEquals
|
|||
import org.junit.jupiter.api.Test
|
||||
|
||||
class ExportVaultViewModelTest : BaseViewModelTest() {
|
||||
private val settingsRepository: SettingsRepository = mockk()
|
||||
private val authRepository: AuthRepository = mockk()
|
||||
|
||||
private val savedStateHandle = SavedStateHandle()
|
||||
|
||||
|
@ -42,10 +43,10 @@ class ExportVaultViewModelTest : BaseViewModelTest() {
|
|||
fun `ConfirmExportVaultClicked correct password should emit ShowToast`() = runTest {
|
||||
val password = "password"
|
||||
coEvery {
|
||||
settingsRepository.validatePassword(
|
||||
authRepository.validatePassword(
|
||||
password = password,
|
||||
)
|
||||
} returns true
|
||||
} returns ValidatePasswordResult.Success(isValid = true)
|
||||
|
||||
val viewModel = createViewModel()
|
||||
viewModel.eventFlow.test {
|
||||
|
@ -88,10 +89,10 @@ class ExportVaultViewModelTest : BaseViewModelTest() {
|
|||
fun `ConfirmExportVaultClicked invalid password should show an error`() = runTest {
|
||||
val password = "password"
|
||||
coEvery {
|
||||
settingsRepository.validatePassword(
|
||||
authRepository.validatePassword(
|
||||
password = password,
|
||||
)
|
||||
} returns false
|
||||
} returns ValidatePasswordResult.Success(isValid = false)
|
||||
|
||||
val viewModel = createViewModel()
|
||||
viewModel.eventFlow.test {
|
||||
|
@ -111,6 +112,33 @@ class ExportVaultViewModelTest : BaseViewModelTest() {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `ConfirmExportVaultClicked error checking password should show an error`() = runTest {
|
||||
val password = "password"
|
||||
coEvery {
|
||||
authRepository.validatePassword(
|
||||
password = password,
|
||||
)
|
||||
} returns ValidatePasswordResult.Error
|
||||
|
||||
val viewModel = createViewModel()
|
||||
viewModel.eventFlow.test {
|
||||
viewModel.trySendAction(ExportVaultAction.PasswordInputChanged(password))
|
||||
|
||||
viewModel.trySendAction(ExportVaultAction.ConfirmExportVaultClicked)
|
||||
assertEquals(
|
||||
DEFAULT_STATE.copy(
|
||||
dialogState = ExportVaultState.DialogState.Error(
|
||||
title = null,
|
||||
message = R.string.generic_error_message.asText(),
|
||||
),
|
||||
passwordInput = password,
|
||||
),
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `ExportFormatOptionSelect should update the selected export format in the state`() =
|
||||
runTest {
|
||||
|
@ -146,7 +174,7 @@ class ExportVaultViewModelTest : BaseViewModelTest() {
|
|||
|
||||
private fun createViewModel(): ExportVaultViewModel =
|
||||
ExportVaultViewModel(
|
||||
settingsRepository = settingsRepository,
|
||||
authRepository = authRepository,
|
||||
savedStateHandle = savedStateHandle,
|
||||
)
|
||||
}
|
||||
|
|
Loading…
Add table
Reference in a new issue