1456487 - Cannot re-connect Firefox Account after "Clear data" in Android Settings

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Firefox Accounts, defect)

Description

[Oana Horvath [:ohorvath]]

Device:
Huawei MediaPad M3 Lite 10 (Android 7.0)
Huawei P8 Lite (Android 6.0)

Build: Nightly 61.0a1 (2018-04-24);

Steps to reproduce:
1. Sign in to Sync with a Firefox account.
2. Clear app data from Android settings>Apps>Firefox Nightly>Storage>Clear data.
3. Open the browser and go to Settings>Sign in.
4. Log in again with the same account.

Expected result:
The sync account should not be saved when clearing the app data. 
If the username is saved though, the user should be able to log back in.

Actual result:
The username is saved in Sync, but the user is not logged in. An error is displayed: "Cannot connect. Tap to sign in."
Tapping the message and trying to sign in, you get a success message that the account is connected.
Going back to settings>sync, the account is still not connected, the same error is displayed.
Can only sign in after the previous account is disconnected.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

[Tracking Requested - why for this release]: worth considering this a blocker for 60. This is not a good state for the user to try to recover from.

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Kevin Brosnan [:kbrosnan] from comment #1)
> [Tracking Requested - why for this release]: worth considering this a
> blocker for 60. This is not a good state for the user to try to recover from.

In the temporary absence of Grisha, I will do the first round of investigation here.  Based on Oana's STRs, it rather sounds like the Status Activity is displaying stale information.  This is generally because a sync is needed to drive the state machine forward; it might be that we need to force a sync more aggressively when we witness part of this flow.

Comment 3

[Julien Cristau [:jcristau]]

Marking as blocker per Kevin's comment.

Comment 4

[Nick Alexander :nalexander [he/him]]

Installed

http://archive.mozilla.org/pub/mobile/nightly/2018/04/2018-04-25-10-01-18-mozilla-central-android-api-16/fennec-61.0a1.multi.android-arm.apk

Signed in to a freshly created test account; complete verification loop.

Tapped Settings > Sync; verify that account has synced; tapped Sync Now; verify that account syncs again.

Go to Settings > Storage.  Tap Clear Data.

Go to Settings > Accounts.  Observe that test account is still listed as a Firefox Account!

Kill and restart Nightly.  Observe in the logcat:
04-25 10:43:44.348 31116 31200 I FxAccounts: fennec_aurora :: FirefoxAccountUtils :: Pickle file absent; separating the account.
04-25 10:43:44.352 31116 31200 I FxAccounts: fennec_aurora :: AndroidFxAccount :: Moving account named like [e-mail] to state Separated
04-25 10:43:44.378 31116 31200 I FxAccounts: fennec_aurora :: FxAccountNotificationManager :: State Separated needs action; offering notification with title: Sync is not connected
04-25 10:43:44.407 31116 31200 I FxAccounts: fennec_aurora :: FxAccountSyncAdapter :: Syncing in a 'first run' scenario without a pickle file; skipping sync & separating the account.
04-25 10:43:44.417 31116 31116 I GeckoLogger: fennec_aurora :: FxAccountNotificationManager :: State Separated needs action; offering notification with title: Sync is not connected
04-25 10:43:44.434 31116 31128 I GeckoLogger: fennec_aurora :: FxAccountNotificationManager :: State Separated needs action; offering notification with title: Sync is not connected
04-25 10:43:49.527 31116 31188 D GeckoFxAccounts: FxAccountsWebChannel registered: account_updates with origin https://accounts.firefox.com
04-25 10:43:49.870 31116 31188 I GeckoLogger: fennec_aurora :: FxAccountNotificationManager :: State Separated needs action; offering notification with title: Sync is not connected
04-25 10:43:51.361 31116 31181 I GeckoPushService: FxA account not in Married state, not proceeding with registration renewal

Go to Settings > Sync; observe error with "Cannot connect.  Tap to sign in."

Tap error; go to about:accounts; observe user name is filled in.  Entered password, transitioned to "Signed in" screen in about:accounts.

