Skip to content

Commit

Permalink
Bug 1729640 - P3. Notify event when a form with password is submitted…
Browse files Browse the repository at this point in the history
… r=sfoster,tgiles,geckoview-reviewers,agi

Notify "passwordmgr-form-submission-detected" when a form with password
is submitted.
The event should be notified regardless of whether the password manager
decides to show the doorhanger or not. To support this, this patch
also refactors _maybeSendFormInteractionMessage function to distinguish
"onFormSubmit" event and "showDoorhanger" event.

Differential Revision: https://phabricator.services.mozilla.com/D127104
  • Loading branch information
DimiDL committed Nov 5, 2021
1 parent 00cff81 commit 38a7efa
Show file tree
Hide file tree
Showing 21 changed files with 220 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ add_task(async function test_policy_masterpassword_doorhanger() {

// Submit the form with the new credentials. This will cause the doorhanger
// notification to be displayed.
let formSubmittedPromise = listenForTestNotification("FormSubmit");
let formSubmittedPromise = listenForTestNotification("ShowDoorhanger");
await SpecialPowers.spawn(browser, [], async function() {
let doc = this.content.document;
doc.getElementById("form-basic").submit();
Expand Down
2 changes: 1 addition & 1 deletion mobile/android/actors/GeckoViewAutoFillChild.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class GeckoViewAutoFillChild extends GeckoViewActorChild {
}
break;
}
case "PasswordManager:onFormSubmit": {
case "PasswordManager:ShowDoorhanger": {
const { form: formLike } = aEvent.detail;
this._autofill.commitAutofill(formLike);
break;
Expand Down
2 changes: 1 addition & 1 deletion mobile/android/chrome/geckoview/geckoview.js
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ function startup() {
mozSystemGroup: true,
capture: false,
},
"PasswordManager:onFormSubmit": {},
"PasswordManager:ShowDoorhanger": {},
},
},
allFrames: true,
Expand Down
142 changes: 95 additions & 47 deletions toolkit/components/passwordmgr/LoginManagerChild.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -1928,7 +1928,7 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {

this._maybeSendFormInteractionMessage(
form,
"PasswordManager:onFormSubmit",
"PasswordManager:ShowDoorhanger",
{
targetField: null,
isSubmission: true,
Expand All @@ -1943,14 +1943,25 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
* Extracts and validates information from a form-like element on the page. If validation is
* successful, sends a message to the parent process requesting that it show a dialog.
*
* If validation fails, this method is a noop.
* The validation works are divided into two parts:
* 1. Whether this is a valid form with a password (validate in this function)
* 2. Whether the password manager decides to send interaction message for this form
* (validate in _maybeSendFormInteractionMessageContinue)
*
* When the function is triggered by a form submission event, and the form is valid (pass #1),
* We still send the message to the parent even the validation of #2 fails. This is because
* there might be someone who is interested in form submission events regardless of whether
* the password manager decides to show the doorhanger or not.
*
* @param {LoginForm} form
* @param {string} messageName used to categorize the type of message sent to the parent process.
* @param {Element?} options.targetField
* @param {boolean} options.isSubmission if true, this function call was prompted by a form submission.
* @param {boolean?} options.triggeredByFillingGenerated whether or not this call was triggered by a
* generated password being filled into a form-like element.
* @param {boolean?} options.ignoreConnect Whether to ignore isConnected attribute of a element.
*
* @returns {Boolean} whether the message is sent to the parent process.
*/
_maybeSendFormInteractionMessage(
form,
Expand All @@ -1959,12 +1970,90 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
) {
let doc = form.ownerDocument;
let win = doc.defaultView;
let logMessagePrefix = isSubmission ? "form submission" : "field edit";
let passwordField = null;
if (targetField && targetField.hasBeenTypePassword) {
passwordField = targetField;
}

let origin = LoginHelper.getLoginOrigin(doc.documentURI);
if (!origin) {
log(`(${logMessagePrefix} ignored -- invalid origin)`);
return;
}

// Get the appropriate fields from the form.
let recipes = LoginRecipesContent.getRecipes(this, origin, win);
let fields = {
targetField,
...this._getFormFields(form, true, recipes, { ignoreConnect }),
};

// It's possible the field triggering this message isn't one of those found by _getFormFields' heuristics
if (
passwordField &&
passwordField != fields.newPasswordField &&
passwordField != fields.oldPasswordField &&
passwordField != fields.confirmPasswordField
) {
fields.newPasswordField = passwordField;
}

// Need at least 1 valid password field to do anything.
if (fields.newPasswordField == null) {
if (isSubmission && fields.usernameField) {
log(
"_onFormSubmit: username-only form. Record the username field but not sending prompt"
);
this.stateForDocument(doc).mockUsernameOnlyField = {
name: fields.usernameField.name,
value: fields.usernameField.value,
};
}
return;
}

this._maybeSendFormInteractionMessageContinue(form, messageName, {
...fields,
isSubmission,
triggeredByFillingGenerated,
});

if (isSubmission) {
// Notify `PasswordManager:onFormSubmit` as long as we detect submission event on a
// valid form with a password field.
this.sendAsyncMessage(
"PasswordManager:onFormSubmit",
{},
{
fields,
isSubmission,
triggeredByFillingGenerated,
}
);
}
}

/**
* Continues the works that are not done in _maybeSendFormInteractionMessage.
* See comments in _maybeSendFormInteractionMessage for more details.
*/
_maybeSendFormInteractionMessageContinue(
form,
messageName,
{
targetField,
usernameField,
newPasswordField,
oldPasswordField,
confirmPasswordField,
isSubmission,
triggeredByFillingGenerated,
}
) {
let logMessagePrefix = isSubmission ? "form submission" : "field edit";
let dismissedPrompt = !isSubmission;
let doc = form.ownerDocument;
let win = doc.defaultView;

let detail = { messageSent: false };
try {
Expand All @@ -1985,50 +2074,6 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
return;
}

let origin = LoginHelper.getLoginOrigin(doc.documentURI);
if (!origin) {
log(`(${logMessagePrefix} ignored -- invalid origin)`);
return;
}

let formActionOrigin = LoginHelper.getFormActionOrigin(form);

let recipes = LoginRecipesContent.getRecipes(this, origin, win);

// Get the appropriate fields from the form.
let {
usernameField,
newPasswordField,
oldPasswordField,
confirmPasswordField,
} = this._getFormFields(form, true, recipes, { ignoreConnect });

// It's possible the field triggering this message isn't one of those found by _getFormFields' heuristics
if (
passwordField &&
passwordField != newPasswordField &&
passwordField != oldPasswordField &&
passwordField != confirmPasswordField
) {
newPasswordField = passwordField;
}

let docState = this.stateForDocument(doc);

// Need at least 1 valid password field to do anything.
if (newPasswordField == null) {
if (isSubmission && usernameField) {
log(
"_onFormSubmit: username-only form. Record the username field but not sending prompt"
);
docState.mockUsernameOnlyField = {
name: usernameField.name,
value: usernameField.value,
};
}
return;
}

let fullyMungedPattern = /^\*+$|^•+$|^\.+$/;
// Check `isSubmission` to allow munged passwords in dismissed by default doorhangers (since
// they are initiated by the user) in case this matches their actual password.
Expand All @@ -2043,6 +2088,7 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
// form doesn't have one. This means if there is a username field found in the current
// form, we don't compare it to the saved one, which might be a better choice in some cases.
// The reason we are not doing it now is because we haven't found a real world example.
let docState = this.stateForDocument(doc);
if (!usernameField) {
if (docState.mockUsernameOnlyField) {
usernameField = docState.mockUsernameOnlyField;
Expand Down Expand Up @@ -2087,6 +2133,7 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
// if the password field is a three digit number. Also dismiss prompt if
// the password is a credit card number and the password field has attribute
// autocomplete="cc-number".
let dismissedPrompt = !isSubmission;
let newPasswordFieldValue = newPasswordField.value;
if (
(!dismissedPrompt &&
Expand Down Expand Up @@ -2128,6 +2175,7 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
let { login: autoFilledLogin } =
docState.fillsByRootElement.get(form.rootElement) || {};
let browsingContextId = win.windowGlobalChild.browsingContext.id;
let formActionOrigin = LoginHelper.getFormActionOrigin(form);

detail = {
browsingContextId,
Expand Down
30 changes: 21 additions & 9 deletions toolkit/components/passwordmgr/LoginManagerParent.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -295,13 +295,11 @@ class LoginManagerParent extends JSWindowActorParent {
}

case "PasswordManager:onFormSubmit": {
let browser = this.getRootBrowser();
let submitPromise = this.onFormSubmit(browser, context.origin, data);
if (gListenerForTests) {
submitPromise.then(() => {
gListenerForTests("FormSubmit", { origin: context.origin, data });
});
}
Services.obs.notifyObservers(
null,
"passwordmgr-form-submission-detected",
context.origin
);
break;
}

Expand All @@ -325,6 +323,20 @@ class LoginManagerParent extends JSWindowActorParent {
break;
}

case "PasswordManager:ShowDoorhanger": {
let browser = this.getRootBrowser();
let submitPromise = this.showDoorhanger(browser, context.origin, data);
if (gListenerForTests) {
submitPromise.then(() => {
gListenerForTests("ShowDoorhanger", {
origin: context.origin,
data,
});
});
}
break;
}

case "PasswordManager:autoCompleteLogins": {
return this.doAutocompleteSearch(context.origin, data);
}
Expand Down Expand Up @@ -826,7 +838,7 @@ class LoginManagerParent extends JSWindowActorParent {
return prompterSvc;
}

async onFormSubmit(
async showDoorhanger(
browser,
formOrigin,
{
Expand Down Expand Up @@ -865,7 +877,7 @@ class LoginManagerParent extends JSWindowActorParent {
let browsingContext = BrowsingContext.get(browsingContextId);
let framePrincipalOrigin =
browsingContext.currentWindowGlobal.documentPrincipal.origin;
log("onFormSubmit, got framePrincipalOrigin: ", framePrincipalOrigin);
log("showDoorhanger, got framePrincipalOrigin: ", framePrincipalOrigin);

let formLogin = new LoginInfo(
formOrigin,
Expand Down
1 change: 1 addition & 0 deletions toolkit/components/passwordmgr/test/browser/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ skip-if =
os == 'mac' && webrender && bits == 64 # Bug 1683848
os == 'linux' && !debug && bits == 64 # Bug 1683848
win10_2004 && !fission # Bug 1723573
[browser_message_onFormSubmit.js]
[browser_openPasswordManager.js]
[browser_private_window.js]
support-files =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ add_task(async function test() {
}
);

let processedPromise = listenForTestNotification("FormSubmit");
let processedPromise = listenForTestNotification("ShowDoorhanger");
SpecialPowers.spawn(tab.linkedBrowser, [], () => {
content.document.getElementById("form-basic").submit();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
function getDataFromNextSubmitMessage() {
return new Promise(resolve => {
LoginManagerParent.setListenerForTests((msg, data) => {
if (msg == "FormSubmit") {
if (msg == "ShowDoorhanger") {
resolve(data);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ async function test_save_change({
}
);

let formSubmittedPromise = listenForTestNotification("FormSubmit");
let formSubmittedPromise = listenForTestNotification("ShowDoorhanger");
await SpecialPowers.spawn(browser, [], async function() {
let doc = this.content.document;
doc.getElementById("form-basic").submit();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function listenForNotifications(count, expectedFormOrigin) {
LoginManagerParent.setListenerForTests((msg, data) => {
if (msg == "FormProcessed") {
notifications.push("FormProcessed: " + data.browsingContext.id);
} else if (msg == "FormSubmit") {
} else if (msg == "ShowDoorhanger") {
is(
data.origin,
expectedFormOrigin,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ add_task(async function test_doorhanger_dismissal_un() {
// password edited vs form submitted w. cc number as username
await clearMessageCache(browser);

let processedPromise = listenForTestNotification("FormSubmit");
let processedPromise = listenForTestNotification("ShowDoorhanger");
await SpecialPowers.spawn(browser, [], async () => {
content.document.getElementById("form-basic-submit").click();
});
Expand Down Expand Up @@ -80,7 +80,7 @@ add_task(async function test_doorhanger_dismissal_pw() {
// password edited vs form submitted w. cc number as password
await clearMessageCache(browser);

let processedPromise = listenForTestNotification("FormSubmit");
let processedPromise = listenForTestNotification("ShowDoorhanger");
await SpecialPowers.spawn(browser, [], async () => {
content.document.getElementById("form-basic-submit").click();
});
Expand Down Expand Up @@ -121,7 +121,7 @@ add_task(async function test_doorhanger_shown_on_un_with_invalid_ccnumber() {
// password edited vs form submitted w. cc number as password
await clearMessageCache(browser);

let processedPromise = listenForTestNotification("FormSubmit");
let processedPromise = listenForTestNotification("ShowDoorhanger");
await SpecialPowers.spawn(browser, [], async () => {
content.document.getElementById("form-basic-submit").click();
});
Expand Down Expand Up @@ -178,7 +178,7 @@ add_task(async function test_doorhanger_dismissal_on_change() {
// password edited vs form submitted w. cc number as username
await clearMessageCache(browser);

let processedPromise = listenForTestNotification("FormSubmit");
let processedPromise = listenForTestNotification("ShowDoorhanger");
await SpecialPowers.spawn(browser, [], async () => {
content.document.getElementById("form-basic-submit").click();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ async function test_save_change(testData) {

// Submit the form.
info(`submit the password-only form`);
let formSubmittedPromise = listenForTestNotification("FormSubmit");
let formSubmittedPromise = listenForTestNotification("ShowDoorhanger");
await SpecialPowers.spawn(browser, [], async function() {
let doc = this.content.document;
doc.getElementById("form-basic-submit").click();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ add_task(async function test_edit_password() {
// Submit the form in the content page with the credentials from the test
// case. This will cause the doorhanger notification to be displayed.
info("Submitting the form");
let formSubmittedPromise = listenForTestNotification("FormSubmit");
let formSubmittedPromise = listenForTestNotification("ShowDoorhanger");
let promiseShown = BrowserTestUtils.waitForEvent(
PopupNotifications.panel,
"popupshown",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ async function test_save_change(testData) {

// Submit the form with the new credentials. This will cause the doorhanger
// notification to be displayed.
let formSubmittedPromise = listenForTestNotification("FormSubmit");
let formSubmittedPromise = listenForTestNotification("ShowDoorhanger");
await SpecialPowers.spawn(browser, [], async function() {
let doc = this.content.document;
doc.getElementById("form-basic").submit();
Expand Down
Loading

0 comments on commit 38a7efa

Please sign in to comment.