Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Issue #10363 - Handle authentication of the credit card selection for a prompt request #10369

Merged
merged 2 commits into from
May 28, 2021
Merged

Issue #10363 - Handle authentication of the credit card selection for a prompt request #10369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
304e7dd
Issue #10363 - Handle authentication of the credit card selection for…
gabrielluong May 28, 2021
3f2f043
Merge branch 'master' into 10363
mergify[bot] May 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 53 additions & 5 deletions components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ internal const val FRAGMENT_TAG = "mozac_feature_prompt_dialog"
* a selectable prompt list of credit card options.
* @property onManageCreditCards A callback invoked when a user selects "Manage credit cards" from
* the select credit card prompt.
* @property onSelectCreditCard A callback invoked when a user selects a credit card from the
* select credit card prompt.
* @property onNeedToRequestPermissions A callback invoked when permissions
* need to be requested before a prompt (e.g. a file picker) can be displayed.
* Once the request is completed, [onPermissionsResult] needs to be invoked.
Expand All @@ -140,6 +142,7 @@ class PromptFeature private constructor(
private val onManageLogins: () -> Unit = {},
private val creditCardPickerView: SelectablePromptView<CreditCard>? = null,
private val onManageCreditCards: () -> Unit = {},
private val onSelectCreditCard: () -> Unit = {},
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : LifecycleAwareFeature, PermissionsFeature, Prompter, ActivityResultHandler,
UserInteractionHandler {
Expand Down Expand Up @@ -170,6 +173,7 @@ class PromptFeature private constructor(
onManageLogins: () -> Unit = {},
creditCardPickerView: SelectablePromptView<CreditCard>? = null,
onManageCreditCards: () -> Unit = {},
onSelectCreditCard: () -> Unit = {},
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this(
container = PromptContainer.Activity(activity),
Expand All @@ -185,7 +189,8 @@ class PromptFeature private constructor(
loginPickerView = loginPickerView,
onManageLogins = onManageLogins,
creditCardPickerView = creditCardPickerView,
onManageCreditCards = onManageCreditCards
onManageCreditCards = onManageCreditCards,
onSelectCreditCard = onSelectCreditCard
)

constructor(
Expand All @@ -202,6 +207,7 @@ class PromptFeature private constructor(
onManageLogins: () -> Unit = {},
creditCardPickerView: SelectablePromptView<CreditCard>? = null,
onManageCreditCards: () -> Unit = {},
onSelectCreditCard: () -> Unit = {},
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this(
container = PromptContainer.Fragment(fragment),
Expand All @@ -217,7 +223,8 @@ class PromptFeature private constructor(
loginPickerView = loginPickerView,
onManageLogins = onManageLogins,
creditCardPickerView = creditCardPickerView,
onManageCreditCards = onManageCreditCards
onManageCreditCards = onManageCreditCards,
onSelectCreditCard = onSelectCreditCard
)

@Deprecated("Pass only activity or fragment instead")
Expand All @@ -231,6 +238,7 @@ class PromptFeature private constructor(
onManageLogins: () -> Unit = {},
creditCardPickerView: SelectablePromptView<CreditCard>? = null,
onManageCreditCards: () -> Unit = {},
onSelectCreditCard: () -> Unit = {},
onNeedToRequestPermissions: OnNeedToRequestPermissions
) : this(
container = activity?.let { PromptContainer.Activity(it) }
Expand All @@ -248,7 +256,8 @@ class PromptFeature private constructor(
loginPickerView = loginPickerView,
onManageLogins = onManageLogins,
creditCardPickerView = creditCardPickerView,
onManageCreditCards = onManageCreditCards
onManageCreditCards = onManageCreditCards,
onSelectCreditCard = onSelectCreditCard
)

private val filePicker = FilePicker(container, store, customTabId, onNeedToRequestPermissions)
Expand All @@ -259,7 +268,15 @@ class PromptFeature private constructor(

@VisibleForTesting(otherwise = PRIVATE)
internal var creditCardPicker =
creditCardPickerView?.let { CreditCardPicker(store, it, onManageCreditCards, customTabId) }
creditCardPickerView?.let {
CreditCardPicker(
store = store,
creditCardSelectBar = it,
manageCreditCardsCallback = onManageCreditCards,
selectCreditCardCallback = onSelectCreditCard,
sessionId = customTabId
)
}

override val onNeedToRequestPermissions
get() = filePicker.onNeedToRequestPermissions
Expand Down Expand Up @@ -343,15 +360,41 @@ class PromptFeature private constructor(

/**
* Notifies the feature of intent results for prompt requests handled by
* other apps like file chooser requests.
* other apps like credit card and file chooser requests.
*
* @param requestCode The code of the app that requested the intent.
* @param data The result of the request.
* @param resultCode The code of the result.
*/
override fun onActivityResult(requestCode: Int, data: Intent?, resultCode: Int): Boolean {
if (requestCode == PIN_REQUEST) {
if (resultCode == Activity.RESULT_OK) {
creditCardPicker?.onAuthSuccess()
} else {
creditCardPicker?.onAuthFailure()
}

return true
}

return filePicker.onActivityResult(requestCode, resultCode, data)
}

/**
* Notifies the feature that the biometric authentication was completed. It will then
* either process or dismiss the prompt request.
*
* @param isAuthenticated True if the user is authenticated successfully from the biometric
* authentication prompt or false otherwise.
*/
fun onBiometricResult(isAuthenticated: Boolean) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit on naming: we're sort of lock ourselves into the idea that we always want biometrics for confirmation.

This isn't worth fixing at this point though.

if (isAuthenticated) {
creditCardPicker?.onAuthSuccess()
} else {
creditCardPicker?.onAuthFailure()
}
}

/**
* Notifies the feature that the permissions request was completed. It will then
* either process or dismiss the prompt request.
Expand Down Expand Up @@ -764,6 +807,11 @@ class PromptFeature private constructor(

return result
}

companion object {
// The PIN request code
const val PIN_REQUEST = 303
gabrielluong marked this conversation as resolved.
Show resolved Hide resolved
}
}

internal fun BrowserStore.consumePromptFrom(
Expand Down
38 changes: 36 additions & 2 deletions ...e/prompts/src/main/java/mozilla/components/feature/prompts/creditcard/CreditCardPicker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package mozilla.components.feature.prompts.creditcard

import androidx.annotation.VisibleForTesting
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.prompt.CreditCard
import mozilla.components.concept.engine.prompt.PromptRequest
Expand All @@ -20,30 +21,63 @@ import mozilla.components.support.base.log.logger.Logger
* prompt will be inflated.
* @property manageCreditCardsCallback A callback invoked when a user selects "Manage credit cards"
* from the select credit card prompt.
* @property selectCreditCardCallback A callback invoked when a user selects a credit card option
* from the select credit card prompt
* @property sessionId The session ID which requested the prompt.
*/
class CreditCardPicker(
private val store: BrowserStore,
private val creditCardSelectBar: SelectablePromptView<CreditCard>,
private val manageCreditCardsCallback: () -> Unit = {},
private val selectCreditCardCallback: () -> Unit = {},
private var sessionId: String? = null
) : SelectablePromptView.Listener<CreditCard> {

init {
creditCardSelectBar.listener = this
}

// The selected credit card option to confirm.
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal var selectedCreditCard: CreditCard? = null

override fun onManageOptions() {
manageCreditCardsCallback.invoke()
dismissSelectCreditCardRequest()
}

override fun onOptionSelect(option: CreditCard) {
selectedCreditCard = option
creditCardSelectBar.hidePrompt()
selectCreditCardCallback.invoke()
}

/**
* Called on a successful authentication to confirm the selected credit card option.
*/
fun onAuthSuccess() {
store.consumePromptFrom(sessionId) {
if (it is PromptRequest.SelectCreditCard) it.onConfirm(option)
if (it is PromptRequest.SelectCreditCard) {
selectedCreditCard?.let { creditCard ->
it.onConfirm(creditCard)
}

selectedCreditCard = null
}
}
}

creditCardSelectBar.hidePrompt()
/**
* Called on a failed authentication to dismiss the current select credit card prompt request.
*/
fun onAuthFailure() {
selectedCreditCard = null

store.consumePromptFrom(sessionId) {
if (it is PromptRequest.SelectCreditCard) {
it.onDismiss()
}
}
}

/**
Expand Down
76 changes: 75 additions & 1 deletion ...nts/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ class PromptFeatureTest {
}

@Test
fun `onActivityResult with RESULT_CANCELED will consume PromptRequest call onDismiss `() {
fun `onActivityResult with RESULT_CANCELED will consume PromptRequest call onDismiss`() {
var onDismissWasCalled = false

val filePickerRequest =
Expand All @@ -778,6 +778,80 @@ class PromptFeatureTest {
assertNull(tab()?.content?.promptRequest)
}

@Test
fun `WHEN onActivityResult is called with PIN_REQUEST and RESULT_OK THEN onAuthSuccess) is called`() {
val creditCardPickerView: SelectablePromptView<CreditCard> = mock()
val feature =
PromptFeature(
activity = mock(),
store = store,
fragmentManager = fragmentManager,
creditCardPickerView = creditCardPickerView,
isCreditCardAutofillEnabled = { true }
) { }
feature.creditCardPicker = creditCardPicker
val intent = Intent()

feature.onActivityResult(PromptFeature.PIN_REQUEST, intent, RESULT_OK)

verify(creditCardPicker).onAuthSuccess()
}

@Test
fun `WHEN onActivityResult is called with PIN_REQUEST and RESULT_CANCELED THEN onAuthFailure is called`() {
val creditCardPickerView: SelectablePromptView<CreditCard> = mock()
val feature =
PromptFeature(
activity = mock(),
store = store,
fragmentManager = fragmentManager,
creditCardPickerView = creditCardPickerView,
isCreditCardAutofillEnabled = { true }
) { }
feature.creditCardPicker = creditCardPicker
val intent = Intent()

feature.onActivityResult(PromptFeature.PIN_REQUEST, intent, RESULT_CANCELED)

verify(creditCardPicker).onAuthFailure()
}

@Test
fun `GIVEN user successfully authenticates by biometric prompt WHEN onBiometricResult is called THEN onAuthSuccess is called`() {
val creditCardPickerView: SelectablePromptView<CreditCard> = mock()
val feature =
PromptFeature(
activity = mock(),
store = store,
fragmentManager = fragmentManager,
creditCardPickerView = creditCardPickerView,
isCreditCardAutofillEnabled = { true }
) { }
feature.creditCardPicker = creditCardPicker

feature.onBiometricResult(isAuthenticated = true)

verify(creditCardPicker).onAuthSuccess()
}

@Test
fun `GIVEN user fails to authenticate by biometric prompt WHEN onBiometricResult is called THEN onAuthFailure) is called`() {
val creditCardPickerView: SelectablePromptView<CreditCard> = mock()
val feature =
PromptFeature(
activity = mock(),
store = store,
fragmentManager = fragmentManager,
creditCardPickerView = creditCardPickerView,
isCreditCardAutofillEnabled = { true }
) { }
feature.creditCardPicker = creditCardPicker

feature.onBiometricResult(isAuthenticated = false)

verify(creditCardPicker).onAuthFailure()
}

@Test
fun `Selecting a login confirms the request`() {
var onDismissWasCalled = false
Expand Down
39 changes: 36 additions & 3 deletions ...ompts/src/test/java/mozilla/components/feature/prompts/creditcard/CreditCardPickerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import mozilla.components.concept.engine.prompt.PromptRequest
import mozilla.components.support.test.mock
import mozilla.components.support.test.whenever
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
Expand Down Expand Up @@ -46,7 +47,9 @@ class CreditCardPickerTest {
)

var manageCreditCardsCalled = false
var selectCreditCardCalled = false
private val manageCreditCardsCallback: () -> Unit = { manageCreditCardsCalled = true }
private val selectCreditCardCallback: () -> Unit = { selectCreditCardCalled = true }

@Before
fun setup() {
Expand All @@ -56,21 +59,25 @@ class CreditCardPickerTest {
creditCardPicker = CreditCardPicker(
store = store,
creditCardSelectBar = creditCardSelectBar,
manageCreditCardsCallback = manageCreditCardsCallback
manageCreditCardsCallback = manageCreditCardsCallback,
selectCreditCardCallback = selectCreditCardCallback
)

whenever(store.state).thenReturn(state)
}

@Test
fun `WHEN onOptionSelect is called with a credit card THEN the confirmed credit card is received and prompt is hidden`() {
fun `WHEN onOptionSelect is called with a credit card THEN selectCreditCardCallback is invoked and prompt is hidden`() {
assertNull(creditCardPicker.selectedCreditCard)

setupSessionState(promptRequest)

creditCardPicker.onOptionSelect(creditCard)

verify(creditCardSelectBar).hidePrompt()

assertEquals(creditCard, confirmedCreditCard)
assertTrue(selectCreditCardCalled)
assertEquals(creditCard, creditCardPicker.selectedCreditCard)
}

@Test
Expand Down Expand Up @@ -105,6 +112,32 @@ class CreditCardPickerTest {
verify(creditCardSelectBar).showPrompt(promptRequest.creditCards)
}

@Test
fun `GIVEN a selected credit card WHEN onAuthSuccess is called THEN the confirmed credit card is received`() {
assertNull(creditCardPicker.selectedCreditCard)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also confirm the prompt request was consume and confirmed? :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do that by assertEquals(creditCard, confirmedCreditCard) check below

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification, as this value is shared for multiple tests, could be reset it on setup() 👍🏽


setupSessionState(promptRequest)

creditCardPicker.onOptionSelect(creditCard)
creditCardPicker.onAuthSuccess()

assertEquals(creditCard, confirmedCreditCard)
assertNull(creditCardPicker.selectedCreditCard)
gabrielluong marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
fun `GIVEN a selected credit card WHEN onAuthFailure is called THEN the prompt request is dismissed`() {
assertNull(creditCardPicker.selectedCreditCard)

setupSessionState(promptRequest)

creditCardPicker.onOptionSelect(creditCard)
creditCardPicker.onAuthFailure()

assertNull(creditCardPicker.selectedCreditCard)
assertTrue(onDismissCalled)
}

private fun setupSessionState(request: PromptRequest? = null): TabSessionState {
val promptRequest: PromptRequest = request ?: mock()
val content: ContentState = mock()
Expand Down
3 changes: 2 additions & 1 deletion docs/changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ permalink: /changelog/

* **feature-prompts**:
* Refactor `LoginPickerView` into a more generic view `SelectablePromptView` that can be reused by any prompts that displays a list of selectable options. [#10216](/~https://github.com/mozilla-mobile/android-components/issues/10216)
* Added optional `creditCardPickerView` and `onManageCreditCards` parameters to `PromptFeature` for a new `CreditCardPicker` to display a view for selecting credit cards to autofill into a site. [#9457](/~https://github.com/mozilla-mobile/android-components/issues/9457)
* Added optional `creditCardPickerView`, `onManageCreditCards` and `onSelectCreditCard` parameters to `PromptFeature` for a new `CreditCardPicker` to display a view for selecting credit cards to autofill into a site. [#9457](/~https://github.com/mozilla-mobile/android-components/issues/9457), [#10369](/~https://github.com/mozilla-mobile/android-components/pull/10369)
* Added handling of biometric authentication for a credit card selection prompt request. [#10369](/~https://github.com/mozilla-mobile/android-components/pull/10369)

* **concept-engine**
* 🌟️ `getBlockedSchemes()` now exposes the list of url shemes that the engine won't load.
Expand Down