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

app/build.gradle.kts

@@ -56,7 +56,7 @@ dependencies {
   implementation(projects.autofillParser)
   implementation(projects.coroutineUtils)
   implementation(projects.cryptoPgpainless)
-  implementation(projects.formatCommon)
+  implementation(projects.formatCommonImpl)
   implementation(projects.passgen.diceware)
   implementation(projects.passgen.random)
   implementation(projects.uiCompose)

detekt-baselines/app.xml

@@ -14,7 +14,6 @@
     <ID>ComplexMethod:PasswordCreationActivity.kt$PasswordCreationActivity$override fun onCreate(savedInstanceState: Bundle?)</ID>
     <ID>ComplexMethod:PasswordCreationActivity.kt$PasswordCreationActivity$private fun encrypt()</ID>
     <ID>ComplexMethod:PasswordFragment.kt$PasswordFragment$private fun initializePasswordList()</ID>
-    <ID>EmptyDoWhileBlock:PasswordDialog.kt$PasswordDialog${}</ID>
     <ID>EmptyFunctionBlock:BasicBottomSheet.kt$BasicBottomSheet.&lt;no name provided&gt;${}</ID>
     <ID>EmptyFunctionBlock:ItemCreationBottomSheet.kt$ItemCreationBottomSheet.&lt;no name provided&gt;${}</ID>
     <ID>EmptyFunctionBlock:PasswordFragment.kt$PasswordFragment.&lt;no name provided&gt;.&lt;no name provided&gt;${}</ID>
@@ -58,7 +57,6 @@
     <ID>MagicNumber:SshKey.kt$SshKey.Algorithm.Ecdsa$256</ID>
     <ID>MagicNumber:SshKey.kt$SshKey.Algorithm.Rsa$3072</ID>
     <ID>MagicNumber:SshjSessionFactory.kt$SshjSession$22</ID>
-    <ID>MagicNumber:UriTotpFinder.kt$UriTotpFinder$30</ID>
     <ID>MatchingDeclarationName:AutofillViewUtils.kt$DatasetMetadata</ID>
     <ID>MaxLineLength:BaseGitActivity.kt$BaseGitActivity$"The server does not support multiple Git operations per SSH session. Please try again, a slower fallback mode will be used."</ID>
     <ID>MaxLineLength:BaseGitActivity.kt$BaseGitActivity$"WARNING: The remote host key has changed. If this is expected, please go to Git server settings and clear the saved host key."</ID>
@@ -82,7 +80,6 @@
     <ID>ReturnCount:OreoAutofillService.kt$OreoAutofillService$override fun onSaveRequest(request: SaveRequest, callback: SaveCallback)</ID>
     <ID>ReturnCount:PasswordStore.kt$PasswordStore$override fun onKeyDown(keyCode: Int, event: KeyEvent): Boolean</ID>
     <ID>ReturnCount:ShortcutHandler.kt$ShortcutHandler$fun addPinnedShortcut(item: PasswordItem, intent: Intent)</ID>
-    <ID>ReturnCount:UriTotpFinder.kt$UriTotpFinder$override fun findSecret(content: String): String?</ID>
     <ID>SpreadOperator:Api30AutofillResponseBuilder.kt$Api30AutofillResponseBuilder$(*ignoredIds.toTypedArray())</ID>
     <ID>SpreadOperator:AutofillResponseBuilder.kt$AutofillResponseBuilder$(*ignoredIds.toTypedArray())</ID>
     <ID>SpreadOperator:BreakOutOfDetached.kt$BreakOutOfDetached$( // abort the rebase git.rebase().setOperation(RebaseCommand.Operation.ABORT), *resetCommands, )</ID>

detekt-baselines/format-common-impl.xml

@@ -0,0 +1,7 @@
+<?xml version="1.0" ?>
+<SmellBaseline>
+  <ManuallySuppressedIssues></ManuallySuppressedIssues>
+  <CurrentIssues>
+    <ID>ReturnCount:UriTotpFinder.kt$UriTotpFinder$override fun findSecret(content: String): String?</ID>
+  </CurrentIssues>
+</SmellBaseline>

format-common-impl/build.gradle.kts

@@ -0,0 +1,17 @@
+plugins {
+  id("com.github.android-password-store.android-library")
+  id("com.github.android-password-store.kotlin-android")
+  id("com.github.android-password-store.kotlin-library")
+}
+
+android { namespace = "app.passwordstore.format.common.impl" }
+
+dependencies {
+  api(projects.formatCommon)
+  implementation(libs.dagger.hilt.core)
+  testImplementation(projects.coroutineUtilsTesting)
+  testImplementation(libs.bundles.testDependencies)
+  testImplementation(libs.kotlin.coroutines.test)
+  testImplementation(libs.testing.robolectric)
+  testImplementation(libs.testing.turbine)
+}

format-common-impl/src/main/kotlin/app/passwordstore/util/totp/UriTotpFinder.kt

@@ -1,15 +1,14 @@
-/*
- * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved.
- * SPDX-License-Identifier: GPL-3.0-only
- */
-
 package app.passwordstore.util.totp
 
 import android.net.Uri
 import javax.inject.Inject
 
 /** [Uri] backed TOTP URL parser. */
-class UriTotpFinder @Inject constructor() : TotpFinder {
+public class UriTotpFinder @Inject constructor() : TotpFinder {
+
+  private companion object {
+    private const val DEFAULT_TOTP_PERIOD = 30L
+  }
 
   override fun findSecret(content: String): String? {
     content.split("\n".toRegex()).forEach { line ->
@@ -28,7 +27,7 @@ class UriTotpFinder @Inject constructor() : TotpFinder {
   }
 
   override fun findPeriod(content: String): Long {
-    return getQueryParameter(content, "period")?.toLongOrNull() ?: 30
+    return getQueryParameter(content, "period")?.toLongOrNull() ?: DEFAULT_TOTP_PERIOD
   }
 
   override fun findAlgorithm(content: String): String {

format-common-impl/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt

@@ -9,7 +9,7 @@ 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 app.passwordstore.util.totp.UriTotpFinder
 import java.util.Locale
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -21,15 +21,20 @@ import kotlin.time.ExperimentalTime
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.runTest
 import org.junit.Rule
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
 
 @OptIn(ExperimentalCoroutinesApi::class, ExperimentalTime::class)
+@RunWith(RobolectricTestRunner::class)
 class PasswordEntryTest {
 
   @get:Rule val coroutineTestRule: CoroutineTestRule = CoroutineTestRule()
+  private val totpFinder = UriTotpFinder()
+
   private fun makeEntry(content: String, clock: UserClock = fakeClock) =
     PasswordEntry(
       clock,
-      testFinder,
+      totpFinder,
       content.encodeToByteArray(),
     )
 
@@ -144,7 +149,7 @@ class PasswordEntryTest {
   }
 
   /**
-   * Same as [testGeneratesOtpFromTotpUri], but advances the clock by 5 seconds. This exercises the
+   * Same as [generatesOtpFromTotpUri], 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.
    */
@@ -199,28 +204,5 @@ class PasswordEntryTest {
       "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"
-        }
-      }
   }
 }

format-common-impl/src/test/kotlin/app/passwordstore/util/totp/OtpTest.kt

@@ -137,7 +137,7 @@ class OtpTest {
       )
     val paddedOtp =
       generateOtp(
-        1593367171420 / (1000 * 30),
+        counter = 1593367171420 / (1000 * 30),
         secret = "ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA====",
       )
 
@@ -145,4 +145,24 @@ class OtpTest {
     assertNotNull(paddedOtp)
     assertEquals(unpaddedOtp, paddedOtp)
   }
+
+  @Test
+  fun generateSteamTotp() {
+    val issuerOtp =
+      generateOtp(
+        counter = 48297900 / (1000 * 30),
+        secret = "STK7746GVMCHMNH5FBIAQXGPV3I7ZHRG",
+        issuer = "Steam",
+      )
+    val digitsOtp =
+      generateOtp(
+        counter = 48297900 / (1000 * 30),
+        secret = "STK7746GVMCHMNH5FBIAQXGPV3I7ZHRG",
+        digits = "s",
+      )
+    assertNotNull(issuerOtp)
+    assertNotNull(digitsOtp)
+    assertEquals("6M3CT", issuerOtp)
+    assertEquals("6M3CT", digitsOtp)
+  }
 }

format-common/build.gradle.kts

@@ -9,14 +9,10 @@ plugins {
 }
 
 dependencies {
+  api(libs.thirdparty.kotlinResult)
   implementation(projects.coroutineUtils)
   implementation(libs.androidx.annotation)
   implementation(libs.dagger.hilt.core)
   implementation(libs.thirdparty.commons.codec)
-  implementation(libs.thirdparty.kotlinResult)
   implementation(libs.kotlin.coroutines.core)
-  testImplementation(projects.coroutineUtilsTesting)
-  testImplementation(libs.bundles.testDependencies)
-  testImplementation(libs.kotlin.coroutines.test)
-  testImplementation(libs.testing.turbine)
 }

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:",

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)

settings.gradle.kts

@@ -178,6 +178,8 @@ include("crypto-pgpainless")
 
 include("format-common")
 
+include("format-common-impl")
+
 include("passgen:diceware")
 
 include("passgen:random")