STR: - Open UI Tests App - Go to API tab, Firefox Accounts mozId - click refreshAuth button, and enter the correct password. - click refreshAuth button again, and when the password field shows, enter a wrong password. close the error message, and click X on top left corner to close the dialog. - click refreshAuth again. Expected: Password field shows up. Actual: User is asked to enter password. Version info: Gaia 34cdda626d6bb7b0c74a82b4607162c2e967b80a Gecko 9a6a74ab706d271815593f0c91b5e0a3878c560d BuildID 20140610130808 Version 33.0a1 ro.build.version.incremental=eng.mozilla.20140610.125223 ro.build.date=Tue Jun 10 12:52:39 EDT 2014

This patch fixes a silly mistake on my part. I did a big rewrite for bug 1004319 in the context of a server-side password change, forgetting to implement the common case of an RP-requested refreshAuth. I added a comment to job my future self's memory of the state machine.

Comment on attachment 8438009 [details] [diff] [review] 1023463-refreshAuth-fail.patch Review of attachment 8438009 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the fix, Sam. Looks good to me! ::: services/fxaccounts/FxAccountsManager.jsm @@ +216,5 @@ > + * There are two very different scenarios: > + * 1) The password has changed on the server. Failure should log > + * the current account OUT. > + * 2) The person typing can't prove knowledge of the password used > + * to log in. Failure should do nothing. comment is helpful @@ +218,5 @@ > + * the current account OUT. > + * 2) The person typing can't prove knowledge of the password used > + * to log in. Failure should do nothing. > + */ > + _refreshAuthentication: function(aAudience, aEmail, logoutOnFailure) { Since logoutOnFailure is optional, you could give it a default value? (logoutOnFailure=false)

Thanks Jed! Default parameter added. https://tbpl.mozilla.org/?tree=Try&rev=a2f23d979763

This patch fixes an FxA bug which will impact Find My Device in 2.0.

[User impact] User will be logged out of entire device if friend they lend phone to fails a password check. Low occurrence, medium impact. [Risk] Adds a default parameter. Tested manually and via try server. [Strings] No string changes.

(In reply to Sam Penrose from comment #5) > [User impact] User will be logged out of entire device if friend they lend > phone to fails a password check. Low occurrence, medium impact. > [Risk] Adds a default parameter. Tested manually and via try server. > [Strings] No string changes. Looks like this is a server side issue to us so would not block.

Bhavana -- It's an FxOS user experience issue that can only be fixed by uplifting this code; I am not sure how it is a server-side issue. This is not the most important uplift issue we face, and I am OK with deciding it is not important enough to uplift, but can you very that we agree that failing to uplift may have client-side impact?

(In reply to Sam Penrose from comment #7) > Bhavana -- It's an FxOS user experience issue that can only be fixed by > uplifting this code; I am not sure how it is a server-side issue. This is > not the most important uplift issue we face, and I am OK with deciding it is > not important enough to uplift, but can you very that we agree that failing > to uplift may have client-side impact? Apologies, may be I mixed up the bugs here :-/ and thanks for clarifying. But I remember considering vishy's opinion on this and he said this might be edge-casey and not something we would block on. Can you seek approval on this once its ready ? , that way this can get uplifted to 2.0 as this looks low risk(Refer guidelines here: https://wiki.mozilla.org/Release_Management/Uplift_rules). I would wait till its landed on central..

Requesting approval, thanks! (In reply to bhavana bajaj [:bajaj] from comment #8) > Can you seek approval on > this once its ready ? , that way this can get uplifted to 2.0 as this looks > low risk(Refer guidelines here: > https://wiki.mozilla.org/Release_Management/Uplift_rules). I would wait till > its landed on central..

(In reply to Sam Penrose from comment #11) > Requesting approval, thanks! > > (In reply to bhavana bajaj [:bajaj] from comment #8) > > Can you seek approval on > > this once its ready ? , that way this can get uplifted to 2.0 as this looks > > low risk(Refer guidelines here: > > https://wiki.mozilla.org/Release_Management/Uplift_rules). I would wait till > > its landed on central.. Hey sam, Blocking and approval are two different things. Blocking is used on bugs that are regressions, functional issues etc typically anything that you will stop-ship the release for. Whereas approvals can be used on nice-to-have's, polish issues,papercuts etc.. To seek approval, please click on "details" on the patch attachment, set approval-gaia-2.0:? and make sure to answer the attachment questionnaire. Once you do that I will go through the details you've filled and a+ it(after weighing risk/reward) after which sheriff does the uplift for you.

(In reply to bhavana bajaj [:bajaj] from comment #12) > (In reply to Sam Penrose from comment #11) > > Requesting approval, thanks! > > > > (In reply to bhavana bajaj [:bajaj] from comment #8) > > > Can you seek approval on > > > this once its ready ? , that way this can get uplifted to 2.0 as this looks > > > low risk(Refer guidelines here: > > > https://wiki.mozilla.org/Release_Management/Uplift_rules). I would wait till > > > its landed on central.. > > Hey sam, > > Blocking and approval are two different things. Blocking is used on bugs > that are regressions, functional issues etc typically anything that you will > stop-ship the release for. Whereas approvals can be used on nice-to-have's, > polish issues,papercuts etc.. To seek approval, please click on "details" on > the patch attachment, set approval-gaia-2.0:? and make sure to answer the or approval-mozilla-aurora in this case as this is a gecko patch :) > attachment questionnaire. Once you do that I will go through the details > you've filled and a+ it(after weighing risk/reward) after which sheriff does > the uplift for you.

Comment on attachment 8438043 [details] [diff] [review] 1023463-refreshAuth-fail.patch [Approval Request Comment] Bug caused by (feature/regressing bug #): 949093 (Comment 0 "A failure of a 3rd party initiated Forced authentication should not log out the user.") User impact if declined: User will be logged out on failure. Testing completed (on m-c, etc.): https://tbpl.mozilla.org/?tree=Try&rev=a2f23d979763 Risk to taking this patch (and alternatives if risky): some risk to User Story if there is a lurking error. String or IDL/UUID changes made by this patch: None

Comment on attachment 8438043 [details] [diff] [review] 1023463-refreshAuth-fail.patch Aurora approval granted.