Backport patches for 1.10.2 release

.github/workflows/pull_request.yml

@@ -69,8 +69,20 @@ jobs:
       if: ${{ steps.service-changed.outputs.result == 'true' }}
       run: ./gradlew test${{ matrix.variant }} lint${{ matrix.variant}} -Dpre-dex=false
 
-    - name: Run instrumentation tests
-      if: ${{ steps.service-changed.outputs.result == 'true' }}
+    - name: Run instrumentation tests on free flavor
+      if: ${{ steps.service-changed.outputs.result == 'true' && matrix.variant == 'freeRelease' }}
+      uses: reactivecircus/android-emulator-runner@v2.11.0
+      with:
+        api-level: ${{ matrix.api-level }}
+        target: default
+        script: |
+          adb shell settings put global animator_duration_scale 0
+          adb shell settings put global transition_animation_scale 0
+          adb shell settings put global window_animation_scale 0
+          ./gradlew :app:connectedFreeDebugAndroidTest
+
+    - name: Run instrumentation tests on nonFree flavor
+      if: ${{ steps.service-changed.outputs.result == 'true' && matrix.variant == 'nonFreeRelease' }}
       uses: reactivecircus/android-emulator-runner@v2.11.0
       with:
         api-level: ${{ matrix.api-level }}
@@ -79,4 +91,4 @@ jobs:
           adb shell settings put global animator_duration_scale 0
           adb shell settings put global transition_animation_scale 0
           adb shell settings put global window_animation_scale 0
-          ./gradlew connectedCheck
+          ./gradlew :app:connectedNonFreeDebugAndroidTest

CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Fixed
+
+- Properly handle cases where files contain only TOTP secrets and no password
+- Correctly hide TOTP import button when TOTP secret/OTPAUTH URL is already present in extra content
+- SMS OTP Autofill no longer crashes when invoked and correctly asks for the required permission on first use
+
 ## [1.10.1] - 2020-07-23
 
 ### Fixed

app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt

