Skip to content

Commit

Permalink
Bug 1592467 - Hide the breach alert and vulnerable alert if the passw…
Browse files Browse the repository at this point in the history
…ord is changed. r=sfoster

Differential Revision: https://phabricator.services.mozilla.com/D68241
  • Loading branch information
msujaws committed Mar 28, 2020
1 parent b381bbd commit a0ef643
Show file tree
Hide file tree
Showing 10 changed files with 320 additions and 23 deletions.
24 changes: 24 additions & 0 deletions browser/components/aboutlogins/AboutLoginsParent.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
15 changes: 12 additions & 3 deletions browser/components/aboutlogins/content/components/login-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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 || "",
});
Expand Down Expand Up @@ -285,15 +290,19 @@ export default class LoginItem extends HTMLElement {

_internalSetMonitorData(internalMemberName, mapByLoginGUID) {
this[internalMemberName] = mapByLoginGUID;
this.render();
this.render({ onlyUpdateErrorsAndAlerts: true });
}

_internalUpdateMonitorData(internalMemberName, mapByLoginGUID) {
if (!this[internalMemberName]) {
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]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ export default class LoginListItemFactory {
alertIcon,
"about-logins-list-item-vulnerable-password-icon"
);
} else {
alertIcon.src = "";
}
}
}
43 changes: 28 additions & 15 deletions browser/components/aboutlogins/content/components/login-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,31 +388,44 @@ 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;
}
for (let [loginGuid] of mapByLoginGUID) {
let { login, listItem } = this._logins[loginGuid];
LoginListItemFactory.update(listItem, login);
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();
}
}
const alertsSortOptionElement = this._sortSelect.namedItem("alerts");
alertsSortOptionElement.hidden = false;
this._sortSelect.selectedIndex = alertsSortOptionElement.index;
this._applySortAndScrollToTop();
this._selectFirstVisibleLogin();
this.render();
}

_internalUpdateMonitorData(internalMemberName, mapByLoginGUID) {
if (!this[internalMemberName]) {
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) {
Expand Down
1 change: 1 addition & 0 deletions browser/components/aboutlogins/tests/browser/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
@@ -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
);
}
);
});
Loading

0 comments on commit a0ef643

Please sign in to comment.