Go to Settings > Sync; observe error with "Cannot connect.  Tap to sign in."  Also, "Last synchronized: never."  In the logcat, I observe:

04-25 10:49:53.037 31116 31188 I GeckoLogger: fennec_aurora :: AndroidFxAccount :: Moving account named like [e-mail] to state Engaged
04-25 10:49:53.074 31116 31188 I GeckoFxAccounts: Created or updated Firefox Account.

but then immediately:

04-25 10:50:16.099 31116 31127 I GeckoLogger: fennec_aurora :: AndroidFxAccount :: Moving account named like [e-mail] to state Separated
04-25 10:50:16.146 31116  1704 I FxAccounts: fennec_aurora :: FxAccountSyncAdapter :: Syncing FxAccount account named like [e-mail] for authority org.mozilla.fennec_aurora.db.browser with instance org.mozilla.gecko.fxa.sync.FxAccountSyncAdapter@3be90f6.

So in some way, the sign in is moving the account to Engaged (correct!) but we then immediately move it to Separated (incorrect!).  My guess is that we're doing that on the basis of the "first run flagging" logic that Grisha relatively recently implemented to handle this situation.  My belief is that we assumed that "Clear data" would kill and restart the App, but locally that seems to not be true.  (I'll confirm or deny.)

In any case, I can confirm the poor experience that Oana filed (thanks, Oana!).

Comment 5

[Nick Alexander :nalexander [he/him]]

OK, a little more investigation.  It does appear that Clear data is killing and restarting the App (phew!).  The "Clear data" flow is being recognized, and the Firefox Account is being moved to Separated based on the mismatched first-run UUIDs.  The issue is that reconnecting the account does not update the first-run UUID, so that the next Sync-related action immediately moves it back to Separated.  That is, we only set the first-run UUID when adding the account, not when reconnecting it.

It's not clear to me exactly what the entry points for "reconnected are"; I'll try to track those through now.

Comment 6

[Nick Alexander :nalexander [he/him]]

Patch inbound.  I believe the work-around is for the user to start, and then kill the App entirely, start again, and _then_ re-connect the Account.  That should get us out of the First Run session logic that is causing this cycle.

Comment 7

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1456487 - Update Firefox Account's first run UUID when re-connecting.

The behaviour of Android Firefox Account instances recently changed in
the face of system "Clear data" commands.  To align more closely with
common Apps like Dropbox and Whatsapp (which generally don't use
Android Account instances), after a "Clear data" Firefox Account's are
moved to the Separated state, requiring the user to re-connect them
with a password challenge.  To achieve this, newly created accounts
include a first run UUID; after a "Clear data", the App is killed and
restarted, Sync sees a different first run UUID, and the Account is
moved to the Separated state.  (I honestly don't know what happens if
the Sync code never sees a different first run UUID, but that's for
another day.)  If the user then, in the same first run session,
re-connects the Firefox Account... the Sync code will again see the
different first run UUID and move the Account to the Separated state.

This patch updates the first run UUID when the Account is
re-connected, breaking that cycle.

Review commit: https://reviewboard.mozilla.org/r/239738/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/239738/

Comment 8

[Richard Newman [:rnewman]]

Comment on attachment 8971001 [details]
Bug 1456487 - Update Firefox Account's first run UUID when re-connecting.

https://reviewboard.mozilla.org/r/239738/#review245474

::: commit-message-88abd:6
(Diff revision 1)
> +Bug 1456487 - Update Firefox Account's first run UUID when re-connecting. r=rnewman
> +
> +The behaviour of Android Firefox Account instances recently changed in
> +the face of system "Clear data" commands.  To align more closely with
> +common Apps like Dropbox and Whatsapp (which generally don't use
> +Android Account instances), after a "Clear data" Firefox Account's are

Account's -> Accounts

