Closed Bug 1590622 Opened 3 months ago Closed 2 months ago

https: login from a different subdomain can prevent the http: login from the form subdomain from filling or showing "From this website"

Categories

(Toolkit :: Password Manager, defect, P1)

71 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla72
Tracking Status
firefox-esr68 --- unaffected
firefox70 --- disabled
firefox71 + verified
firefox72 --- verified

People

(Reporter: dana, Assigned: MattN)

References

(Regression)

Details

(Keywords: regression)

Attachments

(10 files)

Attached image dropdown.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:71.0) Gecko/20100101 Firefox/71.0

Steps to reproduce:

When i focus a log-in form, the browser shows a drop-down list of all the log-ins it thinks might be useful for the current site. Something has changed in recent versions of Firefox (70 or 71, not sure which) that has made this feature really irritating.

Most immediately, the drop-down is styled such that (on macOS at least) it now fills the entire vertical span of the screen when there are many entries. This is incredibly distracting, and it often blocks out content on the page that i'm trying to read. I have no opinion on how many total entries appear in the drop-down, but i can't see any reason it needs to show more than 3, maybe 5, at a time (without scrolling).

Also, maybe it was already like this, and i just didn't notice because the previous styling hid it from me, but i think recent versions of Firefox may have changed how it determines which log-ins are relevant to the current site. I say that because i now see dozens of log-ins for sites that have absolutely nothing to do with the one i'm trying to log in to, simply because they are under the same domain. (e.g., i'm trying to log in to a.example.com, but i'm getting a million results for b.example.com and c.example.com and d.example.com....)

This works fine in cases like www.reddit.com vs np.reddit.com (where the different sub-domains are actually the same application), but it seems ill-suited to domains that host many different applications, such as those used for company intranets.

I imagine this was already considered and rejected as a major concern, but it seems related, so i thought i would bring it up just in case.

Actual results:

