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

fix: prevent potential email abuse on invitations endpoint #4569

Merged
merged 4 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions front/lib/api/invitation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
import { Err, sanitizeString } from "@dust-tt/types";
import sgMail from "@sendgrid/mail";
import { sign } from "jsonwebtoken";
import { Op } from "sequelize";

import config from "@app/lib/api/config";
import type { Authenticator } from "@app/lib/auth";
Expand Down Expand Up @@ -178,3 +179,40 @@ export async function getPendingInvitations(
};
});
}

/**
* Returns the pending or revoked inviations that were created today
* associated with the authenticator's owner workspace.
* @param auth Authenticator
* @returns MenbershipInvitation[] members of the workspace
*/

export async function getRecentPendingOrRevokedInvitations(
auth: Authenticator
): Promise<MembershipInvitationType[]> {
const owner = auth.workspace();
if (!owner) {
fontanierh marked this conversation as resolved.
Show resolved Hide resolved
return [];
}
const oneDayAgo = new Date();
oneDayAgo.setDate(oneDayAgo.getDate() - 1);
const invitations = await MembershipInvitation.findAll({
where: {
workspaceId: owner.id,
status: ["pending", "revoked"],
createdAt: {
[Op.gt]: oneDayAgo,
},
},
});

return invitations.map((i) => {
return {
sId: i.sId,
id: i.id,
status: i.status,
inviteEmail: i.inviteEmail,
initialRole: i.initialRole,
};
});
}
2 changes: 2 additions & 0 deletions front/lib/invitations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Maxmimum allowed number of unconsumed invitations per workspace per day.
export const MAX_UNCONSUMED_INVITATIONS_PER_WORKSPACE_PER_DAY = 50;
36 changes: 35 additions & 1 deletion front/pages/api/w/[wId]/invitations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ import * as reporter from "io-ts-reporters";
import type { NextApiRequest, NextApiResponse } from "next";

import {
getRecentPendingOrRevokedInvitations,
sendWorkspaceInvitationEmail,
updateOrCreateInvitation,
} from "@app/lib/api/invitation";
import { getPendingInvitations } from "@app/lib/api/invitation";
import { getMembers } from "@app/lib/api/workspace";
import { Authenticator, getSession } from "@app/lib/auth";
import { MAX_UNCONSUMED_INVITATIONS_PER_WORKSPACE_PER_DAY } from "@app/lib/invitations";
import { MembershipResource } from "@app/lib/resources/membership_resource";
import { isEmailValid } from "@app/lib/utils";
import logger from "@app/logger/logger";
Expand Down Expand Up @@ -152,7 +154,39 @@ async function handler(
});
}
const existingMembers = await getMembers(auth);

const unconsumedInvitations = await getRecentPendingOrRevokedInvitations(
auth
);
if (
unconsumedInvitations.length >
MAX_UNCONSUMED_INVITATIONS_PER_WORKSPACE_PER_DAY
) {
return apiError(req, res, {
status_code: 400,
api_error: {
type: "invalid_request_error",
message: `Too many unconsumed invitations. Please ask your members to consume their invitations before sending more.`,
},
});
}
const emailsWithRecentUnconsumedInvitations = new Set(
unconsumedInvitations.map((i) => i.inviteEmail.toLowerCase().trim())
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we do this when creating those invitations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ceinture bretelles

);
if (
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming one email address already has a fresh recent invitation, the entire operation will fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, would you prefer that we only filter those out ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean as long as we have an explicit error message in the UI and the user does not lose its email addresses list, I'm fine.

invitationRequests.some((r) =>
emailsWithRecentUnconsumedInvitations.has(
r.email.toLowerCase().trim()
)
)
) {
return apiError(req, res, {
status_code: 400,
api_error: {
type: "invalid_request_error",
message: `Some of the emails have already received an invitation in the last 24 hours. Please wait before sending another invitation.`,
},
});
}
const invitationResults = await Promise.all(
invitationRequests.map(async ({ email, role }) => {
if (existingMembers.find((m) => m.email === email)) {
Expand Down
12 changes: 12 additions & 0 deletions front/pages/w/[wId]/members/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import {
PRO_PLAN_29_COST,
} from "@app/lib/client/subscription";
import { withDefaultUserAuthRequirements } from "@app/lib/iam/session";
import { MAX_UNCONSUMED_INVITATIONS_PER_WORKSPACE_PER_DAY } from "@app/lib/invitations";
import { isUpgraded, PRO_PLAN_SEAT_29_CODE } from "@app/lib/plans/plan_codes";
import { useMembers, useWorkspaceInvitations } from "@app/lib/swr";
import { classNames, isEmailValid } from "@app/lib/utils";
Expand Down Expand Up @@ -484,6 +485,17 @@ function InviteEmailModal({
async function handleSendInvitations(
inviteEmailsList: string[]
): Promise<void> {
if (
inviteEmailsList.length > MAX_UNCONSUMED_INVITATIONS_PER_WORKSPACE_PER_DAY
) {
sendNotification({
type: "error",
title: "Too many invitations",
description: `Your cannot send more than ${MAX_UNCONSUMED_INVITATIONS_PER_WORKSPACE_PER_DAY} invitations per day.`,
});
return;
}

const invitesByCase = {
activeSameRole: members.filter((m) =>
inviteEmailsList.find(
Expand Down
Loading