1586913 - Add a policy for DisablePasswordReveal

Status: RESOLVED FIXED

(Firefox :: Enterprise Policies, enhancement, P3)

Description

[Mike Kaply [:mkaply]]

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.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: First pass

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!

Comment 2

[(parental leave 10/24-mid-February) Jared Wein [:jaws] (please needinfo? me)]

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?

Comment 3

[Mike Kaply [:mkaply]]

Attachment: Bug 1586913 - Add policy for DisablePasswordReveal.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/479097530e46
Add policy for DisablePasswordReveal. r=jaws,flod,fluent-reviewers

Comment 5

[Raul Gurzau (:RaulG)]

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 - asyncnextTick@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

Comment 6

[Mike Kaply [:mkaply]]

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a728ab171025
Add policy for DisablePasswordReveal. r=jaws,flod,fluent-reviewers

Comment 8

[Mike Kaply [:mkaply]]

Attachment: ESR version of patch

Comment 9

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/a728ab171025

Comment 10

[Mike Kaply [:mkaply]]

Attachment: Bug 1586913 - Add policy for DisablePasswordReveal.

Comment 11

[Mike Kaply [:mkaply]]

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