Refactor `format-common` module (#2196)

* fix: touch up `PasswordEntryTest` KDoc

* feat: add format-common-impl module

* refactor: switch app to format-common-impl

* refactor: move `format-common` tests to `format-common-impl`

* feat: add a test for Steam OTP

5 files changed, 10 insertions, 413 deletions

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 38aa4a20..d48c2ca3 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
@@ -5,6 +5,7 @@
 
 package app.passwordstore.data.passfile
 
+import androidx.annotation.VisibleForTesting
 import app.passwordstore.util.time.UserClock
 import app.passwordstore.util.totp.Otp
 import app.passwordstore.util.totp.TotpFinder
@@ -202,10 +203,11 @@ constructor(
     public fun create(bytes: ByteArray): PasswordEntry
   }
 
-  internal companion object {
+  public companion object {
 
     private const val EXTRA_CONTENT = "Extra Content"
-    internal val USERNAME_FIELDS =
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    public val USERNAME_FIELDS: Array<String> =
       arrayOf(
         "login:",
         "username:",
@@ -218,7 +220,8 @@ constructor(
         "id:",
         "identity:",
       )
-    internal val PASSWORD_FIELDS =
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    public val PASSWORD_FIELDS: Array<String> =
       arrayOf(
         "password:",
         "secret:",
diff --git a/format-common/src/main/kotlin/app/passwordstore/util/totp/Otp.kt b/format-common/src/main/kotlin/app/passwordstore/util/totp/Otp.kt
index 5abc0337..10e771fe 100644
--- a/format-common/src/main/kotlin/app/passwordstore/util/totp/Otp.kt
+++ b/format-common/src/main/kotlin/app/passwordstore/util/totp/Otp.kt
@@ -6,6 +6,7 @@
 package app.passwordstore.util.totp
 
 import com.github.michaelbull.result.Err
+import com.github.michaelbull.result.Result
 import com.github.michaelbull.result.runCatching
 import java.nio.ByteBuffer
 import java.util.Locale
@@ -14,7 +15,7 @@ import javax.crypto.spec.SecretKeySpec
 import kotlin.experimental.and
 import org.apache.commons.codec.binary.Base32
 
-internal object Otp {
+public object Otp {
 
   private val BASE_32 = Base32()
   private val STEAM_ALPHABET = "23456789BCDFGHJKMNPQRTVWXY".toCharArray()
@@ -26,13 +27,13 @@ internal object Otp {
   private const val ALPHABET_LENGTH = 26
   private const val MOST_SIGNIFICANT_BYTE = 0x7f
 
-  fun calculateCode(
+  public fun calculateCode(
     secret: String,
     counter: Long,
     algorithm: String,
     digits: String,
     issuer: String?,
-  ) = runCatching {
+  ): Result<String, Throwable> = runCatching {
     val algo = "Hmac${algorithm.uppercase(Locale.ROOT)}"
     val decodedSecret = BASE_32.decode(secret)
     val secretKey = SecretKeySpec(decodedSecret, algo)
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
deleted file mode 100644
index 2022d968..00000000
--- a/format-common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt
+++ /dev/null
@@ -1,226 +0,0 @@
-/*
- * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved.
- * SPDX-License-Identifier: GPL-3.0-only
- */
-
-package app.passwordstore.data.passfile
-
-import app.cash.turbine.test
-import app.passwordstore.test.CoroutineTestRule
-import app.passwordstore.util.time.TestUserClock
-import app.passwordstore.util.time.UserClock
-import app.passwordstore.util.totp.TotpFinder
-import java.util.Locale
-import kotlin.test.Test
-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
-import org.junit.Rule
-
-@OptIn(ExperimentalCoroutinesApi::class, ExperimentalTime::class)
-class PasswordEntryTest {
-
-  @get:Rule val coroutineTestRule: CoroutineTestRule = CoroutineTestRule()
-  private fun makeEntry(content: String, clock: UserClock = fakeClock) =
-    PasswordEntry(
-      clock,
-      testFinder,
-      content.encodeToByteArray(),
-    )
-
-  @Test
-  fun getPassword() {
-    assertEquals("fooooo", makeEntry("fooooo\nbla\n").password)
-    assertEquals("fooooo", makeEntry("fooooo\nbla").password)
-    assertEquals("fooooo", makeEntry("fooooo\n").password)
-    assertEquals("fooooo", makeEntry("fooooo").password)
-    assertEquals("", makeEntry("\nblubb\n").password)
-    assertEquals("", makeEntry("\nblubb").password)
-    assertEquals("", makeEntry("\n").password)
-    assertEquals("", makeEntry("").password)
-    for (field in PasswordEntry.PASSWORD_FIELDS) {
-      assertEquals("fooooo", makeEntry("\n$field fooooo").password)
-      assertEquals("fooooo", makeEntry("\n${field.uppercase(Locale.getDefault())} fooooo").password)
-      assertEquals("fooooo", makeEntry("GOPASS-SECRET-1.0\n$field fooooo").password)
-      assertEquals("fooooo", makeEntry("someFirstLine\nUsername: bar\n$field fooooo").password)
-    }
-  }
-
-  @Test
-  fun getExtraContent() {
-    assertEquals("bla\n", makeEntry("fooooo\nbla\n").extraContentString)
-    assertEquals("bla", makeEntry("fooooo\nbla").extraContentString)
-    assertEquals("", makeEntry("fooooo\n").extraContentString)
-    assertEquals("", makeEntry("fooooo").extraContentString)
-    assertEquals("blubb\n", makeEntry("\nblubb\n").extraContentString)
-    assertEquals("blubb", makeEntry("\nblubb").extraContentString)
-    assertEquals("blubb", makeEntry("blubb\npassword: foo").extraContentString)
-    assertEquals("blubb", makeEntry("password: foo\nblubb").extraContentString)
-    assertEquals(
-      "blubb\nusername: bar",
-      makeEntry("blubb\npassword: foo\nusername: bar").extraContentString
-    )
-    assertEquals("", makeEntry("\n").extraContentString)
-    assertEquals("", makeEntry("").extraContentString)
-  }
-
-  @Test
-  fun parseExtraContentWithoutAuth() {
-    var entry = makeEntry("username: abc\npassword: abc\ntest: abcdef")
-    assertEquals(1, entry.extraContent.size)
-    assertTrue(entry.extraContent.containsKey("test"))
-    assertEquals("abcdef", entry.extraContent["test"])
-
-    entry = makeEntry("username: abc\npassword: abc\ntest: :abcdef:")
-    assertEquals(1, entry.extraContent.size)
-    assertTrue(entry.extraContent.containsKey("test"))
-    assertEquals(":abcdef:", entry.extraContent["test"])
-
-    entry = makeEntry("username: abc\npassword: abc\ntest : ::abc:def::")
-    assertEquals(1, entry.extraContent.size)
-    assertTrue(entry.extraContent.containsKey("test"))
-    assertEquals("::abc:def::", entry.extraContent["test"])
-
-    entry = makeEntry("username: abc\npassword: abc\ntest: abcdef\ntest2: ghijkl")
-    assertEquals(2, entry.extraContent.size)
-    assertTrue(entry.extraContent.containsKey("test2"))
-    assertEquals("ghijkl", entry.extraContent["test2"])
-
-    entry = makeEntry("username: abc\npassword: abc\ntest: abcdef\n: ghijkl\n mnopqr:")
-    assertEquals(2, entry.extraContent.size)
-    assertTrue(entry.extraContent.containsKey("Extra Content"))
-    assertEquals(": ghijkl\n mnopqr:", entry.extraContent["Extra Content"])
-
-    entry = makeEntry("username: abc\npassword: abc\n:\n\n")
-    assertEquals(1, entry.extraContent.size)
-    assertTrue(entry.extraContent.containsKey("Extra Content"))
-    assertEquals(":", entry.extraContent["Extra Content"])
-  }
-
-  @Test
-  fun getUsername() {
-    for (field in PasswordEntry.USERNAME_FIELDS) {
-      assertEquals("username", makeEntry("\n$field username").username)
-      assertEquals(
-        "username",
-        makeEntry("\n${field.uppercase(Locale.getDefault())} username").username
-      )
-    }
-    assertEquals("username", makeEntry("secret\nextra\nlogin: username\ncontent\n").username)
-    assertEquals("username", makeEntry("\nextra\nusername: username\ncontent\n").username)
-    assertEquals("username", makeEntry("\nUSERNaMe:  username\ncontent\n").username)
-    assertEquals("username", makeEntry("\nlogin:    username").username)
-    assertEquals("foo@example.com", makeEntry("\nemail: foo@example.com").username)
-    assertEquals("username", makeEntry("\nidentity: username\nlogin: another_username").username)
-    assertEquals("username", makeEntry("\nLOGiN:username").username)
-    assertEquals("foo@example.com", makeEntry("pass\nmail:    foo@example.com").username)
-    assertNull(makeEntry("secret\nextra\ncontent\n").username)
-  }
-
-  @Test
-  fun hasUsername() {
-    assertNotNull(makeEntry("secret\nextra\nlogin: username\ncontent\n").username)
-    assertNull(makeEntry("secret\nextra\ncontent\n").username)
-    assertNull(makeEntry("secret\nlogin failed\n").username)
-    assertNull(makeEntry("\n").username)
-    assertNull(makeEntry("").username)
-  }
-
-  @Test
-  fun generatesOtpFromTotpUri() = runTest {
-    val entry = makeEntry("secret\nextra\n$TOTP_URI")
-    assertTrue(entry.hasTotp())
-    entry.totp.test {
-      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 generatedOtpHasCorrectRemainingTime() = runTest {
-    val entry = makeEntry("secret\nextra\n$TOTP_URI", TestUserClock.withAddedSeconds(5))
-    assertTrue(entry.hasTotp())
-    entry.totp.test {
-      val otp = expectMostRecentItem()
-      assertEquals("818800", otp.value)
-      assertEquals(25.seconds, otp.remainingTime)
-      cancelAndIgnoreRemainingEvents()
-    }
-  }
-
-  @Test
-  fun generatesOtpWithOnlyUriInFile() = runTest {
-    val entry = makeEntry(TOTP_URI)
-    assertNull(entry.password)
-    entry.totp.test {
-      val otp = expectMostRecentItem()
-      assertEquals("818800", otp.value)
-      assertEquals(30.seconds, otp.remainingTime)
-      cancelAndIgnoreRemainingEvents()
-    }
-  }
-
-  @Test
-  fun onlyLooksForUriInFirstLine() {
-    val entry = makeEntry("id:\n$TOTP_URI")
-    assertNotNull(entry.password)
-    assertTrue(entry.hasTotp())
-    assertNull(entry.username)
-  }
-
-  // https://github.com/android-password-store/Android-Password-Store/issues/1190
-  @Test
-  fun extraContentWithMultipleUsernameFields() {
-    val entry = makeEntry("pass\nuser: user\nid: id\n$TOTP_URI")
-    assertTrue(entry.extraContent.isNotEmpty())
-    assertTrue(entry.hasTotp())
-    assertNotNull(entry.username)
-    assertEquals("pass", entry.password)
-    assertEquals("user", entry.username)
-    assertEquals(mapOf("id" to "id"), entry.extraContent)
-  }
-
-  companion object {
-
-    @Suppress("MaxLineLength")
-    const val TOTP_URI =
-      "otpauth://totp/ACME%20Co:john@example.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=SHA1&digits=6&period=30"
-
-    val fakeClock = TestUserClock()
-
-    // This implementation is hardcoded for the URI above.
-    val testFinder =
-      object : TotpFinder {
-        override fun findSecret(content: String): String {
-          return "HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ"
-        }
-
-        override fun findDigits(content: String): String {
-          return "6"
-        }
-
-        override fun findPeriod(content: String): Long {
-          return 30
-        }
-
-        override fun findAlgorithm(content: String): String {
-          return "SHA1"
-        }
-        override fun findIssuer(content: String): String {
-          return "ACME Co"
-        }
-      }
-  }
-}
diff --git a/format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt b/format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt
deleted file mode 100644
index 8a860a39..00000000
--- a/format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved.
- * SPDX-License-Identifier: GPL-3.0-only
- */
-
-package app.passwordstore.util.time
-
-import java.time.Clock
-import java.time.Instant
-import java.time.ZoneId
-import java.time.ZoneOffset.UTC
-
-/**
- * Implementation of [UserClock] that is fixed to [Instant.EPOCH] for deterministic time-based tests
- */
-class TestUserClock(instant: Instant) : UserClock() {
-
-  constructor() : this(Instant.EPOCH)
-
-  private var clock = fixed(instant, UTC)
-
-  override fun withZone(zone: ZoneId): Clock = clock.withZone(zone)
-
-  override fun getZone(): ZoneId = UTC
-
-  override fun instant(): Instant = clock.instant()
-
-  companion object {
-    fun withAddedSeconds(secondsToAdd: Long): TestUserClock {
-      return TestUserClock(Instant.EPOCH.plusSeconds(secondsToAdd))
-    }
-  }
-}
diff --git a/format-common/src/test/kotlin/app/passwordstore/util/totp/OtpTest.kt b/format-common/src/test/kotlin/app/passwordstore/util/totp/OtpTest.kt
deleted file mode 100644
index bf8cd186..00000000
--- a/format-common/src/test/kotlin/app/passwordstore/util/totp/OtpTest.kt
+++ /dev/null
@@ -1,148 +0,0 @@
-/*
- * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved.
- * SPDX-License-Identifier: GPL-3.0-only
- */
-
-package app.passwordstore.util.totp
-
-import com.github.michaelbull.result.get
-import kotlin.test.Test
-import kotlin.test.assertEquals
-import kotlin.test.assertNotNull
-import kotlin.test.assertNull
-
-class OtpTest {
-
-  private fun generateOtp(
-    counter: Long,
-    secret: String = "JBSWY3DPEHPK3PXP",
-    algorithm: String = "SHA1",
-    digits: String = "6",
-    issuer: String? = null,
-  ): String? {
-    return Otp.calculateCode(secret, counter, algorithm, digits, issuer).get()
-  }
-
-  @Test
-  fun otpGeneration6Digits() {
-    assertEquals(
-      "953550",
-      generateOtp(
-        counter = 1593333298159 / (1000 * 30),
-      )
-    )
-    assertEquals(
-      "275379",
-      generateOtp(
-        counter = 1593333571918 / (1000 * 30),
-      )
-    )
-    assertEquals(
-      "867507",
-      generateOtp(
-        counter = 1593333600517 / (1000 * 57),
-      )
-    )
-  }
-
-  @Test
-  fun otpGeneration10Digits() {
-    assertEquals(
-      "0740900914",
-      generateOtp(
-        counter = 1593333655044 / (1000 * 30),
-        digits = "10",
-      )
-    )
-    assertEquals(
-      "0070632029",
-      generateOtp(
-        counter = 1593333691405 / (1000 * 30),
-        digits = "10",
-      )
-    )
-    assertEquals(
-      "1017265882",
-      generateOtp(
-        counter = 1593333728893 / (1000 * 83),
-        digits = "10",
-      )
-    )
-  }
-
-  @Test
-  fun otpGenerationIllegalInput() {
-    assertNull(
-      generateOtp(
-        counter = 10000,
-        algorithm = "SHA0",
-        digits = "10",
-      )
-    )
-    assertNull(
-      generateOtp(
-        counter = 10000,
-        digits = "a",
-      )
-    )
-    assertNull(
-      generateOtp(
-        counter = 10000,
-        algorithm = "SHA1",
-        digits = "5",
-      )
-    )
-    assertNull(
-      generateOtp(
-        counter = 10000,
-        digits = "11",
-      )
-    )
-    assertNull(
-      generateOtp(
-        counter = 10000,
-        secret = "JBSWY3DPEHPK3PXPAAAAB",
-        digits = "6",
-      )
-    )
-  }
-
-  @Test
-  fun otpGenerationUnusualSecrets() {
-    assertEquals(
-      "127764",
-      generateOtp(
-        counter = 1593367111963 / (1000 * 30),
-        secret = "JBSWY3DPEHPK3PXPAAAAAAAA",
-      )
-    )
-    assertEquals(
-      "047515",
-      generateOtp(
-        counter = 1593367171420 / (1000 * 30),
-        secret = "JBSWY3DPEHPK3PXPAAAAA",
-      )
-    )
-  }
-
-  @Test
-  fun otpGenerationUnpaddedSecrets() {
-    // Secret was generated with `echo 'string with some padding needed' | base32`
-    // We don't care for the resultant OTP's actual value, we just want both the padded and
-    // unpadded variant to generate the same one.
-    val unpaddedOtp =
-      generateOtp(
-        counter = 1593367171420 / (1000 * 30),
-        secret = "ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA",
-      )
-    val paddedOtp =
-      generateOtp(
-        1593367171420 / (1000 * 30),
-        secret = "ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA====",
-      )
-
-    assertNotNull(unpaddedOtp)
-    assertNotNull(paddedOtp)
-    assertEquals(unpaddedOtp, paddedOtp)
-  }
-}