mirror of
https://github.com/bitwarden/android.git
synced 2026-05-18 05:55:39 -05:00
PM-34085: Remove the Authenticator Sync backwards compatibility
This commit is contained in:
@@ -12,7 +12,6 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
|
||||
import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountJson
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.toAccountCryptographicState
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams
|
||||
import com.x8bit.bitwarden.data.platform.repository.util.sanitizeTotpUri
|
||||
import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource
|
||||
import com.x8bit.bitwarden.data.vault.datasource.sdk.ScopedVaultSdkSource
|
||||
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.InitializeCryptoResult
|
||||
@@ -104,11 +103,6 @@ class AuthenticatorBridgeRepositoryImpl(
|
||||
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,
|
||||
|
||||
@@ -1,75 +0,0 @@
|
||||
package com.x8bit.bitwarden.data.platform.repository.util
|
||||
|
||||
import java.net.URLEncoder
|
||||
|
||||
private const val OTPAUTH_PREFIX = "otpauth://totp/"
|
||||
private const val STEAM_PREFIX = "steam://"
|
||||
|
||||
/**
|
||||
* Utility for ensuring that a given TOTP string is a properly formatted otpauth:// or steam:// URI.
|
||||
* If the input TOTP is already a valid URI, it is returned as-is.
|
||||
* If the TOTP is manually entered and does not follow the URI format,
|
||||
* this function reconstructs it using the provided issuer and username.
|
||||
*
|
||||
* Uses this as a guide for format
|
||||
* https://github.com/google/google-authenticator/wiki/Key-Uri-Format
|
||||
*
|
||||
* Replace spaces (+) with %20, and encode the label and issuer (per the above link)
|
||||
* https://datatracker.ietf.org/doc/html/rfc5234
|
||||
* */
|
||||
fun String?.sanitizeTotpUri(
|
||||
issuer: String?,
|
||||
username: String?,
|
||||
): String? {
|
||||
if (this.isNullOrBlank()) return null
|
||||
|
||||
return if (this.startsWith(OTPAUTH_PREFIX) || this.startsWith(STEAM_PREFIX)) {
|
||||
// ✅ Already a valid TOTP or Steam URI, return as-is.
|
||||
this
|
||||
} else {
|
||||
// ❌ Manually entered secret, reconstruct as otpauth://totp/ URI.
|
||||
|
||||
// Trim spaces from issuer and username
|
||||
val trimmedIssuer = issuer
|
||||
?.trim()
|
||||
?.takeIf { it.isNotEmpty() }
|
||||
|
||||
val trimmedUsername = username
|
||||
?.trim()
|
||||
?.takeIf { it.isNotEmpty() }
|
||||
|
||||
// Determine raw label correctly (avoid empty `:` issue)
|
||||
val rawLabel = if (trimmedIssuer != null && trimmedUsername != null) {
|
||||
"$trimmedIssuer:$trimmedUsername"
|
||||
} else {
|
||||
trimmedUsername
|
||||
}
|
||||
|
||||
// Encode label only if it's not empty
|
||||
val encodedLabel = rawLabel
|
||||
?.let {
|
||||
URLEncoder
|
||||
.encode(it, "UTF-8")
|
||||
.replace("+", "%20")
|
||||
}
|
||||
.orEmpty()
|
||||
|
||||
// Encode issuer separately for the query parameter
|
||||
val encodedIssuer = trimmedIssuer?.let {
|
||||
URLEncoder
|
||||
.encode(it, "UTF-8")
|
||||
.replace("+", "%20")
|
||||
}
|
||||
|
||||
// Construct the issuer query parameter.
|
||||
val issuerParameter = encodedIssuer
|
||||
?.let { "&issuer=$it" }
|
||||
.orEmpty()
|
||||
|
||||
// Remove spaces from the manually entered secret
|
||||
val sanitizedSecret = this.filterNot { it.isWhitespace() }
|
||||
|
||||
// Construct final TOTP URI
|
||||
"$OTPAUTH_PREFIX$encodedLabel?secret=$sanitizedSecret$issuerParameter"
|
||||
}
|
||||
}
|
||||
@@ -19,7 +19,6 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountJson
|
||||
import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountTokensJson
|
||||
import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson
|
||||
import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource
|
||||
import com.x8bit.bitwarden.data.platform.repository.util.sanitizeTotpUri
|
||||
import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource
|
||||
import com.x8bit.bitwarden.data.vault.datasource.sdk.ScopedVaultSdkSource
|
||||
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.InitializeCryptoResult
|
||||
@@ -166,10 +165,6 @@ class AuthenticatorBridgeRepositoryTest {
|
||||
coEvery {
|
||||
scopedVaultSdkSource.decryptCipher(USER_2_ID, USER_2_ENCRYPTED_SDK_TOTP_CIPHER)
|
||||
} returns USER_2_DECRYPTED_TOTP_CIPHER.asSuccess()
|
||||
mockkStatic(String::sanitizeTotpUri)
|
||||
every {
|
||||
any<String>().sanitizeTotpUri(any(), any())
|
||||
} returns "totp"
|
||||
}
|
||||
|
||||
@AfterEach
|
||||
@@ -178,7 +173,6 @@ class AuthenticatorBridgeRepositoryTest {
|
||||
unmockkStatic(
|
||||
SyncResponseJson.Cipher::toEncryptedSdkCipher,
|
||||
EnvironmentUrlDataJson::toEnvironmentUrlsOrDefault,
|
||||
String::sanitizeTotpUri,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -557,7 +551,6 @@ private val USER_2_DECRYPTED_TOTP_CIPHER = mockk<CipherView> {
|
||||
private val USER_1_EXPECTED_CIPHER_LIST = listOf(
|
||||
SharedAccountData.CipherData(
|
||||
uri = "totp",
|
||||
legacyUri = "totp",
|
||||
id = "id1",
|
||||
name = "cipher1",
|
||||
username = "username",
|
||||
@@ -567,7 +560,6 @@ private val USER_1_EXPECTED_CIPHER_LIST = listOf(
|
||||
private val USER_2_EXPECTED_CIPHER_LIST = listOf(
|
||||
SharedAccountData.CipherData(
|
||||
uri = "totp",
|
||||
legacyUri = "totp",
|
||||
id = "id2",
|
||||
name = "cipher1",
|
||||
username = "username",
|
||||
|
||||
@@ -1,153 +0,0 @@
|
||||
package com.x8bit.bitwarden.data.platform.repository.util
|
||||
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.Assertions.assertNull
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
class TotpUriSanitizerTest {
|
||||
|
||||
@Test
|
||||
fun `valid TOTP URI should remain unchanged`() {
|
||||
val validUri = "otpauth://totp/Company:test@test.com?secret=abcdef12345&issuer=Company"
|
||||
val result = validUri.sanitizeTotpUri("Company", "test@test.com")
|
||||
|
||||
assertEquals(validUri, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `manually entered TOTP with both issuer and account should be converted correctly`() {
|
||||
val rawTotp = "zdd3 jqnn"
|
||||
val expectedUri = "otpauth://totp/Company%3Atest%40test.com?secret=zdd3jqnn&issuer=Company"
|
||||
val result = rawTotp.sanitizeTotpUri("Company", "test@test.com")
|
||||
|
||||
assertEquals(expectedUri, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `only issuer provided should not be included in label but should be in query`() {
|
||||
val rawTotp = "secret123"
|
||||
val expectedUri = "otpauth://totp/?secret=secret123&issuer=Company"
|
||||
val result = rawTotp.sanitizeTotpUri("Company", null)
|
||||
|
||||
assertEquals(expectedUri, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `only account name provided should be included in label`() {
|
||||
val rawTotp = "secret456"
|
||||
val expectedUri = "otpauth://totp/test%40email.com?secret=secret456"
|
||||
val result = rawTotp.sanitizeTotpUri(null, "test@email.com")
|
||||
|
||||
assertEquals(expectedUri, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `both issuer and account missing should generate minimal valid URI`() {
|
||||
val rawTotp = "secret789"
|
||||
val expectedUri = "otpauth://totp/?secret=secret789"
|
||||
val result = rawTotp.sanitizeTotpUri(null, null)
|
||||
|
||||
assertEquals(expectedUri, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `extra spaces in TOTP secret should be removed`() {
|
||||
val rawTotp = "a b c d e f 1 2 3"
|
||||
val expectedUri = "otpauth://totp/Issuer%3Auser%40domain.com?secret=abcdef123&issuer=Issuer"
|
||||
val result = rawTotp.sanitizeTotpUri("Issuer", "user@domain.com")
|
||||
|
||||
assertEquals(expectedUri, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `null TOTP input should return null`() {
|
||||
val result = null.sanitizeTotpUri("Company", "test@test.com")
|
||||
assertNull(result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `empty TOTP should return null`() {
|
||||
val result = "".sanitizeTotpUri("Company", "test@test.com")
|
||||
assertNull(result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `invalid characters in issuer and account should be properly encoded`() {
|
||||
val rawTotp = "secure"
|
||||
val expected = "otpauth://totp/My%20App%3Auser%40example.com?secret=secure&issuer=My%20App"
|
||||
val result = rawTotp.sanitizeTotpUri("My App", "user@example.com")
|
||||
|
||||
assertEquals(expected, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `issuer with special characters should be encoded correctly`() {
|
||||
val rawTotp = "tokenvalue"
|
||||
val expected = "otpauth://totp/?secret=tokenvalue&issuer=Super%26Secure"
|
||||
val result = rawTotp.sanitizeTotpUri("Super&Secure", null)
|
||||
|
||||
assertEquals(expected, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `account name with special characters should be encoded correctly`() {
|
||||
val rawTotp = "secret999"
|
||||
val expected = "otpauth://totp/user%2Bname%40email.com?secret=secret999"
|
||||
val result = rawTotp.sanitizeTotpUri(null, "user+name@email.com")
|
||||
|
||||
assertEquals(expected, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `both issuer and account name empty should generate minimal valid URI`() {
|
||||
val rawTotp = "secretminimal"
|
||||
val expected = "otpauth://totp/?secret=secretminimal"
|
||||
val result = rawTotp.sanitizeTotpUri("", "")
|
||||
|
||||
assertEquals(expected, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `both issuer and account name are null should generate minimal valid URI`() {
|
||||
val rawTotp = "secretminimal"
|
||||
val expected = "otpauth://totp/?secret=secretminimal"
|
||||
val result = rawTotp.sanitizeTotpUri(null, null)
|
||||
|
||||
assertEquals(expected, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `account name with spaces should be properly encoded`() {
|
||||
val rawTotp = "secret1234"
|
||||
val expectedUri = "otpauth://totp/John%20Doe%40email.com?secret=secret1234"
|
||||
val result = rawTotp.sanitizeTotpUri(null, "John Doe@email.com")
|
||||
|
||||
assertEquals(expectedUri, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `TOTP secret with leading and trailing spaces should be sanitized`() {
|
||||
val rawTotp = " a1b2c3d4 "
|
||||
val expected = "otpauth://totp/Company%3Atest%40test.com?secret=a1b2c3d4&issuer=Company"
|
||||
val result = rawTotp.sanitizeTotpUri("Company", "test@test.com")
|
||||
|
||||
assertEquals(expected, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `issuer and account name with trailing spaces should be trimmed before encoding`() {
|
||||
val rawTotp = "secure"
|
||||
val expected = "otpauth://totp/Company%3Auser%40secure.com?secret=secure&issuer=Company"
|
||||
val result = rawTotp.sanitizeTotpUri(" Company ", " user@secure.com ")
|
||||
|
||||
assertEquals(expected, result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `valid Steam URI should remain unchanged`() {
|
||||
val validSteamUri = "steam://abcdef12345"
|
||||
val result = validSteamUri.sanitizeTotpUri("Steam", "test@test.com")
|
||||
|
||||
assertEquals(validSteamUri, result)
|
||||
}
|
||||
}
|
||||
@@ -16,11 +16,7 @@ fun List<SharedAccountData.Account>.toAuthenticatorItems(): List<AuthenticatorIt
|
||||
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()
|
||||
}
|
||||
?: cipherData.name
|
||||
val label = uri
|
||||
.pathSegments
|
||||
.firstOrNull()
|
||||
|
||||
@@ -30,16 +30,13 @@ data class SharedAccountData(
|
||||
* 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(
|
||||
data class CipherData(
|
||||
val uri: String,
|
||||
// TODO: PM-34085 Remove the legacyUri.
|
||||
val legacyUri: String?,
|
||||
val id: String,
|
||||
val name: String,
|
||||
val username: String?,
|
||||
|
||||
@@ -23,8 +23,6 @@ internal data class SharedAccountDataJson(
|
||||
* @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
|
||||
@@ -41,13 +39,8 @@ internal data class SharedAccountDataJson(
|
||||
@SerialName("environmentLabel")
|
||||
val environmentLabel: String,
|
||||
|
||||
// TODO: PM-34085 Remove totpUris.
|
||||
@SerialName("totpUris")
|
||||
val totpUris: List<String>,
|
||||
|
||||
// TODO: PM-34085 Make cipherData nonnull.
|
||||
@SerialName("cipherData")
|
||||
val cipherData: List<CipherJson>?,
|
||||
val cipherData: List<CipherJson>,
|
||||
)
|
||||
|
||||
/**
|
||||
|
||||
@@ -19,8 +19,6 @@ 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.
|
||||
@@ -176,8 +174,6 @@ private fun SharedAccountData.toJsonModel(): SharedAccountDataJson = SharedAccou
|
||||
name = account.name,
|
||||
environmentLabel = account.environmentLabel,
|
||||
email = account.email,
|
||||
// TODO: PM-34085 Remove totpUris from this model.
|
||||
totpUris = account.cipherData.mapNotNull { it.legacyUri },
|
||||
cipherData = account.cipherData.map { it.toJsonModel() },
|
||||
)
|
||||
},
|
||||
@@ -206,9 +202,7 @@ private fun SharedAccountDataJson.toDomainModel(): SharedAccountData = SharedAcc
|
||||
name = account.name,
|
||||
environmentLabel = account.environmentLabel,
|
||||
email = account.email,
|
||||
cipherData = account.cipherData?.map { it.toCipherData() }
|
||||
// TODO: PM-34085 Remove this mapping from totpUris.
|
||||
?: account.totpUris.map { it.toCipherData() },
|
||||
cipherData = account.cipherData.map { it.toCipherData() },
|
||||
)
|
||||
},
|
||||
)
|
||||
@@ -220,27 +214,12 @@ private fun SharedAccountDataJson.toDomainModel(): SharedAccountData = SharedAcc
|
||||
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].
|
||||
*/
|
||||
|
||||
@@ -176,7 +176,6 @@ private val SHARED_ACCOUNT_DATA = SharedAccountData(
|
||||
cipherData = listOf(
|
||||
SharedAccountData.CipherData(
|
||||
uri = "test.com",
|
||||
legacyUri = "test.com",
|
||||
id = "1234",
|
||||
name = "test",
|
||||
username = null,
|
||||
|
||||
Reference in New Issue
Block a user