Closed Bug 1586913 Opened Last month Closed Last month

Add a policy for DisablePasswordReveal

Categories

(Firefox :: Enterprise Policies, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 71
Tracking Status
firefox71 --- fixed

People

(Reporter: mkaply, Assigned: mkaply)

Details

Attachments

(4 files)

So apparently some enterprises have policies against viewing passwords on screen.

Internet Explorer has a policy for this:

DisablePasswordReveal

It just removes the ability to show the password.

We should add that for the new about:logins.

We'll need a separate patch for ESR68 for the old dialog.

Attached patch First passSplinter Review

jaws:

Can you offer any advice on why this doesn't work? in particular, I get:

JavaScript error: chrome://browser/content/aboutlogins/components/login-item.js, line 191: TypeError: window.AboutLoginsUtils is undefined

When I go to about:logins. I see window.AboutLoginsUtils used elsewhere in this file and in login-list.js it's used during render.

Sorry it's not in phabricator. Have too many changesets right now.

Thanks!

Attachment #9099414 - Flags: feedback?(jaws)
Type: defect → enhancement
Priority: -- → P3
Comment on attachment 9099414 [details] [diff] [review]
First pass

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

The error you're seeing is because AboutLoginsUtils hasn't been defined yet. It's defined in response to the Init message, but login-item's render gets called before that. login-list's render also gets called early but the _loginGuidsSortedOrder array will be empty thus the contents of the filter() call will not get evaluated and there will be no exception for an undefined AboutLoginsUtils.

You can check first to see if AboutLoginsUtils is defined.

::: browser/components/aboutlogins/AboutLoginsParent.jsm
@@ +412,4 @@
>              syncState,
>              selectedBadgeLanguages,
>              masterPasswordEnabled: LoginHelper.isMasterPasswordSet(),
> +            passwordReveal: Services.policies.isAllowed("passwordReveal"),

Can we call the property something like "passwordRevealVisible" on the about:logins side?
Attachment #9099414 - Flags: feedback?(jaws) → feedback+
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/479097530e46
Add policy for DisablePasswordReveal. r=jaws,flod,fluent-reviewers

Backed out changeset 479097530e46 (bug 1586913) for failing at test_login_item.html on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/b40d7c3da6f9a99285901732fc720ef4355dc117

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&searchStr=%28c1%29&revision=479097530e46f2ee9aa1420d0283979582117e0a&selectedJob=270524002

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=270524002&repo=autoland&lineNumber=1494

Log snippet:
[task 2019-10-09T17:37:12.200Z] 17:37:12 INFO - add_task | Leaving test test_edit_login
[task 2019-10-09T17:37:12.200Z] 17:37:12 INFO - add_task | Entering test test_edit_login_cancel
[task 2019-10-09T17:37:12.200Z] 17:37:12 INFO - TEST-PASS | browser/components/aboutlogins/tests/chrome/test_login_item.html | loginItem should be in 'edit' mode
[task 2019-10-09T17:37:12.201Z] 17:37:12 INFO - TEST-PASS | browser/components/aboutlogins/tests/chrome/test_login_item.html | loginItem should not be in 'isNewLogin' mode
[task 2019-10-09T17:37:12.201Z] 17:37:12 INFO - TEST-PASS | browser/components/aboutlogins/tests/chrome/test_login_item.html | loginItem should not be in 'edit' mode
[task 2019-10-09T17:37:12.201Z] 17:37:12 INFO - TEST-PASS | browser/components/aboutlogins/tests/chrome/test_login_item.html | loginItem should not be in 'isNewLogin' mode
[task 2019-10-09T17:37:12.201Z] 17:37:12 INFO - add_task | Leaving test test_edit_login_cancel
[task 2019-10-09T17:37:12.201Z] 17:37:12 INFO - add_task | Entering test test_reveal_password_change_selected_login
[task 2019-10-09T17:37:12.201Z] 17:37:12 INFO - TEST-PASS | browser/components/aboutlogins/tests/chrome/test_login_item.html | reveal-checkbox should not be checked by default
[task 2019-10-09T17:37:12.201Z] 17:37:12 INFO - TEST-PASS | browser/components/aboutlogins/tests/chrome/test_login_item.html | Password should be masked by default
[task 2019-10-09T17:37:12.201Z] 17:37:12 INFO - TEST-PASS | browser/components/aboutlogins/tests/chrome/test_login_item.html | reveal-checkbox should be checked after clicking
[task 2019-10-09T17:37:12.207Z] 17:37:12 INFO - Buffered messages finished
[task 2019-10-09T17:37:12.207Z] 17:37:12 INFO - TEST-UNEXPECTED-FAIL | browser/components/aboutlogins/tests/chrome/test_login_item.html | waiting for password input type to change after checking for master password
[task 2019-10-09T17:37:12.207Z] 17:37:12 INFO - SimpleTest.ok@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:277:18
[task 2019-10-09T17:37:12.207Z] 17:37:12 INFO - SimpleTest.waitForCondition/interval<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1045:9
[task 2019-10-09T17:37:12.207Z] 17:37:12 INFO - setInterval handlerSimpleTest.waitForCondition@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1043:29
[task 2019-10-09T17:37:12.207Z] 17:37:12 INFO - SimpleTest.promiseWaitForCondition/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1065:10
[task 2019-10-09T17:37:12.208Z] 17:37:12 INFO - SimpleTest.promiseWaitForCondition@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1064:10
[task 2019-10-09T17:37:12.208Z] 17:37:12 INFO - test_reveal_password_change_selected_login@chrome://mochitests/content/chrome/browser/components/aboutlogins/tests/chrome/test_login_item.html:233:20
[task 2019-10-09T17:37:12.208Z] 17:37:12 INFO - nextTick/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1795:34
[task 2019-10-09T17:37:12.208Z] 17:37:12 INFO - async
nextTick@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1811:11
[task 2019-10-09T17:37:12.208Z] 17:37:12 INFO - setTimeout handler*SimpleTest_setTimeoutShim@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:686:43
[task 2019-10-09T17:37:12.208Z] 17:37:12 INFO - add_task@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1755:17
[task 2019-10-09T17:37:12.208Z] 17:37:12 INFO - @chrome://mochitests/content/chrome/browser/components/aboutlogins/tests/chrome/test_login_item.html:58:9
[task 2019-10-09T17:37:12.208Z] 17:37:12 INFO - Not taking screenshot here: see the one that was previously logged

Flags: needinfo?(mozilla)

Needed an explicit === check for false to account for possibility of undefined.

Flags: needinfo?(mozilla)
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/a728ab171025
Add policy for DisablePasswordReveal. r=jaws,flod,fluent-reviewers
Status: ASSIGNED → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
Flags: qe-verify+
QA Contact: emil.ghitta

Comment on attachment 9110037 [details]
Bug 1586913 - Add policy for DisablePasswordReveal.

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Policy specific, adds import policy for passwords
    This is an ESR only version of patch.
  • User impact if declined: Admins unable to prevent viewing passwords
  • Fix Landed on Version: 71 (71 specific)
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Policy only.
  • String or UUID changes made by this patch: Yes, accepted by l10n
Attachment #9110037 - Flags: approval-mozilla-esr68?
Attachment #9100020 - Flags: approval-mozilla-esr68?
You need to log in before you can comment on or make changes to this bug.