diff --git a/toolkit/components/passwordmgr/LoginManagerContent.jsm b/toolkit/components/passwordmgr/LoginManagerContent.jsm index c3d8ce6f7cf9..46b9c916174b 100644 --- a/toolkit/components/passwordmgr/LoginManagerContent.jsm +++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm @@ -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) { diff --git a/toolkit/components/passwordmgr/LoginRecipes.jsm b/toolkit/components/passwordmgr/LoginRecipes.jsm index 5ffc72285f55..130bf082e7e5 100644 --- a/toolkit/components/passwordmgr/LoginRecipes.jsm +++ b/toolkit/components/passwordmgr/LoginRecipes.jsm @@ -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 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))$" + }, + ] +}; diff --git a/toolkit/components/passwordmgr/test/mochitest.ini b/toolkit/components/passwordmgr/test/mochitest.ini index 1ff826d9650d..330599f591f2 100644 --- a/toolkit/components/passwordmgr/test/mochitest.ini +++ b/toolkit/components/passwordmgr/test/mochitest.ini @@ -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] diff --git a/toolkit/components/passwordmgr/test/pwmgr_common.js b/toolkit/components/passwordmgr/test/pwmgr_common.js index 17ec20035954..b71bb9d7d513 100644 --- a/toolkit/components/passwordmgr/test/pwmgr_common.js +++ b/toolkit/components/passwordmgr/test/pwmgr_common.js @@ -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); + })); +} diff --git a/toolkit/components/passwordmgr/test/test_recipe_login_fields.html b/toolkit/components/passwordmgr/test/test_recipe_login_fields.html new file mode 100644 index 000000000000..68805712fc7a --- /dev/null +++ b/toolkit/components/passwordmgr/test/test_recipe_login_fields.html @@ -0,0 +1,70 @@ + + + + Test for recipes overriding login fields + + + + + + +

+ +
+ // Forms are inserted dynamically +
+

+
+
diff --git a/toolkit/components/passwordmgr/test/unit/head.js b/toolkit/components/passwordmgr/test/unit/head.js
index a3a047d0f9a3..14ae04dbc6f3 100644
--- a/toolkit/components/passwordmgr/test/unit/head.js
+++ b/toolkit/components/passwordmgr/test/unit/head.js
@@ -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 
. + */ + createTestForm(aDocumentURL, aHTML = "") { + 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; + } }; //////////////////////////////////////////////////////////////////////////////// diff --git a/toolkit/components/passwordmgr/test/unit/test_recipes_add.js b/toolkit/components/passwordmgr/test/unit/test_recipes_add.js index ef6ea57a421e..828d9a6356e3 100644 --- a/toolkit/components/passwordmgr/test/unit/test_recipes_add.js +++ b/toolkit/components/passwordmgr/test/unit/test_recipes_add.js @@ -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 */ diff --git a/toolkit/components/passwordmgr/test/unit/test_recipes_content.js b/toolkit/components/passwordmgr/test/unit/test_recipes_content.js new file mode 100644 index 000000000000..f1cb8540877b --- /dev/null +++ b/toolkit/components/passwordmgr/test/unit/test_recipes_content.js @@ -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"); +}); diff --git a/toolkit/components/passwordmgr/test/unit/xpcshell.ini b/toolkit/components/passwordmgr/test/unit/xpcshell.ini index 59292491cfe6..1ed9313049af 100644 --- a/toolkit/components/passwordmgr/test/unit/xpcshell.ini +++ b/toolkit/components/passwordmgr/test/unit/xpcshell.ini @@ -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]