Closed Bug 1168126 Opened 9 years ago Closed 5 years ago

Password manager pollutes non-login pages with user's login and password data

Categories

(Toolkit :: Password Manager, defect, P3)

38 Branch
defect

Tracking

()

RESOLVED DUPLICATE of bug 1119063

People

(Reporter: sergei, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, Whiteboard: [passwords:heuristics])

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.65 Safari/537.36

Steps to reproduce:

1. Go to https://demo3-xh2gpdx.nextrade360.com/staffonly
2. Login with RodrickBPerez@dayrep.com as email and lop567 as password
3. Make sure to click "Save password" on password manager's prompt shown after logging in.
4. Go to https://demo3-xh2gpdx.nextrade360.com/staffonly/trading_members/40/new_person?after_submit=%2Fstaffonly%2Ftrading_members%2F40%2Fpeople


Actual results:

1. Zip code field on the right is populated with RodrickBPerez@dayrep.com
2. Password field on the left bottom is populated with lop567 password


Expected results:

1. Zip code should be blank
2. Password field should be blank.
This became an issue since password manager was set loose per https://bugzilla.mozilla.org/show_bug.cgi?id=956906 and now auto-fills controls in odd places without any option for me (as a developer) to disable it. This is a serious issue for SAAS vendors.
Component: Layout: Form Controls → Password Manager
Product: Core → Toolkit
I can confirm this is a blocking issue for us.

Firefox 39 changes the values of non-password related fields on our profile page, some of them used by our web framework, blocking our users from changing their passwords.

Will let them know to use Chrome or IE until this is fixed.
(In reply to Jim Meyer from comment #2)
> I can confirm this is a blocking issue for us.
> 
> Firefox 39 changes the values of non-password related fields on our profile
> page

Jim, are you talking about the same website as comment 0, or a different site?

Firefox's password-manager does some heuristics to figure out which which fields are likely to be username/password fields (since real-world login pages have a variety of ways of requesting username/password).

In your case (/cases), it sounds like those heuristics are mistakenly identifying some random fields as being username/password fields, possibly due to the ID attribute on those fields.

(In reply to sergei from comment #0)
> 1. Zip code field on the right is populated with RodrickBPerez@dayrep.com
> 2. Password field on the left bottom is populated with lop567 password

In this case, the autopopulated zip code field looks like this:
<input class="member-related-person-home-zip us-zip-code" id="member_related_person[home_zip]" name="member_related_person[home_zip]" type="text">

Dolske, do you think id="member..." is what's causing the password manager to mis-fire here?
Flags: needinfo?(dolske)
My understanding is that both IE11 and Chrome also stopped supporting autocomplete=off for password forms but I can't reproduce the problem in them.

Looking at chrome://password-manager-internals/, the reason for Chrome not auto-filling isn't clear to me and I can't figure out how to get any useful information out of IE11 though I confirmed on a simpler test page that they don't respect autocomplete=off for autofill on it.

There are 2 issues here:
1) We are auto-filling on a registration-like form which normally isn't useful.
2) We are trying to auto-fill the username into the zip code field. The solution to this is for the page to use @autocomplete to specify the data type and for us to implement bug 917325. Lets leave that discussion to the existing bug and focus on #1.

