-
Notifications
You must be signed in to change notification settings - Fork 224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add links to company websites for breach resolution #2961
Changes from all commits
fdac319
b075643
8944a11
e930adb
58f3d2a
f5d2da1
1b25b2c
aa00a5a
47c7c51
838e01e
25d4ff7
c0a14d0
2d1f4ca
5a26bbb
5b539ab
3f7097b
2724dca
d88ab70
91dfced
32f95cf
54ce192
95f2f3e
d574bee
c1eb5ec
e05ea1a
94152f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
* License, v. 2.0. If a copy of the MPL was not distributed with this | ||
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
||
import AppConstants from '../appConstants.js' | ||
import { getMessage } from './fluent.js' | ||
|
||
/** | ||
|
@@ -34,8 +35,8 @@ const BreachDataTypes = { | |
const breachResolutionDataTypes = { | ||
[BreachDataTypes.Passwords]: { | ||
priority: 1, | ||
header: 'breach-checklist-pw-header-2', | ||
body: 'breach-checklist-pw-body-2' | ||
flozia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
header: 'breach-checklist-pw-header-text', | ||
body: 'breach-checklist-pw-body-text' | ||
}, | ||
[BreachDataTypes.Email]: { | ||
priority: 2, | ||
|
@@ -85,8 +86,8 @@ const breachResolutionDataTypes = { | |
}, | ||
[BreachDataTypes.SecurityQuestions]: { | ||
priority: 11, | ||
header: 'breach-checklist-sq-header-2', | ||
body: 'breach-checklist-sq-body' | ||
header: 'breach-checklist-sq-header-text', | ||
body: 'breach-checklist-sq-body-text' | ||
}, | ||
[BreachDataTypes.HistoricalPasswords]: { | ||
priority: 12, | ||
|
@@ -109,19 +110,24 @@ const breachResolutionDataTypes = { | |
*/ | ||
function appendBreachResolutionChecklist (userBreachData, options = {}) { | ||
const { verifiedEmails } = userBreachData | ||
|
||
for (const { breaches } of verifiedEmails) { | ||
breaches.forEach(b => { | ||
const dataClasses = b.DataClasses | ||
const blockList = (AppConstants.HIBP_BREACH_DOMAIN_BLOCKLIST ?? '').split(',') | ||
const showLink = b.Domain && !blockList.includes(b.Domain) | ||
|
||
const args = { | ||
companyName: b.Name, | ||
breachedCompanyLink: showLink ? `https://${b.Domain}` : '', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aside: in my personal testing, I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, thanks for the note. With us trying to be cautious where we link out to I think I’d feel more comfortable linking out to |
||
firefoxRelayLink: `<a href="https://relay.firefox.com/?utm_medium=mozilla-websites&utm_source=monitor&utm_campaign=&utm_content=breach-resolution" target="_blank">${getMessage('breach-checklist-link-firefox-relay')}</a>`, | ||
passwordManagerLink: `<a href="https://www.mozilla.org/firefox/features/password-manager/?utm_medium=mozilla-websites&utm_source=monitor&utm_campaign=&utm_content=breach-resolution" target="_blank">${getMessage('breach-checklist-link-password-manager')}</a>`, | ||
mozillaVpnLink: `<a href="https://www.mozilla.org/products/vpn/?utm_medium=mozilla-websites&utm_source=monitor&utm_campaign=&utm_content=breach-resolution" target="_blank">${getMessage('breach-checklist-link-mozilla-vpn')}</a>`, | ||
equifaxLink: '<a href="https://www.equifax.com/personal/credit-report-services/credit-freeze/" target="_blank">Equifax</a>', | ||
experianLink: '<a href="https://www.experian.com/freeze/center.html" target="_blank">Experian</a>', | ||
transUnionLink: '<a href="https://www.transunion.com/credit-freeze" target="_blank">TransUnion</a>' | ||
} | ||
b.breachChecklist = getResolutionRecsPerBreach(dataClasses, args, { ...options, hideBreachLink: b.Domain.length <= 0 }) | ||
b.breachChecklist = getResolutionRecsPerBreach(dataClasses, args, options) | ||
}) | ||
} | ||
} | ||
|
@@ -134,39 +140,71 @@ function appendBreachResolutionChecklist (userBreachData, options = {}) { | |
* @param {object} args contains necessary variables for the fluent file | ||
* - companyName | ||
* - breachedCompanyUrl | ||
* @param {Partial<{ countryCode: string, hideBreachLink: boolean }>} options | ||
* @param {Partial<{ countryCode: string }>} options | ||
* @returns {Map} map of relevant breach resolution recommendations | ||
*/ | ||
function getResolutionRecsPerBreach (dataTypes, args, options = {}) { | ||
const filteredBreachRecs = {} | ||
|
||
// filter breachResolutionDataTypes based on relevant data types passed in | ||
for (const [key, value] of Object.entries(breachResolutionDataTypes)) { | ||
for (const resolution of Object.entries(breachResolutionDataTypes)) { | ||
const [key, value] = resolution | ||
if ( | ||
dataTypes.includes(key) && | ||
// Hide the security question or password resolution if we can't link to the breached site: | ||
flozia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
(![BreachDataTypes.Passwords, BreachDataTypes.SecurityQuestions].includes(key) || !options.hideBreachLink) && | ||
// Hide resolutions that apply to other countries than the user's: | ||
(!options.countryCode || !Array.isArray(value.applicableCountryCodes) || value.applicableCountryCodes.includes(options.countryCode.toLowerCase())) | ||
) { | ||
filteredBreachRecs[key] = getRecommendationFromResolution(value, args) | ||
filteredBreachRecs[key] = getRecommendationFromResolution(resolution, args) | ||
} | ||
} | ||
|
||
// If we did not have any recommendations, add a generic recommendation: | ||
if (Object.keys(filteredBreachRecs).length === 0) { | ||
filteredBreachRecs[BreachDataTypes.General] = getRecommendationFromResolution(breachResolutionDataTypes[BreachDataTypes.General], args) | ||
const resolutionTypeGeneral = BreachDataTypes.General | ||
filteredBreachRecs[resolutionTypeGeneral] = getRecommendationFromResolution( | ||
[ | ||
resolutionTypeGeneral, | ||
breachResolutionDataTypes[resolutionTypeGeneral] | ||
], | ||
args | ||
) | ||
} | ||
|
||
// loop through the breach recs | ||
return filteredBreachRecs | ||
} | ||
|
||
/** | ||
* Get the fluent string for the body | ||
* | ||
* @param {string} body for the fluent body string | ||
* @param {object} args | ||
* @returns {string} body string | ||
*/ | ||
function getBodyMessage (body, args) { | ||
const { stringArgs } = args | ||
|
||
const companyLink = stringArgs.breachedCompanyLink | ||
return getMessage(body, stringArgs) | ||
.replace( | ||
'<breached-company-link>', | ||
companyLink ? `<a href="${companyLink}" target="_blank">` : '' | ||
) | ||
.replace( | ||
'</breached-company-link>', | ||
companyLink ? '</a>' : '' | ||
) | ||
} | ||
|
||
// find fluent text based on fluent ids | ||
function getRecommendationFromResolution (resolution, args) { | ||
let { header, body, priority } = resolution | ||
const [resolutionType, resolutionContent] = resolution | ||
let { header, body, priority } = resolutionContent | ||
|
||
header = header ? getMessage(header, args) : '' | ||
body = body ? getMessage(body, args) : '' | ||
body = body | ||
? getBodyMessage(body, { resolutionType, stringArgs: args }) | ||
: '' | ||
return { header, body, priority } | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,9 @@ test('appendBreachResolutionChecklist: two data classes', t => { | |
appendBreachResolutionChecklist(userBreachData) | ||
t.truthy(userBreachData.verifiedEmails[0].breaches[0].breachChecklist) | ||
t.is(userBreachData.verifiedEmails[0].breaches[0].breachChecklist[BreachDataTypes.Passwords].header, | ||
'Go to the company’s website to change your password and enable two-factor authentication (2FA).') | ||
'Update your passwords and enable two-factor authentication (2FA).') | ||
t.is(userBreachData.verifiedEmails[0].breaches[0].breachChecklist[BreachDataTypes.Passwords].body, | ||
'In most cases, we’d recommend that you change your password on the company’s website. But <b>their website may be down or contain malicious content</b>, so use caution if you <a href="https://companyName.com" target="_blank">visit the site</a>. For added protection, make sure you’re using unique passwords for all accounts, so that any leaked passwords can’t be used to access other accounts. <a href="https://www.mozilla.org/firefox/features/password-manager/?utm_medium=mozilla-websites&utm_source=monitor&utm_campaign=&utm_content=breach-resolution" target="_blank">Firefox Password Manager</a> can help you securely keep track of all of your passwords.') | ||
t.is(userBreachData.verifiedEmails[0].breaches[0].breachChecklist[BreachDataTypes.Passwords].priority, 1) | ||
}) | ||
|
||
|
@@ -147,13 +149,22 @@ test('appendBreachResolutionChecklist: data class with a resolution referring to | |
unverifiedEmails: [] | ||
} | ||
appendBreachResolutionChecklist(userBreachData) | ||
// There should only be a resolution for `BreachDataTypes.Phone`, as | ||
// `BreachDataTypes.Passwords` and `BreachDataTypes.SecurityQuestions` refer | ||
// to the breached company's domain, which we don't know: | ||
// There should be a resolution for `BreachDataTypes.Phone`, | ||
// `BreachDataTypes.Passwords` and `BreachDataTypes.SecurityQuestions`. | ||
// The last two should fallback to a more generic header string that does not | ||
// include the breached company's domain, which we don't know: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know how easy it is to mock AppConstants, but if it's easy (but only then - you've been working on this PR for long enough), maybe a test for the blocklist would be a good addition? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point! I create an issue for this and will address this in a follow-up in order to not block this PR. |
||
t.deepEqual( | ||
Object.keys(userBreachData.verifiedEmails[0].breaches[0].breachChecklist), | ||
[BreachDataTypes.Phone] | ||
[BreachDataTypes.Passwords, BreachDataTypes.Phone, BreachDataTypes.SecurityQuestions] | ||
) | ||
t.is(userBreachData.verifiedEmails[0].breaches[0].breachChecklist[BreachDataTypes.Passwords].header, | ||
'Update your passwords and enable two-factor authentication (2FA).') | ||
t.is(userBreachData.verifiedEmails[0].breaches[0].breachChecklist[BreachDataTypes.Passwords].body, | ||
'In most cases, we’d recommend that you change your password on the company’s website. But <b>their website may be down or contain malicious content</b>, so use caution if you visit the site. For added protection, make sure you’re using unique passwords for all accounts, so that any leaked passwords can’t be used to access other accounts. <a href="https://www.mozilla.org/firefox/features/password-manager/?utm_medium=mozilla-websites&utm_source=monitor&utm_campaign=&utm_content=breach-resolution" target="_blank">Firefox Password Manager</a> can help you securely keep track of all of your passwords.') | ||
t.is(userBreachData.verifiedEmails[0].breaches[0].breachChecklist[BreachDataTypes.SecurityQuestions].header, | ||
'Update your security questions.') | ||
t.is(userBreachData.verifiedEmails[0].breaches[0].breachChecklist[BreachDataTypes.SecurityQuestions].body, | ||
'In most cases, we’d recommend that you update your security questions on the company’s website. But <b>their website may be down or contain malicious content</b>, so use caution if you visit the site. For added protection, update these security questions on any important accounts where you’ve used them, and create unique passwords for all accounts.') | ||
}) | ||
|
||
test('appendBreachResolutionChecklist: data class with a resolution referring to the breach\'s domain, which is available', t => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a limit how long an ENV var can be?
It feels like this could be REALLY long if we end up blocking over 20-30 domains.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to SRE, there is no limit that we would hit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it's better as an ENV var that has to be coordinated w/ SRE, versus maybe some JSON file that lives in the repo that is a single source of truth that we can audit occasionally. (unless there are reasons to keep it as a secret/ENV). 🤷
Although I guess I could technically recreate the blocklist locally by scraping the 650 breaches on the Monitor site and see if the outbound link is a link or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m still not sure about that as well, but one good argument for handling the list in the env is that we would be able to make adjustments without a release. Especially in the beginning, when we might need to test and audit the sites manually.