Bug 386150 - Can't save a login containing only a password. r=gavin, a=mconnor

This commit is contained in:
dolske@mozilla.com 2007-09-20 18:52:44 -07:00
parent bc1961884c
commit 6cbe32641f
10 changed files with 197 additions and 135 deletions

View File

@ -344,8 +344,9 @@ LoginManager.prototype = {
if (login.hostname == null || login.hostname.length == 0)
throw "Can't add a login with a null or empty hostname.";
if (login.username == null || login.username.length == 0)
throw "Can't add a login with a null or empty username.";
// For logins w/o a username, set to "", not null.
if (login.username == null)
throw "Can't add a login with a null username.";
if (login.password == null || login.password.length == 0)
throw "Can't add a login with a null or empty password.";
@ -585,8 +586,14 @@ LoginManager.prototype = {
}
// If too few or too many fields, bail out.
if (pwFields.length == 0 || pwFields.length > 3)
if (pwFields.length == 0) {
this.log("(form ignored -- no password fields.)");
return null;
} else if (pwFields.length > 3) {
this.log("(form ignored -- too many password fields. [got " +
pwFields.length + "])");
return null;
}
return pwFields;
},
@ -609,13 +616,13 @@ LoginManager.prototype = {
* that is being changed.
*/
_getFormFields : function (form, isSubmission) {
var usernameField = null;
// Locate the password field(s) in the form. Up to 3 supported.
// If there's no password field, there's nothing for us to do.
var pwFields = this._getPasswordFields(form, isSubmission);
if (!pwFields) {
this.log("(form ignored -- either 0 or >3 pw fields.)");
if (!pwFields)
return [null, null, null];
}
// Locate the username field in the form by searching backwards
@ -624,7 +631,7 @@ LoginManager.prototype = {
// already logged in to the site.
for (var i = pwFields[0].index - 1; i >= 0; i--) {
if (form.elements[i].type == "text") {
var usernameField = form.elements[i];
usernameField = form.elements[i];
break;
}
}
@ -706,24 +713,6 @@ LoginManager.prototype = {
};
// local helper function
function findExistingLogin(pwmgr, hostname,
formSubmitURL, usernameField) {
var searchLogin = new pwmgr._nsLoginInfo();
searchLogin.init(hostname, formSubmitURL, null,
usernameField.value, "",
usernameField.name, "");
var logins = pwmgr.findLogins({}, hostname, formSubmitURL, null);
var existingLogin;
var found = logins.some(function(l) {
existingLogin = l;
return searchLogin.equalsIgnorePassword(l);
});
return (found ? existingLogin : null);
}
function getPrompter(aWindow) {
var prompterSvc = Cc["@mozilla.org/login-manager/prompter;1"].
createInstance(Ci.nsILoginManagerPrompter);
@ -731,8 +720,6 @@ LoginManager.prototype = {
return prompterSvc;
}
var doc = form.ownerDocument;
var win = doc.defaultView;
@ -771,9 +758,9 @@ LoginManager.prototype = {
var formLogin = new this._nsLoginInfo();
formLogin.init(hostname, formSubmitURL, null,
(usernameField ? usernameField.value : null),
(usernameField ? usernameField.value : ""),
newPasswordField.value,
(usernameField ? usernameField.name : null),
(usernameField ? usernameField.name : ""),
newPasswordField.name);
// If we didn't find a username field, but seem to be changing a
@ -814,22 +801,60 @@ LoginManager.prototype = {
return;
}
if (!usernameField && !oldPasswordField) {
this.log("XXX not handled yet");
return;
// Look for an existing login that matches the form login.
var existingLogin = null;
var logins = this.findLogins({}, hostname, formSubmitURL, null);
for (var i = 0; i < logins.length; i++) {
var same, login = logins[i];
// If one login has a username but the other doesn't, ignore
// the username when comparing and only match if they have the
// same password. Otherwise, compare the logins and match even
// if the passwords differ.
if (!login.username && formLogin.username) {
var restoreMe = formLogin.username;
formLogin.username = "";
same = formLogin.equals(login);
formLogin.username = restoreMe;
} else if (!formLogin.username && login.username) {
formLogin.username = login.username;
same = formLogin.equals(login);
formLogin.username = ""; // we know it's always blank.
} else {
same = formLogin.equalsIgnorePassword(login);
}
if (same) {
existingLogin = login;
break;
}
}
// We have a username. Look for an existing login that matches the
// form data (other than the password, which might be different)
existingLogin = findExistingLogin(this, hostname, formSubmitURL,
usernameField);
if (existingLogin) {
this.log("Found an existing login matching this form submission");
// Change password if needed
/*
* Change password if needed.
*
* If the login has a username, change the password w/o prompting
* (because we can be fairly sure there's only one password
* associated with the username). But for logins without a
* username, ask the user... Some sites use a password-only "login"
* in different contexts (enter your PIN, answer a security
* question, etc), and without a username we can't be sure if
* modifying an existing login is the right thing to do.
*/
if (existingLogin.password != formLogin.password) {
this.log("...Updating password for existing login.");
this.modifyLogin(existingLogin, formLogin);
if (formLogin.username) {
this.log("...Updating password for existing login.");
this.modifyLogin(existingLogin, formLogin);
} else {
this.log("...passwords differ, prompting to change.");
prompter = getPrompter(win);
prompter.promptToChangePassword(existingLogin, formLogin);
}
}
return;
@ -966,9 +991,10 @@ LoginManager.prototype = {
if (autofillForm) {
// If username was specified in the form, only fill in the
// password if we find a matching login.
if (usernameField && usernameField.value) {
// If username was specified in the form, only fill in the
// password if we find a matching login.
var username = usernameField.value;
var matchingLogin;
@ -979,6 +1005,19 @@ LoginManager.prototype = {
if (found)
passwordField.value = matchingLogin.password;
} else if (usernameField && logins.length == 2) {
// Special case, for sites which have a normal user+pass
// login *and* a password-only login (eg, a PIN)...
// When we have a username field and 1 of 2 available
// logins is password-only, go ahead and prefill the
// one with a username.
if (!logins[0].username && logins[1].username) {
usernameField.value = logins[1].username;
passwordField.value = logins[1].password;
} else if (!logins[1].username && logins[0].username) {
usernameField.value = logins[0].username;
passwordField.value = logins[0].password;
}
} else if (logins.length == 1) {
if (usernameField)
usernameField.value = logins[0].username;

View File

@ -462,9 +462,15 @@ LoginManagerPrompter.prototype = {
promptToChangePassword : function (aOldLogin, aNewLogin) {
const buttonFlags = Ci.nsIPrompt.STD_YES_NO_BUTTONS;
var dialogText = this._getLocalizedString(
var dialogText;
if (aOldLogin.username)
dialogText = this._getLocalizedString(
"passwordChangeText",
[aOldLogin.username]);
else
dialogText = this._getLocalizedString(
"passwordChangeTextNoUser");
var dialogTitle = this._getLocalizedString(
"passwordChangeTitle");

View File

@ -799,8 +799,8 @@ LoginManagerStorage_legacy.prototype = {
break;
// If decryption failed (corrupt entry?) skip it.
// XXX remove it from the original list entirely?
if (!username || !password)
// Note that we allow password-only logins, so username con be "".
if (username == null || !password)
continue;
// We could set the decrypted values on a copy of the object, to

View File

@ -57,6 +57,7 @@ MOCHI_TESTS = \
test_basic_form_2pw_2.html \
test_basic_form_3pw_1.html \
test_basic_form_autocomplete.html \
test_basic_form_pwonly.html \
test_bug_227640.html \
test_bug_242956.html \
test_bug_360493_1.html \

View File

@ -29,3 +29,78 @@ function $_(formNum, name) {
return element;
}
/*
* checkForm
*
* Check a form for expected values. If an argument is null, a field's
* expected value will be the default value.
*
* <form id="form#">
* checkForm(#, "foo");
*/
function checkForm(formNum, val1, val2, val3) {
var e, form = document.getElementById("form" + formNum);
ok(form, "Locating form " + formNum);
var numToCheck = arguments.length - 1;
if (!numToCheck--)
return;
e = form.elements[0];
if (val1 == null)
is(e.value, e.defaultValue, "Test default value of field " + e.name +
" in form " + formNum);
else
is(e.value, val1, "Test value of field " + e.name +
" in form " + formNum);
if (!numToCheck--)
return;
e = form.elements[1];
if (val2 == null)
is(e.value, e.defaultValue, "Test default value of field " + e.name +
" in form " + formNum);
else
is(e.value, val2, "Test value of field " + e.name +
" in form " + formNum);
if (!numToCheck--)
return;
e = form.elements[2];
if (val3 == null)
is(e.value, e.defaultValue, "Test default value of field " + e.name +
" in form " + formNum);
else
is(e.value, val3, "Test value of field " + e.name +
" in form " + formNum);
}
/*
* checkUnmodifiedForm
*
* Check a form for unmodified values from when page was loaded.
*
* <form id="form#">
* checkUnmodifiedForm(#);
*/
function checkUnmodifiedForm(formNum) {
var form = document.getElementById("form" + formNum);
ok(form, "Locating form " + formNum);
for (var i = 0; i < form.elements.length; i++) {
var ele = form.elements[i];
// No point in checking form submit/reset buttons.
if (ele.type == "submit" || ele.type == "reset")
continue;
is(ele.value, ele.defaultValue, "Test to default value of field " +
ele.name + " in form " + formNum);
}
}

View File

@ -64,7 +64,11 @@ ok(login != null, "Adding login");
login.init("http://localhost:8888", "http://localhost:8888", null,
"testuser", "testpass", "uname", "pword");
pwmgr.addLogin(login);
try {
pwmgr.addLogin(login);
} catch (e) {
ok(false, "addLogin threw: " + e);
}
// Simple check to see if everything was added fine.
var logins = pwmgr.getAllLogins({});

View File

@ -2800,34 +2800,6 @@ function startTest() {
SimpleTest.finish();
}
function checkForm(formNum, val1, val2, val3) {
var form = document.getElementById("form" + formNum);
ok(form, "Locating form " + formNum);
is(form.elements[0].value, val1, "Test value of field " +
form.elements[0].name + " in form " + formNum);
if (val2 != null)
is(form.elements[1].value, val2, "Test value of field " +
form.elements[1].name + " in form " + formNum);
if (val3 != null)
is(form.elements[2].value, val3, "Test value of field " +
form.elements[2].name + " in form " + formNum);
}
function checkUnmodifiedForm(formNum) {
var form = document.getElementById("form" + formNum);
ok(form, "Locating form " + formNum);
// Ignore last two elements, they're the two buttons.
for (var i = 0; i < form.elements.length - 2; i++) {
var ele = form.elements[i];
is(ele.value, ele.defaultValue, "Test to default value of field " +
ele.name + " in form " + formNum);
}
}
window.onload = startTest;

View File

@ -15656,42 +15656,6 @@ function startTest() {
SimpleTest.finish();
}
function checkForm(formNum, val1, val2, val3) {
var e, form = document.getElementById("form" + formNum);
ok(form, "Locating form " + formNum);
e = form.elements[0];
if (!val1)
is(e.value, e.defaultValue, "Test default value of field " + e.name +
" in form " + formNum);
else
is(e.value, val1, "Test value of field " + e.name +
" in form " + formNum);
e = form.elements[1];
if (!val2)
is(e.value, e.defaultValue, "Test default value of field " + e.name +
" in form " + formNum);
else
is(e.value, val2, "Test value of field " + e.name +
" in form " + formNum);
e = form.elements[2];
if (!val3)
is(e.value, e.defaultValue, "Test default value of field " + e.name +
" in form " + formNum);
else
is(e.value, val3, "Test value of field " + e.name +
" in form " + formNum);
}
function checkUnmodifiedForm(formNum) {
checkForm(formNum, null, null, null);
}
window.onload = startTest;

View File

@ -88,7 +88,7 @@ function restoreForm() {
// Check for expected username/password in form.
function checkForm(expectedUsername, expectedPassword) {
function checkACForm(expectedUsername, expectedPassword) {
is(uname.value, expectedUsername, "Checking form username");
is(pword.value, expectedPassword, "Checking form password");
}
@ -130,7 +130,7 @@ function runTest(testNum) {
switch(testNum) {
case 1:
// Make sure initial form is empty.
checkForm("", "");
checkACForm("", "");
// Trigger autocomplete popup
restoreForm();
doKey("down");
@ -139,9 +139,9 @@ function runTest(testNum) {
case 2:
// Check first entry
doKey("down");
checkForm("", ""); // value shouldn't update
checkACForm("", ""); // value shouldn't update
doKey("return"); // not "enter"!
checkForm("tempuser1", "temppass1");
checkACForm("tempuser1", "temppass1");
// Trigger autocomplete popup
restoreForm();
@ -153,7 +153,7 @@ function runTest(testNum) {
doKey("down");
doKey("down");
doKey("return"); // not "enter"!
checkForm("testuser2", "testpass2");
checkACForm("testuser2", "testpass2");
// Trigger autocomplete popup
restoreForm();
@ -166,7 +166,7 @@ function runTest(testNum) {
doKey("down");
doKey("down");
doKey("return");
checkForm("testuser3", "testpass3");
checkACForm("testuser3", "testpass3");
// Trigger autocomplete popup
restoreForm();
@ -180,7 +180,7 @@ function runTest(testNum) {
doKey("down");
doKey("down");
doKey("return");
checkForm("zzzuser4", "zzzpass4");
checkACForm("zzzuser4", "zzzpass4");
// Trigger autocomplete popup
restoreForm();
@ -196,7 +196,7 @@ function runTest(testNum) {
doKey("down"); // deselects
doKey("down");
doKey("return");
checkForm("tempuser1", "temppass1");
checkACForm("tempuser1", "temppass1");
// Trigger autocomplete popup
restoreForm();
@ -207,7 +207,7 @@ function runTest(testNum) {
// Check the last entry via arrow-up
doKey("up");
doKey("return");
checkForm("zzzuser4", "zzzpass4");
checkACForm("zzzuser4", "zzzpass4");
// Trigger autocomplete popup
restoreForm();
@ -220,7 +220,7 @@ function runTest(testNum) {
doKey("up"); // selects nothing!
doKey("up"); // select last entry
doKey("return");
checkForm("zzzuser4", "zzzpass4");
checkACForm("zzzuser4", "zzzpass4");
// Trigger autocomplete popup
restoreForm();
@ -238,7 +238,7 @@ function runTest(testNum) {
doKey("up"); // deselects
doKey("up"); // last entry
doKey("return");
checkForm("zzzuser4", "zzzpass4");
checkACForm("zzzuser4", "zzzpass4");
// Trigger autocomplete popup
restoreForm();
@ -249,7 +249,7 @@ function runTest(testNum) {
// Set first entry w/o triggering autocomplete
doKey("down");
doKey("right");
checkForm("tempuser1", ""); // empty password
checkACForm("tempuser1", ""); // empty password
// Trigger autocomplete popup
restoreForm();
@ -260,7 +260,7 @@ function runTest(testNum) {
// Set first entry w/o triggering autocomplete
doKey("down");
doKey("left");
checkForm("tempuser1", ""); // empty password
checkACForm("tempuser1", ""); // empty password
// Trigger autocomplete popup
restoreForm();
@ -273,7 +273,7 @@ function runTest(testNum) {
doKey("down");
doKey("page_up");
doKey("return");
checkForm("tempuser1", "temppass1");
checkACForm("tempuser1", "temppass1");
// Trigger autocomplete popup
restoreForm();
@ -285,7 +285,7 @@ function runTest(testNum) {
doKey("down");
doKey("page_down");
doKey("return");
checkForm("zzzuser4", "zzzpass4");
checkACForm("zzzuser4", "zzzpass4");
// Trigger autocomplete popup
restoreForm();
@ -299,7 +299,7 @@ function runTest(testNum) {
// doKey("down");
// doKey("down");
// doKey("escape");
// checkForm("", "");
// checkACForm("", "");
// Trigger autocomplete popup
// restoreForm();
@ -323,11 +323,11 @@ function runTest(testNum) {
// On Win/Linux, shift-backspace does not work, delete and shift-delete do.
doKey("delete", shiftModifier);
checkForm("zzzuser4", ""); // why does a value get set here?
checkACForm("zzzuser4", ""); // why does a value get set here?
numLogins = pwmgr.countLogins("http://localhost:8888", "http://autocomplete:8888", null);
is(numLogins, 3, "Correct number of logins after deleting one");
doKey("return");
checkForm("testuser2", "testpass2");
checkACForm("testuser2", "testpass2");
// Trigger autocomplete popup
restoreForm();
@ -338,7 +338,7 @@ function runTest(testNum) {
// Check the new first entry (of 3)
doKey("down");
doKey("return");
checkForm("testuser2", "testpass2");
checkACForm("testuser2", "testpass2");
// Trigger autocomplete popup
restoreForm();
@ -350,11 +350,11 @@ function runTest(testNum) {
doKey("down");
doKey("down");
doKey("delete", shiftModifier);
checkForm("testuser2", ""); // XXX why does a value get set here?
checkACForm("testuser2", ""); // XXX why does a value get set here?
numLogins = pwmgr.countLogins("http://localhost:8888", "http://autocomplete:8888", null);
is(numLogins, 2, "Correct number of logins after deleting one");
doKey("return");
checkForm("zzzuser4", "zzzpass4");
checkACForm("zzzuser4", "zzzpass4");
// Trigger autocomplete popup
restoreForm();
@ -365,7 +365,7 @@ function runTest(testNum) {
// Check the new second entry (of 2)
doKey("down");
doKey("return");
checkForm("testuser2", "testpass2");
checkACForm("testuser2", "testpass2");
// Trigger autocomplete popup
restoreForm();
@ -377,11 +377,11 @@ function runTest(testNum) {
doKey("down");
doKey("down");
doKey("delete", shiftModifier);
checkForm("testuser2", ""); // XXX why does a value get set here?
checkACForm("testuser2", ""); // XXX why does a value get set here?
numLogins = pwmgr.countLogins("http://localhost:8888", "http://autocomplete:8888", null);
is(numLogins, 1, "Correct number of logins after deleting one");
doKey("return");
checkForm("testuser2", "testpass2");
checkACForm("testuser2", "testpass2");
// Trigger autocomplete popup
restoreForm();
@ -392,7 +392,7 @@ function runTest(testNum) {
// Check the new second entry (of 2)
doKey("down");
doKey("return");
checkForm("testuser2", "testpass2");
checkACForm("testuser2", "testpass2");
// Trigger autocomplete popup
restoreForm();
@ -404,7 +404,7 @@ function runTest(testNum) {
doKey("down");
doKey("delete", shiftModifier);
//doKey("return");
checkForm("", "");
checkACForm("", "");
numLogins = pwmgr.countLogins("http://localhost:8888", "http://autocomplete:8888", null);
is(numLogins, 0, "Correct number of logins after deleting one");

View File

@ -48,6 +48,7 @@ notifyBarRememberButtonText = Remember
notifyBarRememberButtonAccessKey = R
passwordChangeTitle = Confirm Password Change
passwordChangeText = Would you like to have Password Manager change the stored password for %S?
passwordChangeTextNoUser = Would you like to have Password Manager change the stored password for this login?
userSelectText = Please confirm which user you are changing the password for
hidePasswords=Hide Passwords
showPasswords=Show Passwords