From 6d73b63435a2760ec9026090ecfea4846157b6ab Mon Sep 17 00:00:00 2001 From: Newton Cesar Date: Sat, 8 Oct 2022 12:03:29 -0300 Subject: [PATCH] Fix detekt warnings in format-common (#2167) * fix warning ktlint to format-common folder * update to code review * adjusted spotless --- detekt-baselines/format-common.xml | 15 +-------- .../data/passfile/PasswordEntry.kt | 14 ++++++-- .../util/time/{Clocks.kt => UserClock.kt} | 0 .../kotlin/app/passwordstore/util/totp/Otp.kt | 33 ++++++++++++------- .../data/passfile/PasswordEntryTest.kt | 1 + .../time/{TestClocks.kt => TestUserClock.kt} | 0 6 files changed, 35 insertions(+), 28 deletions(-) rename format-common/src/main/kotlin/app/passwordstore/util/time/{Clocks.kt => UserClock.kt} (100%) rename format-common/src/test/kotlin/app/passwordstore/util/time/{TestClocks.kt => TestUserClock.kt} (100%) diff --git a/detekt-baselines/format-common.xml b/detekt-baselines/format-common.xml index 9ff123b7..c373eea4 100644 --- a/detekt-baselines/format-common.xml +++ b/detekt-baselines/format-common.xml @@ -1,18 +1,5 @@ - - MagicNumber:Otp.kt$Otp$0x7f - MagicNumber:Otp.kt$Otp$10 - MagicNumber:Otp.kt$Otp$26 - MagicNumber:Otp.kt$Otp$4 - MagicNumber:Otp.kt$Otp$5 - MagicNumber:Otp.kt$Otp$6 - MagicNumber:Otp.kt$Otp$8 - MagicNumber:PasswordEntry.kt$PasswordEntry$1000 - MatchingDeclarationName:Clocks.kt$UserClock : Clock - MatchingDeclarationName:TestClocks.kt$TestUserClock : UserClock - MaxLineLength:PasswordEntryTest.kt$PasswordEntryTest.Companion$"otpauth://totp/ACME%20Co:john@example.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=SHA1&digits=6&period=30" - ReturnCount:PasswordEntry.kt$PasswordEntry$private fun findAndStripPassword(passContent: List<String>): Pair<String?, List<String>> - + diff --git a/format-common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt b/format-common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt index 9543eb8c..38aa4a20 100644 --- a/format-common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt +++ b/format-common/src/main/kotlin/app/passwordstore/data/passfile/PasswordEntry.kt @@ -62,7 +62,7 @@ constructor( do { val otp = calculateTotp() emit(otp) - delay(1000L) + delay(ONE_SECOND.seconds) } while (coroutineContext.isActive) } else { awaitCancellation() @@ -90,6 +90,7 @@ constructor( return totpSecret != null } + @Suppress("ReturnCount") private fun findAndStripPassword(passContent: List): Pair> { if (TotpFinder.TOTP_FIELDS.any { passContent[0].startsWith(it) }) return Pair(null, passContent) for (line in passContent) { @@ -180,8 +181,14 @@ constructor( val totpAlgorithm = totpFinder.findAlgorithm(content) val issuer = totpFinder.findIssuer(content) val millis = clock.millis() - val remainingTime = (totpPeriod - ((millis / 1000) % totpPeriod)).seconds - Otp.calculateCode(totpSecret!!, millis / (1000 * totpPeriod), totpAlgorithm, digits, issuer) + val remainingTime = (totpPeriod - ((millis / ONE_SECOND) % totpPeriod)).seconds + Otp.calculateCode( + totpSecret!!, + millis / (ONE_SECOND * totpPeriod), + totpAlgorithm, + digits, + issuer + ) .mapBoth( { code -> return Totp(code, remainingTime) @@ -217,5 +224,6 @@ constructor( "secret:", "pass:", ) + private const val ONE_SECOND = 1000 } } diff --git a/format-common/src/main/kotlin/app/passwordstore/util/time/Clocks.kt b/format-common/src/main/kotlin/app/passwordstore/util/time/UserClock.kt similarity index 100% rename from format-common/src/main/kotlin/app/passwordstore/util/time/Clocks.kt rename to format-common/src/main/kotlin/app/passwordstore/util/time/UserClock.kt diff --git a/format-common/src/main/kotlin/app/passwordstore/util/totp/Otp.kt b/format-common/src/main/kotlin/app/passwordstore/util/totp/Otp.kt index 72d5cffe..5abc0337 100644 --- a/format-common/src/main/kotlin/app/passwordstore/util/totp/Otp.kt +++ b/format-common/src/main/kotlin/app/passwordstore/util/totp/Otp.kt @@ -18,6 +18,13 @@ internal object Otp { private val BASE_32 = Base32() private val STEAM_ALPHABET = "23456789BCDFGHJKMNPQRTVWXY".toCharArray() + private const val BYTE_BUFFER_CAPACITY = 8 + private const val END_INDEX_OFFSET = 4 + private const val STEAM_GUARD_DIGITS = 5 + private const val MINIMUM_DIGITS = 6 + private const val MAXIMUM_DIGITS = 10 + private const val ALPHABET_LENGTH = 26 + private const val MOST_SIGNIFICANT_BYTE = 0x7f fun calculateCode( secret: String, @@ -32,13 +39,13 @@ internal object Otp { val digest = Mac.getInstance(algo).run { init(secretKey) - doFinal(ByteBuffer.allocate(8).putLong(counter).array()) + doFinal(ByteBuffer.allocate(BYTE_BUFFER_CAPACITY).putLong(counter).array()) } // Least significant 4 bits are used as an offset into the digest. val offset = (digest.last() and 0xf).toInt() // Extract 32 bits at the offset and clear the most significant bit. - val code = digest.copyOfRange(offset, offset + 4) - code[0] = (0x7f and code[0].toInt()).toByte() + val code = digest.copyOfRange(offset, offset.plus(END_INDEX_OFFSET)) + code[0] = (MOST_SIGNIFICANT_BYTE and code[0].toInt()).toByte() val codeInt = ByteBuffer.wrap(code).int check(codeInt > 0) // SteamGuard is a horrible OTP implementation that generates non-standard 5 digit OTPs as @@ -47,9 +54,9 @@ internal object Otp { if (digits == "s" || issuer == "Steam") { var remainingCodeInt = codeInt buildString { - repeat(5) { + repeat(STEAM_GUARD_DIGITS) { append(STEAM_ALPHABET[remainingCodeInt % STEAM_ALPHABET.size]) - remainingCodeInt /= 26 + remainingCodeInt /= ALPHABET_LENGTH } } } else { @@ -59,17 +66,21 @@ internal object Otp { numDigits == null -> { return Err(IllegalArgumentException("Digits specifier has to be either 's' or numeric")) } - numDigits < 6 -> { - return Err(IllegalArgumentException("TOTP codes have to be at least 6 digits long")) + numDigits < MINIMUM_DIGITS -> { + return Err( + IllegalArgumentException("TOTP codes have to be at least $MINIMUM_DIGITS digits long") + ) } - numDigits > 10 -> { - return Err(IllegalArgumentException("TOTP codes can be at most 10 digits long")) + numDigits > MAXIMUM_DIGITS -> { + return Err( + IllegalArgumentException("TOTP codes can be at most $MAXIMUM_DIGITS digits long") + ) } else -> { // 2^31 = 2_147_483_648, so we can extract at most 10 digits with the first one // always being 0, 1, or 2. Pad with leading zeroes. - val codeStringBase10 = codeInt.toString(10).padStart(10, '0') - check(codeStringBase10.length == 10) + val codeStringBase10 = codeInt.toString(MAXIMUM_DIGITS).padStart(MAXIMUM_DIGITS, '0') + check(codeStringBase10.length == MAXIMUM_DIGITS) codeStringBase10.takeLast(numDigits) } } diff --git a/format-common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt b/format-common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt index f492604d..2022d968 100644 --- a/format-common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt +++ b/format-common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt @@ -194,6 +194,7 @@ class PasswordEntryTest { companion object { + @Suppress("MaxLineLength") 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/format-common/src/test/kotlin/app/passwordstore/util/time/TestClocks.kt b/format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt similarity index 100% rename from format-common/src/test/kotlin/app/passwordstore/util/time/TestClocks.kt rename to format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt