diff --git a/toolkit/components/passwordmgr/LoginManagerParent.jsm b/toolkit/components/passwordmgr/LoginManagerParent.jsm index 73a3d9954e6e..93b2096e9099 100644 --- a/toolkit/components/passwordmgr/LoginManagerParent.jsm +++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm @@ -734,7 +734,8 @@ this.LoginManagerParent = { password ); - let shouldSaveLogin = true; + let autoSaveLogin = true; + let loginToChange = null; // This will throw if we can't look up the entry in the password/origin map if (!generatedPW.filled) { @@ -753,7 +754,7 @@ this.LoginManagerParent = { "_onGeneratedPasswordFilledOrEdited: saving is disabled for:", formOrigin ); - shouldSaveLogin = false; + autoSaveLogin = false; } // Check if we already have a login saved for this site since we don't want to overwrite it in @@ -765,46 +766,50 @@ this.LoginManagerParent = { ignoreActionAndRealm: false, }); - if (logins.length > 0) { - log( - "_onGeneratedPasswordFilledOrEdited: Login already saved for this site" - ); - shouldSaveLogin = false; - for (let login of logins) { - if (formLoginWithoutUsername.matches(login, false)) { - // This login is already saved so show no new UI. - log( - "_onGeneratedPasswordFilledOrEdited: Matching login already saved" - ); - return; - } + let matchedLogin = logins.find(login => + formLoginWithoutUsername.matches(login, true) + ); + if (matchedLogin) { + autoSaveLogin = false; + if (matchedLogin.password == formLoginWithoutUsername.password) { + // This login is already saved so show no new UI. + log("_onGeneratedPasswordFilledOrEdited: Matching login already saved"); + return; } + // We're updating a previously-saved login + loginToChange = matchedLogin; + log( + "_onGeneratedPasswordFilledOrEdited: Login with empty username already saved for this site" + ); } - if (shouldSaveLogin) { - Services.logins.addLogin(formLoginWithoutUsername); + if (autoSaveLogin) { + log( + "_onGeneratedPasswordFilledOrEdited: auto-saving new login with empty username" + ); + loginToChange = Services.logins.addLogin(formLoginWithoutUsername); + } else { + log("_onGeneratedPasswordFilledOrEdited: not auto-saving this login"); } - log( - "_onGeneratedPasswordFilledOrEdited: show dismissed save-password notification" - ); let browser = browsingContext.top.embedderElement; let prompter = this._getPrompter(browser, openerTopWindowID); - if (shouldSaveLogin) { - // If we auto-saved the login then show a change doorhanger to allow - // modifying it e.g. adding a username. + if (loginToChange) { + // Show a change doorhanger to allow modifying an already-saved login + // e.g. to add a username or update the password. prompter.promptToChangePassword( - formLoginWithoutUsername, + loginToChange, formLogin, true, // dimissed prompt - shouldSaveLogin // notifySaved + autoSaveLogin // notifySaved ); return; } + log("_onGeneratedPasswordFilledOrEdited: no matching login to save/update"); prompter.promptToSavePassword( formLogin, true, // dimissed prompt - shouldSaveLogin // notifySaved + autoSaveLogin // notifySaved ); }, diff --git a/toolkit/components/passwordmgr/LoginManagerPrompter.jsm b/toolkit/components/passwordmgr/LoginManagerPrompter.jsm index 9794c1cd4177..edab0beb3866 100644 --- a/toolkit/components/passwordmgr/LoginManagerPrompter.jsm +++ b/toolkit/components/passwordmgr/LoginManagerPrompter.jsm @@ -1432,11 +1432,12 @@ LoginManagerPrompter.prototype = { dismissed = false, notifySaved = false ) { - aOldLogin.origin = aNewLogin.origin; - aOldLogin.formActionOrigin = aNewLogin.formActionOrigin; - aOldLogin.password = aNewLogin.password; - aOldLogin.username = aNewLogin.username; - this._showLoginCaptureDoorhanger(aOldLogin, "password-change", { + let login = aOldLogin.clone(); + login.origin = aNewLogin.origin; + login.formActionOrigin = aNewLogin.formActionOrigin; + login.password = aNewLogin.password; + login.username = aNewLogin.username; + this._showLoginCaptureDoorhanger(login, "password-change", { dismissed, notifySaved, extraAttr: notifySaved ? "attention" : "", diff --git a/toolkit/components/passwordmgr/test/LoginTestUtils.jsm b/toolkit/components/passwordmgr/test/LoginTestUtils.jsm index 12d8944726c2..8001fec64609 100644 --- a/toolkit/components/passwordmgr/test/LoginTestUtils.jsm +++ b/toolkit/components/passwordmgr/test/LoginTestUtils.jsm @@ -45,10 +45,15 @@ this.LoginTestUtils = { /** * Add a new login to the store */ - async addLogin({ username, password, origin = "https://example.com" }) { + async addLogin({ + username, + password, + origin = "https://example.com", + formActionOrigin, + }) { const login = LoginTestUtils.testData.formLogin({ origin, - formActionOrigin: origin, + formActionOrigin: formActionOrigin || origin, username, password, }); diff --git a/toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js b/toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js index ac1efc6d9bcb..a8ab91c8fba7 100644 --- a/toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js +++ b/toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js @@ -14,8 +14,20 @@ const passwordInputSelector = "#form-basic-password"; const usernameInputSelector = "#form-basic-username"; let login1; -function addOneLogin() { - login1 = LoginTestUtils.addLogin({ username: "username", password: "pass1" }); +async function setup_withOneLogin(username = "username", password = "pass1") { + // Reset to a single, known login + Services.logins.removeAllLogins(); + login1 = await LoginTestUtils.addLogin({ username, password }); +} + +async function setup_withNoLogins() { + // Reset to a single, known login + Services.logins.removeAllLogins(); + is( + Services.logins.getAllLogins().length, + 0, + "0 logins at the start of the test" + ); } async function fillGeneratedPasswordFromACPopup( @@ -51,14 +63,26 @@ async function fillGeneratedPasswordFromACPopup( await inputEventPromise; } -async function checkPromptContents(anchorElement, browser) { +async function checkPromptContents( + anchorElement, + browser, + expectedPasswordLength = 0 +) { let { panel } = PopupNotifications; - let promiseShown = BrowserTestUtils.waitForEvent(panel, "popupshown"); - await SimpleTest.promiseFocus(browser); - info("Clicking on anchor to show popup."); - anchorElement.click(); - await promiseShown; + ok(PopupNotifications.isPanelOpen, "Confirm popup is open"); let notificationElement = panel.childNodes[0]; + if (expectedPasswordLength) { + info( + `Waiting for password value to be ${expectedPasswordLength} chars long` + ); + await BrowserTestUtils.waitForCondition(() => { + return ( + notificationElement.querySelector("#password-notification-password") + .value.length == expectedPasswordLength + ); + }, "Wait for nsLoginManagerPrompter writeDataToUI()"); + } + return { passwordValue: notificationElement.querySelector( "#password-notification-password" @@ -69,6 +93,210 @@ async function checkPromptContents(anchorElement, browser) { }; } +async function verifyGeneratedPasswordWasFilled( + browser, + passwordInputSelector +) { + await ContentTask.spawn( + browser, + [passwordInputSelector], + function checkFinalFieldValue(inputSelector) { + let passwordInput = content.document.querySelector(inputSelector); + is( + passwordInput.value.length, + 15, + "Password field was filled with generated password" + ); + } + ); +} + +async function verifyConfirmationHint(hintElem) { + info("verifyConfirmationHint"); + info("verifyConfirmationHint, hintPromiseShown resolved"); + is( + hintElem.anchorNode.id, + "password-notification-icon", + "Hint should be anchored on the password notification icon" + ); + info("verifyConfirmationHint, assertion ok, wait for poopuphidden"); + await BrowserTestUtils.waitForEvent(hintElem, "popuphidden"); + info("verifyConfirmationHint, /popuphidden"); +} + +async function openFormInNewTab(url, formValues, taskFn) { + await BrowserTestUtils.withNewTab( + { + gBrowser, + url, + }, + async function(browser) { + await SimpleTest.promiseFocus(browser.ownerGlobal); + await ContentTask.spawn( + browser, + formValues, + async function prepareAndCheckForm({ + password: passwordProps, + username: usernameProps, + }) { + let doc = content.document; + // give the form an action so we can know when submit is complete + doc.querySelector("form").action = "/"; + + let props = passwordProps; + if (props) { + // We'll reuse the form_basic.html, but ensure we'll get the generated password autocomplete option + let field = doc.querySelector(props.selector); + field.setAttribute("autocomplete", "new-password"); + if (props.hasOwnProperty("expectedValue")) { + is( + field.value, + props.expectedValue, + "Check autofilled password value" + ); + } + if (props.hasOwnProperty("setValue")) { + let gotInput = ContentTaskUtils.waitForEvent( + field, + "input", + "field value changed" + ); + field.setUserInput(props.setValue); + await gotInput; + } + } + props = usernameProps; + if (props) { + let field = doc.querySelector(props.selector); + if (props.hasOwnProperty("expectedValue")) { + is( + field.value, + props.expectedValue, + "Check autofilled username value" + ); + } + if (props.hasOwnProperty("setValue")) { + let gotInput = ContentTaskUtils.waitForEvent( + field, + "input", + "field value changed" + ); + field.setUserInput(props.setValue); + await gotInput; + } + } + } + ); + await taskFn(browser); + } + ); +} + +async function waitForDoorhanger(browser, type) { + await TestUtils.waitForCondition(() => { + let notif = PopupNotifications.getNotification("password", browser); + return notif && notif.options.passwordNotificationType == type; + }, `Waiting for a ${type} notification`); +} + +async function openAndVerifyDoorhanger(browser, type, expected) { + // check a dismissed prompt was shown with extraAttr attribute + let notif = getCaptureDoorhanger(type); + ok(notif, `${type} doorhanger was created`); + is( + notif.dismissed, + expected.dismissed, + "Check notification dismissed property" + ); + is( + notif.anchorElement.getAttribute("extraAttr"), + expected.anchorExtraAttr, + "Check icon extraAttr attribute" + ); + if (!PopupNotifications.isPanelOpen) { + let promiseShown = BrowserTestUtils.waitForEvent( + PopupNotifications.panel, + "popupshown" + ); + await SimpleTest.promiseFocus(browser); + info("Clicking on anchor to show popup."); + notif.anchorElement.click(); + await promiseShown; + } + // if the doorhanged is dimissed, we will open it to check panel contents + let { passwordValue, usernameValue } = await checkPromptContents( + notif.anchorElement, + browser, + expected.passwordLength + ); + is( + passwordValue.length, + 15, + "Doorhanger password field has generated 15-char value" + ); + is( + usernameValue, + expected.usernameValue, + "Doorhanger username field was popuplated" + ); + return notif; +} + +async function hideDoorhangerPopup(browser) { + info("hideDoorhangerPopup"); + if (!PopupNotifications.isPanelOpen) { + return; + } + let { panel } = PopupNotifications; + let promiseHidden = BrowserTestUtils.waitForEvent(panel, "popuphidden"); + panel.hidePopup(); + await promiseHidden; + info("got popuphidden from notification panel"); +} + +async function submitForm(browser) { + // Submit the form + info("Now submit the form with the generated password"); + + await ContentTask.spawn(browser, null, async function() { + content.document.querySelector("form").submit(); + + await ContentTaskUtils.waitForCondition(() => { + return ( + content.location.pathname == "/" && + content.document.readyState == "complete" + ); + }, "Wait for form submission load"); + }); +} + +function verifyLogins(expectedValues) { + let allLogins = Services.logins.getAllLogins(); + is(allLogins.length, expectedValues.count, "Check saved logins count"); + for (let i = 0; i < expectedValues.loginProperties.length; i++) { + let expected = expectedValues[i]; + if (expected) { + let login = allLogins[i]; + if (expected.hasOwnProperty("timesUsed")) { + is(login.timesUsed, expected.timesUsed, "Check timesUsed"); + } + if (expected.hasOwnProperty("passwordLength")) { + is( + login.password.length, + expected.passwordLength, + "Check passwordLength" + ); + } + if (expected.hasOwnProperty("username")) { + is(login.username, expected.username, "Check username"); + } + if (expected.hasOwnProperty("usedSince")) { + ok(login.timeLastUsed > expected.usedSince, "Check timeLastUsed"); + } + } + } +} + add_task(async function setup() { await SpecialPowers.pushPrefEnv({ set: [ @@ -82,345 +310,402 @@ add_task(async function setup() { }); add_task(async function autocomplete_generated_password_auto_saved() { - await BrowserTestUtils.withNewTab( + // confirm behavior when filling a generated password via autocomplete + // when there are no other logins + await setup_withNoLogins(); + await openFormInNewTab( + TEST_ORIGIN + FORM_PAGE_PATH, { - gBrowser, - url: TEST_ORIGIN + FORM_PAGE_PATH, + password: { selector: passwordInputSelector, expectedValue: "" }, + username: { selector: usernameInputSelector, expectedValue: "" }, }, - async function(browser) { - await SimpleTest.promiseFocus(browser.ownerGlobal); - await ContentTask.spawn( - browser, - [passwordInputSelector, usernameInputSelector], - function prepareAndCheckForm([passwordSelector, usernameSelector]) { - let passwordInput = content.document.querySelector(passwordSelector); - // We'll reuse the form_basic.html, but ensure we'll get the generated password autocomplete option - passwordInput.setAttribute("autocomplete", "new-password"); - passwordInput.value = ""; - let usernameInput = content.document.querySelector(usernameSelector); - usernameInput.setUserInput("user1"); - } - ); - + async function taskFn(browser) { let storageChangedPromise = TestUtils.topicObserved( "passwordmgr-storage-changed", (_, data) => data == "addLogin" ); - let confirmationHint = document.getElementById("confirmation-hint"); let hintPromiseShown = BrowserTestUtils.waitForEvent( confirmationHint, "popupshown" ); await fillGeneratedPasswordFromACPopup(browser, passwordInputSelector); - await ContentTask.spawn( - browser, - [passwordInputSelector], - function checkFinalFieldValue(inputSelector) { - let passwordInput = content.document.querySelector(inputSelector); - is( - passwordInput.value.length, - 15, - "Password field was filled with generated password" - ); - } - ); - let [{ username, password }] = await storageChangedPromise; + await verifyGeneratedPasswordWasFilled(browser, passwordInputSelector); // Make sure confirmation hint was shown await hintPromiseShown; - - Assert.equal( - confirmationHint.anchorNode.id, - "password-notification-icon", - "Hint should be anchored on the password notification icon" - ); - - let hintPromiseHidden = BrowserTestUtils.waitForEvent( - confirmationHint, - "popuphidden" - ); - await hintPromiseHidden; + await verifyConfirmationHint(confirmationHint); // Check properties of the newly auto-saved login is(username, "", "Saved login should have no username"); is(password.length, 15, "Saved login should have generated password"); - // check a dismissed prompt was shown with extraAttr attribute - let notif = getCaptureDoorhanger("password-change"); - ok(notif && notif.dismissed, "Dismissed notification was created"); - is( - notif.anchorElement.getAttribute("extraAttr"), - "attention", - "Check if icon has the extraAttr attribute" - ); - - let { passwordValue, usernameValue } = await checkPromptContents( - notif.anchorElement, - browser - ); - is( - passwordValue.length, - 15, - "Doorhanger password field has generated 15-char value" - ); - is(usernameValue, "user1", "Doorhanger username field was popuplated"); - - info("Hiding popup."); - let { panel } = PopupNotifications; - let promiseHidden = BrowserTestUtils.waitForEvent(panel, "popuphidden"); - panel.hidePopup(); - await promiseHidden; - + let notif = await openAndVerifyDoorhanger(browser, "password-change", { + dismissed: true, + anchorExtraAttr: "attention", + usernameValue: "", + passwordLength: 15, + }); + await clickDoorhangerButton(notif, DONT_CHANGE_BUTTON); // confirm the extraAttr attribute is removed after opening & dismissing the doorhanger ok( !notif.anchorElement.hasAttribute("extraAttr"), "Check if the extraAttr attribute was removed" ); notif.remove(); + + storageChangedPromise = TestUtils.topicObserved( + "passwordmgr-storage-changed", + (_, data) => data == "modifyLogin" + ); + let [autoSavedLogin] = Services.logins.getAllLogins(); + info("waiting for submitForm"); + await submitForm(browser); + await storageChangedPromise; + verifyLogins({ + count: 1, + loginProperties: [ + { + timesUsed: autoSavedLogin.timesUsed + 1, + username: "", + }, + ], + }); + await hideDoorhangerPopup(browser); // make sure the popup is closed for next test } ); }); -add_task(async function setup_logins() { - // Reset all passwords - Services.logins.removeAllLogins(); - // ..and use a single matching login for the following tests - await addOneLogin(); -}); - -add_task(async function contextfill_generated_password_with_matching_logins() { - // test that we can fill a generated password when there are matching logins - await BrowserTestUtils.withNewTab( +add_task(async function autocomplete_generated_password_saved_empty_username() { + // confirm behavior when filling a generated password via autocomplete + // when there is an existing saved login with a "" username + await setup_withOneLogin("", "xyzpassword"); + await openFormInNewTab( + TEST_ORIGIN + FORM_PAGE_PATH, { - gBrowser, - url: TEST_ORIGIN + FORM_PAGE_PATH, + password: { + selector: passwordInputSelector, + expectedValue: "xyzpassword", + setValue: "", + }, + username: { selector: usernameInputSelector, expectedValue: "" }, }, - async function(browser) { - await SimpleTest.promiseFocus(browser.ownerGlobal); - await ContentTask.spawn( - browser, - [passwordInputSelector], - async function waitForFilledFieldValue(inputSelector) { - let passwordInput = content.document.querySelector(inputSelector); - await ContentTaskUtils.waitForCondition( - () => passwordInput.value == "pass1", - "Password field got autofilled value" - ); - } - ); - await doFillGeneratedPasswordContextMenuItem( - browser, - passwordInputSelector - ); - await ContentTask.spawn( - browser, - [passwordInputSelector], - function checkFinalFieldValue(inputSelector) { - is( - content.document.querySelector(inputSelector).value.length, - 15, - "Password field was filled with generated password" - ); - } - ); - // check a dismissed prompt was shown - let notif = getCaptureDoorhanger("password-save"); - ok(notif && notif.dismissed, "Dismissed notification was created"); - - let { passwordValue } = await checkPromptContents( - notif.anchorElement, - browser - ); - is( - passwordValue.length, - 15, - "Doorhanger password field has generated 15-char value" - ); - ok( - !notif.anchorElement.hasAttribute("extraAttr"), - "Check if icon has the extraAttr attribute" - ); - notif.remove(); - } - ); -}); - -add_task(async function contextfill_generated_password_with_username() { - // test that the prompt resulting from filling with a generated password displays the username - await BrowserTestUtils.withNewTab( - { - gBrowser, - url: TEST_ORIGIN + FORM_PAGE_PATH, - }, - async function(browser) { - await SimpleTest.promiseFocus(browser.ownerGlobal); - await ContentTask.spawn( - browser, - [passwordInputSelector, usernameInputSelector], - function checkAndSetFieldValue([passwordSelector, usernameSelector]) { - is( - content.document.querySelector(passwordSelector).value, - "pass1", - "Password field has initial autofilled value" - ); - content.document - .querySelector(usernameSelector) - .setUserInput("user1"); - } - ); - - await doFillGeneratedPasswordContextMenuItem( - browser, - passwordInputSelector - ); - - // check a dismissed prompt was shown - let notif = getCaptureDoorhanger("password-save"); - ok(notif && notif.dismissed, "Dismissed notification was created"); - - let { passwordValue, usernameValue } = await checkPromptContents( - notif.anchorElement - ); - is( - passwordValue.length, - 15, - "Doorhanger password field has generated 15-char value" - ); - is( - usernameValue, - "user1", - "Doorhanger username field has the username field value" - ); - ok( - !notif.anchorElement.hasAttribute("extraAttr"), - "Check if icon has the extraAttr attribute" - ); - notif.remove(); - } - ); -}); - -add_task(async function autocomplete_generated_password() { - await BrowserTestUtils.withNewTab( - { - gBrowser, - url: TEST_ORIGIN + FORM_PAGE_PATH, - }, - async function(browser) { - await SimpleTest.promiseFocus(browser.ownerGlobal); - await ContentTask.spawn( - browser, - [passwordInputSelector, usernameInputSelector], - function prepareAndCheckForm([passwordSelector, usernameSelector]) { - let passwordInput = content.document.querySelector(passwordSelector); - // We'll reuse the form_basic.html, but ensure we'll get the generated password autocomplete option - passwordInput.setAttribute("autocomplete", "new-password"); - passwordInput.value = ""; - let usernameInput = content.document.querySelector(usernameSelector); - usernameInput.setUserInput("user1"); - } + async function taskFn(browser) { + let [savedLogin] = Services.logins.getAllLogins(); + let storageChangedPromise = TestUtils.topicObserved( + "passwordmgr-storage-changed", + (_, data) => data == "modifyLogin" ); await fillGeneratedPasswordFromACPopup(browser, passwordInputSelector); + await waitForDoorhanger(browser, "password-change"); + info("Waiting to openAndVerifyDoorhanger"); + await openAndVerifyDoorhanger(browser, "password-change", { + dismissed: true, + anchorExtraAttr: "", + usernameValue: "", + passwordLength: 15, + }); + await hideDoorhangerPopup(browser); + info("Waiting to verifyGeneratedPasswordWasFilled"); + await verifyGeneratedPasswordWasFilled(browser, passwordInputSelector); + + info("waiting for submitForm"); + await submitForm(browser); + let notif = await openAndVerifyDoorhanger(browser, "password-change", { + dismissed: false, + anchorExtraAttr: "", + usernameValue: "", + passwordLength: 15, + }); + + await clickDoorhangerButton(notif, CHANGE_BUTTON); + info("Waiting for modifyLogin"); + await storageChangedPromise; + verifyLogins({ + count: 1, + loginProperties: [ + { + timesUsed: savedLogin.timesUsed + 1, + username: "", + }, + ], + }); + await hideDoorhangerPopup(browser); // make sure the popup is closed for next test + notif && notif.remove(); + } + ); +}); + +add_task(async function contextfill_generated_password_saved_empty_username() { + // confirm behavior when filling a generated password via context menu + // when there is an existing saved login with a "" username + await setup_withOneLogin("", "xyzpassword"); + await openFormInNewTab( + TEST_ORIGIN + FORM_PAGE_PATH, + { + password: { + selector: passwordInputSelector, + expectedValue: "xyzpassword", + setValue: "", + }, + username: { selector: usernameInputSelector, expectedValue: "" }, + }, + async function taskFn(browser) { + let [savedLogin] = Services.logins.getAllLogins(); + let storageChangedPromise = TestUtils.topicObserved( + "passwordmgr-storage-changed", + (_, data) => data == "modifyLogin" + ); + await doFillGeneratedPasswordContextMenuItem( + browser, + passwordInputSelector + ); + await waitForDoorhanger(browser, "password-change"); + info("Waiting to openAndVerifyDoorhanger"); + await openAndVerifyDoorhanger(browser, "password-change", { + dismissed: true, + anchorExtraAttr: "", + usernameValue: "", + passwordLength: 15, + }); + await hideDoorhangerPopup(browser); + info("Waiting to verifyGeneratedPasswordWasFilled"); + await verifyGeneratedPasswordWasFilled(browser, passwordInputSelector); + + info("waiting for submitForm"); + await submitForm(browser); + let notif = await openAndVerifyDoorhanger(browser, "password-change", { + dismissed: false, + anchorExtraAttr: "", + usernameValue: "", + passwordLength: 15, + }); + + await clickDoorhangerButton(notif, CHANGE_BUTTON); + info("Waiting for modifyLogin"); + await storageChangedPromise; + verifyLogins({ + count: 1, + loginProperties: [ + { + timesUsed: savedLogin.timesUsed + 1, + username: "", + }, + ], + }); + await hideDoorhangerPopup(browser); // make sure the popup is closed for next test + notif && notif.remove(); + } + ); +}); + +add_task(async function contextmenu_fill_generated_password_and_set_username() { + // test when filling with a generated password and editing the username in the form + // * the prompt should display the form's username + // * the auto-saved login should have "" for username + // * confirming the prompt should edit the "" login and add the username + await setup_withOneLogin("olduser", "xyzpassword"); + await openFormInNewTab( + TEST_ORIGIN + FORM_PAGE_PATH, + { + password: { + selector: passwordInputSelector, + expectedValue: "xyzpassword", + setValue: "", + }, + username: { + selector: usernameInputSelector, + expectedValue: "olduser", + setValue: "differentuser", + }, + }, + async function taskFn(browser) { + let storageChangedPromise = TestUtils.topicObserved( + "passwordmgr-storage-changed", + (_, data) => data == "addLogin" + ); + let confirmationHint = document.getElementById("confirmation-hint"); + let hintPromiseShown = BrowserTestUtils.waitForEvent( + confirmationHint, + "popupshown" + ); await ContentTask.spawn( browser, - [passwordInputSelector], - function checkFinalFieldValue(inputSelector) { - let passwordInput = content.document.querySelector(inputSelector); + [passwordInputSelector, usernameInputSelector], + function checkEmptyPasswordField([passwordSelector, usernameSelector]) { is( - passwordInput.value.length, - 15, - "Password field was filled with generated password" + content.document.querySelector(passwordSelector).value, + "", + "Password field is empty" ); } ); + info("waiting to fill generated password using context menu"); + await doFillGeneratedPasswordContextMenuItem( + browser, + passwordInputSelector + ); + info("waiting for password-change doorhanger"); + await waitForDoorhanger(browser, "password-change"); + // Make sure confirmation hint was shown + await hintPromiseShown; + await verifyConfirmationHint(confirmationHint); - // check a dismissed prompt was shown - let notif = getCaptureDoorhanger("password-save"); - ok(notif && notif.dismissed, "Dismissed notification was created"); - ok( - !notif.anchorElement.hasAttribute("extraAttr"), - "Check if icon has the extraAttr attribute" - ); + info("waiting for addLogin"); + await storageChangedPromise; + // Check properties of the newly auto-saved login + verifyLogins({ + count: 2, + loginProperties: [ + null, // ignore the first one + { + timesUsed: 1, + username: "", + passwordLength: 15, + }, + ], + }); - let { passwordValue, usernameValue } = await checkPromptContents( - notif.anchorElement, - browser + info("Waiting to openAndVerifyDoorhanger"); + await openAndVerifyDoorhanger(browser, "password-change", { + dismissed: true, + anchorExtraAttr: "attention", + usernameValue: "differentuser", + passwordLength: 15, + }); + await hideDoorhangerPopup(browser); + info("Waiting to verifyGeneratedPasswordWasFilled"); + await verifyGeneratedPasswordWasFilled(browser, passwordInputSelector); + + info("waiting for submitForm"); + await submitForm(browser); + let notif = await openAndVerifyDoorhanger(browser, "password-change", { + dismissed: false, + anchorExtraAttr: "", + usernameValue: "differentuser", + passwordLength: 15, + }); + + storageChangedPromise = TestUtils.topicObserved( + "passwordmgr-storage-changed", + (_, data) => data == "modifyLogin" ); - is( - passwordValue.length, - 15, - "Doorhanger password field has generated 15-char value" - ); - is(usernameValue, "user1", "Doorhanger username field was popuplated"); - notif.remove(); + await clickDoorhangerButton(notif, CHANGE_BUTTON); + info("Waiting for modifyLogin"); + await storageChangedPromise; + verifyLogins({ + count: 2, + loginProperties: [ + null, + { + username: "differentuser", + passwordLength: 15, + timesUsed: 1, + }, + ], + }); + await hideDoorhangerPopup(browser); // make sure the popup is closed for next test + notif && notif.remove(); } ); }); -add_task(async function password_change_without_username() { +add_task(async function contextmenu_password_change_form_without_username() { + // test doorhanger behavior when a generated password is filled into a change-password + // form with no username + await setup_withOneLogin("user1", "xyzpassword"); + await LoginTestUtils.addLogin({ username: "username2", password: "pass2" }); + const passwordInputSelector = "#newpass"; + const CHANGE_FORM_PATH = "/browser/toolkit/components/passwordmgr/test/browser/form_password_change.html"; - await BrowserTestUtils.withNewTab( + await openFormInNewTab( + TEST_ORIGIN + CHANGE_FORM_PATH, { - gBrowser, - url: TEST_ORIGIN + CHANGE_FORM_PATH, + password: { + selector: passwordInputSelector, + expectedValue: "", + }, }, - async function(browser) { - await SimpleTest.promiseFocus(browser.ownerGlobal); - // Save 2nd login different from the 1st one - LoginTestUtils.addLogin({ - username: "username2", - password: "pass2", - }); + async function taskFn(browser) { + let storageChangedPromise = TestUtils.topicObserved( + "passwordmgr-storage-changed", + (_, data) => data == "addLogin" + ); + let confirmationHint = document.getElementById("confirmation-hint"); + let hintPromiseShown = BrowserTestUtils.waitForEvent( + confirmationHint, + "popupshown" + ); // Make the 2nd field use a generated password - await doFillGeneratedPasswordContextMenuItem(browser, "#newpass"); + info("Using contextmenu to fill with a generated password"); + await doFillGeneratedPasswordContextMenuItem( + browser, + passwordInputSelector + ); - // check a dismissed prompt was shown - let notif = getCaptureDoorhanger("password-save"); - ok(notif && notif.dismissed, "Dismissed notification was created"); + info("waiting for password-change doorhanger"); + await waitForDoorhanger(browser, "password-change"); + // Make sure confirmation hint was shown + await hintPromiseShown; + await verifyConfirmationHint(confirmationHint); - let { passwordValue, usernameValue } = await checkPromptContents( - notif.anchorElement - ); - is( - passwordValue.length, - 15, - "Doorhanger password field has generated 15-char value" - ); - is( - usernameValue, - "", - "Doorhanger username field has the username field value" - ); - ok( - !notif.anchorElement.hasAttribute("extraAttr"), - "Check if icon has the extraAttr attribute" - ); - notif.remove(); - - // Submit the form - await ContentTask.spawn(browser, null, function() { - content.document.querySelector("#form").submit(); + info("waiting for addLogin"); + await storageChangedPromise; + // Check properties of the newly auto-saved login + verifyLogins({ + count: 3, + loginProperties: [ + null, // ignore the first one + null, // ignore the 2nd one + { + timesUsed: 1, + username: "", + passwordLength: 15, + }, + ], }); - // Check a non-dismissed prompt was shown - notif = getCaptureDoorhanger("password-save"); - ok(notif && !notif.dismissed, "Non-dismissed notification was created"); + info("Waiting to openAndVerifyDoorhanger"); + let notif = await openAndVerifyDoorhanger(browser, "password-change", { + dismissed: true, + anchorExtraAttr: "attention", + usernameValue: "", + passwordLength: 15, + }); + await hideDoorhangerPopup(browser); + // remove notification so we can unambiguously check no new notification gets created later + notif && notif.remove(); - ok(!EventUtils.isHidden(notif.anchorElement), "Anchor should be shown"); - let { - passwordValue: passwordValue2, - usernameValue: usernameValue2, - } = await checkPromptContents(notif.anchorElement); - is( - passwordValue2.length, - 15, - "Doorhanger password field has generated 15-char value" + info("Waiting to verifyGeneratedPasswordWasFilled"); + await verifyGeneratedPasswordWasFilled(browser, passwordInputSelector); + + storageChangedPromise = TestUtils.topicObserved( + "passwordmgr-storage-changed", + (_, data) => data == "modifyLogin" ); - is(usernameValue2, "", "Doorhanger username field has no value"); - notif.remove(); + let { timeLastUsed } = Services.logins.getAllLogins()[2]; + + info("waiting for submitForm"); + await submitForm(browser); + + info("Waiting for modifyLogin"); + await storageChangedPromise; + verifyLogins({ + count: 3, + loginProperties: [ + null, // ignore the first one + null, // ignore the 2nd one + { + timesUsed: 2, + usedSince: timeLastUsed, + }, + ], + }); + // Check no new doorhanger was shown + notif = getCaptureDoorhanger("password-change"); + ok(!notif, "No new doorhanger should be shown"); } ); }); diff --git a/toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_onGeneratedPasswordFilledOrEdited.js b/toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_onGeneratedPasswordFilledOrEdited.js index dcda97842feb..4071c7f62515 100644 --- a/toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_onGeneratedPasswordFilledOrEdited.js +++ b/toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_onGeneratedPasswordFilledOrEdited.js @@ -11,6 +11,10 @@ const { LoginManagerParent: LMP } = ChromeUtils.import( const { TestUtils } = ChromeUtils.import( "resource://testing-common/TestUtils.jsm" ); +const loginTemplate = Object.freeze({ + origin: "https://www.example.com", + formActionOrigin: "https://www.mozilla.org", +}); function stubPrompter() { let fakePromptToSavePassword = sinon.stub(); @@ -117,6 +121,21 @@ function startTestConditions(contextId) { ); } +/* + * Compare login details excluding usernameField and passwordField + */ +function assertLoginProperties(actualLogin, expected) { + equal(actualLogin.origin, expected.origin, "Compare origin"); + equal( + actualLogin.formActionOrigin, + expected.formActionOrigin, + "Compare formActionOrigin" + ); + equal(actualLogin.httpRealm, expected.httpRealm, "Compare httpRealm"); + equal(actualLogin.username, expected.username, "Compare username"); + equal(actualLogin.password, expected.password, "Compare password"); +} + add_task(async function setup() { // Get a profile for storage. do_get_profile(); @@ -146,6 +165,7 @@ add_task(async function test_onGeneratedPasswordFilledOrEdited() { browsingContextId: 99, formActionOrigin: "https://www.mozilla.org", password: generatedPassword, + username: "someusername", }); let [login] = await storageChangedPromised; @@ -153,7 +173,7 @@ add_task(async function test_onGeneratedPasswordFilledOrEdited() { "https://www.example.com", "https://www.mozilla.org", null, - "", + "", // verify we don't include the username when auto-saving a login generatedPassword ); @@ -273,3 +293,170 @@ add_task( Services.logins.removeAllLogins(); } ); + +add_task( + async function test_onGeneratedPasswordFilledOrEdited_withSavedEmptyUsername() { + startTestConditions(); + let login0Props = Object.assign({}, loginTemplate, { + username: "", + password: "qweqweq", + }); + info("Adding initial login: " + JSON.stringify(login0Props)); + await LoginTestUtils.addLogin(login0Props); + + info( + "Saved initial login: " + + JSON.stringify(Services.logins.getAllLogins()[0]) + ); + + let { + generatedPassword: password1, + } = stubGeneratedPasswordForBrowsingContextId(99); + let { restorePrompter, fakePromptToChangePassword } = stubPrompter(); + + await LMP._onGeneratedPasswordFilledOrEdited({ + browsingContextId: 99, + formActionOrigin: "https://www.mozilla.org", + password: password1, + }); + equal( + Services.logins.getAllLogins().length, + 1, + "Should just have the previously-saved login with empty username" + ); + assertLoginProperties(Services.logins.getAllLogins()[0], login0Props); + + ok(LMP._getPrompter.calledOnce, "Checking _getPrompter was called"); + ok( + fakePromptToChangePassword.calledOnce, + "Checking promptToChangePassword was called" + ); + ok( + fakePromptToChangePassword.getCall(0).args[2], + "promptToChangePassword had a truthy 'dismissed' argument" + ); + ok( + !fakePromptToChangePassword.getCall(0).args[3], + "promptToChangePassword had a falsey 'notifySaved' argument" + ); + + LMP._browsingContextGlobal.get.restore(); + restorePrompter(); + LMP._generatedPasswordsByPrincipalOrigin.clear(); + Services.logins.removeAllLogins(); + } +); + +add_task( + async function test_onGeneratedPasswordFilledOrEdited_withEmptyUsernameDifferentFormActionOrigin() { + startTestConditions(); + let login0Props = Object.assign({}, loginTemplate, { + username: "", + password: "qweqweq", + }); + await LoginTestUtils.addLogin(login0Props); + + let { + generatedPassword: password1, + } = stubGeneratedPasswordForBrowsingContextId(99); + let { restorePrompter, fakePromptToChangePassword } = stubPrompter(); + + await LMP._onGeneratedPasswordFilledOrEdited({ + browsingContextId: 99, + formActionOrigin: "https://www.elsewhere.com", + password: password1, + }); + + let savedLogins = Services.logins.getAllLogins(); + equal( + savedLogins.length, + 2, + "Should have saved the generated-password login" + ); + + assertLoginProperties(savedLogins[0], login0Props); + assertLoginProperties( + savedLogins[1], + Object.assign({}, loginTemplate, { + formActionOrigin: "https://www.elsewhere.com", + username: "", + password: password1, + }) + ); + + ok(LMP._getPrompter.calledOnce, "Checking _getPrompter was called"); + ok( + fakePromptToChangePassword.calledOnce, + "Checking promptToChangePassword was called" + ); + ok( + fakePromptToChangePassword.getCall(0).args[1], + "promptToChangePassword had a truthy 'dismissed' argument" + ); + ok( + fakePromptToChangePassword.getCall(0).args[2], + "promptToChangePassword had a truthy 'notifySaved' argument" + ); + + LMP._browsingContextGlobal.get.restore(); + restorePrompter(); + LMP._generatedPasswordsByPrincipalOrigin.clear(); + Services.logins.removeAllLogins(); + } +); + +add_task( + async function test_onGeneratedPasswordFilledOrEdited_withSavedUsername() { + startTestConditions(); + let login0Props = Object.assign({}, loginTemplate, { + username: "previoususer", + password: "qweqweq", + }); + await LoginTestUtils.addLogin(login0Props); + + let { + generatedPassword: password1, + } = stubGeneratedPasswordForBrowsingContextId(99); + let { restorePrompter, fakePromptToChangePassword } = stubPrompter(); + + await LMP._onGeneratedPasswordFilledOrEdited({ + browsingContextId: 99, + formActionOrigin: "https://www.mozilla.org", + password: password1, + }); + + let savedLogins = Services.logins.getAllLogins(); + equal( + savedLogins.length, + 2, + "Should have saved the generated-password login" + ); + assertLoginProperties(Services.logins.getAllLogins()[0], login0Props); + assertLoginProperties( + savedLogins[1], + Object.assign({}, loginTemplate, { + username: "", + password: password1, + }) + ); + + ok(LMP._getPrompter.calledOnce, "Checking _getPrompter was called"); + ok( + fakePromptToChangePassword.calledOnce, + "Checking promptToChangePassword was called" + ); + ok( + fakePromptToChangePassword.getCall(0).args[1], + "promptToChangePassword had a truthy 'dismissed' argument" + ); + ok( + fakePromptToChangePassword.getCall(0).args[2], + "promptToChangePassword had a truthy 'notifySaved' argument" + ); + + LMP._browsingContextGlobal.get.restore(); + restorePrompter(); + LMP._generatedPasswordsByPrincipalOrigin.clear(); + Services.logins.removeAllLogins(); + } +);