From 94a11023f53e26cee7a9cdb0c73a919dc04b6f8e 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 | 55 ++++++++++++++++ .../mozilla/fenix/AppRequestInterceptor.kt | 9 ++- .../java/org/mozilla/fenix/Experiments.kt | 2 + .../mozilla/fenix/IntentReceiverActivity.kt | 1 + .../mozilla/fenix/browser/BrowserFragment.kt | 30 +++++++++ .../fenix/components/BackgroundServices.kt | 62 ++++++++++++------- .../mozilla/fenix/components/Components.kt | 2 +- .../org/mozilla/fenix/components/Services.kt | 3 +- .../components/metrics/GleanMetricsService.kt | 18 +++++- .../fenix/components/metrics/Metrics.kt | 6 +- .../fenix/customtabs/AuthCustomTabActivity.kt | 1 - .../customtabs/ExternalAppBrowserFragment.kt | 18 ++++++ .../fenix/components/TestComponents.kt | 2 +- 13 files changed, 178 insertions(+), 31 deletions(-) diff --git a/app/metrics.yaml b/app/metrics.yaml index a8290fb3c0b0..c6edd173b629 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -809,6 +809,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 signed into FxA via an unknown mechanism + 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: > diff --git a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt index d32ebe7383da..c548cab01d06 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 e664ec831fe2..f1d6a1ca0842 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 b2cb7ee63322..f42d6e31bbeb 100644 --- a/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt @@ -63,6 +63,7 @@ class IntentReceiverActivity : Activity() { private fun setIntentActivity(intent: Intent) { val openToBrowser = when { components.utils.customTabIntentProcessor.matches(intent) -> { + // TODO This is potentially unsafe. Any app on the system can set the auth extra. val activityClass = if (intent.hasExtra(EXTRA_AUTH_CUSTOM_TAB)) { AuthCustomTabActivity::class } else { 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 b55e8ca3e1ff..943866535675 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -26,11 +26,14 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.session.Session +import mozilla.components.feature.accounts.FxaWebChannelFeature import mozilla.components.feature.readerview.ReaderViewFeature +import mozilla.components.feature.session.ThumbnailsFeature import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.support.base.feature.BackHandler import mozilla.components.support.base.feature.ViewBoundFeatureWrapper +import org.mozilla.fenix.Experiments import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.browser.readermode.DefaultReaderModeController @@ -46,6 +49,7 @@ import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.home.sessioncontrol.SessionControlChange import org.mozilla.fenix.home.sessioncontrol.TabCollection +import org.mozilla.fenix.isInExperiment import org.mozilla.fenix.mvi.getManagedEmitter import org.mozilla.fenix.quickactionsheet.DefaultQuickActionSheetController import org.mozilla.fenix.quickactionsheet.QuickActionSheetSessionObserver @@ -62,6 +66,8 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler { private var quickActionSheetSessionObserver: QuickActionSheetSessionObserver? = null private val readerViewFeature = ViewBoundFeatureWrapper() + private val thumbnailsFeature = ViewBoundFeatureWrapper() + private val webchannelIntegration = ViewBoundFeatureWrapper() override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -92,6 +98,30 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler { return super.initializeUI(view)?.also { + thumbnailsFeature.set( + feature = ThumbnailsFeature( + requireContext(), + view.engineView, + requireComponents.core.sessionManager + ), + owner = this, + view = view + ) + + if (!requireContext().isInExperiment(Experiments.asFeatureWebChannelsDisabled)) { + webchannelIntegration.set( + feature = FxaWebChannelFeature( + requireContext(), + customTabSessionId, + requireComponents.core.engine, + requireComponents.core.sessionManager, + requireComponents.backgroundServices.accountManager + ), + owner = this, + view = view + ) + } + 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 aacabc5c4d66..4fa09f3cd522 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,33 @@ 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 +166,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 +190,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 585f4fc8d28b..65e29c0be81d 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 8d066b1f6138..1638841042d3 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 1cd0aff11a01..9e47920ac608 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 @@ -224,15 +224,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 8ca18090a06b..28811e901ad6 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/java/org/mozilla/fenix/customtabs/AuthCustomTabActivity.kt b/app/src/main/java/org/mozilla/fenix/customtabs/AuthCustomTabActivity.kt index 8e74944d4a68..53c34a9373c7 100644 --- a/app/src/main/java/org/mozilla/fenix/customtabs/AuthCustomTabActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/customtabs/AuthCustomTabActivity.kt @@ -8,7 +8,6 @@ import mozilla.components.concept.sync.AccountObserver import mozilla.components.concept.sync.AuthType import mozilla.components.concept.sync.OAuthAccount import org.mozilla.fenix.ext.components - /** * A special custom tab for signing into a Firefox Account. The activity is closed once the user is signed in. */ diff --git a/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt index b20c42139d5c..04cad336e39e 100644 --- a/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt @@ -11,16 +11,19 @@ import kotlinx.android.synthetic.main.fragment_browser.view.* import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ObsoleteCoroutinesApi import mozilla.components.browser.session.Session +import mozilla.components.feature.accounts.FxaWebChannelFeature import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.support.base.feature.BackHandler import mozilla.components.support.base.feature.ViewBoundFeatureWrapper +import org.mozilla.fenix.Experiments import org.mozilla.fenix.R import org.mozilla.fenix.browser.BaseBrowserFragment import org.mozilla.fenix.components.toolbar.BrowserToolbarController import org.mozilla.fenix.components.toolbar.BrowserToolbarInteractor import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents +import org.mozilla.fenix.isInExperiment /** * Fragment used for browsing the web within external apps. @@ -30,6 +33,7 @@ import org.mozilla.fenix.ext.requireComponents class ExternalAppBrowserFragment : BaseBrowserFragment(), BackHandler { private val customTabsIntegration = ViewBoundFeatureWrapper() + private val webchannelIntegration = ViewBoundFeatureWrapper() override fun initializeUI(view: View): Session? { return super.initializeUI(view)?.also { @@ -50,6 +54,20 @@ class ExternalAppBrowserFragment : BaseBrowserFragment(), BackHandler { view = view) } + if (!requireContext().isInExperiment(Experiments.asFeatureWebChannelsDisabled)) { + webchannelIntegration.set( + feature = FxaWebChannelFeature( + requireContext(), + customTabSessionId, + requireComponents.core.engine, + requireComponents.core.sessionManager, + requireComponents.backgroundServices.accountManager + ), + owner = this, + view = view + ) + } + consumeFrom(browserStore) { browserToolbarView.update(it) } 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 c4b1fbc6834f..c07ae1d8f973 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 {