From f26c402f0a394ea36705d897a680ac25f6a6fe62 Mon Sep 17 00:00:00 2001 From: Jeff Boek Date: Tue, 4 Jun 2019 15:57:24 -0700 Subject: [PATCH] For #2395 - Properly takes you back to where you start authentication on completion --- .../mozilla/fenix/components/Components.kt | 2 +- .../org/mozilla/fenix/components/Services.kt | 7 +- .../features/FirefoxAccountsAuthFeature.kt | 67 +++++++++++++++++++ .../library/bookmarks/BookmarkFragment.kt | 2 +- .../SelectBookmarkFolderFragment.kt | 2 +- .../fenix/settings/AccountProblemFragment.kt | 2 +- .../mozilla/fenix/settings/PairFragment.kt | 13 ++-- .../fenix/settings/TurnOnSyncFragment.kt | 38 +++++++---- 8 files changed, 106 insertions(+), 27 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/components/features/FirefoxAccountsAuthFeature.kt 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 ec278249a..084b25b27 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -15,7 +15,7 @@ class Components(private val context: Context) { val backgroundServices by lazy { BackgroundServices(context, core.historyStorage, core.bookmarksStorage, utils.notificationManager) } - val services by lazy { Services(backgroundServices.accountManager, useCases.tabsUseCases) } + val services by lazy { Services(backgroundServices.accountManager) } val core by lazy { Core(context) } val search by lazy { Search(context) } val useCases by lazy { UseCases(context, core.sessionManager, core.engine.settings, search.searchEngineManager) } 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 9f37f4309..5454f16f2 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Services.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Services.kt @@ -4,9 +4,8 @@ package org.mozilla.fenix.components -import mozilla.components.feature.accounts.FirefoxAccountsAuthFeature -import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.service.fxa.manager.FxaAccountManager +import org.mozilla.fenix.components.features.FirefoxAccountsAuthFeature import org.mozilla.fenix.test.Mockable /** @@ -14,13 +13,11 @@ import org.mozilla.fenix.test.Mockable */ @Mockable class Services( - private val accountManager: FxaAccountManager, - private val tabsUseCases: TabsUseCases + private val accountManager: FxaAccountManager ) { val accountsAuthFeature by lazy { FirefoxAccountsAuthFeature( accountManager, - tabsUseCases, redirectUrl = BackgroundServices.REDIRECT_URL ) } diff --git a/app/src/main/java/org/mozilla/fenix/components/features/FirefoxAccountsAuthFeature.kt b/app/src/main/java/org/mozilla/fenix/components/features/FirefoxAccountsAuthFeature.kt new file mode 100644 index 000000000..2581c6954 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/components/features/FirefoxAccountsAuthFeature.kt @@ -0,0 +1,67 @@ +package org.mozilla.fenix.components.features + +import android.content.Context +import android.net.Uri +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import mozilla.components.concept.engine.EngineSession +import mozilla.components.concept.engine.request.RequestInterceptor +import mozilla.components.service.fxa.manager.FxaAccountManager +import org.mozilla.fenix.settings.SupportUtils +import kotlin.coroutines.CoroutineContext + +class FirefoxAccountsAuthFeature( + private val accountManager: FxaAccountManager, + private val redirectUrl: String, + private val coroutineContext: CoroutineContext = Dispatchers.Main +) { + fun beginAuthentication(context: Context) { + beginAuthenticationAsync(context) { + accountManager.beginAuthenticationAsync().await() + } + } + + fun beginPairingAuthentication(context: Context, pairingUrl: String) { + beginAuthenticationAsync(context) { + accountManager.beginAuthenticationAsync(pairingUrl).await() + } + } + + private fun beginAuthenticationAsync(context: Context, beginAuthentication: suspend () -> String?) { + CoroutineScope(coroutineContext).launch { + // FIXME return a fallback URL provided by Config... + // https://github.com/mozilla-mobile/android-components/issues/2496 + val authUrl = beginAuthentication() ?: "https://accounts.firefox.com/signin" + + // TODO + // We may fail to obtain an authentication URL, for example due to transient network errors. + // If that happens, open up a fallback URL in order to present some kind of a "no network" + // UI to the user. + // It's possible that the underlying problem will go away by the time the tab actually + // loads, resulting in a confusing experience. + val intent = SupportUtils.createCustomTabIntent(context, authUrl) + context.startActivity(intent) + } + } + + val interceptor = object : RequestInterceptor { + override fun onLoadRequest(session: EngineSession, uri: String): RequestInterceptor.InterceptionResponse? { + if (uri.startsWith(redirectUrl)) { + val parsedUri = Uri.parse(uri) + val code = parsedUri.getQueryParameter("code") + + if (code != null) { + val state = parsedUri.getQueryParameter("state") as String + + // Notify the state machine about our success. + accountManager.finishAuthenticationAsync(code, state) + + return RequestInterceptor.InterceptionResponse.Url(redirectUrl) + } + } + + return null + } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt index 0c386d183..71a7ee6b9 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt @@ -284,7 +284,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve .subscribe { when (it) { is SignInAction.ClickedSignIn -> { - context?.components?.services?.accountsAuthFeature?.beginAuthentication() + context?.components?.services?.accountsAuthFeature?.beginAuthentication(requireContext()) (activity as HomeActivity).openToBrowser(BrowserDirection.FromBookmarks) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt index 767e4e451..3129e6e83 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt @@ -91,7 +91,7 @@ class SelectBookmarkFolderFragment : Fragment(), CoroutineScope, AccountObserver .subscribe { when (it) { is SignInAction.ClickedSignIn -> { - requireComponents.services.accountsAuthFeature.beginAuthentication() + requireComponents.services.accountsAuthFeature.beginAuthentication(requireContext()) view?.let { (activity as HomeActivity).openToBrowser(BrowserDirection.FromBookmarksFolderSelect) } diff --git a/app/src/main/java/org/mozilla/fenix/settings/AccountProblemFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/AccountProblemFragment.kt index 6da67c86b..f7d94e332 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/AccountProblemFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/AccountProblemFragment.kt @@ -39,7 +39,7 @@ class AccountProblemFragment : PreferenceFragmentCompat() { private fun getClickListenerForSignIn(): Preference.OnPreferenceClickListener { return Preference.OnPreferenceClickListener { - requireComponents.services.accountsAuthFeature.beginAuthentication() + requireComponents.services.accountsAuthFeature.beginAuthentication(requireContext()) // TODO The sign-in web content populates session history, // so pressing "back" after signing in won't take us back into the settings screen, but rather up the // session history stack. diff --git a/app/src/main/java/org/mozilla/fenix/settings/PairFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/PairFragment.kt index e2e5d6220..7307b320e 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/PairFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/PairFragment.kt @@ -16,8 +16,6 @@ import androidx.navigation.fragment.NavHostFragment.findNavController import mozilla.components.feature.qr.QrFeature import mozilla.components.support.base.feature.BackHandler import mozilla.components.support.base.feature.ViewBoundFeatureWrapper -import org.mozilla.fenix.BrowserDirection -import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.ext.requireComponents @@ -43,8 +41,12 @@ class PairFragment : Fragment(), BackHandler { requestPermissions(permissions, REQUEST_CODE_CAMERA_PERMISSIONS) }, onScanResult = { pairingUrl -> - requireComponents.services.accountsAuthFeature.beginPairingAuthentication(pairingUrl) - (activity as HomeActivity).openToBrowser(BrowserDirection.FromPair) + requireComponents.services.accountsAuthFeature.beginPairingAuthentication( + requireContext(), + pairingUrl + ) + findNavController(this@PairFragment) + .popBackStack(R.id.turnOnSyncFragment, false) }), owner = this, view = view @@ -63,7 +65,8 @@ class PairFragment : Fragment(), BackHandler { override fun onBackPressed(): Boolean { qrFeature.onBackPressed() - findNavController(this@PairFragment).navigateUp() + findNavController(this@PairFragment) + .popBackStack(R.id.turnOnSyncFragment, false) return true } diff --git a/app/src/main/java/org/mozilla/fenix/settings/TurnOnSyncFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/TurnOnSyncFragment.kt index ad8793f7d..859ba4eef 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/TurnOnSyncFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/TurnOnSyncFragment.kt @@ -7,17 +7,20 @@ package org.mozilla.fenix.settings import android.os.Bundle import androidx.appcompat.app.AppCompatActivity import androidx.navigation.Navigation +import androidx.navigation.fragment.NavHostFragment.findNavController import androidx.preference.Preference import androidx.preference.PreferenceFragmentCompat -import org.mozilla.fenix.BrowserDirection -import org.mozilla.fenix.HomeActivity +import mozilla.components.concept.sync.AccountObserver +import mozilla.components.concept.sync.OAuthAccount +import mozilla.components.concept.sync.Profile import org.mozilla.fenix.R +import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.ext.requireComponents -class TurnOnSyncFragment : PreferenceFragmentCompat() { - +@SuppressWarnings("TooManyFunctions") +class TurnOnSyncFragment : PreferenceFragmentCompat(), AccountObserver { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) requireComponents.analytics.metrics.track(Event.SyncAuthOpened) @@ -30,6 +33,11 @@ class TurnOnSyncFragment : PreferenceFragmentCompat() { override fun onResume() { super.onResume() + if (requireComponents.backgroundServices.accountManager.authenticatedAccount() != null) { + findNavController(this).popBackStack() + } + + requireComponents.backgroundServices.accountManager.register(this, owner = this) (activity as AppCompatActivity).title = getString(R.string.preferences_sync) (activity as AppCompatActivity).supportActionBar?.show() } @@ -50,16 +58,13 @@ class TurnOnSyncFragment : PreferenceFragmentCompat() { private fun getClickListenerForSignIn(): Preference.OnPreferenceClickListener { return Preference.OnPreferenceClickListener { - requireComponents.services.accountsAuthFeature.beginAuthentication() + requireComponents.services.accountsAuthFeature.beginAuthentication(requireContext()) // TODO The sign-in web content populates session history, // so pressing "back" after signing in won't take us back into the settings screen, but rather up the // session history stack. // We could auto-close this tab once we get to the end of the authentication process? // Via an interceptor, perhaps. requireComponents.analytics.metrics.track(Event.SyncAuthSignIn) - view?.let { - (activity as HomeActivity).openToBrowser(BrowserDirection.FromTurnOnSync) - } true } } @@ -67,11 +72,8 @@ class TurnOnSyncFragment : PreferenceFragmentCompat() { private fun getClickListenerForCreateAccount(): Preference.OnPreferenceClickListener { // Currently the same as sign in, as FxA handles this, however we want to emit a different telemetry event return Preference.OnPreferenceClickListener { - requireComponents.services.accountsAuthFeature.beginAuthentication() + requireComponents.services.accountsAuthFeature.beginAuthentication(requireContext()) requireComponents.analytics.metrics.track(Event.SyncAuthCreateAccount) - view?.let { - (activity as HomeActivity).openToBrowser(BrowserDirection.FromTurnOnSync) - } true } } @@ -85,4 +87,14 @@ class TurnOnSyncFragment : PreferenceFragmentCompat() { true } } -} + + override fun onAuthenticated(account: OAuthAccount) { + FenixSnackbar.make(view!!, FenixSnackbar.LENGTH_SHORT) + .setText(requireContext().getString(R.string.sync_syncing_in_progress)) + .show() + } + + override fun onAuthenticationProblems() {} + override fun onError(error: Exception) {} + override fun onLoggedOut() {} + override fun onProfileUpdated(profile: Profile) {} }