Refactor TOTP implementation and expand SteamGuard hacks (#1460)

* UriTotpFinder: commonize query parameter handling

* gitignore: add more IDEA files

* TotpFinder: add `findIssuer`

* PasswordEntry: don't eagerly fetch TOTP related fields

* format-common: expand SteamGuard workaround

* CHANGELOG: add SteamGuard workaround

8 files changed, 50 insertions, 31 deletions

diff --git a/.gitignore b/.gitignore
index 19fea860..6dbf3b8c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -104,6 +104,7 @@ obj/
 .idea/assetWizardSettings.xml
 .idea/gradle.xml
 .idea/jarRepositories.xml
+.idea/runConfigurations.xml
 
 # OS-specific files
 .DS_Store
diff --git a/CHANGELOG.md b/CHANGELOG.md
index de026371..66a756f1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file.
 -   Parse extra content as individual fields
 -   Improve search result filtering logic
 -   Allow pinning shortcuts directly to the launcher home screen
+-   Another workaround for SteamGuard's non-standard OTP format
 
 ### Fixed
 
diff --git a/app/src/main/java/dev/msfjarvis/aps/util/totp/UriTotpFinder.kt b/app/src/main/java/dev/msfjarvis/aps/util/totp/UriTotpFinder.kt
index fa8481a9..a420fe5d 100644
--- a/app/src/main/java/dev/msfjarvis/aps/util/totp/UriTotpFinder.kt
+++ b/app/src/main/java/dev/msfjarvis/aps/util/totp/UriTotpFinder.kt
@@ -24,32 +24,29 @@ class UriTotpFinder @Inject constructor() : TotpFinder {
   }
 
   override fun findDigits(content: String): String {
-    content.split("\n".toRegex()).forEach { line ->
-      if (line.startsWith(TOTP_FIELDS[0]) && Uri.parse(line).getQueryParameter("digits") != null) {
-        return Uri.parse(line).getQueryParameter("digits")!!
-      }
-    }
-    return "6"
+    return getQueryParameter(content, "digits") ?: "6"
   }
 
   override fun findPeriod(content: String): Long {
-    content.split("\n".toRegex()).forEach { line ->
-      if (line.startsWith(TOTP_FIELDS[0]) && Uri.parse(line).getQueryParameter("period") != null) {
-        val period = Uri.parse(line).getQueryParameter("period")!!.toLongOrNull()
-        if (period != null && period > 0) return period
-      }
-    }
-    return 30
+    return getQueryParameter(content, "period")?.toLongOrNull() ?: 30
   }
 
   override fun findAlgorithm(content: String): String {
+    return getQueryParameter(content, "algorithm") ?: "sha1"
+  }
+
+  override fun findIssuer(content: String): String? {
+    return getQueryParameter(content, "issuer") ?: Uri.parse(content).authority
+  }
+
+  private fun getQueryParameter(content: String, parameterName: String): String? {
     content.split("\n".toRegex()).forEach { line ->
-      if (line.startsWith(TOTP_FIELDS[0]) && Uri.parse(line).getQueryParameter("algorithm") != null
-      ) {
-        return Uri.parse(line).getQueryParameter("algorithm")!!
+      val uri = Uri.parse(line)
+      if (line.startsWith(TOTP_FIELDS[0]) && uri.getQueryParameter(parameterName) != null) {
+        return uri.getQueryParameter(parameterName)
       }
     }
-    return "sha1"
+    return null
   }
 
   companion object {
diff --git a/app/src/test/java/dev/msfjarvis/aps/util/totp/UriTotpFinderTest.kt b/app/src/test/java/dev/msfjarvis/aps/util/totp/UriTotpFinderTest.kt
index f04913a1..37cbef2f 100644
--- a/app/src/test/java/dev/msfjarvis/aps/util/totp/UriTotpFinderTest.kt
+++ b/app/src/test/java/dev/msfjarvis/aps/util/totp/UriTotpFinderTest.kt
@@ -45,6 +45,12 @@ class UriTotpFinderTest {
     assertEquals("SHA256", totpFinder.findAlgorithm(PASS_FILE_CONTENT))
   }
 
+  @Test
+  fun findIssuer() {
+    assertEquals("ACME Co", totpFinder.findIssuer(TOTP_URI))
+    assertEquals("ACME Co", totpFinder.findIssuer(PASS_FILE_CONTENT))
+  }
+
   companion object {
 
     const val TOTP_URI =
diff --git a/format-common/api/format-common.api b/format-common/api/format-common.api
index 09ff55a2..6d2377f9 100644
--- a/format-common/api/format-common.api
+++ b/format-common/api/format-common.api
@@ -21,6 +21,7 @@ public abstract interface class dev/msfjarvis/aps/util/totp/TotpFinder {
 	public static final field Companion Ldev/msfjarvis/aps/util/totp/TotpFinder$Companion;
 	public abstract fun findAlgorithm (Ljava/lang/String;)Ljava/lang/String;
 	public abstract fun findDigits (Ljava/lang/String;)Ljava/lang/String;
+	public abstract fun findIssuer (Ljava/lang/String;)Ljava/lang/String;
 	public abstract fun findPeriod (Ljava/lang/String;)J
 	public abstract fun findSecret (Ljava/lang/String;)Ljava/lang/String;
 }
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 4fc9667d..0e8f6d2e 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
@@ -68,10 +68,7 @@ constructor(
    * and usernames stripped.
    */
   public val extraContentWithoutAuthData: String
-  private val digits: String
   private val totpSecret: String?
-  private val totpPeriod: Long
-  private val totpAlgorithm: String
 
   init {
     val (foundPassword, passContent) = findAndStripPassword(content.split("\n".toRegex()))
@@ -80,17 +77,18 @@ constructor(
     extraContentWithoutAuthData = generateExtraContentWithoutAuthData()
     extraContent = generateExtraContentPairs()
     username = findUsername()
-    digits = totpFinder.findDigits(content)
     totpSecret = totpFinder.findSecret(content)
-    totpPeriod = totpFinder.findPeriod(content)
-    totpAlgorithm = totpFinder.findAlgorithm(content)
     if (totpSecret != null) {
       scope.launch {
-        updateTotp(clock.millis())
+        val digits = totpFinder.findDigits(content)
+        val totpPeriod = totpFinder.findPeriod(content)
+        val totpAlgorithm = totpFinder.findAlgorithm(content)
+        val issuer = totpFinder.findIssuer(content)
         val remainingTime = totpPeriod - (clock.millis() % totpPeriod)
+        updateTotp(clock.millis(), totpPeriod, totpAlgorithm, digits, issuer)
         delay(Duration.seconds(remainingTime))
         repeat(Int.MAX_VALUE) {
-          updateTotp(clock.millis())
+          updateTotp(clock.millis(), totpPeriod, totpAlgorithm, digits, issuer)
           delay(Duration.seconds(totpPeriod))
         }
       }
@@ -186,9 +184,15 @@ constructor(
     return null
   }
 
-  private fun updateTotp(millis: Long) {
+  private fun updateTotp(
+    millis: Long,
+    totpPeriod: Long,
+    totpAlgorithm: String,
+    digits: String,
+    issuer: String?,
+  ) {
     if (totpSecret != null) {
-      Otp.calculateCode(totpSecret, millis / (1000 * totpPeriod), totpAlgorithm, digits)
+      Otp.calculateCode(totpSecret, millis / (1000 * totpPeriod), totpAlgorithm, digits, issuer)
         .mapBoth({ code -> _totp.value = code }, { throwable -> throw throwable })
     }
   }
diff --git a/format-common/src/main/kotlin/dev/msfjarvis/aps/util/totp/Otp.kt b/format-common/src/main/kotlin/dev/msfjarvis/aps/util/totp/Otp.kt
index 65284441..f1f71e00 100644
--- a/format-common/src/main/kotlin/dev/msfjarvis/aps/util/totp/Otp.kt
+++ b/format-common/src/main/kotlin/dev/msfjarvis/aps/util/totp/Otp.kt
@@ -23,8 +23,13 @@ internal object Otp {
     check(STEAM_ALPHABET.size == 26)
   }
 
-  fun calculateCode(secret: String, counter: Long, algorithm: String, digits: String) =
-      runCatching {
+  fun calculateCode(
+    secret: String,
+    counter: Long,
+    algorithm: String,
+    digits: String,
+    issuer: String?,
+  ) = runCatching {
     val algo = "Hmac${algorithm.uppercase(Locale.ROOT)}"
     val decodedSecret = BASE_32.decode(secret)
     val secretKey = SecretKeySpec(decodedSecret, algo)
@@ -40,8 +45,9 @@ internal object Otp {
     code[0] = (0x7f and code[0].toInt()).toByte()
     val codeInt = ByteBuffer.wrap(code).int
     check(codeInt > 0)
-    if (digits == "s") {
-      // Steam
+    // SteamGuard is a horrible OTP implementation that generates non-standard 5 digit OTPs as well
+    // as uses a custom character set.
+    if (digits == "s" || issuer == "Steam") {
       var remainingCodeInt = codeInt
       buildString {
         repeat(5) {
diff --git a/format-common/src/main/kotlin/dev/msfjarvis/aps/util/totp/TotpFinder.kt b/format-common/src/main/kotlin/dev/msfjarvis/aps/util/totp/TotpFinder.kt
index 64f22065..1cb7de97 100644
--- a/format-common/src/main/kotlin/dev/msfjarvis/aps/util/totp/TotpFinder.kt
+++ b/format-common/src/main/kotlin/dev/msfjarvis/aps/util/totp/TotpFinder.kt
@@ -20,6 +20,9 @@ public interface TotpFinder {
   /** Get the algorithm for the TOTP secret. */
   public fun findAlgorithm(content: String): String
 
+  /** Get the issuer for the TOTP secret, if any. */
+  public fun findIssuer(content: String): String?
+
   public companion object {
     public val TOTP_FIELDS: Array<String> = arrayOf("otpauth://totp", "totp:")
   }