Closed Bug 1397537 Opened 7 years ago Closed 7 years ago

Desktop fails to check verification status when there's no session token

Categories

(Firefox :: Firefox Accounts, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: markh, Assigned: markh)

Details

Attachments

(1 file)

I'm not sure *why* I haven't got a session token yet, but when Firefox gets into that state for an unverified user it gets stuck. But to reproduce, create a new account, exit the browser before verification, edit signedInUser.json and remove the session token, then restart. If the patch in bug 1397530 is applied, you should see:

> 1504741130013   FirefoxAccounts DEBUG   startVerifiedCheck: false
> 1504741130013   FirefoxAccounts ERROR   checkEmailStatus failed to poll: Error: checkEmailStatus called without a session token (resource://gre/modules/FxAccounts.jsm:473:29) JS Stack trace: checkEmailStatus@FxAccounts.jsm:473:29 < pollEmailStatus@FxAccounts.jsm:1199:30 ....

The only way to recover from this is to disconnect and reconnect the account.

(This is tricky - the only way to get another session token is to log back in (ie, to re-enter your credentials) - which we typically handle in browserid_identity to force the UI into a "needs reauth" state. However, we can never enter this state for an unverified user. I suspect we'll want to change the UI code so that where it checks if the user is unverified, it also needs to check if we have a session token and say "needs reauth" instead of "needs verification")
This patch would possibly have been simpler if we just checked data.sessionToken directly, although that seems to be relying too much on internal state and IMO would make the general FxA ugliness worse. With this patch a user in that strange state is able to recover by re-entering their FxA password. There's still lots of log noise about the session token missing, and while that's not ideal I don't think we should make even more intrusive changes at this point.
Assignee: nobody → markh
Status: NEW → ASSIGNED
Comment on attachment 8905314 [details]
Bug 1397537 - check FxA has a session token and enter a needs-reauth state even when the user is unverified.

https://reviewboard.mozilla.org/r/177112/#review182360

That is fine with me.

::: services/fxaccounts/FxAccounts.jsm:888
(Diff revision 1)
> +   *        the account status, you should first call this method, and if this
> +   *        returns true, you should then call sessionStatus() to check with
> +   *        the server.
> +   */
> +  async hasLocalSession() {
> +    let data = await this.getSignedInUser();

It's unfortunate that we are going to end up calling getSignedInUser twice, but that's the price of abstraction. I wonder if we could have added an additional property to the object it returns and remove hasLocalSession alltogether. Anyway not an issue!
Attachment #8905314 - Flags: review?(eoger) → review+
Pushed by mhammond@skippinet.com.au:
https://hg.mozilla.org/integration/autoland/rev/def8963114c3
check FxA has a session token and enter a needs-reauth state even when the user is unverified. r=eoger
https://hg.mozilla.org/mozilla-central/rev/def8963114c3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Product: Core → Firefox
Target Milestone: mozilla57 → Firefox 57
You need to log in before you can comment on or make changes to this bug.