From c2fb99a73fb896725d0f9ff81efd68da3dd65058 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Thu, 22 Aug 2019 16:56:13 -0700 Subject: [PATCH] FxA WebChannels integration This patch includes: - WebChannels support enabled by default, with ability to disable it via remote flag - expanded FxA telemetry (closes #4971) Co-authored-by: Arturo Mejia --- app/metrics.yaml | 66 +++++++++++++--- .../mozilla/fenix/AppRequestInterceptor.kt | 9 ++- .../java/org/mozilla/fenix/Experiments.kt | 2 + .../mozilla/fenix/IntentReceiverActivity.kt | 1 + .../fenix/browser/BaseBrowserFragment.kt | 18 +++++ .../mozilla/fenix/browser/BrowserFragment.kt | 1 - .../fenix/components/BackgroundServices.kt | 75 +++++++++++++------ .../mozilla/fenix/components/Components.kt | 2 +- .../org/mozilla/fenix/components/Services.kt | 3 +- .../components/metrics/GleanMetricsService.kt | 18 ++++- .../fenix/components/metrics/Metrics.kt | 6 +- app/src/main/res/values/preference_keys.xml | 1 - .../fenix/components/TestComponents.kt | 2 +- docs/metrics.md | 6 +- 14 files changed, 166 insertions(+), 44 deletions(-) diff --git a/app/metrics.yaml b/app/metrics.yaml index 512fc410c..d628f9860 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -816,6 +816,61 @@ sync_auth: notification_emails: - fenix-core@mozilla.com expires: "2020-03-01" + sign_up: + type: event + description: > + User registered a new Firefox Account, and was signed into it + bugs: + - 4971 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/4931#issuecomment-529740300 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-03-01" + paired: + type: event + description: > + User signed into FxA by pairing with a different Firefox browser, using a QR code + bugs: + - 4971 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/4931#issuecomment-529740300 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-03-01" + auto_login: + type: event + description: > + User signed into FxA via an account shared from another locally installed Mozilla application (e.g. Fennec) + bugs: + - 4971 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/4931#issuecomment-529740300 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-03-01" + recovered: + type: event + description: > + Account manager automatically recovered FxA authentication state without direct user involvement + bugs: + - 4971 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/4931#issuecomment-529740300 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-03-01" + other_external: + type: event + description: > + User authenticated via FxA using an unknown mechanism. "Known" mechanisms are currently sign-in, sign-up and pairing + bugs: + - 4971 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/4931#issuecomment-529740300 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-03-01" scan_pairing: type: event description: > @@ -827,17 +882,6 @@ sync_auth: notification_emails: - fenix-core@mozilla.com expires: "2020-03-01" - create_account: - type: event - description: > - A user pressed the create account button on the sync authentication page - bugs: - - 1190 - data_reviews: - - https://github.com/mozilla-mobile/fenix/pull/2745#issuecomment-494918532 - notification_emails: - - fenix-core@mozilla.com - expires: "2020-03-01" sync_account: opened: diff --git a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt index d32ebe738..c548cab01 100644 --- a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt +++ b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt @@ -25,8 +25,13 @@ class AppRequestInterceptor(private val context: Context) : RequestInterceptor { } adjustTrackingProtection(host, context, session) - // Accounts uses interception to check for a "success URL" in the sign-in flow to finalize authentication. - return context.components.services.accountsAuthFeature.interceptor.onLoadRequest(session, uri) + + // WebChannel-driven authentication does not require a separate redirect interceptor. + return if (context.isInExperiment(Experiments.asFeatureWebChannelsDisabled)) { + context.components.services.accountsAuthFeature.interceptor.onLoadRequest(session, uri) + } else { + null + } } private fun adjustTrackingProtection(host: String, context: Context, session: EngineSession) { diff --git a/app/src/main/java/org/mozilla/fenix/Experiments.kt b/app/src/main/java/org/mozilla/fenix/Experiments.kt index e664ec831..f1d6a1ca0 100644 --- a/app/src/main/java/org/mozilla/fenix/Experiments.kt +++ b/app/src/main/java/org/mozilla/fenix/Experiments.kt @@ -19,6 +19,8 @@ object Experiments { val asFeatureSyncDisabled = ExperimentDescriptor("asFeatureSyncDisabled") // application services flag to disable Firefox Accounts pairing button. val asFeatureFxAPairingDisabled = ExperimentDescriptor("asFeatureFxAPairingDisabled") + // application services flag to disable Firefox Accounts WebChannel integration. + val asFeatureWebChannelsDisabled = ExperimentDescriptor("asFeatureWebChannelsDisabled") } val Context.app: FenixApplication diff --git a/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt b/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt index 89243b1b9..cd88cf842 100644 --- a/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt @@ -68,6 +68,7 @@ class IntentReceiverActivity : Activity() { private fun setIntentActivity(intent: Intent) { val openToBrowser = when { components.utils.customTabIntentProcessor.matches(intent) -> { + // TODO this needs to change: https://github.com/mozilla-mobile/fenix/issues/5225 val activityClass = if (intent.hasExtra(EXTRA_AUTH_CUSTOM_TAB)) { AuthCustomTabActivity::class } else { diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 941862800..bef44d40b 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -31,6 +31,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager +import mozilla.components.feature.accounts.FxaWebChannelFeature import mozilla.components.feature.app.links.AppLinksFeature import mozilla.components.feature.contextmenu.ContextMenuFeature import mozilla.components.feature.downloads.DownloadsFeature @@ -49,6 +50,7 @@ import mozilla.components.support.base.feature.BackHandler import mozilla.components.support.base.feature.PermissionsFeature import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import mozilla.components.support.ktx.android.view.exitImmersiveModeIfNeeded +import org.mozilla.fenix.Experiments import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.IntentReceiverActivity @@ -70,6 +72,7 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.enterToImmersiveMode import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings +import org.mozilla.fenix.isInExperiment import org.mozilla.fenix.quickactionsheet.QuickActionSheetBehavior import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.theme.ThemeManager @@ -96,6 +99,7 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs private val sitePermissionsFeature = ViewBoundFeatureWrapper() private val fullScreenFeature = ViewBoundFeatureWrapper() private val swipeRefreshFeature = ViewBoundFeatureWrapper() + private val webchannelIntegration = ViewBoundFeatureWrapper() var customTabSessionId: String? = null @@ -374,6 +378,20 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs view.swipeRefresh.setOnChildScrollUpCallback { _, _ -> true } } + if (!requireContext().isInExperiment(Experiments.asFeatureWebChannelsDisabled)) { + webchannelIntegration.set( + feature = FxaWebChannelFeature( + requireContext(), + customTabSessionId, + requireComponents.core.engine, + requireComponents.core.sessionManager, + requireComponents.backgroundServices.accountManager + ), + owner = this, + view = view + ) + } + (activity as HomeActivity).updateThemeForSession(session) } } diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index e32c37b60..ec3fc2708 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -102,7 +102,6 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler { val sessionManager = context.components.core.sessionManager return super.initializeUI(view)?.also { - readerViewFeature.set( feature = ReaderViewFeature( context, diff --git a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt index aacabc5c4..bedfaaacf 100644 --- a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt +++ b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt @@ -5,9 +5,7 @@ package org.mozilla.fenix.components import android.content.Context -import android.content.SharedPreferences import android.os.Build -import android.preference.PreferenceManager import androidx.lifecycle.ProcessLifecycleOwner import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -55,7 +53,12 @@ class BackgroundServices( ) { companion object { const val CLIENT_ID = "a2270f727f45f648" - const val REDIRECT_URL = "https://accounts.firefox.com/oauth/success/$CLIENT_ID" + + fun redirectUrl(context: Context) = if (context.isInExperiment(Experiments.asFeatureWebChannelsDisabled)) { + "https://accounts.firefox.com/oauth/success/$CLIENT_ID" + } else { + "urn:ietf:wg:oauth:2.0:oob:oauth-redirect-webchannel" + } } fun defaultDeviceName(context: Context): String = context.getString( @@ -65,7 +68,7 @@ class BackgroundServices( Build.MODEL ) - private val serverConfig = ServerConfig.release(CLIENT_ID, REDIRECT_URL) + private val serverConfig = ServerConfig.release(CLIENT_ID, redirectUrl(context)) private val deviceConfig = DeviceConfig( name = defaultDeviceName(context), type = DeviceType.MOBILE, @@ -122,6 +125,46 @@ class BackgroundServices( } } + private val telemetryAccountObserver = object : AccountObserver { + override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { + when (authType) { + // User signed-in into an existing FxA account. + AuthType.Signin -> + context.components.analytics.metrics.track(Event.SyncAuthSignIn) + + // User created a new FxA account. + AuthType.Signup -> + context.components.analytics.metrics.track(Event.SyncAuthSignUp) + + // User paired to an existing account via QR code scanning. + AuthType.Pairing -> + context.components.analytics.metrics.track(Event.SyncAuthPaired) + + // User signed-in into an FxA account shared from another locally installed app + // (e.g. Fennec). + AuthType.Shared -> + context.components.analytics.metrics.track(Event.SyncAuthFromShared) + + // Account Manager recovered a broken FxA auth state, without direct user involvement. + AuthType.Recovered -> + context.components.analytics.metrics.track(Event.SyncAuthRecovered) + + // User signed-in into an FxA account via unknown means. + // Exact mechanism identified by the 'action' param. + is AuthType.OtherExternal -> + context.components.analytics.metrics.track(Event.SyncAuthOtherExternal) + } + // Used by Leanplum as a context variable. + context.settings.fxaSignedIn = true + } + + override fun onLoggedOut() { + context.components.analytics.metrics.track(Event.SyncAuthSignOut) + // Used by Leanplum as a context variable. + context.settings.fxaSignedIn = false + } + } + /** * When we login/logout of FxA, we need to update our push subscriptions to match the newly * logged in account. @@ -136,32 +179,18 @@ class BackgroundServices( * We should have this removed when we are more confident * of the send-tab/push feature: https://github.com/mozilla-mobile/fenix/issues/4063 */ - private val accountObserver = object : AccountObserver { + private val pushAccountObserver = object : AccountObserver { override fun onLoggedOut() { push.unsubscribeForType(PushType.Services) - - context.components.analytics.metrics.track(Event.SyncAuthSignOut) - - context.settings.fxaSignedIn = false } override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { if (authType != AuthType.Existing) { push.subscribeForType(PushType.Services) } - - context.components.analytics.metrics.track(Event.SyncAuthSignIn) - - context.settings.fxaSignedIn = true } } - private val preferences: SharedPreferences by lazy { - PreferenceManager.getDefaultSharedPreferences( - context - ) - } - val accountManager = FxaAccountManager( context, serverConfig, @@ -174,13 +203,17 @@ class BackgroundServices( // See https://github.com/mozilla-mobile/android-components/issues/3732 setOf("https://identity.mozilla.com/apps/oldsync") ).also { + // TODO this needs to change once we have a SyncManager context.settings.fxaHasSyncedItems = syncConfig?.supportedEngines?.isNotEmpty() ?: false it.registerForDeviceEvents(deviceEventObserver, ProcessLifecycleOwner.get(), false) + // Register a telemetry account observer to keep track of FxA auth metrics. + it.register(telemetryAccountObserver) + // Enable push if we have the config. if (pushConfig != null) { - // Register our account observer so we know how to update our push subscriptions. - it.register(accountObserver) + // Register the push account observer so we know how to update our push subscriptions. + it.register(pushAccountObserver) val logger = Logger("AutoPushFeature") diff --git a/app/src/main/java/org/mozilla/fenix/components/Components.kt b/app/src/main/java/org/mozilla/fenix/components/Components.kt index 585f4fc8d..65e29c0be 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -16,7 +16,7 @@ class Components(private val context: Context) { val backgroundServices by lazy { BackgroundServices(context, core.historyStorage, core.bookmarksStorage) } - val services by lazy { Services(backgroundServices.accountManager) } + val services by lazy { Services(context, backgroundServices.accountManager) } val core by lazy { Core(context) } val search by lazy { Search(context) } val useCases by lazy { diff --git a/app/src/main/java/org/mozilla/fenix/components/Services.kt b/app/src/main/java/org/mozilla/fenix/components/Services.kt index 8d066b1f6..163884104 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Services.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Services.kt @@ -25,12 +25,13 @@ import org.mozilla.fenix.test.Mockable */ @Mockable class Services( + private val context: Context, private val accountManager: FxaAccountManager ) { val accountsAuthFeature by lazy { FirefoxAccountsAuthFeature( accountManager, - redirectUrl = BackgroundServices.REDIRECT_URL + redirectUrl = BackgroundServices.redirectUrl(context) ) { context, authUrl -> CoroutineScope(Dispatchers.Main).launch { val intent = SupportUtils.createAuthCustomTabIntent(context, authUrl) diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index 8aa869c9d..744f16eb3 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -225,15 +225,27 @@ private val Event.wrapper: EventWrapper<*>? is Event.SyncAuthSignIn -> EventWrapper( { SyncAuth.signIn.record(it) } ) + is Event.SyncAuthSignUp -> EventWrapper( + { SyncAuth.signUp.record(it) } + ) + is Event.SyncAuthPaired -> EventWrapper( + { SyncAuth.paired.record(it) } + ) + is Event.SyncAuthOtherExternal -> EventWrapper( + { SyncAuth.otherExternal.record(it) } + ) + is Event.SyncAuthFromShared -> EventWrapper( + { SyncAuth.autoLogin.record(it) } + ) + is Event.SyncAuthRecovered -> EventWrapper( + { SyncAuth.recovered.record(it) } + ) is Event.SyncAuthSignOut -> EventWrapper( { SyncAuth.signOut.record(it) } ) is Event.SyncAuthScanPairing -> EventWrapper( { SyncAuth.scanPairing.record(it) } ) - is Event.SyncAuthCreateAccount -> EventWrapper( - { SyncAuth.createAccount.record(it) } - ) is Event.SyncAccountOpened -> EventWrapper( { SyncAccount.opened.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt index a7e2566e3..3c86d15c8 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt @@ -64,10 +64,14 @@ sealed class Event { object LibraryClosed : Event() object SyncAuthOpened : Event() object SyncAuthClosed : Event() + object SyncAuthSignUp : Event() object SyncAuthSignIn : Event() object SyncAuthSignOut : Event() object SyncAuthScanPairing : Event() - object SyncAuthCreateAccount : Event() + object SyncAuthPaired : Event() + object SyncAuthRecovered : Event() + object SyncAuthOtherExternal : Event() + object SyncAuthFromShared : Event() object SyncAccountOpened : Event() object SyncAccountClosed : Event() object SyncAccountSyncNow : Event() diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index 88731cd26..a335dd168 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -57,7 +57,6 @@ pref_key_cached_account pref_key_sync_pair pref_key_sync_sign_in - pref_key_sync_create_account pref_key_sync_problem project_id pref_key_fxa_signed_in diff --git a/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt b/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt index c4b1fbc68..c07ae1d8f 100644 --- a/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt +++ b/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt @@ -13,7 +13,7 @@ class TestComponents(private val context: Context) : Components(context) { override val backgroundServices by lazy { mockk(relaxed = true) } - override val services by lazy { Services(backgroundServices.accountManager) } + override val services by lazy { Services(context, backgroundServices.accountManager) } override val core by lazy { TestCore(context) } override val search by lazy { Search(context) } override val useCases by lazy { diff --git a/docs/metrics.md b/docs/metrics.md index 095b79241..b62cf1418 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -128,12 +128,16 @@ The following metrics are added to the ping: | sync_account.send_tab |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user sent the current tab to another FxA device |[1](https://github.com/mozilla-mobile/fenix/pull/5106)||2020-03-01 | | sync_account.sign_in_to_send_tab |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user pressed the "sign in to send tab" button inside the share tab menu |[1](https://github.com/mozilla-mobile/fenix/pull/5106)||2020-03-01 | | sync_account.sync_now |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user pressed the sync now button on the sync account page |[1](https://github.com/mozilla-mobile/fenix/pull/2745#issuecomment-494918532)||2020-03-01 | +| sync_auth.auto_login |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |User signed into FxA via an account shared from another locally installed Mozilla application (e.g. Fennec) |[1](https://github.com/mozilla-mobile/fenix/pull/4931#issuecomment-529740300)||2020-03-01 | | sync_auth.closed |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user closed the sync page |[1](https://github.com/mozilla-mobile/fenix/pull/2745#issuecomment-494918532)||2020-03-01 | -| sync_auth.create_account |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user pressed the create account button on the sync authentication page |[1](https://github.com/mozilla-mobile/fenix/pull/2745#issuecomment-494918532)||2020-03-01 | | sync_auth.opened |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user opened the sync authentication page |[1](https://github.com/mozilla-mobile/fenix/pull/2745#issuecomment-494918532)||2020-03-01 | +| sync_auth.other_external |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |User authenticated via FxA using an unknown mechanism. "Known" mechanisms are currently sign-in, sign-up and pairing |[1](https://github.com/mozilla-mobile/fenix/pull/4931#issuecomment-529740300)||2020-03-01 | +| sync_auth.paired |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |User signed into FxA by pairing with a different Firefox browser, using a QR code |[1](https://github.com/mozilla-mobile/fenix/pull/4931#issuecomment-529740300)||2020-03-01 | +| sync_auth.recovered |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |Account manager automatically recovered FxA authentication state without direct user involvement |[1](https://github.com/mozilla-mobile/fenix/pull/4931#issuecomment-529740300)||2020-03-01 | | sync_auth.scan_pairing |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user pressed the scan pairing button on the sync authentication page |[1](https://github.com/mozilla-mobile/fenix/pull/2745#issuecomment-494918532)||2020-03-01 | | sync_auth.sign_in |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user pressed the sign in button on the sync authentication page and was successfully signed in to FxA |[1](https://github.com/mozilla-mobile/fenix/pull/2745#issuecomment-494918532)||2020-03-01 | | sync_auth.sign_out |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |A user pressed the sign out button on the sync account page and was successfully signed out of FxA |[1](https://github.com/mozilla-mobile/fenix/pull/2745#issuecomment-494918532)||2020-03-01 | +| sync_auth.sign_up |[event](https://mozilla.github.io/glean/book/user/metrics/event.html) |User registered a new Firefox Account, and was signed into it |[1](https://github.com/mozilla-mobile/fenix/pull/4931#issuecomment-529740300)||2020-03-01 | ## metrics This is a built-in ping that is assembled out of the box by the Glean SDK.