@@ -0,0 +1,114 @@
+/*
+ * Copyright © 2014-2020 The Android Password Store Authors. All Rights Reserved.
+ * SPDX-License-Identifier: GPL-3.0-only
+ */
+
+package com.zeapo.pwdstore.model
+
+import com.zeapo.pwdstore.utils.Otp
+import com.zeapo.pwdstore.utils.UriTotpFinder
+import java.util.Date
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertNotNull
+import kotlin.test.assertNull
+import kotlin.test.assertTrue
+import org.junit.Test
+
+class PasswordEntryAndroidTest {
+
+    private fun makeEntry(content: String) = PasswordEntry(content, UriTotpFinder())
+
+    @Test fun testGetPassword() {
+        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)
+    }
+
+    @Test fun testGetExtraContent() {
+        assertEquals("bla\n", makeEntry("fooooo\nbla\n").extraContent)
+        assertEquals("bla", makeEntry("fooooo\nbla").extraContent)
+        assertEquals("", makeEntry("fooooo\n").extraContent)
+        assertEquals("", makeEntry("fooooo").extraContent)
+        assertEquals("blubb\n", makeEntry("\nblubb\n").extraContent)
+        assertEquals("blubb", makeEntry("\nblubb").extraContent)
+        assertEquals("", makeEntry("\n").extraContent)
+        assertEquals("", makeEntry("").extraContent)
+    }
+
+    @Test fun testGetUsername() {
+        for (field in PasswordEntry.USERNAME_FIELDS) {
+            assertEquals("username", makeEntry("\n$field username").username)
+            assertEquals("username", makeEntry("\n${field.toUpperCase()} 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)
+        assertNull(makeEntry("secret\nextra\ncontent\n").username)
+    }
+
+    @Test fun testHasUsername() {
+        assertTrue(makeEntry("secret\nextra\nlogin: username\ncontent\n").hasUsername())
+        assertFalse(makeEntry("secret\nextra\ncontent\n").hasUsername())
+        assertFalse(makeEntry("secret\nlogin failed\n").hasUsername())
+        assertFalse(makeEntry("\n").hasUsername())
+        assertFalse(makeEntry("").hasUsername())
+    }
+
+    @Test fun testGeneratesOtpFromTotpUri() {
+        val entry = makeEntry("secret\nextra\n$TOTP_URI")
+        assertTrue(entry.hasTotp())
+        val code = Otp.calculateCode(
+            entry.totpSecret!!,
+            // The hardcoded date value allows this test to stay reproducible.
+            Date(8640000).time / (1000 * entry.totpPeriod),
+            entry.totpAlgorithm,
+            entry.digits
+        )
+        assertNotNull(code) { "Generated OTP cannot be null" }
+        assertEquals(entry.digits.toInt(), code.length)
+        assertEquals("545293", code)
+    }
+
+    @Test fun testGeneratesOtpWithOnlyUriInFile() {
+        val entry = makeEntry(TOTP_URI)
+        assertTrue(entry.password.isEmpty())
+        assertTrue(entry.hasTotp())
+        val code = Otp.calculateCode(
+            entry.totpSecret!!,
+            // The hardcoded date value allows this test to stay reproducible.
+            Date(8640000).time / (1000 * entry.totpPeriod),
+            entry.totpAlgorithm,
+            entry.digits
+        )
+        assertNotNull(code) { "Generated OTP cannot be null" }
+        assertEquals(entry.digits.toInt(), code.length)
+        assertEquals("545293", code)
+    }
+
+    @Test fun testOnlyLooksForUriInFirstLine() {
+        val entry = makeEntry("id:\n$TOTP_URI")
+        assertTrue(entry.password.isNotEmpty())
+        assertTrue(entry.hasTotp())
+        assertFalse(entry.hasUsername())
+    }
+
+    companion object {
+
+        const val TOTP_URI = "otpauth://totp/ACME%20Co:john@example.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=SHA1&digits=6&period=30"
+    }
+}

app/src/androidTest/java/com/zeapo/pwdstore/utils/UriTotpFinderTest.kt

@@ -16,25 +16,30 @@ class UriTotpFinderTest {
     fun findSecret() {
         assertEquals("HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ", totpFinder.findSecret(TOTP_URI))
         assertEquals("HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ", totpFinder.findSecret("name\npassword\ntotp: HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ"))
+        assertEquals("HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ", totpFinder.findSecret(PASS_FILE_CONTENT))
     }
 
     @Test
     fun findDigits() {
         assertEquals("12", totpFinder.findDigits(TOTP_URI))
+        assertEquals("12", totpFinder.findDigits(PASS_FILE_CONTENT))
     }
 
     @Test
     fun findPeriod() {
         assertEquals(25, totpFinder.findPeriod(TOTP_URI))
+        assertEquals(25, totpFinder.findPeriod(PASS_FILE_CONTENT))
     }
 
     @Test
     fun findAlgorithm() {
         assertEquals("SHA256", totpFinder.findAlgorithm(TOTP_URI))
+        assertEquals("SHA256", totpFinder.findAlgorithm(PASS_FILE_CONTENT))
     }
 
     companion object {
 
         const val TOTP_URI = "otpauth://totp/ACME%20Co:john@example.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=SHA256&digits=12&period=25"
+        const val PASS_FILE_CONTENT = "password\n$TOTP_URI"
     }
 }

app/src/main/java/com/zeapo/pwdstore/crypto/PasswordCreationActivity.kt

@@ -177,13 +177,11 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
                                 extraContent.setText(entry.extraContentWithoutAuthData)
                             }
                         }
-                        updateViewState()
                     }
                 }
                 listOf(filename, extraContent).forEach {
                     it.doOnTextChanged { _, _, _, _ -> updateViewState() }
                 }
-                updateViewState()
             }
             suggestedPass?.let {
                 password.setText(it)
@@ -195,6 +193,7 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
                 password.inputType = InputType.TYPE_CLASS_TEXT or InputType.TYPE_TEXT_VARIATION_PASSWORD
             }
         }
