Skip to content

Commit

Permalink
Only pass relevant email data to the client side
Browse files Browse the repository at this point in the history
  • Loading branch information
Vinnl committed Mar 6, 2024
1 parent 0f78e37 commit 7f7faf7
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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";
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,18 @@
* 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";
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;
Expand All @@ -26,7 +27,7 @@ export type Props = {
};
fxaSettingsUrl: string;
fxaSubscriptionsUrl: string;
emailAddresses: EmailRow[];
emailAddresses: EmailAddressRow[];
breachCountByEmailAddress: Record<string, number>;
};

Expand Down Expand Up @@ -66,7 +67,7 @@ export const SettingsView = (props: Props) => {
return (
<li key={emailAddress.email}>
<EmailListing
email={emailAddress}
email={sanitizeEmailRow(emailAddress)}
breachCount={
props.breachCountByEmailAddress[emailAddress.email]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { revalidatePath } from "next/cache";
import { SubscriberRow } from "knex/types/tables";
import { getServerSession } from "../../../../../../functions/server/getServerSession";
import {
EmailRow,
addSubscriberUnverifiedEmailHash,
removeOneSecondaryEmail,
} from "../../../../../../../db/tables/emailAddresses";
Expand All @@ -22,6 +21,7 @@ import { sendVerificationEmail } from "../../../../../../api/utils/email";
import { getL10n } from "../../../../../../functions/server/l10n";
import { logger } from "../../../../../../functions/server/logging";
import { CONST_MAX_NUM_ADDRESSES } from "../../../../../../../constants";
import { SanitizedEmailAddressRow } from "../../../../../../functions/server/sanitizeEmailRow";

export type AddEmailFormState =
| { success?: never }
Expand Down Expand Up @@ -128,7 +128,7 @@ export async function onAddEmail(
}
}

export async function onRemoveEmail(email: EmailRow) {
export async function onRemoveEmail(email: SanitizedEmailAddressRow) {
const l10n = getL10n();
const session = await getServerSession();
if (!session?.user.subscriber?.fxa_uid) {
Expand Down
18 changes: 9 additions & 9 deletions src/app/deprecated/(authenticated)/user/settings/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@
import { redirect } from "next/navigation";
import Image from "next/image";
import Script from "next/script";
import { EmailAddressRow } from "knex/types/tables";
import AppConstants from "../../../../../appConstants";
import { getL10n } from "../../../../functions/server/l10n";
import ImageIconDelete from "../../../../../client/images/icon-delete.svg";
import "../../../../../client/css/partials/settings.css";
import React from "react";
import {
EmailRow,
getUserEmails,
} from "../../../../../db/tables/emailAddresses";
import { getUserEmails } from "../../../../../db/tables/emailAddresses";
import { getBreaches } from "../../../../functions/server/getBreaches";
import { getBreachesForEmail } from "../../../../../utils/hibp";
import { getSha1 } from "../../../../../utils/fxa";
Expand All @@ -22,7 +20,7 @@ import { getNonce } from "../../../functions/server/getNonce";
import { CONST_MAX_NUM_ADDRESSES } from "../../../../../constants";
import { getServerSession } from "../../../../functions/server/getServerSession";

const emailNeedsVerificationSub = (email: EmailRow) => {
const emailNeedsVerificationSub = (email: EmailAddressRow) => {
const l10n = getL10n();

return (
Expand All @@ -38,7 +36,7 @@ const emailNeedsVerificationSub = (email: EmailRow) => {
);
};

const deleteButton = (email: EmailRow) => {
const deleteButton = (email: EmailAddressRow) => {
const l10n = getL10n();

return (
Expand All @@ -57,7 +55,7 @@ const deleteButton = (email: EmailRow) => {
};

const createEmailItem = (
email: EmailRow & { primary?: boolean },
email: EmailAddressRow & { primary?: boolean },
breachCounts: Map<string, number>,
) => {
const l10n = getL10n();
Expand All @@ -82,7 +80,9 @@ const createEmailItem = (
};

// Moves the primary email to the front and sorts the rest alphabeticaly.
const getSortedEmails = (emails: Array<EmailRow & { primary?: boolean }>) =>
const getSortedEmails = (
emails: Array<EmailAddressRow & { primary?: boolean }>,
) =>
[...emails].sort((a, b) => {
if (a.primary) {
return -1;
Expand All @@ -96,7 +96,7 @@ const getSortedEmails = (emails: Array<EmailRow & { primary?: boolean }>) =>
});

const createEmailList = (
emails: EmailRow[],
emails: EmailAddressRow[],
breachCounts: Map<string, number>,
) => {
return (
Expand Down
4 changes: 2 additions & 2 deletions src/app/functions/server/getUserBreaches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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
Expand All @@ -29,7 +29,7 @@ export type UserBreaches = {
emailTotalCount: number;
emailSelectIndex: number;
breachesData: {
unverifiedEmails: EmailRow[];
unverifiedEmails: EmailAddressRow[];
verifiedEmails: BundledVerifiedEmails[];
};
};
Expand Down
36 changes: 36 additions & 0 deletions src/app/functions/server/sanitizeEmailRow.ts
Original file line number Diff line number Diff line change
@@ -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,
};
}
11 changes: 1 addition & 10 deletions src/db/tables/emailAddresses.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<EmailRow[]>}
* @returns {Promise<import('knex/types/tables').EmailAddressRow[]>}
*/
// Not covered by tests; mostly side-effects. See test-coverage.md#mock-heavy
/* c8 ignore start */
Expand Down
2 changes: 1 addition & 1 deletion src/utils/breaches.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
3 changes: 3 additions & 0 deletions src/utils/subscriberBreaches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 7f7faf7

Please sign in to comment.