Bug 1612255 - Also watch for username changes to prompt to save/update a login. r=MattN

Differential Revision: https://phabricator.services.mozilla.com/D63439

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Sam Foster 2020-02-29 01:27:40 +00:00
Родитель 83bf83bf19
Коммит 0bef0512b7
6 изменённых файлов: 577 добавлений и 34 удалений

Просмотреть файл

@ -239,20 +239,41 @@ const observer = {
// Used to watch for changes to fields filled with generated passwords.
case "change": {
let triggeredByFillingGenerated = docState.generatedPasswordFields.has(
aEvent.target
);
if (
aEvent.target.hasBeenTypePassword &&
(triggeredByFillingGenerated ||
LoginHelper.passwordEditCaptureEnabled)
) {
LoginManagerChild.forWindow(window)._passwordEditedOrGenerated(
aEvent.target,
{
triggeredByFillingGenerated,
}
if (aEvent.target.hasBeenTypePassword) {
let triggeredByFillingGenerated = docState.generatedPasswordFields.has(
aEvent.target
);
if (
triggeredByFillingGenerated ||
LoginHelper.passwordEditCaptureEnabled
) {
LoginManagerChild.forWindow(window)._passwordEditedOrGenerated(
aEvent.target,
{
triggeredByFillingGenerated,
}
);
}
} else {
let [usernameField, passwordField] = LoginManagerChild.forWindow(
window
).getUserNameAndPasswordFields(aEvent.target);
if (
usernameField &&
aEvent.target == usernameField &&
passwordField &&
passwordField.value &&
LoginHelper.passwordEditCaptureEnabled
) {
LoginManagerChild.forWindow(window)._passwordEditedOrGenerated(
passwordField,
{
triggeredByFillingGenerated: docState.generatedPasswordFields.has(
passwordField
),
}
);
}
}
break;
}

Просмотреть файл

@ -847,7 +847,8 @@ class LoginManagerParent extends JSWindowActorParent {
matchedLogin.username == formLogin.username)
) {
log("The filled login matches the changed fields. Nothing to change.");
return;
// We may want to update an existing doorhanger
existingLogin = matchedLogin;
}
}
@ -939,8 +940,9 @@ class LoginManagerParent extends JSWindowActorParent {
shouldAutoSaveLogin = false;
if (matchedLogin.password == formLoginWithoutUsername.password) {
// This login is already saved so show no new UI.
// We may want to update an existing doorhanger though...
log("_onPasswordEditedOrGenerated: Matching login already saved");
return;
existingLogin = matchedLogin;
}
log(
"_onPasswordEditedOrGenerated: Login with empty username already saved for this site"
@ -1092,6 +1094,23 @@ class LoginManagerParent extends JSWindowActorParent {
);
} else {
log("_onPasswordEditedOrGenerated: No change to existing login");
// is there a doorhanger we should update?
let popupNotifications = promptBrowser.ownerGlobal.PopupNotifications;
let notif = popupNotifications.getNotification("password", browser);
log(
"_onPasswordEditedOrGenerated: Has doorhanger?",
notif && notif.dismissed
);
if (notif && notif.dismissed) {
prompter.promptToChangePassword(
promptBrowser,
existingLogin,
formLogin,
true, // dismissed prompt
false, // notifySaved
autoSavedStorageGUID // autoSavedLoginGuid
);
}
}
return;
}

Просмотреть файл

@ -127,8 +127,112 @@ let testCases = [
doorhanger: null,
},
},
{
name: "Change to new username",
prefEnabled: true,
logins: [{ username: "user1", password: "pass1" }],
formDefaults: {},
formChanges: {
[usernameInputSelector]: "user2",
},
expected: {
initialForm: {
username: "user1",
password: "pass1",
},
doorhanger: {
type: "password-save",
dismissed: true,
anchorExtraAttr: "",
username: "user2",
password: "pass1",
toggle: "visible",
},
},
},
// Disabled test tracked in bug 1619030
// {
// name: "Change to existing username, different password",
// prefEnabled: true,
// logins: [{ username: "user-saved", password: "pass1" }],
// formDefaults: {
// [usernameInputSelector]: "user-prefilled",
// [passwordInputSelector]: "pass2",
// },
// formChanges: {
// [usernameInputSelector]: "user-saved",
// },
// expected: {
// initialForm: {
// username: "user-prefilled",
// password: "pass2",
// },
// doorhanger: {
// type: "password-change",
// dismissed: true,
// anchorExtraAttr: "",
// username: "user-saved",
// password: "pass2",
// toggle: "visible",
// },
// },
// },
{
name: "Add username to existing password",
prefEnabled: true,
logins: [{ username: "", password: "pass1" }],
formDefaults: {},
formChanges: {
[usernameInputSelector]: "user1",
},
expected: {
initialForm: {
username: "",
password: "pass1",
},
doorhanger: {
type: "password-change",
dismissed: true,
anchorExtraAttr: "",
username: "user1",
password: "pass1",
toggle: "visible",
},
},
},
{
name: "Change to existing username, password",
prefEnabled: true,
logins: [{ username: "user1", password: "pass1" }],
formDefaults: {
[usernameInputSelector]: "user",
[passwordInputSelector]: "pass",
},
formChanges: {
[passwordInputSelector]: "pass1",
[usernameInputSelector]: "user1",
},
expected: {
initialForm: {
username: "user",
password: "pass",
},
// Eventually we'll want to remove the doorhanger in this case
doorhanger: {
type: "password-change",
dismissed: true,
anchorExtraAttr: "",
username: "user1",
password: "pass1",
toggle: "visible",
},
},
},
];
requestLongerTimeout(2);
SimpleTest.requestCompleteLog();
for (let testData of testCases) {
let tmp = {
async [testData.name]() {
@ -143,6 +247,17 @@ for (let testData of testCases) {
add_task(tmp[testData.name]);
}
async function waitForPromise(promise, timeoutMs = 5000) {
let timedOut = new Promise((resolve, reject) => {
/* eslint-disable-next-line mozilla/no-arbitrary-setTimeout */
let timerId = setTimeout(() => {
clearTimeout(timerId);
reject(`Timed out in ${timeoutMs} ms.`);
}, timeoutMs);
});
await Promise.race([promise, timedOut]);
}
async function testPasswordChange({
logins = [],
formDefaults = {},
@ -182,27 +297,21 @@ async function testPasswordChange({
let passwordEditedMessage = listenForTestNotification(
"PasswordEditedOrGenerated"
);
await changeContentFormValues(browser, formChanges);
info("form edited, waiting for change message");
info(
"form edited, waiting for test notification of PasswordEditedOrGenerated"
);
let gotMessage;
passwordEditedMessage.then(() => (gotMessage = true));
try {
await TestUtils.waitForCondition(
() => {
return gotMessage;
},
`Waiting for passwordEditedMessage`,
undefined,
5
);
await waitForPromise(passwordEditedMessage, 5000);
ok(expected.doorhanger, "Message sent");
} catch (ex) {
ok(!expected.doorhanger, "Message not sent");
ok(!expected.doorhanger, "No message sent");
}
info("Resolved listenForTestNotification promise");
if (expected.doorhanger) {
is(gotMessage, !!expected.doorhanger, "Check message was sent");
} else {
if (!expected.doorhanger) {
let notif;
try {
await TestUtils.waitForCondition(
@ -214,7 +323,7 @@ async function testPasswordChange({
},
`Waiting to ensure no notification`,
undefined,
5
25
);
} catch (ex) {}
ok(!notif, "No doorhanger expected");

Просмотреть файл

@ -223,7 +223,7 @@ async function openAndVerifyDoorhanger(browser, type, expected) {
async function submitForm(browser) {
// Submit the form
info("Now submit the form with the generated password");
info("Now submit the form");
await SpecialPowers.spawn(browser, [], async function() {
content.document.querySelector("form").submit();
@ -1189,7 +1189,7 @@ add_task(async function autosaved_login_updated_to_existing_login_onsubmit() {
});
await cleanupDoorhanger(notif);
// now submit the form with the user1 username and the generated password
// now update and submit the form with the user1 username and the generated password
info(`submitting form`);
let submitResults = await submitFormAndGetResults(
browser,
@ -1289,3 +1289,203 @@ add_task(async function autosaved_login_updated_to_existing_login_onsubmit() {
}
);
});
add_task(async function form_change_from_autosaved_login_to_existing_login() {
// test when changing from a generated password in a form to an existing saved login
// * the auto-saved login should not be deleted
// * the metadata for the matching login should be updated
// * the by-origin cache for the password should point at the autosaved login
await SpecialPowers.pushPrefEnv({
set: [["signon.passwordEditCapture.enabled", true]],
});
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.getGeneratedPasswordsByPrincipalOrigin().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 notif = await openAndVerifyDoorhanger(browser, "password-change", {
dismissed: true,
anchorExtraAttr: "attention",
usernameValue: "",
password: autoSavedLogin.password,
});
// close but don't remove the doorhanger, we want to ensure it is updated/replaced on further form edits
let promiseHidden = BrowserTestUtils.waitForEvent(
PopupNotifications.panel,
"popuphidden"
);
let PN = notif.owner;
PN.panel.hidePopup();
await promiseHidden;
// now update the form with the user1 username and password
info(`updating form`);
let passwordEditedMessage = listenForTestNotification(
"PasswordEditedOrGenerated"
);
let passwordChangeDoorhangerPromise = waitForDoorhanger(
browser,
"password-change"
);
let hintDidShow = false;
hintPromiseShown = BrowserTestUtils.waitForPopupEvent(
confirmationHint,
"shown"
);
hintPromiseShown.then(() => (hintDidShow = true));
await changeContentFormValues(browser, {
[passwordInputSelector]: user1LoginSnapshot.password,
[usernameInputSelector]: user1LoginSnapshot.username,
});
info(
"form edited, waiting for test notification of PasswordEditedOrGenerated"
);
await passwordEditedMessage;
info("Resolved listenForTestNotification promise");
await passwordChangeDoorhangerPromise;
// wait to ensure there's no confirmation hint
try {
await TestUtils.waitForCondition(
() => {
return hintDidShow;
},
`Waiting for confirmationHint popup`,
undefined,
25
);
} catch (ex) {
info("Got expected timeout from the waitForCondition: ", ex);
} finally {
ok(!hintDidShow, "No confirmation hint shown");
}
// the previous doorhanger would have old values, verify it was updated/replaced with new values from the form
notif = await openAndVerifyDoorhanger(browser, "password-change", {
dismissed: true,
anchorExtraAttr: "",
usernameValue: user1LoginSnapshot.username,
passwordLength: user1LoginSnapshot.password.length,
});
await cleanupDoorhanger(notif);
storageChangedPromise = TestUtils.topicObserved(
"passwordmgr-storage-changed",
(_, data) => data == "modifyLogin"
);
// submit the form to ensure the correct updates are made
await submitForm(browser);
info("form submitted, waiting for storage changed");
await storageChangedPromise;
// Check the auto-saved login has not changed and only metadata on the original login updated
verifyLogins([
{
username: "user1",
password: "xyzpassword",
timeCreated: user1LoginSnapshot.timeCreated,
usedSince: user1LoginSnapshot.timeLastUsed,
},
{
username: "",
password: autoSavedLogin.password,
timeCreated: autoSavedLogin.timeCreated,
timeLastUsed: autoSavedLogin.timeLastUsed,
},
]);
// 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 is unchanged with the removal of the auto-saved login
is(
autoSavedLogin.password,
LoginManagerParent.getGeneratedPasswordsByPrincipalOrigin().get(
"https://example.com"
).value,
"Generated password cache entry has the expected password value"
);
}
);
await SpecialPowers.popPrefEnv();
});

Просмотреть файл

@ -730,6 +730,7 @@ async function changeContentFormValues(browser, selectorValues) {
for (let [sel, value] of Object.entries(selectorValues)) {
info("changeContentFormValues, update: " + sel + ", to: " + value);
await changeContentInputValue(browser, sel, value);
await TestUtils.waitForTick();
}
}

Просмотреть файл

@ -11,6 +11,10 @@ const { LoginManagerParent } = ChromeUtils.import(
const { LoginManagerPrompter } = ChromeUtils.import(
"resource://gre/modules/LoginManagerPrompter.jsm"
);
const { PopupNotifications } = ChromeUtils.import(
"resource://gre/modules/PopupNotifications.jsm"
);
const { TestUtils } = ChromeUtils.import(
"resource://testing-common/TestUtils.jsm"
);
@ -407,8 +411,22 @@ add_task(
add_task(async function test_addUsernameBeforeAutoSaveEdit() {
startTestConditions(99);
let { generatedPassword } = stubGeneratedPasswordForBrowsingContextId(99);
let { fakePromptToChangePassword, restorePrompter } = stubPrompter();
let {
fakePromptToChangePassword,
restorePrompter,
resetPrompterHistory,
} = stubPrompter();
let rootBrowser = LMP.getRootBrowser();
let fakePopupNotifications = {
getNotification: sinon.stub().returns({ dismissed: true }),
};
sinon.stub(LoginHelper, "getBrowserForPrompt").callsFake(() => {
return {
ownerGlobal: {
PopupNotifications: fakePopupNotifications,
},
};
});
let storageChangedPromised = TestUtils.topicObserved(
"passwordmgr-storage-changed",
@ -454,6 +472,13 @@ add_task(async function test_addUsernameBeforeAutoSaveEdit() {
"promptToChangePassword had a truthy 'notifySaved' argument"
);
info("Checking the getNotification stub");
ok(
!fakePopupNotifications.getNotification.called,
"getNotification didn't get called yet"
);
resetPrompterHistory();
info("Add a username in storage");
let loginWithUsername = login.clone();
loginWithUsername.username = "added_username";
@ -465,6 +490,7 @@ add_task(async function test_addUsernameBeforeAutoSaveEdit() {
"passwordmgr-storage-changed",
(_, data) => data == "modifyLogin"
);
// will update the doorhanger with changed username
await LMP._onPasswordEditedOrGenerated(rootBrowser, {
browsingContextId: 99,
origin: "https://www.example.com",
@ -489,6 +515,26 @@ add_task(async function test_addUsernameBeforeAutoSaveEdit() {
"Should have 1 saved login still"
);
ok(
fakePopupNotifications.getNotification.calledOnce,
"getNotification was called"
);
ok(LMP._getPrompter.calledOnce, "Checking _getPrompter was called");
ok(
fakePromptToChangePassword.calledOnce,
"Checking promptToChangePassword was called"
);
ok(
fakePromptToChangePassword.getCall(0).args[3],
"promptToChangePassword had a truthy 'dismissed' argument"
);
// No new password is being saved, so notifySaved should be false
ok(
!fakePromptToChangePassword.getCall(0).args[4],
"promptToChangePassword should have a falsey 'notifySaved' argument"
);
resetPrompterHistory();
info(
"Simulate a second edit to check that the telemetry event for the first edit is not recorded twice"
);
@ -497,6 +543,7 @@ add_task(async function test_addUsernameBeforeAutoSaveEdit() {
"passwordmgr-storage-changed",
(_, data) => data == "modifyLogin"
);
info("Calling _onPasswordEditedOrGenerated again");
await LMP._onPasswordEditedOrGenerated(rootBrowser, {
browsingContextId: 99,
origin: "https://www.example.com",
@ -523,7 +570,153 @@ add_task(async function test_addUsernameBeforeAutoSaveEdit() {
checkEditTelemetryRecorded(1, "with auto-save");
ok(
fakePromptToChangePassword.calledOnce,
"Checking promptToChangePassword was called"
);
equal(
fakePromptToChangePassword.getCall(0).args[2].password,
newerPassword,
"promptToChangePassword had the updated password"
);
ok(
fakePromptToChangePassword.getCall(0).args[3],
"promptToChangePassword had a truthy 'dismissed' argument"
);
LoginManagerParent._browsingContextGlobal.get.restore();
LoginHelper.getBrowserForPrompt.restore();
restorePrompter();
LoginManagerParent.getGeneratedPasswordsByPrincipalOrigin().clear();
Services.logins.removeAllLogins();
Services.telemetry.clearEvents();
});
add_task(async function test_editUsernameOfFilledSavedLogin() {
startTestConditions(99);
stubGeneratedPasswordForBrowsingContextId(99);
let {
fakePromptToChangePassword,
fakePromptToSavePassword,
restorePrompter,
resetPrompterHistory,
} = stubPrompter();
let rootBrowser = LMP.getRootBrowser();
let fakePopupNotifications = {
getNotification: sinon.stub().returns({ dismissed: true }),
};
sinon.stub(LoginHelper, "getBrowserForPrompt").callsFake(() => {
return {
ownerGlobal: {
PopupNotifications: fakePopupNotifications,
},
};
});
let login0Props = Object.assign({}, loginTemplate, {
username: "someusername",
password: "qweqweq",
});
info("Adding initial login: " + JSON.stringify(login0Props));
let savedLogin = await LoginTestUtils.addLogin(login0Props);
info(
"Saved initial login: " + JSON.stringify(Services.logins.getAllLogins()[0])
);
equal(
Services.logins.getAllLogins().length,
1,
"Should have 1 saved login at the start of the test"
);
// first prompt to save a new login
let newUsername = "differentuser";
let newPassword = login0Props.password + "🔥";
await LMP._onPasswordEditedOrGenerated(rootBrowser, {
browsingContextId: 99,
origin: "https://www.example.com",
formActionOrigin: "https://www.mozilla.org",
autoFilledLoginGuid: savedLogin.guid,
newPasswordField: { value: newPassword },
usernameField: { value: newUsername },
triggeredByFillingGenerated: false,
});
let expected = new LoginInfo(
login0Props.origin,
login0Props.formActionOrigin,
null,
newUsername,
newPassword
);
ok(LMP._getPrompter.calledOnce, "Checking _getPrompter was called");
info("Checking the getNotification stub");
ok(
!fakePopupNotifications.getNotification.called,
"getNotification was not called"
);
ok(
fakePromptToSavePassword.calledOnce,
"Checking promptToSavePassword was called"
);
ok(
fakePromptToSavePassword.getCall(0).args[2],
"promptToSavePassword had a truthy 'dismissed' argument"
);
ok(
!fakePromptToSavePassword.getCall(0).args[3],
"promptToSavePassword had a falsey 'notifySaved' argument"
);
assertLoginProperties(fakePromptToSavePassword.getCall(0).args[1], expected);
resetPrompterHistory();
// then prompt with matching username/password
await LMP._onPasswordEditedOrGenerated(rootBrowser, {
browsingContextId: 99,
origin: "https://www.example.com",
formActionOrigin: "https://www.mozilla.org",
autoFilledLoginGuid: savedLogin.guid,
newPasswordField: { value: login0Props.password },
usernameField: { value: login0Props.username },
triggeredByFillingGenerated: false,
});
expected = new LoginInfo(
login0Props.origin,
login0Props.formActionOrigin,
null,
login0Props.username,
login0Props.password
);
ok(LMP._getPrompter.calledOnce, "Checking _getPrompter was called");
info("Checking the getNotification stub");
ok(
fakePopupNotifications.getNotification.called,
"getNotification was called"
);
ok(
fakePromptToChangePassword.calledOnce,
"Checking promptToChangePassword was called"
);
ok(
fakePromptToChangePassword.getCall(0).args[3],
"promptToChangePassword had a truthy 'dismissed' argument"
);
ok(
!fakePromptToChangePassword.getCall(0).args[4],
"promptToChangePassword had a falsey 'notifySaved' argument"
);
assertLoginProperties(
fakePromptToChangePassword.getCall(0).args[2],
expected
);
resetPrompterHistory();
LoginManagerParent._browsingContextGlobal.get.restore();
LoginHelper.getBrowserForPrompt.restore();
restorePrompter();
LoginManagerParent.getGeneratedPasswordsByPrincipalOrigin().clear();
Services.logins.removeAllLogins();