From 6d73b63435a2760ec9026090ecfea4846157b6ab Mon Sep 17 00:00:00 2001 From: Newton Cesar Date: Sat, 8 Oct 2022 12:03:29 -0300 Subject: 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 +--------- .../passwordstore/data/passfile/PasswordEntry.kt | 14 +++++++-- .../kotlin/app/passwordstore/util/time/Clocks.kt | 26 ----------------- .../app/passwordstore/util/time/UserClock.kt | 26 +++++++++++++++++ .../main/kotlin/app/passwordstore/util/totp/Otp.kt | 33 ++++++++++++++-------- .../data/passfile/PasswordEntryTest.kt | 1 + .../app/passwordstore/util/time/TestClocks.kt | 33 ---------------------- .../app/passwordstore/util/time/TestUserClock.kt | 33 ++++++++++++++++++++++ 8 files changed, 94 insertions(+), 87 deletions(-) delete mode 100644 format-common/src/main/kotlin/app/passwordstore/util/time/Clocks.kt create mode 100644 format-common/src/main/kotlin/app/passwordstore/util/time/UserClock.kt delete mode 100644 format-common/src/test/kotlin/app/passwordstore/util/time/TestClocks.kt create mode 100644 format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt 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/Clocks.kt deleted file mode 100644 index 4ffeb6a6..00000000 --- a/format-common/src/main/kotlin/app/passwordstore/util/time/Clocks.kt +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved. - * SPDX-License-Identifier: GPL-3.0-only - */ - -package app.passwordstore.util.time - -import java.time.Clock -import java.time.Instant -import java.time.ZoneId -import javax.inject.Inject - -/** - * A subclass of [Clock] that uses [Clock.systemDefaultZone] to get a clock that works for the - * user's current time zone. - */ -public open class UserClock @Inject constructor() : Clock() { - - private val clock = systemDefaultZone() - - override fun withZone(zone: ZoneId): Clock = clock.withZone(zone) - - override fun getZone(): ZoneId = clock.zone - - override fun instant(): Instant = clock.instant() -} diff --git a/format-common/src/main/kotlin/app/passwordstore/util/time/UserClock.kt b/format-common/src/main/kotlin/app/passwordstore/util/time/UserClock.kt new file mode 100644 index 00000000..4ffeb6a6 --- /dev/null +++ b/format-common/src/main/kotlin/app/passwordstore/util/time/UserClock.kt @@ -0,0 +1,26 @@ +/* + * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved. + * SPDX-License-Identifier: GPL-3.0-only + */ + +package app.passwordstore.util.time + +import java.time.Clock +import java.time.Instant +import java.time.ZoneId +import javax.inject.Inject + +/** + * A subclass of [Clock] that uses [Clock.systemDefaultZone] to get a clock that works for the + * user's current time zone. + */ +public open class UserClock @Inject constructor() : Clock() { + + private val clock = systemDefaultZone() + + override fun withZone(zone: ZoneId): Clock = clock.withZone(zone) + + override fun getZone(): ZoneId = clock.zone + + override fun instant(): Instant = clock.instant() +} 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/TestClocks.kt deleted file mode 100644 index 8a860a39..00000000 --- a/format-common/src/test/kotlin/app/passwordstore/util/time/TestClocks.kt +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved. - * SPDX-License-Identifier: GPL-3.0-only - */ - -package app.passwordstore.util.time - -import java.time.Clock -import java.time.Instant -import java.time.ZoneId -import java.time.ZoneOffset.UTC - -/** - * Implementation of [UserClock] that is fixed to [Instant.EPOCH] for deterministic time-based tests - */ -class TestUserClock(instant: Instant) : UserClock() { - - constructor() : this(Instant.EPOCH) - - private var clock = fixed(instant, UTC) - - override fun withZone(zone: ZoneId): Clock = clock.withZone(zone) - - override fun getZone(): ZoneId = UTC - - override fun instant(): Instant = clock.instant() - - companion object { - fun withAddedSeconds(secondsToAdd: Long): TestUserClock { - return TestUserClock(Instant.EPOCH.plusSeconds(secondsToAdd)) - } - } -} diff --git a/format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt b/format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt new file mode 100644 index 00000000..8a860a39 --- /dev/null +++ b/format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt @@ -0,0 +1,33 @@ +/* + * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved. + * SPDX-License-Identifier: GPL-3.0-only + */ + +package app.passwordstore.util.time + +import java.time.Clock +import java.time.Instant +import java.time.ZoneId +import java.time.ZoneOffset.UTC + +/** + * Implementation of [UserClock] that is fixed to [Instant.EPOCH] for deterministic time-based tests + */ +class TestUserClock(instant: Instant) : UserClock() { + + constructor() : this(Instant.EPOCH) + + private var clock = fixed(instant, UTC) + + override fun withZone(zone: ZoneId): Clock = clock.withZone(zone) + + override fun getZone(): ZoneId = UTC + + override fun instant(): Instant = clock.instant() + + companion object { + fun withAddedSeconds(secondsToAdd: Long): TestUserClock { + return TestUserClock(Instant.EPOCH.plusSeconds(secondsToAdd)) + } + } +} -- cgit v1.2.3