+        updateViewState()
     }
 
     override fun onCreateOptionsMenu(menu: Menu?): Boolean {
@@ -235,7 +234,7 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
         val entry = PasswordEntry("PLACEHOLDER\n${extraContent.text}")
         encryptUsername.apply {
             if (visibility != View.VISIBLE)
-                return@with
+                return@apply
             val hasUsernameInFileName = filename.text.toString().isNotBlank()
             val hasUsernameInExtras = entry.hasUsername()
             isEnabled = hasUsernameInFileName xor hasUsernameInExtras
@@ -420,8 +419,7 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
                                         AutofillPreferences.directoryStructure(applicationContext)
                                     val entry = PasswordEntry(content)
                                     returnIntent.putExtra(RETURN_EXTRA_PASSWORD, entry.password)
-                                    val username = PasswordEntry(content).username
-                                        ?: directoryStructure.getUsernameFor(file)
+                                    val username = entry.username ?: directoryStructure.getUsernameFor(file)
                                     returnIntent.putExtra(RETURN_EXTRA_USERNAME, username)
                                 }
 

app/src/main/java/com/zeapo/pwdstore/model/PasswordEntry.kt

@@ -31,7 +31,7 @@ class PasswordEntry(content: String, private val totpFinder: TotpFinder = UriTot
 
     init {
         val passContent = content.split("\n".toRegex(), 2).toTypedArray()
-        password = passContent[0]
+        password = if (UriTotpFinder.TOTP_FIELDS.any { passContent[0].startsWith(it) }) "" else passContent[0]
         extraContent = findExtraContent(passContent)
         username = findUsername()
         digits = findOtpDigits(content)
@@ -85,8 +85,10 @@ class PasswordEntry(content: String, private val totpFinder: TotpFinder = UriTot
         return null
     }
 
-    private fun findExtraContent(passContent: Array<String>): String {
-        return if (passContent.size > 1) passContent[1] else ""
+    private fun findExtraContent(passContent: Array<String>) = when {
+        password.isEmpty() && passContent[0].isNotEmpty() -> passContent[0]
+        passContent.size > 1 -> passContent[1]
+        else -> ""
     }
 
     private fun findTotpSecret(decryptedContent: String): String? {

app/src/main/java/com/zeapo/pwdstore/utils/UriTotpFinder.kt

@@ -14,10 +14,10 @@ class UriTotpFinder : TotpFinder {
 
     override fun findSecret(content: String): String? {
         content.split("\n".toRegex()).forEach { line ->
-            if (line.startsWith("otpauth://totp/")) {
+            if (line.startsWith(TOTP_FIELDS[0])) {
                 return Uri.parse(line).getQueryParameter("secret")
             }
-            if (line.startsWith("totp:", ignoreCase = true)) {
+            if (line.startsWith(TOTP_FIELDS[1], ignoreCase = true)) {
                 return line.split(": *".toRegex(), 2).toTypedArray()[1]
             }
         }
@@ -26,7 +26,7 @@ class UriTotpFinder : TotpFinder {
 
     override fun findDigits(content: String): String {
         content.split("\n".toRegex()).forEach { line ->
-            if (line.startsWith("otpauth://totp/") &&
+            if (line.startsWith(TOTP_FIELDS[0]) &&
                 Uri.parse(line).getQueryParameter("digits") != null) {
                 return Uri.parse(line).getQueryParameter("digits")!!
             }
@@ -36,7 +36,7 @@ class UriTotpFinder : TotpFinder {
 
     override fun findPeriod(content: String): Long {
         content.split("\n".toRegex()).forEach { line ->
-            if (line.startsWith("otpauth://totp/") &&
+            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)
@@ -48,11 +48,18 @@ class UriTotpFinder : TotpFinder {
 
     override fun findAlgorithm(content: String): String {
         content.split("\n".toRegex()).forEach { line ->
-            if (line.startsWith("otpauth://totp/") &&
+            if (line.startsWith(TOTP_FIELDS[0]) &&
                 Uri.parse(line).getQueryParameter("algorithm") != null) {
                 return Uri.parse(line).getQueryParameter("algorithm")!!
             }
         }
         return "sha1"
     }
+
+    companion object {
+        val TOTP_FIELDS = arrayOf(
+            "otpauth://totp",
+            "totp:"
+        )
+    }
 }

app/src/nonFree/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillSmsActivity.kt

@@ -24,13 +24,30 @@ import com.google.android.gms.auth.api.phone.SmsRetriever
 import com.google.android.gms.common.ConnectionResult
 import com.google.android.gms.common.GoogleApiAvailability
 import com.google.android.gms.common.api.ResolvableApiException
-import com.google.android.gms.tasks.Tasks
+import com.google.android.gms.tasks.Task
 import com.zeapo.pwdstore.autofill.oreo.AutofillAction
 import com.zeapo.pwdstore.autofill.oreo.Credentials
 import com.zeapo.pwdstore.autofill.oreo.FillableForm
 import com.zeapo.pwdstore.databinding.ActivityOreoAutofillSmsBinding
 import com.zeapo.pwdstore.utils.viewBinding
+import java.util.concurrent.ExecutionException
+import kotlin.coroutines.resume
+import kotlin.coroutines.resumeWithException
+import kotlin.coroutines.suspendCoroutine
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
+
+suspend fun <T> Task<T>.suspendableAwait() = suspendCoroutine<T> { cont ->
+    addOnSuccessListener { result: T ->
+        cont.resume(result)
+    }
+    addOnFailureListener { e ->
+        // Unwrap specific exceptions (e.g. ResolvableApiException) from ExecutionException.
+        val cause = (e as? ExecutionException)?.cause ?: e
+        cont.resumeWithException(cause)
+    }
+}
 
 @RequiresApi(Build.VERSION_CODES.O)
 class AutofillSmsActivity : AppCompatActivity() {
@@ -105,15 +122,21 @@ class AutofillSmsActivity : AppCompatActivity() {
         }
     }
 
-    private fun waitForSms() {
+    private suspend fun waitForSms() {
         val smsClient = SmsCodeRetriever.getAutofillClient(this@AutofillSmsActivity)
         try {
-            Tasks.await(smsClient.startSmsCodeRetriever())
+            withContext(Dispatchers.IO) {
+                smsClient.startSmsCodeRetriever().suspendableAwait()
+            }
         } catch (e: ResolvableApiException) {
-            e.startResolutionForResult(this, 1)
+            withContext(Dispatchers.Main) {
+                e.startResolutionForResult(this@AutofillSmsActivity, 1)
+            }
         } catch (e: Exception) {
             e(e)
-            finish()
+            withContext(Dispatchers.Main) {
+                finish()
+            }
         }
     }
 

app/src/test/java/com/zeapo/pwdstore/model/PasswordEntryTest.kt

@@ -83,6 +83,29 @@ class PasswordEntryTest {
         assertEquals("545293", code)
     }
 
+    @Test fun testGeneratesOtpWithOnlyUriInFile() {
+        val entry = makeEntry(TOTP_URI)
+        assertTrue(entry.password.isEmpty())
+        assertTrue(entry.hasTotp())
+        val code = Otp.calculateCode(
+            entry.totpSecret!!,
+            // The hardcoded date value allows this test to stay reproducible.
+            Date(8640000).time / (1000 * entry.totpPeriod),
+            entry.totpAlgorithm,
+            entry.digits
+        )
+        assertNotNull(code) { "Generated OTP cannot be null" }
+        assertEquals(entry.digits.toInt(), code.length)
+        assertEquals("545293", code)
+    }
+
+    @Test fun testOnlyLooksForUriInFirstLine() {
+        val entry = makeEntry("id:\n$TOTP_URI")
+        assertTrue(entry.password.isNotEmpty())
+        assertTrue(entry.hasTotp())
+        assertFalse(entry.hasUsername())
+    }
+
     companion object {
 
         const val TOTP_URI = "otpauth://totp/ACME%20Co:john@example.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=SHA1&digits=6&period=30"