229762 - If username field is prefilled by page to a value that matches a stored signon, pwmgr should prefill password

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect, P2)

Description

[Ben Goodger (use ben at mozilla dot org for email)]

If I go to dell.com and try to sign in to my account there, dell.com uses a
cookie with my username saved to prefill the username field on the login page.
This causes pwmgr to NOT fill in the password because it thinks there's a
default value and it shouldn't meddle, even though the default value matches a
username that the pwmgr has stored. 

I think the logic should work like this:

if (page default value)
  if (page default value == any of the saved signons)
    pwmgr fill password for that signon
  else
    do nothing, attach autocomplete widget.
else
  if (more than one signon)
    do nothing, attach autocomplete widget
  else
    prefill username and password

This means that in the case where there is more than one signon saved, cookies
set by a page have the power to select a password from the available set. This
would streamline entry to sites where users have more than one signon but only
one is frequently used.

Comment 1

[Ben Goodger (use ben at mozilla dot org for email)]

Twiddling bits. 

Comment 2

[Jesse Ruderman]

The fix for this might also fix bug 218135, "Going back to page with saved
password does not refill password".

Comment 3

[Brian Ryner (not reading)]

*** Bug 233723 has been marked as a duplicate of this bug. ***

Comment 4

[WADA:World Anti-bad-Duping Agency]

Operational workaround for prefilled ID ;
 - Click ID field in the form, then Firefox fills password
This workaround was effective at amazaon.co.jp site.

Comment 5

[Seth Kinast]

Confirming workaround. This should be upgraded from enhancement, though... it's
really annoying!

Comment 6

[Ian Pottinger]

In bug 242239 I describe a second case of this bug, where the username field is
prefilled by the page with a value that is a leading-substring of a stored signon.

This bug and case 1 in bug 242239 can be considered a subcase of what I
described above - meaning the leading-substring *is* the complete username

Comment 7

[Ben Goodger (use ben at mozilla dot org for email)]

*** Bug 218135 has been marked as a duplicate of this bug. ***

Comment 8

[Brian Ryner (not reading)]

Attachment: patch

Instead of bailing when there's a prefilled value, check whether it's the same
as the username we're looking at.  If it is, prefill the corresponding
password.  Also, don't bail out of the loop as soon as we discover a second
matching username, since there may be a later one that matches the prefilled
value.	Instead, just keep track of whether we've attached the autocomplete
listener yet.  Finally, this patch changes the behavior when DecryptData()
fails to avoid multiple prompts if the user is trying to cancel the master
password dialog.  If decryption fails, we bail immediately.

I tested this at paypal which I believe prefills the username value if you log
out and then go to log in again.

Comment 9

[Ben Goodger (use ben at mozilla dot org for email)]

will test with this tomorrow on some of the usual suspects

Comment 10

[Brian Ryner (not reading)]

checked in per ben's request to get wider testing

Comment 11

[Jeff Walden [:Waldo]]

Comment on attachment 154244 [details] [diff] [review]
patch

Clearing obsolete review request...

Comment 12

[Frank Wein [:mcsmurf]]

(In reply to comment #0)
> if (page default value)
>   if (page default value == any of the saved signons)
>     pwmgr fill password for that signon
>   else
>     do nothing, attach autocomplete widget.

Might it be the else case has not been fixed/regressed? See for example the
testcase under http://www-lehre.inf.uos.de/~rfreund/firefox-pw-test.php. Type in
some username/password pair, save it and revisit website. Observe username
value="test" gets overwritten, per Comment 0 this shouldn't happen IMHO (has
default value, saved signon doesn't match default value).

Comment 13

[Brian Ryner (not reading)]

reopening per reporter comments.

We need to make sure this works as expected or it can become a major usability
problem with password manager.  Nominating for 1.1 so we don't lose track of it.

Comment 14

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #12)
Comfirmed problem of comment #12 on Mozilla trunk 2005041005(Win-2K) too. 
When I changed pre-filled value of "test" to other string(say "abcd"), password
manager saved with Username of "abcd".
And the value of "test" in following <input> field was replaced by "abcd" when
visiting the test site again and password was filled automatically. 
 <input type="text" name="username" value="test" />
Password managers said "abcd" is the Username for this site, and "Show Password"
displayed saved password correctly.

Fix for this bug is applied to Mozilla too?
(In my test in the past, this bug's original problem didn't occur on Mozilla and
it was Firefox only problem.)
Regression by fix for this bug? Or independent problem?

Comment 15

[Frank Wein [:mcsmurf]]

(In reply to comment #14)
> (In reply to comment #12)
> Comfirmed problem of comment #12 on Mozilla trunk 2005041005(Win-2K) too.
[...]
> Regression by fix for this bug? Or independent problem?

Independent problem, Firefox uses different code for Password Manager.

Comment 16

[Phil Ringnalda (:philor)]

*** Bug 294458 has been marked as a duplicate of this bug. ***

Comment 17

[Phil Ringnalda (:philor)]

As bug 294458 noted, this is extra ugly if the filled-in username that we are
clobbering is actually a readonly field - we don't attach the autocomplete
widget to a readonly field if you have two usernames saved, but if you only have
one we clobber the filled-in value and you're stuck with no way of changing it.

Comment 18

[Ulrich Windl]

Just another comment: I had similar problems with mailman: The web interface
requests a "list administration password" (the user is implicit the list's
name). Firefox then prompt "Do you want to change the default password for
XXXX?". In some other cases the password isn't rememebered (or suggested to do
so) at all.
I don't know if you can do much about that, but when you are on it...

Comment 19

[Mike Connor [:mconnor]]

lets take a crack at getting a fix here, the readonly case sounds wonderfully
annoying.

Comment 20

[Mike Connor [:mconnor]]

most likely not going to make it for 1.5

hopefully we can get some traction on this early in 

Comment 21

[Phil Ringnalda (:philor)]

*** Bug 265985 has been marked as a duplicate of this bug. ***

Comment 22

[Phil Ringnalda (:philor)]

Marking depends on bug 270558, which also covers the part that this is reopened
for, complete with testcases, since I keep finding just one or the other instead
of both.

Comment 23

[Adam Guthrie]

*** Bug 312877 has been marked as a duplicate of this bug. ***

Comment 24

[Phil Ringnalda (:philor)]

Fixed in bug 270558, so back to FIXED