Don't show any logins on about:logins until Master Password is logged in

VERIFIED FIXED in Firefox 69

Status

()

enhancement
P1
normal
VERIFIED FIXED
2 months ago
Last month

People

(Reporter: jaws, Assigned: jaws)

Tracking

(Blocks 1 bug, Regressed 1 bug)

Trunk
mozilla69
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox69 verified)

Details

Attachments

(5 attachments, 3 obsolete attachments)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

If Master Password is enabled, we shouldn't show any logins on about:logins. Upon loading about:logins, the user should be prompted to log in to Master Password. If the user cancels the logins then we should not show any logins, along with an error message.

Flags: qe-verify+
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #9064860 - Attachment is obsolete: true

I submitted an alternate patch proposal that doesn't add special behaviour using isLoggedIn and uses our existing notification bar UI rather than making new custom one-off UI. It only needs the test to be adapted and to use FTL strings otherwise it's ready to go. Let me know what you think.

Flags: needinfo?(jaws)
Attachment #9065837 - Attachment description: Bug 1550131 - Remove master password login about:logins notifications when the user logs in. r=jaws → Bug 1550131 - Remove about:logins master password login notifications when the user logs in. r=jaws

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #8)

I submitted an alternate patch proposal that doesn't add special behaviour using isLoggedIn and uses our existing notification bar UI rather than making new custom one-off UI. It only needs the test to be adapted and to use FTL strings otherwise it's ready to go. Let me know what you think.

Thank you, this is great. Do you want to finish this bug or would you like me to fix the TODOs and update the test?

Flags: needinfo?(jaws) → needinfo?(MattN+bmo)
Flags: needinfo?(MattN+bmo)
Attachment #9065740 - Attachment is obsolete: true
Attachment #9065741 - Attachment is obsolete: true

Comment 11

Last month
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b562886d574d
Expose an iterator for AboutLoginsParent subscribers. r=jaws
https://hg.mozilla.org/integration/autoland/rev/32ff6a0b8436
Show a notification when master password login is cancelled while about:logins is open. r=jaws
https://hg.mozilla.org/integration/autoland/rev/97eabc062946
Remove about:logins master password login notifications when the user logs in. r=jaws
https://hg.mozilla.org/integration/autoland/rev/8e7d6a1c30fa
Move notification strings to Fluent file. r=MattN,Pike
https://hg.mozilla.org/integration/autoland/rev/95918413069f
Automated test for master password prompt from about:logins. r=MattN

Backed out 5 changesets (bug 1550131) for browser-chrome failures at browser/components/aboutlogins/tests/browser/browser_masterPassword.js

Backout: https://hg.mozilla.org/integration/autoland/rev/d650f8a160f8c80d2645f4b2936175db013460ec

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=95918413069f5ff80feb050f8ef8d45dd8e08b6e

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=247427718&repo=autoland&lineNumber=2244

task 2019-05-20T21:52:55.076Z] 21:52:55 INFO - TEST-PASS | browser/components/aboutlogins/tests/browser/browser_masterPassword.js | No logins should be displayed when MP is set and unauthenticated -
[task 2019-05-20T21:52:55.077Z] 21:52:55 INFO - Buffered messages finished
[task 2019-05-20T21:52:55.079Z] 21:52:55 INFO - TEST-UNEXPECTED-FAIL | browser/components/aboutlogins/tests/browser/browser_masterPassword.js | Notification should be visible -
[task 2019-05-20T21:52:55.080Z] 21:52:55 INFO - Stack trace:
[task 2019-05-20T21:52:55.081Z] 21:52:55 INFO - chrome://mochikit/content/browser-test.js:test_ok:1314
[task 2019-05-20T21:52:55.081Z] 21:52:55 INFO - chrome://mochitests/content/browser/browser/components/aboutlogins/tests/browser/browser_masterPassword.js:test:62
[task 2019-05-20T21:52:55.081Z] 21:52:55 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1116
[task 2019-05-20T21:52:55.082Z] 21:52:55 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1144
[task 2019-05-20T21:52:55.082Z] 21:52:55 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1005
[task 2019-05-20T21:52:55.083Z] 21:52:55 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
[task 2019-05-20T21:52:55.084Z] 21:52:55 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-05-20T21:52:55.084Z] 21:52:55 INFO - TEST-UNEXPECTED-FAIL | browser/components/aboutlogins/tests/browser/browser_masterPassword.js | Uncaught exception - at chrome://mochitests/content/browser/browser/components/aboutlogins/tests/browser/browser_masterPassword.js:63 - TypeError: notification is null
[task 2019-05-20T21:52:55.085Z] 21:52:55 INFO - Stack trace:
[task 2019-05-20T21:52:55.085Z] 21:52:55 INFO - test@chrome://mochitests/content/browser/browser/components/aboutlogins/tests/browser/browser_masterPassword.js:63:3
[task 2019-05-20T21:52:55.086Z] 21:52:55 INFO - Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1116:34
[task 2019-05-20T21:52:55.086Z] 21:52:55 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:1144:12
[task 2019-05-20T21:52:55.087Z] 21:52:55 INFO - nextTest/<@chrome://mochikit/content/browser-test.js:1005:14
[task 2019-05-20T21:52:55.087Z] 21:52:55 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:803:59
[task 2019-05-20T21:52:55.088Z] 21:52:55 INFO - Leaving test bound test
[task 2019-05-20T21:52:55.089Z] 21:52:55 INFO - GECKO(1969) | MP change from omgsecret! to
[task 2019-05-20T21:52:55.089Z] 21:52:55 INFO - GECKO(1969) | MEMORY STAT | vsize 20974938MB | residentFast 1016MB

Flags: needinfo?(jaws)
Flags: needinfo?(jaws)

Comment 13

Last month
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d84f90dcd584
Expose an iterator for AboutLoginsParent subscribers. r=jaws
https://hg.mozilla.org/integration/autoland/rev/9dbb7bb20ca3
Show a notification when master password login is cancelled while about:logins is open. r=jaws
https://hg.mozilla.org/integration/autoland/rev/1e40a8e7a404
Remove about:logins master password login notifications when the user logs in. r=jaws
https://hg.mozilla.org/integration/autoland/rev/4ff5f1d8f9c4
Move notification strings to Fluent file. r=MattN,Pike
https://hg.mozilla.org/integration/autoland/rev/8518f578b163
Automated test for master password prompt from about:logins. r=MattN
Regressions: 1553196

Comment 15

Last month

Verified - fixed on latest Nightly on Windows 10 and Mac OS 10.13. and Ubuntu 18.04.

  • Master Password is requested after reaching about: logins
  • Dismissing the master password modal will trigger the yellow notification bar (along with the "Log in" and "X" button)
  • There are no entries displayed if the master password was not inputted
  • If the correct master password is applied, the entries will become visible
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.