Closed Bug 1217156 Opened 4 years ago Closed 4 years ago

Add a preference to turn on/off insecure password warnings

Categories

(Firefox :: Security, defect, P1)

44 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 45
Iteration:
44.3 - Nov 2
Tracking Status
firefox44 --- fixed
firefox45 --- fixed

People

(Reporter: tanvi, Assigned: tanvi)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fxprivacy])

Attachments

(1 file, 1 obsolete file)

We should add a pref to enable/disable insecure password warnings in case we decide we need to hold it back from release, but want to keep it in the other channels (especially dev edition).
Whiteboard: [fxprivacy] → [fxprivacy] [triage]
Priority: -- → P2
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Comment on attachment 8680901 [details] [diff] [review]
Bug1217156-10-29-15.patch

Review of attachment 8680901 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me, I'll push to try on aurora and fx-team to make sure we are good there

::: browser/base/content/browser.js
@@ +6929,5 @@
>  
> +  get _hasInsecureLoginForms() {
> +    // checks if the page has been flagged for an insecure login. Also checks
> +    // if the flag to degrade the UI is set to true
> +    return (LoginManagerParent.hasInsecureLoginForms(gBrowser.selectedBrowser) && Services.prefs.getBoolPref("security.insecure_password.ui.enabled"));

Nit - don't need the extra parens, and please wrap this into two lines:

return LoginManagerParent.hasInsecureLoginForms(gBrowser.selectedBrowser) &&
       Services.prefs.getBoolPref("security.insecure_password.ui.enabled");
Attachment #8680901 - Flags: review?(bgrinstead) → review+
Assignee: nobody → tanvi
Status: NEW → ASSIGNED
Fixed nits.  Carrying over r+ from bgrins.  Needs check in and uplift.
Attachment #8680901 - Attachment is obsolete: true
Attachment #8680915 - Flags: review+
(In reply to Tanvi Vyas [:tanvi] from comment #4)
> Needs check in and uplift.

But I'll wait for some try results first.
Comment on attachment 8680915 [details] [diff] [review]
Bug1217156-10-29-15B.patch

Approval Request Comment
[Feature/regressing bug #]: https://bugzilla.mozilla.org/show_bug.cgi?id=1179961
[User impact if declined]: Developers will see insecure password warnings on HTTP pages AND localhost pages, without learn more links.  We need to fix localhost and add learn more before we can expose this to developers
[Describe test coverage new/current, TreeHerder]: There is a test for the feature that is still enabled.  This bug just disables the feature for everything but nightly.
[Risks and why]: Developers will become habituated to the warning since they will see it on localhost.  Which is the opposite of what we want - we want them to fix it.
[String/UUID change made/needed]: None
Attachment #8680915 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/3189c9d88f13
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Iteration: --- → 44.3 - Nov 2
Flags: qe-verify?
Priority: P2 → P1
Comment on attachment 8680915 [details] [diff] [review]
Bug1217156-10-29-15B.patch

Make sense, taking it.
Should be in the first devedition release.
Attachment #8680915 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Carsten Book [:Tomcat] from comment #10)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/904dd138cbc9
>        status-firefox44: affected → fixed

(In reply to Carsten Book [:Tomcat] from comment #12)
> https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/904dd138cbc9
>        status-b2g-v2.5: fixed → ---

Huh? This is the exact same patch. How can it fix the problem on desktop and unfix it on B2G?
Flags: needinfo?(cbook)
(In reply to Tony Mechelynck [:tonymec] from comment #13)
> (In reply to Carsten Book [:Tomcat] from comment #10)
> > https://hg.mozilla.org/releases/mozilla-aurora/rev/904dd138cbc9
> >        status-firefox44: affected → fixed
> 
> (In reply to Carsten Book [:Tomcat] from comment #12)
> > https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/904dd138cbc9
> >        status-b2g-v2.5: fixed → ---
> 
> Huh? This is the exact same patch. How can it fix the problem on desktop and
> unfix it on B2G?

it was a simple problem with the setup of the 2.5 branch. we merged mozilla-central to 2.5 and reverted this. (ended up doing a merge from mozilla-aurora instead of m-c).
Flags: needinfo?(cbook)
You need to log in before you can comment on or make changes to this bug.