Skip to content
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

MNTOR-3435 - Convert Breaches.js to Typescript #4876

Merged
merged 11 commits into from
Aug 5, 2024
Merged
4 changes: 1 addition & 3 deletions src/app/functions/server/breachResolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import { getL10n } from "../l10n/serverComponents";
import AppConstants from "../../../appConstants.js";
import { BreachDataTypes } from "../universal/breach";
import { AllEmailsAndBreaches } from "../../../utils/breaches.js";
import { AllEmailsAndBreaches } from "../../../utils/breaches";

/**
* TODO: Map from google doc: https://docs.google.com/document/d/1KoItFsTYVIBInIG2YmA7wSxkKS4vti_X0A0td_yaHVM/edit#
Expand Down Expand Up @@ -99,8 +99,6 @@ function appendBreachResolutionChecklist(
const { verifiedEmails } = userBreachData;

for (const { breaches } of verifiedEmails) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Vinnl I had to rebase this code, could you check that it did not affect the changes in the hibp.js typescript migration?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added ec002a1 to restore a couple of changes that were resolved the wrong way around.

FWIW, if I have a merge conflict after rebasing from main, I simply look at the code that is in main - if the conflict is different from main and is not a change that I intentionally made as part of the PR, I restore it back to what's currently in main.

// Old untyped code, adding type defitions now isn't worth the effort:
/* eslint-disable @typescript-eslint/no-explicit-any */
breaches.forEach((b) => {
const dataClasses = b.DataClasses;
const blockList = (AppConstants.HIBP_BREACH_DOMAIN_BLOCKLIST ?? "").split(
Expand Down
2 changes: 1 addition & 1 deletion src/app/functions/server/getBreaches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export async function getBreaches(): Promise<HibpLikeDbBreach[]> {
return breaches;
}
breaches = await getAllBreachesFromDb();
logger.debug("loaded breaches from database", {
logger.debug("loaded_breaches_from_database", {
breachesLength: breaches.length,
});

Expand Down
2 changes: 1 addition & 1 deletion src/knex-tables.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ declare module "knex/types/tables" {
fxa_uid: null | string;
// TODO: Find unknown type
// eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents
breaches_last_shown: null | unknown;
breaches_last_shown: Date;
// NOTE: this field is inherited from an older version of the product, it only applies to instant alerts
all_emails_to_primary: boolean | null; // added null in MNTOR-1368
// TODO: Find unknown type
Expand Down
160 changes: 0 additions & 160 deletions src/utils/breaches.js

This file was deleted.

173 changes: 173 additions & 0 deletions src/utils/breaches.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* 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 { getUserEmails } from "../db/tables/emailAddresses.js";
import {
getBreachesForEmail,
getFilteredBreaches,
HibpLikeDbBreach,
} from "./hibp";
import { getSha1 } from "./fxa";
import { captureMessage } from "@sentry/node";
import { EmailAddressRow, SubscriberRow } from "knex/types/tables";

export type BundledVerifiedEmails = {
email: string;
breaches: HibpLikeDbBreach[];
id: number;
primary: boolean;
verified: boolean;
hasNewBreaches?: number;
};

export type AllEmailsAndBreaches = {
unverifiedEmails: EmailAddressRow[];
verifiedEmails: BundledVerifiedEmails[];
};

type userType =
| ({
email_addresses: Array<{
id: EmailAddressRow["id"];
email: EmailAddressRow["email"];
}>;
} & SubscriberRow)
| undefined;

async function getAllEmailsAndBreaches(
user: userType,
allBreaches: HibpLikeDbBreach[],
): Promise<AllEmailsAndBreaches> {
// @ts-ignore: function will be deprecated
const verifiedEmails: BundledVerifiedEmails[] = [];
// @ts-ignore: function will be deprecated
const unverifiedEmails: EmailAddressRow[] = [];

if (!user) {
const errMsg = "getAllEmailsAndBreaches: subscriber cannot be undefined";
console.error(errMsg);
captureMessage(errMsg);

// @ts-ignore: function will be deprecated
return { verifiedEmails, unverifiedEmails };
}
if (!allBreaches || allBreaches.length === 0) {
const errMsg =
"getAllEmailsAndBreaches: allBreaches object cannot be empty";
console.error(errMsg);
captureMessage(errMsg);
// @ts-ignore: function will be deprecated
return { verifiedEmails, unverifiedEmails };
}

const monitoredEmails = await getUserEmails(user.id);
verifiedEmails.push(
await bundleVerifiedEmails({
user,
email: user.primary_email,
recordId: user.id,
recordVerified: user.primary_verified,
allBreaches,
}),
);
for (const email of monitoredEmails) {
if (email.verified) {
verifiedEmails.push(
await bundleVerifiedEmails({
user,
email: user.primary_email,
recordId: email.id,
recordVerified: email.verified,
allBreaches,
}),
);
} else {
unverifiedEmails.push(email);
}
}

// get new breaches since last shown
for (const emailEntry of verifiedEmails) {
// /** @type {any[]} */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this comment be removed now?

const newBreachesForEmail = emailEntry.breaches.filter(
(breach) => breach.AddedDate >= user.breaches_last_shown,
);

for (const newBreachForEmail of newBreachesForEmail) {
newBreachForEmail.NewBreach = true; // add "NewBreach" property to the new breach.
emailEntry.hasNewBreaches = newBreachesForEmail.length; // add the number of new breaches to the email
}
}

return { verifiedEmails, unverifiedEmails };
}

function addRecencyIndex(foundBreaches: HibpLikeDbBreach[]) {
// /**
// * @type {any[]}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question re: this type definition - we were using these JSDoc type annotations to get a little type checking for JS files before we fully switched to typescript but we shouldn't need them anymore.

// */
const annotatedBreaches: HibpLikeDbBreach[] = [];
// slice() the array to make a copy so before reversing so we don't
// reverse foundBreaches in-place
const oldestToNewestFoundBreaches = foundBreaches.slice().reverse();
oldestToNewestFoundBreaches.forEach((annotatingBreach, index) => {
const foundBreach = foundBreaches.find(
(foundBreach) => foundBreach.Name === annotatingBreach.Name,
);
annotatedBreaches.push(Object.assign({ recencyIndex: index }, foundBreach));
});
return annotatedBreaches.reverse();
}

type options = {
user: userType;
email: string;
recordId: number;
recordVerified: boolean;
allBreaches: HibpLikeDbBreach[];
};
async function bundleVerifiedEmails(
options: options,
): Promise<BundledVerifiedEmails> {
const { user, email, recordId, recordVerified, allBreaches } = options;
const lowerCaseEmailSha = getSha1(email.toLowerCase());

// find all breaches relevant to the current email
const foundBreaches = await getBreachesForEmail(
lowerCaseEmailSha,
allBreaches,
true,
false,
);

// TODO: remove after migration MNTOR-978
// adding index to breaches based on recency
const foundBreachesWithRecency = addRecencyIndex(foundBreaches);

if (!user) {
const errMsg = "breachResolutionV2: subscriber cannot be undefined";
console.error(errMsg);
captureMessage(errMsg);

// @ts-ignore: function will be deprecated
Copy link
Collaborator

@rhelmer rhelmer Aug 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you put @deprecated in a comment above the function definition then VSCode and other tooling will pick that up.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads-up that that needs to be in a JSDoc comment, i.e. one that starts with /** rather than just /* or //.

return { verifiedEmails, unverifiedEmails };
}

// filter out irrelevant breaches based on HIBP
const filteredAnnotatedFoundBreaches = getFilteredBreaches(
foundBreachesWithRecency,
);

const emailEntry: BundledVerifiedEmails = {
email: email,
breaches: filteredAnnotatedFoundBreaches,
primary: email === user.primary_email,
id: recordId,
verified: recordVerified,
};

return emailEntry;
}

export { getAllEmailsAndBreaches };
1 change: 1 addition & 0 deletions src/utils/hibp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ export type HibpLikeDbBreach = {
IsSpamList: BreachRow["is_spam_list"];
IsMalware: BreachRow["is_malware"];
FaviconUrl?: BreachRow["favicon_url"];
NewBreach?: boolean;
};

/**
Expand Down
Loading