github-starred/android
2
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2026-03-21 22:00:42 -05:00
Code Issues 144 Packages Projects Releases 116 Wiki Activity

PM-17568 - Authenticator Sync: Sometimes synced verification codes only display the TOTP Key, not Issuer/username (#4783)

Browse Source
This commit is contained in:
Phil Cappelli
2025-02-27 16:42:38 -05:00
committed by GitHub
parent 4448ab05ce
commit 3f1a6e97fd
4 changed files with 257 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
app/src/main/java/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryImpl.kt
View File

@@ -3,6 +3,7 @@ package com.x8bit.bitwarden.data.platform.repository
import com.bitwarden.authenticatorbridge.model.SharedAccountData
import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
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.VaultSdkSource
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
@@ -96,18 +97,21 @@ class AuthenticatorBridgeRepositoryImpl(
val totpUris = vaultDiskSource
.getCiphers(userId)
.first()
// Filter out any ciphers without a totp item and also deleted ciphers:
// Filter out any ciphers without a totp item and also deleted ciphers
.filter { it.login?.totp != null && it.deletedDate == null }
.mapNotNull {
// Decrypt each cipher and take just totp codes:
vaultSdkSource
val decryptedCipher = vaultSdkSource
.decryptCipher(
userId = userId,
cipher = it.toEncryptedSdkCipher(),
)
.getOrNull()
?.login
?.totp
val rawTotp = decryptedCipher?.login?.totp
val cipherName = decryptedCipher?.name
val username = decryptedCipher?.login?.username
rawTotp.sanitizeTotpUri(cipherName, username)
}
// Lock the user's vault if we unlocked it for this operation:

75
app/src/main/java/com/x8bit/bitwarden/data/platform/repository/util/TotpUriSanitizer.kt Normal file
View File

@@ -0,0 +1,75 @@
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"
}
}

24
app/src/test/java/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryTest.kt
View File

@@ -8,6 +8,7 @@ import com.bitwarden.vault.CipherView
import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.platform.repository.util.sanitizeTotpUri
import com.x8bit.bitwarden.data.platform.util.asSuccess
import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource
import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
@@ -112,12 +113,17 @@ class AuthenticatorBridgeRepositoryTest {
coEvery {
vaultSdkSource.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
fun teardown() {
confirmVerified(authRepository, vaultSdkSource, vaultRepository, vaultDiskSource)
unmockkStatic(SyncResponseJson.Cipher::toEncryptedSdkCipher)
unmockkStatic(String::sanitizeTotpUri)
}
@Test
@@ -385,31 +391,41 @@ private val USER_STATE = UserState(
private val USER_1_TOTP_CIPHER = mockk<SyncResponseJson.Cipher> {
every { login?.totp } returns "encryptedTotp1"
every { login?.username } returns "username"
every { deletedDate } returns null
every { name } returns "cipher1"
}
private val USER_1_DELETED_TOTP_CIPHER = mockk<SyncResponseJson.Cipher> {
every { login?.totp } returns "encryptedTotp1Deleted"
every { login?.username } returns "username"
every { deletedDate } returns ZonedDateTime.now()
every { name } returns "cipher1"
}
private val USER_2_TOTP_CIPHER = mockk<SyncResponseJson.Cipher> {
every { login?.totp } returns "encryptedTotp2"
every { login?.username } returns "username"
every { deletedDate } returns null
every { name } returns "cipher2"
}
private val USER_1_ENCRYPTED_SDK_TOTP_CIPHER = mockk<Cipher>()
private val USER_2_ENCRYPTED_SDK_TOTP_CIPHER = mockk<Cipher>()
private val USER_1_DECRYPTED_TOTP_CIPHER = mockk<CipherView> {
every { login?.totp } returns "totp1"
every { login?.totp } returns "totp"
every { login?.username } returns "username"
every { name } returns "cipher1"
}
private val USER_2_DECRYPTED_TOTP_CIPHER = mockk<CipherView> {
every { login?.totp } returns "totp2"
every { login?.totp } returns "totp"
every { login?.username } returns "username"
every { name } returns "cipher1"
}
private val USER_1_EXPECTED_TOTP_LIST = listOf("totp1")
private val USER_2_EXPECTED_TOTP_LIST = listOf("totp2")
private val USER_1_EXPECTED_TOTP_LIST = listOf("totp")
private val USER_2_EXPECTED_TOTP_LIST = listOf("totp")
private val USER_1_SHARED_ACCOUNT = SharedAccountData.Account(
userId = ACCOUNT_1.userId,

153
app/src/test/java/com/x8bit/bitwarden/data/platform/repository/util/TotpUriSanitizerTest.kt Normal file
View File

@@ -0,0 +1,153 @@
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)
}
}