From 7308fe7e97e07fb0521acce458a10e4bf4ae1f67 Mon Sep 17 00:00:00 2001 From: Sam Foster Date: Tue, 10 Sep 2019 16:51:56 +0000 Subject: [PATCH] Bug 1579539 - Part 2: Always provide autoSavedLoginGuid to promptToChangePassword when available. r=MattN Differential Revision: https://phabricator.services.mozilla.com/D45250 --HG-- extra : moz-landing-system : lando --- .../passwordmgr/LoginManagerParent.jsm | 22 +- .../browser_generated_password_doorhangers.js | 224 ++++++++++++++++++ .../passwordmgr/test/formsubmit.sjs | 4 +- 3 files changed, 230 insertions(+), 20 deletions(-) diff --git a/toolkit/components/passwordmgr/LoginManagerParent.jsm b/toolkit/components/passwordmgr/LoginManagerParent.jsm index 2be8987d6f04..104d5db6fc62 100644 --- a/toolkit/components/passwordmgr/LoginManagerParent.jsm +++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm @@ -603,6 +603,10 @@ this.LoginManagerParent = { let generatedPW = this._generatedPasswordsByPrincipalOrigin.get( framePrincipalOrigin ); + let autoSavedStorageGUID = ""; + if (generatedPW && generatedPW.storageGUID) { + autoSavedStorageGUID = generatedPW.storageGUID; + } // If we didn't find a username field, but seem to be changing a // password, allow the user to select from a list of applicable @@ -622,15 +626,6 @@ this.LoginManagerParent = { return; } - let autoSavedStorageGUID = ""; - if ( - generatedPW && - generatedPW.storageGUID == oldLogin.guid && - generatedPW.value == formLogin.password - ) { - // this login has a generated password, auto-saved in this session - autoSavedStorageGUID = generatedPW.storageGUID; - } formLogin.username = oldLogin.username; formLogin.usernameField = oldLogin.usernameField; @@ -690,15 +685,6 @@ this.LoginManagerParent = { if (existingLogin) { log("Found an existing login matching this form submission"); - let autoSavedStorageGUID = ""; - if ( - generatedPW && - generatedPW.storageGUID == existingLogin.guid && - generatedPW.value == formLogin.password - ) { - // this login has a generated password, auto-saved in this session - autoSavedStorageGUID = generatedPW.storageGUID; - } // Change password if needed. if (existingLogin.password != formLogin.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 b61fb45156fa..4a606119fefb 100644 --- a/toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js +++ b/toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js @@ -1052,3 +1052,227 @@ add_task(async function autosaved_login_updated_to_existing_login() { } ); }); + +add_task(async function autosaved_login_updated_to_existing_login() { + // test when selecting auto-saved generated password in a form filled with an + // existing login and submitting the form: + // * the matching login should be updated + // * the auto-saved login should be deleted + // * the metadata for the matching login should be updated + // * the by-origin cache for the password should point at the updated login + + // clear both fields which should be autofilled with our single login + await setup_withOneLogin("user1", "xyzpassword"); + await openFormInNewTab( + TEST_ORIGIN + FORM_PAGE_PATH, + { + password: { + selector: passwordInputSelector, + expectedValue: "xyzpassword", + setValue: "", + }, + username: { + selector: usernameInputSelector, + expectedValue: "user1", + setValue: "", + }, + }, + async function taskFn(browser) { + await SimpleTest.promiseFocus(browser.ownerGlobal); + + // first, create an auto-saved login with generated password + let storageChangedPromise = TestUtils.topicObserved( + "passwordmgr-storage-changed", + (_, data) => data == "addLogin" + ); + let confirmationHint = document.getElementById("confirmation-hint"); + let hintPromiseShown = BrowserTestUtils.waitForEvent( + confirmationHint, + "popupshown" + ); + + info("waiting to fill generated password using context menu"); + await doFillGeneratedPasswordContextMenuItem( + browser, + passwordInputSelector + ); + + info("waiting for dismissed password-change notification"); + await waitForDoorhanger(browser, "password-change"); + // Make sure confirmation hint was shown + await hintPromiseShown; + await verifyConfirmationHint(confirmationHint); + + info("waiting for addLogin"); + await storageChangedPromise; + info("addLogin promise resolved"); + // Check properties of the newly auto-saved login + let [user1LoginSnapshot, autoSavedLogin] = verifyLogins([ + null, // ignore the first one + { + timesUsed: 1, + username: "", + passwordLength: LoginTestUtils.generation.LENGTH, + }, + ]); + info("user1LoginSnapshot, guid: " + user1LoginSnapshot.guid); + info("autoSavedLogin, guid: " + autoSavedLogin.guid); + + info("verifyLogins ok"); + let passwordCacheEntry = LoginManagerParent._generatedPasswordsByPrincipalOrigin.get( + "https://example.com" + ); + + ok( + passwordCacheEntry, + "Got the cached generated password entry for https://example.com" + ); + is( + passwordCacheEntry.value, + autoSavedLogin.password, + "Cached password matches the auto-saved login password" + ); + is( + passwordCacheEntry.storageGUID, + autoSavedLogin.guid, + "Cached password guid matches the auto-saved login guid" + ); + + let messagePromise = new Promise(resolve => { + const eventName = "PasswordManager:onGeneratedPasswordFilledOrEdited"; + browser.messageManager.addMessageListener( + eventName, + function mgsHandler(msg) { + if (msg.target != browser) { + return; + } + browser.messageManager.removeMessageListener(eventName, mgsHandler); + info("Got onGeneratedPasswordFilledOrEdited, resolving"); + // allow LMP to handle the message, then resolve + SimpleTest.executeSoon(resolve); + } + ); + }); + + info("Waiting to openAndVerifyDoorhanger"); + // also moves focus, producing another onGeneratedPasswordFilledOrEdited message from content + let notif = await openAndVerifyDoorhanger(browser, "password-change", { + dismissed: true, + anchorExtraAttr: "attention", + usernameValue: "", + password: autoSavedLogin.password, + }); + ok(notif, "Got password-change notification"); + + // content sends a 2nd message when we blur the password field, + // wait for that before interacting with doorhanger + info("waiting for messagePromise"); + await messagePromise; + + info("Hiding popup."); + await cleanupDoorhanger(notif); + info("/Hiding popup."); + + // now submit the form with the user1 username and the generated password + let doorhangerShown = waitForDoorhanger(browser, "password-change"); + + // check and ok the password-change doorhanger + let loginModifiedPromise = TestUtils.topicObserved( + "passwordmgr-storage-changed", + (_, data) => { + if (data == "modifyLogin") { + info("passwordmgr-storage-changed, action: " + data); + info("subject: " + JSON.stringify(_)); + return true; + } + return false; + } + ); + let loginRemovedPromise = TestUtils.topicObserved( + "passwordmgr-storage-changed", + (_, data) => { + if (data == "removeLogin") { + info("passwordmgr-storage-changed, action: " + data); + info("subject: " + JSON.stringify(_)); + return true; + } + return false; + } + ); + + // submit the form with the generated password and username set to user1 + info(`submitting form`); + let submitResults = await submitFormAndGetResults( + browser, + "formsubmit.sjs", + { + "#form-basic-username": "user1", + } + ); + is( + submitResults.username, + "user1", + "Form submitted with expected username" + ); + is( + submitResults.password, + autoSavedLogin.password, + "Form submitted with expected password" + ); + info( + `form was submitted, got username/password ${submitResults.username}/${ + submitResults.password + }` + ); + + await doorhangerShown; + notif = await openAndVerifyDoorhanger(browser, "password-change", { + dismissed: false, + anchorExtraAttr: "", + usernameValue: "user1", + password: autoSavedLogin.password, + }); + + let promiseHidden = BrowserTestUtils.waitForEvent( + PopupNotifications.panel, + "popuphidden" + ); + info("clicking change button"); + clickDoorhangerButton(notif, CHANGE_BUTTON); + await promiseHidden; + + info("Waiting for modifyLogin promise"); + await loginModifiedPromise; + + info("Waiting for removeLogin promise"); + await loginRemovedPromise; + + info("storage-change promises resolved"); + // Check the auto-saved login was removed and the original login updated + verifyLogins([ + { + username: "user1", + password: autoSavedLogin.password, + timeCreated: user1LoginSnapshot.timeCreated, + timeLastUsed: user1LoginSnapshot.timeLastUsed, + passwordChangedSince: autoSavedLogin.timePasswordChanged, + }, + ]); + + // Check we have no notifications at this point + ok(!PopupNotifications.isPanelOpen, "No doorhanger is open"); + ok( + !PopupNotifications.getNotification("password", browser), + "No notifications" + ); + + // make sure the cache entry was removed with the removal of the auto-saved login + ok( + !LoginManagerParent._generatedPasswordsByPrincipalOrigin.has( + "https://example.com" + ), + "Generated password cache entry has been removed" + ); + } + ); +}); diff --git a/toolkit/components/passwordmgr/test/formsubmit.sjs b/toolkit/components/passwordmgr/test/formsubmit.sjs index 4b4a387f7b73..e2e2669d74d5 100644 --- a/toolkit/components/passwordmgr/test/formsubmit.sjs +++ b/toolkit/components/passwordmgr/test/formsubmit.sjs @@ -18,12 +18,12 @@ function reallyHandleRequest(request, response) { var user = null, pass = null; // user=xxx - match = /user=([^&]*)/.exec(query); + match = /user(?:name)?=([^&]*)/.exec(query); if (match) user = match[1]; // pass=xxx - match = /pass=([^&]*)/.exec(query); + match = /pass(?:word)?=([^&]*)/.exec(query); if (match) pass = match[1];