t3stack-todo-app
t3stack-todo-app copied to clipboard
Missing access control while doing some CRUD operations.
In src/trpc/rotuer/todo.ts
these procedures only checks if the user is authenticated but does not check if the task it wants to mutate belongs to him.
This allows any logged in user to see every other users tasks by modifying trpc requests and trying different task id's.
I just started learning trpc and prisma and this really confused me. The getTasks and createTask function does use ctx.session.user.id
but these do not
getSingleTask: authedProcedure
.input(getSingleTaskSchema)
.query(({ ctx, input }) => {
return ctx.prisma.task.findUnique({
where: {
id: input.taskId,
},
});
}),
updateTask: authedProcedure
.input(updateTaskSchema)
.mutation(async ({ ctx, input }) => {
const task = await ctx.prisma.task.update({
where: {
id: input.taskId,
},
data: {
title: input.title,
body: input.body,
},
});
return task;
}),
deleteTask: authedProcedure
.input(deleteTaskSchema)
.mutation(async ({ ctx, input }) => {
await ctx.prisma.task.delete({
where: {
id: input.taskId,
},
});
}),
The fixed code
getSingleTask: authedProcedure
.input(getSingleTaskSchema)
.query(({ ctx, input }) => {
// Used findFirst because findUnique doesn't allow userID?? I don't know why
return ctx.prisma.task.findFirst({
where: {
id: input.taskId,
// Check for userID
userId: ctx.session.user.id,
},
});
}),
updateTask: authedProcedure
.input(updateTaskSchema)
.mutation(async ({ ctx, input }) => {
// Check if THIS user actually has the task
if (
!(await ctx.prisma.task.findFirst({
where: {
id: input.taskId,
userId: ctx.session.user.id,
},
}))
)
throw new Error("Task not found");
const task = await ctx.prisma.task.update({
where: {
id: input.taskId,
},
data: {
title: input.title,
body: input.body,
},
});
return task;
}),
deleteTask: authedProcedure
.input(deleteTaskSchema)
.mutation(async ({ ctx, input }) => {
// Same as above
if (
!(await ctx.prisma.task.findFirst({
where: {
id: input.taskId,
userId: ctx.session.user.id,
},
}))
)
throw new Error("Task not found");
await ctx.prisma.task.delete({
where: {
id: input.taskId,
},
});
}),
Diff
diff --git a/src/server/trpc/router/todo.ts b/src/server/trpc/router/todo.ts
index e8a4aaa..8b7f60c 100644
--- a/src/server/trpc/router/todo.ts
+++ b/src/server/trpc/router/todo.ts
@@ -35,15 +35,28 @@ export const todoRouter = t.router({
getSingleTask: authedProcedure
.input(getSingleTaskSchema)
.query(({ ctx, input }) => {
- return ctx.prisma.task.findUnique({
+ // Used findFirst because findUnique doesn't allow userID?? I don't know why
+ return ctx.prisma.task.findFirst({
where: {
id: input.taskId,
+ // Check for userID
+ userId: ctx.session.user.id,
},
});
}),
updateTask: authedProcedure
.input(updateTaskSchema)
.mutation(async ({ ctx, input }) => {
+ // Check if THIS user actually has the task
+ if (
+ !(await ctx.prisma.task.findFirst({
+ where: {
+ id: input.taskId,
+ userId: ctx.session.user.id,
+ },
+ }))
+ )
+ throw new Error("Task not found");
const task = await ctx.prisma.task.update({
where: {
id: input.taskId,
@@ -58,6 +71,16 @@ export const todoRouter = t.router({
deleteTask: authedProcedure
.input(deleteTaskSchema)
.mutation(async ({ ctx, input }) => {
+ // Same as above
+ if (
+ !(await ctx.prisma.task.findFirst({
+ where: {
+ id: input.taskId,
+ userId: ctx.session.user.id,
+ },
+ }))
+ )
+ throw new Error("Task not found");
await ctx.prisma.task.delete({
where: {
id: input.taskId,