Bug 1592467 - Hide the breach alert and vulnerable alert if the password is changed. r=sfoster

Differential Revision: https://phabricator.services.mozilla.com/D68241 --HG-- extra : moz-landing-system : lando
2020-03-28 02:13:12 +00:00 · 2020-03-28 02:13:12 +00:00 · d3e81eca6e
--- 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;
      }
--- 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);
  }

--- 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]);
  }
--- 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 = "";
    }
  }
 }
--- 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) {
--- 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]
--- a/browser/components/aboutlogins/tests/browser/browser_alertDismissedAfterChangingPassword.js
+++ 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
+      );
+    }
+  );
+});
--- 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],
--- 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"
--- 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;
 }