1792553 - User disconnected from FxA after using the Try Again button and entering the correct primary password from Firefox View

Status: RESOLVED DUPLICATE of bug 1792550

(Firefox :: Firefox View, defect, P1)

Description

[Bogdan Maris, Desktop QA]

Attachment: Archive.zip

Found in

• Firefox 106.0b4

Affected versions

• Firefox 106.0b4
• Latest Nightly 107.0a1

Tested platforms

• Affected platforms: macOS 11, Windows 10, Ubuntu 18.04

Steps to reproduce

1. Set a primary password
2. Restart Firefox
3. Login to FxA
4. Dismiss the primary password modal (if prompted)
5. Restart Firefox
6. Dismiss the primary password modal (if prompted)
7. Notice that the user is still logged in to FxA
8. Go to Firefox View and click Try Again
9. Enter the correct primary password

Expected result

• User is logged in to FxA and Tab Pickup is populated with a few websites

Actual result

• User is disconnected from FxA and Tab Pickup is empty

Regression range

• This behaviour is from the time the Try Again button was implemented in Bug 1787619 so it is not a regression.

Additional notes

• I had 4 files in about:sync-log so I archived all of them and attached them here.
• Here are the logs from browser console:

1664281966813	addons.xpi	WARN	Checking /Users/bogdan.maris/Applications/Firefox.app/Contents/Resources/distribution/extensions for addons
b is null mozjexl.js:1
NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsITelemetry.getHistogramById] 3 TerminatorTelemetry.jsm:96
BackgroundUpdate: _reasonsToNotScheduleUpdates: Failed to check for Maintenance Service Registry Key: [Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIUpdateProcessor.getServiceRegKeyExists]"  nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame :: resource://gre/modules/BackgroundUpdate.jsm :: _reasonsToNotScheduleUpdates :: line 250"  data: no] BackgroundUpdate.jsm:252
NS_ERROR_ABORT: User canceled primary password entry crypto-SDR.js:91
Sync encountered an error - see about:sync-log for the log file. policies.js:975
Referrer Policy: Ignoring the less restricted referrer policy “no-referrer-when-downgrade” for the cross-site request: https://us.creativecdn.com/tags?id=pr_XPHppWVQSlTBR2sZU22q_home tags
<empty string>
Referrer Policy: Ignoring the less restricted referrer policy “origin-when-cross-origin” for the cross-site request: https://42b5c68e284494372a16e3c60fe74068.safeframe.googlesyndication.com/safeframe/1-0-38/html/container.html 6 container.html
Referrer Policy: Ignoring the less restricted referrer policy “origin-when-cross-origin” for the cross-site request: https://login.microsoftonline.com/common/oauth2/authorize?client_id=9ea1ad79-fdb6-4f9a-8bc3-2b70f96e34c7&response_type=id_token+code&nonce=d9735e66-398e-4dbb-aeff-71e72a2affd0&redirect_uri=https%3a%2f%2fwww.bing.com%2forgid%2fidtoken%2fconditional&scope=openid&response_mode=form_post&instance_aware=true&msafed=0&prompt=none&state=%7b%22ig%22%3a%22BF90D0755D22412186E3DFF63A1CE322%22%7d authorize
1664282002257	FirefoxView.TabsSetup	DEBUG	TabsSetupFlowManager constructor, fxaSignedIn:: true
1664282002257	FirefoxView.TabsSetup	DEBUG	onSignedInChange, fxaSignedIn:: true
1664282002257	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, conditions not met to exit state: : error-state
1664282002257	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, will notify update?:: true
1664282002257	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, in error state:: sync-error
1664282002260	FirefoxView.TabsSetup	DEBUG	refreshDevices, mobileDeviceConnected: false, : secondaryDeviceConnected: true
1664282002260	FirefoxView.TabsSetup	DEBUG	refreshDevices: no device state change
1664282002260	FirefoxView.TabsSetup	DEBUG	onSignedInChange, tabSyncNeeded:: true
1664282002260	FirefoxView.TabsSetup	DEBUG	isPrimaryPasswordLocked:: true
1664282002260	FirefoxView.TabsSetup	DEBUG	onSignedInChange, no recentTabs, calling syncTabs
1664282002260	FirefoxView.TabsSetup	DEBUG	onSignedInChange, no tab sync expected
1664282053503	FirefoxView.TabsSetup	DEBUG	tryToClearError: triggering new tab sync
1664282073066	FirefoxView.TabsSetup	DEBUG	Handling passwordmgr-crypto-login
1664282073068	FirefoxView.TabsSetup	DEBUG	Handling UIState update
1664282073068	FirefoxView.TabsSetup	DEBUG	onSignedInChange, fxaSignedIn:: false
1664282073068	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, conditions not met to exit state: : not-signed-in
1664282073068	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, will notify update?:: true
Skipping device list refresh; not signed in browser-sync.js:618:15
1664282073111	FirefoxView.TabsSetup	DEBUG	tryToClearError: triggering new tab sync
1664282073113	FirefoxView.TabsSetup	DEBUG	Handling UIState update
1664282073113	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, conditions not met to exit state: : not-signed-in
1664282073113	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, will notify update?:: false
Skipping device list refresh; not signed in browser-sync.js:618:15
Sync encountered an error - see about:sync-log for the log file. 2 policies.js:975

