User disconnected from FxA after using the Try Again button and entering the correct primary password from Firefox View
Categories
(Firefox :: Firefox View, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr102 | --- | unaffected |
firefox105 | --- | unaffected |
firefox106 | --- | affected |
firefox107 | --- | affected |
People
(Reporter: bmaris, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [fidefe-2022-mr1-firefox-view])
Attachments
(1 file)
(deleted),
application/zip
|
Details |
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
- Set a primary password
- Restart Firefox
- Login to FxA
- Dismiss the primary password modal (if prompted)
- Restart Firefox
- Dismiss the primary password modal (if prompted)
- Notice that the user is still logged in to FxA
- Go to Firefox View and click Try Again
- 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
Reporter | ||
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Comment 1•2 years ago
|
||
Mark/Sammy, any idea what's happening here?
Updated•2 years ago
|
Comment 2•2 years ago
|
||
As I noted in bug 1792550 comment 5, I can't reproduce this.
Reporter | ||
Comment 3•2 years ago
|
||
As also replied in bug 1792550 I can still reproduce this issue using latest Nightly 107.0a1 (buildID: 20221003212025) from today.
Reporter | ||
Comment 4•2 years ago
|
||
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•2 years ago
|
||
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•2 years ago
|
||
(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•2 years ago
|
||
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•2 years ago
|
||
(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 theservice.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.
Updated•2 years ago
|
Updated•2 years ago
|
Comment 9•2 years ago
|
||
(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.
Description
•