diff options
12 files changed, 73 insertions, 49 deletions
diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 32291d33..54bf2af7 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -56,7 +56,7 @@ dependencies { implementation(projects.autofillParser) implementation(projects.coroutineUtils) implementation(projects.cryptoPgpainless) - implementation(projects.formatCommon) + implementation(projects.formatCommonImpl) implementation(projects.passgen.diceware) implementation(projects.passgen.random) implementation(projects.uiCompose) diff --git a/detekt-baselines/app.xml b/detekt-baselines/app.xml index 5a5f9df9..4ef4ba76 100644 --- a/detekt-baselines/app.xml +++ b/detekt-baselines/app.xml @@ -14,7 +14,6 @@ <ID>ComplexMethod:PasswordCreationActivity.kt$PasswordCreationActivity$override fun onCreate(savedInstanceState: Bundle?)</ID> <ID>ComplexMethod:PasswordCreationActivity.kt$PasswordCreationActivity$private fun encrypt()</ID> <ID>ComplexMethod:PasswordFragment.kt$PasswordFragment$private fun initializePasswordList()</ID> - <ID>EmptyDoWhileBlock:PasswordDialog.kt$PasswordDialog${}</ID> <ID>EmptyFunctionBlock:BasicBottomSheet.kt$BasicBottomSheet.<no name provided>${}</ID> <ID>EmptyFunctionBlock:ItemCreationBottomSheet.kt$ItemCreationBottomSheet.<no name provided>${}</ID> <ID>EmptyFunctionBlock:PasswordFragment.kt$PasswordFragment.<no name provided>.<no name provided>${}</ID> @@ -58,7 +57,6 @@ <ID>MagicNumber:SshKey.kt$SshKey.Algorithm.Ecdsa$256</ID> <ID>MagicNumber:SshKey.kt$SshKey.Algorithm.Rsa$3072</ID> <ID>MagicNumber:SshjSessionFactory.kt$SshjSession$22</ID> - <ID>MagicNumber:UriTotpFinder.kt$UriTotpFinder$30</ID> <ID>MatchingDeclarationName:AutofillViewUtils.kt$DatasetMetadata</ID> <ID>MaxLineLength:BaseGitActivity.kt$BaseGitActivity$"The server does not support multiple Git operations per SSH session. Please try again, a slower fallback mode will be used."</ID> <ID>MaxLineLength:BaseGitActivity.kt$BaseGitActivity$"WARNING: The remote host key has changed. If this is expected, please go to Git server settings and clear the saved host key."</ID> @@ -82,7 +80,6 @@ <ID>ReturnCount:OreoAutofillService.kt$OreoAutofillService$override fun onSaveRequest(request: SaveRequest, callback: SaveCallback)</ID> <ID>ReturnCount:PasswordStore.kt$PasswordStore$override fun onKeyDown(keyCode: Int, event: KeyEvent): Boolean</ID> <ID>ReturnCount:ShortcutHandler.kt$ShortcutHandler$fun addPinnedShortcut(item: PasswordItem, intent: Intent)</ID> - <ID>ReturnCount:UriTotpFinder.kt$UriTotpFinder$override fun findSecret(content: String): String?</ID> <ID>SpreadOperator:Api30AutofillResponseBuilder.kt$Api30AutofillResponseBuilder$(*ignoredIds.toTypedArray())</ID> <ID>SpreadOperator:AutofillResponseBuilder.kt$AutofillResponseBuilder$(*ignoredIds.toTypedArray())</ID> <ID>SpreadOperator:BreakOutOfDetached.kt$BreakOutOfDetached$( // abort the rebase git.rebase().setOperation(RebaseCommand.Operation.ABORT), *resetCommands, )</ID> diff --git a/detekt-baselines/format-common-impl.xml b/detekt-baselines/format-common-impl.xml new file mode 100644 index 00000000..5a23ddfe --- /dev/null +++ b/detekt-baselines/format-common-impl.xml @@ -0,0 +1,7 @@ +<?xml version="1.0" ?> +<SmellBaseline> + <ManuallySuppressedIssues></ManuallySuppressedIssues> + <CurrentIssues> + <ID>ReturnCount:UriTotpFinder.kt$UriTotpFinder$override fun findSecret(content: String): String?</ID> + </CurrentIssues> +</SmellBaseline> diff --git a/format-common-impl/build.gradle.kts b/format-common-impl/build.gradle.kts new file mode 100644 index 00000000..c3092f6d --- /dev/null +++ b/format-common-impl/build.gradle.kts @@ -0,0 +1,17 @@ +plugins { + id("com.github.android-password-store.android-library") + id("com.github.android-password-store.kotlin-android") + id("com.github.android-password-store.kotlin-library") +} + +android { namespace = "app.passwordstore.format.common.impl" } + +dependencies { + api(projects.formatCommon) + implementation(libs.dagger.hilt.core) + testImplementation(projects.coroutineUtilsTesting) + testImplementation(libs.bundles.testDependencies) + testImplementation(libs.kotlin.coroutines.test) + testImplementation(libs.testing.robolectric) + testImplementation(libs.testing.turbine) +} diff --git a/app/src/main/java/app/passwordstore/util/totp/UriTotpFinder.kt b/format-common-impl/src/main/kotlin/app/passwordstore/util/totp/UriTotpFinder.kt index 30447690..741a21a7 100644 --- a/app/src/main/java/app/passwordstore/util/totp/UriTotpFinder.kt +++ b/format-common-impl/src/main/kotlin/app/passwordstore/util/totp/UriTotpFinder.kt @@ -1,15 +1,14 @@ -/* - * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved. - * SPDX-License-Identifier: GPL-3.0-only - */ - package app.passwordstore.util.totp import android.net.Uri import javax.inject.Inject /** [Uri] backed TOTP URL parser. */ -class UriTotpFinder @Inject constructor() : TotpFinder { +public class UriTotpFinder @Inject constructor() : TotpFinder { + + private companion object { + private const val DEFAULT_TOTP_PERIOD = 30L + } override fun findSecret(content: String): String? { content.split("\n".toRegex()).forEach { line -> @@ -28,7 +27,7 @@ class UriTotpFinder @Inject constructor() : TotpFinder { } override fun findPeriod(content: String): Long { - return getQueryParameter(content, "period")?.toLongOrNull() ?: 30 + return getQueryParameter(content, "period")?.toLongOrNull() ?: DEFAULT_TOTP_PERIOD } override fun findAlgorithm(content: String): String { diff --git a/format-common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt b/format-common-impl/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt index 2022d968..1fa50188 100644 --- a/format-common/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt +++ b/format-common-impl/src/test/kotlin/app/passwordstore/data/passfile/PasswordEntryTest.kt @@ -9,7 +9,7 @@ import app.cash.turbine.test import app.passwordstore.test.CoroutineTestRule import app.passwordstore.util.time.TestUserClock import app.passwordstore.util.time.UserClock -import app.passwordstore.util.totp.TotpFinder +import app.passwordstore.util.totp.UriTotpFinder import java.util.Locale import kotlin.test.Test import kotlin.test.assertEquals @@ -21,15 +21,20 @@ import kotlin.time.ExperimentalTime import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Rule +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner @OptIn(ExperimentalCoroutinesApi::class, ExperimentalTime::class) +@RunWith(RobolectricTestRunner::class) class PasswordEntryTest { @get:Rule val coroutineTestRule: CoroutineTestRule = CoroutineTestRule() + private val totpFinder = UriTotpFinder() + private fun makeEntry(content: String, clock: UserClock = fakeClock) = PasswordEntry( clock, - testFinder, + totpFinder, content.encodeToByteArray(), ) @@ -144,7 +149,7 @@ class PasswordEntryTest { } /** - * Same as [testGeneratesOtpFromTotpUri], but advances the clock by 5 seconds. This exercises the + * Same as [generatesOtpFromTotpUri], but advances the clock by 5 seconds. This exercises the * [Totp.remainingTime] calculation logic, and acts as a regression test to resolve the bug which * blocked https://msfjarvis.dev/aps/issue/1550. */ @@ -199,28 +204,5 @@ class PasswordEntryTest { "otpauth://totp/ACME%20Co:john@example.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=SHA1&digits=6&period=30" val fakeClock = TestUserClock() - - // This implementation is hardcoded for the URI above. - val testFinder = - object : TotpFinder { - override fun findSecret(content: String): String { - return "HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ" - } - - override fun findDigits(content: String): String { - return "6" - } - - override fun findPeriod(content: String): Long { - return 30 - } - - override fun findAlgorithm(content: String): String { - return "SHA1" - } - override fun findIssuer(content: String): String { - return "ACME Co" - } - } } } diff --git a/format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt b/format-common-impl/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt index 8a860a39..8a860a39 100644 --- a/format-common/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt +++ b/format-common-impl/src/test/kotlin/app/passwordstore/util/time/TestUserClock.kt diff --git a/format-common/src/test/kotlin/app/passwordstore/util/totp/OtpTest.kt b/format-common-impl/src/test/kotlin/app/passwordstore/util/totp/OtpTest.kt index bf8cd186..54ac492d 100644 --- a/format-common/src/test/kotlin/app/passwordstore/util/totp/OtpTest.kt +++ b/format-common-impl/src/test/kotlin/app/passwordstore/util/totp/OtpTest.kt @@ -137,7 +137,7 @@ class OtpTest { ) val paddedOtp = generateOtp( - 1593367171420 / (1000 * 30), + counter = 1593367171420 / (1000 * 30), secret = "ON2HE2LOM4QHO2LUNAQHG33NMUQHAYLEMRUW4ZZANZSWKZDFMQFA====", ) @@ -145,4 +145,24 @@ class OtpTest { assertNotNull(paddedOtp) assertEquals(unpaddedOtp, paddedOtp) } + + @Test + fun generateSteamTotp() { + val issuerOtp = + generateOtp( + counter = 48297900 / (1000 * 30), + secret = "STK7746GVMCHMNH5FBIAQXGPV3I7ZHRG", + issuer = "Steam", + ) + val digitsOtp = + generateOtp( + counter = 48297900 / (1000 * 30), + secret = "STK7746GVMCHMNH5FBIAQXGPV3I7ZHRG", + digits = "s", + ) + assertNotNull(issuerOtp) + assertNotNull(digitsOtp) + assertEquals("6M3CT", issuerOtp) + assertEquals("6M3CT", digitsOtp) + } } diff --git a/format-common/build.gradle.kts b/format-common/build.gradle.kts index f18dc906..722f5259 100644 --- a/format-common/build.gradle.kts +++ b/format-common/build.gradle.kts @@ -9,14 +9,10 @@ plugins { } dependencies { + api(libs.thirdparty.kotlinResult) implementation(projects.coroutineUtils) implementation(libs.androidx.annotation) implementation(libs.dagger.hilt.core) implementation(libs.thirdparty.commons.codec) - implementation(libs.thirdparty.kotlinResult) implementation(libs.kotlin.coroutines.core) - testImplementation(projects.coroutineUtilsTesting) - testImplementation(libs.bundles.testDependencies) - testImplementation(libs.kotlin.coroutines.test) - testImplementation(libs.testing.turbine) } 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 38aa4a20..d48c2ca3 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 @@ -5,6 +5,7 @@ package app.passwordstore.data.passfile +import androidx.annotation.VisibleForTesting import app.passwordstore.util.time.UserClock import app.passwordstore.util.totp.Otp import app.passwordstore.util.totp.TotpFinder @@ -202,10 +203,11 @@ constructor( public fun create(bytes: ByteArray): PasswordEntry } - internal companion object { + public companion object { private const val EXTRA_CONTENT = "Extra Content" - internal val USERNAME_FIELDS = + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + public val USERNAME_FIELDS: Array<String> = arrayOf( "login:", "username:", @@ -218,7 +220,8 @@ constructor( "id:", "identity:", ) - internal val PASSWORD_FIELDS = + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + public val PASSWORD_FIELDS: Array<String> = arrayOf( "password:", "secret:", 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 5abc0337..10e771fe 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 @@ -6,6 +6,7 @@ package app.passwordstore.util.totp import com.github.michaelbull.result.Err +import com.github.michaelbull.result.Result import com.github.michaelbull.result.runCatching import java.nio.ByteBuffer import java.util.Locale @@ -14,7 +15,7 @@ import javax.crypto.spec.SecretKeySpec import kotlin.experimental.and import org.apache.commons.codec.binary.Base32 -internal object Otp { +public object Otp { private val BASE_32 = Base32() private val STEAM_ALPHABET = "23456789BCDFGHJKMNPQRTVWXY".toCharArray() @@ -26,13 +27,13 @@ internal object Otp { private const val ALPHABET_LENGTH = 26 private const val MOST_SIGNIFICANT_BYTE = 0x7f - fun calculateCode( + public fun calculateCode( secret: String, counter: Long, algorithm: String, digits: String, issuer: String?, - ) = runCatching { + ): Result<String, Throwable> = runCatching { val algo = "Hmac${algorithm.uppercase(Locale.ROOT)}" val decodedSecret = BASE_32.decode(secret) val secretKey = SecretKeySpec(decodedSecret, algo) diff --git a/settings.gradle.kts b/settings.gradle.kts index df15a3a0..6e6ce14f 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -178,6 +178,8 @@ include("crypto-pgpainless") include("format-common") +include("format-common-impl") + include("passgen:diceware") include("passgen:random") |