diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/model/LoginResultExtensions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/model/LoginResultExtensions.kt index c7a8687807..cc2daa5ca0 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/model/LoginResultExtensions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/model/LoginResultExtensions.kt @@ -13,6 +13,7 @@ fun VaultUnlockError.toLoginErrorResult(): LoginResult.Error = when (this) { } is VaultUnlockResult.BiometricDecodingError, + is VaultUnlockResult.BiometricKeystoreAuthorizationError, is VaultUnlockResult.GenericError, is VaultUnlockResult.InvalidStateError, -> LoginResult.Error(error = this.error) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryImpl.kt index 8692185cf4..85826112e2 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryImpl.kt @@ -66,6 +66,7 @@ class AuthenticatorBridgeRepositoryImpl( when (vaultUnlockResult) { is VaultUnlockResult.AuthenticationError, is VaultUnlockResult.BiometricDecodingError, + is VaultUnlockResult.BiometricKeystoreAuthorizationError, is VaultUnlockResult.GenericError, is VaultUnlockResult.InvalidStateError, -> { diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/util/ThrowableExtensions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/util/ThrowableExtensions.kt index 856454be66..26402dbc5e 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/util/ThrowableExtensions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/util/ThrowableExtensions.kt @@ -1,5 +1,9 @@ package com.x8bit.bitwarden.data.platform.util +import android.os.Build +import android.security.KeyStoreException +import androidx.annotation.RequiresApi +import com.bitwarden.core.util.isBuildVersionAtLeast import com.bitwarden.network.exception.CookieRedirectException import com.bitwarden.network.exception.LocalNetworkAccessException @@ -13,3 +17,36 @@ val Throwable.userFriendlyMessage: String? is CookieRedirectException -> message else -> null } + +/** + * Returns true when this [Throwable] or one of its causes is an Android Keystore exception that + * reports [KeyStoreException.ERROR_USER_AUTHENTICATION_REQUIRED]. + * + * The public numeric-error API is only queried on API 33+. + */ +fun Throwable.isKeystoreUserAuthenticationRequired(): Boolean { + if (!isBuildVersionAtLeast(Build.VERSION_CODES.TIRAMISU)) { + return false + } + return hasUserAuthenticationRequiredKeystoreCause() +} + +/** + * Returns true when this [Throwable] or one of its causes is a [KeyStoreException] reporting + * [KeyStoreException.ERROR_USER_AUTHENTICATION_REQUIRED]. + */ +@RequiresApi(Build.VERSION_CODES.TIRAMISU) +private fun Throwable.hasUserAuthenticationRequiredKeystoreCause(): Boolean { + val seen = mutableSetOf() + var current: Throwable? = this + while (current != null && seen.add(current)) { + if ( + current is KeyStoreException && + current.numericErrorCode == KeyStoreException.ERROR_USER_AUTHENTICATION_REQUIRED + ) { + return true + } + current = current.cause + } + return false +} diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt index 9ab8a0fdf8..8176da8531 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt @@ -49,6 +49,7 @@ import com.x8bit.bitwarden.data.vault.repository.util.toEncryptedSdkCipher import com.x8bit.bitwarden.data.vault.repository.util.toEncryptedSdkFolder import com.x8bit.bitwarden.data.vault.repository.util.toSdkAccount import com.x8bit.bitwarden.data.vault.repository.util.toSdkMasterPasswordUnlock +import com.x8bit.bitwarden.data.vault.repository.util.toVaultUnlockResult import com.x8bit.bitwarden.ui.vault.feature.vault.model.VaultFilterType import com.x8bit.bitwarden.ui.vault.feature.vault.util.toFilteredList import kotlinx.coroutines.CoroutineScope @@ -302,7 +303,7 @@ internal class VaultRepositoryImpl( .decodeToString() } catch (e: GeneralSecurityException) { Timber.w(e, "unlockVaultWithBiometrics failed when decrypting biometrics key") - return VaultUnlockResult.BiometricDecodingError(error = e) + return e.toVaultUnlockResult() } } ?: biometricsKey @@ -316,7 +317,7 @@ internal class VaultRepositoryImpl( .toString(Charsets.ISO_8859_1) } catch (e: GeneralSecurityException) { Timber.w(e, "unlockVaultWithBiometrics failed to migrate the user to IV encryption") - return VaultUnlockResult.BiometricDecodingError(error = e) + return e.toVaultUnlockResult() } } else { null diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/model/VaultUnlockResult.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/model/VaultUnlockResult.kt index beaaeb798b..09228d0f13 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/model/VaultUnlockResult.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/model/VaultUnlockResult.kt @@ -25,6 +25,13 @@ sealed class VaultUnlockResult { override val error: Throwable?, ) : VaultUnlockResult(), VaultUnlockError + /** + * Biometric cipher is missing its Keystore authorization. + */ + data class BiometricKeystoreAuthorizationError( + override val error: Throwable, + ) : VaultUnlockResult(), VaultUnlockError + /** * Unable to access user state information. */ diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/util/VaultUnlockResultExtensions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/util/VaultUnlockResultExtensions.kt index bd61a2fdf1..024d9f014e 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/util/VaultUnlockResultExtensions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/util/VaultUnlockResultExtensions.kt @@ -1,7 +1,9 @@ package com.x8bit.bitwarden.data.vault.repository.util +import com.x8bit.bitwarden.data.platform.util.isKeystoreUserAuthenticationRequired import com.x8bit.bitwarden.data.vault.datasource.sdk.model.InitializeCryptoResult import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockResult +import java.security.GeneralSecurityException /** * Transform a [InitializeCryptoResult] to [VaultUnlockResult]. @@ -17,3 +19,16 @@ fun InitializeCryptoResult.toVaultUnlockResult(): VaultUnlockResult = InitializeCryptoResult.Success -> VaultUnlockResult.Success } + +/** + * Maps a biometric cipher security failure to a [VaultUnlockResult]. + * + * A Keystore authorization miss is reported separately so callers can retry before clearing + * biometrics. All other security failures remain decoding errors. + */ +fun GeneralSecurityException.toVaultUnlockResult(): VaultUnlockResult = + if (isKeystoreUserAuthenticationRequired()) { + VaultUnlockResult.BiometricKeystoreAuthorizationError(error = this) + } else { + VaultUnlockResult.BiometricDecodingError(error = this) + } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt index afb6aee4c7..8060790922 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt @@ -48,6 +48,7 @@ import javax.crypto.Cipher import javax.inject.Inject private const val KEY_STATE = "state" +private const val KEYSTORE_AUTH_FAILURES_BEFORE_CLEAR = 2 /** * Manages application state for the initial vault unlock screen. @@ -365,6 +366,10 @@ class VaultUnlockViewModel @Inject constructor( } } + is VaultUnlockResult.BiometricKeystoreAuthorizationError -> { + handleBiometricKeystoreAuthorizationError(result) + } + is VaultUnlockResult.GenericError, is VaultUnlockResult.InvalidStateError, -> { @@ -380,15 +385,54 @@ class VaultUnlockViewModel @Inject constructor( } } - VaultUnlockResult.Success -> { - if (specialCircumstanceManager.specialCircumstance == null) { - specialCircumstanceManager.specialCircumstance = - appResumeManager.getResumeSpecialCircumstance() - } + VaultUnlockResult.Success -> handleVaultUnlockSuccess() + } + } - mutableStateFlow.update { it.copy(dialog = null) } - // Don't do anything, we'll navigate to the right place. + private fun handleVaultUnlockSuccess() { + if (specialCircumstanceManager.specialCircumstance == null) { + specialCircumstanceManager.specialCircumstance = + appResumeManager.getResumeSpecialCircumstance() + } + + mutableStateFlow.update { + it.copy( + dialog = null, + keystoreAuthorizationFailureCount = 0, + ) + } + // Don't do anything, we'll navigate to the right place. + } + + private fun handleBiometricKeystoreAuthorizationError( + result: VaultUnlockResult.BiometricKeystoreAuthorizationError, + ) { + val failureCount = state.keystoreAuthorizationFailureCount + 1 + if (failureCount >= KEYSTORE_AUTH_FAILURES_BEFORE_CLEAR) { + authRepository.clearBiometrics(userId = state.userId) + mutableStateFlow.update { + it.copy( + isBiometricsValid = false, + keystoreAuthorizationFailureCount = failureCount, + dialog = VaultUnlockState.VaultUnlockDialog.Error( + title = BitwardenString.biometrics_failed.asText(), + message = BitwardenString.biometrics_decoding_failure.asText(), + throwable = result.error, + ), + ) } + return + } + + mutableStateFlow.update { + it.copy( + keystoreAuthorizationFailureCount = failureCount, + dialog = VaultUnlockState.VaultUnlockDialog.Error( + title = BitwardenString.an_error_has_occurred.asText(), + message = BitwardenString.generic_error_message.asText(), + throwable = result.error, + ), + ) } } @@ -427,6 +471,11 @@ class VaultUnlockViewModel @Inject constructor( vaultUnlockType = userState.activeAccount.vaultUnlockType, input = "", hasMasterPassword = userState.activeAccount.hasMasterPassword, + keystoreAuthorizationFailureCount = if (it.userId != userState.activeUserId) { + 0 + } else { + it.keystoreAuthorizationFailureCount + }, ) } @@ -467,6 +516,7 @@ data class VaultUnlockState( val createCredentialRequest: CreateCredentialRequest? = null, private val hasMasterPassword: Boolean, val isFromLockFlow: Boolean, + val keystoreAuthorizationFailureCount: Int = 0, ) : Parcelable { /** diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/exportitems/verifypassword/VerifyPasswordViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/exportitems/verifypassword/VerifyPasswordViewModel.kt index 14f59a6751..55cac1f707 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/exportitems/verifypassword/VerifyPasswordViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/exportitems/verifypassword/VerifyPasswordViewModel.kt @@ -298,6 +298,7 @@ class VerifyPasswordViewModel @Inject constructor( is VaultUnlockResult.InvalidStateError, is VaultUnlockResult.BiometricDecodingError, + is VaultUnlockResult.BiometricKeystoreAuthorizationError, is VaultUnlockResult.GenericError, -> { showGenericErrorDialog(throwable = action.vaultUnlockResult.error) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/model/LoginResultExtensionsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/model/LoginResultExtensionsTest.kt index 47cc465d26..4fd6238038 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/model/LoginResultExtensionsTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/model/LoginResultExtensionsTest.kt @@ -36,10 +36,14 @@ class LoginResultExtensionsTest { val genericErrorResult = VaultUnlockResult.GenericError(error = error).toLoginErrorResult() val biometricErrorResult = VaultUnlockResult.BiometricDecodingError(error = error).toLoginErrorResult() + val keystoreAuthorizationResult = VaultUnlockResult + .BiometricKeystoreAuthorizationError(error = error) + .toLoginErrorResult() val expectedResult = LoginResult.Error(error = error) assertEquals(expectedResult, invalidStateResult) assertEquals(expectedResult, genericErrorResult) assertEquals(expectedResult, biometricErrorResult) + assertEquals(expectedResult, keystoreAuthorizationResult) } } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/util/ThrowableExtensionsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/util/ThrowableExtensionsTest.kt index 4d2e5e906b..35b5112b69 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/util/ThrowableExtensionsTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/util/ThrowableExtensionsTest.kt @@ -1,14 +1,37 @@ package com.x8bit.bitwarden.data.platform.util +import android.os.Build +import android.security.KeyStoreException +import com.bitwarden.core.util.isBuildVersionAtLeast import com.bitwarden.network.exception.CookieRedirectException import com.bitwarden.network.exception.LocalNetworkAccessException +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkStatic +import io.mockk.verify +import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import java.io.IOException +import javax.crypto.IllegalBlockSizeException class ThrowableExtensionsTest { + @BeforeEach + fun setup() { + mockkStatic(::isBuildVersionAtLeast) + } + + @AfterEach + fun tearDown() { + unmockkStatic(::isBuildVersionAtLeast) + } + @Test fun `userFriendlyMessage should return message for CookieRedirectException`() { val message = "Your request was interrupted because the app needed to " + @@ -41,4 +64,110 @@ class ThrowableExtensionsTest { val exception = RuntimeException("runtime error") assertNull(exception.userFriendlyMessage) } + + @Suppress("MaxLineLength") + @Test + fun `isKeystoreUserAuthenticationRequired should return true when the cause reports user authentication required on API 33`() { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + val (error, _) = createUserAuthenticationRequiredError() + + assertTrue(error.isKeystoreUserAuthenticationRequired()) + } + + @Suppress("MaxLineLength") + @Test + fun `isKeystoreUserAuthenticationRequired should return true for a nested user authentication required cause`() { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + val (error, _) = createUserAuthenticationRequiredError(additionalWrapperCount = 2) + + assertTrue(error.isKeystoreUserAuthenticationRequired()) + } + + @Suppress("MaxLineLength") + @Test + fun `isKeystoreUserAuthenticationRequired should return true when the throwable itself is the keystore exception`() { + mockBuildVersion(sdkInt = Build.VERSION_CODES.BAKLAVA) + val keyStoreException = mockKeyStoreException( + numericErrorCode = KeyStoreException.ERROR_USER_AUTHENTICATION_REQUIRED, + ) + + assertTrue(keyStoreException.isKeystoreUserAuthenticationRequired()) + } + + @Suppress("MaxLineLength") + @Test + fun `isKeystoreUserAuthenticationRequired should return false for a nonmatching numeric code`() { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + val keyStoreException = mockKeyStoreException( + numericErrorCode = KeyStoreException.ERROR_KEY_CORRUPTED, + ) + val error = wrapKeyStoreException(keyStoreException = keyStoreException) + + assertFalse(error.isKeystoreUserAuthenticationRequired()) + } + + @Test + fun `isKeystoreUserAuthenticationRequired should return false when the cause is missing`() { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + + assertFalse(IllegalBlockSizeException().isKeystoreUserAuthenticationRequired()) + } + + @Suppress("MaxLineLength") + @Test + fun `isKeystoreUserAuthenticationRequired should return false below API 33 without querying the numeric code`() { + mockBuildVersion(sdkInt = Build.VERSION_CODES.S_V2) + val (error, keyStoreException) = createUserAuthenticationRequiredError() + + assertFalse(error.isKeystoreUserAuthenticationRequired()) + verify(exactly = 0) { keyStoreException.numericErrorCode } + } + + @Suppress("MaxLineLength") + @Test + fun `isKeystoreUserAuthenticationRequired should return false and stop when the cause chain cycles`() { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + val keyStoreException = mockKeyStoreException( + numericErrorCode = KeyStoreException.ERROR_KEY_CORRUPTED, + ) + val wrapper = IllegalBlockSizeException() + every { keyStoreException.cause } returns wrapper + wrapper.initCause(keyStoreException) + + assertFalse(wrapper.isKeystoreUserAuthenticationRequired()) + } + + private fun mockBuildVersion(sdkInt: Int) { + every { isBuildVersionAtLeast(any()) } answers { sdkInt >= firstArg() } + } + + private fun mockKeyStoreException(numericErrorCode: Int): KeyStoreException { + val keyStoreException = mockk() + every { keyStoreException.numericErrorCode } returns numericErrorCode + every { keyStoreException.cause } returns null + return keyStoreException + } + + private fun wrapKeyStoreException( + keyStoreException: KeyStoreException, + additionalWrapperCount: Int = 0, + ): IllegalBlockSizeException { + var cause: Throwable = keyStoreException + repeat(additionalWrapperCount) { index -> + cause = RuntimeException("wrapper-$index", cause) + } + return IllegalBlockSizeException().apply { initCause(cause) } + } + + private fun createUserAuthenticationRequiredError( + additionalWrapperCount: Int = 0, + ): Pair { + val keyStoreException = mockKeyStoreException( + numericErrorCode = KeyStoreException.ERROR_USER_AUTHENTICATION_REQUIRED, + ) + return wrapKeyStoreException( + keyStoreException = keyStoreException, + additionalWrapperCount = additionalWrapperCount, + ) to keyStoreException + } } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt index fb8fb54173..4325e964a9 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt @@ -1,5 +1,7 @@ package com.x8bit.bitwarden.data.vault.repository +import android.os.Build +import android.security.KeyStoreException import app.cash.turbine.test import com.bitwarden.collections.CollectionView import com.bitwarden.core.InitUserCryptoMethod @@ -11,6 +13,7 @@ import com.bitwarden.core.data.repository.error.MissingPropertyException import com.bitwarden.core.data.repository.model.DataState import com.bitwarden.core.data.util.asFailure import com.bitwarden.core.data.util.asSuccess +import com.bitwarden.core.util.isBuildVersionAtLeast import com.bitwarden.exporters.ExportFormat import com.bitwarden.fido.Fido2CredentialAutofillView import com.bitwarden.network.model.CipherTypeJson @@ -71,8 +74,11 @@ import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.mockkConstructor +import io.mockk.mockkStatic import io.mockk.runs import io.mockk.unmockkConstructor +import io.mockk.unmockkStatic +import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest @@ -82,9 +88,11 @@ import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import java.security.GeneralSecurityException +import java.security.InvalidKeyException import java.time.Instant import javax.crypto.BadPaddingException import javax.crypto.Cipher +import javax.crypto.IllegalBlockSizeException @Suppress("LargeClass") class VaultRepositoryTest { @@ -157,12 +165,15 @@ class VaultRepositoryTest { every { anyConstructed() == any() } returns true + mockkStatic(::isBuildVersionAtLeast) + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) } @AfterEach fun tearDown() { unmockkConstructor(NoActiveUserException::class) unmockkConstructor(MissingPropertyException::class) + unmockkStatic(::isBuildVersionAtLeast) } @Test @@ -260,6 +271,156 @@ class VaultRepositoryTest { ) } + @Suppress("MaxLineLength") + @Test + fun `unlockVaultWithBiometrics with stored IV and Keystore user-authentication-required error on API 33 should return BiometricKeystoreAuthorizationError without unlocking or mutating biometrics`() = + runTest { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + val (error, _) = createUserAuthenticationRequiredDoFinalError() + + val result = unlockVaultWithBiometricsDoFinalError(error = error) + + assertEquals( + VaultUnlockResult.BiometricKeystoreAuthorizationError(error = error), + result, + ) + assertEquals( + error, + (result as VaultUnlockResult.BiometricKeystoreAuthorizationError).error, + ) + assertStoredBiometricsUnchanged() + verifyUnlockAndPinDerivationNeverCalled() + } + + @Suppress("MaxLineLength") + @Test + fun `unlockVaultWithBiometrics with stored IV and Keystore user-authentication-required error on API 36 should return BiometricKeystoreAuthorizationError without unlocking or mutating biometrics`() = + runTest { + mockBuildVersion(sdkInt = Build.VERSION_CODES.BAKLAVA) + val (error, _) = createUserAuthenticationRequiredDoFinalError() + + val result = unlockVaultWithBiometricsDoFinalError(error = error) + + assertEquals( + VaultUnlockResult.BiometricKeystoreAuthorizationError(error = error), + result, + ) + assertStoredBiometricsUnchanged() + verifyUnlockAndPinDerivationNeverCalled() + } + + @Suppress("MaxLineLength") + @Test + fun `unlockVaultWithBiometrics with stored IV and nested Keystore user-authentication-required error should return BiometricKeystoreAuthorizationError`() = + runTest { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + val (error, _) = createUserAuthenticationRequiredDoFinalError(additionalWrapperCount = 2) + + val result = unlockVaultWithBiometricsDoFinalError(error = error) + + assertEquals( + VaultUnlockResult.BiometricKeystoreAuthorizationError(error = error), + result, + ) + assertStoredBiometricsUnchanged() + verifyUnlockAndPinDerivationNeverCalled() + } + + @Suppress("MaxLineLength") + @Test + fun `unlockVaultWithBiometrics without IV and Keystore user-authentication-required error should return BiometricKeystoreAuthorizationError without migrating biometrics`() = + runTest { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + val (error, _) = createUserAuthenticationRequiredDoFinalError() + + val result = unlockVaultWithBiometricsDoFinalError( + error = error, + initVector = null, + ) + + assertEquals( + VaultUnlockResult.BiometricKeystoreAuthorizationError(error = error), + result, + ) + assertStoredBiometricsUnchanged(initVector = null) + verifyUnlockAndPinDerivationNeverCalled() + } + + @Suppress("MaxLineLength") + @Test + fun `unlockVaultWithBiometrics with stored IV and Keystore error with a nonmatching numeric code should return BiometricDecodingError`() = + runTest { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + val keyStoreException = mockKeyStoreException( + numericErrorCode = KeyStoreException.ERROR_KEY_CORRUPTED, + ) + val error = wrapKeyStoreException(keyStoreException = keyStoreException) + + val result = unlockVaultWithBiometricsDoFinalError(error = error) + + assertEquals(VaultUnlockResult.BiometricDecodingError(error = error), result) + assertStoredBiometricsUnchanged() + verifyUnlockAndPinDerivationNeverCalled() + } + + @Suppress("MaxLineLength") + @Test + fun `unlockVaultWithBiometrics with stored IV and missing exception cause should return BiometricDecodingError`() = + runTest { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + val error = IllegalBlockSizeException() + + val result = unlockVaultWithBiometricsDoFinalError(error = error) + + assertEquals(VaultUnlockResult.BiometricDecodingError(error = error), result) + assertStoredBiometricsUnchanged() + verifyUnlockAndPinDerivationNeverCalled() + } + + @Suppress("MaxLineLength") + @Test + fun `unlockVaultWithBiometrics with stored IV and permanently invalidated key should return BiometricDecodingError`() = + runTest { + mockBuildVersion(sdkInt = Build.VERSION_CODES.TIRAMISU) + val error = InvalidKeyException("Key permanently invalidated") + + val result = unlockVaultWithBiometricsDoFinalError(error = error) + + assertEquals(VaultUnlockResult.BiometricDecodingError(error = error), result) + assertStoredBiometricsUnchanged() + verifyUnlockAndPinDerivationNeverCalled() + } + + @Suppress("MaxLineLength") + @Test + fun `unlockVaultWithBiometrics with stored IV and Keystore user-authentication-required error on API 29 should return BiometricDecodingError without querying the numeric code`() = + runTest { + mockBuildVersion(sdkInt = Build.VERSION_CODES.Q) + val (error, keyStoreException) = createUserAuthenticationRequiredDoFinalError() + + val result = unlockVaultWithBiometricsDoFinalError(error = error) + + assertEquals(VaultUnlockResult.BiometricDecodingError(error = error), result) + assertStoredBiometricsUnchanged() + verifyUnlockAndPinDerivationNeverCalled() + verify(exactly = 0) { keyStoreException.numericErrorCode } + } + + @Suppress("MaxLineLength") + @Test + fun `unlockVaultWithBiometrics with stored IV and Keystore user-authentication-required error on API 32 should return BiometricDecodingError without querying the numeric code`() = + runTest { + mockBuildVersion(sdkInt = Build.VERSION_CODES.S_V2) + val (error, keyStoreException) = createUserAuthenticationRequiredDoFinalError() + + val result = unlockVaultWithBiometricsDoFinalError(error = error) + + assertEquals(VaultUnlockResult.BiometricDecodingError(error = error), result) + assertStoredBiometricsUnchanged() + verifyUnlockAndPinDerivationNeverCalled() + verify(exactly = 0) { keyStoreException.numericErrorCode } + } + @Suppress("MaxLineLength") @Test fun `unlockVaultWithBiometrics with an IV and VaultLockManager Success should store the updated key and IV and unlock for the current user and return Success`() = @@ -1846,9 +2007,87 @@ class VaultRepositoryTest { ) } returns unlockResult } + + private fun mockBuildVersion(sdkInt: Int) { + every { isBuildVersionAtLeast(any()) } answers { sdkInt >= firstArg() } + } + + private fun mockKeyStoreException(numericErrorCode: Int): KeyStoreException { + val keyStoreException = mockk() + every { keyStoreException.numericErrorCode } returns numericErrorCode + every { keyStoreException.cause } returns null + return keyStoreException + } + + private fun wrapKeyStoreException( + keyStoreException: KeyStoreException, + additionalWrapperCount: Int = 0, + ): IllegalBlockSizeException { + var cause: Throwable = keyStoreException + repeat(additionalWrapperCount) { index -> + cause = RuntimeException("wrapper-$index", cause) + } + return IllegalBlockSizeException().apply { initCause(cause) } + } + + private fun createUserAuthenticationRequiredDoFinalError( + additionalWrapperCount: Int = 0, + ): Pair { + val keyStoreException = mockKeyStoreException( + numericErrorCode = KeyStoreException.ERROR_USER_AUTHENTICATION_REQUIRED, + ) + return wrapKeyStoreException( + keyStoreException = keyStoreException, + additionalWrapperCount = additionalWrapperCount, + ) to keyStoreException + } + + private suspend fun unlockVaultWithBiometricsDoFinalError( + error: Throwable, + initVector: ByteArray? = BIOMETRIC_INIT_VECTOR, + biometricsKey: String = BIOMETRIC_UNLOCK_KEY, + ): VaultUnlockResult { + val userId = MOCK_USER_STATE.activeUserId + fakeAuthDiskSource.userState = MOCK_USER_STATE + val cipher = mockk { + every { doFinal(any()) } throws error + } + fakeAuthDiskSource.apply { + storeUserBiometricInitVector(userId = userId, iv = initVector) + storeUserBiometricUnlockKey(userId = userId, biometricsKey = biometricsKey) + storeAccountCryptographicState( + userId = userId, + accountCryptographicState = MOCK_ACCOUNT_CRYPTOGRAPHIC_STATE, + ) + } + return vaultRepository.unlockVaultWithBiometrics(cipher = cipher) + } + + private fun assertStoredBiometricsUnchanged( + initVector: ByteArray? = BIOMETRIC_INIT_VECTOR, + biometricsKey: String = BIOMETRIC_UNLOCK_KEY, + ) { + val userId = MOCK_USER_STATE.activeUserId + fakeAuthDiskSource.assertBiometricsKey(userId = userId, biometricsKey = biometricsKey) + fakeAuthDiskSource.assertBiometricInitVector(userId = userId, iv = initVector) + } + + private fun verifyUnlockAndPinDerivationNeverCalled() { + coVerify(exactly = 0) { + vaultLockManager.unlockVault(any(), any(), any(), any(), any(), any()) + } + coVerify(exactly = 0) { + pinProtectedUserKeyManager.deriveTemporaryPinProtectedUserKeyIfNecessary( + userId = any(), + ) + } + } //endregion Helper functions } +private const val BIOMETRIC_UNLOCK_KEY = "asdf1234" +private val BIOMETRIC_INIT_VECTOR = byteArrayOf(2, 2) + private val MOCK_BASE_PROFILE = AccountJson.Profile( userId = "mockId-1", email = "email", diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt index aeb474a9cb..ee0e564b53 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt @@ -1111,7 +1111,215 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test - fun `on BiometricsUnlockSuccess should display error dialog on unlockVaultWithBiometrics AuthenticationError`() { + fun `on BiometricsUnlockSuccess should display error dialog on unlockVaultWithBiometrics AuthenticationError without clearing biometrics and allow retry`() = + runTest { + val initialState = DEFAULT_STATE.copy(isBiometricEnabled = true) + mutableUserStateFlow.value = DEFAULT_USER_STATE.copy( + accounts = listOf(DEFAULT_ACCOUNT.copy(isBiometricsEnabled = true)), + ) + val viewModel = createViewModel(state = initialState) + val error = Throwable("Fail") + coEvery { + vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER) + } returns VaultUnlockResult.AuthenticationError(error = error) + + viewModel.eventFlow.test { + assertEquals(VaultUnlockEvent.PromptForBiometrics(CIPHER), awaitItem()) + + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(CIPHER)) + + val expectedErrorState = initialState.copy( + dialog = VaultUnlockState.VaultUnlockDialog.Error( + title = BitwardenString.an_error_has_occurred.asText(), + message = BitwardenString.generic_error_message.asText(), + throwable = error, + ), + ) + assertEquals(expectedErrorState, viewModel.stateFlow.value) + verify(exactly = 0) { + authRepository.clearBiometrics(userId = any()) + bitwardenCredentialManager.isUserVerified = true + } + + viewModel.trySendAction(VaultUnlockAction.DismissDialog) + assertEquals(expectedErrorState.copy(dialog = null), viewModel.stateFlow.value) + + val retryCipher = mockk() + every { authRepository.getOrCreateCipher(USER_ID) } returns retryCipher + + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockClick) + assertEquals(VaultUnlockEvent.PromptForBiometrics(retryCipher), awaitItem()) + + coEvery { + vaultRepository.unlockVaultWithBiometrics(cipher = retryCipher) + } returns VaultUnlockResult.Success + + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(retryCipher)) + assertEquals( + expectedErrorState.copy(dialog = null), + viewModel.stateFlow.value, + ) + verify(exactly = 0) { + authRepository.clearBiometrics(userId = any()) + bitwardenCredentialManager.isUserVerified = true + } + coVerify(exactly = 1) { + vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER) + vaultRepository.unlockVaultWithBiometrics(cipher = retryCipher) + } + verify { authRepository.getOrCreateCipher(USER_ID) } + } + } + + @Suppress("MaxLineLength") + @Test + fun `on BiometricsUnlockSuccess should preserve biometrics on the first Keystore authorization error and clear them on the second`() = + runTest { + val initialState = DEFAULT_STATE.copy(isBiometricEnabled = true) + mutableUserStateFlow.value = DEFAULT_USER_STATE.copy( + accounts = listOf(DEFAULT_ACCOUNT.copy(isBiometricsEnabled = true)), + ) + val viewModel = createViewModel(state = initialState) + val error = Throwable("Fail") + coEvery { + vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER) + } returns VaultUnlockResult.BiometricKeystoreAuthorizationError(error = error) + every { authRepository.clearBiometrics(userId = USER_ID) } just runs + + viewModel.eventFlow.test { + assertEquals(VaultUnlockEvent.PromptForBiometrics(CIPHER), awaitItem()) + + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(CIPHER)) + + val firstFailureState = initialState.copy( + keystoreAuthorizationFailureCount = 1, + dialog = VaultUnlockState.VaultUnlockDialog.Error( + title = BitwardenString.an_error_has_occurred.asText(), + message = BitwardenString.generic_error_message.asText(), + throwable = error, + ), + ) + assertEquals(firstFailureState, viewModel.stateFlow.value) + verify(exactly = 0) { + authRepository.clearBiometrics(userId = any()) + bitwardenCredentialManager.isUserVerified = true + } + + viewModel.trySendAction(VaultUnlockAction.DismissDialog) + assertEquals(firstFailureState.copy(dialog = null), viewModel.stateFlow.value) + + val retryCipher = mockk() + every { authRepository.getOrCreateCipher(USER_ID) } returns retryCipher + coEvery { + vaultRepository.unlockVaultWithBiometrics(cipher = retryCipher) + } returns VaultUnlockResult.BiometricKeystoreAuthorizationError(error = error) + + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockClick) + assertEquals(VaultUnlockEvent.PromptForBiometrics(retryCipher), awaitItem()) + + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(retryCipher)) + + assertEquals( + initialState.copy( + isBiometricsValid = false, + keystoreAuthorizationFailureCount = 2, + dialog = VaultUnlockState.VaultUnlockDialog.Error( + title = BitwardenString.biometrics_failed.asText(), + message = BitwardenString.biometrics_decoding_failure.asText(), + throwable = error, + ), + ), + viewModel.stateFlow.value, + ) + verify(exactly = 1) { + authRepository.clearBiometrics(userId = USER_ID) + } + verify(exactly = 0) { + bitwardenCredentialManager.isUserVerified = true + } + } + } + + @Suppress("MaxLineLength") + @Test + fun `on BiometricsUnlockSuccess should clear biometrics on a Keystore authorization error when the state already holds a prior failure`() { + val initialState = DEFAULT_STATE.copy( + isBiometricEnabled = true, + keystoreAuthorizationFailureCount = 1, + ) + mutableUserStateFlow.value = DEFAULT_USER_STATE.copy( + accounts = listOf(DEFAULT_ACCOUNT.copy(isBiometricsEnabled = true)), + ) + val viewModel = createViewModel(state = initialState) + val error = Throwable("Fail") + coEvery { + vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER) + } returns VaultUnlockResult.BiometricKeystoreAuthorizationError(error = error) + every { authRepository.clearBiometrics(userId = USER_ID) } just runs + + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(CIPHER)) + + assertEquals( + initialState.copy( + isBiometricsValid = false, + keystoreAuthorizationFailureCount = 2, + dialog = VaultUnlockState.VaultUnlockDialog.Error( + title = BitwardenString.biometrics_failed.asText(), + message = BitwardenString.biometrics_decoding_failure.asText(), + throwable = error, + ), + ), + viewModel.stateFlow.value, + ) + verify(exactly = 1) { authRepository.clearBiometrics(userId = USER_ID) } + } + + @Suppress("MaxLineLength") + @Test + fun `on BiometricsUnlockSuccess should not clear biometrics when a Keystore authorization error follows a successful unlock`() = + runTest { + val initialState = DEFAULT_STATE.copy(isBiometricEnabled = true) + mutableUserStateFlow.value = DEFAULT_USER_STATE.copy( + accounts = listOf(DEFAULT_ACCOUNT.copy(isBiometricsEnabled = true)), + ) + val viewModel = createViewModel(state = initialState) + val error = Throwable("Fail") + coEvery { + vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER) + } returns VaultUnlockResult.BiometricKeystoreAuthorizationError(error = error) + every { authRepository.clearBiometrics(userId = any()) } just runs + + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(CIPHER)) + verify(exactly = 0) { authRepository.clearBiometrics(userId = any()) } + assertEquals(1, viewModel.stateFlow.value.keystoreAuthorizationFailureCount) + + coEvery { + vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER) + } returns VaultUnlockResult.Success + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(CIPHER)) + assertEquals(initialState.copy(dialog = null), viewModel.stateFlow.value) + + coEvery { + vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER) + } returns VaultUnlockResult.BiometricKeystoreAuthorizationError(error = error) + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(CIPHER)) + + assertEquals( + initialState.copy( + keystoreAuthorizationFailureCount = 1, + dialog = VaultUnlockState.VaultUnlockDialog.Error( + title = BitwardenString.an_error_has_occurred.asText(), + message = BitwardenString.generic_error_message.asText(), + throwable = error, + ), + ), + viewModel.stateFlow.value, + ) + verify(exactly = 0) { authRepository.clearBiometrics(userId = any()) } + } + + @Test + fun `keystore authorization failure count should reset when the active user changes`() { val initialState = DEFAULT_STATE.copy(isBiometricEnabled = true) mutableUserStateFlow.value = DEFAULT_USER_STATE.copy( accounts = listOf(DEFAULT_ACCOUNT.copy(isBiometricsEnabled = true)), @@ -1120,23 +1328,29 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { val error = Throwable("Fail") coEvery { vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER) - } returns VaultUnlockResult.AuthenticationError(error = error) + } returns VaultUnlockResult.BiometricKeystoreAuthorizationError(error = error) + every { authRepository.clearBiometrics(userId = any()) } just runs + + viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(CIPHER)) + verify(exactly = 0) { authRepository.clearBiometrics(userId = any()) } + assertEquals(1, viewModel.stateFlow.value.keystoreAuthorizationFailureCount) + + val updatedUserId = "updatedUserId" + mutableUserStateFlow.value = DEFAULT_USER_STATE.copy( + activeUserId = updatedUserId, + accounts = listOf( + DEFAULT_ACCOUNT.copy( + userId = updatedUserId, + isVaultUnlocked = false, + isBiometricsEnabled = true, + ), + ), + ) + assertEquals(0, viewModel.stateFlow.value.keystoreAuthorizationFailureCount) viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(CIPHER)) - assertEquals( - initialState.copy( - dialog = VaultUnlockState.VaultUnlockDialog.Error( - title = BitwardenString.an_error_has_occurred.asText(), - message = BitwardenString.generic_error_message.asText(), - throwable = error, - ), - ), - viewModel.stateFlow.value, - ) - coVerify(exactly = 1) { - vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER) - } + verify(exactly = 0) { authRepository.clearBiometrics(userId = any()) } } @Suppress("MaxLineLength")