From 2f034bc2372d29b4ddebf74d532279073fb3d92b Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Fri, 11 Mar 2022 01:52:39 +0530 Subject: Show remaining time in TOTP field (#1766) * Pass down remaining time for TOTPs to UI layer * format-common: switch TOTP flow to use co-operative cancelation * format-common: add a regression test for OTP duration calculation * Abstract out labels * Switch to launchIn --- .../msfjarvis/aps/data/passfile/PasswordEntry.kt | 21 +++++++++------ .../kotlin/dev/msfjarvis/aps/data/passfile/Totp.kt | 8 ++++++ .../aps/data/passfile/PasswordEntryTest.kt | 31 +++++++++++++++++++--- .../dev/msfjarvis/aps/util/time/TestClocks.kt | 6 +++++ 4 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/Totp.kt (limited to 'format-common/src') diff --git a/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntry.kt b/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntry.kt index 94149477..a0a85f3e 100644 --- a/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntry.kt +++ b/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntry.kt @@ -13,12 +13,17 @@ import dev.msfjarvis.aps.util.time.UserClock import dev.msfjarvis.aps.util.totp.Otp import dev.msfjarvis.aps.util.totp.TotpFinder import kotlin.collections.set +import kotlin.coroutines.coroutineContext +import kotlin.time.Duration.Companion.seconds +import kotlin.time.ExperimentalTime import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.isActive /** Represents a single entry in the password store. */ +@OptIn(ExperimentalTime::class) public class PasswordEntry @AssistedInject constructor( @@ -52,13 +57,13 @@ constructor( * does not have a TOTP secret, the flow will never emit. Users should call [hasTotp] before * collection to check if it is valid to collect this [Flow]. */ - public val totp: Flow = flow { + public val totp: Flow = flow { if (totpSecret != null) { - repeat(Int.MAX_VALUE) { - val (otp, remainingTime) = calculateTotp() + do { + val otp = calculateTotp() emit(otp) - delay(remainingTime) - } + delay(1000L) + } while (coroutineContext.isActive) } else { awaitCancellation() } @@ -169,17 +174,17 @@ constructor( return null } - private fun calculateTotp(): Pair { + private fun calculateTotp(): Totp { 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 % totpPeriod) + val remainingTime = (totpPeriod - ((millis / 1000) % totpPeriod)).seconds Otp.calculateCode(totpSecret!!, millis / (1000 * totpPeriod), totpAlgorithm, digits, issuer) .mapBoth( { code -> - return code to remainingTime + return Totp(code, remainingTime) }, { throwable -> throw throwable } ) diff --git a/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/Totp.kt b/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/Totp.kt new file mode 100644 index 00000000..a43cce6a --- /dev/null +++ b/format-common/src/main/kotlin/dev/msfjarvis/aps/data/passfile/Totp.kt @@ -0,0 +1,8 @@ +package dev.msfjarvis.aps.data.passfile + +import kotlin.time.Duration +import kotlin.time.ExperimentalTime + +/** Holder for a TOTP secret and the duration for which it is valid. */ +@OptIn(ExperimentalTime::class) +public data class Totp(public val value: String, public val remainingTime: Duration) diff --git a/format-common/src/test/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntryTest.kt b/format-common/src/test/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntryTest.kt index d8a1fdc2..55c62533 100644 --- a/format-common/src/test/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntryTest.kt +++ b/format-common/src/test/kotlin/dev/msfjarvis/aps/data/passfile/PasswordEntryTest.kt @@ -8,6 +8,7 @@ package dev.msfjarvis.aps.data.passfile import dev.msfjarvis.aps.test.CoroutineTestRule import dev.msfjarvis.aps.test.test2 import dev.msfjarvis.aps.util.time.TestUserClock +import dev.msfjarvis.aps.util.time.UserClock import dev.msfjarvis.aps.util.totp.TotpFinder import java.util.Locale import kotlin.test.Test @@ -15,6 +16,7 @@ import kotlin.test.assertEquals import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue +import kotlin.time.Duration.Companion.seconds import kotlin.time.ExperimentalTime import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest @@ -24,9 +26,9 @@ import org.junit.Rule class PasswordEntryTest { @get:Rule val coroutineTestRule: CoroutineTestRule = CoroutineTestRule() - private fun makeEntry(content: String) = + private fun makeEntry(content: String, clock: UserClock = fakeClock) = PasswordEntry( - fakeClock, + clock, testFinder, content.encodeToByteArray(), ) @@ -134,7 +136,26 @@ class PasswordEntryTest { val entry = makeEntry("secret\nextra\n$TOTP_URI") assertTrue(entry.hasTotp()) entry.totp.test2 { - assertEquals("818800", expectMostRecentItem()) + val otp = expectMostRecentItem() + assertEquals("818800", otp.value) + assertEquals(30.seconds, otp.remainingTime) + cancelAndIgnoreRemainingEvents() + } + } + + /** + * Same as [testGeneratesOtpFromTotpUri], but advances the clock by 5 seconds. This exercises the + * [Totp.remainingTime] calculation logic, and acts as a regression test to resolve the bug which + * blocked https://msfjarvis.dev/aps/issue/1550. + */ + @Test + fun testGeneratedOtpHasCorrectRemainingTime() = runTest { + val entry = makeEntry("secret\nextra\n$TOTP_URI", TestUserClock.withAddedSeconds(5)) + assertTrue(entry.hasTotp()) + entry.totp.test2 { + val otp = expectMostRecentItem() + assertEquals("818800", otp.value) + assertEquals(25.seconds, otp.remainingTime) cancelAndIgnoreRemainingEvents() } } @@ -144,7 +165,9 @@ class PasswordEntryTest { val entry = makeEntry(TOTP_URI) assertNull(entry.password) entry.totp.test2 { - assertEquals("818800", expectMostRecentItem()) + val otp = expectMostRecentItem() + assertEquals("818800", otp.value) + assertEquals(30.seconds, otp.remainingTime) cancelAndIgnoreRemainingEvents() } } diff --git a/format-common/src/test/kotlin/dev/msfjarvis/aps/util/time/TestClocks.kt b/format-common/src/test/kotlin/dev/msfjarvis/aps/util/time/TestClocks.kt index 5098bec9..ee74a40d 100644 --- a/format-common/src/test/kotlin/dev/msfjarvis/aps/util/time/TestClocks.kt +++ b/format-common/src/test/kotlin/dev/msfjarvis/aps/util/time/TestClocks.kt @@ -24,4 +24,10 @@ class TestUserClock(instant: Instant) : UserClock() { override fun getZone(): ZoneId = UTC override fun instant(): Instant = clock.instant() + + companion object { + fun withAddedSeconds(secondsToAdd: Long): TestUserClock { + return TestUserClock(Instant.EPOCH.plusSeconds(secondsToAdd)) + } + } } -- cgit v1.2.3