Add tests for GPG identifier parsing (#1319)

4 files changed, 82 insertions, 40 deletions

diff --git a/app/src/main/java/dev/msfjarvis/aps/ui/crypto/PasswordCreationActivity.kt b/app/src/main/java/dev/msfjarvis/aps/ui/crypto/PasswordCreationActivity.kt
index 43e9b70b..749ff741 100644
--- a/app/src/main/java/dev/msfjarvis/aps/ui/crypto/PasswordCreationActivity.kt
+++ b/app/src/main/java/dev/msfjarvis/aps/ui/crypto/PasswordCreationActivity.kt
@@ -34,6 +34,7 @@ import dev.msfjarvis.aps.ui.dialogs.PasswordGeneratorDialogFragment
 import dev.msfjarvis.aps.ui.dialogs.XkPasswordGeneratorDialogFragment
 import dev.msfjarvis.aps.util.autofill.AutofillPreferences
 import dev.msfjarvis.aps.util.autofill.DirectoryStructure
+import dev.msfjarvis.aps.util.crypto.GpgIdentifier
 import dev.msfjarvis.aps.util.extensions.base64
 import dev.msfjarvis.aps.util.extensions.commitChange
 import dev.msfjarvis.aps.util.extensions.getString
@@ -50,7 +51,6 @@ import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import me.msfjarvis.openpgpktx.util.OpenPgpApi
 import me.msfjarvis.openpgpktx.util.OpenPgpServiceConnection
-import me.msfjarvis.openpgpktx.util.OpenPgpUtils
 
 class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnBound {
 
@@ -271,39 +271,6 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
         otpImportButton.isVisible = !entry.hasTotp()
     }
 
-    private sealed class GpgIdentifier {
-        data class KeyId(val id: Long) : GpgIdentifier()
-        data class UserId(val email: String) : GpgIdentifier()
-    }
-
-    @OptIn(ExperimentalUnsignedTypes::class)
-    private fun parseGpgIdentifier(identifier: String): GpgIdentifier? {
-        if (identifier.isEmpty()) return null
-        // Match long key IDs:
-        // FF22334455667788 or 0xFF22334455667788
-        val maybeLongKeyId = identifier.removePrefix("0x").takeIf {
-            it.matches("[a-fA-F0-9]{16}".toRegex())
-        }
-        if (maybeLongKeyId != null) {
-            val keyId = maybeLongKeyId.toULong(16)
-            return GpgIdentifier.KeyId(keyId.toLong())
-        }
-
-        // Match fingerprints:
-        // FF223344556677889900112233445566778899 or 0xFF223344556677889900112233445566778899
-        val maybeFingerprint = identifier.removePrefix("0x").takeIf {
-            it.matches("[a-fA-F0-9]{40}".toRegex())
-        }
-        if (maybeFingerprint != null) {
-            // Truncating to the long key ID is not a security issue since OpenKeychain only accepts
-            // non-ambiguous key IDs.
-            val keyId = maybeFingerprint.takeLast(16).toULong(16)
-            return GpgIdentifier.KeyId(keyId.toLong())
-        }
-
-        return OpenPgpUtils.splitUserId(identifier).email?.let { GpgIdentifier.UserId(it) }
-    }
-
     /**
      * Encrypts the password and the extra content
      */
@@ -340,7 +307,7 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
             val gpgIdentifiers = gpgIdentifierFile.readLines()
                 .filter { it.isNotBlank() }
                 .map { line ->
-                    parseGpgIdentifier(line) ?: run {
+                    GpgIdentifier.fromString(line) ?: run {
                         // The line being empty means this is most likely an empty `.gpg-id` file
                         // we created. Skip the validation so we can make the user add a real ID.
                         if (line.isEmpty()) return@run
diff --git a/app/src/main/java/dev/msfjarvis/aps/util/crypto/GpgIdentifier.kt b/app/src/main/java/dev/msfjarvis/aps/util/crypto/GpgIdentifier.kt
new file mode 100644
index 00000000..51b3b76b
--- /dev/null
+++ b/app/src/main/java/dev/msfjarvis/aps/util/crypto/GpgIdentifier.kt
@@ -0,0 +1,38 @@
+package dev.msfjarvis.aps.util.crypto
+
+import me.msfjarvis.openpgpktx.util.OpenPgpUtils
+
+sealed class GpgIdentifier {
+    data class KeyId(val id: Long) : GpgIdentifier()
+    data class UserId(val email: String) : GpgIdentifier()
+
+    companion object {
+        @OptIn(ExperimentalUnsignedTypes::class)
+        fun fromString(identifier: String): GpgIdentifier? {
+            if (identifier.isEmpty()) return null
+            // Match long key IDs:
+            // FF22334455667788 or 0xFF22334455667788
+            val maybeLongKeyId = identifier.removePrefix("0x").takeIf {
+                it.matches("[a-fA-F0-9]{16}".toRegex())
+            }
+            if (maybeLongKeyId != null) {
+                val keyId = maybeLongKeyId.toULong(16)
+                return KeyId(keyId.toLong())
+            }
+
+            // Match fingerprints:
+            // FF223344556677889900112233445566778899 or 0xFF223344556677889900112233445566778899
+            val maybeFingerprint = identifier.removePrefix("0x").takeIf {
+                it.matches("[a-fA-F0-9]{40}".toRegex())
+            }
+            if (maybeFingerprint != null) {
+                // Truncating to the long key ID is not a security issue since OpenKeychain only accepts
+                // non-ambiguous key IDs.
+                val keyId = maybeFingerprint.takeLast(16).toULong(16)
+                return KeyId(keyId.toLong())
+            }
+
+            return OpenPgpUtils.splitUserId(identifier).email?.let { UserId(it) }
+        }
+    }
+}
diff --git a/app/src/test/java/dev/msfjarvis/aps/util/crypto/GpgIdentifierTest.kt b/app/src/test/java/dev/msfjarvis/aps/util/crypto/GpgIdentifierTest.kt
new file mode 100644
index 00000000..efb08f7b
--- /dev/null
+++ b/app/src/test/java/dev/msfjarvis/aps/util/crypto/GpgIdentifierTest.kt
@@ -0,0 +1,38 @@
+package dev.msfjarvis.aps.util.crypto
+
+import kotlin.test.Ignore
+import kotlin.test.assertNotNull
+import kotlin.test.assertTrue
+import kotlin.test.Test
+
+class GpgIdentifierTest {
+
+    @Test
+    fun `parses hexadecimal key id without leading 0x`() {
+        val identifier = GpgIdentifier.fromString("79E8208280490C77")
+        assertNotNull(identifier)
+        assertTrue { identifier is GpgIdentifier.KeyId }
+    }
+
+    @Test
+    fun `parses hexadecimal key id`() {
+        val identifier = GpgIdentifier.fromString("0x79E8208280490C77")
+        assertNotNull(identifier)
+        assertTrue { identifier is GpgIdentifier.KeyId }
+    }
+
+    @Test
+    fun `parses email as user id`() {
+        val identifier = GpgIdentifier.fromString("aps@msfjarvis.dev")
+        assertNotNull(identifier)
+        assertTrue { identifier is GpgIdentifier.UserId }
+    }
+
+    @Test
+    @Ignore("OpenKeychain can't yet handle these so we don't either")
+    fun `parses non-email user id`() {
+        val identifier = GpgIdentifier.fromString("john.doe")
+        assertNotNull(identifier)
+        assertTrue { identifier is GpgIdentifier.UserId }
+    }
+}
diff --git a/openpgp-ktx/src/main/java/me/msfjarvis/openpgpktx/util/OpenPgpUtils.kt b/openpgp-ktx/src/main/java/me/msfjarvis/openpgpktx/util/OpenPgpUtils.kt
index cca90653..439c9278 100644
--- a/openpgp-ktx/src/main/java/me/msfjarvis/openpgpktx/util/OpenPgpUtils.kt
+++ b/openpgp-ktx/src/main/java/me/msfjarvis/openpgpktx/util/OpenPgpUtils.kt
@@ -6,7 +6,6 @@ package me.msfjarvis.openpgpktx.util
 
 import android.content.Context
 import android.content.Intent
-import android.text.TextUtils
 import java.io.Serializable
 import java.util.Locale
 import java.util.regex.Pattern
@@ -64,7 +63,7 @@ public object OpenPgpUtils {
      * See SplitUserIdTest for examples.
      */
     public fun splitUserId(userId: String): UserId {
-        if (!TextUtils.isEmpty(userId)) {
+        if (userId.isNotEmpty()) {
             val matcher = USER_ID_PATTERN.matcher(userId)
             if (matcher.matches()) {
                 var name = if (matcher.group(1).isEmpty()) null else matcher.group(1)
@@ -95,15 +94,15 @@ public object OpenPgpUtils {
      */
     public fun createUserId(userId: UserId): String? {
         val userIdBuilder = StringBuilder()
-        if (!TextUtils.isEmpty(userId.name)) {
+        if (!userId.name.isNullOrEmpty()) {
             userIdBuilder.append(userId.name)
         }
-        if (!TextUtils.isEmpty(userId.comment)) {
+        if (!userId.comment.isNullOrEmpty()) {
             userIdBuilder.append(" (")
             userIdBuilder.append(userId.comment)
             userIdBuilder.append(")")
         }
-        if (!TextUtils.isEmpty(userId.email)) {
+        if (!userId.email.isNullOrEmpty()) {
             userIdBuilder.append(" <")
             userIdBuilder.append(userId.email)
             userIdBuilder.append(">")