Closed Bug 446109 Opened 12 years ago Closed 11 years ago
Password manager can overwrite an existing password in an input
If a page has a login form, but the password field already has a value, the password manager will ignore that value and overwrite whatever's there when filling the form. If there's already a value filled in, we should assume it's there for a reason and not overwrite it. Bug 327977 fixed this for the case where the there's no username field.
This patch was originally added to bug 444515, but there's other stuff going on there, so I'd rather have a bug just for this issue. [This patch is on top of the fixes for bug 327977 and bug 400795.] From bug 444515 comment 5: I remember talking about this before somewhere, but can't find it. Guess it never got filed as a bug. Anyway. There's ambiguity when we fill in a 2-password form like: <form> <input name="username"> <input name="pw1" type="password"> <input name="pw2" type="password"> </form> Possible uses for such a form: 1. The first pwfield is for the user's password, and the second pwfield is for the user's PIN (or some other secondary authentication). We want to fill the first field with the one password we store with the login. (The user will have to enter the 2nd auth manually, unsupported). 2. This could be a poorly designed change-password field, without confirmation. We want to fill in the first field, and the user will enter their password in the second field (and pray they don't make a typo!) 3. This could be a properly-designed change-password field, where the user is already known to be logged in (or otherwise has access to change the password), so the current password is not needed and we should ideally not fill in anything.
Assignee: nobody → dolske
Status: NEW → ASSIGNED
Attachment #330312 - Flags: review?(gavin.sharp)
Comment on attachment 330312 [details] [diff] [review] Patch v.1 Don't forget bug 327977 comment 4.
Attachment #330312 - Flags: review?(gavin.sharp) → review+
Pushed changeset eb0decf65376. Enabled previously-disabled, per last comment. Also changed the expected results for a few tests... Some existing tests had the password field prefilled with the expected password, and checked to see that we would fill in the username. This seems unlikely to happen in the real world, and it's probably just safer to bail out when we see a prefilled password (since it would seem the page is already weird). I suppose we can revisit this if it turns out there's a use/need for it.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [need review gavin]
Target Milestone: mozilla1.9.1 → mozilla1.9.1b1
You need to log in before you can comment on or make changes to this bug.