PM-27771: Improve TOTP parsing (#6119)

This commit is contained in:
David Perez
2025-11-04 14:17:03 -06:00
committed by GitHub
parent 448ba97ae2
commit ed47ff4d18
27 changed files with 192 additions and 83 deletions

View File

@@ -242,7 +242,4 @@ fun String.prefixHttpsIfNecessary(): String =
/**
* Checks if a string is using base32 digits.
*/
fun String.isBase32(): Boolean {
val regex = ("^[A-Z2-7]+=*$").toRegex()
return regex.matches(this)
}
fun String.isBase32(): Boolean = "^[A-Za-z2-7]+=*$".toRegex().matches(this)

View File

@@ -0,0 +1,55 @@
package com.bitwarden.ui.platform.model
import android.os.Parcelable
import kotlinx.parcelize.Parcelize
/**
* Represents the data for TOTP deeplink.
*
* @property uri The raw uri as a string.
* @property issuer The issuer parameter is a string value indicating the provider or service this
* account is associated with, URL-encoded according to RFC 3986.
* @property accountName The users email address.
* @property secret The secret parameter is an arbitrary key value encoded in Base32 according to
* RFC 3548. The padding specified in RFC 3548 section 2.2 is not required and should be omitted.
* @property digits The digits parameter may have the values 6 or 8, and determines how long of a
* one-time passcode to display to the user.
* @property period The period parameter defines a period that a TOTP code will be valid for, in
* seconds.
* @property algorithm The algorithm may have the values.
*/
@Parcelize
data class TotpData(
val uri: String,
val issuer: String?,
val accountName: String?,
val secret: String,
val digits: Int,
val period: Int,
val algorithm: CryptoHashAlgorithm,
) : Parcelable {
/**
* A representation of the various cryptographic hash algorithms used by TOTP.
*/
enum class CryptoHashAlgorithm(val value: String) {
SHA_1(value = "sha1"),
SHA_256(value = "sha256"),
SHA_512(value = "sha512"),
MD_5(value = "md5"),
;
@Suppress("UndocumentedPublicClass")
companion object {
/**
* Attempts to convert the string [value] to a valid [CryptoHashAlgorithm] or null if
* a match could not be found.
*/
fun parse(
value: String?,
): CryptoHashAlgorithm? =
CryptoHashAlgorithm
.entries
.firstOrNull { it.value.equals(other = value, ignoreCase = true) }
}
}
}

View File

@@ -0,0 +1,109 @@
package com.bitwarden.ui.platform.util
import android.net.Uri
import com.bitwarden.ui.platform.base.util.isBase32
import com.bitwarden.ui.platform.model.TotpData
private const val TOTP_HOST_NAME: String = "totp"
private const val TOTP_SCHEME_NAME: String = "otpauth"
private const val PARAM_NAME_ALGORITHM: String = "algorithm"
private const val PARAM_NAME_DIGITS: String = "digits"
private const val PARAM_NAME_ISSUER: String = "issuer"
private const val PARAM_NAME_PERIOD: String = "period"
private const val PARAM_NAME_SECRET: String = "secret"
/**
* Checks if the given [Uri] contains valid data for a TOTP. The [TotpData] will be returned when
* the correct data is present or `null` if data is invalid or missing.
*/
fun Uri.getTotpDataOrNull(): TotpData? {
// Must be a "otpauth" scheme
if (!this.scheme.equals(other = TOTP_SCHEME_NAME, ignoreCase = true)) return null
// Must be a "totp" host
if (!this.host.equals(other = TOTP_HOST_NAME, ignoreCase = true)) return null
val secret = this.getSecret() ?: return null
val digits = this.getDigits() ?: return null
val period = this.getPeriod() ?: return null
val algorithm = this.getAlgorithm() ?: return null
val segments = this.pathSegments?.firstOrNull()?.split(":")
val segmentCount = segments?.size ?: 0
return TotpData(
uri = this.toString(),
issuer = this.getQueryParameter(PARAM_NAME_ISSUER)
?: segments?.firstOrNull()?.trim()?.takeIf { segmentCount > 1 },
accountName = if (segmentCount > 1) {
segments?.getOrNull(index = 1)?.trim()
} else {
segments?.firstOrNull()?.trim()
},
secret = secret,
digits = digits,
period = period,
algorithm = algorithm,
)
}
/**
* Attempts to extract the algorithm from the given totp [Uri].
*/
private fun Uri.getAlgorithm(): TotpData.CryptoHashAlgorithm? {
val algorithm = this
.getQueryParameter(PARAM_NAME_ALGORITHM)
?.trim()
?.lowercase()
return if (algorithm == null) {
// If no value was provided, then we'll default to SHA_1.
TotpData.CryptoHashAlgorithm.SHA_1
} else {
// If the value is unidentifiable, then it's invalid.
// If it's identifiable, then we return the valid value.
// We specifically do not use a `let` here, since we do not want to map an unidentified
// value to the default value.
TotpData.CryptoHashAlgorithm.parse(value = algorithm)
}
}
/**
* Attempts to extract the digits from the given totp [Uri].
*/
@Suppress("MagicNumber")
private fun Uri.getDigits(): Int? {
val digits = this.getQueryParameter(PARAM_NAME_DIGITS)?.trim()?.toIntOrNull()
return if (digits == null) {
// If no value was provided, then we'll default to 6.
6
} else if (digits < 1 || digits > 10) {
// If the value is less than 1 or greater than 10, then it's invalid.
null
} else {
// If the value is valid, then we'll return it.
digits
}
}
/**
* Attempts to extract the period from the given totp [Uri].
*/
@Suppress("MagicNumber")
private fun Uri.getPeriod(): Int? {
val period = this.getQueryParameter(PARAM_NAME_PERIOD)?.trim()?.toIntOrNull()
return if (period == null) {
// If no value was provided, then we'll default to 30.
30
} else if (period < 1) {
// If the value is less than 1, then it's invalid.
null
} else {
// If the value is valid, then we'll return it.
period
}
}
/**
* Attempts to extract the secret from the given totp [Uri].
*/
private fun Uri.getSecret(): String? =
this
.getQueryParameter(PARAM_NAME_SECRET)
?.trim()
?.takeIf { it.isNotEmpty() && it.isBase32() }

View File

@@ -0,0 +1,150 @@
package com.bitwarden.ui.platform.util
import android.net.Uri
import com.bitwarden.ui.platform.model.TotpData
import io.mockk.every
import io.mockk.mockk
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.Test
class TotpUriUtilsTest {
@Test
fun `getTotpDataOrNull with incorrect scheme returns null`() {
val uri = mockk<Uri> {
every { scheme } returns "wrong"
}
assertNull(uri.getTotpDataOrNull())
}
@Test
fun `getTotpDataOrNull with incorrect host returns null`() {
val uri = mockk<Uri> {
every { scheme } returns "otpauth"
every { host } returns "hotp"
}
assertNull(uri.getTotpDataOrNull())
}
@Test
fun `getTotpDataOrNull without secret returns null`() {
val uri = mockk<Uri> {
every { scheme } returns "otpauth"
every { host } returns "totp"
every { getQueryParameter("secret") } returns null
}
assertNull(uri.getTotpDataOrNull())
}
@Test
fun `getTotpDataOrNull with invalid secret returns null`() {
val uri = mockk<Uri> {
every { scheme } returns "otpauth"
every { host } returns "totp"
every { getQueryParameter("secret") } returns "1234567890qwertyuiop"
}
assertNull(uri.getTotpDataOrNull())
}
@Test
fun `getTotpDataOrNull with invalid digits returns null`() {
val uri = mockk<Uri> {
every { scheme } returns "otpauth"
every { host } returns "totp"
every { getQueryParameter("secret") } returns "secret"
every { getQueryParameter("digits") } returns "11"
}
assertNull(uri.getTotpDataOrNull())
}
@Test
fun `getTotpDataOrNull with invalid period returns null`() {
val uri = mockk<Uri> {
every { scheme } returns "otpauth"
every { host } returns "totp"
every { getQueryParameter("secret") } returns "secret"
every { getQueryParameter("digits") } returns "5"
every { getQueryParameter("period") } returns "0"
}
assertNull(uri.getTotpDataOrNull())
}
@Test
fun `getTotpDataOrNull with invalid algorithm returns null`() {
val uri = mockk<Uri> {
every { scheme } returns "otpauth"
every { host } returns "totp"
every { getQueryParameter("secret") } returns "secret"
every { getQueryParameter("digits") } returns "5"
every { getQueryParameter("period") } returns "10"
every { getQueryParameter("algorithm") } returns "sha22"
}
assertNull(uri.getTotpDataOrNull())
}
@Test
fun `getTotpDataOrNull with minimum required values returns TotpData with defaults`() {
val secret = "secret"
val uri = mockk<Uri> {
every { scheme } returns "otpauth"
every { host } returns "totp"
every { pathSegments } returns emptyList()
every { getQueryParameter("secret") } returns secret
every { getQueryParameter("digits") } returns null
every { getQueryParameter("issuer") } returns null
every { getQueryParameter("period") } returns null
every { getQueryParameter("algorithm") } returns null
}
val expectedResult = TotpData(
uri = uri.toString(),
issuer = null,
accountName = null,
secret = secret,
digits = 6,
period = 30,
algorithm = TotpData.CryptoHashAlgorithm.SHA_1,
)
assertEquals(expectedResult, uri.getTotpDataOrNull())
}
@Test
fun `getTotpDataOrNull with complete values returns custom TotpData`() {
val secret = "secret"
val digits = 8
val issuer = "Bitwarden"
val period = 25
val algorithm = "sha256"
val accountName = "test@bitwarden.com"
val uri = mockk<Uri> {
every { scheme } returns "otpauth"
every { host } returns "totp"
every { pathSegments } returns listOf("$issuer:$accountName")
every { getQueryParameter("secret") } returns secret
every { getQueryParameter("digits") } returns digits.toString()
every { getQueryParameter("issuer") } returns issuer
every { getQueryParameter("period") } returns period.toString()
every { getQueryParameter("algorithm") } returns algorithm
}
val expectedResult = TotpData(
uri = uri.toString(),
issuer = issuer,
accountName = accountName,
secret = secret,
digits = digits,
period = period,
algorithm = TotpData.CryptoHashAlgorithm.SHA_256,
)
assertEquals(expectedResult, uri.getTotpDataOrNull())
}
}