From 730da7ef0faf55f7bb90eccc41e7b57997ba839d Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Sat, 5 Sep 2020 05:39:17 +0530 Subject: [PATCH] Otp: use runCatching to replace exception handling Signed-off-by: Harsh Shandilya --- .../model/PasswordEntryAndroidTest.kt | 5 ++- .../com/zeapo/pwdstore/model/PasswordEntry.kt | 3 +- .../main/java/com/zeapo/pwdstore/utils/Otp.kt | 39 ++++++------------- .../zeapo/pwdstore/model/PasswordEntryTest.kt | 5 ++- .../java/com/zeapo/pwdstore/utils/OtpTest.kt | 31 ++++++++------- 5 files changed, 35 insertions(+), 48 deletions(-) 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 1928a5f4..53df8209 100644 --- a/app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt +++ b/app/src/androidTest/java/com/zeapo/pwdstore/model/PasswordEntryAndroidTest.kt @@ -5,6 +5,7 @@ package com.zeapo.pwdstore.model +import com.github.michaelbull.result.get import com.zeapo.pwdstore.utils.Otp import com.zeapo.pwdstore.utils.UriTotpFinder import java.util.Date @@ -78,7 +79,7 @@ class PasswordEntryAndroidTest { Date(8640000).time / (1000 * entry.totpPeriod), entry.totpAlgorithm, entry.digits - ) + ).get() assertNotNull(code) { "Generated OTP cannot be null" } assertEquals(entry.digits.toInt(), code.length) assertEquals("545293", code) @@ -94,7 +95,7 @@ class PasswordEntryAndroidTest { Date(8640000).time / (1000 * entry.totpPeriod), entry.totpAlgorithm, entry.digits - ) + ).get() assertNotNull(code) { "Generated OTP cannot be null" } assertEquals(entry.digits.toInt(), code.length) assertEquals("545293", code) 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 2cd31256..ea6255c4 100644 --- a/app/src/main/java/com/zeapo/pwdstore/model/PasswordEntry.kt +++ b/app/src/main/java/com/zeapo/pwdstore/model/PasswordEntry.kt @@ -4,6 +4,7 @@ */ package com.zeapo.pwdstore.model +import com.github.michaelbull.result.get import com.zeapo.pwdstore.utils.Otp import com.zeapo.pwdstore.utils.TotpFinder import com.zeapo.pwdstore.utils.UriTotpFinder @@ -55,7 +56,7 @@ class PasswordEntry(content: String, private val totpFinder: TotpFinder = UriTot fun calculateTotpCode(): String? { if (totpSecret == null) return null - return Otp.calculateCode(totpSecret, Date().time / (1000 * totpPeriod), totpAlgorithm, digits) + return Otp.calculateCode(totpSecret, Date().time / (1000 * totpPeriod), totpAlgorithm, digits).get() } val extraContentWithoutAuthData by lazy { diff --git a/app/src/main/java/com/zeapo/pwdstore/utils/Otp.kt b/app/src/main/java/com/zeapo/pwdstore/utils/Otp.kt index b2e4faba..994b4d82 100644 --- a/app/src/main/java/com/zeapo/pwdstore/utils/Otp.kt +++ b/app/src/main/java/com/zeapo/pwdstore/utils/Otp.kt @@ -5,10 +5,9 @@ package com.zeapo.pwdstore.utils -import com.github.ajalt.timberkt.e +import com.github.michaelbull.result.Err +import com.github.michaelbull.result.runCatching import java.nio.ByteBuffer -import java.security.InvalidKeyException -import java.security.NoSuchAlgorithmException import java.util.Locale import javax.crypto.Mac import javax.crypto.spec.SecretKeySpec @@ -24,26 +23,13 @@ object Otp { check(STEAM_ALPHABET.size == 26) } - fun calculateCode(secret: String, counter: Long, algorithm: String, digits: String): String? { + fun calculateCode(secret: String, counter: Long, algorithm: String, digits: String) = runCatching { val algo = "Hmac${algorithm.toUpperCase(Locale.ROOT)}" - val decodedSecret = try { - BASE_32.decode(secret) - } catch (e: Exception) { - e(e) { "Failed to decode secret" } - return null - } + val decodedSecret = BASE_32.decode(secret) val secretKey = SecretKeySpec(decodedSecret, algo) - val digest = try { - Mac.getInstance(algo).run { - init(secretKey) - doFinal(ByteBuffer.allocate(8).putLong(counter).array()) - } - } catch (e: NoSuchAlgorithmException) { - e(e) - return null - } catch (e: InvalidKeyException) { - e(e) { "Key is malformed" } - return null + val digest = Mac.getInstance(algo).run { + init(secretKey) + doFinal(ByteBuffer.allocate(8).putLong(counter).array()) } // Least significant 4 bits are used as an offset into the digest. val offset = (digest.last() and 0xf).toInt() @@ -52,7 +38,7 @@ object Otp { code[0] = (0x7f and code[0].toInt()).toByte() val codeInt = ByteBuffer.wrap(code).int check(codeInt > 0) - return if (digits == "s") { + if (digits == "s") { // Steam var remainingCodeInt = codeInt buildString { @@ -66,16 +52,13 @@ object Otp { val numDigits = digits.toIntOrNull() when { numDigits == null -> { - e { "Digits specifier has to be either 's' or numeric" } - return null + return Err(IllegalArgumentException("Digits specifier has to be either 's' or numeric")) } numDigits < 6 -> { - e { "TOTP codes have to be at least 6 digits long" } - return null + return Err(IllegalArgumentException("TOTP codes have to be at least 6 digits long")) } numDigits > 10 -> { - e { "TOTP codes can be at most 10 digits long" } - return null + return Err(IllegalArgumentException("TOTP codes can be at most 10 digits long")) } else -> { // 2^31 = 2_147_483_648, so we can extract at most 10 digits with the first one 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 f9c55167..6eecfe45 100644 --- a/app/src/test/java/com/zeapo/pwdstore/model/PasswordEntryTest.kt +++ b/app/src/test/java/com/zeapo/pwdstore/model/PasswordEntryTest.kt @@ -4,6 +4,7 @@ */ package com.zeapo.pwdstore.model +import com.github.michaelbull.result.get import com.zeapo.pwdstore.utils.Otp import com.zeapo.pwdstore.utils.TotpFinder import java.util.Date @@ -77,7 +78,7 @@ class PasswordEntryTest { Date(8640000).time / (1000 * entry.totpPeriod), entry.totpAlgorithm, entry.digits - ) + ).get() assertNotNull(code) { "Generated OTP cannot be null" } assertEquals(entry.digits.toInt(), code.length) assertEquals("545293", code) @@ -93,7 +94,7 @@ class PasswordEntryTest { Date(8640000).time / (1000 * entry.totpPeriod), entry.totpAlgorithm, entry.digits - ) + ).get() assertNotNull(code) { "Generated OTP cannot be null" } assertEquals(entry.digits.toInt(), code.length) assertEquals("545293", code) diff --git a/app/src/test/java/com/zeapo/pwdstore/utils/OtpTest.kt b/app/src/test/java/com/zeapo/pwdstore/utils/OtpTest.kt index 6e6e7f21..d6938019 100644 --- a/app/src/test/java/com/zeapo/pwdstore/utils/OtpTest.kt +++ b/app/src/test/java/com/zeapo/pwdstore/utils/OtpTest.kt @@ -5,6 +5,7 @@ package com.zeapo.pwdstore.utils +import com.github.michaelbull.result.get import kotlin.test.assertEquals import kotlin.test.assertNotNull import kotlin.test.assertNull @@ -14,31 +15,31 @@ class OtpTest { @Test fun testOtpGeneration6Digits() { - assertEquals("953550", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333298159 / (1000 * 30), "SHA1", "6")) - assertEquals("275379", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333571918 / (1000 * 30), "SHA1", "6")) - assertEquals("867507", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333600517 / (1000 * 57), "SHA1", "6")) + assertEquals("953550", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333298159 / (1000 * 30), "SHA1", "6").get()) + assertEquals("275379", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333571918 / (1000 * 30), "SHA1", "6").get()) + assertEquals("867507", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333600517 / (1000 * 57), "SHA1", "6").get()) } @Test fun testOtpGeneration10Digits() { - assertEquals("0740900914", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333655044 / (1000 * 30), "SHA1", "10")) - assertEquals("0070632029", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333691405 / (1000 * 30), "SHA1", "10")) - assertEquals("1017265882", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333728893 / (1000 * 83), "SHA1", "10")) + assertEquals("0740900914", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333655044 / (1000 * 30), "SHA1", "10").get()) + assertEquals("0070632029", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333691405 / (1000 * 30), "SHA1", "10").get()) + assertEquals("1017265882", Otp.calculateCode("JBSWY3DPEHPK3PXP", 1593333728893 / (1000 * 83), "SHA1", "10").get()) } @Test fun testOtpGenerationIllegalInput() { - assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA0", "10")) - assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA1", "a")) - assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA1", "5")) - assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA1", "11")) - assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXPAAAAB", 10000, "SHA1", "6")) + assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA0", "10").get()) + assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA1", "a").get()) + assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA1", "5").get()) + assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXP", 10000, "SHA1", "11").get()) + assertNull(Otp.calculateCode("JBSWY3DPEHPK3PXPAAAAB", 10000, "SHA1", "6").get()) } @Test fun testOtpGenerationUnusualSecrets() { - assertEquals("127764", Otp.calculateCode("JBSWY3DPEHPK3PXPAAAAAAAA", 1593367111963 / (1000 * 30), "SHA1", "6")) - assertEquals("047515", Otp.calculateCode("JBSWY3DPEHPK3PXPAAAAA", 1593367171420 / (1000 * 30), "SHA1", "6")) + assertEquals("127764", Otp.calculateCode("JBSWY3DPEHPK3PXPAAAAAAAA", 1593367111963 / (1000 * 30), "SHA1", "6").get()) + assertEquals("047515", Otp.calculateCode("JBSWY3DPEHPK3PXPAAAAA", 1593367171420 / (1000 * 30), "SHA1", "6").get()) } @Test @@ -46,8 +47,8 @@ class OtpTest { // Secret was generated with `echo 'string with some padding needed' | base32` // We don't care for the resultant OTP's actual value, we just want both the padded and // unpadded variant to generate the same one. - val unpaddedOtp = Otp.calculateCode("ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA", 1593367171420 / (1000 * 30), "SHA1", "6") - val paddedOtp = Otp.calculateCode("ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA====", 1593367171420 / (1000 * 30), "SHA1", "6") + val unpaddedOtp = Otp.calculateCode("ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA", 1593367171420 / (1000 * 30), "SHA1", "6").get() + val paddedOtp = Otp.calculateCode("ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA====", 1593367171420 / (1000 * 30), "SHA1", "6").get() assertNotNull(unpaddedOtp) assertNotNull(paddedOtp) assertEquals(unpaddedOtp, paddedOtp)