From 693d9f18db4651ec788046162cf92a2b044ba7d3 Mon Sep 17 00:00:00 2001
From: David Perez <david@livefront.com>
Date: Wed, 26 Mar 2025 12:29:34 -0500
Subject: [PATCH] PM-19498: Update cipher migration logic (#4921)

---
 .../data/vault/manager/CipherManagerImpl.kt   | 48 +++++++++++--------
 .../data/vault/manager/CipherManagerTest.kt   |  6 +++
 2 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt
index f7ac707d79..ddaf078723 100644
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt
@@ -474,30 +474,36 @@ class CipherManagerImpl(
         userId: String,
         cipherId: String,
     ): Result<Cipher> =
-        if (this.key == null) {
-            vaultSdkSource
-                .encryptCipher(userId = userId, cipherView = this)
-                .flatMap {
-                    ciphersService.updateCipher(
-                        cipherId = cipherId,
-                        body = it.toEncryptedNetworkCipher(),
-                    )
-                }
-                .flatMap { response ->
-                    when (response) {
-                        is UpdateCipherResponseJson.Invalid -> {
-                            IllegalStateException(response.message).asFailure()
-                        }
+        vaultSdkSource
+            .encryptCipher(userId = userId, cipherView = this)
+            .flatMap {
+                // We only migrate the cipher if the original cipher did not have a key and the
+                // new cipher does. This means the SDK created the key and migration is required.
+                if (it.key != null && this.key == null) {
+                    ciphersService
+                        .updateCipher(
+                            cipherId = cipherId,
+                            body = it.toEncryptedNetworkCipher(),
+                        )
+                        .flatMap { response ->
+                            when (response) {
+                                is UpdateCipherResponseJson.Invalid -> {
+                                    IllegalStateException(response.message).asFailure()
+                                }
 
-                        is UpdateCipherResponseJson.Success -> {
-                            vaultDiskSource.saveCipher(userId = userId, cipher = response.cipher)
-                            response.cipher.toEncryptedSdkCipher().asSuccess()
+                                is UpdateCipherResponseJson.Success -> {
+                                    vaultDiskSource.saveCipher(
+                                        userId = userId,
+                                        cipher = response.cipher,
+                                    )
+                                    response.cipher.toEncryptedSdkCipher().asSuccess()
+                                }
+                            }
                         }
-                    }
+                } else {
+                    it.asSuccess()
                 }
-        } else {
-            vaultSdkSource.encryptCipher(userId = userId, cipherView = this)
-        }
+            }
 
     private suspend fun migrateAttachments(
         userId: String,
diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt
index a3c124eec3..03a79f8918 100644
--- a/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt
@@ -1707,6 +1707,7 @@ class CipherManagerTest {
 
         val attachmentId = "mockId-1"
         val cipher = mockk<Cipher> {
+            every { key } returns "key"
             every { attachments } returns emptyList()
             every { id } returns "mockId-1"
         }
@@ -1747,6 +1748,7 @@ class CipherManagerTest {
                 every { id } returns attachmentId
             }
             val cipher = mockk<Cipher> {
+                every { key } returns "key"
                 every { attachments } returns listOf(attachment)
                 every { id } returns "mockId-1"
             }
@@ -1792,6 +1794,7 @@ class CipherManagerTest {
             every { id } returns attachmentId
         }
         val cipher = mockk<Cipher> {
+            every { key } returns "key"
             every { attachments } returns listOf(attachment)
             every { id } returns "mockId-1"
         }
@@ -1839,6 +1842,7 @@ class CipherManagerTest {
             every { id } returns attachmentId
         }
         val cipher = mockk<Cipher> {
+            every { key } returns "key"
             every { attachments } returns listOf(attachment)
             every { id } returns "mockId-1"
         }
@@ -1892,6 +1896,7 @@ class CipherManagerTest {
                 every { id } returns attachmentId
             }
             val cipher = mockk<Cipher> {
+                every { key } returns "key"
                 every { attachments } returns listOf(attachment)
                 every { id } returns "mockId-1"
             }
@@ -1967,6 +1972,7 @@ class CipherManagerTest {
                 every { id } returns attachmentId
             }
             val cipher = mockk<Cipher> {
+                every { key } returns "key"
                 every { attachments } returns listOf(attachment)
                 every { id } returns "mockId-1"
             }