From 936418800dd58dcc9b5780fe15b815b6752df7a7 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 26 Aug 2026 17:22:54 +0200 Subject: [PATCH 01/12] feat(otp): add sms code detection repository --- feature/autofill/build.gradle.kts | 17 +++- .../repository/NoopSmsCodeRepository.kt | 15 +++ .../autofill/domain/model/SmsCodeEvent.kt | 8 ++ .../domain/repository/SmsCodeRepository.kt | 10 ++ .../domain/repository/GmsSmsCodeRepository.kt | 93 +++++++++++++++++++ gradle/libs.versions.toml | 3 + 6 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/domain/repository/NoopSmsCodeRepository.kt create mode 100644 feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/model/SmsCodeEvent.kt create mode 100644 feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/repository/SmsCodeRepository.kt create mode 100644 feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/domain/repository/GmsSmsCodeRepository.kt diff --git a/feature/autofill/build.gradle.kts b/feature/autofill/build.gradle.kts index 39daa7419..51e1c21ef 100644 --- a/feature/autofill/build.gradle.kts +++ b/feature/autofill/build.gradle.kts @@ -7,13 +7,20 @@ plugins { android { namespace = "de.davis.keygo.feature.autofill" - defaultConfig { - missingDimensionStrategy("store", "playStore") - } - testFixtures { enable = true } + + flavorDimensions += listOf("store") + productFlavors { + create("playStore") { + dimension = "store" + } + + create("fdroid") { + dimension = "store" + } + } } dependencies { @@ -24,6 +31,8 @@ dependencies { implementation(libs.okhttp) implementation(libs.kotlinx.collections.immutable) + "playStoreImplementation"(libs.gms.auth.api.phone) + implementation(projects.core.item) implementation(projects.core.util) implementation(projects.core.ui) diff --git a/feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/domain/repository/NoopSmsCodeRepository.kt b/feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/domain/repository/NoopSmsCodeRepository.kt new file mode 100644 index 000000000..4ffc4ded9 --- /dev/null +++ b/feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/domain/repository/NoopSmsCodeRepository.kt @@ -0,0 +1,15 @@ +package de.davis.keygo.feature.autofill.domain.repository + +import de.davis.keygo.feature.autofill.domain.model.SmsCodeEvent +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flowOf +import org.koin.core.annotation.Single + +@Single +internal class NoopSmsCodeRepository : SmsCodeRepository { + + override suspend fun canOfferSuggestion(targetPackage: String): Boolean = false + + override fun smsCodes(): Flow = + flowOf(SmsCodeEvent.Failed(IllegalStateException("SMS code autofill unavailable"))) +} \ No newline at end of file diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/model/SmsCodeEvent.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/model/SmsCodeEvent.kt new file mode 100644 index 000000000..d6071b1b2 --- /dev/null +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/model/SmsCodeEvent.kt @@ -0,0 +1,8 @@ +package de.davis.keygo.feature.autofill.domain.model + +sealed interface SmsCodeEvent { + data class SmsCodeReceived(val code: String) : SmsCodeEvent + data class Failed(val cause: Throwable) : SmsCodeEvent + + data object Timeout : SmsCodeEvent +} diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/repository/SmsCodeRepository.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/repository/SmsCodeRepository.kt new file mode 100644 index 000000000..267683b10 --- /dev/null +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/repository/SmsCodeRepository.kt @@ -0,0 +1,10 @@ +package de.davis.keygo.feature.autofill.domain.repository + +import de.davis.keygo.feature.autofill.domain.model.SmsCodeEvent +import kotlinx.coroutines.flow.Flow + +interface SmsCodeRepository { + + suspend fun canOfferSuggestion(targetPackage: String): Boolean + fun smsCodes(): Flow +} \ No newline at end of file diff --git a/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/domain/repository/GmsSmsCodeRepository.kt b/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/domain/repository/GmsSmsCodeRepository.kt new file mode 100644 index 000000000..1c7bdc8e7 --- /dev/null +++ b/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/domain/repository/GmsSmsCodeRepository.kt @@ -0,0 +1,93 @@ +package de.davis.keygo.feature.autofill.domain.repository + +import android.content.BroadcastReceiver +import android.content.Context +import android.content.Intent +import android.content.IntentFilter +import android.util.Log +import androidx.core.content.ContextCompat +import androidx.core.content.IntentCompat +import com.google.android.gms.auth.api.phone.SmsCodeAutofillClient +import com.google.android.gms.auth.api.phone.SmsCodeRetriever +import com.google.android.gms.auth.api.phone.SmsRetriever +import com.google.android.gms.common.api.ApiException +import com.google.android.gms.common.api.CommonStatusCodes +import com.google.android.gms.common.api.Status +import de.davis.keygo.feature.autofill.domain.model.SmsCodeEvent +import kotlinx.coroutines.channels.awaitClose +import kotlinx.coroutines.flow.callbackFlow +import kotlinx.coroutines.tasks.await +import org.koin.core.annotation.Single + +@Single +internal class GmsSmsCodeRepository( + private val context: Context +) : SmsCodeRepository { + + private val client by lazy { + SmsCodeRetriever.getAutofillClient(context) + } + + override suspend fun canOfferSuggestion(targetPackage: String): Boolean = try { + if (client.hasOngoingSmsRequest(targetPackage).await()) return false + when (client.checkPermissionState().await()) { + SmsCodeAutofillClient.PermissionState.GRANTED, + SmsCodeAutofillClient.PermissionState.NONE -> true + + else -> false + } + } catch (e: ApiException) { + Log.e(TAG, "SMS code autofill unavailable: ${e.statusCode}", e) + false + } + + override fun smsCodes() = callbackFlow { + val receiver = object : BroadcastReceiver() { + override fun onReceive(c: Context, intent: Intent) { + if (intent.action != SmsCodeRetriever.SMS_CODE_RETRIEVED_ACTION) return + val status = IntentCompat.getParcelableExtra( + intent, SmsRetriever.EXTRA_STATUS, Status::class.java + ) + + when (status?.statusCode) { + CommonStatusCodes.SUCCESS -> { + val code = intent.getStringExtra(SmsCodeRetriever.EXTRA_SMS_CODE) + trySend( + if (code.isNullOrBlank()) + SmsCodeEvent.Failed(IllegalStateException("empty code")) + else SmsCodeEvent.SmsCodeReceived(code) + ) + } + + CommonStatusCodes.TIMEOUT -> trySend(SmsCodeEvent.Timeout) + else -> trySend( + SmsCodeEvent.Failed(IllegalStateException("status=${status?.statusCode}")) + ) + } + } + } + + ContextCompat.registerReceiver( + context, + receiver, + IntentFilter(SmsCodeRetriever.SMS_CODE_RETRIEVED_ACTION), + SmsRetriever.SEND_PERMISSION, + null, + ContextCompat.RECEIVER_EXPORTED + ) + + // Start only after the receiver is live, otherwise a code that is + // already sitting in the inbox can be delivered before you listen. + try { + client.startSmsCodeRetriever().await() + } catch (e: Exception) { + trySend(SmsCodeEvent.Failed(e)) + } + + awaitClose { runCatching { context.unregisterReceiver(receiver) } } + } + + private companion object { + const val TAG = "GmsSmsCodeRepository" + } +} \ No newline at end of file diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 52a063036..a1e95b3d6 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -7,6 +7,7 @@ autofill = "1.3.0" accompanist = "0.37.3" camera = "1.6.1" credentials = "1.6.0" +gmsAuthApiPhone = "18.3.1" gmsMlkitBarcodeScanning = "18.3.1" okhttp = "5.5.0" zxingBarcodeScanning = "3.5.4" @@ -117,6 +118,8 @@ jna = { group = "net.java.dev.jna", name = "jna", version.ref = "jna" } offrange-passgen = { group = "com.github.offrange", name = "passgen", version.ref = "passGen" } +gms-auth-api-phone = { group = "com.google.android.gms", name = "play-services-auth-api-phone", version.ref = "gmsAuthApiPhone" } + gms-mlkit-barcode-scanning = { group = "com.google.android.gms", name = "play-services-mlkit-barcode-scanning", version.ref = "gmsMlkitBarcodeScanning" } zxing-barcode-scanning = { group = "com.google.zxing", name = "core", version.ref = "zxingBarcodeScanning" } devnied-emvnfccard = { group = "com.github.devnied.emvnfccard", name = "library", version.ref = "emvnfccard" } From fd7f3d3cef740e04486f672985a98309a861c8d1 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 26 Aug 2026 17:39:38 +0200 Subject: [PATCH 02/12] feat(otp): add sms otp suggestion --- .../presentation/AutofillDatasetProvider.kt | 10 +++- .../activity/AutofillViewModel.kt | 2 + .../dataset/inline/InlineDatasetBuilder.kt | 59 +++++++++++++++---- .../dataset/menu/MenuDatasetBuilder.kt | 43 ++++++++++++-- .../presentation/model/RequestData.kt | 12 +++- .../src/main/res/drawable/outline_sms_24.xml | 27 +++++++++ .../autofill/src/main/res/values/strings.xml | 1 + 7 files changed, 138 insertions(+), 16 deletions(-) create mode 100644 feature/autofill/src/main/res/drawable/outline_sms_24.xml diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/AutofillDatasetProvider.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/AutofillDatasetProvider.kt index c9e5e1058..9b6f3ca5f 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/AutofillDatasetProvider.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/AutofillDatasetProvider.kt @@ -31,13 +31,21 @@ internal class AutofillDatasetProviderImpl( request: FillRequest, form: Form ): List { + val targetPackage = request.fillContexts + .lastOrNull() + ?.structure + ?.activityComponent + ?.packageName + ?: "" + if (systemSupportsInlineSuggestions(request)) return inlineDatasetBuilder.buildInlineDatasets( + targetPackage = targetPackage, specs = request.inlineSuggestionsRequest!!.inlinePresentationSpecs, form = form ) - return menuDatasetBuilder.buildMenuDatasets(form = form) + return menuDatasetBuilder.buildMenuDatasets(targetPackage = targetPackage, form = form) } override fun getFillingDataset(values: List) = diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt index 9d4cc3de0..b6336294f 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt @@ -137,6 +137,8 @@ internal class AutofillViewModel( it.copy(showGeneratePassword = true) } + is FillRequestData.SmsOtp -> TODO() + is FillRequestData.Suggestion -> handleSuggestionRequest(requestData) } } diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/inline/InlineDatasetBuilder.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/inline/InlineDatasetBuilder.kt index 86d754a5c..45f703bc2 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/inline/InlineDatasetBuilder.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/inline/InlineDatasetBuilder.kt @@ -6,10 +6,12 @@ import android.graphics.drawable.Icon import android.os.Build import android.service.autofill.Dataset import android.service.autofill.InlinePresentation +import android.util.Log import android.widget.inline.InlinePresentationSpec import androidx.annotation.RequiresApi import de.davis.keygo.core.item.domain.model.lite.LiteVaultItem import de.davis.keygo.feature.autofill.R +import de.davis.keygo.feature.autofill.domain.repository.SmsCodeRepository import de.davis.keygo.feature.autofill.presentation.dataset.DatasetBuilder import de.davis.keygo.feature.autofill.presentation.dataset.SuggestionFinder import de.davis.keygo.feature.autofill.presentation.getOnLongClickPendingIntent @@ -21,6 +23,7 @@ import de.davis.keygo.feature.autofill.presentation.model.FormType import de.davis.keygo.feature.autofill.presentation.model.appRequestData import de.davis.keygo.feature.autofill.presentation.model.generatePasswordRequestData import de.davis.keygo.feature.autofill.presentation.model.pinnedRequestData +import de.davis.keygo.feature.autofill.presentation.model.smsOtpRequestData import de.davis.keygo.feature.autofill.presentation.model.suggestionRequestData import de.davis.keygo.feature.autofill.presentation.subtitle import org.koin.core.annotation.Single @@ -32,35 +35,52 @@ internal class InlineDatasetBuilder( private val inlineSuggestionFactory: InlineSuggestionFactory, private val datasetBuilder: DatasetBuilder, private val suggestionFinder: SuggestionFinder, + private val smsCodeRepository: SmsCodeRepository, private val context: Context, ) { @RequiresApi(Build.VERSION_CODES.R) suspend fun buildInlineDatasets( + targetPackage: String, specs: List, form: Form ): List = when (form.type) { - is FormType.TOTP -> buildTotpInlineDataset(specs, form) + is FormType.TOTP -> buildTotpInlineDataset(targetPackage, specs, form) else -> buildDefaultInlineDatasets(specs, form) } @RequiresApi(Build.VERSION_CODES.R) private suspend fun buildTotpInlineDataset( + targetPackage: String, specs: List, form: Form ) = when (specs.size) { 0 -> emptyList() else -> { - val suggestions = - suggestionFinder.findVaultSuggestions(form, count = 1) - - suggestions.mapIndexed { index, suggestion -> - buildInlineSuggestionDataset( - spec = specs[index], - index = index, - form = form, - suggestion = suggestion + val canOfferSmsOtp = smsCodeRepository.canOfferSuggestion(targetPackage) + Log.d(TAG, "canOfferSmsOtp: $canOfferSmsOtp") + + val suggestions = suggestionFinder.findVaultSuggestions(form, count = 1) + + buildList { + addAll( + suggestions.mapIndexed { index, suggestion -> + buildInlineSuggestionDataset( + spec = specs[index], + index = index, + form = form, + suggestion = suggestion + ) + } ) + + if (specs.size > suggestions.size && canOfferSmsOtp) + add( + buildSmsOtpInlineSuggestionDataset( + spec = specs[suggestions.size], + form = form + ) + ) } } } @@ -146,6 +166,21 @@ internal class InlineDatasetBuilder( return presentation.buildDataset(appRequestData(form)) } + @RequiresApi(Build.VERSION_CODES.R) + private fun buildSmsOtpInlineSuggestionDataset( + spec: InlinePresentationSpec, + form: Form + ): Dataset { + val presentation = inlineSuggestionFactory.buildPresentation( + spec = spec, + pendingIntent = context.getOnLongClickPendingIntent(), + icon = Icon.createWithResource(context, R.drawable.outline_sms_24), + title = context.getString(R.string.sms_code) + ) + + return presentation.buildDataset(smsOtpRequestData(form)) + } + @RequiresApi(Build.VERSION_CODES.R) private fun buildGeneratePasswordInlineSuggestionDataset( spec: InlinePresentationSpec, @@ -197,4 +232,8 @@ internal class InlineDatasetBuilder( setTintBlendMode(BlendMode.DST) } else null + + private companion object { + const val TAG = "InlineDatasetBuilder" + } } diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/menu/MenuDatasetBuilder.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/menu/MenuDatasetBuilder.kt index 40ef8b44e..e49297836 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/menu/MenuDatasetBuilder.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/menu/MenuDatasetBuilder.kt @@ -3,9 +3,11 @@ package de.davis.keygo.feature.autofill.presentation.dataset.menu import android.content.Context import android.os.Build import android.service.autofill.Dataset +import android.util.Log import androidx.annotation.DeprecatedSinceApi import de.davis.keygo.core.item.domain.model.lite.LiteVaultItem import de.davis.keygo.feature.autofill.R +import de.davis.keygo.feature.autofill.domain.repository.SmsCodeRepository import de.davis.keygo.feature.autofill.presentation.dataset.DatasetBuilder import de.davis.keygo.feature.autofill.presentation.dataset.SuggestionFinder import de.davis.keygo.feature.autofill.presentation.getSelectionPendingIntent @@ -14,6 +16,7 @@ import de.davis.keygo.feature.autofill.presentation.model.Form import de.davis.keygo.feature.autofill.presentation.model.FormType import de.davis.keygo.feature.autofill.presentation.model.appRequestData import de.davis.keygo.feature.autofill.presentation.model.generatePasswordRequestData +import de.davis.keygo.feature.autofill.presentation.model.smsOtpRequestData import de.davis.keygo.feature.autofill.presentation.model.suggestionRequestData import de.davis.keygo.feature.autofill.presentation.subtitle import org.koin.core.annotation.Single @@ -25,18 +28,32 @@ internal class MenuDatasetBuilder( private val suggestionFinder: SuggestionFinder, private val menuDatasetBuilder: MenuSuggestionFactory, private val datasetBuilder: DatasetBuilder, + private val smsCodeRepository: SmsCodeRepository, private val context: Context ) { suspend fun buildMenuDatasets( + targetPackage: String, form: Form ): List = when (form.type) { is FormType.TOTP -> { - val suggestions = - suggestionFinder.findVaultSuggestions(form, count = 1) + val suggestions = suggestionFinder.findVaultSuggestions(form, count = 1) - suggestions.mapIndexed { index, suggestion -> - buildSuggestionDataset(index, form, suggestion) + val canOfferSmsOtp = smsCodeRepository.canOfferSuggestion(targetPackage) + Log.d(TAG, "canOfferSmsOtp: $canOfferSmsOtp") + + buildList { + addAll( + suggestions.mapIndexed { index, suggestion -> + buildSuggestionDataset( + index = index, + form = form, + suggestion = suggestion + ) + } + ) + + if (canOfferSmsOtp) add(buildSmsOtpDataset(form = form)) } } @@ -79,6 +96,20 @@ internal class MenuDatasetBuilder( ) } + private fun buildSmsOtpDataset(form: Form): Dataset { + val remoteViews = menuDatasetBuilder.buildMenuSuggestion( + title = context.getString(R.string.sms_code), + subtitle = context.getString(R.string.autofill_service), + icon = R.drawable.outline_sms_24 + ) + + return datasetBuilder.buildDataset( + remoteViews = remoteViews, + intentSender = context.getSelectionPendingIntent(smsOtpRequestData(form)).intentSender, + form = form, + ) + } + private fun buildSuggestionDataset( index: Int, form: Form, @@ -101,4 +132,8 @@ internal class MenuDatasetBuilder( form = form, ) } + + private companion object { + const val TAG = "MenuDatasetBuilder" + } } \ No newline at end of file diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/model/RequestData.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/model/RequestData.kt index d0f632862..8c8588938 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/model/RequestData.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/model/RequestData.kt @@ -35,6 +35,13 @@ internal sealed interface FillRequestData : RequestData { override val requestId: Int = 1010 + index } + data class SmsOtp( + override val form: Form, + ) : FillRequestData { + @IgnoredOnParcel + override val requestId: Int = 1004 + } + data class App( override val form: Form, ) : FillRequestData { @@ -73,4 +80,7 @@ internal fun suggestionRequestData(formInformation: Form, vaultId: ItemId, index form = formInformation, vaultId = vaultId, index = index - ) \ No newline at end of file + ) + +internal fun smsOtpRequestData(formInformation: Form) = + FillRequestData.SmsOtp(form = formInformation) \ No newline at end of file diff --git a/feature/autofill/src/main/res/drawable/outline_sms_24.xml b/feature/autofill/src/main/res/drawable/outline_sms_24.xml new file mode 100644 index 000000000..0e3b6906b --- /dev/null +++ b/feature/autofill/src/main/res/drawable/outline_sms_24.xml @@ -0,0 +1,27 @@ + + + + + + diff --git a/feature/autofill/src/main/res/values/strings.xml b/feature/autofill/src/main/res/values/strings.xml index c60abee2a..02da9889b 100644 --- a/feature/autofill/src/main/res/values/strings.xml +++ b/feature/autofill/src/main/res/values/strings.xml @@ -19,4 +19,5 @@ Cancel Use KeyGo Autofill + SMS Code \ No newline at end of file From c6d0025bf45487c8c58e475f803f30dc4216a175 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 26 Aug 2026 17:40:58 +0200 Subject: [PATCH 03/12] refactor: move impl out of repository --- .../autofill/data/{repository => }/SignatureInfoProviderImpl.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/{repository => }/SignatureInfoProviderImpl.kt (96%) diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/repository/SignatureInfoProviderImpl.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/SignatureInfoProviderImpl.kt similarity index 96% rename from feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/repository/SignatureInfoProviderImpl.kt rename to feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/SignatureInfoProviderImpl.kt index 37c137482..3cd7fc3c4 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/repository/SignatureInfoProviderImpl.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/SignatureInfoProviderImpl.kt @@ -1,4 +1,4 @@ -package de.davis.keygo.feature.autofill.data.repository +package de.davis.keygo.feature.autofill.data import android.content.Context import android.content.pm.PackageManager From c027d517657472a7818f12262b1d34215b0f7178 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 26 Aug 2026 18:09:01 +0200 Subject: [PATCH 04/12] rebase --- .../presentation/activity/AutofillActivity.kt | 12 +++++- .../activity/AutofillViewModel.kt | 37 ++++++++++++++++++- .../presentation/model/AutofillUiState.kt | 1 + 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt index 8304b6992..c733c43a8 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt @@ -10,7 +10,10 @@ import android.os.PersistableBundle import android.service.autofill.Dataset import android.view.autofill.AutofillManager import androidx.activity.compose.setContent +import androidx.compose.material3.BasicAlertDialog import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi +import androidx.compose.material3.LoadingIndicator import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.ui.platform.LocalClipboard @@ -49,7 +52,7 @@ import org.koin.androidx.compose.koinViewModel */ internal class AutofillActivity : FragmentActivity() { - @OptIn(ExperimentalMaterial3Api::class) + @OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterial3ExpressiveApi::class) override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -154,6 +157,13 @@ internal class AutofillActivity : FragmentActivity() { onGenerated = { viewModel.onEvent(AutofillUiEvent.OnGeneratedPassword(it)) }, onDismiss = { viewModel.onEvent(AutofillUiEvent.OnDismissGeneratePassword) } ) + + if (uiState.showSmsPending) + BasicAlertDialog( + onDismissRequest = {} + ) { + LoadingIndicator() + } } } } diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt index b6336294f..50d915bdd 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt @@ -18,6 +18,8 @@ import de.davis.keygo.core.security.domain.crypto.model.WrappedVaultKeyInformati import de.davis.keygo.core.security.domain.crypto.wrappedItemKeyInformation import de.davis.keygo.core.security.domain.model.BiometricAuthError import de.davis.keygo.core.util.getOrNull +import de.davis.keygo.feature.autofill.domain.model.SmsCodeEvent +import de.davis.keygo.feature.autofill.domain.repository.SmsCodeRepository import de.davis.keygo.feature.autofill.domain.usecase.AddRegistrableDomainsToLoginUseCase import de.davis.keygo.feature.autofill.domain.usecase.DoesItemHaveDomainReferencesUseCase import de.davis.keygo.feature.autofill.domain.usecase.IsAppLinkedToWebsiteUseCase @@ -38,6 +40,7 @@ import de.davis.keygo.feature.autofill.presentation.model.RequestData import de.davis.keygo.feature.autofill.presentation.model.SaveRequestData import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation import de.davis.keygo.feature.totp.domain.repository.TotpGenerator +import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow @@ -53,6 +56,7 @@ internal class AutofillViewModel( private val loginRepository: LoginRepository, private val totpRepository: TotpRepository, private val itemRepository: ItemRepository, + private val smsCodeRepository: SmsCodeRepository, private val cryptographicScopeProvider: CryptographicScopeProvider, private val autofillDatasetProvider: AutofillDatasetProvider, private val doesItemHaveDomainReferences: DoesItemHaveDomainReferencesUseCase, @@ -73,6 +77,7 @@ internal class AutofillViewModel( private val _uiState = MutableStateFlow(AutofillUiState()) val uiState = _uiState.asStateFlow() + private var smsOtpJob: Job? = null fun start() { handleRequestData() @@ -137,7 +142,7 @@ internal class AutofillViewModel( it.copy(showGeneratePassword = true) } - is FillRequestData.SmsOtp -> TODO() + is FillRequestData.SmsOtp -> handleSmsOtpRequest(requestData) is FillRequestData.Suggestion -> handleSuggestionRequest(requestData) } @@ -152,6 +157,36 @@ internal class AutofillViewModel( biometricChannel.send(AutofillBiometricRequest.UnlockItem(itemName)) } + private fun handleSmsOtpRequest(smsOtpInfo: FillRequestData.SmsOtp) { + _uiState.update { it.copy(showSmsPending = true) } + + smsOtpJob?.cancel() + smsOtpJob = viewModelScope.launch { + smsCodeRepository.smsCodes().collect { event -> + when (event) { + is SmsCodeEvent.SmsCodeReceived -> { + _uiState.update { it.copy(showSmsPending = false) } + eventChannel.send( + AutofillEvent.Fill( + autofillDatasetProvider.getFillingDataset( + listOf( + AutofillValue( + autofillId = smsOtpInfo.form.fields.first { it.type == FieldType.TOTP }.autofillId, + value = event.code + ) + ) + ) + ) + ) + } + + SmsCodeEvent.Timeout, + is SmsCodeEvent.Failed -> eventChannel.send(AutofillEvent.Abort) + } + } + } + } + fun onBiometricLoginFailed(error: UnlockError) { viewModelScope.launch { when(error) { diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/model/AutofillUiState.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/model/AutofillUiState.kt index bd1d7cc41..659b3d807 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/model/AutofillUiState.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/model/AutofillUiState.kt @@ -9,5 +9,6 @@ internal data class AutofillUiState( val associationDialogVisibility: AssociationDialogVisibility = AssociationDialogVisibility.Hidden, val suspicionDialogVisibility: SuspicionDialogVisibility = SuspicionDialogVisibility.Hidden, val showGeneratePassword: Boolean = false, + val showSmsPending: Boolean = false, val itemId: ItemId? = null ) \ No newline at end of file From 4bda53b03e93e441b1af16c8e3a096a963953feb Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 26 Aug 2026 18:09:47 +0200 Subject: [PATCH 05/12] fix: abort if requiredIds is empty --- .../feature/autofill/presentation/dataset/SaveInfoAppender.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppender.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppender.kt index cd6126bb1..a83e98938 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppender.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppender.kt @@ -27,6 +27,7 @@ internal fun FillResponse.Builder.applySaveInfo( val requiredIds = updatedForm.fields.map { it.autofillId }.toTypedArray() + if (requiredIds.isEmpty()) return Log.d( TAG, "Applied Save Info:\n" + From b0c0003f78c05fd92a7d30244fecdc6a61736e9c Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 26 Aug 2026 20:41:39 +0200 Subject: [PATCH 06/12] refactor(autofill): make sms code retrieval a one shot request --- .../repository/NoopSmsCodeRepository.kt | 15 ---- .../presentation/sms/NoopSmsCodeRepository.kt | 13 ++++ .../autofill/domain/model/SmsCodeEvent.kt | 8 --- .../domain/repository/SmsCodeRepository.kt | 10 --- .../activity/AutofillViewModel.kt | 61 +++++++++------- .../dataset/inline/InlineDatasetBuilder.kt | 2 +- .../dataset/menu/MenuDatasetBuilder.kt | 2 +- .../presentation/sms/SmsCodeFailure.kt | 20 ++++++ .../presentation/sms/SmsCodeRepository.kt | 20 ++++++ .../sms}/GmsSmsCodeRepository.kt | 70 ++++++++++++------- .../activity/AutofillViewModelTest.kt | 65 +++++++++++++++++ .../feature/autofill/FakeSmsCodeRepository.kt | 35 ++++++++++ 12 files changed, 238 insertions(+), 83 deletions(-) delete mode 100644 feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/domain/repository/NoopSmsCodeRepository.kt create mode 100644 feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/presentation/sms/NoopSmsCodeRepository.kt delete mode 100644 feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/model/SmsCodeEvent.kt delete mode 100644 feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/repository/SmsCodeRepository.kt create mode 100644 feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/sms/SmsCodeFailure.kt create mode 100644 feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/sms/SmsCodeRepository.kt rename feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/{domain/repository => presentation/sms}/GmsSmsCodeRepository.kt (50%) create mode 100644 feature/autofill/src/testFixtures/kotlin/de/davis/keygo/core/feature/autofill/FakeSmsCodeRepository.kt diff --git a/feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/domain/repository/NoopSmsCodeRepository.kt b/feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/domain/repository/NoopSmsCodeRepository.kt deleted file mode 100644 index 4ffc4ded9..000000000 --- a/feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/domain/repository/NoopSmsCodeRepository.kt +++ /dev/null @@ -1,15 +0,0 @@ -package de.davis.keygo.feature.autofill.domain.repository - -import de.davis.keygo.feature.autofill.domain.model.SmsCodeEvent -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.flowOf -import org.koin.core.annotation.Single - -@Single -internal class NoopSmsCodeRepository : SmsCodeRepository { - - override suspend fun canOfferSuggestion(targetPackage: String): Boolean = false - - override fun smsCodes(): Flow = - flowOf(SmsCodeEvent.Failed(IllegalStateException("SMS code autofill unavailable"))) -} \ No newline at end of file diff --git a/feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/presentation/sms/NoopSmsCodeRepository.kt b/feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/presentation/sms/NoopSmsCodeRepository.kt new file mode 100644 index 000000000..d4bacf919 --- /dev/null +++ b/feature/autofill/src/fdroid/kotlin/de/davis/keygo/feature/autofill/presentation/sms/NoopSmsCodeRepository.kt @@ -0,0 +1,13 @@ +package de.davis.keygo.feature.autofill.presentation.sms + +import de.davis.keygo.core.util.Result +import org.koin.core.annotation.Single + +@Single(binds = [SmsCodeRepository::class]) +internal class NoopSmsCodeRepository : SmsCodeRepository { + + override suspend fun canOfferSuggestion(targetPackage: String): Boolean = false + + override suspend fun retrieveSmsCode(): Result = + Result.Failure(SmsCodeFailure.Unavailable) +} diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/model/SmsCodeEvent.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/model/SmsCodeEvent.kt deleted file mode 100644 index d6071b1b2..000000000 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/model/SmsCodeEvent.kt +++ /dev/null @@ -1,8 +0,0 @@ -package de.davis.keygo.feature.autofill.domain.model - -sealed interface SmsCodeEvent { - data class SmsCodeReceived(val code: String) : SmsCodeEvent - data class Failed(val cause: Throwable) : SmsCodeEvent - - data object Timeout : SmsCodeEvent -} diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/repository/SmsCodeRepository.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/repository/SmsCodeRepository.kt deleted file mode 100644 index 267683b10..000000000 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/domain/repository/SmsCodeRepository.kt +++ /dev/null @@ -1,10 +0,0 @@ -package de.davis.keygo.feature.autofill.domain.repository - -import de.davis.keygo.feature.autofill.domain.model.SmsCodeEvent -import kotlinx.coroutines.flow.Flow - -interface SmsCodeRepository { - - suspend fun canOfferSuggestion(targetPackage: String): Boolean - fun smsCodes(): Flow -} \ No newline at end of file diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt index 50d915bdd..9abfb5fce 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt @@ -18,8 +18,8 @@ import de.davis.keygo.core.security.domain.crypto.model.WrappedVaultKeyInformati import de.davis.keygo.core.security.domain.crypto.wrappedItemKeyInformation import de.davis.keygo.core.security.domain.model.BiometricAuthError import de.davis.keygo.core.util.getOrNull -import de.davis.keygo.feature.autofill.domain.model.SmsCodeEvent -import de.davis.keygo.feature.autofill.domain.repository.SmsCodeRepository +import de.davis.keygo.core.util.onFailure +import de.davis.keygo.core.util.onSuccess import de.davis.keygo.feature.autofill.domain.usecase.AddRegistrableDomainsToLoginUseCase import de.davis.keygo.feature.autofill.domain.usecase.DoesItemHaveDomainReferencesUseCase import de.davis.keygo.feature.autofill.domain.usecase.IsAppLinkedToWebsiteUseCase @@ -38,6 +38,7 @@ import de.davis.keygo.feature.autofill.presentation.model.FormType import de.davis.keygo.feature.autofill.presentation.model.Request import de.davis.keygo.feature.autofill.presentation.model.RequestData import de.davis.keygo.feature.autofill.presentation.model.SaveRequestData +import de.davis.keygo.feature.autofill.presentation.sms.SmsCodeRepository import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation import de.davis.keygo.feature.totp.domain.repository.TotpGenerator import kotlinx.coroutines.Job @@ -157,34 +158,46 @@ internal class AutofillViewModel( biometricChannel.send(AutofillBiometricRequest.UnlockItem(itemName)) } - private fun handleSmsOtpRequest(smsOtpInfo: FillRequestData.SmsOtp) { + private suspend fun handleSmsOtpRequest(smsOtpInfo: FillRequestData.SmsOtp) { + // The extractor already narrowed the form to the focused field's group, so a TOTP form holds + // TOTP fields and nothing else. An empty one means there is nothing to fill. + if (smsOtpInfo.form.fields.isEmpty()) { + eventChannel.send(AutofillEvent.Abort) + return + } + _uiState.update { it.copy(showSmsPending = true) } + startSmsRetrieval() + } + private fun startSmsRetrieval() { smsOtpJob?.cancel() smsOtpJob = viewModelScope.launch { - smsCodeRepository.smsCodes().collect { event -> - when (event) { - is SmsCodeEvent.SmsCodeReceived -> { - _uiState.update { it.copy(showSmsPending = false) } - eventChannel.send( - AutofillEvent.Fill( - autofillDatasetProvider.getFillingDataset( - listOf( - AutofillValue( - autofillId = smsOtpInfo.form.fields.first { it.type == FieldType.TOTP }.autofillId, - value = event.code - ) - ) - ) - ) - ) - } + smsCodeRepository.retrieveSmsCode() + .onSuccess { code -> sendSmsFillEvent(code) } + .onFailure { eventChannel.send(AutofillEvent.Abort) } + } + } - SmsCodeEvent.Timeout, - is SmsCodeEvent.Failed -> eventChannel.send(AutofillEvent.Abort) - } - } + private suspend fun sendSmsFillEvent(code: String) { + val targetField = (requestData as? FillRequestData.SmsOtp)?.form?.fields?.firstOrNull() ?: run { + eventChannel.send(AutofillEvent.Abort) + return } + + _uiState.update { it.copy(showSmsPending = false) } + eventChannel.send( + AutofillEvent.Fill( + autofillDatasetProvider.getFillingDataset( + listOf( + AutofillValue( + autofillId = targetField.autofillId, + value = code, + ), + ), + ), + ), + ) } fun onBiometricLoginFailed(error: UnlockError) { diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/inline/InlineDatasetBuilder.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/inline/InlineDatasetBuilder.kt index 45f703bc2..56845b0d2 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/inline/InlineDatasetBuilder.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/inline/InlineDatasetBuilder.kt @@ -11,7 +11,7 @@ import android.widget.inline.InlinePresentationSpec import androidx.annotation.RequiresApi import de.davis.keygo.core.item.domain.model.lite.LiteVaultItem import de.davis.keygo.feature.autofill.R -import de.davis.keygo.feature.autofill.domain.repository.SmsCodeRepository +import de.davis.keygo.feature.autofill.presentation.sms.SmsCodeRepository import de.davis.keygo.feature.autofill.presentation.dataset.DatasetBuilder import de.davis.keygo.feature.autofill.presentation.dataset.SuggestionFinder import de.davis.keygo.feature.autofill.presentation.getOnLongClickPendingIntent diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/menu/MenuDatasetBuilder.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/menu/MenuDatasetBuilder.kt index e49297836..c65396d16 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/menu/MenuDatasetBuilder.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/menu/MenuDatasetBuilder.kt @@ -7,7 +7,7 @@ import android.util.Log import androidx.annotation.DeprecatedSinceApi import de.davis.keygo.core.item.domain.model.lite.LiteVaultItem import de.davis.keygo.feature.autofill.R -import de.davis.keygo.feature.autofill.domain.repository.SmsCodeRepository +import de.davis.keygo.feature.autofill.presentation.sms.SmsCodeRepository import de.davis.keygo.feature.autofill.presentation.dataset.DatasetBuilder import de.davis.keygo.feature.autofill.presentation.dataset.SuggestionFinder import de.davis.keygo.feature.autofill.presentation.getSelectionPendingIntent diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/sms/SmsCodeFailure.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/sms/SmsCodeFailure.kt new file mode 100644 index 000000000..9bed7235d --- /dev/null +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/sms/SmsCodeFailure.kt @@ -0,0 +1,20 @@ +package de.davis.keygo.feature.autofill.presentation.sms + +import android.content.IntentSender + +internal sealed interface SmsCodeFailure { + + /** + * The user has not allowed KeyGo to read SMS verification codes yet. Launching [intentSender] + * lets Google Play services ask them, after which the retrieval can be tried again. + */ + data class ConsentRequired(val intentSender: IntentSender) : SmsCodeFailure + + /** Google Play services waited its full window (about 5 minutes) without seeing a code. */ + data object Timeout : SmsCodeFailure + + /** SMS code retrieval is not available at all, for example on the fdroid flavor. */ + data object Unavailable : SmsCodeFailure + + data class Unknown(val cause: Throwable) : SmsCodeFailure +} diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/sms/SmsCodeRepository.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/sms/SmsCodeRepository.kt new file mode 100644 index 000000000..707cd5793 --- /dev/null +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/sms/SmsCodeRepository.kt @@ -0,0 +1,20 @@ +package de.davis.keygo.feature.autofill.presentation.sms + +import de.davis.keygo.core.util.Result + +/** + * Reads one time codes out of incoming SMS messages. + * + * This lives in presentation rather than domain on purpose. It carries no policy, only platform + * mechanism, and the consent failure has to hand back a framework [android.content.IntentSender]. + */ +internal interface SmsCodeRepository { + + suspend fun canOfferSuggestion(targetPackage: String): Boolean + + /** + * Waits for a single SMS verification code. Cancelling the calling coroutine stops the wait and + * releases the underlying receiver. + */ + suspend fun retrieveSmsCode(): Result +} diff --git a/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/domain/repository/GmsSmsCodeRepository.kt b/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt similarity index 50% rename from feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/domain/repository/GmsSmsCodeRepository.kt rename to feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt index 1c7bdc8e7..3683fff4b 100644 --- a/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/domain/repository/GmsSmsCodeRepository.kt +++ b/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt @@ -1,4 +1,4 @@ -package de.davis.keygo.feature.autofill.domain.repository +package de.davis.keygo.feature.autofill.presentation.sms import android.content.BroadcastReceiver import android.content.Context @@ -10,18 +10,22 @@ import androidx.core.content.IntentCompat import com.google.android.gms.auth.api.phone.SmsCodeAutofillClient import com.google.android.gms.auth.api.phone.SmsCodeRetriever import com.google.android.gms.auth.api.phone.SmsRetriever +import com.google.android.gms.auth.api.phone.SmsRetrieverStatusCodes import com.google.android.gms.common.api.ApiException import com.google.android.gms.common.api.CommonStatusCodes +import com.google.android.gms.common.api.ResolvableApiException import com.google.android.gms.common.api.Status -import de.davis.keygo.feature.autofill.domain.model.SmsCodeEvent +import de.davis.keygo.core.util.Result import kotlinx.coroutines.channels.awaitClose +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.callbackFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.tasks.await import org.koin.core.annotation.Single -@Single +@Single(binds = [SmsCodeRepository::class]) internal class GmsSmsCodeRepository( - private val context: Context + private val context: Context, ) : SmsCodeRepository { private val client by lazy { @@ -41,29 +45,20 @@ internal class GmsSmsCodeRepository( false } - override fun smsCodes() = callbackFlow { + // The flow below only ever produces one value. It stays a flow because awaitClose is what + // unregisters the receiver, and first() cancels the flow on every path: a delivered code, a + // failure, or the caller being cancelled. + override suspend fun retrieveSmsCode(): Result = smsCodes().first() + + private fun smsCodes(): Flow> = callbackFlow { val receiver = object : BroadcastReceiver() { override fun onReceive(c: Context, intent: Intent) { if (intent.action != SmsCodeRetriever.SMS_CODE_RETRIEVED_ACTION) return val status = IntentCompat.getParcelableExtra( - intent, SmsRetriever.EXTRA_STATUS, Status::class.java + intent, SmsRetriever.EXTRA_STATUS, Status::class.java, ) - when (status?.statusCode) { - CommonStatusCodes.SUCCESS -> { - val code = intent.getStringExtra(SmsCodeRetriever.EXTRA_SMS_CODE) - trySend( - if (code.isNullOrBlank()) - SmsCodeEvent.Failed(IllegalStateException("empty code")) - else SmsCodeEvent.SmsCodeReceived(code) - ) - } - - CommonStatusCodes.TIMEOUT -> trySend(SmsCodeEvent.Timeout) - else -> trySend( - SmsCodeEvent.Failed(IllegalStateException("status=${status?.statusCode}")) - ) - } + trySend(intent.toResult(status)) } } @@ -73,21 +68,48 @@ internal class GmsSmsCodeRepository( IntentFilter(SmsCodeRetriever.SMS_CODE_RETRIEVED_ACTION), SmsRetriever.SEND_PERMISSION, null, - ContextCompat.RECEIVER_EXPORTED + ContextCompat.RECEIVER_EXPORTED, ) // Start only after the receiver is live, otherwise a code that is // already sitting in the inbox can be delivered before you listen. try { client.startSmsCodeRetriever().await() + } catch (e: ResolvableApiException) { + trySend(Result.Failure(SmsCodeFailure.ConsentRequired(e.resolution.intentSender))) } catch (e: Exception) { - trySend(SmsCodeEvent.Failed(e)) + trySend(Result.Failure(SmsCodeFailure.Unknown(e))) } awaitClose { runCatching { context.unregisterReceiver(receiver) } } } + private fun Intent.toResult(status: Status?): Result = + when (status?.statusCode) { + CommonStatusCodes.SUCCESS -> { + val code = getStringExtra(SmsCodeRetriever.EXTRA_SMS_CODE) + if (code.isNullOrBlank()) + Result.Failure(SmsCodeFailure.Unknown(IllegalStateException("empty code"))) + else Result.Success(code) + } + + CommonStatusCodes.TIMEOUT -> Result.Failure(SmsCodeFailure.Timeout) + + // The consent resolution can arrive here as well as on the start call, so both paths + // have to produce ConsentRequired. Play services does not document whether this Status + // always carries a resolution, so a missing one degrades to Unavailable. + SmsRetrieverStatusCodes.USER_PERMISSION_REQUIRED -> { + val intentSender = status.resolution?.intentSender + if (intentSender == null) Result.Failure(SmsCodeFailure.Unavailable) + else Result.Failure(SmsCodeFailure.ConsentRequired(intentSender)) + } + + else -> Result.Failure( + SmsCodeFailure.Unknown(IllegalStateException("status=${status?.statusCode}")), + ) + } + private companion object { const val TAG = "GmsSmsCodeRepository" } -} \ No newline at end of file +} diff --git a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt index a99c042be..fbeaa66f6 100644 --- a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt +++ b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.SavedStateHandle import de.davis.keygo.core.feature.autofill.FakeAutofillDatasetProvider import de.davis.keygo.core.feature.autofill.FakeDigitalAssetLinkRepository import de.davis.keygo.core.feature.autofill.FakeSignatureInfoProvider +import de.davis.keygo.core.feature.autofill.FakeSmsCodeRepository import de.davis.keygo.core.feature.autofill.FakeTotpGenerator import de.davis.keygo.core.feature.autofill.FakeTotpRepository import de.davis.keygo.core.feature.autofill.autofillId @@ -37,8 +38,10 @@ import de.davis.keygo.feature.autofill.presentation.model.FormType import de.davis.keygo.feature.autofill.presentation.model.Request import de.davis.keygo.feature.autofill.presentation.model.RequestData import de.davis.keygo.feature.autofill.presentation.model.SaveRequestData +import de.davis.keygo.feature.autofill.presentation.sms.SmsCodeFailure import de.davis.keygo.feature.totp.domain.model.TotpError import de.davis.keygo.feature.totp.domain.model.TotpValue +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.async @@ -54,6 +57,7 @@ import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertIs import kotlin.test.assertTrue @@ -71,6 +75,7 @@ internal class AutofillViewModelTest { private lateinit var signatureProvider: FakeSignatureInfoProvider private lateinit var dalRepo: FakeDigitalAssetLinkRepository private lateinit var totpGenerator: FakeTotpGenerator + private lateinit var smsCodeRepo: FakeSmsCodeRepository @OptIn(ExperimentalCoroutinesApi::class) @Before @@ -96,6 +101,7 @@ internal class AutofillViewModelTest { signatureProvider = FakeSignatureInfoProvider() dalRepo = FakeDigitalAssetLinkRepository() totpGenerator = FakeTotpGenerator() + smsCodeRepo = FakeSmsCodeRepository() } private fun buildVm(requestData: RequestData): AutofillViewModel { @@ -107,6 +113,7 @@ internal class AutofillViewModelTest { loginRepository = loginRepo, totpRepository = totpRepo, itemRepository = fakeItemRepo, + smsCodeRepository = smsCodeRepo, cryptographicScopeProvider = cryptoProvider, autofillDatasetProvider = datasetProvider, doesItemHaveDomainReferences = DoesItemHaveDomainReferencesUseCase(loginRepo, resolver), @@ -133,6 +140,10 @@ internal class AutofillViewModelTest { autofillValue = autofillValue, ) + private fun smsOtpRequest(fields: List) = FillRequestData.SmsOtp( + form(fields = fields, type = FormType.TOTP), + ) + private fun testLogin( username: String? = "alice", name: String = username ?: "Login", @@ -472,4 +483,58 @@ internal class AutofillViewModelTest { assertIs(event) assertEquals("123456", datasetProvider.getFillingDatasetCalls.last().first().value) } + + @Test + fun `sms otp request shows the pending dialog`() = runTest { + smsCodeRepo.gate = CompletableDeferred() + + val vm = buildVm(smsOtpRequest(listOf(credField(FieldType.TOTP, viewId = 1)))) + vm.start() + + assertTrue(vm.uiState.value.showSmsPending) + } + + @Test + fun `sms code received sends Fill event with the code`() = runTest { + smsCodeRepo.enqueue(Result.Success("123456")) + + val vm = buildVm(smsOtpRequest(listOf(credField(FieldType.TOTP, viewId = 1)))) + val eventDeferred = async { vm.events.first() } + vm.start() + val event = eventDeferred.await() + + assertIs(event) + assertEquals("123456", datasetProvider.getFillingDatasetCalls.last().first().value) + assertFalse(vm.uiState.value.showSmsPending) + } + + @Test + fun `sms retrieval failures abort`() = runTest { + val failures = listOf( + SmsCodeFailure.Timeout, + SmsCodeFailure.Unavailable, + SmsCodeFailure.Unknown(IllegalStateException("boom")), + ) + + failures.forEach { failure -> + smsCodeRepo = FakeSmsCodeRepository() + smsCodeRepo.enqueue(Result.Failure(failure)) + + val vm = buildVm(smsOtpRequest(listOf(credField(FieldType.TOTP, viewId = 1)))) + val eventDeferred = async { vm.events.first() } + vm.start() + + assertEquals(AutofillEvent.Abort, eventDeferred.await(), "failed for $failure") + } + } + + @Test + fun `sms otp request with no fields aborts`() = runTest { + val vm = buildVm(smsOtpRequest(emptyList())) + val eventDeferred = async { vm.events.first() } + vm.start() + + assertEquals(AutofillEvent.Abort, eventDeferred.await()) + assertFalse(vm.uiState.value.showSmsPending) + } } diff --git a/feature/autofill/src/testFixtures/kotlin/de/davis/keygo/core/feature/autofill/FakeSmsCodeRepository.kt b/feature/autofill/src/testFixtures/kotlin/de/davis/keygo/core/feature/autofill/FakeSmsCodeRepository.kt new file mode 100644 index 000000000..7f7edeae0 --- /dev/null +++ b/feature/autofill/src/testFixtures/kotlin/de/davis/keygo/core/feature/autofill/FakeSmsCodeRepository.kt @@ -0,0 +1,35 @@ +package de.davis.keygo.core.feature.autofill + +import de.davis.keygo.core.util.Result +import de.davis.keygo.feature.autofill.presentation.sms.SmsCodeFailure +import de.davis.keygo.feature.autofill.presentation.sms.SmsCodeRepository +import kotlinx.coroutines.CompletableDeferred + +/** + * Hands out scripted [retrieveSmsCode] results, one per call, so a consent round trip can be played + * back. Setting [gate] makes each call suspend on it first, which lets a test hold a retrieval open + * and check that cancelling it really takes effect. + */ +internal class FakeSmsCodeRepository : SmsCodeRepository { + + var canOffer: Boolean = true + var gate: CompletableDeferred? = null + + var callCount: Int = 0 + private set + + private val results = ArrayDeque>() + + fun enqueue(vararg values: Result) { + results += values + } + + override suspend fun canOfferSuggestion(targetPackage: String): Boolean = canOffer + + override suspend fun retrieveSmsCode(): Result { + callCount++ + gate?.await() + return results.removeFirstOrNull() + ?: Result.Failure(SmsCodeFailure.Unknown(IllegalStateException("no result enqueued"))) + } +} From a6ebbb51c540cdcea9c56900839cd7973a1d14ad Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 26 Aug 2026 21:08:53 +0200 Subject: [PATCH 07/12] feat(autofill): resolve sms code consent instead of aborting A first run has permission state NONE, so the retrieval comes back needing consent. Forward it to the activity as RequestSmsConsent, launch the resolution with StartIntentSenderForResult, and retry once when the user grants it. Co-Authored-By: Claude Opus 5 --- .../presentation/activity/AutofillActivity.kt | 16 ++++ .../activity/AutofillViewModel.kt | 24 +++++- .../activity/model/AutofillEvent.kt | 3 + .../activity/model/AutofillUiEvent.kt | 2 + .../activity/AutofillViewModelTest.kt | 76 +++++++++++++++++++ 5 files changed, 119 insertions(+), 2 deletions(-) diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt index c733c43a8..c34e95722 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt @@ -9,7 +9,10 @@ import android.os.Bundle import android.os.PersistableBundle import android.service.autofill.Dataset import android.view.autofill.AutofillManager +import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.compose.setContent +import androidx.activity.result.IntentSenderRequest +import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.material3.BasicAlertDialog import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi @@ -71,6 +74,14 @@ internal class AutofillActivity : FragmentActivity() { val clipboard = LocalClipboard.current + val smsConsentLauncher = rememberLauncherForActivityResult( + ActivityResultContracts.StartIntentSenderForResult(), + ) { result -> + viewModel.onEvent( + AutofillUiEvent.OnSmsConsentResult(result.resultCode == RESULT_OK), + ) + } + ObserveAsEvents(viewModel.events) { event -> when (event) { AutofillEvent.Abort -> cancel() @@ -88,6 +99,11 @@ internal class AutofillActivity : FragmentActivity() { finishWithResult(event.dataset) } + + is AutofillEvent.RequestSmsConsent -> + smsConsentLauncher.launch( + IntentSenderRequest.Builder(event.intentSender).build(), + ) } } diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt index 9abfb5fce..ff16d6fc8 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt @@ -38,6 +38,7 @@ import de.davis.keygo.feature.autofill.presentation.model.FormType import de.davis.keygo.feature.autofill.presentation.model.Request import de.davis.keygo.feature.autofill.presentation.model.RequestData import de.davis.keygo.feature.autofill.presentation.model.SaveRequestData +import de.davis.keygo.feature.autofill.presentation.sms.SmsCodeFailure import de.davis.keygo.feature.autofill.presentation.sms.SmsCodeRepository import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation import de.davis.keygo.feature.totp.domain.repository.TotpGenerator @@ -170,15 +171,32 @@ internal class AutofillViewModel( startSmsRetrieval() } - private fun startSmsRetrieval() { + private fun startSmsRetrieval(consentAlreadyRequested: Boolean = false) { smsOtpJob?.cancel() smsOtpJob = viewModelScope.launch { smsCodeRepository.retrieveSmsCode() .onSuccess { code -> sendSmsFillEvent(code) } - .onFailure { eventChannel.send(AutofillEvent.Abort) } + .onFailure { failure -> + when (failure) { + // Asking a second time would mean the consent screen came back OK without + // actually granting anything, so stop rather than spin. + is SmsCodeFailure.ConsentRequired -> + if (consentAlreadyRequested) eventChannel.send(AutofillEvent.Abort) + else eventChannel.send( + AutofillEvent.RequestSmsConsent(failure.intentSender), + ) + + else -> eventChannel.send(AutofillEvent.Abort) + } + } } } + private fun onSmsConsentResult(granted: Boolean) { + if (granted) startSmsRetrieval(consentAlreadyRequested = true) + else viewModelScope.launch { eventChannel.send(AutofillEvent.Abort) } + } + private suspend fun sendSmsFillEvent(code: String) { val targetField = (requestData as? FillRequestData.SmsOtp)?.form?.fields?.firstOrNull() ?: run { eventChannel.send(AutofillEvent.Abort) @@ -285,6 +303,8 @@ internal class AutofillViewModel( is AutofillUiEvent.OnGeneratedPassword -> viewModelScope.launch { sendGeneratedPasswordFillEvent(event.password) } + + is AutofillUiEvent.OnSmsConsentResult -> onSmsConsentResult(event.granted) } } diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillEvent.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillEvent.kt index e74380482..d27050833 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillEvent.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillEvent.kt @@ -1,5 +1,6 @@ package de.davis.keygo.feature.autofill.presentation.activity.model +import android.content.IntentSender import android.service.autofill.Dataset internal sealed interface AutofillEvent { @@ -7,4 +8,6 @@ internal sealed interface AutofillEvent { data object Abort : AutofillEvent data class Fill(val dataset: Dataset, val copyToClipboard: String? = null) : AutofillEvent + + data class RequestSmsConsent(val intentSender: IntentSender) : AutofillEvent } \ No newline at end of file diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillUiEvent.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillUiEvent.kt index 25dd6173d..1a843d4f8 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillUiEvent.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillUiEvent.kt @@ -13,4 +13,6 @@ internal sealed interface AutofillUiEvent { data object OnDismissGeneratePassword : AutofillUiEvent data class OnGeneratedPassword(val password: String) : AutofillUiEvent + + data class OnSmsConsentResult(val granted: Boolean) : AutofillUiEvent } \ No newline at end of file diff --git a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt index fbeaa66f6..c3bbdb5cf 100644 --- a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt +++ b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt @@ -1,5 +1,8 @@ package de.davis.keygo.feature.autofill.presentation.activity +import android.app.PendingIntent +import android.content.Intent +import android.content.IntentSender import androidx.lifecycle.SavedStateHandle import de.davis.keygo.core.feature.autofill.FakeAutofillDatasetProvider import de.davis.keygo.core.feature.autofill.FakeDigitalAssetLinkRepository @@ -54,6 +57,7 @@ import org.junit.After import org.junit.Before import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment import org.robolectric.annotation.Config import kotlin.test.Test import kotlin.test.assertEquals @@ -144,6 +148,13 @@ internal class AutofillViewModelTest { form(fields = fields, type = FormType.TOTP), ) + private fun testIntentSender(): IntentSender = PendingIntent.getActivity( + RuntimeEnvironment.getApplication(), + 0, + Intent(), + PendingIntent.FLAG_IMMUTABLE, + ).intentSender + private fun testLogin( username: String? = "alice", name: String = username ?: "Login", @@ -537,4 +548,69 @@ internal class AutofillViewModelTest { assertEquals(AutofillEvent.Abort, eventDeferred.await()) assertFalse(vm.uiState.value.showSmsPending) } + + @Test + fun `consent required emits RequestSmsConsent and keeps the dialog up`() = runTest { + smsCodeRepo.enqueue(Result.Failure(SmsCodeFailure.ConsentRequired(testIntentSender()))) + + val vm = buildVm(smsOtpRequest(listOf(credField(FieldType.TOTP, viewId = 1)))) + val eventDeferred = async { vm.events.first() } + vm.start() + + assertIs(eventDeferred.await()) + assertTrue(vm.uiState.value.showSmsPending) + } + + @Test + fun `granted consent retries the retrieval and fills`() = runTest { + smsCodeRepo.enqueue( + Result.Failure(SmsCodeFailure.ConsentRequired(testIntentSender())), + Result.Success("654321"), + ) + + val vm = buildVm(smsOtpRequest(listOf(credField(FieldType.TOTP, viewId = 1)))) + val consentDeferred = async { vm.events.first() } + vm.start() + assertIs(consentDeferred.await()) + + val fillDeferred = async { vm.events.first() } + vm.onEvent(AutofillUiEvent.OnSmsConsentResult(granted = true)) + + assertIs(fillDeferred.await()) + assertEquals("654321", datasetProvider.getFillingDatasetCalls.last().first().value) + } + + @Test + fun `denied consent aborts`() = runTest { + smsCodeRepo.enqueue(Result.Failure(SmsCodeFailure.ConsentRequired(testIntentSender()))) + + val vm = buildVm(smsOtpRequest(listOf(credField(FieldType.TOTP, viewId = 1)))) + val consentDeferred = async { vm.events.first() } + vm.start() + consentDeferred.await() + + val abortDeferred = async { vm.events.first() } + vm.onEvent(AutofillUiEvent.OnSmsConsentResult(granted = false)) + + assertEquals(AutofillEvent.Abort, abortDeferred.await()) + } + + @Test + fun `consent required twice aborts instead of looping`() = runTest { + smsCodeRepo.enqueue( + Result.Failure(SmsCodeFailure.ConsentRequired(testIntentSender())), + Result.Failure(SmsCodeFailure.ConsentRequired(testIntentSender())), + ) + + val vm = buildVm(smsOtpRequest(listOf(credField(FieldType.TOTP, viewId = 1)))) + val consentDeferred = async { vm.events.first() } + vm.start() + consentDeferred.await() + + val abortDeferred = async { vm.events.first() } + vm.onEvent(AutofillUiEvent.OnSmsConsentResult(granted = true)) + + assertEquals(AutofillEvent.Abort, abortDeferred.await()) + assertEquals(2, smsCodeRepo.callCount) + } } From c0a9e92d25470632bd89283fab952b026819b0ac Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 26 Aug 2026 21:20:10 +0200 Subject: [PATCH 08/12] feat(autofill): let the user cancel while waiting for an sms code The play services retriever waits about five minutes, and the old dialog had no button, no text and no dismiss, so a user whose message never arrived was stuck for the whole window. Replace it with a real AlertDialog that explains itself and aborts the autofill on cancel. Co-Authored-By: Claude Opus 5 --- .../presentation/activity/AutofillActivity.kt | 14 ++-- .../activity/AutofillViewModel.kt | 8 +++ .../component/SmsCodePendingDialog.kt | 65 +++++++++++++++++++ .../activity/model/AutofillUiEvent.kt | 1 + .../autofill/src/main/res/values/strings.xml | 2 + .../activity/AutofillViewModelTest.kt | 33 ++++++++++ 6 files changed, 113 insertions(+), 10 deletions(-) create mode 100644 feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/component/SmsCodePendingDialog.kt diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt index c34e95722..e9791a078 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt @@ -13,10 +13,6 @@ import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.compose.setContent import androidx.activity.result.IntentSenderRequest import androidx.activity.result.contract.ActivityResultContracts -import androidx.compose.material3.BasicAlertDialog -import androidx.compose.material3.ExperimentalMaterial3Api -import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi -import androidx.compose.material3.LoadingIndicator import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.ui.platform.LocalClipboard @@ -34,6 +30,7 @@ import de.davis.keygo.core.util.onSuccess import de.davis.keygo.core.util.presentation.ObserveAsEvents import de.davis.keygo.feature.auth.presentation.AuthRoute import de.davis.keygo.feature.autofill.presentation.activity.component.AssociationDialog +import de.davis.keygo.feature.autofill.presentation.activity.component.SmsCodePendingDialog import de.davis.keygo.feature.autofill.presentation.activity.component.SuspicionDialog import de.davis.keygo.feature.autofill.presentation.activity.model.AssociationDialogVisibility import de.davis.keygo.feature.autofill.presentation.activity.model.AutofillEvent @@ -55,7 +52,6 @@ import org.koin.androidx.compose.koinViewModel */ internal class AutofillActivity : FragmentActivity() { - @OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterial3ExpressiveApi::class) override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -175,11 +171,9 @@ internal class AutofillActivity : FragmentActivity() { ) if (uiState.showSmsPending) - BasicAlertDialog( - onDismissRequest = {} - ) { - LoadingIndicator() - } + SmsCodePendingDialog( + onCancel = { viewModel.onEvent(AutofillUiEvent.OnCancelSmsCode) } + ) } } } diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt index ff16d6fc8..86f44a450 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt @@ -197,6 +197,13 @@ internal class AutofillViewModel( else viewModelScope.launch { eventChannel.send(AutofillEvent.Abort) } } + private fun cancelSmsRetrieval() { + smsOtpJob?.cancel() + smsOtpJob = null + _uiState.update { it.copy(showSmsPending = false) } + viewModelScope.launch { eventChannel.send(AutofillEvent.Abort) } + } + private suspend fun sendSmsFillEvent(code: String) { val targetField = (requestData as? FillRequestData.SmsOtp)?.form?.fields?.firstOrNull() ?: run { eventChannel.send(AutofillEvent.Abort) @@ -305,6 +312,7 @@ internal class AutofillViewModel( } is AutofillUiEvent.OnSmsConsentResult -> onSmsConsentResult(event.granted) + AutofillUiEvent.OnCancelSmsCode -> cancelSmsRetrieval() } } diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/component/SmsCodePendingDialog.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/component/SmsCodePendingDialog.kt new file mode 100644 index 000000000..ac29f1ca3 --- /dev/null +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/component/SmsCodePendingDialog.kt @@ -0,0 +1,65 @@ +package de.davis.keygo.feature.autofill.presentation.activity.component + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi +import androidx.compose.material3.Icon +import androidx.compose.material3.LoadingIndicator +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.painterResource +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import de.davis.keygo.feature.autofill.R + +@OptIn(ExperimentalMaterial3ExpressiveApi::class) +@Composable +internal fun SmsCodePendingDialog( + onCancel: () -> Unit, + modifier: Modifier = Modifier +) { + AlertDialog( + onDismissRequest = onCancel, + confirmButton = { + TextButton( + onClick = onCancel + ) { + Text(text = stringResource(R.string.cancel)) + } + }, + icon = { + Icon(painter = painterResource(R.drawable.outline_sms_24), contentDescription = null) + }, + title = { + Text(text = stringResource(R.string.waiting_for_sms_code)) + }, + text = { + Column( + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.spacedBy(16.dp), + ) { + Text(text = stringResource(R.string.waiting_for_sms_code_description)) + LoadingIndicator() + } + }, + modifier = modifier + ) +} + +@Preview +@Composable +private fun SmsCodePendingDialogPreview() { + MaterialTheme { + SmsCodePendingDialog( + onCancel = {}, + modifier = Modifier.fillMaxWidth() + ) + } +} diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillUiEvent.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillUiEvent.kt index 1a843d4f8..584781468 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillUiEvent.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/model/AutofillUiEvent.kt @@ -15,4 +15,5 @@ internal sealed interface AutofillUiEvent { data class OnGeneratedPassword(val password: String) : AutofillUiEvent data class OnSmsConsentResult(val granted: Boolean) : AutofillUiEvent + data object OnCancelSmsCode : AutofillUiEvent } \ No newline at end of file diff --git a/feature/autofill/src/main/res/values/strings.xml b/feature/autofill/src/main/res/values/strings.xml index 02da9889b..f7163096c 100644 --- a/feature/autofill/src/main/res/values/strings.xml +++ b/feature/autofill/src/main/res/values/strings.xml @@ -20,4 +20,6 @@ Use KeyGo Autofill SMS Code + Waiting for SMS code + KeyGo will fill in the code as soon as your message arrives. \ No newline at end of file diff --git a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt index c3bbdb5cf..c2ba0de18 100644 --- a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt +++ b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt @@ -613,4 +613,37 @@ internal class AutofillViewModelTest { assertEquals(AutofillEvent.Abort, abortDeferred.await()) assertEquals(2, smsCodeRepo.callCount) } + + @Test + fun `cancelling sms retrieval aborts and clears the pending state`() = runTest { + smsCodeRepo.gate = CompletableDeferred() + + val vm = buildVm(smsOtpRequest(listOf(credField(FieldType.TOTP, viewId = 1)))) + vm.start() + assertTrue(vm.uiState.value.showSmsPending) + + val abortDeferred = async { vm.events.first() } + vm.onEvent(AutofillUiEvent.OnCancelSmsCode) + + assertEquals(AutofillEvent.Abort, abortDeferred.await()) + assertFalse(vm.uiState.value.showSmsPending) + } + + @Test + fun `a code arriving after cancellation does not fill`() = runTest { + val gate = CompletableDeferred() + smsCodeRepo.gate = gate + smsCodeRepo.enqueue(Result.Success("999999")) + + val vm = buildVm(smsOtpRequest(listOf(credField(FieldType.TOTP, viewId = 1)))) + vm.start() + + val abortDeferred = async { vm.events.first() } + vm.onEvent(AutofillUiEvent.OnCancelSmsCode) + assertEquals(AutofillEvent.Abort, abortDeferred.await()) + + gate.complete(Unit) + + assertTrue(datasetProvider.getFillingDatasetCalls.isEmpty()) + } } From 6775880921b3bdd5d5c6d90a87d05e4c38c76b19 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 26 Aug 2026 22:17:37 +0200 Subject: [PATCH 09/12] fix(autofill): harden sms code retrieval against play services failures Broaden canOfferSuggestion's catch and bound its play services calls with a timeout, so a slow or unhappy play services costs at most the sms suggestion rather than the whole fill response. Guard registerReceiver and the consent resolution so a throw aborts the autofill instead of crashing the activity. Fill the focused totp field rather than the first one, and clear the pending flag on every terminal path. Adds the first tests for the status code mapping, including the USER_PERMISSION_REQUIRED branch the design inferred from the api surface. Co-Authored-By: Claude Opus 5 --- .../activity/AutofillViewModel.kt | 19 ++- .../presentation/sms/GmsSmsCodeRepository.kt | 121 ++++++++++------- .../sms/GmsSmsCodeRepositoryTest.kt | 127 ++++++++++++++++++ 3 files changed, 213 insertions(+), 54 deletions(-) create mode 100644 feature/autofill/src/testPlayStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepositoryTest.kt diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt index 86f44a450..a9dc335fc 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModel.kt @@ -181,12 +181,17 @@ internal class AutofillViewModel( // Asking a second time would mean the consent screen came back OK without // actually granting anything, so stop rather than spin. is SmsCodeFailure.ConsentRequired -> - if (consentAlreadyRequested) eventChannel.send(AutofillEvent.Abort) - else eventChannel.send( + if (consentAlreadyRequested) { + _uiState.update { it.copy(showSmsPending = false) } + eventChannel.send(AutofillEvent.Abort) + } else eventChannel.send( AutofillEvent.RequestSmsConsent(failure.intentSender), ) - else -> eventChannel.send(AutofillEvent.Abort) + else -> { + _uiState.update { it.copy(showSmsPending = false) } + eventChannel.send(AutofillEvent.Abort) + } } } } @@ -194,7 +199,10 @@ internal class AutofillViewModel( private fun onSmsConsentResult(granted: Boolean) { if (granted) startSmsRetrieval(consentAlreadyRequested = true) - else viewModelScope.launch { eventChannel.send(AutofillEvent.Abort) } + else viewModelScope.launch { + _uiState.update { it.copy(showSmsPending = false) } + eventChannel.send(AutofillEvent.Abort) + } } private fun cancelSmsRetrieval() { @@ -205,7 +213,8 @@ internal class AutofillViewModel( } private suspend fun sendSmsFillEvent(code: String) { - val targetField = (requestData as? FillRequestData.SmsOtp)?.form?.fields?.firstOrNull() ?: run { + val fields = (requestData as? FillRequestData.SmsOtp)?.form?.fields + val targetField = fields?.firstOrNull { it.focused } ?: fields?.firstOrNull() ?: run { eventChannel.send(AutofillEvent.Abort) return } diff --git a/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt b/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt index 3683fff4b..23fa8e818 100644 --- a/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt +++ b/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt @@ -11,16 +11,17 @@ import com.google.android.gms.auth.api.phone.SmsCodeAutofillClient import com.google.android.gms.auth.api.phone.SmsCodeRetriever import com.google.android.gms.auth.api.phone.SmsRetriever import com.google.android.gms.auth.api.phone.SmsRetrieverStatusCodes -import com.google.android.gms.common.api.ApiException import com.google.android.gms.common.api.CommonStatusCodes import com.google.android.gms.common.api.ResolvableApiException import com.google.android.gms.common.api.Status import de.davis.keygo.core.util.Result +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.tasks.await +import kotlinx.coroutines.withTimeoutOrNull import org.koin.core.annotation.Single @Single(binds = [SmsCodeRepository::class]) @@ -33,15 +34,20 @@ internal class GmsSmsCodeRepository( } override suspend fun canOfferSuggestion(targetPackage: String): Boolean = try { - if (client.hasOngoingSmsRequest(targetPackage).await()) return false - when (client.checkPermissionState().await()) { - SmsCodeAutofillClient.PermissionState.GRANTED, - SmsCodeAutofillClient.PermissionState.NONE -> true + withTimeoutOrNull(PLAY_SERVICES_TIMEOUT_MS) { + if (client.hasOngoingSmsRequest(targetPackage).await()) return@withTimeoutOrNull false - else -> false - } - } catch (e: ApiException) { - Log.e(TAG, "SMS code autofill unavailable: ${e.statusCode}", e) + when (client.checkPermissionState().await()) { + SmsCodeAutofillClient.PermissionState.GRANTED, + SmsCodeAutofillClient.PermissionState.NONE -> true + + else -> false + } + } ?: false + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Log.e(TAG, "SMS code autofill unavailable", e) false } @@ -58,58 +64,75 @@ internal class GmsSmsCodeRepository( intent, SmsRetriever.EXTRA_STATUS, Status::class.java, ) - trySend(intent.toResult(status)) + trySend(intent.toSmsCodeResult(status)) } } - ContextCompat.registerReceiver( - context, - receiver, - IntentFilter(SmsCodeRetriever.SMS_CODE_RETRIEVED_ACTION), - SmsRetriever.SEND_PERMISSION, - null, - ContextCompat.RECEIVER_EXPORTED, - ) - - // Start only after the receiver is live, otherwise a code that is - // already sitting in the inbox can be delivered before you listen. - try { - client.startSmsCodeRetriever().await() - } catch (e: ResolvableApiException) { - trySend(Result.Failure(SmsCodeFailure.ConsentRequired(e.resolution.intentSender))) + val registered = try { + ContextCompat.registerReceiver( + context, + receiver, + IntentFilter(SmsCodeRetriever.SMS_CODE_RETRIEVED_ACTION), + SmsRetriever.SEND_PERMISSION, + null, + ContextCompat.RECEIVER_EXPORTED, + ) + true } catch (e: Exception) { + Log.e(TAG, "Could not register the sms code receiver", e) trySend(Result.Failure(SmsCodeFailure.Unknown(e))) + false } - awaitClose { runCatching { context.unregisterReceiver(receiver) } } - } - - private fun Intent.toResult(status: Status?): Result = - when (status?.statusCode) { - CommonStatusCodes.SUCCESS -> { - val code = getStringExtra(SmsCodeRetriever.EXTRA_SMS_CODE) - if (code.isNullOrBlank()) - Result.Failure(SmsCodeFailure.Unknown(IllegalStateException("empty code"))) - else Result.Success(code) - } - - CommonStatusCodes.TIMEOUT -> Result.Failure(SmsCodeFailure.Timeout) - - // The consent resolution can arrive here as well as on the start call, so both paths - // have to produce ConsentRequired. Play services does not document whether this Status - // always carries a resolution, so a missing one degrades to Unavailable. - SmsRetrieverStatusCodes.USER_PERMISSION_REQUIRED -> { - val intentSender = status.resolution?.intentSender - if (intentSender == null) Result.Failure(SmsCodeFailure.Unavailable) - else Result.Failure(SmsCodeFailure.ConsentRequired(intentSender)) + if (registered) + // Start only after the receiver is live, otherwise a code that is + // already sitting in the inbox can be delivered before you listen. + try { + client.startSmsCodeRetriever().await() + } catch (e: ResolvableApiException) { + val intentSender = e.resolution?.intentSender + if (intentSender == null) trySend(Result.Failure(SmsCodeFailure.Unavailable)) + else trySend(Result.Failure(SmsCodeFailure.ConsentRequired(intentSender))) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + trySend(Result.Failure(SmsCodeFailure.Unknown(e))) } - else -> Result.Failure( - SmsCodeFailure.Unknown(IllegalStateException("status=${status?.statusCode}")), - ) + awaitClose { + if (registered) runCatching { context.unregisterReceiver(receiver) } } + } private companion object { const val TAG = "GmsSmsCodeRepository" + const val PLAY_SERVICES_TIMEOUT_MS = 500L } } + +// toSmsCodeResult is a pure function of an Intent and a Status, kept top level so it can be +// unit tested without a running Play services connection. +internal fun Intent.toSmsCodeResult(status: Status?): Result = + when (status?.statusCode) { + CommonStatusCodes.SUCCESS -> { + val code = getStringExtra(SmsCodeRetriever.EXTRA_SMS_CODE) + if (code.isNullOrBlank()) + Result.Failure(SmsCodeFailure.Unknown(IllegalStateException("empty code"))) + else Result.Success(code) + } + + CommonStatusCodes.TIMEOUT -> Result.Failure(SmsCodeFailure.Timeout) + + // The consent resolution can arrive here as well as on the start call, so both paths + // have to produce ConsentRequired. Play services does not document whether this Status + // always carries a resolution, so a missing one degrades to Unavailable. + SmsRetrieverStatusCodes.USER_PERMISSION_REQUIRED -> { + val intentSender = status.resolution?.intentSender + if (intentSender == null) Result.Failure(SmsCodeFailure.Unavailable) + else Result.Failure(SmsCodeFailure.ConsentRequired(intentSender)) + } + + else -> Result.Failure( + SmsCodeFailure.Unknown(IllegalStateException("status=${status?.statusCode}")), + ) + } diff --git a/feature/autofill/src/testPlayStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepositoryTest.kt b/feature/autofill/src/testPlayStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepositoryTest.kt new file mode 100644 index 000000000..5d449f63d --- /dev/null +++ b/feature/autofill/src/testPlayStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepositoryTest.kt @@ -0,0 +1,127 @@ +package de.davis.keygo.feature.autofill.presentation.sms + +import android.app.PendingIntent +import android.content.Intent +import com.google.android.gms.auth.api.phone.SmsCodeRetriever +import com.google.android.gms.auth.api.phone.SmsRetrieverStatusCodes +import com.google.android.gms.common.api.CommonStatusCodes +import com.google.android.gms.common.api.Status +import de.davis.keygo.core.util.Result +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.annotation.Config +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertIs + +/** + * Covers [Intent.toSmsCodeResult], the pure mapping from a broadcast [Status] to a + * [SmsCodeFailure]-carrying [Result]. The USER_PERMISSION_REQUIRED branch in particular is + * behaviour the design doc inferred from the Play services API surface rather than from + * documentation, so it is worth pinning down explicitly. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +internal class GmsSmsCodeRepositoryTest { + + private fun testPendingIntent(): PendingIntent = PendingIntent.getActivity( + RuntimeEnvironment.getApplication(), + 0, + Intent(), + PendingIntent.FLAG_IMMUTABLE, + ) + + @Test + fun `success with a code returns Success`() { + val intent = Intent().putExtra(SmsCodeRetriever.EXTRA_SMS_CODE, "123456") + val status = Status(CommonStatusCodes.SUCCESS) + + val result = intent.toSmsCodeResult(status) + + assertIs>(result) + assertEquals("123456", result.success) + } + + @Test + fun `success with a missing code returns Unknown`() { + val intent = Intent() + val status = Status(CommonStatusCodes.SUCCESS) + + val result = intent.toSmsCodeResult(status) + + assertIs>(result) + assertIs(result.error) + } + + @Test + fun `success with a blank code returns Unknown`() { + val intent = Intent().putExtra(SmsCodeRetriever.EXTRA_SMS_CODE, " ") + val status = Status(CommonStatusCodes.SUCCESS) + + val result = intent.toSmsCodeResult(status) + + assertIs>(result) + assertIs(result.error) + } + + @Test + fun `timeout returns Timeout`() { + val intent = Intent() + val status = Status(CommonStatusCodes.TIMEOUT) + + val result = intent.toSmsCodeResult(status) + + assertIs>(result) + assertEquals(SmsCodeFailure.Timeout, result.error) + } + + @Test + fun `permission required with a resolution returns ConsentRequired`() { + val pendingIntent = testPendingIntent() + val intent = Intent() + val status = Status( + SmsRetrieverStatusCodes.USER_PERMISSION_REQUIRED, + "permission required", + pendingIntent, + ) + + val result = intent.toSmsCodeResult(status) + + assertIs>(result) + val failure = assertIs(result.error) + assertEquals(pendingIntent.intentSender, failure.intentSender) + } + + @Test + fun `permission required without a resolution returns Unavailable`() { + val intent = Intent() + val status = Status(SmsRetrieverStatusCodes.USER_PERMISSION_REQUIRED) + + val result = intent.toSmsCodeResult(status) + + assertIs>(result) + assertEquals(SmsCodeFailure.Unavailable, result.error) + } + + @Test + fun `unrecognised status code returns Unknown`() { + val intent = Intent() + val status = Status(999_999) + + val result = intent.toSmsCodeResult(status) + + assertIs>(result) + assertIs(result.error) + } + + @Test + fun `null status returns Unknown`() { + val intent = Intent() + + val result = intent.toSmsCodeResult(null) + + assertIs>(result) + assertIs(result.error) + } +} From 49a965b3b4eaa3b2aba1deec2409babce45355a9 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 26 Aug 2026 22:38:05 +0200 Subject: [PATCH 10/12] fix(autofill): unregister the sms receiver when the retrieval is cancelled Rethrowing CancellationException propagated out of the callbackFlow block and skipped the awaitClose call that followed it, so a receiver registered before the cancellation was never unregistered. Move awaitClose into a finally so it runs however the block exits. Also suppress the unnecessary safe call warning on the consent resolution, which is deliberate: the @NonNull annotation on getResolution is not kept by its implementation, which delegates to a nullable Status.getResolution. Co-Authored-By: Claude Opus 5 --- .../presentation/sms/GmsSmsCodeRepository.kt | 42 ++++++++++++------- .../activity/AutofillViewModelTest.kt | 2 + 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt b/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt index 23fa8e818..3e6374c12 100644 --- a/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt +++ b/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt @@ -84,23 +84,33 @@ internal class GmsSmsCodeRepository( false } - if (registered) - // Start only after the receiver is live, otherwise a code that is - // already sitting in the inbox can be delivered before you listen. - try { - client.startSmsCodeRetriever().await() - } catch (e: ResolvableApiException) { - val intentSender = e.resolution?.intentSender - if (intentSender == null) trySend(Result.Failure(SmsCodeFailure.Unavailable)) - else trySend(Result.Failure(SmsCodeFailure.ConsentRequired(intentSender))) - } catch (e: CancellationException) { - throw e - } catch (e: Exception) { - trySend(Result.Failure(SmsCodeFailure.Unknown(e))) + try { + if (registered) + // Start only after the receiver is live, otherwise a code that is + // already sitting in the inbox can be delivered before you listen. + try { + client.startSmsCodeRetriever().await() + } catch (e: ResolvableApiException) { + // ResolvableApiException.getResolution() is annotated @NonNull, but its + // implementation just delegates to Status.getResolution(), which the + // library itself annotates @Nullable and backs with a plain field. The + // annotation is not honored by the implementation, so the safe call stays. + @Suppress("UNNECESSARY_SAFE_CALL") + val intentSender = e.resolution?.intentSender + if (intentSender == null) trySend(Result.Failure(SmsCodeFailure.Unavailable)) + else trySend(Result.Failure(SmsCodeFailure.ConsentRequired(intentSender))) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + trySend(Result.Failure(SmsCodeFailure.Unknown(e))) + } + } finally { + // awaitClose must run however the block above exits, including a rethrown + // CancellationException, otherwise a receiver that was registered above is + // never unregistered and leaks against the application context. + awaitClose { + if (registered) runCatching { context.unregisterReceiver(receiver) } } - - awaitClose { - if (registered) runCatching { context.unregisterReceiver(receiver) } } } diff --git a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt index c2ba0de18..2b7f7eefc 100644 --- a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt +++ b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt @@ -536,6 +536,7 @@ internal class AutofillViewModelTest { vm.start() assertEquals(AutofillEvent.Abort, eventDeferred.await(), "failed for $failure") + assertFalse(vm.uiState.value.showSmsPending, "failed for $failure") } } @@ -593,6 +594,7 @@ internal class AutofillViewModelTest { vm.onEvent(AutofillUiEvent.OnSmsConsentResult(granted = false)) assertEquals(AutofillEvent.Abort, abortDeferred.await()) + assertFalse(vm.uiState.value.showSmsPending) } @Test From de14b74ce597659aad090edfce129588fdd147ba Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 27 Aug 2026 00:17:50 +0200 Subject: [PATCH 11/12] refactor: drop flow.first() --- .../presentation/sms/GmsSmsCodeRepository.kt | 79 ++++++++----------- 1 file changed, 35 insertions(+), 44 deletions(-) diff --git a/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt b/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt index 3e6374c12..301bc9170 100644 --- a/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt +++ b/feature/autofill/src/playStore/kotlin/de/davis/keygo/feature/autofill/presentation/sms/GmsSmsCodeRepository.kt @@ -16,15 +16,13 @@ import com.google.android.gms.common.api.ResolvableApiException import com.google.android.gms.common.api.Status import de.davis.keygo.core.util.Result import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.channels.awaitClose -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.callbackFlow -import kotlinx.coroutines.flow.first +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.tasks.await import kotlinx.coroutines.withTimeoutOrNull import org.koin.core.annotation.Single +import kotlin.time.Duration.Companion.milliseconds -@Single(binds = [SmsCodeRepository::class]) +@Single internal class GmsSmsCodeRepository( private val context: Context, ) : SmsCodeRepository { @@ -34,7 +32,7 @@ internal class GmsSmsCodeRepository( } override suspend fun canOfferSuggestion(targetPackage: String): Boolean = try { - withTimeoutOrNull(PLAY_SERVICES_TIMEOUT_MS) { + withTimeoutOrNull(PLAY_SERVICES_TIMEOUT) { if (client.hasOngoingSmsRequest(targetPackage).await()) return@withTimeoutOrNull false when (client.checkPermissionState().await()) { @@ -51,24 +49,20 @@ internal class GmsSmsCodeRepository( false } - // The flow below only ever produces one value. It stays a flow because awaitClose is what - // unregisters the receiver, and first() cancels the flow on every path: a delivered code, a - // failure, or the caller being cancelled. - override suspend fun retrieveSmsCode(): Result = smsCodes().first() + override suspend fun retrieveSmsCode(): Result { + val code = CompletableDeferred>() - private fun smsCodes(): Flow> = callbackFlow { val receiver = object : BroadcastReceiver() { override fun onReceive(c: Context, intent: Intent) { if (intent.action != SmsCodeRetriever.SMS_CODE_RETRIEVED_ACTION) return val status = IntentCompat.getParcelableExtra( intent, SmsRetriever.EXTRA_STATUS, Status::class.java, ) - - trySend(intent.toSmsCodeResult(status)) + code.complete(intent.toSmsCodeResult(status)) } } - val registered = try { + try { ContextCompat.registerReceiver( context, receiver, @@ -77,46 +71,43 @@ internal class GmsSmsCodeRepository( null, ContextCompat.RECEIVER_EXPORTED, ) - true } catch (e: Exception) { Log.e(TAG, "Could not register the sms code receiver", e) - trySend(Result.Failure(SmsCodeFailure.Unknown(e))) - false + return Result.Failure(SmsCodeFailure.Unknown(e)) } - try { - if (registered) - // Start only after the receiver is live, otherwise a code that is - // already sitting in the inbox can be delivered before you listen. - try { - client.startSmsCodeRetriever().await() - } catch (e: ResolvableApiException) { - // ResolvableApiException.getResolution() is annotated @NonNull, but its - // implementation just delegates to Status.getResolution(), which the - // library itself annotates @Nullable and backs with a plain field. The - // annotation is not honored by the implementation, so the safe call stays. - @Suppress("UNNECESSARY_SAFE_CALL") - val intentSender = e.resolution?.intentSender - if (intentSender == null) trySend(Result.Failure(SmsCodeFailure.Unavailable)) - else trySend(Result.Failure(SmsCodeFailure.ConsentRequired(intentSender))) - } catch (e: CancellationException) { - throw e - } catch (e: Exception) { - trySend(Result.Failure(SmsCodeFailure.Unknown(e))) - } + return try { + // Start only after the receiver is live, otherwise a code that is + // already sitting in the inbox can be delivered before you listen. + client.startSmsCodeRetriever().await() + code.await() + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Result.Failure(e.toSmsCodeFailure()) } finally { - // awaitClose must run however the block above exits, including a rethrown - // CancellationException, otherwise a receiver that was registered above is - // never unregistered and leaks against the application context. - awaitClose { - if (registered) runCatching { context.unregisterReceiver(receiver) } - } + runCatching { context.unregisterReceiver(receiver) } } } + private fun Exception.toSmsCodeFailure(): SmsCodeFailure = when (this) { + is ResolvableApiException -> { + // ResolvableApiException.getResolution() is annotated @NonNull, but its + // implementation just delegates to Status.getResolution(), which the + // library itself annotates @Nullable and backs with a plain field. The + // annotation is not honored by the implementation, so the safe call stays. + @Suppress("UNNECESSARY_SAFE_CALL") + val intentSender = resolution?.intentSender + if (intentSender == null) SmsCodeFailure.Unavailable + else SmsCodeFailure.ConsentRequired(intentSender) + } + + else -> SmsCodeFailure.Unknown(this) + } + private companion object { const val TAG = "GmsSmsCodeRepository" - const val PLAY_SERVICES_TIMEOUT_MS = 500L + val PLAY_SERVICES_TIMEOUT = 500L.milliseconds } } From 9fe3b844bcf0e97258c5aa956cac55f90a405622 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 27 Aug 2026 01:24:10 +0200 Subject: [PATCH 12/12] refactor: cleanup save info appender --- .../presentation/KeyGoAutofillService.kt | 25 +++- .../presentation/dataset/SaveInfoAppender.kt | 133 +++++++++++++----- .../dataset/SaveInfoAppenderTest.kt | 114 ++++++++++++++- 3 files changed, 222 insertions(+), 50 deletions(-) diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/KeyGoAutofillService.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/KeyGoAutofillService.kt index acfd98e32..9e7b643ce 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/KeyGoAutofillService.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/KeyGoAutofillService.kt @@ -85,17 +85,30 @@ internal class KeyGoAutofillService : AutofillService() { Log.d(TAG, "In Compatibility Mode: $inCompatibilityMode") Log.d(TAG, "Extracted form: $form") - val dataset = datasetProvider.getAutofillDatasets(request, form) - val response = FillResponse.Builder().apply { - dataset.forEach(::addDataset) - applySaveInfo( + val datasets = datasetProvider.getAutofillDatasets(request, form) + val builder = FillResponse.Builder() + datasets.forEach(builder::addDataset) + + // A broken save info must never cost us the datasets: without them we cannot fill + // anything at all, while a missing save info only skips a single save prompt. + val appliedSaveInfo = runCatching { + builder.applySaveInfo( form = form, clientInfo = request.clientState ?: bundleOf(), requestId = request.id, inCompatibilityMode = inCompatibilityMode ) - }.build() - callback.onSuccess(response) + }.onFailure { + Log.w(TAG, "Could not apply save info", it) + }.getOrDefault(false) + + if (datasets.isEmpty() && !appliedSaveInfo) { + Log.d(TAG, "Neither datasets nor save info - not filling") + callback.onSuccess(null) + return@launch + } + + callback.onSuccess(builder.build()) } cancellationSignal.setOnCancelListener { diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppender.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppender.kt index a83e98938..d0239ca8e 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppender.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppender.kt @@ -6,6 +6,7 @@ import android.service.autofill.FillResponse import android.service.autofill.RegexValidator import android.service.autofill.SaveInfo import android.util.Log +import android.view.autofill.AutofillId import androidx.core.os.BundleCompat import de.davis.keygo.feature.autofill.presentation.model.FieldType import de.davis.keygo.feature.autofill.presentation.model.Form @@ -14,31 +15,45 @@ import de.davis.keygo.feature.autofill.presentation.model.FormType private const val TAG = "SaveInfoAppender" +/** + * Applies the save info for [form] to the response, if there is anything worth saving at all. + * + * The client state is updated and forwarded on every request, also when no save info is applied, so + * that the fields collected on earlier requests stay available for the requests that follow. + * + * @return whether a save info was applied. + */ internal fun FillResponse.Builder.applySaveInfo( form: Form, clientInfo: Bundle, requestId: Int, inCompatibilityMode: Boolean, -) { +): Boolean { val (updatedClientState, saveType) = clientInfo.updateState(requestId, form) + setClientState(updatedClientState) - val updatedForm = updatedClientState.getForm() - ?: throw IllegalStateException("No form in client state") - - val requiredIds = updatedForm.fields.map { it.autofillId }.toTypedArray() + val savableForm = updatedClientState.getForm() + if (savableForm == null || !savableForm.hasFields()) { + Log.d(TAG, "Nothing to save for request $requestId - no save info applied") + return false + } - if (requiredIds.isEmpty()) return + val saveIds = savableForm.toSaveIds() Log.d( TAG, "Applied Save Info:\n" + "- Request ID: $requestId\n" + - "- Current Form: $updatedForm\n" + - "- Save type: $saveType" + "- Current Form: $savableForm\n" + + "- Save type: $saveType\n" + + "- Required ids: ${saveIds.required}\n" + + "- Optional ids: ${saveIds.optional}" ) + val saveInfo = SaveInfo.Builder(saveType, saveIds.required.toTypedArray()).apply { + if (saveIds.optional.isNotEmpty()) + setOptionalIds(saveIds.optional.toTypedArray()) - val saveInfo = SaveInfo.Builder(saveType, requiredIds).apply { - val password = updatedForm.getPasswordField() + val password = savableForm.getPasswordField() if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { var flag = password?.let { 0 } ?: SaveInfo.FLAG_DELAY_SAVE @@ -57,7 +72,28 @@ internal fun FillResponse.Builder.applySaveInfo( }.build() setSaveInfo(saveInfo) - setClientState(updatedClientState) + return true +} + +internal data class SaveIds( + val required: List, + val optional: List, +) + +/** + * Splits the fields of this form into required and optional save ids. + * + * Must only be called on a form that [has fields][Form.hasFields], the framework rejects a save + * info without a single required id. + */ +internal fun Form.toSaveIds(): SaveIds { + val password = getPasswordField() + ?: return SaveIds(required = fields.map { it.autofillId }, optional = emptyList()) + + return SaveIds( + required = listOf(password.autofillId), + optional = fields.map { it.autofillId } - password.autofillId, + ) } private fun Form.getPasswordField(): FormField? { @@ -70,16 +106,38 @@ private val PASSWORD_REGEX = "^\\p{ASCII}*$".toPattern() private const val KEY_FORM = "form" private const val KEY_SAVE_TYPE = "saveType" +/** + * Folds [form] into the save context carried by this client state and returns the updated state + * together with the save type it accumulated. + * + * The save context only ever holds fields we are actually able to save, so a screen that has + * nothing savable on it (a one time code, for example) leaves the state as it is: it neither + * contributes fields nor invalidates the ones collected before it. + */ internal fun Bundle.updateState( requestId: Int, form: Form, ): Pair { classLoader = Form::class.java.classLoader - var saveType = getInt(KEY_SAVE_TYPE, SaveInfo.SAVE_DATA_TYPE_GENERIC) + val currentSaveType = getInt(KEY_SAVE_TYPE, SaveInfo.SAVE_DATA_TYPE_GENERIC) - val satisfiedForm = form.mapFields { it.copy(requestId = requestId) } - val existingForm = getForm() - val mergedForm = existingForm?.merge(satisfiedForm) ?: satisfiedForm + val savableForm = form + .mapFields { it.copy(requestId = requestId) } + .onlySavableFields() + + if (!savableForm.hasFields()) { + Log.d(TAG, "No savable fields in request $requestId - keeping the save context as is") + return Bundle(this) to currentSaveType + } + + // A form of another type belongs to a screen we are no longer on, so it starts a new save + // context instead of being merged into the stale one. + val previousForm = getForm()?.takeIf { it.type == savableForm.type } + val inheritedSaveType = if (previousForm != null) currentSaveType + else SaveInfo.SAVE_DATA_TYPE_GENERIC + + val mergedForm = previousForm?.let(savableForm::mergeWith) ?: savableForm + val saveType = inheritedSaveType or savableForm.saveDataType() return Bundle(this).apply { putParcelable( @@ -87,40 +145,41 @@ internal fun Bundle.updateState( mergedForm ) - satisfiedForm.fields.forEach { - if (it.requestId != requestId) return@forEach - - saveType = saveType or when (it.type) { - FieldType.Credentials.Password -> SaveInfo.SAVE_DATA_TYPE_PASSWORD - - FieldType.Credentials.Username -> SaveInfo.SAVE_DATA_TYPE_USERNAME - - FieldType.Credentials.EMail -> SaveInfo.SAVE_DATA_TYPE_EMAIL_ADDRESS - - FieldType.TOTP, - FieldType.Credentials.Phone, - FieldType.Undefined -> SaveInfo.SAVE_DATA_TYPE_GENERIC - } - } - putInt(KEY_SAVE_TYPE, saveType) } to saveType } -private fun Form.merge(other: Form): Form { - if (this.type != other.type) - throw IllegalArgumentException("Cannot merge forms of different type") +private fun Form.onlySavableFields() = copy(fields = fields.filter { it.type.includeInSaveInfo }) - val mergedFields = (this.fields + other.fields) - .filter { it.url == this.url /* only keep fields from the same URL */ && it.type.includeInSaveInfo } +/** + * Merges the fields collected by [previous] into this form. This form wins: it describes the screen + * the request came from, so its url decides which of the older fields are still relevant. + */ +private fun Form.mergeWith(previous: Form): Form { + val mergedFields = (previous.fields + fields) + .filter { it.url == url /* only keep fields from the same URL */ } .distinctBy { it.autofillId } return copy(fields = mergedFields) } +private fun Form.saveDataType() = fields.fold(SaveInfo.SAVE_DATA_TYPE_GENERIC) { saveType, field -> + saveType or when (field.type) { + FieldType.Credentials.Password -> SaveInfo.SAVE_DATA_TYPE_PASSWORD + + FieldType.Credentials.Username -> SaveInfo.SAVE_DATA_TYPE_USERNAME + + FieldType.Credentials.EMail -> SaveInfo.SAVE_DATA_TYPE_EMAIL_ADDRESS + + FieldType.TOTP, + FieldType.Credentials.Phone, + FieldType.Undefined -> SaveInfo.SAVE_DATA_TYPE_GENERIC + } +} + internal fun Bundle.getForm(): Form? = getKeyGoParcelable(KEY_FORM) private inline fun Bundle.getKeyGoParcelable(key: String): T? { classLoader = T::class.java.classLoader return BundleCompat.getParcelable(this, key, T::class.java) -} \ No newline at end of file +} diff --git a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppenderTest.kt b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppenderTest.kt index b2dc27d25..145c41b43 100644 --- a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppenderTest.kt +++ b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SaveInfoAppenderTest.kt @@ -1,6 +1,7 @@ package de.davis.keygo.feature.autofill.presentation.dataset import android.os.Bundle +import android.service.autofill.FillResponse import android.service.autofill.SaveInfo import de.davis.keygo.core.feature.autofill.autofillId import de.davis.keygo.feature.autofill.presentation.model.FieldType @@ -11,10 +12,11 @@ import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config import kotlin.test.Test +import kotlin.test.assertContentEquals import kotlin.test.assertEquals -import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertNotNull +import kotlin.test.assertNull import kotlin.test.assertTrue @RunWith(RobolectricTestRunner::class) @@ -45,6 +47,16 @@ internal class SaveInfoAppenderTest { isSuspicious = false, ) + /** + * A form of a type other than [FormType.Credentials] that still carries savable fields. There + * is no such form yet, [FormType.TOTP] only ever holds fields we cannot save, so this stands in + * for the form types still to come (a credit card, for example). + */ + private fun otherTypeForm( + fields: List, + url: String? = "https://example.com", + ) = totpForm(fields = fields, url = url) + private fun field( type: FieldType, viewId: Int = 1, @@ -91,6 +103,29 @@ internal class SaveInfoAppenderTest { assertEquals(SaveInfo.SAVE_DATA_TYPE_GENERIC, saveType) } + @Test + fun `a form without savable fields stores no form to build a save info from`() { + val form = totpForm(fields = listOf(field(FieldType.TOTP))) + val (bundle, _) = Bundle().updateState(requestId = 1, form = form) + assertNull(bundle.getForm()) + } + + @Test + fun `a form without savable fields keeps the collected fields`() { + val credentials = credentialsForm( + fields = listOf(field(FieldType.Credentials.Password, viewId = 1)), + ) + val (firstBundle, firstSaveType) = Bundle().updateState(requestId = 1, form = credentials) + + val otp = totpForm(fields = listOf(field(FieldType.TOTP, viewId = 2))) + val (secondBundle, secondSaveType) = firstBundle.updateState(requestId = 2, form = otp) + + val storedForm = secondBundle.getForm() + assertNotNull(storedForm) + assertEquals(listOf(FieldType.Credentials.Password), storedForm.fields.map { it.type }) + assertEquals(firstSaveType, secondSaveType) + } + @Test fun `undefined field maps to SAVE_DATA_TYPE_GENERIC`() { val form = credentialsForm(fields = listOf(field(FieldType.Undefined))) @@ -121,14 +156,23 @@ internal class SaveInfoAppenderTest { } @Test - fun `second call with a different form type throws`() { + fun `a savable form of another type starts a new save context`() { val firstForm = credentialsForm(fields = listOf(field(FieldType.Credentials.Password))) - val (firstBundle, _) = Bundle().updateState(requestId = 1, form = firstForm) + val (firstBundle, firstSaveType) = Bundle().updateState(requestId = 1, form = firstForm) + assertTrue((firstSaveType and SaveInfo.SAVE_DATA_TYPE_PASSWORD) != 0) + + val secondForm = otherTypeForm( + fields = listOf(field(FieldType.Credentials.Username, viewId = 2)), + ) + val (secondBundle, secondSaveType) = firstBundle.updateState( + requestId = 2, + form = secondForm, + ) - val secondForm = totpForm(fields = listOf(field(FieldType.TOTP, viewId = 2))) - assertFailsWith { - firstBundle.updateState(requestId = 2, form = secondForm) - } + val storedForm = secondBundle.getForm() + assertNotNull(storedForm) + assertEquals(listOf(FieldType.Credentials.Username), storedForm.fields.map { it.type }) + assertEquals(SaveInfo.SAVE_DATA_TYPE_USERNAME, secondSaveType) } @Test @@ -221,4 +265,60 @@ internal class SaveInfoAppenderTest { assertNotNull(mergedForm) assertEquals(1, mergedForm.fields.size) } + + @Test + fun `no save info is applied for a form without savable fields`() { + val form = totpForm(fields = listOf(field(FieldType.TOTP))) + + val applied = FillResponse.Builder().applySaveInfo( + form = form, + clientInfo = Bundle(), + requestId = 1, + inCompatibilityMode = false, + ) + + assertFalse(applied) + } + + @Test + fun `a form without savable fields keeps the save info of the fields collected before`() { + val credentials = credentialsForm( + fields = listOf(field(FieldType.Credentials.Password, viewId = 1)), + ) + val (clientState, _) = Bundle().updateState(requestId = 1, form = credentials) + + val otp = totpForm(fields = listOf(field(FieldType.TOTP, viewId = 2))) + val applied = FillResponse.Builder().applySaveInfo( + form = otp, + clientInfo = clientState, + requestId = 2, + inCompatibilityMode = false, + ) + + assertTrue(applied) + } + + @Test + fun `the password alone is required, the remaining fields are optional`() { + val username = field(FieldType.Credentials.Username, viewId = 1) + val password = field(FieldType.Credentials.Password, viewId = 2) + val form = credentialsForm(fields = listOf(username, password)) + + val saveIds = form.toSaveIds() + + assertContentEquals(listOf(password.autofillId), saveIds.required) + assertContentEquals(listOf(username.autofillId), saveIds.optional) + } + + @Test + fun `without a password every field is required`() { + val username = field(FieldType.Credentials.Username, viewId = 1) + val email = field(FieldType.Credentials.EMail, viewId = 2) + val form = credentialsForm(fields = listOf(username, email)) + + val saveIds = form.toSaveIds() + + assertContentEquals(listOf(username.autofillId, email.autofillId), saveIds.required) + assertTrue(saveIds.optional.isEmpty()) + } }