(In reply to Jim Meyer from comment #2)
> Firefox 39 changes the values of non-password related fields on our profile
> page, some of them used by our web framework, blocking our users from
> changing their passwords.

If by "changing" you mean filling an empty text-like-field then I would recommend implementing @autocomplete attributes to indicate the type of data expected so that this will be fixed by bug 917325. We shouldn't be modify non-empty fields so file a new bug if we are.

(In reply to Daniel Holbert [:dholbert] from comment #3)
> Dolske, do you think id="member..." is what's causing the password manager
> to mis-fire here?

We don't use id/name attributes currently for our default heuristics.

Dolske/ckarlof, it seems like this is related to bullet 3 of bug 1119554. Perhaps one of the heuristics to detect a registration form is the number of fields present? Can we use that here?
Blocks: 1119554
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Matthew N. [:MattN] from comment #4)
> (In reply to Daniel Holbert [:dholbert] from comment #3)
> > Dolske, do you think id="member..." is what's causing the password manager
> > to mis-fire here?
> 
> We don't use id/name attributes currently for our default heuristics.

I should have said that the reason we use the zip code field is because it's the first username field candidate before the password field in DOM order.
Flags: needinfo?(ckarlof)
(In reply to Matthew N. [:MattN] from comment #4)
> Dolske/ckarlof, it seems like this is related to bullet 3 of bug 1119554.
> Perhaps one of the heuristics to detect a registration form is the number of
> fields present? Can we use that here?

To be safe I think we could disable auto-fill on pages we detect as registration forms but leave autocomplete in case we guessed wrong.
Blocks: 1025703
Flags: firefox-backlog?
Keywords: regression
(In reply to Daniel Holbert [:dholbert] from comment #3)
> (In reply to Jim Meyer from comment #2)
> > I can confirm this is a blocking issue for us.
> > 
> > Firefox 39 changes the values of non-password related fields on our profile
> > page
> 
> Jim, are you talking about the same website as comment 0, or a different
> site?
> 

The site is our own product. The issue is with the user profile editor. For some reason, FF autofills the first text input in the DOM with my username. The field has no id, but a name that was assigned by JSF (Java Server Faces by Oracle/SUN). We use the field along with "display: none" to track the user scroll position between full page submits:

<form enctype="application/x-www-form-urlencoded" accept-charset="UTF-8" action="/admin/editor/...../userprofileeditor.xhtml" method="post" name="j_id-1175986798_136c2aa7" id="j_id-1175986798_136c2aa7">

<input type="hidden" value="j_id-1175986798_136c2aa7" name="j_id-1175986798_136c2aa7">

<input type="text" class="editor_scroll" value="*****FF autofills this value with my user name*****" name="j_id-1175986798_136c2aa7xj_id-1175986798_136c2889">

....
</form>
Jim, can you file your own bug as I think I have a solution for you. Could you include in your snippet up to the first <input type-password>?

File a password manager bug here: https://bugzilla.mozilla.org/enter_bug.cgi?product=Toolkit&component=Password Manager
(In reply to Matthew N. [:MattN] from comment #8)
> Jim, can you file your own bug as I think I have a solution for you. Could
> you include in your snippet up to the first <input type-password>?
> 
> File a password manager bug here:
> https://bugzilla.mozilla.org/enter_bug.
> cgi?product=Toolkit&component=Password Manager

Matt, I just changed the editor_scroll input to be type=hidden, and FF now leaves it alone. Should I still file the bug?
If that works for you then maybe it's not necessary. If that page was to edit another existing user, I was going to recommend having that user's username in the <input> immediately preceding their password field. Then the password manager will know that the password field there is associated with that other account for the purposes of saving or (not) filling.
(In reply to Matthew N. [:MattN] from comment #10)
> If that works for you then maybe it's not necessary. If that page was to
> edit another existing user, I was going to recommend having that user's
> username in the <input> immediately preceding their password field. Then the
> password manager will know that the password field there is associated with
> that other account for the purposes of saving or (not) filling.

Yeah, I think the test case that Sergei has attached is representative of the issue we saw. I just commented to confirm that we saw a similar problem with our site, so no need for an additional issue.
Flags: needinfo?(dolske)
MattN, I think that Bug 1119554 should get broken down and the high priority bits be put in backlog.

It sounds like the site has it under control, but these kind of cases were the motivation for "notUsername" support in a recipe system. Probably worth filing a bug on that.
Flags: needinfo?(ckarlof)
Blocks: 1337403
Depends on: 917325
Priority: -- → P3
Whiteboard: [passwords:heuristics]

I'm not sure if this is the right place to comment as I've seen many bugs related to the same topic. I just want to add that I have been experiencing the same issue. In any form that has an input[type=password] Firefox auto-fills with a saved password even if the field is not asking for a password, and it then fills the preceding input[type=text] with a username even if the field is not asking for a username. This is very problematic. Not all password type fields are for logins. Often they are used for obfuscating other sensitive data. And to assume that a preceding text field must be username is just wrong. Since Firefox fills these without any actions on the user’s behalf or notification that it happened, often users don't even realize it happened causing erroneous data to be stored in, i.e., user profiles or submitted in non-login forms. In short, Firefox should only ever auto-fill username and password in login forms. There are bug reports going back at least 10 years with the same issue 499223.

The "new person" page should use the autocomplete="new-password" attribute on <input type=password> to prevent autofilling saved credentials. It will be honoured in Firefox 67+.

(In reply to Jon from comment #14)

We are addressing your issues in various ways such as bug 1189524 and bug 917325.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.