Commit 8bd98b5f authored by Grisha Kruglov's avatar Grisha Kruglov
Browse files

Part 1: Do not erase account storage after hitting an auth problem

Before this patch, we'd clear out account storage after we see an unrecoverable authentication error. In this context, a password change would be an unrecoverable error. We'll transition account into an AuthenticationProblem state, but upon re-initialization of the state machine we'll end up in NotAuthenticated instead, because there's no longer a locally persisted account state. So, this is just a straight-up bug in the state machine.

So, step 1:
    do not erase account storage if we encounter auth problems we can't auto-recover from
        this will make sure account isn't disappearing after a restart
        this has an additional benefit of not erasing fetched profile data, as well! it's part of the persisted account state

Also, add in some debug logging into the samples-sync app to make testing this a bit easier.
parent 7611dfd7
......@@ -794,20 +794,9 @@ open class FxaAccountManager(
// 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, clearing state.")
logger.info("Unable to recover from an auth problem, notifying observers.")
// We perform similar actions to what we do on logout, except for destroying
// the device. We don't have valid access tokens at this point to do that!
// Clean up resources.
profile = null
account.close()
// Delete persisted state.
getAccountStorage().clear()
// Re-initialize account.
account = createAccount(serverConfig)
// Finally, tell our listeners we're in a bad auth state.
// Tell our listeners we're in a bad auth state.
notifyObservers { onAuthenticationProblems() }
}
}
......
......@@ -1232,7 +1232,9 @@ class FxaAccountManagerTest {
verify(accountObserver, times(1)).onAuthenticationProblems()
assertTrue(manager.accountNeedsReauth())
assertEquals(mockAccount, manager.authenticatedAccount())
assertNull(manager.accountProfile())
// Make sure profile is still available.
assertEquals(profile, manager.accountProfile())
// Able to re-authenticate.
reset(accountObserver)
......
......@@ -42,6 +42,7 @@ import mozilla.components.service.fxa.FxaAuthData
import mozilla.components.service.fxa.SyncEngine
import mozilla.components.service.fxa.sync.SyncReason
import mozilla.components.service.fxa.toAuthType
import mozilla.components.support.base.log.logger.Logger
import mozilla.components.support.base.log.sink.AndroidLogSink
import mozilla.components.support.rusthttp.RustHttpConfig
import mozilla.components.support.rustlog.RustLog
......@@ -89,6 +90,8 @@ class MainActivity :
const val REDIRECT_URL = "https://accounts.firefox.com/oauth/success/$CLIENT_ID"
}
private val logger = Logger("SampleSync")
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
RustLog.enable()
......@@ -237,6 +240,8 @@ class MainActivity :
lateinit var lastAuthType: AuthType
override fun onLoggedOut() {
logger.info("onLoggedOut")
launch {
val txtView: TextView = findViewById(R.id.fxaStatusView)
txtView.text = getString(R.string.logged_out)
......@@ -262,6 +267,8 @@ class MainActivity :
}
override fun onAuthenticationProblems() {
logger.info("onAuthenticationProblems")
launch {
val txtView: TextView = findViewById(R.id.fxaStatusView)
txtView.text = getString(R.string.need_reauth)
......@@ -271,6 +278,8 @@ class MainActivity :
}
override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {
logger.info("onAuthenticated")
launch {
lastAuthType = authType
......@@ -292,6 +301,8 @@ class MainActivity :
}
override fun onProfileUpdated(profile: Profile) {
logger.info("onProfileUpdated")
launch {
val txtView: TextView = findViewById(R.id.fxaStatusView)
txtView.text = getString(
......@@ -305,12 +316,14 @@ class MainActivity :
private val syncObserver = object : SyncStatusObserver {
override fun onStarted() {
logger.info("onSyncStarted")
CoroutineScope(Dispatchers.Main).launch {
syncStatus?.text = getString(R.string.syncing)
}
}
override fun onIdle() {
logger.info("onSyncIdle")
CoroutineScope(Dispatchers.Main).launch {
syncStatus?.text = getString(R.string.sync_idle)
......@@ -344,6 +357,7 @@ class MainActivity :
}
override fun onError(error: Exception?) {
logger.error("onSyncError", error)
CoroutineScope(Dispatchers.Main).launch {
syncStatus?.text = getString(R.string.sync_error, error)
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment