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

app/src/main/java/dev/msfjarvis/aps/data/password/FieldItem.kt

@@ -5,31 +5,39 @@
 
 package dev.msfjarvis.aps.data.password
 
+import dev.msfjarvis.aps.data.passfile.Totp
+import kotlin.time.ExperimentalTime
+
+@OptIn(ExperimentalTime::class)
 class FieldItem(val key: String, val value: String, val action: ActionType) {
   enum class ActionType {
     COPY,
     HIDE
   }
 
-  enum class ItemType(val type: String) {
-    USERNAME("Username"),
-    PASSWORD("Password"),
-    OTP("OTP")
+  enum class ItemType(val type: String, val label: String) {
+    USERNAME("Username", "Username"),
+    PASSWORD("Password", "Password"),
+    OTP("OTP", "OTP (expires in %ds)"),
   }
 
   companion object {
 
     // Extra helper methods
-    fun createOtpField(otp: String): FieldItem {
-      return FieldItem(ItemType.OTP.type, otp, ActionType.COPY)
+    fun createOtpField(totp: Totp): FieldItem {
+      return FieldItem(
+        ItemType.OTP.label.format(totp.remainingTime.inWholeSeconds),
+        totp.value,
+        ActionType.COPY,
+      )
     }
 
     fun createPasswordField(password: String): FieldItem {
-      return FieldItem(ItemType.PASSWORD.type, password, ActionType.HIDE)
+      return FieldItem(ItemType.PASSWORD.label, password, ActionType.HIDE)
     }
 
     fun createUsernameField(username: String): FieldItem {
-      return FieldItem(ItemType.USERNAME.type, username, ActionType.COPY)
+      return FieldItem(ItemType.USERNAME.label, username, ActionType.COPY)
     }
   }
 }

app/src/main/java/dev/msfjarvis/aps/ui/adapters/FieldItemAdapter.kt

@@ -13,6 +13,7 @@ import androidx.core.content.ContextCompat
 import androidx.recyclerview.widget.RecyclerView
 import com.google.android.material.textfield.TextInputLayout
 import dev.msfjarvis.aps.R
+import dev.msfjarvis.aps.data.passfile.Totp
 import dev.msfjarvis.aps.data.password.FieldItem
 import dev.msfjarvis.aps.databinding.ItemFieldBinding
 
@@ -35,13 +36,13 @@ class FieldItemAdapter(
     return fieldItemList.size
   }
 
-  fun updateOTPCode(code: String) {
+  fun updateOTPCode(totp: Totp) {
     var otpItemPosition = -1
     fieldItemList =
       fieldItemList.mapIndexed { position, item ->
-        if (item.key.equals(FieldItem.ItemType.OTP.type, true)) {
+        if (item.key.startsWith(FieldItem.ItemType.OTP.type, true)) {
           otpItemPosition = position
-          return@mapIndexed FieldItem.createOtpField(code)
+          return@mapIndexed FieldItem.createOtpField(totp)
         }
 
         return@mapIndexed item

app/src/main/java/dev/msfjarvis/aps/ui/crypto/DecryptActivity.kt

@@ -30,8 +30,8 @@ import kotlin.time.Duration
 import kotlin.time.ExperimentalTime
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.delay
-import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
@@ -210,12 +210,7 @@ class DecryptActivity : BasePgpActivity(), OpenPgpServiceConnection.OnBound {
             binding.recyclerView.adapter = adapter
 
             if (entry.hasTotp()) {
-              lifecycleScope.launch {
-                entry
-                  .totp
-                  .onEach { code -> withContext(Dispatchers.Main) { adapter.updateOTPCode(code) } }
-                  .collect()
-              }
+              entry.totp.onEach(adapter::updateOTPCode).launchIn(lifecycleScope)
             }
           }
             .onFailure { e -> logcat(ERROR) { e.asLog() } }

app/src/main/java/dev/msfjarvis/aps/ui/crypto/DecryptActivityV2.kt

@@ -25,17 +25,18 @@ import dev.msfjarvis.aps.util.settings.PreferenceKeys
 import java.io.ByteArrayOutputStream
 import java.io.File
 import javax.inject.Inject
-import kotlin.time.Duration
+import kotlin.time.Duration.Companion.seconds
 import kotlin.time.ExperimentalTime
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.delay
-import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 
+@OptIn(ExperimentalTime::class)
 @AndroidEntryPoint
 class DecryptActivityV2 : BasePgpActivity() {
 
@@ -90,10 +91,9 @@ class DecryptActivityV2 : BasePgpActivity() {
    * Automatically finishes the activity 60 seconds after decryption succeeded to prevent
    * information leaks from stale activities.
    */
-  @OptIn(ExperimentalTime::class)
   private fun startAutoDismissTimer() {
     lifecycleScope.launch {
-      delay(Duration.seconds(60))
+      delay(60.seconds)
       finish()
     }
   }
@@ -198,12 +198,7 @@ class DecryptActivityV2 : BasePgpActivity() {
       binding.recyclerView.adapter = adapter
 
       if (entry.hasTotp()) {
-        lifecycleScope.launch {
-          entry
-            .totp
-            .onEach { code -> withContext(Dispatchers.Main) { adapter.updateOTPCode(code) } }
-            .collect()
-        }
+        entry.totp.onEach(adapter::updateOTPCode).launchIn(lifecycleScope)
       }
     }
 

app/src/main/java/dev/msfjarvis/aps/util/autofill/AutofillPreferences.kt

@@ -143,7 +143,7 @@ object AutofillPreferences {
     // Always give priority to a username stored in the encrypted extras
     val username =
       entry.username ?: directoryStructure.getUsernameFor(file) ?: context.getDefaultUsername()
-    val totp = if (entry.hasTotp()) runBlocking { entry.totp.first() } else null
+    val totp = if (entry.hasTotp()) runBlocking { entry.totp.first().value } else null
     return Credentials(username, entry.password, totp)
   }
 }

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<String> = flow {
+  public val totp: Flow<Totp> = 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<String, Long> {
+  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 }
       )

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)

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()
     }
   }

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))
+    }
+  }
 }