From 29371bdcb52dd926ef6b575b85c228581867ecdd Mon Sep 17 00:00:00 2001
From: David Perez <david@livefront.com>
Date: Thu, 20 Mar 2025 14:15:44 -0500
Subject: [PATCH] PM-19389: Handle encoding error when migrating biometric key
 (#4900)

---
 .../vault/repository/VaultRepositoryImpl.kt   | 45 ++++++++++++-------
 .../vault/repository/VaultRepositoryTest.kt   | 28 ++++++++++++
 2 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt
index 3a970cd185..f93b045de3 100644
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt
@@ -557,31 +557,46 @@ class VaultRepositoryImpl(
                 error = MissingPropertyException("Biometric key"),
             )
         val iv = authDiskSource.getUserBiometricInitVector(userId = userId)
+        val decryptedUserKey = iv
+            ?.let {
+                try {
+                    cipher
+                        .doFinal(biometricsKey.toByteArray(Charsets.ISO_8859_1))
+                        .decodeToString()
+                } catch (e: GeneralSecurityException) {
+                    return VaultUnlockResult.BiometricDecodingError(error = e)
+                }
+            }
+            ?: biometricsKey
+        val encryptedBiometricsKey = if (iv == null) {
+            // Attempting to setup an encrypted pin before unlocking, if this fails we send back
+            // the biometrics error and users will need to sign in another way and re-setup
+            // biometrics.
+            try {
+                cipher
+                    .doFinal(biometricsKey.encodeToByteArray())
+                    .toString(Charsets.ISO_8859_1)
+            } catch (e: GeneralSecurityException) {
+                return VaultUnlockResult.BiometricDecodingError(error = e)
+            }
+        } else {
+            null
+        }
         return this
             .unlockVaultForUser(
                 userId = userId,
                 initUserCryptoMethod = InitUserCryptoMethod.DecryptedKey(
-                    decryptedUserKey = iv
-                        ?.let {
-                            try {
-                                cipher
-                                    .doFinal(biometricsKey.toByteArray(Charsets.ISO_8859_1))
-                                    .decodeToString()
-                            } catch (e: GeneralSecurityException) {
-                                return VaultUnlockResult.BiometricDecodingError(error = e)
-                            }
-                        }
-                        ?: biometricsKey,
+                    decryptedUserKey = decryptedUserKey,
                 ),
             )
             .also {
                 if (it is VaultUnlockResult.Success) {
-                    if (iv == null) {
+                    encryptedBiometricsKey?.let {
+                        // If this key is present, we store it and the associated IV for future use
+                        // since we want to migrate the user to a more secure form of biometrics.
                         authDiskSource.storeUserBiometricUnlockKey(
                             userId = userId,
-                            biometricsKey = cipher
-                                .doFinal(biometricsKey.encodeToByteArray())
-                                .toString(Charsets.ISO_8859_1),
+                            biometricsKey = it,
                         )
                         authDiskSource.storeUserBiometricInitVector(userId = userId, iv = cipher.iv)
                     }
diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt
index add5d07eed..0879d2828d 100644
--- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt
@@ -131,6 +131,7 @@ import org.junit.jupiter.api.Test
 import retrofit2.HttpException
 import java.io.File
 import java.net.UnknownHostException
+import java.security.GeneralSecurityException
 import java.security.MessageDigest
 import java.time.Clock
 import java.time.Instant
@@ -1296,6 +1297,33 @@ class VaultRepositoryTest {
             }
         }
 
+    @Suppress("MaxLineLength")
+    @Test
+    fun `unlockVaultWithBiometrics with failure to encode biometrics key should return BiometricDecodingError`() =
+        runTest {
+            val userId = MOCK_USER_STATE.activeUserId
+            val biometricsKey = "asdf1234"
+            val error = GeneralSecurityException()
+            val cipher = mockk<Cipher> {
+                every { doFinal(any()) } throws error
+            }
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
+            fakeAuthDiskSource.storeUserBiometricUnlockKey(
+                userId = userId,
+                biometricsKey = biometricsKey,
+            )
+
+            val result = vaultRepository.unlockVaultWithBiometrics(cipher = cipher)
+
+            assertEquals(
+                VaultUnlockResult.BiometricDecodingError(error = error),
+                result,
+            )
+            coVerify(exactly = 0) {
+                vaultSdkSource.derivePinProtectedUserKey(any(), any())
+            }
+        }
+
     @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`() =