diff --git a/.experimenter.yaml b/.experimenter.yaml index 6e83e8dfe..f3e838684 100644 --- a/.experimenter.yaml +++ b/.experimenter.yaml @@ -1,4 +1,12 @@ --- +client-deduplication: + description: A feature to control the sending of the client-deduplication ping. + hasExposure: true + exposureDescription: "" + variables: + enabled: + type: boolean + description: "If true, the ping will be sent." cookie-banners: description: Features for cookie banner handling. hasExposure: true diff --git a/app/metrics.yaml b/app/metrics.yaml index e8a6efdcf..dbf971afa 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -258,6 +258,7 @@ events: send_in_pings: - metrics - baseline + - client-deduplication bugs: - https://github.com/mozilla-mobile/fenix/issues/17089 data_reviews: @@ -1459,6 +1460,7 @@ metrics: send_in_pings: - metrics - baseline + - client-deduplication bugs: - https://github.com/mozilla-mobile/fenix/issues/1158 - https://github.com/mozilla-mobile/fenix/issues/6556 @@ -1754,6 +1756,7 @@ metrics: please see `has_open_tabs`. send_in_pings: - metrics + - client-deduplication bugs: - https://github.com/mozilla-mobile/fenix/issues/11479 data_reviews: @@ -2551,6 +2554,7 @@ search.default_engine: send_in_pings: - metrics - baseline + - client-deduplication bugs: - https://github.com/mozilla-mobile/fenix/issues/800 data_reviews: @@ -2580,6 +2584,7 @@ search.default_engine: send_in_pings: - metrics - baseline + - client-deduplication bugs: - https://github.com/mozilla-mobile/fenix/issues/800 data_reviews: @@ -2983,6 +2988,7 @@ activation: This will never be sent in a ping that also contains the client_id. send_in_pings: - activation + - client-deduplication bugs: - https://bugzilla.mozilla.org/1538011 - https://bugzilla.mozilla.org/1501822 @@ -3165,6 +3171,8 @@ sync_auth: notification_emails: - android-probes@mozilla.com - kbrosnan@mozilla.com + send_in_pings: + - client-deduplication expires: never metadata: tags: @@ -6425,6 +6433,7 @@ browser.search: send_in_pings: - metrics - baseline + - client-deduplication bugs: - https://github.com/mozilla-mobile/fenix/issues/6558 - https://github.com/mozilla-mobile/fenix/issues/28010 @@ -6459,6 +6468,7 @@ browser.search: send_in_pings: - metrics - baseline + - client-deduplication bugs: - https://github.com/mozilla-mobile/fenix/issues/6558 - https://github.com/mozilla-mobile/fenix/issues/28010 @@ -6484,6 +6494,7 @@ browser.search: send_in_pings: - metrics - baseline + - client-deduplication bugs: - https://github.com/mozilla-mobile/fenix/issues/6557 data_reviews: @@ -9001,6 +9012,39 @@ review_prompt: data_sensitivity: - interaction expires: 121 +client_deduplication: + valid_advertising_id: + type: boolean + description: | + Whether or not we get a valid advertising ID from the device. + send_in_pings: + - client-deduplication + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1817029 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1813195#c11 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + - fbertsch@mozilla.com + expires: 122 + experiment_timeframe: + type: string + description: | + A string we use to identify which run of the experiment this is. + send_in_pings: + - client-deduplication + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1817029 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1813195#c11 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + - fbertsch@mozilla.com + expires: 122 private_browsing_shortcut_cfr: add_shortcut: diff --git a/app/pings.yaml b/app/pings.yaml index 981168f5b..0f7ce1d85 100644 --- a/app/pings.yaml +++ b/app/pings.yaml @@ -62,3 +62,21 @@ spoc: - https://github.com/mozilla-mobile/fenix/pull/27550#issuecomment-1295027631 notification_emails: - android-probes@mozilla.com + +client-deduplication: + description: | + Contains data to help identify if client IDs are being regenerated + erroneously. + include_client_id: true + reasons: + active: | + The ping is being sent when the app is coming to the foreground. + inactive: | + The ping is being sent when the app is going to the background. + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1817029 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1813195#c11 + notification_emails: + - android-probes@mozilla.com + - fbertsch@mozilla.com diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 7231b1867..c5c40704b 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -75,6 +75,7 @@ import org.mozilla.fenix.components.Core import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.metrics.MetricServiceType import org.mozilla.fenix.components.metrics.MozillaProductDetector +import org.mozilla.fenix.components.metrics.clientdeduplication.ClientDeduplicationLifecycleObserver import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.experiments.maybeFetchExperiments import org.mozilla.fenix.ext.areNotificationsEnabledSafe @@ -186,6 +187,12 @@ open class FenixApplication : LocaleAwareApplication(), Provider { GlobalScope.launch(Dispatchers.IO) { setStartupMetrics(store, settings()) } + + ProcessLifecycleOwner.get().lifecycle.addObserver( + ClientDeduplicationLifecycleObserver( + this.applicationContext, + ), + ) } @CallSuper diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt index 84a5e1426..6a8e30a07 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/ActivationPing.kt @@ -16,13 +16,6 @@ import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.components.metrics.MetricsUtils.getHashedIdentifier class ActivationPing(private val context: Context) { - companion object { - // The number of iterations to compute the hash. RFC 2898 suggests - // a minimum of 1000 iterations. - const val PBKDF2_ITERATIONS = 1000 - const val PBKDF2_KEY_LEN_BITS = 256 - } - private val prefs: SharedPreferences by lazy { context.getSharedPreferences( "${this.javaClass.canonicalName}.prefs", diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt index 342c047f6..226a3f603 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt @@ -23,6 +23,10 @@ import javax.crypto.SecretKeyFactory import javax.crypto.spec.PBEKeySpec object MetricsUtils { + // The number of iterations to compute the hash. RFC 2898 suggests + // a minimum of 1000 iterations. + private const val PBKDF2_ITERATIONS = 1000 + private const val PBKDF2_KEY_LEN_BITS = 256 /** * Possible sources for a performed search. @@ -57,7 +61,7 @@ object MetricsUtils { } /** - * Get the salt to use for hashing. This is a convenience + * Get the default salt to use for hashing. This is a convenience * function to help with unit tests. */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) @@ -77,20 +81,20 @@ object MetricsUtils { return try { AdvertisingIdClient.getAdvertisingIdInfo(context).id } catch (e: GooglePlayServicesNotAvailableException) { - Logger.debug("ActivationPing - Google Play not installed on the device") + Logger.debug("getAdvertisingID() - Google Play not installed on the device") null } catch (e: GooglePlayServicesRepairableException) { - Logger.debug("ActivationPing - recoverable error connecting to Google Play Services") + Logger.debug("getAdvertisingID() - recoverable error connecting to Google Play Services") null } catch (e: IllegalStateException) { // This is unlikely to happen, as this should be running off the main thread. - Logger.debug("ActivationPing - AdvertisingIdClient must be called off the main thread") + Logger.debug("getAdvertisingID() - AdvertisingIdClient must be called off the main thread") null } catch (e: IOException) { - Logger.debug("ActivationPing - unable to connect to Google Play Services") + Logger.debug("getAdvertisingID() - unable to connect to Google Play Services") null } catch (e: NullPointerException) { - Logger.debug("ActivationPing - no Google Advertising ID available") + Logger.debug("getAdvertisingID() - no Google Advertising ID available") null } } @@ -100,46 +104,49 @@ object MetricsUtils { * We want users using more than one of our products to report a different * ID in each of them. This function runs off the main thread and is CPU-bound. * + * The `customSalt` parameter allows for dynamic setting of the salt for + * certain experiments or any one-off applications. + * * @return an hashed and salted Google Advertising ID or null if it was not possible * to get the Google Advertising ID. */ - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - suspend fun getHashedIdentifier(context: Context): String? = withContext(Dispatchers.Default) { - getAdvertisingID(context)?.let { unhashedID -> - // Add some salt to the ID, before hashing. For this specific use-case, it's ok - // to use the same salt value for all the hashes. We want hashes to be stable - // within a single product, but we don't want hashes to be the same across different - // products (e.g. Fennec vs Fenix). - val salt = getHashingSalt() + suspend fun getHashedIdentifier(context: Context, customSalt: String? = null): String? = + withContext(Dispatchers.Default) { + getAdvertisingID(context)?.let { unhashedID -> + // Add some salt to the ID, before hashing. We have a default salt that is used for + // all the hashes unless you specifically provide something different. This is done + // to stabalize all hashing within a single product. The customSalt allows for tweaking + // in case there are specific use-cases that require something custom. + val salt = customSalt ?: getHashingSalt() - // Apply hashing. - try { - // Note that we intentionally want to use slow hashing functions here in order - // to increase the cost of potentially repeatedly guess the original unhashed - // identifier. - val keySpec = PBEKeySpec( - unhashedID.toCharArray(), - salt.toByteArray(), - ActivationPing.PBKDF2_ITERATIONS, - ActivationPing.PBKDF2_KEY_LEN_BITS, - ) + // Apply hashing. + try { + // Note that we intentionally want to use slow hashing functions here in order + // to increase the cost of potentially repeatedly guess the original unhashed + // identifier. + val keySpec = PBEKeySpec( + unhashedID.toCharArray(), + salt.toByteArray(), + PBKDF2_ITERATIONS, + PBKDF2_KEY_LEN_BITS, + ) - val keyFactory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1") - val hashedBytes = keyFactory.generateSecret(keySpec).encoded - Base64.encodeToString(hashedBytes, Base64.NO_WRAP) - } catch (e: java.lang.NullPointerException) { - Logger.error("ActivationPing - missing or wrong salt parameter") - null - } catch (e: IllegalArgumentException) { - Logger.error("ActivationPing - wrong parameter", e) - null - } catch (e: NoSuchAlgorithmException) { - Logger.error("ActivationPing - algorithm not available") - null - } catch (e: InvalidKeySpecException) { - Logger.error("ActivationPing - invalid key spec") - null + val keyFactory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1") + val hashedBytes = keyFactory.generateSecret(keySpec).encoded + Base64.encodeToString(hashedBytes, Base64.NO_WRAP) + } catch (e: java.lang.NullPointerException) { + Logger.error("getHashedIdentifier() - missing or wrong salt parameter") + null + } catch (e: IllegalArgumentException) { + Logger.error("getHashedIdentifier() - wrong parameter", e) + null + } catch (e: NoSuchAlgorithmException) { + Logger.error("getHashedIdentifier() - algorithm not available") + null + } catch (e: InvalidKeySpecException) { + Logger.error("getHashedIdentifier() - invalid key spec") + null + } } } - } } diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationLifecycleObserver.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationLifecycleObserver.kt new file mode 100644 index 000000000..514a40083 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationLifecycleObserver.kt @@ -0,0 +1,43 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.components.metrics.clientdeduplication + +import android.content.Context +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleEventObserver +import androidx.lifecycle.LifecycleOwner +import org.mozilla.fenix.nimbus.FxNimbus + +/** + * This observer allows us to mimic roughly the same schedule as the Glean SDK baseline ping. + * https://github.com/mozilla/glean/blob/main/glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/GleanLifecycleObserver.kt + */ +class ClientDeduplicationLifecycleObserver(context: Context) : LifecycleEventObserver { + private val clientDeduplicationPing = ClientDeduplicationPing(context) + + override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) { + // The ping will only be sent whenever the Nimbus feature is enabled. + if (FxNimbus.features.clientDeduplication.value().enabled) { + when (event) { + Lifecycle.Event.ON_STOP -> { + clientDeduplicationPing.triggerPing(active = false) + } + Lifecycle.Event.ON_START -> { + // We use ON_START here because we don't want to incorrectly count metrics in + // ON_RESUME as pause/resume can happen when interacting with things like the + // navigation shade which could lead to incorrectly recording the start of a + // duration, etc. + // + // https://developer.android.com/reference/android/app/Activity.html#onStart() + + clientDeduplicationPing.triggerPing(active = true) + } + else -> { + // For other lifecycle events, do nothing + } + } + } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationPing.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationPing.kt new file mode 100644 index 000000000..10f2eeaf3 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationPing.kt @@ -0,0 +1,50 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.components.metrics.clientdeduplication + +import android.content.Context +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import org.mozilla.fenix.GleanMetrics.Activation +import org.mozilla.fenix.GleanMetrics.ClientDeduplication +import org.mozilla.fenix.GleanMetrics.Pings +import org.mozilla.fenix.components.metrics.MetricsUtils.getHashedIdentifier + +/** + * Class to help construct and send the `clientDeduplication` ping. + */ +class ClientDeduplicationPing(private val context: Context) { + private val customHashingSalt = "bug-1813195-02-2023" + + /** + * Fills the metrics and triggers the 'clientDeduplication' ping. + */ + internal fun triggerPing(active: Boolean) { + CoroutineScope(Dispatchers.IO).launch { + val hashedId = getHashedIdentifier(context, customHashingSalt) + + // Record the metrics. + if (hashedId != null) { + // We have a valid, hashed Google Advertising ID. + Activation.identifier.set(hashedId) + ClientDeduplication.validAdvertisingId.set(true) + } else { + ClientDeduplication.validAdvertisingId.set(false) + } + ClientDeduplication.experimentTimeframe.set(customHashingSalt) + + // Set the reason based on if the app is foregrounded or backgrounded. + val reason = if (active) { + Pings.clientDeduplicationReasonCodes.active + } else { + Pings.clientDeduplicationReasonCodes.inactive + } + + // Submit the ping. + Pings.clientDeduplication.submit(reason) + } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationPingTest.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationPingTest.kt new file mode 100644 index 000000000..f37d0d722 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/components/metrics/clientdeduplication/ClientDeduplicationPingTest.kt @@ -0,0 +1,41 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.components.metrics.clientdeduplication + +import mozilla.components.service.glean.testing.GleanTestRule +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.GleanMetrics.ClientDeduplication +import org.mozilla.fenix.GleanMetrics.Pings +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner + +// For gleanTestRule +@RunWith(FenixRobolectricTestRunner::class) +internal class ClientDeduplicationPingTest { + @get:Rule + val gleanTestRule = GleanTestRule(testContext) + + @Test + fun `The clientDeduplication ping is sent`() { + // Record test data. + ClientDeduplication.validAdvertisingId.set(true) + + // Instruct the ping API to validate the ping data. + var validatorRun = false + Pings.clientDeduplication.testBeforeNextSubmit { reason -> + assertEquals(Pings.clientDeduplicationReasonCodes.active, reason) + assertEquals(true, ClientDeduplication.validAdvertisingId.testGetValue()) + validatorRun = true + } + Pings.clientDeduplication.submit(Pings.clientDeduplicationReasonCodes.active) + + // Verify that the validator ran. + assertTrue(validatorRun) + } +} diff --git a/nimbus.fml.yaml b/nimbus.fml.yaml index e746a6308..f726c5b18 100644 --- a/nimbus.fml.yaml +++ b/nimbus.fml.yaml @@ -138,6 +138,21 @@ features: - channel: nightly value: enabled: false + + client-deduplication: + description: A feature to control the sending of the client-deduplication ping. + variables: + enabled: + description: If true, the ping will be sent. + type: Boolean + default: false + defaults: + - channel: nightly + value: + enabled: false + - channel: developer + value: + enabled: false growth-data: description: A feature measuring campaign growth data