::: mobile/android/services/src/main/java/org/mozilla/gecko/fxa/authenticator/AndroidFxAccount.java:652
(Diff revision 1)
>  
> +  /**
> +   * If there's an active First Run session, tag the given account with it.  If there's no active
> +   * First Run session, remove any existing tag.
> +   * We do this in order to reliably determine if an account was created during the current
> +   * "first run"; this allows us to re-connect an account that was no created during the current

no -> not

Comment 9

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8971001 [details]
Bug 1456487 - Update Firefox Account's first run UUID when re-connecting.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/239738/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df36fee2b3ac
Update Firefox Account's first run UUID when re-connecting. r=rnewman

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Once this lands we should verify on nightly and consider uplift to beta 60.

Comment 12

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #6)
> Patch inbound.  I believe the work-around is for the user to start, and then
> kill the App entirely, start again, and _then_ re-connect the Account.  That
> should get us out of the First Run session logic that is causing this cycle.

Oana, could you confirm both the fix when it gets to Nightly, and the work-around for affected versions?  Thanks!

Comment 13

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/df36fee2b3ac

Comment 14

[Oana Horvath [:ohorvath]]

(In reply to Nick Alexander :nalexander from comment #12)
> (In reply to Nick Alexander :nalexander from comment #6)
> > Patch inbound.  I believe the work-around is for the user to start, and then
> > kill the App entirely, start again, and _then_ re-connect the Account.  That
> > should get us out of the First Run session logic that is causing this cycle.
> 
> Oana, could you confirm both the fix when it gets to Nightly, and the
> work-around for affected versions?  Thanks!

Verified the fix in Nightly 61 (2018-04-26), works as expected: the user can reconnect after a clear data.
That workaround doesn't seem to work on Beta 60.b15. However, there is a simple workaround that works: to disconnect an re-connect the account from the Sync menu.

Devices:
Pixel XL (Android 8.1.0)
Asus Nexus 7 (Android 5.1.1)

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Nick, can you please nominate this for uplift to Beta60? Thanks!

Comment 16

[Nick Alexander :nalexander [he/him]]

(In reply to Ritu Kothari (:ritu) from comment #15)
> Hi Nick, can you please nominate this for uplift to Beta60? Thanks!

Yes.  Per discussion with lizzard, we have the Nightly verification we wanted, although I don't fully understand why the simpler work-around I proposed doesn't work.  I'm not going to worry about it right now, and will nominate.

Comment 17

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8971001 [details]
Bug 1456487 - Update Firefox Account's first run UUID when re-connecting.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1429735.

[User impact if declined]: after Clear data, the user needs to disconnect their Sync account (delete it entirely) and reconnect it before they can start syncing again.  This is a burden but _should_ not cause visible issues to the user other than polluting their list of devices to which they can Send Tab.  This pollution is because internally the device re-syncs *everything* and starts fresh, which is both hard on our service (lots of downloads) and very likely to actually cause visible issues (since Sync 1.5 is just not that robust corruption and bad merges are likely).

[Is this code covered by automated tests?]:

No.

[Has the fix been verified in Nightly?]:

Yes.

[Needs manual test from QE? If yes, steps to reproduce]: 

No, I don't think additional testing on Beta would be that useful -- the code in this area is relatively stable.

[List of other uplifts needed for the feature/fix]:

None that I am aware of.

[Is the change risky?]:

Not really.

[Why is the change risky/not risky?]:

The situation is more or less maximally broken, and the proposed fix impacts exactly one flow -- when we reconnect the Account from about:accounts.  Any additional breakage their would both be visible (since it's in response to user action) and unlikely (since the change is minimal).

[String changes made/needed]:

None.

Comment 18

[Julien Cristau [:jcristau]]

Comment on attachment 8971001 [details]
Bug 1456487 - Update Firefox Account's first run UUID when re-connecting.

FxA fix, approved for 60.0b17 and 60.0 rc

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4bdbddf15a01 (FIREFOX_60b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/a803056762bb

Comment 20

[Oana Horvath [:ohorvath]]

Verified on Beta 60.0b17:
Google Pixel (Android 8.1.0)
Sony Xperia Z5 Premium (Android 6.0.1)