Closed Bug 1587653 Opened 1 year ago Closed 6 months ago

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

Categories

(Firefox :: Protections UI, defect, P3)

71 Branch
defect

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox70 --- wontfix
firefox71 --- wontfix
firefox81 --- fixed

People

(Reporter: groovecoder, Assigned: MattN)

Details

Attachments

(1 file)

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.

Gaah

Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Priority: -- → P1

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?

Assignee: jhofmann → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(lcrouch)
Priority: P1 → --

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.

(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. :)

UX team will take a look...

Flags: needinfo?(epang)
Priority: -- → P3

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?

Flags: needinfo?(epang) → needinfo?(chsiang)

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?

Flags: needinfo?(lcrouch) → needinfo?(ewright)
Summary: about:protections Lockwise card doesn't show any breached logins even though about:logins does → about:protections doesn't show any breached logins even though about:logins does

(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.

Flags: needinfo?(ewright)
Flags: needinfo?(chsiang)

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

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?

Status: NEW → RESOLVED
Closed: 9 months ago
Flags: needinfo?(prathikshaprasadsuman)
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

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

Assignee: nobody → MattN+bmo
Status: REOPENED → ASSIGNED
Flags: needinfo?(prathikshaprasadsuman)
Pushed by mozilla@noorenberghe.ca:
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

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
Flags: needinfo?(MattN+bmo)
Pushed by mozilla@noorenberghe.ca:
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
Status: ASSIGNED → RESOLVED
Closed: 9 months ago6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
Summary: about:protections doesn't show any breached logins even though about:logins does → about:protections doesn't show breached logins if Primary Password is enabled but unlocked
You need to log in before you can comment on or make changes to this bug.