Comment 1

[:Gijs (he/him)]

Mark/Sammy, any idea what's happening here?

Comment 2

[Mark Hammond [:markh] [:mhammond]]

As I noted in bug 1792550 comment 5, I can't reproduce this.

Comment 3

[Bogdan Maris, Desktop QA]

As also replied in bug 1792550 I can still reproduce this issue using latest Nightly 107.0a1 (buildID: 20221003212025) from today.

Comment 4

[Bogdan Maris, Desktop QA]

Here is a video showing the issue, it was too large to upload it here so I added it to Mozilla Gdrive: https://drive.google.com/file/d/1fwL0LSYzutflw_rkntHlNdwV-FSX5aZw/view?usp=sharing

Comment 5

[Tim Giles [:tgiles]]

I'm able to reproduce the issue by doing the following:

• Being signed into your Firefox Account on a different Firefox and ensuring you have synced that account once.
  • As far as I understand it, this will force the prompts in Steps 4 and 6 almost immediately
• Then followed the rest of the steps in Comment #0.

I'm not sure how we're hitting the "error.login.reason.account" when you successfully authenticate through your primary password though. It seems like _loginFailed is always returning true because lazy.Weave.Status.login is equal to "error.login.reason.account". I don't know where we change the value for lazy.Weave.Status.login but I'm guessing cancelling the previous primary password prompts is causing an error state to be permanent or something.

So before successfully authenticating with primary password, lazy.Weave.Status.login is "service.master_password_locked". As soon as I successfully authenticate with primary password and before _loginFailed has a chance to execute, lazy.Weave.Status.login is set to "error.login.reason.account".

Okay, I think I have some better information here. So it seems like we're failing in unlockAndVerityAuthState which according to the comment means I probably authenticated without unlocking the primary password or I cleared my saved logins. So then, because unlockAndVerifyAuthState() failed, then verifyLogin in service.js returns false. So somehow we've gotten into a state where we can't get the key for the given scope after we successfully authenticate with primary password. I stepped through canGetKeyForScope in FxAccountsKeys.jsm and hit the info message of "Can't get keys; no key material or tokens available". Not sure where to go from here, but happy to try other paths to get more information

Comment 6

[Tim Giles [:tgiles]]

(In reply to Tim Giles [:tgiles] from comment #5)

Okay, I think I have some better information here. So it seems like we're failing in unlockAndVerityAuthState which according to the comment means I probably authenticated without unlocking the primary password or I cleared my saved logins.

:markh knows the sync codebase more than I do, and this "probably authenticated without unlocking the primary password" lines up with his comment on Bug 1792550, so it seems like duping this bug to Bug 1792550 would be the way to go :)

The problem is that we can't store the FxA credentials in this state, so Sync doesn't think it is logged in, and upon restarting we find no credentials in the login manager and force signing back in - this process will repeat until the user unlocks.

Comment 7

[Mark Hammond [:markh] [:mhammond]]

Yeah, this is quite messy. In general, having the primary-password locked when authenticating is going to be a problem and cause error.login.reason.account once unlocked. However, assuming the tokens and key material are actually in the login store, then starting the browser and not unlocking tends to end up in the service.master_password_locked state and work fine once unlocked. I added a patch to bug 1792550 which aims to prevent you signing in or re-authenticating while the PP is locked to avoid this state in most cases. I'd welcome feedback on that though - the patch prompts for the PP but if canceled, we just stop the login process without any indication to the user why the login isn't happening. I think that's probably OK, but in the absence of an obvious UX resource for this I welcome all feedback.

Comment 8

[:Gijs (he/him)]

(In reply to Mark Hammond [:markh] [:mhammond] from comment #7)

Yeah, this is quite messy. In general, having the primary-password locked when authenticating is going to be a problem and cause error.login.reason.account once unlocked. However, assuming the tokens and key material are actually in the login store, then starting the browser and not unlocking tends to end up in the service.master_password_locked state and work fine once unlocked. I added a patch to bug 1792550 which aims to prevent you signing in or re-authenticating while the PP is locked to avoid this state in most cases. I'd welcome feedback on that though - the patch prompts for the PP but if canceled, we just stop the login process without any indication to the user why the login isn't happening. I think that's probably OK, but in the absence of an obvious UX resource for this I welcome all feedback.

Is there going to be anything left to fix here with the patch for 1792550 landed (which makes sense to me, fwiw) ?

I think once bug 1792680 is fixed, a refresh and/or clicking the main "try again" button on the page would allow re-triggering the PP dialog if initially cancelled.

Comment 9

[Mark Hammond [:markh] [:mhammond]]

(In reply to :Gijs (he/him) from comment #8)

Is there going to be anything left to fix here with the patch for 1792550 landed (which makes sense to me, fwiw) ?

Nope, I think that patch covers both bugs, so I'll mark this as a dupe of that.