Closed Bug 982241 Opened 6 years ago Closed 6 years ago

signing in again after changing password doesn't update menu panel UI

Categories

(Firefox :: Sync, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 30
Tracking Status
firefox29 + verified
firefox30 --- verified

People

(Reporter: edwong, Assigned: Gavin)

References

Details

Attachments

(1 file, 1 obsolete file)

1. setup sync in profile A
2. change password
3. launch new profile B
4. sign in with same account with the new password from step 2

result: you get in an 'signed in' screen but desktop UI says 'reconnect'.  When you reconnect, you get the same result.
methinks some form of invalid assertion is being sent to token server - not sure which component owns this bug.
Blocks: 969593
Severity: normal → critical
Assignee: nobody → ckarlof
also - there are no logs to pull out - so the failure maybe occurring before it attempts to talk to token server
Edwin, I couldn't reproduce this. Can you reproduce?
Ah, when I restart profile A, and "reconnect" the state in the hamburger menu still shows "Reconnect", but the Prefs panel shows the correct state. I don't think the hamburger menu is getting notified properly after the user logs in. Is this what you're seeing?
triaged
Severity: critical → major
Priority: -- → P1
I think that the menu items are missing an observer that the pref panel observes to know that the "reconnect" has happened.
Gavin said he'd take a look at this.
Assignee: ckarlof → gavin.sharp
actually - sync is in fact busted when i use the hamburger > sync button i get this error:

1394650489182	Sync.BrowserIDManager	ERROR	Authentication error in _fetchTokenForUser: AuthenticationError(TokenServerClientServerError({"now":"2014-03-12T18:54:49.181Z","message":"Authentication failed.","cause":"invalid-client-state","response_body":"{\"status\": \"invalid-client-state\", \"errors\": [{\"location\": \"body\", \"name\": \"\", \"description\": \"Unauthorized\"}]}","response_headers":{"content-type":"application/json; charset=UTF-8","date":"Wed, 12 Mar 2014 18:54:52 GMT","server":"nginx/1.4.4","x-timestamp":"1394650492","content-length":"111","connection":"keep-alive"},"response_status":401}))
1394650489182	Sync.Status	DEBUG	Status.login: success.login => error.login.reason.account
1394650489182	Sync.Status	DEBUG	Status.service: success.status_ok => error.login.failed
1394650489183	Sync.BrowserIDManager	ERROR	Background fetch for key bundle failed: AuthenticationError(TokenServerClientServerError({"now":"2014-03-12T18:54:49.181Z","message":"Authentication failed.","cause":"invalid-client-state","response_body":"{\"status\": \"invalid-client-state\", \"errors\": [{\"location\": \"body\", \"name\": \"\", \"description\": \"Unauthorized\"}]}","response_headers":{"content-type":"application/json; charset=UTF-8","date":"Wed, 12 Mar 2014 18:54:52 GMT","server":"nginx/1.4.4","x-timestamp":"1394650492","content-length":"111","connection":"keep-alive"},"response_status":401}))
you're welcome to use my acct which should repro the issue:
edog0@mailinator.com
pw: 123456789
Edwin: I'm seeing that too. I filed bug 982798 to cover it since it's a separate issue.

My STR:
1) Set up profile
2) Shut it down
3) Change password
4) Re-launch original profile
5) re-sign in
6) panel UI still shows "Reconnect", despite tools menu and prefs being updated correctly

What I'm seeing when re-logging in at 5) is that browser-fxaccounts' observer (http://hg.mozilla.org/mozilla-central/annotate/a1639dd9dd3d/browser/base/content/browser-fxaccounts.js#l102) is called for:

  * fxaccounts:onlogin, which calls updateUI, but loginFailed is still true when getSignedInUser returns, at http://hg.mozilla.org/mozilla-central/annotate/a1639dd9dd3d/browser/base/content/browser-fxaccounts.js#l188, so UI stays in the "need to log in" state
  * fxaccounts:onverified, which does not call updateUI, because it's handled separately: http://hg.mozilla.org/mozilla-central/annotate/a1639dd9dd3d/browser/base/content/browser-fxaccounts.js#l109

I think the observer should be calling updateUI for onverified.
Severity: major → critical
Component: Firefox Sync: Backend → Sync
Product: Mozilla Services → Firefox
OS: Mac OS X → All
Hardware: x86 → All
Summary: changing password causes new device/profile to get stuck in reauth loop → signing in again after changing password doesn't update menu panel UI
Attached patch patch (obsolete) — Splinter Review
I think this fixes the issue, hard to test at the moment given that other bug.
Severity: critical → normal
Comment on attachment 8390030 [details] [diff] [review]
patch

This patch doesn't fix it - when the "onverified" message is sent, this.loginFailed is still true (i.e. Weave.Status.login == Weave.LOGIN_FAILED_LOGIN_REJECTED).

I think this might be a bug in browserid_identity.js - I see it setting Weave.Status.login = LOGIN_SUCCEEDED in initializeWithCurrentIdentity, and I see it setting LOGIN_FAILED_LOGIN_REJECTED or LOGIN_FAILED_NETWORK_ERROR in _fetchTokenForUser, but I don't ever see it setting Weave.Status.login in a "logging back in after an auth error" code path.
Attachment #8390030 - Attachment is obsolete: true
> I don't ever see it setting Weave.Status.login in a "logging back in after an auth error" code path.

"logging back in after an auth error" calls initializeWithCurrentIdentity() again.
So the issue is that both the UI and browserid_identity are observing onlogin, so there's an ordering issue. But even ignoring that, initializeWithCurrentIdentity (which is what sets Sync's login status after the sign in) takes several spins of the event loop before it actually sets Weave.Status.login, so it's not set by the time the UI's onlogin handler is called.

Thankfully, initializeWithCurrentIdentity fires "weave:service:setup-complete" for updates that are the result of a login, so we can have the UI just use that. That fixes this issue for me.

(I filed bug 982986 about an implementation detail of browserid_identity that makes it safer for the UI to rely on that notification.)
Attached patch patchSplinter Review
Attachment #8390235 - Flags: review?(ttaubert)
Attachment #8390235 - Flags: review?(ttaubert) → review+
> This patch doesn't fix it - when the "onverified" message is sent, this.loginFailed is still true (i.e. Weave.Status.login == Weave.LOGIN_FAILED_LOGIN_REJECTED).

This makes sense. The "onverified" message probably arrives before browserid_identity.initializeWithIdentity gets around to setting Weave.Status.login = LOGIN_SUCCEEDED.
Attachment #8390235 - Flags: approval-mozilla-aurora+
Keywords: verifyme
QA Contact: twalker
Reproduced the initial issue on old Nightly from 2014-03-11, verified as fixed on Firefox 29 RC and latest Aurora.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.