Bug 1566536 - Support password generation in private windows. r=sfoster

* Inform users in autocomplete that using a generated password will cause it to be automatically saved.
* Don't fill generated passwords from the context menu directly, that menu item (renamed) should simply force the generation UI to appear in autocomplete.

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Matthew Noorenberghe 2019-11-23 03:26:03 +00:00
parent 23395b20e2
commit 66f2568013
17 changed files with 200 additions and 139 deletions

View File

@ -444,10 +444,10 @@
hidden="true"/>
<menuseparator id="generated-password-separator"/>
<menuitem id="fill-login-generated-password"
label="&fillGeneratedPassword.label;"
accesskey="&fillGeneratedPassword.accesskey;"
label="&useGeneratedPassword.label;"
accesskey="&useGeneratedPassword.accesskey;"
hidden="true"
oncommand="gContextMenu.fillGeneratedPassword();"/>
oncommand="gContextMenu.useGeneratedPassword();"/>
<menuseparator id="saved-logins-separator"/>
<menuitem id="fill-login-saved-passwords"
label="&viewSavedLogins.label;"

View File

@ -994,12 +994,6 @@ class nsContextMenu {
this.showItem("fill-login-generated-password", canFillGeneratedPassword);
this.showItem("generated-password-separator", canFillGeneratedPassword);
this.setItemAttr(
"fill-login-generated-password",
"disabled",
PrivateBrowsingUtils.isWindowPrivate(window)
);
if (!fragment) {
return;
}
@ -1019,8 +1013,8 @@ class nsContextMenu {
});
}
fillGeneratedPassword() {
nsContextMenu.LoginManagerContextMenu.fillGeneratedPassword(
useGeneratedPassword() {
nsContextMenu.LoginManagerContextMenu.useGeneratedPassword(
this.targetIdentifier,
this.contentData.documentURIObject,
this.browser

View File

@ -67,6 +67,7 @@
white-space: nowrap;
}
#PopupAutoComplete > richlistbox > richlistitem[originaltype="generatedPassword"] > .two-line-wrapper > .labels-wrapper > .generated-password-autosave,
#PopupAutoComplete > richlistbox > richlistitem > .two-line-wrapper > .labels-wrapper > .line2-label {
padding-top: 2px !important;
opacity: .6;
@ -98,6 +99,11 @@
font-family: monospace;
}
#PopupAutoComplete > richlistbox > richlistitem[originaltype="generatedPassword"] > .two-line-wrapper > .labels-wrapper > .generated-password-autosave {
font-style: italic;
font-size: 0.85em;
}
#PopupAutoComplete > richlistbox > richlistitem[originaltype="login"] + richlistitem[originaltype="generatedPassword"],
#PopupAutoComplete > richlistbox > richlistitem[originaltype="loginWithOrigin"] + richlistitem[originaltype="generatedPassword"] {
/* Separator between logins and generated passwords. This uses --panel-separator-color from default

View File

@ -227,7 +227,7 @@ function LoginAutoCompleteResult(
// Don't show the footer on non-empty password fields as it's not providing
// value and only adding noise since a password was already filled.
if (isPasswordField && aSearchString) {
if (isPasswordField && aSearchString && !generatedPassword) {
log.debug("Hiding footer: non-empty password field");
return false;
}
@ -409,6 +409,7 @@ LoginAutoComplete.prototype = {
// Don't show autocomplete results for about: pages.
return;
}
// Show the insecure login warning in the passwords field on null principal documents.
let isSecure = !isNullPrincipal;
// Avoid loading InsecurePasswordUtils.jsm in a sandboxed document (e.g. an ad. frame) if we
@ -470,8 +471,13 @@ LoginAutoComplete.prototype = {
return;
}
if (isPasswordField && aSearchString) {
// Return empty result on password fields with password already filled.
if (
isPasswordField &&
aSearchString &&
!loginManagerActor.isPasswordGenerationForcedOn(aElement)
) {
// Return empty result on password fields with password already filled,
// unless password generation was forced.
let acLookupPromise = (this._autoCompleteLookupPromise = Promise.resolve({
logins: [],
}));

View File

@ -469,15 +469,6 @@ LoginManager.prototype = {
if (!matchData.hasKey("origin")) {
log.warn("searchLogins: An `origin` is recommended");
}
if (
!matchData.hasKey("formActionOrigin") &&
!matchData.hasKey("httpRealm")
) {
log.warn(
"searchLogins: `formActionOrigin` or `httpRealm` is recommended"
);
}
}
return this._storage.searchLogins(matchData);

View File

@ -393,6 +393,12 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
* frames, to the current state used by the Login Manager.
*/
this._loginFormStateByDocument = new WeakMap();
/**
* Set of fields where the user specifically requested password generation
* (from the context menu) even if we wouldn't offer it on this field by default.
*/
this._fieldsWithPasswordGenerationForcedOn = new WeakSet();
}
static forWindow(window) {
@ -457,33 +463,26 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
break;
}
case "PasswordManager:fillGeneratedPassword": {
// make a login for the password
let generatedLogin = Cc[
"@mozilla.org/login-manager/loginInfo;1"
].createInstance(Ci.nsILoginInfo);
generatedLogin.init(
msg.data.origin,
"", // empty formActionOrigin
null, // no realm
"", // empty username
msg.data.password
);
this.fillForm({
loginFormOrigin: msg.data.origin,
loginsFound: [generatedLogin],
recipes: msg.data.recipes,
inputElementIdentifier: msg.data.inputElementIdentifier,
originMatches: msg.data.originMatches,
});
case "PasswordManager:useGeneratedPassword": {
let inputElement = ContentDOMReference.resolve(
msg.data.inputElementIdentifier
);
if (inputElement) {
this._generatedPasswordFilledOrEdited(inputElement);
} else {
if (!inputElement) {
log("Could not resolve inputElementIdentifier to a living element.");
break;
}
if (inputElement != gFormFillService.focusedInput) {
log("Could not open popup on input that's no longer focused");
break;
}
this._fieldsWithPasswordGenerationForcedOn.add(inputElement);
// Clear the cache of previous autocomplete results so that the
// generation option appears.
gFormFillService.QueryInterface(Ci.nsIAutoCompleteInput);
gFormFillService.controller.resetInternalState();
gFormFillService.showPopup();
break;
}
@ -603,14 +602,21 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
}
: null;
let isPasswordField = aElement.type == "password";
let forcePasswordGeneration = false;
if (isPasswordField) {
forcePasswordGeneration = this.isPasswordGenerationForcedOn(aElement);
}
let messageData = {
autocompleteInfo,
formOrigin,
actionOrigin,
searchString: aSearchString,
previousResult,
forcePasswordGeneration,
isSecure: InsecurePasswordUtils.isFormSecure(form),
isPasswordField: aElement.type == "password",
isPasswordField,
};
if (LoginHelper.showAutoCompleteFooter) {
@ -862,6 +868,10 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
.catch(Cu.reportError);
}
isPasswordGenerationForcedOn(passwordField) {
return this._fieldsWithPasswordGenerationForcedOn.has(passwordField);
}
/**
* Retrieves a reference to the state object associated with the given
* document. This is initialized to an object with default values.
@ -1617,12 +1627,10 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
// Unmask the password field
this._togglePasswordFieldMasking(passwordField, true);
if (PrivateBrowsingUtils.isContentWindowPrivate(win)) {
log(
"_generatedPasswordFilledOrEdited: not automatically saving the password in private browsing mode"
);
return;
}
// Once the generated password was filled we no longer want to autocomplete
// saved logins into a non-empty password field (see LoginAutoComplete.startSearch)
// because it is confusing.
this._fieldsWithPasswordGenerationForcedOn.delete(passwordField);
let loginForm = LoginFormFactory.createFromField(passwordField);
let formActionOrigin = LoginHelper.getFormActionOrigin(loginForm);

View File

@ -142,42 +142,16 @@ this.LoginManagerContextMenu = {
}
},
async fillGeneratedPassword(inputElementIdentifier, documentURI, browser) {
/**
* Show the password autocomplete UI with the generation option forced to appear.
*/
async useGeneratedPassword(inputElementIdentifier, documentURI, browser) {
let browsingContextId = inputElementIdentifier.browsingContextId;
let browsingContext = BrowsingContext.get(browsingContextId);
if (!browsingContext) {
return;
}
let actor = browsingContext.currentWindowGlobal.getActor("LoginManager");
if (!actor) {
return;
}
let password = actor.getGeneratedPassword();
let origin = LoginHelper.getLoginOrigin(documentURI.spec);
log.debug("fillGeneratedPassword into:", inputElementIdentifier, origin);
let recipes = [];
let formHost;
try {
formHost = documentURI.hostPort;
let recipeManager = await LoginManagerParent.recipeParentPromise;
recipes = recipeManager.getRecipesForHost(formHost);
} catch (ex) {
// Some schemes e.g. chrome aren't supported by URL
log.debug("Couldnt get recipes for formHost:", formHost, ex);
}
let browserURI = browser.browsingContext.currentWindowGlobal.documentURI;
let originMatches = LoginHelper.getLoginOrigin(browserURI) == origin;
actor.sendAsyncMessage("PasswordManager:fillGeneratedPassword", {
password,
origin,
originMatches,
actor.sendAsyncMessage("PasswordManager:useGeneratedPassword", {
inputElementIdentifier,
recipes,
});
},

View File

@ -381,6 +381,7 @@ class LoginManagerParent extends JSWindowActorParent {
actionOrigin,
searchString,
previousResult,
forcePasswordGeneration,
isSecure,
isPasswordField,
}) {
@ -446,14 +447,12 @@ class LoginManagerParent extends JSWindowActorParent {
return match && match.toLowerCase().startsWith(searchStringLower);
});
let browser = this.getRootBrowser();
let generatedPassword = null;
if (
isPasswordField &&
autocompleteInfo.fieldName == "new-password" &&
Services.logins.getLoginSavingEnabled(formOrigin) &&
(!browser || !PrivateBrowsingUtils.isWindowPrivate(browser.ownerGlobal))
forcePasswordGeneration ||
(isPasswordField &&
autocompleteInfo.fieldName == "new-password" &&
Services.logins.getLoginSavingEnabled(formOrigin))
) {
generatedPassword = this.getGeneratedPassword();
}

View File

@ -4,7 +4,7 @@ const FORM_PAGE_PATH =
"/browser/toolkit/components/passwordmgr/test/browser/form_basic.html";
const passwordInputSelector = "#form-basic-password";
add_task(async function test_autocomplete_popup_item_hidden() {
add_task(async function test_autocomplete_new_password_popup_item_visible() {
await LoginTestUtils.addLogin({ username: "username", password: "pass1" });
const win = await BrowserTestUtils.openNewBrowserWindow({ private: true });
const doc = win.document;
@ -15,12 +15,20 @@ add_task(async function test_autocomplete_popup_item_hidden() {
},
async function(browser) {
await SimpleTest.promiseFocus(browser.ownerGlobal);
await ContentTask.spawn(
browser,
[passwordInputSelector],
function openAutocomplete(sel) {
content.document.querySelector(sel).autocomplete = "new-password";
}
);
let popup = doc.getElementById("PopupAutoComplete");
ok(popup, "Got popup");
await openACPopup(popup, browser, passwordInputSelector);
let item = popup.querySelector(`[originaltype="generatedPassword"]`);
ok(!item, "Should not get 'Generate password' richlistitem");
ok(item, "Should get 'Generate password' richlistitem");
let onPopupClosed = BrowserTestUtils.waitForCondition(
() => !popup.popupOpen,
@ -35,7 +43,7 @@ add_task(async function test_autocomplete_popup_item_hidden() {
await BrowserTestUtils.closeWindow(win);
});
add_task(async function test_autocomplete_menu_item_disabled() {
add_task(async function test_autocomplete_menu_item_enabled() {
const win = await BrowserTestUtils.openNewBrowserWindow({ private: true });
const doc = win.document;
await BrowserTestUtils.withNewTab(
@ -50,9 +58,9 @@ add_task(async function test_autocomplete_menu_item_disabled() {
"fill-login-generated-password"
);
is(
generatedPasswordItem.getAttribute("disabled"),
"true",
"Generate password context menu item should be disabled in PB mode"
generatedPasswordItem.hasAttribute("disabled"),
false,
"Generate password context menu item should be enabled in PB mode"
);
await closePopup(document.getElementById("contentAreaContextMenu"));
}

View File

@ -161,8 +161,24 @@ add_task(async function fill_generated_password_empty_field() {
"Password field should be highlighted"
);
LTU.loginField.checkPasswordMasked(input, false, "after fill");
info("cleaing the field");
input.setUserInput("");
}
);
let acPopup = document.getElementById("PopupAutoComplete");
await openACPopup(acPopup, browser, passwordInputSelector);
let pwgenItem = acPopup.querySelector(
`[originaltype="generatedPassword"]`
);
ok(
!pwgenItem || EventUtils.isHidden(pwgenItem),
"pwgen item should no longer be shown"
);
await closePopup(acPopup);
}
);
});

View File

@ -39,30 +39,7 @@ async function fillGeneratedPasswordFromACPopup(
let popup = document.getElementById("PopupAutoComplete");
ok(popup, "Got popup");
await openACPopup(popup, browser, passwordInputSelector);
let item = popup.querySelector(`[originaltype="generatedPassword"]`);
ok(item, "Got generated password richlistitem");
await TestUtils.waitForCondition(() => {
return !EventUtils.isHidden(item);
}, "Waiting for item to become visible");
let inputEventPromise = ContentTask.spawn(
browser,
[passwordInputSelector],
async function waitForInput(inputSelector) {
let passwordInput = content.document.querySelector(inputSelector);
await ContentTaskUtils.waitForEvent(
passwordInput,
"input",
"Password input value changed"
);
}
);
info("Clicking the generated password AC item");
EventUtils.synthesizeMouseAtCenter(item, {});
info("Waiting for the content input value to change");
await inputEventPromise;
await fillGeneratedPasswordFromOpenACPopup(browser, passwordInputSelector);
}
async function checkPromptContents(

View File

@ -536,6 +536,42 @@ async function closePopup(popup) {
}
}
async function fillGeneratedPasswordFromOpenACPopup(
browser,
passwordInputSelector
) {
let popup = browser.ownerDocument.getElementById("PopupAutoComplete");
let item;
await TestUtils.waitForCondition(() => {
item = popup.querySelector(`[originaltype="generatedPassword"]`);
return item && !EventUtils.isHidden(item);
}, "Waiting for item to become visible");
let inputEventPromise = ContentTask.spawn(
browser,
[passwordInputSelector],
async function waitForInput(inputSelector) {
let passwordInput = content.document.querySelector(inputSelector);
await ContentTaskUtils.waitForEvent(
passwordInput,
"input",
"Password input value changed"
);
}
);
let passwordGeneratedPromise = listenForTestNotification(
"PasswordFilledOrEdited"
);
info("Clicking the generated password AC item");
EventUtils.synthesizeMouseAtCenter(item, {});
info("Waiting for the content input value to change");
await inputEventPromise;
await passwordGeneratedPromise;
}
// Contextmenu functions //
/**
@ -648,26 +684,16 @@ async function doFillGeneratedPasswordContextMenuItem(browser, passwordInput) {
"separator is visible"
);
let passwordChangedPromise = ContentTask.spawn(
browser,
[passwordInput],
async function(passwordInput) {
let input = content.document.querySelector(passwordInput);
await ContentTaskUtils.waitForEvent(input, "input");
}
);
let popup = document.getElementById("PopupAutoComplete");
ok(popup, "Got popup");
let promiseShown = BrowserTestUtils.waitForEvent(popup, "popupshown");
let passwordGeneratedPromise = listenForTestNotification(
"PasswordFilledOrEdited"
);
await new Promise(resolve => {
SimpleTest.executeSoon(resolve);
});
EventUtils.synthesizeMouseAtCenter(generatedPasswordItem, {});
info(
"doFillGeneratedPasswordContextMenuItem: Waiting for content input event"
);
await passwordChangedPromise;
await passwordGeneratedPromise;
await promiseShown;
await fillGeneratedPasswordFromOpenACPopup(browser, passwordInput);
}

View File

@ -1023,13 +1023,28 @@ add_task(async function test_all_patterns() {
],
},
{
description:
"If a generated password is passed then show it even if there is a search string. This handles when forcing the generation option from the context menu of a non-empty field",
generatedPassword: "9ljgfd4shyktb45",
insecureFieldWarningEnabled: true,
isSecure: true,
isPasswordField: true,
matchingLogins: [],
searchString: "9ljgfd4shyktb45",
items: [],
items: [
{
value: "9ljgfd4shyktb45",
label: "Use a Securely Generated Password",
style: "generatedPassword",
comment: "9ljgfd4shyktb45",
},
{
value: "",
label: "View Saved Logins",
style: "loginsFooter",
comment: "mochi.test",
},
],
},
{
description: "secure username field on sub.mochi.test",

View File

@ -425,7 +425,7 @@
options = { is: "autocomplete-creditcard-insecure-field" };
break;
case "generatedPassword":
options = { is: "autocomplete-two-line-richlistitem" };
options = { is: "autocomplete-generated-password-richlistitem" };
break;
case "insecureWarning":
options = { is: "autocomplete-richlistitem-insecure-warning" };

View File

@ -719,6 +719,36 @@
}
}
class MozAutocompleteGeneratedPasswordRichlistitem extends MozAutocompleteTwoLineRichlistitem {
constructor() {
super();
this.line3 = document.createElement("div");
this.line3.className = "label-row generated-password-autosave";
this.line3.textContent = this._autoSaveString;
}
get _autoSaveString() {
if (!this.__autoSaveString) {
let brandShorterName = Services.strings
.createBundle("chrome://branding/locale/brand.properties")
.GetStringFromName("brandShorterName");
this.__autoSaveString = Services.strings
.createBundle("chrome://passwordmgr/locale/passwordmgr.properties")
.formatStringFromName("generatedPasswordWillBeSaved", [
brandShorterName,
]);
}
return this.__autoSaveString;
}
_adjustAcItem() {
this.querySelector(".labels-wrapper").append(this.line3);
super._adjustAcItem();
}
}
customElements.define(
"autocomplete-richlistitem",
MozElements.MozAutocompleteRichlistitem,
@ -758,4 +788,12 @@
extends: "richlistitem",
}
);
customElements.define(
"autocomplete-generated-password-richlistitem",
MozAutocompleteGeneratedPasswordRichlistitem,
{
extends: "richlistitem",
}
);
}

View File

@ -29,8 +29,8 @@
<!ENTITY fillLoginMenu.label "Fill Login">
<!ENTITY fillLoginMenu.accesskey "F">
<!ENTITY fillGeneratedPassword.label "Use a Securely Generated Password">
<!ENTITY fillGeneratedPassword.accesskey "S">
<!ENTITY useGeneratedPassword.label "Use a Securely Generated Password…">
<!ENTITY useGeneratedPassword.accesskey "S">
<!ENTITY fillPasswordMenu.label "Fill Password">
<!ENTITY fillPasswordMenu.accesskey "F">
<!ENTITY fillUsernameMenu.label "Fill Username">

View File

@ -47,6 +47,9 @@ loginsDescriptionAll2=Logins for the following sites are stored on your computer
# LOCALIZATION NOTE (useASecurelyGeneratedPassword):
# Shown in the autocomplete popup to allow filling a generated password into a password field.
useASecurelyGeneratedPassword=Use a Securely Generated Password
# LOCALIZATION NOTE (generatedPasswordWillBeSaved):
# %S will contain the brandShorterName. This informs the user that the generated password will be automatically saved.
generatedPasswordWillBeSaved=%S will save this password for this website.
# LOCALIZATION NOTE (loginHostAge):
# This is used to show the context menu login items with their age.
# 1st string is the username for the login, 2nd is the login's age.