Please see the attached screen-shot, showing what happens in Firefox 71 when i try to log in to our local GitLab instance at work. (I've blocked out the results for privacy reasons, but every single one of them was for some other random Web site under the company domain.)

Expected results:

Log-ins drop-down should be a reasonable size, maybe like 3 entries tall.

Component: Untriaged → Password Manager
Product: Firefox → Toolkit
Blocks: 1558857
Regressed by: 1558857

I've miss-filled the bugs for the blocked by and regressed by: probably with this update it is more accurate.

On the "regression" keyword, I think we could look at it from the point of view that we are changing the user experience on some particular situations. Some of those are already discussed and fixed on bug 1558857, but I agree with Dana's suggestion that in case of many logins the autocomplete drop-down should have a limit - to "n" elements where "n" to be decided what's optimal (not sure how to determine that -> MattN?).

Given the fact that with 1563330, the number of people hitting this will increase automatically, I think that a bit of consideration should be given to this report.

Additionally, I would note for the many logins autocomplete drop-down:

  • overflows the (resized) window;
  • hides the "View Logins" button;
  • hides the scroll bar sometimes (intermittent - but caught it on Mac and Win10)
Blocks: 1563330
Regressed by: 1563330
No longer regressed by: 1558857

Hello Dana, thank you for the report. Do you actually have different valid passwords for so many subdomains or are some of them old passwords that just haven't been updated to their new value? I ask because we try to de-dupe exact username+password combos in the dropdown.

Do any of your results show the http auth realm (you can identify this with some text in brackets after the username)?

[Tracking Requested - why for this release]: New feature bug which needs a decision before shipping.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(dana)
Priority: -- → P1

Hi Matthew,

In my case, it looks like 17/20 of the entries visible in the list are for unique (sub-)domains. About 2/3 of those are internal test appliances that use HTTP basic auth, and they do show the realm (next to the domain, not the user name). The three duplicates are ones that use basic auth, and it seems like they probably appear in the list because the realm text changed at some point.

So i think the entries make sense, aside from all of them belonging to unrelated applications on different sub-domains. (And even that would be fine if it showed the entries for the current sub-domain at the top — or at all, for that matter. But i might be mixing up multiple, possibly already fixed, issues here)

Flags: needinfo?(dana)

(In reply to dana from comment #3)

So i think the entries make sense, aside from all of them belonging to unrelated applications on different sub-domains. (And even that would be fine if it showed the entries for the current sub-domain at the top — or at all, for that matter.

It should be doing exactly that - an entry for the current sub-domain (exact origin) should appear at the top of the list. Can you confirm you do have a saved login that should be at the top of this list? Is it not showing up at all, or is it somewhere down the list?

Flags: needinfo?(dana)
Attached image gitlab-dropdown.png
Attached image gitlab-saved-login.png

Can you confirm you do have a saved login that should be at the top of this list? Is it not showing up at all, or is it somewhere down the list?

I've just checked four internal applications, all HTTPS sites with host names like foo.company.com.

On two of them (a time-keeping system and an asset-management system), i did see 'From this website' at the top of the list. And, since i have the auto-fill option enabled under Privacy & Security, it used that entry to populate the fields for me.

On the other two (GitLab and our old Mantis bug tracker), i did not get auto-fill, and i did not see 'From this website' — only the other, irrelevant sub-domains i mentioned previously. However, when i clicked 'View Saved Logins' at the bottom of the suggestion drop-down, in both cases it showed me that i had saved log-ins for those sites. I've attached two more screen-shots depicting this in the GitLab case.

It's hard to remember the mundane-routine stuff, but i'm pretty sure that it used to offer suggestions for those GitLab and Mantis log-ins...

(Sorry again if i'm mixing up separate issues)

Flags: needinfo?(dana)

The issue of "from this website" not appearing in cases where you have a duplicate username + password combo for a different subdomain is already being tracked in bug 1590362 and will be fixed before release.

Depends on: 1590362

Thank you!

(In reply to dana from comment #7)

On the other two (GitLab and our old Mantis bug tracker), i did not get auto-fill, and i did not see 'From this website' — only the other, irrelevant sub-domains i mentioned previously. However, when i clicked 'View Saved Logins' at the bottom of the suggestion drop-down, in both cases it showed me that i had saved log-ins for those sites. I've attached two more screen-shots depicting this in the GitLab case.

Can you confirm that "From this website" now properly appears when it should with the latest Nightly?

Flags: needinfo?(dana)
Attached image gitlab-nightly.png

Sorry for the delay in responding. I needed to get to a place where i'd have time to set up Nightly.

Anyway, testing in Nightly 2019-10-30 with a clone of my existing profile, i don't think i'm seeing a difference. See attached screen-shot. As before, clicking 'View Saved Logins' at the bottom confirms that i do in fact have credentials saved for this site. The other site i tested is the same.

I also tried to replicate the original conditions of the problem in a blank profile, by saving a bunch of log-ins for other sites with the same domain + user name + password, but i couldn't manage it; when i start from scratch, it picks up the GitLab credentials and ignores the irrelevant ones, like it should.

Maybe there is some variable in my configuration that isn't being accounted for? Number of log-ins, age of log-ins, source of log-ins? Or am i just doing it wrong? idk

Flags: needinfo?(dana)

Sorry for the delay but I was quite confused about comment 11 and comment 12

Sorry for the delay but I was quite confused about comment 11 and comment 12

I want to understand the issue of no results showing "From this website" (and therefore sorted at the top) even though you believe you have an exact match for this website:

  1. In Firefox Nightly, can you enable input in the Browser Console and paste in the code I'm attaching.
  2. Go to the tab of the login form which has the problem and hit enter while at the end to run it. It will first output non-anonymized information about the logins for your own reference and cross-referencing and then it will output the same information but anonymized so that you can attach it here.
  3. Copy the anonymized output and paste it in Bugzilla
  4. Cross-reference the two results (the order should be the same) to identify the login(s) that you expect to be labelled as "From this website" but isn't and note that in a comment.

Thank you very much!

Flags: needinfo?(dana)

Hi again,

Testing on 72.0a1 (2019-11-06), the site in question is not listed in the output of the function. I did this for both sites i was having the issue with (GitLab and Mantis) and it was the same. I've attached the anonymised output from the GitLab site, for whatever good it does.

I notice that when i look at the saved log-ins for these sites in about:logins, the 'Web Site Address' field shows them with http:// (which was correct when i first saved them several years ago), whilst both of them now use HTTPS. That's the only obvious discrepancy to me. Could that be a factor?

Flags: needinfo?(dana)

Not necessarily the cause of problem discussed here, but I notice from the anonymised log-in information 3 scenarios that we didn't consider for our testing and there is impact that needs to be considered for the wild:

scenario 1:

user has credentials saved for http://what.ever.site.com

expected?
the credentials should show in the for : "this site" area on https://what.ever.site.com

scenario 2:

user has credentials saved for https://what.ever.site.com:port

expected?
loading https://what.ever.site.com, should credentials be shown in :"this site" area ?

scenario 3:

user has credentials saved for https://what.ever.site.com

expected?
should credentials be show in the for : "this site" area on http://what.ever.site.com?

Matt, can you please take a look over the above and advise if there should be separate bugs logged for each?

Flags: needinfo?(MattN+bmo)

(In reply to dana from comment #15)

Testing on 72.0a1 (2019-11-06), the site in question is not listed in the output of the function. I did this for both sites i was having the issue with (GitLab and Mantis) and it was the same. I've attached the anonymised output from the GitLab site, for whatever good it does.

I notice that when i look at the saved log-ins for these sites in about:logins, the 'Web Site Address' field shows them with http:// (which was correct when i first saved them several years ago), whilst both of them now use HTTPS. That's the only obvious discrepancy to me. Could that be a factor?

Aha, I think we're getting closer to figuring this out… it may be related to http: but I'll know better if you can run this one more script using the same steps as comment 14. Sorry about that but I hope this will be the last bit of code to run.

Flags: needinfo?(MattN+bmo) → needinfo?(dana)
Attachment #9106761 - Attachment description: Browser Console code to dump login list → Browser Console code to dump deduped login list
Flags: needinfo?(MattN+bmo)

Hi again, see attached. The GitLab domain appears near the bottom this time.

Flags: needinfo?(dana)

Yay! Thank you very much. I'm now able to reproduce the problem and will figure out where the code goes wrong.

It only took be less than 10 minutes to spot the problem once I could reproduce it and the fix is easy :)

Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED

On an https: page it's more important to use the login was from the same hostPort than to use an https: login from a different subdomain.

This fixes autocomplete to show "From this website" and also fixes autofill in the event that there was a duplicate username+password combo saved on an different https: subdomain while previously saving that combo on the http: version of the same hostPort.

Thank you very much for your persistence on this issue so we could fix this bug for everyone before shipping to release.

Let's focus this bug on this autofill/"from this website" issue which I'm now fixing. If there are still concerns about the number of results then we can investigate more in a new bug.

(In reply to Adrian Florinescu [:adrian_sv] from comment #16)

Not necessarily the cause of problem discussed here, but I notice from the anonymised log-in information 3 scenarios that we didn't consider for our testing and there is impact that needs to be considered for the wild:

scenario 1:

user has credentials saved for http://what.ever.site.com

expected?
the credentials should show in the for : "this site" area on https://what.ever.site.com

Correct and therefore should it should also autofill on https://what.ever.site.com (we should never autofill on http:)

scenario 2:

user has credentials saved for https://what.ever.site.com:port

expected?
loading https://what.ever.site.com, should credentials be shown in :"this site" area ?

No, the "from this website" text relies on the whole domain name and port (aka. hostPort) matching (note that port is empty when it's the default for the scheme so that's why the http: and https: version of the same domain can match even though technically they are served over ports 80 and 443 respectively). This suggestion should still appear in the list though.

scenario 3:

user has credentials saved for https://what.ever.site.com

expected?
should credentials be show in the for : "this site" area on http://what.ever.site.com?

No credentials from https: (on any domain) should even be suggested on http: so we don't even get to considering "from this website"

Matt, can you please take a look over the above and advise if there should be separate bugs logged for each?

Note that the issue in this bug ended up being a different scenario that you probably want to add to your testcases:

scenario 4:

user has credentials saved for http://what.ever.site.com AND https://other.site.com (can be any different subdomain or the base domain as long as its https:) with the same username and password.

Expected:

  • The saved login (from http://what.ever.site.com) should be autofilled (since the username and password are the same you can't really tell which one is autofilled until you submit the form and check the timeLastUsed in about:logins)
  • autocomplete should indicate "from this website"

Thanks for checking Adrian!

Flags: needinfo?(MattN+bmo)
Summary: Saved logins/passwords dropdown fills entire screen → https: login from a different subdomain can prevent the http: login from the form subdomain from filling or showing "From this website"
Flags: qe-verify+
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/07adfe0808dd
Prefer a domain match over a scheme match when deduping logins. r=sfoster

Comment on attachment 9108325 [details]
Bug 1590622 - Prefer a domain match over a scheme match when deduping logins. r=sfoster

Beta/Release Uplift Approval Request

  • User impact if declined: Some saved logins will no longer autofill nor be identified as being "from this website" (and therefore sorted at the top of login autocomplete)
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 22 scenario 4
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Low risk change re-ordering an array of which aspects of a login to prioritized when deduping. It moves the domain matching to be first priority which it should have been from the beginning but it was overlooked.
  • String changes made/needed: None
Attachment #9108325 - Flags: approval-mozilla-beta?
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

Hi Dana, would you mind testing the fix in the latest Nightly update? I have requested it be backported to Beta but that is pending approval so it'd be great to know if it fixes autofill and the "from this website"?

Flags: needinfo?(dana)

Hi again. Testing the 2019-11-14 nightly, i do get 'From this website' and auto-fill now, yes. (The drop-down still fills my whole screen with log-ins for all the other sites under this domain, but i assume that's expected for right now.) Thank you!

Flags: needinfo?(dana)

Awesome, glad to hear that!

(In reply to dana from comment #27)

(The drop-down still fills my whole screen with log-ins for all the other sites under this domain, but i assume that's expected for right now.)

Yes, I know it's not an ideal solution but perhaps some of those passwords are obsolete and can be deleted? Feel free to file a new bug to deal with the long list.

Status: RESOLVED → VERIFIED

Oh, should i just follow bug 1329903? Or is that different?

QA Whiteboard: [qa-triaged]

(In reply to dana from comment #29)

Oh, should i just follow bug 1329903? Or is that different?

That probably works though I don't know when that will be prioritized.

Also confirming the fix on the latest Nightly 72.0a1 (2019-11-14) (64-bit) on Windows 10, MacOS 10.14 and Ubuntu 18.04.
Waiting for uplift to Beta.

Comment on attachment 9108325 [details]
Bug 1590622 - Prefer a domain match over a scheme match when deduping logins. r=sfoster

Low risk, fix verified by the reporter and QA in nightly, uplift approved for 71 beta 11, thanks.

Attachment #9108325 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Backed out changeset 74f713d368c1 (Bug 1590622) for eslint and likely browser-chrome failures in test_autofill_https_upgrade.html:

https://hg.mozilla.org/releases/mozilla-beta/rev/7ae5842900ccdff0f9f9c5d8f072fedfe425d834

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&selectedJob=276760507&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=a25e8472d4ff130f59792b0ac11a133b0514aca0
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=276760507&repo=mozilla-beta

TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/passwordmgr/test/mochitest/test_autofill_https_upgrade.html:141:12 | 'getIframeBrowsingContext' is not defined. (no-undef)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/passwordmgr/test/mochitest/test_autofill_https_upgrade.html:142:9 | 'checkLoginFormInChildFrame' is not defined. (no-undef)

Flags: needinfo?(MattN+bmo)

On an https: page it's more important to use the login was from the same hostPort than to use an https: login from a different subdomain.

This fixes autocomplete to show "From this website" and also fixes autofill in the event that there was a duplicate username+password combo saved on an different https: subdomain while previously saving that combo on the http: version of the same hostPort.

Comment on attachment 9109707 [details]
Bug 1590622 - Prefer a domain match over a scheme match when deduping logins. r=sfoster a=pascalc

Here is the Phab version for Beta… I can't land it myself anymore which is annoying.

Attachment #9109707 - Attachment description: Bug 1590622 - Prefer a domain match over a scheme match when deduping logins. r=sfoster a=pascalc → Phab for beta
Flags: needinfo?(MattN+bmo)
Attachment #9109707 - Attachment description: Phab for beta → Bug 1590622 - Prefer a domain match over a scheme match when deduping logins. r=sfoster a=pascalc
Attachment #9109707 - Attachment description: Bug 1590622 - Prefer a domain match over a scheme match when deduping logins. r=sfoster a=pascalc → Phab for beta
Attachment #9109707 - Attachment description: Phab for beta → Bug 1590622 - Prefer a domain match over a scheme match when deduping logins. r=sfoster a=pascalc

Verified - fixed on latest Firefox Beta 71.0b11 (64-bit) on Windows 10, MacOS 10.13 and Ubuntu 18.04.

QA Whiteboard: [qa-triaged]
Flags: qe-verify+

In my case (Win, 72.0.1, including a "refresh") it does not work and the password manager still shows all the usernames coming from the domain level instead of those coming from the sub-domain where the page/URL actually is.

Flags: needinfo?(MattN+bmo)

(In reply to cata.maican from comment #39)

In my case (Win, 72.0.1, including a "refresh") it does not work and the password manager still shows all the usernames coming from the domain level instead of those coming from the sub-domain where the page/URL actually is.

That sounds like correct behaviour. See bug 589628.

Flags: needinfo?(MattN+bmo)
You need to log in before you can comment on or make changes to this bug.