From 2a5709a531265992eaf27448c5ab84d117fe14c7 Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Thu, 22 Apr 2021 12:37:58 -0400 Subject: [PATCH] Issue #10140 - Part 3: Implement the new Autocomplete.StorageDelegate interface in GeckoStorageDelegateWrapper - Uses the new GV `Autocomplete.StorageDelegate` interface in GeckoStorageDelegateWrapper - Implements `onCreditCardFetch` --- .../autofill/GeckoStorageDelegateWrapper.kt | 48 +++++++++++---- components/concept/storage/build.gradle | 2 + .../storage/CreditCardsAddressesStorage.kt | 58 +++++++++++++++++++ .../AutofillCreditCardsAddressesStorage.kt | 9 +++ .../service/sync/autofill/AutofillCrypto.kt | 15 ++--- ...eckoCreditCardsAddressesStorageDelegate.kt | 7 +++ ...CreditCardsAddressesStorageDelegateTest.kt | 30 +++++++++- docs/changelog.md | 7 ++- 8 files changed, 152 insertions(+), 24 deletions(-) diff --git a/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/autofill/GeckoStorageDelegateWrapper.kt b/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/autofill/GeckoStorageDelegateWrapper.kt index a32c40d75b9..e58d7dbf707 100644 --- a/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/autofill/GeckoStorageDelegateWrapper.kt +++ b/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/autofill/GeckoStorageDelegateWrapper.kt @@ -9,30 +9,58 @@ import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch import mozilla.components.browser.engine.gecko.ext.toLogin import mozilla.components.browser.engine.gecko.ext.toLoginEntry -import mozilla.components.concept.storage.Login +import mozilla.components.concept.storage.CreditCardsAddressesStorageDelegate import mozilla.components.concept.storage.LoginStorageDelegate import org.mozilla.geckoview.Autocomplete import org.mozilla.geckoview.GeckoResult /** - * This class exists only to convert incoming [LoginEntry] arguments into [Login]s, then forward - * them to [storageDelegate]. This allows us to avoid duplicating [LoginStorageDelegate] code - * between different versions of GeckoView, by duplicating this wrapper instead. + * Gecko credit card and login storage delegate that handles runtime storage requests. This allows + * the Gecko runtime to call the underlying storage to handle requests for fetching, saving and + * updating of autocomplete items in the storage. */ -@Suppress("Deprecation") -// This will be addressed in https://github.com/mozilla-mobile/android-components/issues/10093 -class GeckoLoginDelegateWrapper(private val storageDelegate: LoginStorageDelegate) : - Autocomplete.LoginStorageDelegate { +class GeckoStorageDelegateWrapper( + private val creditCardsAddressesStorageDelegate: CreditCardsAddressesStorageDelegate, + private val loginStorageDelegate: LoginStorageDelegate +) : Autocomplete.StorageDelegate { + + override fun onCreditCardFetch(): GeckoResult>? { + val result = GeckoResult>() + + GlobalScope.launch(IO) { + val creditCards = creditCardsAddressesStorageDelegate.onCreditCardsFetch().await() + .mapNotNull { + val plaintextCardNumber = + creditCardsAddressesStorageDelegate.decrypt(it.encryptedCardNumber)?.number + + if (plaintextCardNumber == null) { + null + } else { + Autocomplete.CreditCard.Builder() + .guid(it.guid) + .name(it.billingName) + .number(plaintextCardNumber) + .expirationMonth(it.expiryMonth.toString()) + .expirationYear(it.expiryYear.toString()) + .build() + } + } + .toTypedArray() + result.complete(creditCards) + } + + return result + } override fun onLoginSave(login: Autocomplete.LoginEntry) { - storageDelegate.onLoginSave(login.toLogin()) + loginStorageDelegate.onLoginSave(login.toLogin()) } override fun onLoginFetch(domain: String): GeckoResult>? { val result = GeckoResult>() GlobalScope.launch(IO) { - val storedLogins = storageDelegate.onLoginFetch(domain) + val storedLogins = loginStorageDelegate.onLoginFetch(domain) val logins = storedLogins.await() .map { it.toLoginEntry() } diff --git a/components/concept/storage/build.gradle b/components/concept/storage/build.gradle index fb3dc879862..04f6c4e44d2 100644 --- a/components/concept/storage/build.gradle +++ b/components/concept/storage/build.gradle @@ -27,6 +27,8 @@ dependencies { // dependency, but it will crash at runtime. // Included via 'api' because this module is unusable without coroutines. api Dependencies.kotlin_coroutines + + api project(':lib-dataprotect') } apply from: '../../../publish.gradle' diff --git a/components/concept/storage/src/main/java/mozilla/components/concept/storage/CreditCardsAddressesStorage.kt b/components/concept/storage/src/main/java/mozilla/components/concept/storage/CreditCardsAddressesStorage.kt index 43d0b95d86a..b5e45c0f3d6 100644 --- a/components/concept/storage/src/main/java/mozilla/components/concept/storage/CreditCardsAddressesStorage.kt +++ b/components/concept/storage/src/main/java/mozilla/components/concept/storage/CreditCardsAddressesStorage.kt @@ -7,6 +7,8 @@ package mozilla.components.concept.storage import android.os.Parcelable import kotlinx.android.parcel.Parcelize import kotlinx.coroutines.Deferred +import mozilla.components.lib.dataprotect.KeyProvider +import mozilla.components.lib.dataprotect.ManagedKey /** * An interface which defines read/write methods for credit card and address data. @@ -104,6 +106,53 @@ interface CreditCardsAddressesStorage { * @param guid Unique identifier for the desired address. */ suspend fun touchAddress(guid: String) + + /** + * Returns an instance of [CreditCardCrypto] that knows how to encrypt and decrypt credit card + * numbers. + * + * @return [CreditCardCrypto] instance. + */ + fun getCreditCardCrypto(): CreditCardCrypto + + /** + * Returns an instance of [KeyProvider] that knows how to provide a [ManagedKey]. + * + * @return [KeyProvider] instance. + */ + fun getKeyProvider(): KeyProvider +} + +/** + * An interface that defines methods for encrypting and decrypting a credit card number. + */ +interface CreditCardCrypto { + + /** + * Encrypt a [CreditCardNumber.Plaintext] using the provided key. A `null` result means a + * bad key was provided. In that case caller should obtain a new key and try again. + * + * @param key The encryption key to encrypt the plaintext credit card number. + * @param plaintextCardNumber A plaintext credit card number to be encrypted. + * @return An encrypted credit card number or `null` if a bad [key] was provided. + */ + fun encrypt( + key: ManagedKey, + plaintextCardNumber: CreditCardNumber.Plaintext + ): CreditCardNumber.Encrypted? + + /** + * Decrypt a [CreditCardNumber.Encrypted] using the provided key. A `null` result means a + * bad key was provided. In that case caller should obtain a new key and try again. + * + * @param key The encryption key to decrypt the decrypt credit card number. + * @param encryptedCardNumber An encrypted credit card number to be decrypted. + * @return A plaintext, non-encrypted credit card number or `null` if a bad [key] was provided. + */ + fun decrypt( + key: ManagedKey, + encryptedCardNumber: CreditCardNumber.Encrypted + ): CreditCardNumber.Plaintext? } /** @@ -273,6 +322,15 @@ data class UpdatableAddressFields( */ interface CreditCardsAddressesStorageDelegate { + /** + * Decrypt a [CreditCardNumber.Encrypted] into its plaintext equivalent or `null` if + * it fails to decrypt. + * + * @param encryptedCardNumber An encrypted credit card number to be decrypted. + * @return A plaintext, non-encrypted credit card number. + */ + fun decrypt(encryptedCardNumber: CreditCardNumber.Encrypted): CreditCardNumber.Plaintext? + /** * Returns all stored addresses. This is called when the engine believes an address field * should be autofilled. diff --git a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorage.kt b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorage.kt index 9c767d25eb4..411ec06c401 100644 --- a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorage.kt +++ b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorage.kt @@ -18,6 +18,7 @@ import mozilla.components.concept.storage.UpdatableAddressFields import mozilla.components.concept.storage.UpdatableCreditCardFields import mozilla.components.concept.sync.SyncableStore import mozilla.components.lib.dataprotect.KeyGenerationReason +import mozilla.components.lib.dataprotect.KeyProvider import mozilla.components.lib.dataprotect.KeyRecoveryHandler import mozilla.components.lib.dataprotect.SecureAbove22Preferences import mozilla.components.support.base.log.logger.Logger @@ -157,6 +158,14 @@ class AutofillCreditCardsAddressesStorage( conn.getStorage().touchAddress(guid) } + override fun getCreditCardCrypto(): AutofillCrypto { + return crypto + } + + override fun getKeyProvider(): KeyProvider { + return crypto + } + override fun registerWithSyncManager() { conn.getStorage().registerWithSyncManager() } diff --git a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCrypto.kt b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCrypto.kt index 9a6085138bc..905bb655646 100644 --- a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCrypto.kt +++ b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCrypto.kt @@ -10,6 +10,7 @@ import mozilla.appservices.autofill.ErrorException import mozilla.appservices.autofill.createKey import mozilla.appservices.autofill.decryptString import mozilla.appservices.autofill.encryptString +import mozilla.components.concept.storage.CreditCardCrypto import mozilla.components.concept.storage.CreditCardNumber import mozilla.components.lib.dataprotect.KeyGenerationReason import mozilla.components.lib.dataprotect.KeyProvider @@ -33,15 +34,11 @@ class AutofillCrypto( private val context: Context, private val securePrefs: SecureAbove22Preferences, private val keyRecoveryHandler: KeyRecoveryHandler -) : KeyProvider { +) : CreditCardCrypto, KeyProvider { private val logger = Logger("AutofillCrypto") private val plaintextPrefs by lazy { context.getSharedPreferences(AUTOFILL_PREFS, Context.MODE_PRIVATE) } - /** - * Encrypt a [CreditCardNumber.Plaintext] using provided key. - * A `null` result means a bad key was provided. In that case caller should obtain a new key and try again. - */ - fun encrypt(key: ManagedKey, plaintextCardNumber: CreditCardNumber.Plaintext): CreditCardNumber.Encrypted? { + override fun encrypt(key: ManagedKey, plaintextCardNumber: CreditCardNumber.Plaintext): CreditCardNumber.Encrypted? { return try { CreditCardNumber.Encrypted(encrypt(key, plaintextCardNumber.number)) } catch (e: ErrorException.JsonError) { @@ -53,11 +50,7 @@ class AutofillCrypto( } } - /** - * Decrypt a [CreditCardNumber.Encrypted] using provided key. - * A `null` result means a bad key was provided. In that case caller should obtain a new key and try again. - */ - fun decrypt(key: ManagedKey, encryptedCardNumber: CreditCardNumber.Encrypted): CreditCardNumber.Plaintext? { + override fun decrypt(key: ManagedKey, encryptedCardNumber: CreditCardNumber.Encrypted): CreditCardNumber.Plaintext? { return try { CreditCardNumber.Plaintext(decrypt(key, encryptedCardNumber.number)) } catch (e: ErrorException.JsonError) { 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 ec4db0da93b..6b9bfdaa819 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 @@ -10,6 +10,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.async import mozilla.components.concept.storage.Address import mozilla.components.concept.storage.CreditCard +import mozilla.components.concept.storage.CreditCardNumber import mozilla.components.concept.storage.CreditCardsAddressesStorage import mozilla.components.concept.storage.CreditCardsAddressesStorageDelegate @@ -21,6 +22,12 @@ class GeckoCreditCardsAddressesStorageDelegate( private val scope: CoroutineScope = CoroutineScope(Dispatchers.IO) ) : CreditCardsAddressesStorageDelegate { + override fun decrypt(encryptedCardNumber: CreditCardNumber.Encrypted): CreditCardNumber.Plaintext? { + val crypto = storage.value.getCreditCardCrypto() + val key = storage.value.getKeyProvider().key() + return crypto.decrypt(key, encryptedCardNumber) + } + override fun onAddressesFetch(): Deferred> { return scope.async { storage.value.getAllAddresses() 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 9ab0e0279d4..5f722ee0c82 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,8 +9,11 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.TestCoroutineScope -import mozilla.components.support.test.mock +import mozilla.components.concept.storage.CreditCardNumber +import mozilla.components.concept.storage.NewCreditCardFields +import mozilla.components.lib.dataprotect.SecureAbove22Preferences 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 @@ -21,7 +24,8 @@ import org.mockito.Mockito.verify @RunWith(AndroidJUnit4::class) class GeckoCreditCardsAddressesStorageDelegateTest { - private val storage = AutofillCreditCardsAddressesStorage(testContext, mock()) + private lateinit var storage: AutofillCreditCardsAddressesStorage + private lateinit var securePrefs: SecureAbove22Preferences private lateinit var delegate: GeckoCreditCardsAddressesStorageDelegate private lateinit var scope: TestCoroutineScope @@ -32,9 +36,31 @@ class GeckoCreditCardsAddressesStorageDelegateTest { @Before fun before() = runBlocking { scope = TestCoroutineScope() + // forceInsecure is set in the tests because a keystore wouldn't be configured in the test environment. + securePrefs = SecureAbove22Preferences(testContext, "autofill", forceInsecure = true) + storage = AutofillCreditCardsAddressesStorage(testContext, lazy { securePrefs }) delegate = GeckoCreditCardsAddressesStorageDelegate(lazy { storage }, scope) } + @Test + fun `decrypt`() = runBlocking { + val plaintextNumber = CreditCardNumber.Plaintext("4111111111111111") + val creditCardFields = NewCreditCardFields( + billingName = "Jon Doe", + plaintextCardNumber = plaintextNumber, + cardNumberLast4 = "1111", + expiryMonth = 12, + expiryYear = 2028, + cardType = "amex" + ) + val creditCard = storage.addCreditCard(creditCardFields) + + assertEquals( + plaintextNumber, + delegate.decrypt(creditCard.encryptedCardNumber) + ) + } + @Test fun `onAddressFetch`() { scope.launch { diff --git a/docs/changelog.md b/docs/changelog.md index eace07b72d5..16d373f3384 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -12,9 +12,12 @@ 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) +* **browser-engine-gecko**: + * Implements the new GeckoView `Autocomplete.StorageDelegate` interface in `GeckoStorageDelegateWrapper`. This will replace the deprecated `GeckoLoginDelegateWrapper` and provide additional autocomplete support for credit cards. [#10140](https://github.com/mozilla-mobile/android-components/issues/10140) + * **feature-downloads**: * ⚠️ **This is a breaking change**: `AbstractFetchDownloadService.openFile()` changed its signature from `AbstractFetchDownloadService.openFile(context: Context, filePath: String, contentType: String?)` to `AbstractFetchDownloadService.openFile(applicationContext: Context, download: DownloadState)`. - * 🚒 Bug fixed [issue #](https://github.com/mozilla-mobile/android-components/issues/10138) - The downloaded files cannot be seen. + * 🚒 Bug fixed [issue #10138](https://github.com/mozilla-mobile/android-components/issues/10138) - The downloaded files cannot be seen. * **browser-engine-gecko(-nightly/beta)** * ⚠️ From now on there will be only one `concept-engine` implementation using [GeckoView](https://mozilla.github.io/geckoview/). On `master` this will be the Nightly version. In release versions it will be the corresponding Beta or Release version. More about this in [RFC 7](https://mozac.org/rfc/0007-synchronized-releases). @@ -27,6 +30,8 @@ permalink: /changelog/ * ⚠️ **This is a breaking change**: `CreditCard`'s number field changed to `encryptedCardNumber`, `cardNumberLast4` added. * New `CreditCardNumber` class, which encapsulate either an encrypted or plaintext versions of credit cards. * `AutofillCreditCardsAddressesStorage` reflects these breaking changes. + * Introduced a new `CreditCardCrypto` interface for for encrypting and decrypting a credit card number. [#10140](https://github.com/mozilla-mobile/android-components/issues/10140) + * Added new methods in `AutofillCreditCardsAddressesStorage` for retrieving the `KeyProvider` and `CreditCardCrypto`. [#10140](https://github.com/mozilla-mobile/android-components/issues/10140) * **service-firefox-accounts** * 🌟️ When configuring syncable storage layers, `SyncManager` now takes an optional `KeyProvider` to handle encryption/decryption of protected values.