From 7f7faf74c3e3684fd6b3ead72eb517543b958bae Mon Sep 17 00:00:00 2001 From: Vincent Date: Tue, 27 Feb 2024 16:17:50 +0100 Subject: [PATCH] Only pass relevant email data to the client side --- .../(dashboard)/settings/EmailListing.tsx | 8 +++-- .../settings/SettingsPage.test.tsx | 17 ++++++--- .../user/(dashboard)/settings/View.tsx | 7 ++-- .../user/(dashboard)/settings/actions.ts | 4 +-- .../(authenticated)/user/settings/page.tsx | 18 +++++----- src/app/functions/server/getUserBreaches.ts | 4 +-- src/app/functions/server/sanitizeEmailRow.ts | 36 +++++++++++++++++++ src/db/tables/emailAddresses.js | 11 +----- src/utils/breaches.js | 2 +- src/utils/subscriberBreaches.ts | 3 ++ 10 files changed, 76 insertions(+), 34 deletions(-) create mode 100644 src/app/functions/server/sanitizeEmailRow.ts diff --git a/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/EmailListing.tsx b/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/EmailListing.tsx index 2f4b12c7808..7b26aabd8ea 100644 --- a/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/EmailListing.tsx +++ b/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/EmailListing.tsx @@ -9,16 +9,16 @@ import { useRef, useState } from "react"; import styles from "./EmailListing.module.scss"; import { useL10n } from "../../../../../../hooks/l10n"; import { onRemoveEmail } from "./actions"; -import { EmailRow } from "../../../../../../../db/tables/emailAddresses"; import { CheckIcon, DeleteIcon, ErrorIcon, } from "../../../../../../components/server/Icons"; import { useTelemetry } from "../../../../../../hooks/useTelemetry"; +import { SanitizedEmailAddressRow } from "../../../../../../functions/server/sanitizeEmailRow"; export const EmailListing = (props: { - email: EmailRow | string; + email: SanitizedEmailAddressRow | string; breachCount: number; }) => { const l10n = useL10n(); @@ -105,6 +105,8 @@ export const EmailListing = (props: { ); }; -function isSecondaryEmail(email: string | EmailRow): email is EmailRow { +function isSecondaryEmail( + email: string | SanitizedEmailAddressRow, +): email is SanitizedEmailAddressRow { return typeof email !== "string"; } diff --git a/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/SettingsPage.test.tsx b/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/SettingsPage.test.tsx index 7645c90de05..0c86e6644ce 100644 --- a/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/SettingsPage.test.tsx +++ b/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/SettingsPage.test.tsx @@ -7,11 +7,12 @@ import { render, screen } from "@testing-library/react"; import { axe } from "jest-axe"; import { Session } from "next-auth"; import { userEvent } from "@testing-library/user-event"; +import type { EmailAddressRow } from "knex/types/tables"; import { getOneL10nSync } from "../../../../../../functions/server/mockL10n"; import { TestComponentWrapper } from "../../../../../../../TestComponentWrapper"; -import { EmailRow } from "../../../../../../../db/tables/emailAddresses"; import { SerializedSubscriber } from "../../../../../../../next-auth"; import { onAddEmail, onRemoveEmail } from "./actions"; +import { sanitizeEmailRow } from "../../../../../../functions/server/sanitizeEmailRow"; const mockedSessionUpdate = jest.fn(); const mockedRecordTelemetry = jest.fn(); @@ -55,19 +56,25 @@ const mockedUser: Session["user"] = { twoFactorAuthentication: false, }, }; -const mockedSecondaryVerifiedEmail: EmailRow = { +const mockedSecondaryVerifiedEmail: EmailAddressRow = { id: 1337, email: "secondary_verified@example.com", sha1: "arbitrary string", subscriber_id: subscriberId, verified: true, + created_at: new Date("1337-04-02T04:02:42.000Z"), + updated_at: new Date("1337-04-02T04:02:42.000Z"), + verification_token: "arbitrary_token", }; -const mockedSecondaryUnverifiedEmail: EmailRow = { +const mockedSecondaryUnverifiedEmail: EmailAddressRow = { id: 1337, email: "secondary_unverified@example.com", sha1: "arbitrary string", subscriber_id: subscriberId, verified: false, + created_at: new Date("1337-04-02T04:02:42.000Z"), + updated_at: new Date("1337-04-02T04:02:42.000Z"), + verification_token: "arbitrary_token", }; const mockedSubscriptionBillingAmount = { yearly: 13.37, @@ -361,7 +368,9 @@ it("calls the 'remove' action when clicking the rubbish bin icon", async () => { const removeButtons = screen.getAllByRole("button", { name: "Remove" }); await user.click(removeButtons[0]); - expect(onRemoveEmail).toHaveBeenCalledWith(mockedSecondaryVerifiedEmail); + expect(onRemoveEmail).toHaveBeenCalledWith( + sanitizeEmailRow(mockedSecondaryVerifiedEmail), + ); }); it("hides the Plus cancellation link if the user doesn't have Plus", () => { diff --git a/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/View.tsx b/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/View.tsx index 4c37e2f61c8..c52f0bd8407 100644 --- a/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/View.tsx +++ b/src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/View.tsx @@ -3,10 +3,10 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { Session } from "next-auth"; +import { EmailAddressRow } from "knex/types/tables"; import styles from "./View.module.scss"; import { Toolbar } from "../../../../../../components/client/toolbar/Toolbar"; import { ExtendedReactLocalization } from "../../../../../../hooks/l10n"; -import { EmailRow } from "../../../../../../../db/tables/emailAddresses"; import { OpenInNew } from "../../../../../../components/server/Icons"; import { EmailListing } from "./EmailListing"; import { EmailAddressAdder } from "./EmailAddressAdder"; @@ -14,6 +14,7 @@ import { AlertAddressForm } from "./AlertAddressForm"; import { CONST_MAX_NUM_ADDRESSES } from "../../../../../../../constants"; import { TelemetryLink } from "../../../../../../components/client/TelemetryLink"; import { hasPremium } from "../../../../../../functions/universal/user"; +import { sanitizeEmailRow } from "../../../../../../functions/server/sanitizeEmailRow"; export type Props = { l10n: ExtendedReactLocalization; @@ -26,7 +27,7 @@ export type Props = { }; fxaSettingsUrl: string; fxaSubscriptionsUrl: string; - emailAddresses: EmailRow[]; + emailAddresses: EmailAddressRow[]; breachCountByEmailAddress: Record; }; @@ -66,7 +67,7 @@ export const SettingsView = (props: Props) => { return (
  • { +const emailNeedsVerificationSub = (email: EmailAddressRow) => { const l10n = getL10n(); return ( @@ -38,7 +36,7 @@ const emailNeedsVerificationSub = (email: EmailRow) => { ); }; -const deleteButton = (email: EmailRow) => { +const deleteButton = (email: EmailAddressRow) => { const l10n = getL10n(); return ( @@ -57,7 +55,7 @@ const deleteButton = (email: EmailRow) => { }; const createEmailItem = ( - email: EmailRow & { primary?: boolean }, + email: EmailAddressRow & { primary?: boolean }, breachCounts: Map, ) => { const l10n = getL10n(); @@ -82,7 +80,9 @@ const createEmailItem = ( }; // Moves the primary email to the front and sorts the rest alphabeticaly. -const getSortedEmails = (emails: Array) => +const getSortedEmails = ( + emails: Array, +) => [...emails].sort((a, b) => { if (a.primary) { return -1; @@ -96,7 +96,7 @@ const getSortedEmails = (emails: Array) => }); const createEmailList = ( - emails: EmailRow[], + emails: EmailAddressRow[], breachCounts: Map, ) => { return ( diff --git a/src/app/functions/server/getUserBreaches.ts b/src/app/functions/server/getUserBreaches.ts index 41a586bc202..3995bc3bf48 100644 --- a/src/app/functions/server/getUserBreaches.ts +++ b/src/app/functions/server/getUserBreaches.ts @@ -4,6 +4,7 @@ import { cookies } from "next/headers"; import { Session } from "next-auth"; +import { EmailAddressRow } from "knex/types/tables"; import { getBreaches } from "./getBreaches"; import { appendBreachResolutionChecklist } from "./breachResolution"; @@ -17,7 +18,6 @@ import { SubscriberBreach, getSubBreaches, } from "../../../utils/subscriberBreaches"; -import { EmailRow } from "../../../db/tables/emailAddresses"; import { HibpLikeDbBreach } from "../../../utils/hibp"; //TODO: deprecate with MNTOR-2021 @@ -29,7 +29,7 @@ export type UserBreaches = { emailTotalCount: number; emailSelectIndex: number; breachesData: { - unverifiedEmails: EmailRow[]; + unverifiedEmails: EmailAddressRow[]; verifiedEmails: BundledVerifiedEmails[]; }; }; diff --git a/src/app/functions/server/sanitizeEmailRow.ts b/src/app/functions/server/sanitizeEmailRow.ts new file mode 100644 index 00000000000..de05c87c51d --- /dev/null +++ b/src/app/functions/server/sanitizeEmailRow.ts @@ -0,0 +1,36 @@ +/* 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 "server-only"; +import { EmailAddressRow } from "knex/types/tables"; + +export type SanitizedEmailAddressRow = Pick< + EmailAddressRow, + "email" | "id" | "subscriber_id" | "verified" | "created_at" | "updated_at" +> & { + /** + * The `__s` property is added to make sure this type is structurally + * distinct from `EmailAddressRow`, i.e. so that you're unable to pass in + * the latter where a `SanitizedEmailAddressRow` is expected: + */ + __s: true; +}; + +export function sanitizeEmailRow( + email: EmailAddressRow, +): SanitizedEmailAddressRow { + return { + email: email.email, + id: email.id, + subscriber_id: email.subscriber_id, + created_at: email.created_at, + updated_at: email.updated_at, + verified: email.verified, + // If we want to avoid passing this property to the client-side, we can also + // just assert this object as a `SanitizedEmailAddressRow`, but I didn't + // want to encourage asserting that elsewhere _without_ passing it through + // this function, so I left it in for now: + __s: true, + }; +} diff --git a/src/db/tables/emailAddresses.js b/src/db/tables/emailAddresses.js index 833703d0b3e..848163a6541 100644 --- a/src/db/tables/emailAddresses.js +++ b/src/db/tables/emailAddresses.js @@ -285,18 +285,9 @@ async function _verifyNewEmail (emailHash) { } /* c8 ignore stop */ -/** - * @typedef {object} EmailRow Email data, as returned from the database table `email_addresses` - * @property {number} id - * @property {string} email - * @property {string} sha1 - * @property {boolean} verified - * @property {number} subscriber_id - */ - /** * @param {number} userId - * @returns {Promise} + * @returns {Promise} */ // Not covered by tests; mostly side-effects. See test-coverage.md#mock-heavy /* c8 ignore start */ diff --git a/src/utils/breaches.js b/src/utils/breaches.js index 6a7e3b85969..3d85fcaa357 100644 --- a/src/utils/breaches.js +++ b/src/utils/breaches.js @@ -10,7 +10,7 @@ import { captureMessage } from "@sentry/node"; /** * @typedef {{ - * unverifiedEmails: import('../db/tables/emailAddresses.js').EmailRow[], + * unverifiedEmails: import('knex/types/tables').EmailAddressRow[], * verifiedEmails: BundledVerifiedEmails[], * }} AllEmailsAndBreaches */ diff --git a/src/utils/subscriberBreaches.ts b/src/utils/subscriberBreaches.ts index c3060d815b5..8ef4adc1d9e 100644 --- a/src/utils/subscriberBreaches.ts +++ b/src/utils/subscriberBreaches.ts @@ -71,6 +71,9 @@ export async function getSubBreaches( email: subscriber.primary_email, verified: subscriber.primary_verified, sha1: subscriber.primary_sha1, + verification_token: subscriber.primary_verification_token, + created_at: subscriber.created_at, + updated_at: subscriber.updated_at, }); verifiedEmails = verifiedEmails.filter((e) => e.verified);