From 82329b20a4746f23c54c99d9ffc581cc17aad698 Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Wed, 20 Dec 2023 10:49:14 -0500 Subject: [PATCH] Bug 186779 -- Migrate to using the app-services FxA state machine Also hooked up the new secret debug menu entries for testing it. --- .../components/service/fxa/AccountStorage.kt | 19 +- .../components/service/fxa/FirefoxAccount.kt | 10 + .../service/fxa/FxaDeviceConstellation.kt | 15 - .../manager/AppServicesStateMachineChecker.kt | 210 --------- .../service/fxa/manager/FxaAccountManager.kt | 432 ++++++------------ .../fenix/settings/SyncDebugFragment.kt | 19 + .../src/main/res/values/preference_keys.xml | 3 + .../src/main/res/values/static_strings.xml | 6 + .../main/res/xml/sync_debug_preferences.xml | 9 + 9 files changed, 192 insertions(+), 531 deletions(-) delete mode 100644 android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/AppServicesStateMachineChecker.kt diff --git a/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/AccountStorage.kt b/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/AccountStorage.kt index 37af0f5b76a6..e5f8a665eb54 100644 --- a/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/AccountStorage.kt +++ b/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/AccountStorage.kt @@ -11,7 +11,6 @@ import mozilla.appservices.fxaclient.FxaRustAuthState import mozilla.components.concept.base.crash.CrashReporting import mozilla.components.concept.sync.AccountEvent import mozilla.components.concept.sync.AccountEventsObserver -import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.StatePersistenceCallback import mozilla.components.lib.dataprotect.SecureAbove22Preferences import mozilla.components.service.fxa.manager.FxaAccountManager @@ -26,16 +25,16 @@ const val FXA_STATE_KEY = "fxaState" * Represents state of our account on disk - is it new, or restored? */ internal sealed class AccountOnDisk : WithAccount { - data class Restored(val account: OAuthAccount) : AccountOnDisk() { + data class Restored(val account: FirefoxAccount) : AccountOnDisk() { override fun account() = account } - data class New(val account: OAuthAccount) : AccountOnDisk() { + data class New(val account: FirefoxAccount) : AccountOnDisk() { override fun account() = account } } internal interface WithAccount { - fun account(): OAuthAccount + fun account(): FirefoxAccount } /** @@ -80,16 +79,16 @@ open class StorageWrapper( } } - private fun watchAccount(account: OAuthAccount) { + private fun watchAccount(account: FirefoxAccount) { account.registerPersistenceCallback(statePersistenceCallback) account.deviceConstellation().register(accountEventsIntegration) } /** - * Exists strictly for testing purposes, allowing tests to specify their own implementation of [OAuthAccount]. + * Exists strictly for testing purposes, allowing tests to specify their own implementation of [FirefoxAccount]. */ @VisibleForTesting - open fun obtainAccount(): OAuthAccount = FirefoxAccount(serverConfig, crashReporter) + open fun obtainAccount(): FirefoxAccount = FirefoxAccount(serverConfig, crashReporter) } /** @@ -110,7 +109,7 @@ internal class AccountEventsIntegration( internal interface AccountStorage { @Throws(Exception::class) - fun read(): OAuthAccount? + fun read(): FirefoxAccount? fun write(accountState: String) fun clear() } @@ -156,7 +155,7 @@ internal class SharedPrefAccountStorage( * @throws FxaException if JSON failed to parse into a [FirefoxAccount]. */ @Throws(FxaException::class) - override fun read(): OAuthAccount? { + override fun read(): FirefoxAccount? { val savedJSON = accountPreferences().getString(FXA_STATE_KEY, null) ?: return null @@ -244,7 +243,7 @@ internal class SecureAbove22AccountStorage( * @throws FxaException if JSON failed to parse into a [FirefoxAccount]. */ @Throws(FxaException::class) - override fun read(): OAuthAccount? { + override fun read(): FirefoxAccount? { return store.getString(KEY_ACCOUNT_STATE).also { // If account state is missing, but we expected it to be present, report an exception. if (it == null && prefs.getBoolean(PREF_KEY_HAS_STATE, false)) { diff --git a/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FirefoxAccount.kt b/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FirefoxAccount.kt index 7fc31785e341..3547467d8fe7 100644 --- a/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FirefoxAccount.kt +++ b/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FirefoxAccount.kt @@ -11,6 +11,8 @@ import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.plus import kotlinx.coroutines.withContext import mozilla.appservices.fxaclient.FxaClient +import mozilla.appservices.fxaclient.FxaEvent +import mozilla.appservices.fxaclient.FxaState import mozilla.components.concept.base.crash.CrashReporting import mozilla.components.concept.sync.AuthFlowUrl import mozilla.components.concept.sync.DeviceConstellation @@ -96,6 +98,14 @@ class FirefoxAccount internal constructor( internal fun getAuthState() = inner.getAuthState() + internal fun processEvent(event: FxaEvent): FxaState = inner.processEvent(event) + + internal fun simulateNetworkError() = inner.simulateNetworkError() + + internal fun simulateTemporaryAuthTokenIssue() = inner.simulateTemporaryAuthTokenIssue() + + internal fun simulatePermanentAuthTokenIssue() = inner.simulatePermanentAuthTokenIssue() + override suspend fun beginOAuthFlow( scopes: Set, entryPoint: FxAEntryPoint, diff --git a/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceConstellation.kt b/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceConstellation.kt index 7a7ace119449..a2c2bf754ecb 100644 --- a/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceConstellation.kt +++ b/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceConstellation.kt @@ -12,8 +12,6 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.withContext import mozilla.appservices.fxaclient.FxaClient import mozilla.appservices.fxaclient.FxaException -import mozilla.appservices.fxaclient.FxaStateCheckerEvent -import mozilla.appservices.fxaclient.FxaStateCheckerState import mozilla.appservices.syncmanager.SyncTelemetry import mozilla.components.concept.base.crash.CrashReporting import mozilla.components.concept.sync.AccountEvent @@ -27,7 +25,6 @@ import mozilla.components.concept.sync.DeviceConstellation import mozilla.components.concept.sync.DeviceConstellationObserver import mozilla.components.concept.sync.DevicePushSubscription import mozilla.components.concept.sync.ServiceResult -import mozilla.components.service.fxa.manager.AppServicesStateMachineChecker import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.base.observer.Observable import mozilla.components.support.base.observer.ObserverRegistry @@ -89,33 +86,22 @@ class FxaDeviceConstellation( val capabilities = config.capabilities.map { it.into() }.toSet() if (finalizeAction == DeviceFinalizeAction.Initialize) { try { - AppServicesStateMachineChecker.checkInternalState(FxaStateCheckerState.InitializeDevice) account.initializeDevice(config.name, config.type.into(), capabilities) - AppServicesStateMachineChecker.handleInternalEvent(FxaStateCheckerEvent.InitializeDeviceSuccess) ServiceResult.Ok } catch (e: FxaPanicException) { - AppServicesStateMachineChecker.handleInternalEvent(FxaStateCheckerEvent.CallError) throw e } catch (e: FxaUnauthorizedException) { - AppServicesStateMachineChecker.handleInternalEvent(FxaStateCheckerEvent.CallError) ServiceResult.AuthError } catch (e: FxaException) { - AppServicesStateMachineChecker.handleInternalEvent(FxaStateCheckerEvent.CallError) ServiceResult.OtherError } } else { try { - AppServicesStateMachineChecker.checkInternalState(FxaStateCheckerState.EnsureDeviceCapabilities) account.ensureCapabilities(capabilities) - AppServicesStateMachineChecker.handleInternalEvent( - FxaStateCheckerEvent.EnsureDeviceCapabilitiesSuccess, - ) ServiceResult.Ok } catch (e: FxaPanicException) { - AppServicesStateMachineChecker.handleInternalEvent(FxaStateCheckerEvent.CallError) throw e } catch (e: FxaUnauthorizedException) { - AppServicesStateMachineChecker.handleInternalEvent(FxaStateCheckerEvent.EnsureCapabilitiesAuthError) // Unless we've added a new capability, in practice 'ensureCapabilities' isn't // actually expected to do any work: everything should have been done by initializeDevice. // So if it did, and failed, let's report this so that we're aware of this! @@ -125,7 +111,6 @@ class FxaDeviceConstellation( ) ServiceResult.AuthError } catch (e: FxaException) { - AppServicesStateMachineChecker.handleInternalEvent(FxaStateCheckerEvent.CallError) ServiceResult.OtherError } } diff --git a/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/AppServicesStateMachineChecker.kt b/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/AppServicesStateMachineChecker.kt deleted file mode 100644 index e9042c099dfd..000000000000 --- a/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/AppServicesStateMachineChecker.kt +++ /dev/null @@ -1,210 +0,0 @@ -/* 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 mozilla.components.service.fxa.manager - -import mozilla.appservices.fxaclient.FxaEvent -import mozilla.appservices.fxaclient.FxaRustAuthState -import mozilla.appservices.fxaclient.FxaState -import mozilla.appservices.fxaclient.FxaStateCheckerEvent -import mozilla.appservices.fxaclient.FxaStateCheckerState -import mozilla.appservices.fxaclient.FxaStateMachineChecker -import mozilla.components.concept.sync.DeviceConfig -import mozilla.components.service.fxa.into -import mozilla.appservices.fxaclient.DeviceConfig as ASDeviceConfig - -/** - * Checks the new app-services state machine logic against the current android-components code - * - * This is a temporary measure to prep for migrating the android-components code to using the new - * app-services state machine. It performs a "dry-run" test of the code where we check the new logic - * against the current logic, without performing any side-effects. - * - * If one of the checks fails, then we report an error to Sentry. After that the calls become - * no-ops to avoid spamming sentry with errors from a single client. By checking the Sentry errors, - * we can find places where the application-services logic doesn't match the current - * android-components logic and fix the issue. - * - * Once we determine that the new application-services code is correct, let's switch the - * android-components code to using it (https://bugzilla.mozilla.org/show_bug.cgi?id=1867793) and - * delete all this code. - */ -object AppServicesStateMachineChecker { - /** - * The Rust state machine checker. This handles the actual checks and sentry reporting. - */ - private val rustChecker = FxaStateMachineChecker() - - /** - * Handle an event about to be processed - * - * Call this when processing an android-components event, after checking that it's valid. - */ - internal fun handleEvent(event: Event, deviceConfig: DeviceConfig, scopes: Set) { - val convertedEvent = when (event) { - Event.Account.Start -> FxaEvent.Initialize( - ASDeviceConfig( - name = deviceConfig.name, - deviceType = deviceConfig.type.into(), - capabilities = ArrayList(deviceConfig.capabilities.map { it.into() }), - ), - ) - is Event.Account.BeginEmailFlow -> FxaEvent.BeginOAuthFlow(ArrayList(scopes), event.entrypoint.entryName) - is Event.Account.BeginPairingFlow -> { - // pairingUrl should always be non-null, if it is somehow null let's use a - // placeholder value that can be identified when checking in sentry - val pairingUrl = event.pairingUrl ?: "" - FxaEvent.BeginPairingFlow(pairingUrl, ArrayList(scopes), event.entrypoint.entryName) - } - is Event.Account.AuthenticationError -> FxaEvent.CheckAuthorizationStatus - Event.Account.AccessTokenKeyError -> FxaEvent.CheckAuthorizationStatus - Event.Account.Logout -> FxaEvent.Disconnect - // This is the one ProgressEvent that's considered a "public event" in app-services - is Event.Progress.AuthData -> FxaEvent.CompleteOAuthFlow(event.authData.code, event.authData.state) - is Event.Progress.CancelAuth -> FxaEvent.CancelOAuthFlow - else -> return - } - rustChecker.handlePublicEvent(convertedEvent) - } - - /** - * Check a new account state - * - * Call this after transitioning to an android-components account state. - */ - internal fun checkAccountState(state: AccountState) { - val convertedState = when (state) { - AccountState.NotAuthenticated -> FxaState.Disconnected - is AccountState.Authenticating -> FxaState.Authenticating(state.oAuthUrl) - AccountState.Authenticated -> FxaState.Connected - AccountState.AuthenticationProblem -> FxaState.AuthIssues - } - rustChecker.checkPublicState(convertedState) - } - - /** - * General validation for new progress state being processed by the AC state machine. - * - * This handles all validation for most state transitions in a simple manner. The one transition - * it can't handle is completing oauth, which entails multiple FxA calls and can fail in multiple - * different ways. For that, the lower-level `checkInternalState` and `handleInternalEvent` are - * used. - */ - @Suppress("LongMethod") - internal fun validateProgressEvent(progressEvent: Event.Progress, via: Event, scopes: Set) { - when (progressEvent) { - Event.Progress.AccountRestored -> { - AppServicesStateMachineChecker.checkInternalState(FxaStateCheckerState.GetAuthState) - AppServicesStateMachineChecker.handleInternalEvent( - FxaStateCheckerEvent.GetAuthStateSuccess(FxaRustAuthState.CONNECTED), - ) - } - Event.Progress.AccountNotFound -> { - AppServicesStateMachineChecker.checkInternalState(FxaStateCheckerState.GetAuthState) - AppServicesStateMachineChecker.handleInternalEvent( - FxaStateCheckerEvent.GetAuthStateSuccess(FxaRustAuthState.DISCONNECTED), - ) - } - is Event.Progress.StartedOAuthFlow -> { - when (via) { - is Event.Account.BeginEmailFlow -> { - AppServicesStateMachineChecker.checkInternalState( - FxaStateCheckerState.BeginOAuthFlow(ArrayList(scopes), via.entrypoint.entryName), - ) - AppServicesStateMachineChecker.handleInternalEvent( - FxaStateCheckerEvent.BeginOAuthFlowSuccess(progressEvent.oAuthUrl), - ) - } - is Event.Account.BeginPairingFlow -> { - AppServicesStateMachineChecker.checkInternalState( - FxaStateCheckerState.BeginPairingFlow( - via.pairingUrl!!, - ArrayList(scopes), - via.entrypoint.entryName, - ), - ) - AppServicesStateMachineChecker.handleInternalEvent( - FxaStateCheckerEvent.BeginPairingFlowSuccess(progressEvent.oAuthUrl), - ) - } - // This branch should never be taken, if it does we'll probably see a state - // check error down the line. - else -> Unit - } - } - Event.Progress.FailedToBeginAuth -> { - when (via) { - is Event.Account.BeginEmailFlow -> { - AppServicesStateMachineChecker.checkInternalState( - FxaStateCheckerState.BeginOAuthFlow(ArrayList(scopes), via.entrypoint.entryName), - ) - } - is Event.Account.BeginPairingFlow -> { - AppServicesStateMachineChecker.checkInternalState( - FxaStateCheckerState.BeginPairingFlow( - via.pairingUrl!!, - ArrayList(scopes), - via.entrypoint.entryName, - ), - ) - } - // This branch should never be taken, if it does we'll probably see a state - // check error down the line. - else -> Unit - } - AppServicesStateMachineChecker.handleInternalEvent(FxaStateCheckerEvent.CallError) - } - Event.Progress.LoggedOut -> { - AppServicesStateMachineChecker.checkInternalState( - FxaStateCheckerState.Disconnect, - ) - AppServicesStateMachineChecker.handleInternalEvent( - FxaStateCheckerEvent.DisconnectSuccess, - ) - } - Event.Progress.RecoveredFromAuthenticationProblem -> { - AppServicesStateMachineChecker.checkInternalState(FxaStateCheckerState.CheckAuthorizationStatus) - AppServicesStateMachineChecker.handleInternalEvent( - FxaStateCheckerEvent.CheckAuthorizationStatusSuccess(true), - ) - } - Event.Progress.FailedToRecoverFromAuthenticationProblem -> { - // `via` should always be `AuthenticationError` if not, we'll probably see a state - // check error down the line. - if (via is Event.Account.AuthenticationError) { - if (via.errorCountWithinTheTimeWindow >= AUTH_CHECK_CIRCUIT_BREAKER_COUNT) { - // In this case, the state machine fails early and doesn't actualy make any - // calls - return - } - AppServicesStateMachineChecker.checkInternalState(FxaStateCheckerState.CheckAuthorizationStatus) - AppServicesStateMachineChecker.handleInternalEvent( - FxaStateCheckerEvent.CheckAuthorizationStatusSuccess(false), - ) - } - } - else -> Unit - } - } - - /** - * Check an app-services internal state - * - * The app-services internal states correspond to internal firefox account method calls. Call - * this before making one of those calls. - */ - internal fun checkInternalState(state: FxaStateCheckerState) { - rustChecker.checkInternalState(state) - } - - /** - * Handle an app-services internal event - * - * The app-services internal states correspond the results of internal firefox account method - * calls. Call this before after making a call. - */ - internal fun handleInternalEvent(event: FxaStateCheckerEvent) { - rustChecker.handleInternalEvent(event) - } -} diff --git a/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt b/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt index 925f96e2acf1..c535d41200c6 100644 --- a/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt +++ b/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt @@ -12,8 +12,9 @@ import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.cancel import kotlinx.coroutines.withContext -import mozilla.appservices.fxaclient.FxaStateCheckerEvent -import mozilla.appservices.fxaclient.FxaStateCheckerState +import mozilla.appservices.fxaclient.FxaEvent +import mozilla.appservices.fxaclient.FxaException +import mozilla.appservices.fxaclient.FxaState import mozilla.appservices.syncmanager.DeviceSettings import mozilla.components.concept.base.crash.Breadcrumb import mozilla.components.concept.base.crash.CrashReporting @@ -25,15 +26,12 @@ import mozilla.components.concept.sync.DeviceConfig import mozilla.components.concept.sync.FxAEntryPoint import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.Profile -import mozilla.components.concept.sync.ServiceResult import mozilla.components.service.fxa.AccessTokenUnexpectedlyWithoutKey import mozilla.components.service.fxa.AccountManagerException -import mozilla.components.service.fxa.AccountOnDisk import mozilla.components.service.fxa.AccountStorage import mozilla.components.service.fxa.FxaAuthData import mozilla.components.service.fxa.FxaDeviceSettingsCache import mozilla.components.service.fxa.FxaSyncScopedKeyMissingException -import mozilla.components.service.fxa.Result import mozilla.components.service.fxa.SecureAbove22AccountStorage import mozilla.components.service.fxa.ServerConfig import mozilla.components.service.fxa.SharedPrefAccountStorage @@ -41,7 +39,6 @@ import mozilla.components.service.fxa.StorageWrapper import mozilla.components.service.fxa.SyncAuthInfoCache import mozilla.components.service.fxa.SyncConfig import mozilla.components.service.fxa.SyncEngine -import mozilla.components.service.fxa.asAuthFlowUrl import mozilla.components.service.fxa.asSyncAuthInfo import mozilla.components.service.fxa.emitSyncFailedFact import mozilla.components.service.fxa.into @@ -50,17 +47,14 @@ import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.service.fxa.sync.SyncStatusObserver import mozilla.components.service.fxa.sync.WorkManagerSyncManager import mozilla.components.service.fxa.sync.clearSyncState -import mozilla.components.service.fxa.withRetries -import mozilla.components.service.fxa.withServiceRetries import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.base.observer.Observable import mozilla.components.support.base.observer.ObserverRegistry import mozilla.components.support.base.utils.NamedThreadFactory import java.io.Closeable -import java.util.concurrent.ConcurrentLinkedQueue import java.util.concurrent.Executors -import java.util.concurrent.atomic.AtomicBoolean import kotlin.coroutines.CoroutineContext +import mozilla.appservices.fxaclient.DeviceConfig as ASDeviceConfig // Necessary to fetch a profile. const val SCOPE_PROFILE = "profile" @@ -112,22 +106,6 @@ open class FxaAccountManager( ) : Closeable, Observable by ObserverRegistry() { private val logger = Logger("FirefoxAccountStateMachine") - @Volatile - private var latestAuthState: String? = null - - // Used to detect multiple auth recovery attempts at once - // (https://bugzilla.mozilla.org/show_bug.cgi?id=1864994) - private var authRecoveryScheduled = AtomicBoolean(false) - private fun checkForMultipleRequiveryCalls() { - val alreadyScheduled = authRecoveryScheduled.getAndSet(true) - if (alreadyScheduled) { - crashReporter?.recordCrashBreadcrumb(Breadcrumb("multiple fxa recoveries scheduled at once")) - } - } - private fun finishedAuthRecovery() { - authRecoveryScheduled.set(false) - } - init { GlobalAccountManager.setInstance(this) } @@ -147,7 +125,6 @@ open class FxaAccountManager( @Volatile private var state: State = State.Idle(AccountState.NotAuthenticated) @Volatile private var isAccountManagerReady: Boolean = false - private val eventQueue = ConcurrentLinkedQueue() @VisibleForTesting val accountEventObserverRegistry = ObserverRegistry() @@ -350,12 +327,6 @@ open class FxaAccountManager( pairingUrl: String? = null, entrypoint: FxAEntryPoint, ): String? = withContext(coroutineContext) { - // It's possible that at this point authentication is considered to be "in-progress". - // For example, if user started authentication flow, but cancelled it (closing a custom tab) - // without finishing. - // In a clean scenario (no prior auth attempts), this event will be ignored by the state machine. - processQueue(Event.Progress.CancelAuth) - val event = if (pairingUrl != null) { Event.Account.BeginPairingFlow(pairingUrl, entrypoint) } else { @@ -386,22 +357,9 @@ open class FxaAccountManager( * channel via [WebChannelFeature]. */ suspend fun finishAuthentication(authData: FxaAuthData) = withContext(coroutineContext) { - when { - latestAuthState == null -> { - logger.warn("Trying to finish authentication that was never started.") - false - } - authData.state != latestAuthState -> { - logger.warn("Trying to finish authentication for an invalid auth state; ignoring.") - false - } - authData.state == latestAuthState -> { - authData.declinedEngines?.let { persistDeclinedEngines(it) } - processQueue(Event.Progress.AuthData(authData)) - true - } - else -> throw IllegalStateException("Unexpected finishAuthentication state") - } + authData.declinedEngines?.let { persistDeclinedEngines(it) } + processQueue(Event.Progress.AuthData(authData)) + true } /** @@ -441,7 +399,6 @@ open class FxaAccountManager( operation: String, errorCountWithinTheTimeWindow: Int = 1, ) { - checkForMultipleRequiveryCalls() return withContext(coroutineContext) { processQueue( Event.Account.AuthenticationError(operation, errorCountWithinTheTimeWindow), @@ -453,39 +410,54 @@ open class FxaAccountManager( * Pumps the state machine until all events are processed and their side-effects resolve. */ private suspend fun processQueue(event: Event) { - eventQueue.add(event) - do { - val toProcess: Event = eventQueue.poll()!! - val transitionInto = state.next(toProcess) - - if (transitionInto == null) { - logger.warn("Got invalid event '$toProcess' for state $state.") - continue - } - - AppServicesStateMachineChecker.handleEvent(toProcess, deviceConfig, scopes) - if (transitionInto is State.Idle) { - AppServicesStateMachineChecker.checkAccountState(transitionInto.accountState) - } + var fxaEvent = calcFxaEvent(event) + if (fxaEvent == null) { + logger.warn("processQueue: Got invalid event '$event'") + return + } - logger.info("Processing event '$toProcess' for state $state. Next state is $transitionInto") + val newFxaState = try { + account.processEvent(fxaEvent) + } catch (e: FxaException) { + logger.warn("processQueue: Error in processEvent: $e") + return + } - state = transitionInto + try { + accountStateSideEffects(newFxaState, event) + } catch (_: AccountManagerException.AuthenticationSideEffectsFailed) { + account.processEvent(FxaEvent.Disconnect) + accountStateSideEffects(FxaState.Disconnected, Event.Account.Logout) + } + } - stateActions(state, toProcess)?.let { successiveEvent -> - logger.info("Ran '$toProcess' side-effects for state $state, got successive event $successiveEvent") - if (successiveEvent is Event.Progress) { - // Note: stateActions should only return progress events, so this captures all - // possibilities. - AppServicesStateMachineChecker.validateProgressEvent(successiveEvent, toProcess, scopes) - } - eventQueue.add(successiveEvent) + private fun calcFxaEvent(event: Event): FxaEvent? = when (event) { + Event.Account.Start -> FxaEvent.Initialize( + ASDeviceConfig( + name = deviceConfig.name, + deviceType = deviceConfig.type.into(), + capabilities = ArrayList(deviceConfig.capabilities.map { it.into() }), + ), + ) + is Event.Account.BeginEmailFlow -> FxaEvent.BeginOAuthFlow(ArrayList(scopes), event.entrypoint.entryName) + is Event.Account.BeginPairingFlow -> { + if (event.pairingUrl != null) { + FxaEvent.BeginPairingFlow(event.pairingUrl, ArrayList(scopes), event.entrypoint.entryName) + } else { + crashReporter?.recordCrashBreadcrumb(Breadcrumb("event.pairingUrl is null")) + null } - } while (!eventQueue.isEmpty()) + } + is Event.Account.AuthenticationError -> FxaEvent.CheckAuthorizationStatus + Event.Account.AccessTokenKeyError -> FxaEvent.CheckAuthorizationStatus + Event.Account.Logout -> FxaEvent.Disconnect + // This is the one ProgressEvent that's considered a "public event" in app-services + is Event.Progress.AuthData -> FxaEvent.CompleteOAuthFlow(event.authData.code, event.authData.state) + else -> null } /** - * Side-effects of entering [AccountState] type states + * Side-effects of entering a new FxaState * * Upon entering these states, observers are typically notified. The sole exception occurs * during the completion of authentication, where it is necessary to populate the @@ -495,242 +467,62 @@ open class FxaAccountManager( * run the side effects for a newly authenticated account. */ private suspend fun accountStateSideEffects( - forState: State.Idle, - via: Event, - ): Unit = when (forState.accountState) { - AccountState.NotAuthenticated -> when (via) { - Event.Progress.LoggedOut -> { - resetAccount() - notifyObservers { onLoggedOut() } - } - Event.Progress.FailedToBeginAuth -> { - resetAccount() - notifyObservers { onFlowError(AuthFlowError.FailedToBeginAuth) } - } - Event.Progress.FailedToCompleteAuth -> { - resetAccount() - notifyObservers { onFlowError(AuthFlowError.FailedToCompleteAuth) } - } - Event.Progress.FailedToRecoverFromAuthenticationProblem -> { - finishedAuthRecovery() - } - else -> Unit - } - AccountState.Authenticated -> when (via) { - is Event.Progress.CompletedAuthentication -> { - val operation = when (via.authType) { - AuthType.Existing -> "CompletingAuthentication:accountRestored" - else -> "CompletingAuthentication:AuthData" - } - if (authenticationSideEffects(operation)) { - notifyObservers { onAuthenticated(account, via.authType) } - refreshProfile(ignoreCache = false) - Unit - } else { - throw AccountManagerException.AuthenticationSideEffectsFailed() - } - } - Event.Progress.RecoveredFromAuthenticationProblem -> { - finishedAuthRecovery() - // Clear our access token cache; it'll be re-populated as part of the - // regular state machine flow. - SyncAuthInfoCache(context).clear() - // Should we also call authenticationSideEffects here? - // (https://bugzilla.mozilla.org/show_bug.cgi?id=1865086) - notifyObservers { onAuthenticated(account, AuthType.Recovered) } - refreshProfile(ignoreCache = true) - Unit - } - else -> Unit - } - AccountState.AuthenticationProblem -> { - SyncAuthInfoCache(context).clear() - notifyObservers { onAuthenticationProblems() } - } - else -> Unit - } - - /** - * Side-effects of entering [ProgressState] states. These side-effects are actions we need to take - * to perform a state transition. For example, we wipe local state while entering a [ProgressState.LoggingOut]. - * - * @return An optional follow-up [Event] that we'd like state machine to process after entering [forState] - * and processing its side-effects. - */ - @Suppress("NestedBlockDepth", "LongMethod") - private suspend fun internalStateSideEffects( - forState: State.Active, + forState: FxaState, via: Event, - ): Event? = when (forState.progressState) { - ProgressState.Initializing -> { - when (accountOnDisk) { - is AccountOnDisk.New -> Event.Progress.AccountNotFound - is AccountOnDisk.Restored -> { - Event.Progress.AccountRestored - } - } - } - ProgressState.LoggingOut -> { - Event.Progress.LoggedOut - } - ProgressState.BeginningAuthentication -> when (via) { - is Event.Account.BeginPairingFlow, is Event.Account.BeginEmailFlow -> { - val pairingUrl = if (via is Event.Account.BeginPairingFlow) { - via.pairingUrl - } else { - null - } - val entrypoint = if (via is Event.Account.BeginEmailFlow) { - via.entrypoint - } else if (via is Event.Account.BeginPairingFlow) { - via.entrypoint - } else { - // This should be impossible, both `BeginPairingFlow` and `BeginEmailFlow` - // have a required `entrypoint` and we are matching against only instances - // of those data classes. - throw IllegalStateException("BeginningAuthentication with a flow that is neither email nor pairing") + ) { + when (forState) { + FxaState.Disconnected -> when (via) { + Event.Account.Logout -> { + resetAccount() + notifyObservers { onLoggedOut() } } - val result = withRetries(logger, MAX_NETWORK_RETRIES) { - pairingUrl.asAuthFlowUrl(account, scopes, entrypoint = entrypoint) + is Event.Account.BeginEmailFlow, is Event.Account.BeginPairingFlow -> { + resetAccount() + notifyObservers { onFlowError(AuthFlowError.FailedToBeginAuth) } } - when (result) { - is Result.Success -> { - latestAuthState = result.value!!.state - Event.Progress.StartedOAuthFlow(result.value.url) - } - Result.Failure -> { - Event.Progress.FailedToBeginAuth - } + is Event.Progress.AuthData -> { + resetAccount() + notifyObservers { onFlowError(AuthFlowError.FailedToCompleteAuth) } } + else -> Unit } - else -> null - } - ProgressState.CompletingAuthentication -> when (via) { - Event.Progress.AccountRestored -> { - val authType = AuthType.Existing - when (withServiceRetries(logger, MAX_NETWORK_RETRIES) { finalizeDevice(authType) }) { - ServiceResult.Ok -> { - Event.Progress.CompletedAuthentication(authType) - } - ServiceResult.AuthError -> { - checkForMultipleRequiveryCalls() - Event.Account.AuthenticationError("finalizeDevice") - } - ServiceResult.OtherError -> { - Event.Progress.FailedToCompleteAuthRestore + FxaState.Connected -> when (via) { + is Event.Account.Start -> { + if (authenticationSideEffects("CompletingAuthentication:accountRestored")) { + notifyObservers { onAuthenticated(account, AuthType.Existing) } + refreshProfile(ignoreCache = false) + } else { + throw AccountManagerException.AuthenticationSideEffectsFailed() } } - } - is Event.Progress.AuthData -> { - val completeAuth = suspend { - AppServicesStateMachineChecker.checkInternalState( - FxaStateCheckerState.CompleteOAuthFlow(via.authData.code, via.authData.state), - ) - withRetries(logger, MAX_NETWORK_RETRIES) { - account.completeOAuthFlow(via.authData.code, via.authData.state) - }.also { - if (it is Result.Failure) { - AppServicesStateMachineChecker.handleInternalEvent(FxaStateCheckerEvent.CallError) - } else { - AppServicesStateMachineChecker.handleInternalEvent( - FxaStateCheckerEvent.CompleteOAuthFlowSuccess, - ) - } + is Event.Progress.AuthData -> { + if (authenticationSideEffects("CompletingAuthentication:AuthData")) { + notifyObservers { onAuthenticated(account, via.authData.authType) } + refreshProfile(ignoreCache = false) + } else { + throw AccountManagerException.AuthenticationSideEffectsFailed() } } - val finalize = suspend { - // Note: finalizeDevice state checking happens in the DeviceConstellation.kt - withServiceRetries(logger, MAX_NETWORK_RETRIES) { finalizeDevice(via.authData.authType) } - } - // If we can't 'complete', we won't run 'finalize' due to short-circuiting. - if (completeAuth() is Result.Failure || finalize() !is ServiceResult.Ok) { - Event.Progress.FailedToCompleteAuth - } else { - Event.Progress.CompletedAuthentication(via.authData.authType) + is Event.Account.AuthenticationError, Event.Account.AccessTokenKeyError -> { + // Clear our access token cache; it'll be re-populated as part of the + // regular state machine flow. + SyncAuthInfoCache(context).clear() + // Should we also call authenticationSideEffects here? + // (https://bugzilla.mozilla.org/show_bug.cgi?id=1865086) + notifyObservers { onAuthenticated(account, AuthType.Recovered) } + refreshProfile(ignoreCache = true) } + else -> Unit } - else -> null - } - ProgressState.RecoveringFromAuthProblem -> { - via as Event.Account.AuthenticationError - // Somewhere in the system, we've just hit an authentication problem. - // There are two main causes: - // 1) an access token we've obtain from fxalib via 'getAccessToken' expired - // 2) password was changed, or device was revoked - // We can recover from (1) and test if we're in (2) by asking the fxalib - // to give us a new access token. If it succeeds, then we can go back to whatever - // state we were in before. Future operations that involve access tokens should - // succeed. - // If we fail with a 401, then we know we have a legitimate authentication problem. - logger.info("Hit auth problem. Trying to recover.") - - // Ensure we clear any auth-relevant internal state, such as access tokens. - account.authErrorDetected() - - // Circuit-breaker logic to protect ourselves from getting into endless authorization - // check loops. If we determine that application is trying to check auth status too - // frequently, drive the state machine into an unauthorized state. - if (via.errorCountWithinTheTimeWindow >= AUTH_CHECK_CIRCUIT_BREAKER_COUNT) { - crashReporter?.submitCaughtException( - AccountManagerException.AuthRecoveryCircuitBreakerException(via.operation), - ) - logger.warn("Unable to recover from an auth problem, triggered a circuit breaker.") - - Event.Progress.FailedToRecoverFromAuthenticationProblem - } else { - // Since we haven't hit the circuit-breaker above, perform an authorization check. - // We request an access token for a "profile" scope since that's the only - // scope we're guaranteed to have at this point. That is, we don't rely on - // passed-in application-specific scopes. - when (account.checkAuthorizationStatus(SCOPE_PROFILE)) { - true -> { - logger.info("Able to recover from an auth problem.") - - // And now we can go back to our regular programming. - Event.Progress.RecoveredFromAuthenticationProblem - } - null, false -> { - // We are either certainly in the scenario (2), or were unable to determine - // our connectivity state. Let's assume we need to re-authenticate. - // This uncertainty about real state means that, hopefully rarely, - // we will disconnect users that hit transient network errors during - // an authorization check. - // See https://github.com/mozilla-mobile/android-components/issues/3347 - logger.info("Unable to recover from an auth problem, notifying observers.") - - Event.Progress.FailedToRecoverFromAuthenticationProblem - } - } + FxaState.AuthIssues -> { + SyncAuthInfoCache(context).clear() + notifyObservers { onAuthenticationProblems() } } + else -> Unit } } - /** - * Side-effects matrix. Defines non-pure operations that must take place for state+event combinations. - */ - @Suppress("ComplexMethod", "ReturnCount", "ThrowsCount", "NestedBlockDepth", "LongMethod") - private suspend fun stateActions(forState: State, via: Event): Event? = when (forState) { - // We're about to enter a new state ('forState') via some event ('via'). - // States will have certain side-effects associated with different event transitions. - // In other words, the same state may have different side-effects depending on the event - // which caused a transition. - // For example, a "NotAuthenticated" state may be entered after a logout, and its side-effects - // will include clean-up and re-initialization of an account. Alternatively, it may be entered - // after we've checked local disk, and didn't find a persisted authenticated account. - is State.Idle -> try { - accountStateSideEffects(forState, via) - null - } catch (_: AccountManagerException.AuthenticationSideEffectsFailed) { - Event.Account.Logout - } - is State.Active -> internalStateSideEffects(forState, via) - } - private suspend fun resetAccount() { - // Clean up internal account state and destroy the current FxA device record (if one exists). - // This can fail (network issues, auth problems, etc), but nothing we can do at this point. - account.disconnect() - // Clean up resources. profile = null // Delete persisted state. @@ -920,4 +712,52 @@ open class FxaAccountManager( accountManager.syncStatusObserverRegistry.notifyObservers { onError(error) } } } + + /** + * Hook this up to the secret debug menu to simulate a network error + * + * Typical usage is: + * - `adb logcat | grep fxa_client` + * - Trigger this via the secret debug menu item. + * - Watch the logs. You should see the client perform a call to `get_profile', see a + * network error, then recover. + * - Check the UI, it should be in an authenticated state. + */ + public fun simulateNetworkError() { + account.simulateNetworkError() + account.processEvent(FxaEvent.CallGetProfile) + } + + /** + * Hook this up to the secret debug menu to simulate a temporary auth error + * + * Typical usage is: + * - `adb logcat | grep fxa_client` + * - Trigger this via the secret debug menu item. + * - Watch the logs. You should see the client perform a call to `get_profile', see an + * auth error, then recover. + * - Check the UI, it should be in an authenticated state. + */ + public fun simulateTemporaryAuthTokenIssue() { + account.simulateTemporaryAuthTokenIssue() + SyncAuthInfoCache(context).clear() + account.processEvent(FxaEvent.CallGetProfile) + } + + /** + * Hook this up to the secret debug menu to simulate an unrecoverable auth error + * + * Typical usage is: + * - `adb logcat | grep fxa_client` + * - Trigger this via the secret debug menu item. + * - Initiaite a sync, or perform some other action that requires authentication. + * - Watch the logs. You should see the client perform a call to `get_profile', see an + * auth error, then fail to recover. + * - Check the UI, it should be in an authentication problems state. + */ + public fun simulatePermanentAuthTokenIssue() { + account.simulatePermanentAuthTokenIssue() + SyncAuthInfoCache(context).clear() + account.processEvent(FxaEvent.CallGetProfile) + } } diff --git a/fenix/app/src/main/java/org/mozilla/fenix/settings/SyncDebugFragment.kt b/fenix/app/src/main/java/org/mozilla/fenix/settings/SyncDebugFragment.kt index b7c2113ed061..a8f02f914d74 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/settings/SyncDebugFragment.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/settings/SyncDebugFragment.kt @@ -10,6 +10,7 @@ import androidx.preference.Preference import androidx.preference.Preference.OnPreferenceClickListener import androidx.preference.PreferenceFragmentCompat import org.mozilla.fenix.R +import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar import kotlin.system.exitProcess @@ -55,6 +56,24 @@ class SyncDebugFragment : PreferenceFragmentCompat() { exitProcess(0) } } + requirePreference(R.string.pref_key_sync_debug_network_error).let { pref -> + pref.onPreferenceClickListener = OnPreferenceClickListener { + requireComponents.backgroundServices.accountManager.simulateNetworkError() + true + } + } + requirePreference(R.string.pref_key_sync_debug_temporary_auth_error).let { pref -> + pref.onPreferenceClickListener = OnPreferenceClickListener { + requireComponents.backgroundServices.accountManager.simulateTemporaryAuthTokenIssue() + true + } + } + requirePreference(R.string.pref_key_sync_debug_permanent_auth_error).let { pref -> + pref.onPreferenceClickListener = OnPreferenceClickListener { + requireComponents.backgroundServices.accountManager.simulatePermanentAuthTokenIssue() + true + } + } updateMenu() } diff --git a/fenix/app/src/main/res/values/preference_keys.xml b/fenix/app/src/main/res/values/preference_keys.xml index 6c161555677b..0f11f9e3d6e5 100644 --- a/fenix/app/src/main/res/values/preference_keys.xml +++ b/fenix/app/src/main/res/values/preference_keys.xml @@ -52,6 +52,9 @@ pref_key_override_sync_tokenserver pref_key_override_push_server pref_key_sync_debug_quit + pref_key_sync_debug_network_error + pref_key_sync_debug_temporary_auth_error + pref_key_sync_debug_permanent_auth_error pref_key_customize pref_key_private_browsing pref_key_leakcanary diff --git a/fenix/app/src/main/res/values/static_strings.xml b/fenix/app/src/main/res/values/static_strings.xml index 6432c5bedda1..07a8ff31d323 100644 --- a/fenix/app/src/main/res/values/static_strings.xml +++ b/fenix/app/src/main/res/values/static_strings.xml @@ -64,6 +64,12 @@ Stop Firefox Custom server changes will take effect on the next Firefox run. + + Simulate FxA network error + + Simulate FxA temporary auth error + + Simulate FxA permanent auth error Enable Tabs Tray to Compose rewrite diff --git a/fenix/app/src/main/res/xml/sync_debug_preferences.xml b/fenix/app/src/main/res/xml/sync_debug_preferences.xml index bb3ee3b634d9..c6bd98e69ac3 100644 --- a/fenix/app/src/main/res/xml/sync_debug_preferences.xml +++ b/fenix/app/src/main/res/xml/sync_debug_preferences.xml @@ -25,4 +25,13 @@ android:summary="@string/preferences_sync_debug_quit_button_summary" android:icon="@drawable/ic_close" app:isPreferenceVisible="false" /> + + +