diff --git a/browser/components/aboutlogins/AboutLoginsParent.jsm b/browser/components/aboutlogins/AboutLoginsParent.jsm index 5fa17f5740b5..860dee978e60 100644 --- a/browser/components/aboutlogins/AboutLoginsParent.jsm +++ b/browser/components/aboutlogins/AboutLoginsParent.jsm @@ -560,6 +560,30 @@ var AboutLogins = { if (!login) { return; } + + if (BREACH_ALERTS_ENABLED) { + let breachesForThisLogin = await LoginBreaches.getPotentialBreachesByLoginGUID( + [login] + ); + let breachData = breachesForThisLogin.size + ? breachesForThisLogin.get(login.guid) + : false; + this.messageSubscribers( + "AboutLogins:UpdateBreaches", + new Map([[login.guid, breachData]]) + ); + if (VULNERABLE_PASSWORDS_ENABLED) { + let vulnerablePasswordsForThisLogin = await LoginBreaches.getPotentiallyVulnerablePasswordsByLoginGUID( + [login] + ); + let isLoginVulnerable = !!vulnerablePasswordsForThisLogin.size; + this.messageSubscribers( + "AboutLogins:UpdateVulnerableLogins", + new Map([[login.guid, isLoginVulnerable]]) + ); + } + } + this.messageSubscribers("AboutLogins:LoginModified", login); break; } diff --git a/browser/components/aboutlogins/content/components/login-intro.js b/browser/components/aboutlogins/content/components/login-intro.js index dc1cd1f91752..e44d4654410b 100644 --- a/browser/components/aboutlogins/content/components/login-intro.js +++ b/browser/components/aboutlogins/content/components/login-intro.js @@ -15,6 +15,7 @@ export default class LoginIntro extends HTMLElement { this._importText = shadowRoot.querySelector(".intro-import-text"); this._importText.addEventListener("click", this); + this.addEventListener("AboutLoginsUtilsReady", this); } diff --git a/browser/components/aboutlogins/content/components/login-item.js b/browser/components/aboutlogins/content/components/login-item.js index ac10846eb204..275bc0017b88 100644 --- a/browser/components/aboutlogins/content/components/login-item.js +++ b/browser/components/aboutlogins/content/components/login-item.js @@ -130,7 +130,9 @@ export default class LoginItem extends HTMLElement { } } - async render() { + async render( + { onlyUpdateErrorsAndAlerts } = { onlyUpdateErrorsAndAlerts: false } + ) { if (this._error) { if (this._error.errorMessage.includes("This login already exists")) { document.l10n.setAttributes( @@ -193,6 +195,9 @@ export default class LoginItem extends HTMLElement { window.AboutLoginsUtils.supportBaseURL + "lockwise-alerts" ); } + if (onlyUpdateErrorsAndAlerts) { + return; + } document.l10n.setAttributes(this._timeCreated, "login-item-time-created", { timeCreated: this._login.timeCreated || "", }); @@ -285,7 +290,7 @@ export default class LoginItem extends HTMLElement { _internalSetMonitorData(internalMemberName, mapByLoginGUID) { this[internalMemberName] = mapByLoginGUID; - this.render(); + this.render({ onlyUpdateErrorsAndAlerts: true }); } _internalUpdateMonitorData(internalMemberName, mapByLoginGUID) { @@ -293,7 +298,11 @@ export default class LoginItem extends HTMLElement { this[internalMemberName] = new Map(); } for (const [guid, data] of [...mapByLoginGUID]) { - this[internalMemberName].set(guid, data); + if (data) { + this[internalMemberName].set(guid, data); + } else { + this[internalMemberName].delete(guid); + } } this._internalSetMonitorData(internalMemberName, this[internalMemberName]); } diff --git a/browser/components/aboutlogins/content/components/login-list-item.js b/browser/components/aboutlogins/content/components/login-list-item.js index e261376354a7..5a22d1a15646 100644 --- a/browser/components/aboutlogins/content/components/login-list-item.js +++ b/browser/components/aboutlogins/content/components/login-list-item.js @@ -77,6 +77,8 @@ export default class LoginListItemFactory { alertIcon, "about-logins-list-item-vulnerable-password-icon" ); + } else { + alertIcon.src = ""; } } } diff --git a/browser/components/aboutlogins/content/components/login-list.js b/browser/components/aboutlogins/content/components/login-list.js index e0115600311b..80c88a7394da 100644 --- a/browser/components/aboutlogins/content/components/login-list.js +++ b/browser/components/aboutlogins/content/components/login-list.js @@ -388,21 +388,26 @@ export default class LoginList extends HTMLElement { ); } - _internalSetMonitorData(internalMemberName, mapByLoginGUID) { + _internalSetMonitorData( + internalMemberName, + mapByLoginGUID, + updateSortAndSelectedLogin = true + ) { this[internalMemberName] = mapByLoginGUID; - if (this[internalMemberName].size === 0) { - this.render(); - return; + if (this[internalMemberName].size) { + for (let [loginGuid] of mapByLoginGUID) { + let { login, listItem } = this._logins[loginGuid]; + LoginListItemFactory.update(listItem, login); + } + if (updateSortAndSelectedLogin) { + const alertsSortOptionElement = this._sortSelect.namedItem("alerts"); + alertsSortOptionElement.hidden = false; + this._sortSelect.selectedIndex = alertsSortOptionElement.index; + this._applySortAndScrollToTop(); + this._selectFirstVisibleLogin(); + } } - for (let [loginGuid] of mapByLoginGUID) { - let { login, listItem } = this._logins[loginGuid]; - LoginListItemFactory.update(listItem, login); - } - const alertsSortOptionElement = this._sortSelect.namedItem("alerts"); - alertsSortOptionElement.hidden = false; - this._sortSelect.selectedIndex = alertsSortOptionElement.index; - this._applySortAndScrollToTop(); - this._selectFirstVisibleLogin(); + this.render(); } _internalUpdateMonitorData(internalMemberName, mapByLoginGUID) { @@ -410,9 +415,17 @@ export default class LoginList extends HTMLElement { this[internalMemberName] = new Map(); } for (const [guid, data] of [...mapByLoginGUID]) { - this[internalMemberName].set(guid, data); + if (data) { + this[internalMemberName].set(guid, data); + } else { + this[internalMemberName].delete(guid); + } } - this._internalSetMonitorData(internalMemberName, this[internalMemberName]); + this._internalSetMonitorData( + internalMemberName, + this[internalMemberName], + false + ); } setSortDirection(sortDirection) { diff --git a/browser/components/aboutlogins/tests/browser/browser.ini b/browser/components/aboutlogins/tests/browser/browser.ini index 09cb41a5340c..e1ebf2bf2fe1 100644 --- a/browser/components/aboutlogins/tests/browser/browser.ini +++ b/browser/components/aboutlogins/tests/browser/browser.ini @@ -8,6 +8,7 @@ prefs = # Skip ASAN and debug since waiting for content events is already slow. [browser_aaa_eventTelemetry_run_first.js] skip-if = asan || ccov || debug # bug 1605494 is more prevalent on linux +[browser_alertDismissedAfterChangingPassword.js] [browser_breachAlertShowingForAddedLogin.js] [browser_confirmDeleteDialog.js] [browser_contextmenuFillLogins.js] diff --git a/browser/components/aboutlogins/tests/browser/browser_alertDismissedAfterChangingPassword.js b/browser/components/aboutlogins/tests/browser/browser_alertDismissedAfterChangingPassword.js new file mode 100644 index 000000000000..9de36ddaeb68 --- /dev/null +++ b/browser/components/aboutlogins/tests/browser/browser_alertDismissedAfterChangingPassword.js @@ -0,0 +1,225 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +ChromeUtils.import("resource://testing-common/OSKeyStoreTestUtils.jsm", this); + +EXPECTED_BREACH = { + AddedDate: "2018-12-20T23:56:26Z", + BreachDate: "2018-12-16", + Domain: "breached.example.com", + Name: "Breached", + PwnCount: 1643100, + DataClasses: ["Email addresses", "Usernames", "Passwords", "IP addresses"], + _status: "synced", + id: "047940fe-d2fd-4314-b636-b4a952ee0043", + last_modified: "1541615610052", + schema: "1541615609018", +}; + +let VULNERABLE_TEST_LOGIN2 = new nsLoginInfo( + "https://2.example.com", + "https://2.example.com", + null, + "user2", + "pass3", + "username", + "password" +); + +add_task(async function setup() { + TEST_LOGIN1 = await addLogin(TEST_LOGIN1); + VULNERABLE_TEST_LOGIN2 = await addLogin(VULNERABLE_TEST_LOGIN2); + TEST_LOGIN3 = await addLogin(TEST_LOGIN3); + await BrowserTestUtils.openNewForegroundTab({ + gBrowser, + url: "about:logins", + }); + registerCleanupFunction(() => { + BrowserTestUtils.removeTab(gBrowser.selectedTab); + Services.logins.removeAllLogins(); + }); +}); + +add_task(async function test_added_login_shows_breach_warning() { + let browser = gBrowser.selectedBrowser; + await SpecialPowers.spawn( + browser, + [[TEST_LOGIN1.guid, VULNERABLE_TEST_LOGIN2.guid, TEST_LOGIN3.guid]], + async ([regularLoginGuid, vulnerableLoginGuid, breachedLoginGuid]) => { + let loginList = Cu.waiveXrays( + content.document.querySelector("login-list") + ); + await ContentTaskUtils.waitForCondition( + () => loginList.shadowRoot.querySelectorAll(".login-list-item").length, + "Waiting for login-list to get populated" + ); + let { listItem: regularListItem } = loginList._logins[regularLoginGuid]; + let { listItem: vulnerableListItem } = loginList._logins[ + vulnerableLoginGuid + ]; + let { listItem: breachedListItem } = loginList._logins[breachedLoginGuid]; + await ContentTaskUtils.waitForCondition(() => { + return ( + !regularListItem.matches(".breached.vulnerable") && + vulnerableListItem.matches(".vulnerable") && + breachedListItem.matches(".breached") + ); + }, `waiting for the list items to get their classes updated: ${regularListItem.className} / ${vulnerableListItem.className} / ${breachedListItem.className}`); + ok( + !regularListItem.classList.contains("breached") && + !regularListItem.classList.contains("vulnerable"), + "regular login should not be marked breached or vulnerable: " + + regularLoginGuid.className + ); + ok( + !vulnerableListItem.classList.contains("breached") && + vulnerableListItem.classList.contains("vulnerable"), + "vulnerable login should be marked vulnerable: " + + vulnerableListItem.className + ); + ok( + breachedListItem.classList.contains("breached") && + !breachedListItem.classList.contains("vulnerable"), + "breached login should be marked breached: " + + breachedListItem.className + ); + + breachedListItem.click(); + let loginItem = Cu.waiveXrays( + content.document.querySelector("login-item") + ); + await ContentTaskUtils.waitForCondition(() => { + return loginItem._login && loginItem._login.guid == breachedLoginGuid; + }, "waiting for breached login to get selected"); + ok( + !ContentTaskUtils.is_hidden( + loginItem.shadowRoot.querySelector(".breach-alert") + ), + "the breach alert should be visible" + ); + } + ); + + if (!OSKeyStoreTestUtils.canTestOSKeyStoreLogin()) { + info( + "leaving test early since the remaining part of the test requires 'edit' mode which requires 'oskeystore' login" + ); + return; + } + + let reauthObserved = forceAuthTimeoutAndWaitForOSKeyStoreLogin({ + loginResult: true, + }); + // Change the password on the breached login and check that the + // login is no longer marked as breached. The vulnerable login + // should still be marked as vulnerable afterwards. + await SpecialPowers.spawn(browser, [], () => { + let loginItem = Cu.waiveXrays(content.document.querySelector("login-item")); + loginItem.shadowRoot.querySelector(".edit-button").click(); + }); + await reauthObserved; + await SpecialPowers.spawn( + browser, + [[TEST_LOGIN1.guid, VULNERABLE_TEST_LOGIN2.guid, TEST_LOGIN3.guid]], + async ([regularLoginGuid, vulnerableLoginGuid, breachedLoginGuid]) => { + let loginList = Cu.waiveXrays( + content.document.querySelector("login-list") + ); + let loginItem = Cu.waiveXrays( + content.document.querySelector("login-item") + ); + await ContentTaskUtils.waitForCondition( + () => loginItem.dataset.editing == "true", + "waiting for login-item to enter edit mode" + ); + let passwordInput = loginItem.shadowRoot.querySelector( + "input[type='password']" + ); + const CHANGED_PASSWORD_VALUE = "changedPassword"; + passwordInput.value = CHANGED_PASSWORD_VALUE; + let saveChangesButton = loginItem.shadowRoot.querySelector( + ".save-changes-button" + ); + saveChangesButton.click(); + + await ContentTaskUtils.waitForCondition(() => { + return ( + loginList._logins[breachedLoginGuid].login.password == + CHANGED_PASSWORD_VALUE + ); + }, "waiting for stored login to get updated"); + + ok( + ContentTaskUtils.is_hidden( + loginItem.shadowRoot.querySelector(".breach-alert") + ), + "the breach alert should be hidden now" + ); + + let { listItem: breachedListItem } = loginList._logins[breachedLoginGuid]; + let { listItem: vulnerableListItem } = loginList._logins[ + vulnerableLoginGuid + ]; + ok( + !breachedListItem.classList.contains("breached") && + !breachedListItem.classList.contains("vulnerable"), + "the originally breached login should no longer be marked as breached" + ); + ok( + !vulnerableListItem.classList.contains("breached") && + vulnerableListItem.classList.contains("vulnerable"), + "vulnerable login should still be marked vulnerable: " + + vulnerableListItem.className + ); + + // Change the password on the vulnerable login and check that the + // login is no longer marked as vulnerable. + vulnerableListItem.click(); + await ContentTaskUtils.waitForCondition(() => { + return loginItem._login && loginItem._login.guid == vulnerableLoginGuid; + }, "waiting for vulnerable login to get selected"); + ok( + !ContentTaskUtils.is_hidden( + loginItem.shadowRoot.querySelector(".vulnerable-alert") + ), + "the vulnerable alert should be visible" + ); + loginItem.shadowRoot.querySelector(".edit-button").click(); + await ContentTaskUtils.waitForCondition( + () => loginItem.dataset.editing == "true", + "waiting for login-item to enter edit mode" + ); + passwordInput = loginItem.shadowRoot.querySelector( + "input[type='password']" + ); + passwordInput.value = CHANGED_PASSWORD_VALUE; + saveChangesButton.click(); + + await ContentTaskUtils.waitForCondition(() => { + return ( + loginList._logins[vulnerableLoginGuid].login.password == + CHANGED_PASSWORD_VALUE + ); + }, "waiting for stored login to get updated"); + + ok( + ContentTaskUtils.is_hidden( + loginItem.shadowRoot.querySelector(".vulnerable-alert") + ), + "the vulnerable alert should be hidden now" + ); + is( + vulnerableListItem.querySelector(".alert-icon").src, + "", + ".alert-icon for the vulnerable list item should have its source removed" + ); + vulnerableListItem = loginList._logins[vulnerableLoginGuid].listItem; + ok( + !vulnerableListItem.classList.contains("breached") && + !vulnerableListItem.classList.contains("vulnerable"), + "vulnerable login should no longer be marked vulnerable: " + + vulnerableListItem.className + ); + } + ); +}); diff --git a/browser/components/aboutlogins/tests/browser/browser_createLogin.js b/browser/components/aboutlogins/tests/browser/browser_createLogin.js index 3e52f384dc1a..b4fca5135b60 100644 --- a/browser/components/aboutlogins/tests/browser/browser_createLogin.js +++ b/browser/components/aboutlogins/tests/browser/browser_createLogin.js @@ -189,6 +189,10 @@ add_task(async function test_create_login() { let usernameInput = loginItem.shadowRoot.querySelector( "input[name='username']" ); + await ContentTaskUtils.waitForCondition( + () => usernameInput.placeholder, + "waiting for placeholder to get set" + ); ok( usernameInput.placeholder, "there should be a placeholder on the username input when not in edit mode" @@ -247,9 +251,18 @@ add_task(async function test_create_login() { let loginList = Cu.waiveXrays( content.document.querySelector("login-list") ); - let login = Object.values(loginList._logins).find( - obj => obj.login.origin == aOriginTuple[1] - ).login; + let login; + await ContentTaskUtils.waitForCondition(() => { + login = Object.values(loginList._logins).find( + obj => obj.login.origin == aOriginTuple[1] + ).login; + info(`${login.origin} / ${login.username} / ${login.password}`); + return ( + login.origin == aOriginTuple[1] && + login.username == "testuser2" && + login.password == "testpass2" + ); + }, "waiting for the login to get updated"); is( login.origin, aOriginTuple[1], diff --git a/browser/components/aboutlogins/tests/browser/browser_openSite.js b/browser/components/aboutlogins/tests/browser/browser_openSite.js index 4f05bde29fe6..1d4cfa073713 100644 --- a/browser/components/aboutlogins/tests/browser/browser_openSite.js +++ b/browser/components/aboutlogins/tests/browser/browser_openSite.js @@ -85,6 +85,9 @@ add_task(async function test_launch_login_item() { BrowserTestUtils.removeTab(newTab); await SpecialPowers.spawn(browser, [], async () => { + await ContentTaskUtils.waitForCondition(() => { + return !content.document.querySelector("confirmation-dialog").hidden; + }, "waiting for confirmation-dialog to appear"); ok( !content.document.querySelector("confirmation-dialog").hidden, "discard-changes confirmation-dialog should be visible after logging in to a site with a modified login present in the form" diff --git a/browser/components/aboutlogins/tests/browser/head.js b/browser/components/aboutlogins/tests/browser/head.js index 18a19241455a..3c3292587ff8 100644 --- a/browser/components/aboutlogins/tests/browser/head.js +++ b/browser/components/aboutlogins/tests/browser/head.js @@ -65,10 +65,16 @@ async function addLogin(login) { Ci.nsIWritablePropertyBag2 ); matchData.setPropertyAsAUTF8String("guid", login.guid); - if (!Services.logins.searchLogins(matchData).length) { + + let logins = Services.logins.searchLogins(matchData); + if (!logins.length) { return; } - Services.logins.removeLogin(login); + // Use the login that was returned from searchLogins + // in case the initial login object was changed by the test code, + // since removeLogin makes sure that the login argument exactly + // matches the login that it will be removing. + Services.logins.removeLogin(logins[0]); }); return login; }