First use of privileged app w/ FxA triggers redundant refreshAuthentication



4 years ago
11 months ago


(Reporter: spenrose, Unassigned)


Firefox Tracking Flags

(Not tracked)


(Whiteboard: [dependency: marketplace-partners])


(2 attachments)



4 years ago
Created attachment 8519225 [details]


1) Install a privileged app such as attached into an affected build of FxOS.
2) While not logged into FxA, open the app. The app will (correctly), invoke FxA's login flow.
3) Sign in with a verified account.

Expected: app displays white screen with some fxa events logged.

4) Actual: FxA throws up second password entry screen.

  - seen on 20141106000201 (Flame)
  - not seen on v188/20141016183911 (Flame)
To add to the Actual flow:
After the second password entry screen, onerror is always fired rather than onlogin.
Possible Dupe of bug 1052267

Also: Please see as an example of how to format a bug - you are missing some very important key information - namely what branch you are testing on

Also: We do not do regression windows on non-blocking bugs - please see for information on when to add this tag
Keywords: regressionwindow-wanted
Blocks: 1095816
Whiteboard: [dependency: marketplace-partners]
We haven't been seeing this since.  Closing as invalid.  This was on 2.0.
Last Resolved: 4 years ago
Resolution: --- → INVALID
Nevermind.  Seeing this today, with the test case after the call to navigator.mozId.request.

Serial: 7f86ca16 (State: device)
Build ID               20141118000207
Gaia Revision          1ede2666f1e6c1b3fd3b282011caf0cbc59544b0
Gaia Date              2014-11-17 17:03:27
Gecko Revision
Gecko Version          32.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20141118.035009
Firmware Date          Tue Nov 18 03:50:19 EST 2014
Bootloader             L1TC00011880
Resolution: INVALID → ---
I just had the same behaviour testing WMW:

Serial: e444da0c (State: device)
Build ID               20141120000206
Gaia Revision          1ede2666f1e6c1b3fd3b282011caf0cbc59544b0
Gaia Date              2014-11-17 17:03:27
Gecko Revision
Gecko Version          32.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20141120.034342
Firmware Date          Thu Nov 20 03:43:52 EST 2014
Bootloader             L1TC10011880
Duplicate of this bug: 1105449

Comment 7

4 years ago
The bug essentially amounts to "FxAccountsManager.getAssertion(), which is what mozId.request() boils down to, was called a second time with the same principal (app) and reached here:

before the first call reached here:

Nick observes in IRC that one factor may be whether the first call was from a verified account. If it wasn't, FxAccountsManager would never call _getAssertion, which means the app would never get added to the security manager. The fix would then be to move the call to _addPermission from _getAssertion to the result block in _uiRequest().
I was just able to repro this twice in a row.

1. Sign in to an app with an unverified account.  Do not verify.  Do not close the app.
2. Go to settings, FxA, sign out.  Do not close the app from step 1.
3. Return to app from step 1.
4. Enter email and password to a previously verified email address.

success screen, onlogin to fire

Second PW prompt and then onerror fires.  onlogin fires immediately if request is called again, without email/username prompt.

Comment 9

4 years ago
I've got a patch following up on comment #7 (plus a new unit test) in flight on Treeherder; if it passes I'll ask for review.

Comment 10

4 years ago
Created attachment 8530699 [details] [diff] [review]

Fernando, this patch closely the hole that Nick noticed, where a user who signs into FxA via a privileged app with an unverified account will not have their granting of FxA permission to that app recorded. I also added a small test, and Treeherder looks good. Thanks for your time, and no hurry on this one.
Attachment #8530699 - Flags: review?(ferjmoreno)
Comment on attachment 8530699 [details] [diff] [review]

Review of attachment 8530699 [details] [diff] [review]:

Thank you Sam! Nice catch. The patch looks good, but I'd like to try a different approach for the test if it is possible. Thanks!

::: services/fxaccounts/FxAccountsManager.jsm
@@ +321,5 @@
>        result => {
> +        if (result && aPrincipal) {
> +          this._addPermission(aPrincipal);
> +        }
> +        // Only return assertions for verified accounts.

The old comment was also a good explanation worth keeping :)

::: services/fxaccounts/tests/xpcshell/test_manager.js
@@ +174,5 @@
> +FxAccountsManager._addPermissionCalled = false;
> +FxAccountsManager._addPermission = function() {
> +  this._addPermissionCalled = true;
> +}

We are testing FxAccountsManager here, so we should try not to mock its functionality whenever it's possible. I believe we can play with the PermissionManager here and see if we added the permission or not. We already do this in other tests like [1] or [2].

Attachment #8530699 - Flags: review?(ferjmoreno) → feedback+


4 years ago
Assignee: spenrose → nobody

Comment 12

11 months ago
Firefox OS is not being worked on
Last Resolved: 4 years ago11 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.