Bug 1120129 - Allow per-site recipes to adjust the username/password field detection for autofill. r=dolske

This commit is contained in:
Matthew Noorenberghe 2015-03-17 17:18:14 -07:00
parent a3b6e76f8d
commit acaf166985
9 changed files with 409 additions and 52 deletions

View File

@ -8,16 +8,16 @@
this.EXPORTED_SYMBOLS = [ "LoginManagerContent",
"UserAutoCompleteResult" ];
const Ci = Components.interfaces;
const Cr = Components.results;
const Cc = Components.classes;
const Cu = Components.utils;
const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/PrivateBrowsingUtils.jsm");
Cu.import("resource://gre/modules/Promise.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "LoginRecipesContent",
"resource://gre/modules/LoginRecipes.jsm");
// These mirror signon.* prefs.
var gEnabled, gDebug, gAutofillForms, gStoreWhenAutocompleteOff;
@ -183,8 +183,11 @@ var LoginManagerContent = {
switch (msg.name) {
case "RemoteLogins:loginsFound": {
let loginsFound = jsLoginsToXPCOM(msg.data.logins);
request.promise.resolve({ form: request.form,
loginsFound: loginsFound});
request.promise.resolve({
form: request.form,
loginsFound: loginsFound,
recipes: msg.data.recipes,
});
break;
}
@ -196,7 +199,14 @@ var LoginManagerContent = {
}
},
_asyncFindLogins: function(form, options) {
/**
* Get relevant logins and recipes from the parent
*
* @param {HTMLFormElement} form - form to get login data for
* @param {Object} options
* @param {boolean} options.showMasterPassword - whether to show a master password prompt
*/
_getLoginDataFromParent: function(form, options) {
let doc = form.ownerDocument;
let win = doc.defaultView;
@ -257,16 +267,16 @@ var LoginManagerContent = {
let form = event.target;
log("onFormPassword for", form.ownerDocument.documentURI);
this._asyncFindLogins(form, { showMasterPassword: true })
this._getLoginDataFromParent(form, { showMasterPassword: true })
.then(this.loginsFound.bind(this))
.then(null, Cu.reportError);
},
loginsFound: function({ form, loginsFound }) {
loginsFound: function({ form, loginsFound, recipes }) {
let doc = form.ownerDocument;
let autofillForm = gAutofillForms && !PrivateBrowsingUtils.isContentWindowPrivate(doc.defaultView);
this._fillForm(form, autofillForm, false, false, loginsFound);
this._fillForm(form, autofillForm, false, false, loginsFound, recipes);
},
/*
@ -307,9 +317,9 @@ var LoginManagerContent = {
var [usernameField, passwordField, ignored] =
this._getFormFields(acForm, false);
if (usernameField == acInputField && passwordField) {
this._asyncFindLogins(acForm, { showMasterPassword: false })
.then(({ form, loginsFound }) => {
this._fillForm(form, true, true, true, loginsFound);
this._getLoginDataFromParent(acForm, { showMasterPassword: false })
.then(({ form, loginsFound, recipes }) => {
this._fillForm(form, true, true, true, loginsFound, recipes);
})
.then(null, Cu.reportError);
} else {
@ -377,14 +387,15 @@ var LoginManagerContent = {
},
/*
* _getFormFields
*
/**
* Returns the username and password fields found in the form.
* Can handle complex forms by trying to figure out what the
* relevant fields are.
*
* Returns: [usernameField, newPasswordField, oldPasswordField]
* @param {HTMLFormElement} form
* @param {bool} isSubmission
* @param {Set} recipes
* @return {Array} [usernameField, newPasswordField, oldPasswordField]
*
* usernameField may be null.
* newPasswordField will always be non-null.
@ -393,25 +404,52 @@ var LoginManagerContent = {
* change-password field, with oldPasswordField containing the password
* that is being changed.
*/
_getFormFields : function (form, isSubmission) {
_getFormFields : function (form, isSubmission, recipes) {
var usernameField = null;
var pwFields = null;
var fieldOverrideRecipe = LoginRecipesContent.getFieldOverrides(recipes, form);
if (fieldOverrideRecipe) {
var pwOverrideField = LoginRecipesContent.queryLoginField(
form,
fieldOverrideRecipe.passwordSelector
);
if (pwOverrideField) {
pwFields = [{
index : [...pwOverrideField.form.elements].indexOf(pwOverrideField),
element : pwOverrideField,
}];
}
// 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)
var usernameOverrideField = LoginRecipesContent.queryLoginField(
form,
fieldOverrideRecipe.usernameSelector
);
if (usernameOverrideField) {
usernameField = usernameOverrideField;
}
}
if (!pwFields) {
// 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.
pwFields = this._getPasswordFields(form, isSubmission);
}
if (!pwFields) {
return [null, null, null];
}
// Locate the username field in the form by searching backwards
// from the first passwordfield, assume the first text field is the
// username. We might not find a username field if the user is
// already logged in to the site.
for (var i = pwFields[0].index - 1; i >= 0; i--) {
var element = form.elements[i];
if (this._isUsernameFieldType(element)) {
usernameField = element;
break;
if (!usernameField) {
// Locate the username field in the form by searching backwards
// from the first passwordfield, assume the first text field is the
// username. We might not find a username field if the user is
// already logged in to the site.
for (var i = pwFields[0].index - 1; i >= 0; i--) {
var element = form.elements[i];
if (this._isUsernameFieldType(element)) {
usernameField = element;
break;
}
}
}
@ -570,21 +608,21 @@ var LoginManagerContent = {
{ openerWin: opener });
},
/*
* _fillform
*
/**
* Attempt to find the username and password fields in a form, and fill them
* in using the provided logins.
* in using the provided logins and recipes.
*
* - autofillForm denotes if we should fill the form in automatically
* - clobberPassword controls if an existing password value can be
* overwritten
* - userTriggered is an indication of whether this filling was triggered by
* the user
* - foundLogins is an array of nsILoginInfo that could be used for the form
* @param {HTMLFormElement} form
* @param {bool} autofillForm denotes if we should fill the form in automatically
* @param {bool} clobberPassword controls if an existing password value can be
* overwritten
* @param {bool} userTriggered is an indication of whether this filling was triggered by
* the user
* @param {nsILoginInfo[]} foundLogins is an array of nsILoginInfo that could be used for the form
* @param {Set} recipes that could be used to affect how the form is filled
*/
_fillForm : function (form, autofillForm, clobberPassword,
userTriggered, foundLogins) {
userTriggered, foundLogins, recipes) {
let ignoreAutocomplete = true;
const AUTOFILL_RESULT = {
FILLED: 0,
@ -621,7 +659,7 @@ var LoginManagerContent = {
// so that the user isn't prompted for a master password
// without need.
var [usernameField, passwordField, ignored] =
this._getFormFields(form, false);
this._getFormFields(form, false, recipes);
// Need a valid password field to do anything.
if (passwordField == null) {

View File

@ -4,11 +4,11 @@
"use strict";
this.EXPORTED_SYMBOLS = ["LoginRecipesParent"];
this.EXPORTED_SYMBOLS = ["LoginRecipesContent", "LoginRecipesParent"];
const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
const REQUIRED_KEYS = ["hosts"];
const OPTIONAL_KEYS = ["description", "pathRegex"];
const OPTIONAL_KEYS = ["description", "passwordSelector", "pathRegex", "usernameSelector"];
const SUPPORTED_KEYS = REQUIRED_KEYS.concat(OPTIONAL_KEYS);
Cu.importGlobalProperties(["URL"]);
@ -21,14 +21,29 @@ XPCOMUtils.defineLazyModuleGetter(this, "LoginHelper",
XPCOMUtils.defineLazyGetter(this, "log", () => LoginHelper.createLogger("LoginRecipes"));
function LoginRecipesParent() {
/**
* Create an instance of the object to manage recipes in the parent process.
* Consumers should wait until {@link initializationPromise} resolves before
* calling methods on the object.
*
* @constructor
* @param {boolean} [aOptions.defaults=true] whether to load default application recipes.
*/
function LoginRecipesParent(aOptions = { defaults: true }) {
if (Services.appinfo.processType != Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
throw new Error("LoginRecipesParent should only be used from the main process");
}
this._recipesByHost = new Map();
this.initializationPromise = Promise.resolve(this);
if (aOptions.defaults) {
// XXX: Bug 1134850 will handle reading recipes from a file.
this.initializationPromise = this.load(DEFAULT_RECIPES).then(resolve => {
return this;
});
} else {
this.initializationPromise = Promise.resolve(this);
}
}
LoginRecipesParent.prototype = {
@ -51,15 +66,21 @@ LoginRecipesParent.prototype = {
* @return {Promise} resolving when the recipes are loaded
*/
load(aRecipes) {
let recipeErrors = 0;
for (let rawRecipe of aRecipes.siteRecipes) {
try {
rawRecipe.pathRegex = rawRecipe.pathRegex ? new RegExp(rawRecipe.pathRegex) : undefined;
this.add(rawRecipe);
} catch (ex) {
recipeErrors++;
log.error("Error loading recipe", rawRecipe, ex);
}
}
if (recipeErrors) {
return Promise.reject(`There were ${recipeErrors} recipe error(s)`);
}
return Promise.resolve();
},
@ -93,8 +114,11 @@ LoginRecipesParent.prototype = {
throw new Error("'pathRegex' must be a regular expression");
}
if (recipe.description && typeof(recipe.description) != "string") {
throw new Error("'description' must be a string");
const OPTIONAL_STRING_PROPS = ["description", "passwordSelector", "usernameSelector"];
for (let prop of OPTIONAL_STRING_PROPS) {
if (recipe[prop] && typeof(recipe[prop]) != "string") {
throw new Error(`'${prop}' must be a string`);
}
}
// Add the recipe to the map for each host
@ -121,3 +145,93 @@ LoginRecipesParent.prototype = {
return hostRecipes;
},
};
let LoginRecipesContent = {
/**
* @param {Set} aRecipes - Possible recipes that could apply to the form
* @param {HTMLFormElement} aForm - We use a form instead of just a URL so we can later apply
* tests to the page contents.
* @return {Set} a subset of recipes that apply to the form with the order preserved
*/
_filterRecipesForForm(aRecipes, aForm) {
let formDocURL = aForm.ownerDocument.location;
let host = formDocURL.host;
let hostRecipes = aRecipes;
let recipes = new Set();
log.debug("_filterRecipesForForm", aRecipes);
if (!hostRecipes) {
return recipes;
}
for (let hostRecipe of hostRecipes) {
if (hostRecipe.pathRegex && !hostRecipe.pathRegex.test(formDocURL.pathname)) {
continue;
}
recipes.add(hostRecipe);
}
return recipes;
},
/**
* Given a set of recipes that apply to the host, choose the one most applicable for
* overriding login fields in the form.
*
* @param {Set} aRecipes The set of recipes to consider for the form
* @param {HTMLFormElement} aForm The form where login fields exist.
* @return {Object} The recipe that is most applicable for the form.
*/
getFieldOverrides(aRecipes, aForm) {
let recipes = this._filterRecipesForForm(aRecipes, aForm);
log.debug("getFieldOverrides: filtered recipes:", recipes);
if (!recipes.size) {
return null;
}
let chosenRecipe = null;
// Find the first (most-specific recipe that involves field overrides).
for (let recipe of recipes) {
if (!recipe.usernameSelector && !recipe.passwordSelector) {
continue;
}
chosenRecipe = recipe;
break;
}
return chosenRecipe;
},
/**
* @param {HTMLElement} aParent the element to query for the selector from.
* @param {CSSSelector} aSelector the CSS selector to query for the login field.
* @return {HTMLElement|null}
*/
queryLoginField(aParent, aSelector) {
if (!aSelector) {
return null;
}
let field = aParent.ownerDocument.querySelector(aSelector);
if (!field) {
log.warn("Login field selector wasn't matched:", aSelector);
return null;
}
if (!(field instanceof aParent.ownerDocument.defaultView.HTMLInputElement)) {
log.warn("Login field isn't an <input> so ignoring it:", aSelector);
return null;
}
return field;
},
};
const DEFAULT_RECIPES = {
"siteRecipes": [
{
"description": "okta uses a hidden password field to disable filling",
"hosts": ["mozilla.okta.com"],
"passwordSelector": "#pass-signin",
"pathRegex": "^(|\/login\/(login.htm|do\-login))$"
},
]
};

View File

@ -69,6 +69,8 @@ skip-if = os == "linux" || toolkit == 'android' # bug 934057
skip-if = os == "linux" || toolkit == 'android' #TIMED_OUT
[test_prompt_async.html]
skip-if = toolkit == 'android' #TIMED_OUT
[test_recipe_login_fields.html]
skip-if = buildapp == 'mulet' || buildapp == 'b2g'
[test_xhr.html]
skip-if = toolkit == 'android' #TIMED_OUT
[test_xml_load.html]

View File

@ -258,3 +258,25 @@ function dumpLogin(label, login) {
loginText += login.passwordField;
ok(true, label + loginText);
}
// Code to run when loaded as a chrome script in tests via loadChromeScript
if (this.addMessageListener) {
const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
var SpecialPowers = { Cc, Ci, Cr, Cu, };
var ok, is;
// Ignore ok/is in commonInit since they aren't defined in a chrome script.
ok = is = () => {};
Cu.import("resource://gre/modules/Task.jsm");
addMessageListener("setupParent", () => {
commonInit(true);
sendAsyncMessage("doneSetup");
});
addMessageListener("loadRecipes", Task.async(function* loadRecipes(recipes) {
var { LoginManagerParent } = Cu.import("resource://gre/modules/LoginManagerParent.jsm", {});
var recipeParent = yield LoginManagerParent.recipeParentPromise;
yield recipeParent.load(recipes);
sendAsyncMessage("loadedRecipes", recipes);
}));
}

View File

@ -0,0 +1,70 @@
<!DOCTYPE html>
<html>
<head>
<title>Test for recipes overriding login fields</title>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" href="/tests/SimpleTest/test.css" />
</head>
<body>
<script type="application/javascript;version=1.8">
SimpleTest.waitForExplicitFinish();
let parentScriptURL = SimpleTest.getTestFileURL("pwmgr_common.js");
mm = SpecialPowers.loadChromeScript(parentScriptURL);
// Tell the parent to setup test logins.
mm.sendAsyncMessage("setupParent");
// When the setup is done, load a recipe for this test.
mm.addMessageListener("doneSetup", function doneSetup() {
mm.sendAsyncMessage("loadRecipes", {
siteRecipes: [{
hosts: ["mochi.test:8888"],
usernameSelector: "input[name='uname1']",
passwordSelector: "input[name='pword2']",
}],
});
});
mm.addMessageListener("loadedRecipes", function loadedRecipes() {
// Append the form dynamically so autofill is triggered after setup above.
document.getElementById("content").innerHTML += `
<!-- form with recipe for the username and password -->
<form id="form1">
<input type="text" name="uname1" oninput="reportFill(this, true)">
<input type="text" name="uname2" oninput="reportFill(this, false)">
<input type="password" name="pword1" oninput="reportFill(this, false)">
<input type="password" name="pword2" oninput="reportFill(this, true)">
</form>`;
});
const EXPECTED_FILLS = 4;
let fillCount = 0;
function reportFill(element, expected) {
ok(expected, `${element.name} was filled`);
if (++fillCount == EXPECTED_FILLS) {
SimpleTest.finish();
} else if (fillCount == 2) {
document.getElementById("content").innerHTML = `
<!-- Fallback to the default heuristics since the selectors don't match -->
<form id="form2">
<input type="text" name="uname3" oninput="reportFill(this, false)">
<input type="text" name="uname4" oninput="reportFill(this, true)">
<input type="password" name="pword3" oninput="reportFill(this, true)">
<input type="password" name="pword4" oninput="reportFill(this, false)">
</form>`;
} else if (fillCount > EXPECTED_FILLS) {
ok(false, "Too many fills");
}
}
</script>
<p id="display"></p>
<div id="content">
// Forms are inserted dynamically
</div>
<pre id="test"></pre>
</body>
</html>

View File

@ -201,8 +201,41 @@ const LoginTest = {
const RecipeHelpers = {
initNewParent() {
return (new LoginRecipesParent()).initializationPromise;
return (new LoginRecipesParent({ defaults: false })).initializationPromise;
},
/**
* Create a document for the given URL containing the given HTML containing a
* form and return the <form>.
*/
createTestForm(aDocumentURL, aHTML = "<form>") {
let parser = Cc["@mozilla.org/xmlextras/domparser;1"].
createInstance(Ci.nsIDOMParser);
parser.init();
let parsedDoc = parser.parseFromString(aHTML, "text/html");
// Mock the document.location object so we can unit test without a frame. We use a proxy
// instead of just assigning to the property since it's not configurable or writable.
let document = new Proxy(parsedDoc, {
get(target, property, receiver) {
// document.location is normally null when a document is outside of a "browsing context".
// See https://html.spec.whatwg.org/#the-location-interface
if (property == "location") {
return new URL(aDocumentURL);
}
return target[property];
},
});
let form = parsedDoc.forms[0];
// Assign form.ownerDocument to the proxy so document.location works.
Object.defineProperty(form, "ownerDocument", {
value: document,
});
return form;
}
};
////////////////////////////////////////////////////////////////////////////////

View File

@ -8,7 +8,7 @@
"use strict";
add_task(function* test_init() {
let parent = new LoginRecipesParent();
let parent = new LoginRecipesParent({ defaults: false });
let initPromise1 = parent.initializationPromise;
let initPromise2 = parent.initializationPromise;
Assert.strictEqual(initPromise1, initPromise2, "Check that the same promise is returned");
@ -102,6 +102,26 @@ add_task(function* test_add_pathRegex() {
Assert.strictEqual(recipe.pathRegex.toString(), "/^\\/mypath\\//", "Check the pathRegex");
});
add_task(function* test_add_selectors() {
let recipesParent = yield RecipeHelpers.initNewParent();
recipesParent.add({
hosts: ["example.com"],
usernameSelector: "#my-username",
passwordSelector: "#my-form > input.password",
});
Assert.strictEqual(recipesParent._recipesByHost.size, 1,
"Check number of hosts after the addition");
let exampleRecipes = recipesParent.getRecipesForHost("example.com");
Assert.strictEqual(exampleRecipes.size, 1, "Check recipe count for example.com");
let recipe = [...exampleRecipes][0];
Assert.strictEqual(typeof(recipe), "object", "Check recipe type");
Assert.strictEqual(recipe.hosts.length, 1, "Check that one host is present");
Assert.strictEqual(recipe.hosts[0], "example.com", "Check the one host");
Assert.strictEqual(recipe.usernameSelector, "#my-username", "Check the usernameSelector");
Assert.strictEqual(recipe.passwordSelector, "#my-form > input.password", "Check the passwordSelector");
});
/* Begin checking errors with add */
add_task(function* test_add_missing_prop() {
@ -138,4 +158,20 @@ add_task(function* test_add_pathRegex_non_regexp() {
}), /regular expression/, "pathRegex should be a RegExp");
});
add_task(function* test_add_usernameSelector_non_string() {
let recipesParent = yield RecipeHelpers.initNewParent();
Assert.throws(() => recipesParent.add({
hosts: ["example.com"],
usernameSelector: 404,
}), /string/, "usernameSelector should be a string");
});
add_task(function* test_add_passwordSelector_non_string() {
let recipesParent = yield RecipeHelpers.initNewParent();
Assert.throws(() => recipesParent.add({
hosts: ["example.com"],
passwordSelector: 404,
}), /string/, "passwordSelector should be a string");
});
/* End checking errors with add */

View File

@ -0,0 +1,41 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
/**
* Test filtering recipes in LoginRecipesContent.
*/
"use strict";
Cu.import("resource://testing-common/httpd.js");
Cu.importGlobalProperties(["URL"]);
Cu.import("resource://gre/modules/devtools/Console.jsm");
add_task(function* test_getFieldOverrides() {
let recipes = new Set([
{ // path doesn't match but otherwise good
hosts: ["example.com:8080"],
passwordSelector: "#password",
pathRegex: /^\/$/,
usernameSelector: ".username",
},
{ // match with no field overrides
hosts: ["example.com:8080"],
},
{ // best match (field selectors + path match)
description: "best match",
hosts: ["a.invalid", "example.com:8080", "other.invalid"],
passwordSelector: "#password",
pathRegex: /^\/first\/second\/$/,
usernameSelector: ".username",
},
]);
let form = RecipeHelpers.createTestForm("http://localhost:8080/first/second/");
let override = LoginRecipesContent.getFieldOverrides(recipes, form);
Assert.strictEqual(override.description, "best match",
"Check the best field override recipe was returned");
Assert.strictEqual(override.usernameSelector, ".username", "Check usernameSelector");
Assert.strictEqual(override.passwordSelector, "#password", "Check passwordSelector");
});

View File

@ -24,5 +24,6 @@ skip-if = os != "android"
[test_logins_search.js]
[test_notifications.js]
[test_recipes_add.js]
[test_recipes_content.js]
[test_storage.js]
[test_telemetry.js]