Closed Bug 1095752 Opened 11 years ago Closed 7 years ago

First use of privileged app w/ FxA triggers redundant refreshAuthentication

Categories

(Firefox OS Graveyard :: FxA, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: spenrose, Unassigned)

References

Details

(Whiteboard: [dependency: marketplace-partners])

Attachments

(2 files)

Attached file test_case.zip
STR: 1) Install a privileged app such as attached test_case.zip 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. Builds: - 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 https://bugzilla.mozilla.org/show_bug.cgi?id=1095744#c0 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 https://wiki.mozilla.org/B2G/QA/Triage#Summary_of_Keywords_and_Flags for information on when to add this tag
Blocks: 1095816
Whiteboard: [dependency: marketplace-partners]
We haven't been seeing this since. Closing as invalid. This was on 2.0.
Status: NEW → RESOLVED
Closed: 11 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 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/2bea026d4f86 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
Status: RESOLVED → REOPENED
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 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/54f1b0ee07a6 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
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: https://github.com/mozilla/gecko-dev/blob/master/services/fxaccounts/FxAccountsManager.jsm#L559 before the first call reached here: https://github.com/mozilla/gecko-dev/blob/master/services/fxaccounts/FxAccountsManager.jsm#L345 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. STR: 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. Expected: success screen, onlogin to fire Actual: Second PW prompt and then onerror fires. onlogin fires immediately if request is called again, without email/username prompt.
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.
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. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=04cf991f267e
Attachment #8530699 - Flags: review?(ferjmoreno)
Comment on attachment 8530699 [details] [diff] [review] 1095752-move-addPermission.patch 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]. [1] https://github.com/mozilla/gecko-dev/blob/master/services/fxaccounts/tests/xpcshell/test_manager.js#L525 [2] https://github.com/mozilla/gecko-dev/blob/master/services/fxaccounts/tests/xpcshell/test_manager.js#L568
Attachment #8530699 - Flags: review?(ferjmoreno) → feedback+
Assignee: spenrose → nobody
Firefox OS is not being worked on
Status: REOPENED → RESOLVED
Closed: 11 years ago7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: