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.

Sign up for GitHub

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

Jump to bottom

Fixes security issue when updating workflows #3661

Merged
merged 8 commits into from
Aug 9, 2022
Merged

Fixes security issue when updating workflows #3661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
07d922c
check if workflow is deleted before deleting reminders
Aug 2, 2022
a2e1809
disable continue button when mutation is loading
Aug 2, 2022
029c517
check if new active event types belong to user
Aug 2, 2022
8d4cc75
fix issue with onCascade deletion
Aug 3, 2022
d8e3c70
Merge branch 'main' into fix/security-fixes-workflows
zomars Aug 9, 2022
b3dc321
Simplified ANDs
zomars Aug 9, 2022
796ff67
Merge branch 'main' into fix/security-fixes-workflows
CarinaWolli Aug 9, 2022
0fabe2d
Merge branch 'main' into fix/security-fixes-workflows
kodiakhq[bot] Aug 9, 2022
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
2 changes: 1 addition & 1 deletion packages/features/ee/workflows/components/NewWorkflowButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ export function NewWorkflowButton() {
</div>
)}
</>
<div className="mt-8 flex flex-row-reverse gap-x-2">
<div className=" mt-8 flex flex-row-reverse gap-x-2">
<Button type="submit" disabled={createMutation.isLoading}>
{t("continue")}
</Button>
Expand Down
124 changes: 74 additions & 50 deletions packages/trpc/server/routers/viewer/workflows.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,8 @@ export const workflowsRouter = createProtectedRouter()
async resolve({ ctx, input }) {
const workflow = await ctx.prisma.workflow.findFirst({
where: {
AND: [
{
userId: ctx.user.id,
},
{
id: input.id,
},
],
userId: ctx.user.id,
id: input.id,
},
select: {
id: true,
Expand Down Expand Up @@ -136,41 +130,43 @@ export const workflowsRouter = createProtectedRouter()
async resolve({ ctx, input }) {
const { id } = input;

//delete all scheduled reminders of this workflow
const scheduledReminders = await ctx.prisma.workflowReminder.findMany({
const workflowToDelete = await ctx.prisma.workflow.findFirst({
where: {
workflowStep: {
workflowId: id,
},
scheduled: true,
NOT: {
referenceId: null,
},
id,
userId: ctx.user.id,
},
});

scheduledReminders.forEach((reminder) => {
if (reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
}
});

await ctx.prisma.workflow.deleteMany({
where: {
AND: [
{
userId: ctx.user.id,
if (workflowToDelete) {
const scheduledReminders = await ctx.prisma.workflowReminder.findMany({
where: {
workflowStep: {
workflowId: id,
},
{
id,
scheduled: true,
NOT: {
referenceId: null,
},
],
},
});
},
});

scheduledReminders.forEach((reminder) => {
if (reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
}
});

await ctx.prisma.workflow.deleteMany({
where: {
userId: ctx.user.id,
id,
},
});
}

return {
id,
Expand Down Expand Up @@ -214,7 +210,6 @@ export const workflowsRouter = createProtectedRouter()

if (!userWorkflow || userWorkflow.userId !== user.id) throw new TRPCError({ code: "UNAUTHORIZED" });

//remove all scheduled Email and SMS reminders for eventTypes that are not active any more
const oldActiveOnEventTypes = await ctx.prisma.workflowsOnEventTypes.findMany({
where: {
workflowId: id,
Expand All @@ -224,6 +219,46 @@ export const workflowsRouter = createProtectedRouter()
},
});

const newActiveEventTypes = activeOn.filter((eventType) => {
if (
!oldActiveOnEventTypes ||
!oldActiveOnEventTypes
.map((oldEventType) => {
return oldEventType.eventTypeId;
})
.includes(eventType)
) {
return eventType;
}
});

//check if new event types belong to user
for (const newEventTypeId of newActiveEventTypes) {
const newEventType = await ctx.prisma.eventType.findFirst({
where: {
id: newEventTypeId,
},
include: {
team: {
include: {
members: true,
},
},
},
});

if (
newEventType &&
newEventType.userId !== user.id &&
newEventType?.team?.members.filter((membership) => {
membership.userId === user.id;
}).length
) {
throw new TRPCError({ code: "UNAUTHORIZED" });
Copy link
Member Author

@CarinaWolli CarinaWolli Aug 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Throw an error when a user tries to set an event type as active that does not belong to the user or a team where the user is a member

}
}

//remove all scheduled Email and SMS reminders for eventTypes that are not active any more
const removedEventTypes = oldActiveOnEventTypes
.map((eventType) => {
return eventType.eventTypeId;
Expand Down Expand Up @@ -300,18 +335,7 @@ export const workflowsRouter = createProtectedRouter()
let newEventTypes: number[] = [];
if (activeOn.length) {
if (trigger === WorkflowTriggerEvents.BEFORE_EVENT) {
newEventTypes = activeOn.filter((eventType) => {
if (
!oldActiveOnEventTypes ||
!oldActiveOnEventTypes
.map((oldEventType) => {
return oldEventType.eventTypeId;
})
.includes(eventType)
) {
return eventType;
}
});
newEventTypes = newActiveEventTypes;
}
if (newEventTypes.length > 0) {
//create reminders for all bookings with newEventTypes
Expand Down
Loading