From 62dbc183d52d93860228316b209ec5aa15f16a08 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 --- 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(-) 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