From 5e42e7161d5226dec44160eaf55136aaffcc7e35 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Tue, 25 May 2021 20:16:48 +0300 Subject: [PATCH] For #10331 - Allow dynamically toggling CC autofill (#10332) --- .../feature/prompts/PromptFeature.kt | 10 ++- .../feature/prompts/PromptFeatureTest.kt | 76 ++++++++++++++++++- ...eckoCreditCardsAddressesStorageDelegate.kt | 12 ++- ...CreditCardsAddressesStorageDelegateTest.kt | 29 ++++++- docs/changelog.md | 3 + 5 files changed, 123 insertions(+), 7 deletions(-) diff --git a/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt b/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt index b5283ad9b75..9db4b44665a 100644 --- a/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt +++ b/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt @@ -108,6 +108,9 @@ internal const val FRAGMENT_TAG = "mozac_feature_prompt_dialog" * 'save login'prompts will not be shown. * @property isSaveLoginEnabled A callback invoked when a login prompt is triggered. If false, * 'save login'prompts will not be shown. + * @property isCreditCardAutofillEnabled A callback invoked when credit card fields are detected in the webpage. + * If this resolves to `true` a prompt allowing the user to select the credit card details to be autocompleted + * will be shown. * @property loginExceptionStorage An implementation of [LoginExceptions] that saves and checks origins * the user does not want to see a save login dialog for. * @property loginPickerView The [SelectablePromptView] used for [LoginPicker] to display a @@ -131,6 +134,7 @@ class PromptFeature private constructor( private val shareDelegate: ShareDelegate, override val loginValidationDelegate: LoginValidationDelegate? = null, private val isSaveLoginEnabled: () -> Boolean = { false }, + private val isCreditCardAutofillEnabled: () -> Boolean = { false }, override val loginExceptionStorage: LoginExceptions? = null, private val loginPickerView: SelectablePromptView? = null, private val onManageLogins: () -> Unit = {}, @@ -160,6 +164,7 @@ class PromptFeature private constructor( shareDelegate: ShareDelegate = DefaultShareDelegate(), loginValidationDelegate: LoginValidationDelegate? = null, isSaveLoginEnabled: () -> Boolean = { false }, + isCreditCardAutofillEnabled: () -> Boolean = { false }, loginExceptionStorage: LoginExceptions? = null, loginPickerView: SelectablePromptView? = null, onManageLogins: () -> Unit = {}, @@ -174,6 +179,7 @@ class PromptFeature private constructor( shareDelegate = shareDelegate, loginValidationDelegate = loginValidationDelegate, isSaveLoginEnabled = isSaveLoginEnabled, + isCreditCardAutofillEnabled = isCreditCardAutofillEnabled, loginExceptionStorage = loginExceptionStorage, onNeedToRequestPermissions = onNeedToRequestPermissions, loginPickerView = loginPickerView, @@ -190,6 +196,7 @@ class PromptFeature private constructor( shareDelegate: ShareDelegate = DefaultShareDelegate(), loginValidationDelegate: LoginValidationDelegate? = null, isSaveLoginEnabled: () -> Boolean = { false }, + isCreditCardAutofillEnabled: () -> Boolean = { false }, loginExceptionStorage: LoginExceptions? = null, loginPickerView: SelectablePromptView? = null, onManageLogins: () -> Unit = {}, @@ -204,6 +211,7 @@ class PromptFeature private constructor( shareDelegate = shareDelegate, loginValidationDelegate = loginValidationDelegate, isSaveLoginEnabled = isSaveLoginEnabled, + isCreditCardAutofillEnabled = isCreditCardAutofillEnabled, loginExceptionStorage = loginExceptionStorage, onNeedToRequestPermissions = onNeedToRequestPermissions, loginPickerView = loginPickerView, @@ -369,7 +377,7 @@ class PromptFeature private constructor( is File -> filePicker.handleFileRequest(promptRequest) is Share -> handleShareRequest(promptRequest, session) is SelectCreditCard -> { - if (promptRequest.creditCards.isNotEmpty()) { + if (isCreditCardAutofillEnabled() && promptRequest.creditCards.isNotEmpty()) { creditCardPicker?.handleSelectCreditCardRequest(promptRequest) } } diff --git a/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt b/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt index 8c5da6de991..06edf28a922 100644 --- a/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt +++ b/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt @@ -44,6 +44,8 @@ import mozilla.components.concept.engine.prompt.PromptRequest.SingleChoice import mozilla.components.concept.engine.prompt.PromptRequest.TextPrompt import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.concept.storage.Login +import mozilla.components.feature.prompts.concept.SelectablePromptView +import mozilla.components.feature.prompts.creditcard.CreditCardPicker import mozilla.components.feature.prompts.dialog.ChoiceDialogFragment import mozilla.components.feature.prompts.dialog.ConfirmDialogFragment import mozilla.components.feature.prompts.dialog.MultiButtonDialogFragment @@ -51,8 +53,6 @@ import mozilla.components.feature.prompts.dialog.PromptDialogFragment import mozilla.components.feature.prompts.dialog.SaveLoginDialogFragment import mozilla.components.feature.prompts.file.FilePicker.Companion.FILE_PICKER_ACTIVITY_REQUEST_CODE import mozilla.components.feature.prompts.login.LoginPicker -import mozilla.components.feature.prompts.concept.SelectablePromptView -import mozilla.components.feature.prompts.creditcard.CreditCardPicker import mozilla.components.feature.prompts.share.ShareDelegate import mozilla.components.support.test.any import mozilla.components.support.test.eq @@ -1282,7 +1282,8 @@ class PromptFeatureTest { activity = mock(), store = store, fragmentManager = fragmentManager, - creditCardPickerView = creditCardPickerView + creditCardPickerView = creditCardPickerView, + isCreditCardAutofillEnabled = { true } ) { } feature.creditCardPicker = creditCardPicker val onDismiss: () -> Unit = {} @@ -1342,6 +1343,75 @@ class PromptFeatureTest { ) } + @Test + fun `GIVEN credit card autofill enabled and cards available WHEN getting a SelectCreditCard request THEN that request is handled`() { + val feature = spy( + PromptFeature( + mock(), + store, + customTabId = "custom-tab", + fragmentManager = fragmentManager, + isCreditCardAutofillEnabled = { true } + ) { } + ) + feature.creditCardPicker = creditCardPicker + feature.start() + val selectCreditCardRequest = PromptRequest.SelectCreditCard(listOf(mock()), {}, {}) + + store.dispatch(ContentAction.UpdatePromptRequestAction("custom-tab", selectCreditCardRequest)) + .joinBlocking() + testDispatcher.advanceUntilIdle() + + verify(feature).onPromptRequested(store.state.customTabs.first()) + verify(creditCardPicker).handleSelectCreditCardRequest(selectCreditCardRequest) + } + + @Test + fun `GIVEN credit card autofill enabled but no cards available WHEN getting a SelectCreditCard request THEN that request is not acted upon`() { + val feature = spy( + PromptFeature( + mock(), + store, + customTabId = "custom-tab", + fragmentManager = fragmentManager, + isCreditCardAutofillEnabled = { true } + ) { } + ) + feature.creditCardPicker = creditCardPicker + feature.start() + val selectCreditCardRequest = PromptRequest.SelectCreditCard(emptyList(), {}, {}) + + store.dispatch(ContentAction.UpdatePromptRequestAction("custom-tab", selectCreditCardRequest)) + .joinBlocking() + testDispatcher.advanceUntilIdle() + + verify(feature).onPromptRequested(store.state.customTabs.first()) + verify(creditCardPicker, never()).handleSelectCreditCardRequest(selectCreditCardRequest) + } + + @Test + fun `GIVEN credit card autofill disabled and cards available WHEN getting a SelectCreditCard request THEN that request is handled`() { + val feature = spy( + PromptFeature( + mock(), + store, + customTabId = "custom-tab", + fragmentManager = fragmentManager, + isCreditCardAutofillEnabled = { false } + ) { } + ) + feature.creditCardPicker = creditCardPicker + feature.start() + val selectCreditCardRequest = PromptRequest.SelectCreditCard(listOf(mock()), {}, {}) + + store.dispatch(ContentAction.UpdatePromptRequestAction("custom-tab", selectCreditCardRequest)) + .joinBlocking() + testDispatcher.advanceUntilIdle() + + verify(feature).onPromptRequested(store.state.customTabs.first()) + verify(creditCardPicker, never()).handleSelectCreditCardRequest(selectCreditCardRequest) + } + @Test fun `Selecting an item in a share dialog will consume promptRequest`() { val delegate: ShareDelegate = mock() diff --git a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt index fd6c1dd202e..c12c39ac998 100644 --- a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt +++ b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt @@ -4,6 +4,7 @@ package mozilla.components.service.sync.autofill +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Deferred import kotlinx.coroutines.Dispatchers @@ -16,10 +17,15 @@ import mozilla.components.concept.storage.CreditCardsAddressesStorageDelegate /** * [CreditCardsAddressesStorageDelegate] implementation. + * + * @param storage The [CreditCardsAddressesStorage] used for looking up addresses and credit cards to autofill. + * @param scope [CoroutineScope] for long running operations. Defaults to using the [Dispatchers.IO]. + * @param isCreditCardAutofillEnabled callback allowing to limit [storage] operations if autofill is disabled. */ class GeckoCreditCardsAddressesStorageDelegate( private val storage: Lazy, - private val scope: CoroutineScope = CoroutineScope(Dispatchers.IO) + private val scope: CoroutineScope = CoroutineScope(Dispatchers.IO), + private val isCreditCardAutofillEnabled: () -> Boolean = { false } ) : CreditCardsAddressesStorageDelegate { override fun decrypt(encryptedCardNumber: CreditCardNumber.Encrypted): CreditCardNumber.Plaintext? { @@ -39,6 +45,10 @@ class GeckoCreditCardsAddressesStorageDelegate( } override fun onCreditCardsFetch(): Deferred> { + if (isCreditCardAutofillEnabled().not()) { + return CompletableDeferred(listOf()) + } + return scope.async { storage.value.getAllCreditCards() } diff --git a/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt b/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt index 5f722ee0c82..4dd998ce0eb 100644 --- a/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt +++ b/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt @@ -9,14 +9,17 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.TestCoroutineScope +import mozilla.components.concept.storage.CreditCard import mozilla.components.concept.storage.CreditCardNumber import mozilla.components.concept.storage.NewCreditCardFields import mozilla.components.lib.dataprotect.SecureAbove22Preferences +import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.Mockito.doReturn import org.mockito.Mockito.times import org.mockito.Mockito.verify @@ -70,10 +73,32 @@ class GeckoCreditCardsAddressesStorageDelegateTest { } @Test - fun `onCreditCardFetch`() { + fun `GIVEN autofill enabled WHEN onCreditCardsFetch is called THEN it returns all stored cards`() { scope.launch { - delegate.onCreditCardsFetch() + val storage: AutofillCreditCardsAddressesStorage = mock() + val storedCards = listOf(mock()) + doReturn(storedCards).`when`(storage).getAllCreditCards() + delegate = GeckoCreditCardsAddressesStorageDelegate(lazy { storage }, scope) { true } + + val result = delegate.onCreditCardsFetch() + + verify(storage, times(1)).getAllCreditCards() + assertEquals(storedCards, result) + } + } + + @Test + fun `GIVEN autofill disabled WHEN onCreditCardsFetch is called THEN it returns an empty list of cards`() { + scope.launch { + val storage: AutofillCreditCardsAddressesStorage = mock() + val storedCards = listOf(mock()) + doReturn(storedCards).`when`(storage).getAllCreditCards() + delegate = GeckoCreditCardsAddressesStorageDelegate(lazy { storage }, scope) { false } + + val result = delegate.onCreditCardsFetch() + verify(storage, times(1)).getAllCreditCards() + assertEquals(emptyList(), result) } } } diff --git a/docs/changelog.md b/docs/changelog.md index 79ff61d1878..cd80fb59d70 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -12,6 +12,9 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/.config.yml) +* **feature-prompts** **browser-storage-sync** + * ⚠️ A new `isCreditCardAutofillEnabled` callback is available in `PromptFeature` and `GeckoCreditCardsAddressesStorageDelegate` to allow clients controlling whether credit cards should be autofilled or not. Default is false* + * **service-pocket** * ⚠️ **This is a breaking change**: Rebuilt from the ground up to better support offering to clients Pocket recommended articles. * See component's [README](https://github.com/mozilla-mobile/android-components/blob/master/components/service/pocket/README.md) to get more info.