From 35a8e8b42cd828a1c5634d4ee1d50fc4d0d98f5c Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Fri, 24 Jul 2020 14:33:55 +0530 Subject: Expand OTP and PasswordEntry tests (#968) (cherry picked from commit e3cf73885c112bc553d6a0cc01d594a87728f448) --- .../pwdstore/model/PasswordEntryAndroidTest.kt | 91 ++++++++++++++++++++++ .../com/zeapo/pwdstore/utils/UriTotpFinderTest.kt | 5 ++ 2 files changed, 96 insertions(+) create mode 100644 app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt (limited to 'app') diff --git a/app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt b/app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt new file mode 100644 index 00000000..ba373f48 --- /dev/null +++ b/app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt @@ -0,0 +1,91 @@ +/* + * 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) + } + + companion object { + + const val TOTP_URI = "otpauth://totp/ACME%20Co:john@example.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=SHA1&digits=6&period=30" + } +} diff --git a/app/src/androidTest/java/com/zeapo/pwdstore/utils/UriTotpFinderTest.kt b/app/src/androidTest/java/com/zeapo/pwdstore/utils/UriTotpFinderTest.kt index 8a836f89..0e4ac51c 100644 --- a/app/src/androidTest/java/com/zeapo/pwdstore/utils/UriTotpFinderTest.kt +++ b/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" } } -- cgit v1.2.3 From 64a6e0f4e916122fdd00df2e2b48d96ea7d49c1c Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Sat, 25 Jul 2020 14:37:16 +0530 Subject: Properly handle files without passwords (#969) * Properly handle files without passwords Fixes #967 Signed-off-by: Harsh Shandilya * Fix tests Signed-off-by: Harsh Shandilya * Only look for TOTP URI Signed-off-by: Harsh Shandilya (cherry picked from commit 62dbc183d52d93860228316b209ec5aa15f16a08) --- CHANGELOG.md | 4 ++++ .../pwdstore/model/PasswordEntryAndroidTest.kt | 23 ++++++++++++++++++++++ .../java/com/zeapo/pwdstore/model/PasswordEntry.kt | 8 +++++--- .../java/com/zeapo/pwdstore/utils/UriTotpFinder.kt | 17 +++++++++++----- .../com/zeapo/pwdstore/model/PasswordEntryTest.kt | 23 ++++++++++++++++++++++ 5 files changed, 67 insertions(+), 8 deletions(-) (limited to 'app') diff --git a/CHANGELOG.md b/CHANGELOG.md index b186d093..93b247b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ 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 + ## [1.10.1] - 2020-07-23 ### Fixed diff --git a/app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt b/app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt index ba373f48..1928a5f4 100644 --- a/app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt +++ b/app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt @@ -84,6 +84,29 @@ class PasswordEntryAndroidTest { 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" diff --git a/app/src/main/java/com/zeapo/pwdstore/model/PasswordEntry.kt b/app/src/main/java/com/zeapo/pwdstore/model/PasswordEntry.kt index b4fd0165..2cd31256 100644 --- a/app/src/main/java/com/zeapo/pwdstore/model/PasswordEntry.kt +++ b/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 { - return if (passContent.size > 1) passContent[1] else "" + private fun findExtraContent(passContent: Array) = when { + password.isEmpty() && passContent[0].isNotEmpty() -> passContent[0] + passContent.size > 1 -> passContent[1] + else -> "" } private fun findTotpSecret(decryptedContent: String): String? { diff --git a/app/src/main/java/com/zeapo/pwdstore/utils/UriTotpFinder.kt b/app/src/main/java/com/zeapo/pwdstore/utils/UriTotpFinder.kt index 57c8f580..aab72d38 100644 --- a/app/src/main/java/com/zeapo/pwdstore/utils/UriTotpFinder.kt +++ b/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:" + ) + } } diff --git a/app/src/test/java/com/zeapo/pwdstore/model/PasswordEntryTest.kt b/app/src/test/java/com/zeapo/pwdstore/model/PasswordEntryTest.kt index 16cbd855..f9c55167 100644 --- a/app/src/test/java/com/zeapo/pwdstore/model/PasswordEntryTest.kt +++ b/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" -- cgit v1.2.3 From c132cc98e632036731b118d886709d783a55cc71 Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Wed, 29 Jul 2020 13:06:22 +0530 Subject: Fix TOTP import button check semantics (#982) * Improve TOTP checking semantics Signed-off-by: Harsh Shandilya * Fix return label Signed-off-by: Harsh Shandilya * update CHANGELOG Signed-off-by: Harsh Shandilya * Move updateViewState() call outside with(binding) scope Signed-off-by: Harsh Shandilya (cherry picked from commit ecf96aa0668a758b2408284facda41ac70fe10aa) --- CHANGELOG.md | 1 + .../java/com/zeapo/pwdstore/crypto/PasswordCreationActivity.kt | 8 +++----- 2 files changed, 4 insertions(+), 5 deletions(-) (limited to 'app') diff --git a/CHANGELOG.md b/CHANGELOG.md index 93b247b4..2c79702e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. ### 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 ## [1.10.1] - 2020-07-23 diff --git a/app/src/main/java/com/zeapo/pwdstore/crypto/PasswordCreationActivity.kt b/app/src/main/java/com/zeapo/pwdstore/crypto/PasswordCreationActivity.kt index 3c1757f1..615b695e 100644 --- a/app/src/main/java/com/zeapo/pwdstore/crypto/PasswordCreationActivity.kt +++ b/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) } -- cgit v1.2.3 From 23158ce6da8332bdb1f75a589b31ebcd1d819e00 Mon Sep 17 00:00:00 2001 From: Fabian Henneke Date: Thu, 30 Jul 2020 10:29:01 +0200 Subject: Fix two SMS Autofill crashes (#985) SMS OTP Autofill currently crashes for two reasons: 1. Tasks.await has a precondition of not running on the UI thread. 2. Exceptions thrown from Tasks are always wrapped into ExecutionExceptions and need to be unwrapped before they can be identified as ResolvableApiException. This commit addresses both issues by making waitForSms a proper coroutine using withContext and a custom wrapper around Task that relies on suspendCoroutine and automatically unwraps exceptions. (cherry picked from commit 3afeff45d8bd5fff66e1d0fa2c15fa2527487af1) --- CHANGELOG.md | 1 + .../autofill/oreo/ui/AutofillSmsActivity.kt | 33 ++++++++++++++++++---- 2 files changed, 29 insertions(+), 5 deletions(-) (limited to 'app') diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c79702e..38eb7eb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file. - 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 diff --git a/app/src/nonFree/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillSmsActivity.kt b/app/src/nonFree/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillSmsActivity.kt index 02394867..4413ea10 100644 --- a/app/src/nonFree/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillSmsActivity.kt +++ b/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 Task.suspendableAwait() = suspendCoroutine { 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() + } } } -- cgit v1.2.3 From 1c7daff5baa71a659d5f5a94001df0f21feea4bc Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Thu, 30 Jul 2020 14:30:39 +0530 Subject: build: bump version Signed-off-by: Harsh Shandilya --- app/build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/build.gradle b/app/build.gradle index 78d2c50a..ca356b83 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -25,8 +25,8 @@ android { defaultConfig { applicationId 'dev.msfjarvis.aps' - versionCode 11011 - versionName '1.11.0-SNAPSHOT' + versionCode 11020 + versionName '1.10.2' } lintOptions { -- cgit v1.2.3