From 41b7ca2b68a13fa2601ba360c0eadb760ca1596f Mon Sep 17 00:00:00 2001 From: David Perez Date: Wed, 25 Mar 2026 09:07:25 -0500 Subject: [PATCH] BWA-238: bug: Send additional cipher data for Authenticator Sync (#6714) --- .../AuthenticatorBridgeRepositoryImpl.kt | 22 +++++-- .../AuthenticatorBridgeRepositoryTest.kt | 30 ++++++++-- .../util/SharedAccountDataExtensions.kt | 20 +++++-- .../feature/itemlisting/ItemListingScreen.kt | 1 + .../repository/AuthenticatorRepositoryTest.kt | 6 +- .../model/SharedAccountData.kt | 25 +++++++- .../model/SharedAccountDataJson.kt | 47 ++++++++++++--- .../util/EncryptionUtils.kt | 59 +++++++++++++++++-- .../util/EncryptionUtilTest.kt | 11 +++- 9 files changed, 188 insertions(+), 33 deletions(-) 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 9a4a7624d0..b5787a546d 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 @@ -48,6 +48,7 @@ class AuthenticatorBridgeRepositoryImpl( } } + @Suppress("LongMethod") override suspend fun getSharedAccounts(): SharedAccountData { return authDiskSource .userState @@ -88,7 +89,7 @@ class AuthenticatorBridgeRepositoryImpl( } // Vault is unlocked, query vault disk source for totp logins: - val totpUris = vaultDiskSource + val cipherData = vaultDiskSource .getTotpCiphers(userId = userId) // Filter out any deleted and archived ciphers. .filter { it.deletedDate == null && it.archivedDate == null } @@ -97,10 +98,23 @@ class AuthenticatorBridgeRepositoryImpl( .decryptCipher(userId = userId, cipher = it.toEncryptedSdkCipher()) .getOrNull() ?.let { decryptedCipher -> - val rawTotp = decryptedCipher.login?.totp + val cipherId = decryptedCipher.id ?: return@let null val cipherName = decryptedCipher.name val username = decryptedCipher.login?.username - rawTotp.sanitizeTotpUri(issuer = cipherName, username = username) + decryptedCipher.login?.totp?.let { rawTotp -> + SharedAccountData.CipherData( + uri = rawTotp, + // TODO: PM-34085 Remove the legacyUri. + legacyUri = rawTotp.sanitizeTotpUri( + issuer = cipherName, + username = username, + ), + id = cipherId, + name = cipherName, + username = username, + isFavorite = decryptedCipher.favorite, + ) + } } } @@ -116,7 +130,7 @@ class AuthenticatorBridgeRepositoryImpl( .environmentUrlData .toEnvironmentUrlsOrDefault() .label, - totpUris = totpUris, + cipherData = cipherData, ) } .let(::SharedAccountData) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryTest.kt index 2bcc4a3840..b5ccde9408 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryTest.kt @@ -540,25 +540,47 @@ private val USER_1_ENCRYPTED_SDK_TOTP_CIPHER = mockk() private val USER_2_ENCRYPTED_SDK_TOTP_CIPHER = mockk() private val USER_1_DECRYPTED_TOTP_CIPHER = mockk { + every { id } returns "id1" every { login?.totp } returns "totp" every { login?.username } returns "username" every { name } returns "cipher1" + every { favorite } returns true } private val USER_2_DECRYPTED_TOTP_CIPHER = mockk { + every { id } returns "id2" every { login?.totp } returns "totp" every { login?.username } returns "username" every { name } returns "cipher1" + every { favorite } returns false } -private val USER_1_EXPECTED_TOTP_LIST = listOf("totp") -private val USER_2_EXPECTED_TOTP_LIST = listOf("totp") +private val USER_1_EXPECTED_CIPHER_LIST = listOf( + SharedAccountData.CipherData( + uri = "totp", + legacyUri = "totp", + id = "id1", + name = "cipher1", + username = "username", + isFavorite = true, + ), +) +private val USER_2_EXPECTED_CIPHER_LIST = listOf( + SharedAccountData.CipherData( + uri = "totp", + legacyUri = "totp", + id = "id2", + name = "cipher1", + username = "username", + isFavorite = false, + ), +) private val USER_1_SHARED_ACCOUNT = SharedAccountData.Account( userId = ACCOUNT_JSON_1.profile.userId, name = ACCOUNT_JSON_1.profile.name, email = ACCOUNT_JSON_1.profile.email, environmentLabel = Environment.Us.label, - totpUris = USER_1_EXPECTED_TOTP_LIST, + cipherData = USER_1_EXPECTED_CIPHER_LIST, ) private val USER_2_SHARED_ACCOUNT = SharedAccountData.Account( @@ -566,7 +588,7 @@ private val USER_2_SHARED_ACCOUNT = SharedAccountData.Account( name = ACCOUNT_JSON_2.profile.name, email = ACCOUNT_JSON_2.profile.email, environmentLabel = Environment.Us.label, - totpUris = USER_2_EXPECTED_TOTP_LIST, + cipherData = USER_2_EXPECTED_CIPHER_LIST, ) private val USER_1_CIPHERS = listOf( diff --git a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/util/SharedAccountDataExtensions.kt b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/util/SharedAccountDataExtensions.kt index 3c77c5c1f7..7420fe11fb 100644 --- a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/util/SharedAccountDataExtensions.kt +++ b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/util/SharedAccountDataExtensions.kt @@ -10,13 +10,23 @@ import com.bitwarden.authenticatorbridge.model.SharedAccountData */ fun List.toAuthenticatorItems(): List = flatMap { sharedAccount -> - sharedAccount.totpUris.mapNotNull { totpUriString -> + sharedAccount.cipherData.mapNotNull { cipherData -> runCatching { - val uri = totpUriString.toUri() - val issuer = uri.getQueryParameter(TotpCodeManager.ISSUER_PARAM) - val label = uri.pathSegments + val uri = cipherData.uri.toUri() + val issuer = uri + .getQueryParameter(TotpCodeManager.ISSUER_PARAM) + ?.takeUnless { it.isBlank() } + ?: cipherData.name.takeUnless { + // TODO: PM-34085 The cipher name will never be blank once we + // TODO: remove the legacy support. + it.isBlank() + } + val label = uri + .pathSegments .firstOrNull() ?.removePrefix("$issuer:") + ?.takeUnless { it.isBlank() } + ?: cipherData.username AuthenticatorItem( source = AuthenticatorItem.Source.Shared( @@ -25,7 +35,7 @@ fun List.toAuthenticatorItems(): List { item(key = "shared_codes_error") { + Spacer(modifier = Modifier.height(height = 8.dp)) Text( text = stringResource(BitwardenString.shared_codes_error), color = BitwardenTheme.colorScheme.text.secondary, diff --git a/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryTest.kt b/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryTest.kt index 8520501083..c9ff924d33 100644 --- a/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryTest.kt +++ b/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryTest.kt @@ -4,7 +4,6 @@ import android.net.Uri import app.cash.turbine.test import com.bitwarden.authenticator.data.authenticator.datasource.disk.util.FakeAuthenticatorDiskSource import com.bitwarden.authenticator.data.authenticator.datasource.entity.createMockAuthenticatorItemEntity -import com.bitwarden.data.manager.file.FileManager import com.bitwarden.authenticator.data.authenticator.manager.TotpCodeManager import com.bitwarden.authenticator.data.authenticator.manager.model.VerificationCodeItem import com.bitwarden.authenticator.data.authenticator.repository.model.AuthenticatorItem @@ -25,6 +24,7 @@ import com.bitwarden.authenticatorbridge.model.SharedAccountData import com.bitwarden.core.data.manager.dispatcher.FakeDispatcherManager import com.bitwarden.core.data.repository.model.DataState import com.bitwarden.core.data.util.mockBuilder +import com.bitwarden.data.manager.file.FileManager import com.bitwarden.ui.platform.model.FileData import io.mockk.coEvery import io.mockk.coVerify @@ -182,7 +182,7 @@ class AuthenticatorRepositoryTest { name = null, email = "test@test.com", environmentLabel = "bitwarden.com", - totpUris = emptyList(), + cipherData = emptyList(), ), ) authenticatorRepository.firstTimeAccountSyncFlow.test { @@ -203,7 +203,7 @@ class AuthenticatorRepositoryTest { name = null, email = "test@test.com", environmentLabel = "bitwarden.com", - totpUris = emptyList(), + cipherData = emptyList(), ), ) authenticatorRepository.firstTimeAccountSyncFlow.test { diff --git a/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/model/SharedAccountData.kt b/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/model/SharedAccountData.kt index c5f155ce1e..2bed51ee4a 100644 --- a/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/model/SharedAccountData.kt +++ b/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/model/SharedAccountData.kt @@ -16,14 +16,33 @@ data class SharedAccountData( * @param name name associated with the account. * @param email email associated with the account. * @param environmentLabel environment associated with the account. - * @param totpUris list of totp URIs associated with the account. - * @param lastSyncTime the last time the account was synced by the main Bitwarden app. + * @param cipherData list of ciphers containing totp URIs associated with the account. */ data class Account( val userId: String, val name: String?, val email: String, val environmentLabel: String, - val totpUris: List, + val cipherData: List, + ) + + /** + * Models a single shared cipher containing a totp. + * + * @param uri the totp URI. + * @param legacyUri the legacy totp URI. + * @param id unique ID for this item. + * @param name the name of the cipher. + * @param username the username of the item. + * @param isFavorite indicates that this item is a favorite. + */ + data class CipherData constructor( + val uri: String, + // TODO: PM-34085 Remove the legacyUri. + val legacyUri: String?, + val id: String, + val name: String, + val username: String?, + val isFavorite: Boolean, ) } diff --git a/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/model/SharedAccountDataJson.kt b/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/model/SharedAccountDataJson.kt index 6da8744d63..06d822ba68 100644 --- a/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/model/SharedAccountDataJson.kt +++ b/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/model/SharedAccountDataJson.kt @@ -8,7 +8,7 @@ import kotlinx.serialization.Serializable * * For domain level model, see [SharedAccountData]. * - * @param accounts The list of shared accounts. + * @property accounts The list of shared accounts. */ @Serializable internal data class SharedAccountDataJson( @@ -19,12 +19,13 @@ internal data class SharedAccountDataJson( /** * Models a single shared account in a serializable format. * - * @param userId user ID tied to the account. - * @param name name associated with the account. - * @param email email associated with the account. - * @param environmentLabel environment associated with the account. - * @param totpUris list of totp URIs associated with the account. - * @param lastSyncTime the last time the account was synced by the main Bitwarden app. + * @property userId user ID tied to the account. + * @property name name associated with the account. + * @property email email associated with the account. + * @property environmentLabel environment associated with the account. + * @property totpUris list of totp URIs associated with the account. This is for legacy use + * only. + * @property cipherData list of ciphers containing totp URIs associated with the account. */ @Serializable data class AccountJson( @@ -40,7 +41,39 @@ internal data class SharedAccountDataJson( @SerialName("environmentLabel") val environmentLabel: String, + // TODO: PM-34085 Remove totpUris. @SerialName("totpUris") val totpUris: List, + + // TODO: PM-34085 Make cipherData nonnull. + @SerialName("cipherData") + val cipherData: List?, + ) + + /** + * Models a single shared cipher in a serializable format. + * + * @property uri the totp URI associated with this cipher. + * @property id the ID of this cipher. + * @property name the name of this cipher. + * @property username the username for this cipher. + * @property isFavorite indicates if this cipher is favorited. + */ + @Serializable + data class CipherJson( + @SerialName("uri") + val uri: String, + + @SerialName("id") + val id: String, + + @SerialName("cipherName") + val name: String, + + @SerialName("username") + val username: String?, + + @SerialName("isFavorite") + val isFavorite: Boolean, ) } diff --git a/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/util/EncryptionUtils.kt b/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/util/EncryptionUtils.kt index 8d8d390d89..d4d9bb46f1 100644 --- a/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/util/EncryptionUtils.kt +++ b/authenticatorbridge/src/main/kotlin/com/bitwarden/authenticatorbridge/util/EncryptionUtils.kt @@ -19,6 +19,8 @@ import javax.crypto.KeyGenerator import javax.crypto.SecretKey import javax.crypto.spec.IvParameterSpec import javax.crypto.spec.SecretKeySpec +import kotlin.uuid.ExperimentalUuidApi +import kotlin.uuid.Uuid /** * Generate a symmetric [SecretKey] that will used for encrypting IPC traffic. @@ -167,37 +169,82 @@ private fun generateCipher(): Cipher = /** * Helper function for converting [SharedAccountData] to a serializable [SharedAccountDataJson]. */ -private fun SharedAccountData.toJsonModel() = SharedAccountDataJson( +private fun SharedAccountData.toJsonModel(): SharedAccountDataJson = SharedAccountDataJson( accounts = this.accounts.map { account -> SharedAccountDataJson.AccountJson( userId = account.userId, name = account.name, environmentLabel = account.environmentLabel, email = account.email, - totpUris = account.totpUris, + // TODO: PM-34085 Remove totpUris from this model. + totpUris = account.cipherData.mapNotNull { it.legacyUri }, + cipherData = account.cipherData.map { it.toJsonModel() }, ) }, ) +/** + * Helper function for converting [SharedAccountData.CipherData] to a + * [SharedAccountDataJson.CipherJson]. + */ +private fun SharedAccountData.CipherData.toJsonModel(): SharedAccountDataJson.CipherJson = + SharedAccountDataJson.CipherJson( + uri = this.uri, + id = this.id, + name = this.name, + username = this.username, + isFavorite = this.isFavorite, + ) + /** * Helper function for converting [SharedAccountDataJson] to a [SharedAccountData]. */ -private fun SharedAccountDataJson.toDomainModel() = SharedAccountData( +private fun SharedAccountDataJson.toDomainModel(): SharedAccountData = SharedAccountData( accounts = this.accounts.map { account -> SharedAccountData.Account( userId = account.userId, name = account.name, environmentLabel = account.environmentLabel, email = account.email, - totpUris = account.totpUris, + cipherData = account.cipherData?.map { it.toCipherData() } + // TODO: PM-34085 Remove this mapping from totpUris. + ?: account.totpUris.map { it.toCipherData() }, ) }, ) +/** + * Helper function for converting [SharedAccountDataJson.CipherJson] to a + * [SharedAccountData.CipherData]. + */ +private fun SharedAccountDataJson.CipherJson.toCipherData(): SharedAccountData.CipherData = + SharedAccountData.CipherData( + uri = this.uri, + legacyUri = this.uri, + id = this.id, + name = this.name, + username = this.username, + isFavorite = this.isFavorite, + ) + +/** + * Helper function for converting [String] URI to a [SharedAccountData.CipherData]. + * TODO: PM-34085 Remove this function, it is only needed for legacy support. + */ +@OptIn(ExperimentalUuidApi::class) +private fun String.toCipherData(): SharedAccountData.CipherData = SharedAccountData.CipherData( + uri = this, + legacyUri = this, + id = Uuid.random().toString(), + name = "", + username = null, + isFavorite = false, +) + /** * Helper function for converting [AddTotpLoginItemDataJson] to a [AddTotpLoginItemData]. */ -private fun AddTotpLoginItemDataJson.toDomainModel() = AddTotpLoginItemData( +private fun AddTotpLoginItemDataJson.toDomainModel(): AddTotpLoginItemData = AddTotpLoginItemData( totpUri = totpUri, ) @@ -205,6 +252,6 @@ private fun AddTotpLoginItemDataJson.toDomainModel() = AddTotpLoginItemData( * Helper function for converting [AddTotpLoginItemData] to a serializable * [AddTotpLoginItemDataJson]. */ -private fun AddTotpLoginItemData.toJsonModel() = AddTotpLoginItemDataJson( +private fun AddTotpLoginItemData.toJsonModel(): AddTotpLoginItemDataJson = AddTotpLoginItemDataJson( totpUri = totpUri, ) diff --git a/authenticatorbridge/src/test/kotlin/com/bitwarden/authenticatorbridge/util/EncryptionUtilTest.kt b/authenticatorbridge/src/test/kotlin/com/bitwarden/authenticatorbridge/util/EncryptionUtilTest.kt index db9e991587..f491c5a0fe 100644 --- a/authenticatorbridge/src/test/kotlin/com/bitwarden/authenticatorbridge/util/EncryptionUtilTest.kt +++ b/authenticatorbridge/src/test/kotlin/com/bitwarden/authenticatorbridge/util/EncryptionUtilTest.kt @@ -173,7 +173,16 @@ private val SHARED_ACCOUNT_DATA = SharedAccountData( name = "Johnny Appleseed", email = "johnyapples@test.com", environmentLabel = "bitwarden.com", - totpUris = listOf("test.com"), + cipherData = listOf( + SharedAccountData.CipherData( + uri = "test.com", + legacyUri = "test.com", + id = "1234", + name = "test", + username = null, + isFavorite = false, + ), + ), ), ), )