diff --git a/format/common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt b/format/common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt index 1618374f..f4dcc2b7 100644 --- a/format/common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt +++ b/format/common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt @@ -9,6 +9,9 @@ import androidx.annotation.VisibleForTesting import app.passwordstore.util.time.UserClock import app.passwordstore.util.totp.Otp import app.passwordstore.util.totp.TotpFinder +import com.github.michaelbull.result.Err +import com.github.michaelbull.result.Ok +import com.github.michaelbull.result.Result import com.github.michaelbull.result.mapBoth import dagger.assisted.Assisted import dagger.assisted.AssistedFactory @@ -60,14 +63,22 @@ constructor( require(totpSecret != null) { "Cannot collect this flow without a TOTP secret" } do { val otp = calculateTotp() - emit(otp) - delay(THOUSAND_MILLIS.milliseconds) + if (otp.isOk) { + emit(otp.value) + delay(THOUSAND_MILLIS.milliseconds) + } else { + throw otp.error + } } while (coroutineContext.isActive) } /** Obtain the [Totp.value] for this [PasswordEntry] at the current time. */ public val currentOtp: String - get() = calculateTotp().value + get() { + val otp = calculateTotp() + check(otp.isOk) + return otp.value.value + } /** * String representation of [extraContent] but with authentication related data such as TOTP URIs @@ -83,7 +94,14 @@ constructor( extraContentWithoutAuthData = generateExtraContentWithoutAuthData() extraContent = generateExtraContentPairs() username = findUsername() - totpSecret = totpFinder.findSecret(content) + // Verify the TOTP secret is valid and disable TOTP if not. + val secret = totpFinder.findSecret(content) + totpSecret = + if (secret != null && calculateTotp(secret).isOk) { + secret + } else { + null + } } public fun hasTotp(): Boolean { @@ -175,26 +193,21 @@ constructor( return null } - private fun calculateTotp(): Totp { + private fun calculateTotp(secret: String = totpSecret!!): Result { val digits = totpFinder.findDigits(content) val totpPeriod = totpFinder.findPeriod(content) val totpAlgorithm = totpFinder.findAlgorithm(content) val issuer = totpFinder.findIssuer(content) val millis = clock.millis() val remainingTime = (totpPeriod - ((millis / THOUSAND_MILLIS) % totpPeriod)).seconds - Otp.calculateCode( - totpSecret!!, + return Otp.calculateCode( + secret, millis / (THOUSAND_MILLIS * totpPeriod), totpAlgorithm, digits, issuer ) - .mapBoth( - { code -> - return Totp(code, remainingTime) - }, - { throwable -> throw throwable } - ) + .mapBoth({ code -> Ok(Totp(code, remainingTime)) }, ::Err) } @AssistedFactory diff --git a/format/common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt b/format/common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt index 865a13c1..95fb3e62 100644 --- a/format/common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt +++ b/format/common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt @@ -13,6 +13,7 @@ import app.passwordstore.util.totp.UriTotpFinder import java.util.Locale import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -181,6 +182,13 @@ class PasswordEntryTest { } } + // https://github.com/android-password-store/Android-Password-Store/issues/2949 + @Test + fun disablesTotpForInvalidUri() = runTest { + val entry = makeEntry("password\notpauth://totp/otp-secret?secret=") + assertFalse(entry.hasTotp()) + } + @Test fun onlyLooksForUriInFirstLine() { val entry = makeEntry("id:\n$TOTP_URI")