1587653 - about:protections doesn't show breached logins if Primary Password is enabled but unlocked

Status: RESOLVED FIXED

(Firefox :: Protections UI, defect, P3)

Description

[Luke Crouch [:groovecoder]]

STR:

1. Have a breached login in about:logins (instructions for simulating one are here: https://bugzilla.mozilla.org/show_bug.cgi?id=1565326#c1)
2. Go to about:logins and see the red alert indicator next to the login
3. Open about:protections

Expected results:
In the Lockwise card, expect to see "1 saved login may have been exposed in a data breach ..."

Actual results:
There's no warning indicator of breached logins.

Comment 1

[Luke Crouch [:groovecoder]]

[Tracking Requested - why for this release]: This is a highlight feature for 70 (blog post: https://docs.google.com/document/d/1oUOclhWA4pOajk-O73pDMf6C6od7wR6xcXWA7uZfT0g/edit#https://docs.google.com/document/d/1oUOclhWA4pOajk-O73pDMf6C6od7wR6xcXWA7uZfT0g/edit#)

Comment 2

[Johann Hofmann [:johannh]]

Gaah

Comment 3

[Johann Hofmann [:johannh]]

Ok, after looking into this, this seems to work as originally intended and I'm fairly sure this doesn't need tracking for 70.

Luke, your comment says that the breached logins indicator is supposed to appear in the Lockwise card, but in all designs, including the one you linked in the blog post, the indicator appears on the Monitor card, and that's what we implemented. The trick here is that you have to have a Monitor account and be signed in in order to see it. While I agree that's not strictly necessary for this to work and it would be better to de-couple it from the Monitor card, it's a UX question that we should just re-evaluate in the future.

Any thoughts?

Comment 4

[Matthew N. [:MattN]]

We are going to be adding more types of "alerts" for saved logins soon (UX spec) and I also think they should all called out on about:protections. This would kinda meet the needs of bug 1220617.

Comment 5

[Johann Hofmann [:johannh]]

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

We are going to be adding more types of "alerts" for saved logins soon (UX spec) and I also think they should all called out on about:protections. This would kinda meet the needs of bug 1220617.

Yeah, I agree, and I still think this bug is valid in the sense that we should move the password alert to the Lockwise card. We should probably sync up about the work that needs to be done here, also with UX. :)

Comment 6

[katieC]

UX team will take a look...

Comment 7

[Eric Pang [:epang] UX]

moving this alert to the Lockwise card makes sense to me as well. Cindy, you mentioned waiting for user reserach before we shift things around. What do you think?

Comment 8

[Luke Crouch [:groovecoder]]

I found out what's causing my bug where it doesn't show on either the Monitor card NOR the Lockwise card.

I'm signed into the browser/sync and signed into monitor.firefox.com, but I have a master password set in Firefox, which fails this check:

https://searchfox.org/mozilla-central/source/browser/components/about/AboutProtectionsHandler.jsm#222

:ewright - I suspect we added that check so someone with a master password set isn't hit by the prompt when they open about:protections?

Comment 9

[Erica Wright [:ewright]]

(In reply to Luke Crouch [:groovecoder] from comment #8)

I found out what's causing my bug where it doesn't show on either the Monitor card NOR the Lockwise card.
:ewright - I suspect we added that check so someone with a master password set isn't hit by the prompt when they open about:protections?

Indeed, that is why we have it! I did not realize you were not seeing the breached login on either card. I'll see what can be done there.

Comment 10

[Matthew N. [:MattN]]

I think we can change the check to instead use Services.logins.isLoggedIn and then we will avoid any new prompt.

Comment 11

[Erica Wright [:ewright]]

This bug is partly fixed by Bug 1627337 (where we moved the breached logins notification to the lockwise card). Prathiksha, what do we do now in the case of a master password?

Comment 12

[Matthew N. [:MattN]]

Attachment: Bug 1587653 - Include breached logins in about:protections without a Monitor account and if PP is logged in. r=ewright

Also separate the Lockwise/Monitor data collection to map to the cards

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b46f9754641
Include breached logins in about:protections without a Monitor account and if PP is logged in. r=ewright

Comment 14

[Cosmin Sabou [:CosminS]]

Backed out for bc failures browser_protections_lockwise.js.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=7b46f97546418377b2ca4b6617309521bac432e7&selectedTaskRun=VIdu51-GRICsk_Z3RS7BMQ.0

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=312219692&repo=autoland

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

[task 2020-08-06T04:41:24.071Z] 04:41:24     INFO - TEST-PASS | browser/components/protections/test/browser/browser_protections_lockwise.js | Save passwords button is visible in the header - true == true - 
[task 2020-08-06T04:41:24.072Z] 04:41:24     INFO - Click on the save passwords button and check that it opens about:logins in a new tab
[task 2020-08-06T04:41:24.073Z] 04:41:24     INFO - about:logins was successfully opened in a new tab
[task 2020-08-06T04:41:24.073Z] 04:41:24     INFO - Buffered messages logged at 04:41:22
[task 2020-08-06T04:41:24.073Z] 04:41:24     INFO - Leaving test bound testNoLoginsLockwiseCardUI
[task 2020-08-06T04:41:24.073Z] 04:41:24     INFO - Entering test bound testLockwiseCardUIWithLogins
[task 2020-08-06T04:41:24.073Z] 04:41:24     INFO - Add a login and check that lockwise card content for a logged in user is displayed correctly
[task 2020-08-06T04:41:24.075Z] 04:41:24     INFO - Console message: [JavaScript Error: "There was an error fetching the user's token: " {file: "resource:///actors/AboutProtectionsParent.jsm" line: 274}]
[task 2020-08-06T04:41:24.075Z] 04:41:24     INFO - getMonitorScopedOAuthToken@resource:///actors/AboutProtectionsParent.jsm:274:10
[task 2020-08-06T04:41:24.075Z] 04:41:24     INFO - 
[task 2020-08-06T04:41:24.076Z] 04:41:24     INFO - Buffered messages logged at 04:41:23
[task 2020-08-06T04:41:24.077Z] 04:41:24     INFO - Console message: [JavaScript Error: "There was an error fetching the user's token: " {file: "resource:///actors/AboutProtectionsParent.jsm" line: 274}]
[task 2020-08-06T04:41:24.078Z] 04:41:24     INFO - getMonitorScopedOAuthToken@resource:///actors/AboutProtectionsParent.jsm:274:10
[task 2020-08-06T04:41:24.078Z] 04:41:24     INFO - 
[task 2020-08-06T04:41:24.078Z] 04:41:24     INFO - Console message: [JavaScript Error: "Unknown Collection "main/fxmonitor-breaches"" {file: "resource://services-settings/RemoteSettingsClient.jsm" line: 159}]
[task 2020-08-06T04:41:24.078Z] 04:41:24     INFO - UnknownCollectionError@resource://services-settings/RemoteSettingsClient.jsm:159:5
[task 2020-08-06T04:41:24.078Z] 04:41:24     INFO - sync@resource://services-settings/RemoteSettingsClient.jsm:469:13
[task 2020-08-06T04:41:24.078Z] 04:41:24     INFO - 
[task 2020-08-06T04:41:24.078Z] 04:41:24     INFO - Buffered messages finished
[task 2020-08-06T04:41:24.079Z] 04:41:24     INFO - TEST-UNEXPECTED-FAIL | browser/components/protections/test/browser/browser_protections_lockwise.js | Correct lockwise title is shown - "\n                \n              " == "Password Management" - got "\n                \n              ", expected "Password Management" (operator ==)
[task 2020-08-06T04:41:24.079Z] 04:41:24     INFO - Stack trace:
[task 2020-08-06T04:41:24.079Z] 04:41:24     INFO - is@resource://specialpowers/SpecialPowersSandbox.jsm:90:21
[task 2020-08-06T04:41:24.079Z] 04:41:24     INFO - @chrome://mochitests/content/browser/browser/components/protections/test/browser/browser_protections_lockwise.js:122:7
[task 2020-08-06T04:41:24.079Z] 04:41:24     INFO - TEST-PASS | browser/components/protections/test/browser/browser_protections_lockwise.js | How it works link is visible - true == true - 
[task 2020-08-06T04:41:24.080Z] 04:41:24     INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-08-06T04:41:24.083Z] 04:41:24     INFO - TEST-UNEXPECTED-FAIL | browser/components/protections/test/browser/browser_protections_lockwise.js | Correct lockwise header string is shown - false == true - got false, expected true (operator ==)
[task 2020-08-06T04:41:24.083Z] 04:41:24     INFO - Stack trace:
[task 2020-08-06T04:41:24.084Z] 04:41:24     INFO - ok@resource://specialpowers/SpecialPowersSandbox.jsm:87:21
[task 2020-08-06T04:41:24.084Z] 04:41:24     INFO - @chrome://mochitests/content/browser/browser/components/protections/test/browser/browser_protections_lockwise.js:139:7
[task 2020-08-06T04:41:24.084Z] 04:41:24     INFO - TEST-PASS | browser/components/protections/test/browser/browser_protections_lockwise.js | Lockwise scanned wrapper is visible - true == true - 
[task 2020-08-06T04:41:24.084Z] 04:41:24     INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-08-06T04:41:24.085Z] 04:41:24     INFO - TEST-UNEXPECTED-FAIL | browser/components/protections/test/browser/browser_protections_lockwise.js | Correct lockwise scanned text is shown - "\n                  \n                " == "1 password stored securely." - got "\n                  \n                ", expected "1 password stored securely." (operator ==)
[task 2020-08-06T04:41:24.085Z] 04:41:24     INFO - Stack trace:
[task 2020-08-06T04:41:24.085Z] 04:41:24     INFO - is@resource://specialpowers/SpecialPowersSandbox.jsm:90:21
[task 2020-08-06T04:41:24.085Z] 04:41:24     INFO - @chrome://mochitests/content/browser/browser/components/protections/test/browser/browser_protections_lockwise.js:157:7

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dad032880c49
Include breached logins in about:protections without a Monitor account and if PP is logged in. r=ewright

Comment 16

[Narcis Beleuzu [:NarcisB]]

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