From 9ef532019f878ebbc6878ebfb86883ab5237de8e Mon Sep 17 00:00:00 2001 From: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com> Date: Wed, 4 Sep 2024 23:13:38 +0400 Subject: [PATCH] fix: v1 API Teams access (#16403) --- .../api/bookings/[id]/_auth-middleware.ts | 65 ++-- .../v1/pages/api/event-types/[id]/_delete.ts | 16 +- .../_utils/checkParentEventOwnership.ts | 11 +- .../_utils/checkTeamEventEditPermission.ts | 10 +- .../api/teams/[teamId]/_auth-middleware.ts | 2 +- .../_auth-middleware.integration-test.ts | 326 +++++++++++++++--- .../test/lib/event-types/[id]/_delete.test.ts | 94 +++++ 7 files changed, 442 insertions(+), 82 deletions(-) create mode 100644 apps/api/v1/test/lib/event-types/[id]/_delete.test.ts diff --git a/apps/api/v1/pages/api/bookings/[id]/_auth-middleware.ts b/apps/api/v1/pages/api/bookings/[id]/_auth-middleware.ts index 41225f0455f65d..e198674602d56e 100644 --- a/apps/api/v1/pages/api/bookings/[id]/_auth-middleware.ts +++ b/apps/api/v1/pages/api/bookings/[id]/_auth-middleware.ts @@ -30,40 +30,59 @@ async function authMiddleware(req: NextApiRequest) { } } - const userWithBookingsAndTeamIds = await prisma.user.findUnique({ + const user = await prisma.user.findUnique({ where: { id: userId }, - include: { + select: { + email: true, bookings: true, - teams: { - select: { - teamId: true, - }, - }, }, }); - if (!userWithBookingsAndTeamIds) throw new HttpError({ statusCode: 404, message: "User not found" }); + if (!user) throw new HttpError({ statusCode: 404, message: "User not found" }); + + const filteredBookings = user?.bookings?.filter((booking) => booking.id === id); + const userIsHost = !!filteredBookings?.length; + + const bookingsAsAttendee = prisma.booking.findMany({ + where: { + id, + attendees: { some: { email: user.email } }, + }, + }); - const userBookingIds = userWithBookingsAndTeamIds.bookings.map((booking) => booking.id); + const bookingsAsEventTypeOwner = prisma.booking.findMany({ + where: { + id, + eventType: { + owner: { id: userId }, + }, + }, + }); - if (!userBookingIds.includes(id)) { - const teamBookings = await prisma.booking.findUnique({ - where: { - id: id, - eventType: { - team: { - id: { - in: userWithBookingsAndTeamIds.teams.map((team) => team.teamId), - }, + const bookingsAsTeamOwnerOrAdmin = prisma.booking.findMany({ + where: { + id, + eventType: { + team: { + members: { + some: { userId, role: { in: ["ADMIN", "OWNER"] }, accepted: true }, }, }, }, - }); + }, + }); - if (!teamBookings) { - throw new HttpError({ statusCode: 403, message: "You are not authorized" }); - } - } + const [resultOne, resultTwo, resultThree] = await Promise.all([ + bookingsAsAttendee, + bookingsAsEventTypeOwner, + bookingsAsTeamOwnerOrAdmin, + ]); + + const teamBookingsAsOwnerOrAdmin = [...resultOne, ...resultTwo, ...resultThree]; + const userHasTeamBookings = !!teamBookingsAsOwnerOrAdmin.length; + + if (!userIsHost && !userHasTeamBookings) + throw new HttpError({ statusCode: 403, message: "You are not authorized" }); } export default authMiddleware; diff --git a/apps/api/v1/pages/api/event-types/[id]/_delete.ts b/apps/api/v1/pages/api/event-types/[id]/_delete.ts index 62ad8e13b69256..efcc3c98efae3c 100644 --- a/apps/api/v1/pages/api/event-types/[id]/_delete.ts +++ b/apps/api/v1/pages/api/event-types/[id]/_delete.ts @@ -6,6 +6,9 @@ import prisma from "@calcom/prisma"; import { schemaQueryIdParseInt } from "~/lib/validations/shared/queryIdTransformParseInt"; +import checkParentEventOwnership from "../_utils/checkParentEventOwnership"; +import checkTeamEventEditPermission from "../_utils/checkTeamEventEditPermission"; + /** * @swagger * /event-types/{id}: @@ -49,9 +52,18 @@ async function checkPermissions(req: NextApiRequest) { const { userId, isSystemWideAdmin } = req; const { id } = schemaQueryIdParseInt.parse(req.query); if (isSystemWideAdmin) return; - /** Only event type owners can delete it */ - const eventType = await prisma.eventType.findFirst({ where: { id, userId } }); + + const eventType = await prisma.eventType.findFirst({ where: { id } }); + if (!eventType) throw new HttpError({ statusCode: 403, message: "Forbidden" }); + + /** Only event type owners or team owners for team events can delete it */ + if (eventType.teamId) return await checkTeamEventEditPermission(req, { teamId: eventType.teamId }); + + if (eventType.parentId) return await checkParentEventOwnership(req); + + if (eventType.userId && eventType.userId !== userId) + throw new HttpError({ statusCode: 403, message: "Forbidden" }); } export default defaultResponder(deleteHandler); diff --git a/apps/api/v1/pages/api/event-types/_utils/checkParentEventOwnership.ts b/apps/api/v1/pages/api/event-types/_utils/checkParentEventOwnership.ts index 7a7025296fbe48..38e9fe78c25c0a 100644 --- a/apps/api/v1/pages/api/event-types/_utils/checkParentEventOwnership.ts +++ b/apps/api/v1/pages/api/event-types/_utils/checkParentEventOwnership.ts @@ -18,12 +18,8 @@ export default async function checkParentEventOwnership(req: NextApiRequest) { /** These are already parsed upstream, we can assume they're good here. */ const parentId = Number(body.parentId); const parentEventType = await prisma.eventType.findUnique({ - where: { - id: parentId, - }, - select: { - teamId: true, - }, + where: { id: parentId }, + select: { teamId: true }, }); if (!parentEventType) { @@ -44,7 +40,8 @@ export default async function checkParentEventOwnership(req: NextApiRequest) { where: { teamId: parentEventType.teamId, userId: userId, - OR: [{ role: "OWNER" }, { role: "ADMIN" }], + role: { in: ["ADMIN", "OWNER"] }, + accepted: true, }, }); diff --git a/apps/api/v1/pages/api/event-types/_utils/checkTeamEventEditPermission.ts b/apps/api/v1/pages/api/event-types/_utils/checkTeamEventEditPermission.ts index 76c7cba3a7d3a4..edba1dcea4da4e 100644 --- a/apps/api/v1/pages/api/event-types/_utils/checkTeamEventEditPermission.ts +++ b/apps/api/v1/pages/api/event-types/_utils/checkTeamEventEditPermission.ts @@ -10,21 +10,17 @@ export default async function checkTeamEventEditPermission( req: NextApiRequest, body: Pick, "teamId" | "userId"> ) { - const { isSystemWideAdmin } = req; - let userId = req.userId; - if (isSystemWideAdmin && body.userId) { - userId = body.userId; - } if (body.teamId) { const membership = await prisma.membership.findFirst({ where: { - userId, + userId: req.userId, teamId: body.teamId, accepted: true, + role: { in: ["ADMIN", "OWNER"] }, }, }); - if (!membership?.role || !["ADMIN", "OWNER"].includes(membership.role)) { + if (!membership) { throw new HttpError({ statusCode: 403, message: "No permission to operate on event-type for this team", diff --git a/apps/api/v1/pages/api/teams/[teamId]/_auth-middleware.ts b/apps/api/v1/pages/api/teams/[teamId]/_auth-middleware.ts index f98cfbef8a420c..2ca975a09db709 100644 --- a/apps/api/v1/pages/api/teams/[teamId]/_auth-middleware.ts +++ b/apps/api/v1/pages/api/teams/[teamId]/_auth-middleware.ts @@ -41,7 +41,7 @@ export async function canUserAccessTeamWithRole( /** If not ADMIN then we check if the actual user belongs to team and matches the required role */ if (!isSystemWideAdmin) args.where = { ...args.where, members: { some: { userId, role } } }; const team = await prisma.team.findFirst(args); - if (!team) throw new HttpError({ statusCode: 401, message: `Unauthorized: ${role.toString()} required` }); + if (!team) throw new HttpError({ statusCode: 401, message: `Unauthorized: OWNER or ADMIN role required` }); return team; } diff --git a/apps/api/v1/test/lib/bookings/_auth-middleware.integration-test.ts b/apps/api/v1/test/lib/bookings/_auth-middleware.integration-test.ts index b0fd7d888e05f8..7b449700828e1e 100644 --- a/apps/api/v1/test/lib/bookings/_auth-middleware.integration-test.ts +++ b/apps/api/v1/test/lib/bookings/_auth-middleware.integration-test.ts @@ -1,90 +1,332 @@ +import prismock from "../../../../../../tests/libs/__mocks__/prisma"; + import type { Request, Response } from "express"; import type { NextApiRequest, NextApiResponse } from "next"; import { createMocks } from "node-mocks-http"; -import { describe, it, expect } from "vitest"; +import { describe, it, expect, test } from "vitest"; -import type { HttpError } from "@calcom/lib/http-error"; -import prisma from "@calcom/prisma"; +import { MembershipRole } from "@calcom/prisma/enums"; import authMiddleware from "../../../pages/api/bookings/[id]/_auth-middleware"; type CustomNextApiRequest = NextApiRequest & Request; type CustomNextApiResponse = NextApiResponse & Response; -describe("Bookings auth middleware", () => { - it("Returns 403 when user has no permission to the booking", async () => { - const trialUser = await prisma.user.findFirstOrThrow({ where: { email: "trial@example.com" } }); - const proUser = await prisma.user.findFirstOrThrow({ where: { email: "pro@example.com" } }); - const booking = await prisma.booking.findFirstOrThrow({ where: { userId: proUser.id } }); +describe("Booking ownership and access in Middleware", () => { + const adminUserId = 1; + const ownerUserId = 2; + const memberUserId = 3; + const orgOwnerUserId = 4; + const adminUserEmail = "admin@example.com"; + const ownerUserEmail = "owner@example.com"; + const memberUserEmail = "member@example.com"; + const orgOwnerUserEmail = "org-owner@example.com"; + // mock user data + function buildMockData() { + //Create Users + prismock.user.create({ + data: { + id: adminUserId, + username: "admin", + name: "Admin User", + email: adminUserEmail, + }, + }); + prismock.user.create({ + data: { + id: ownerUserId, + username: "owner", + name: "Owner User", + email: ownerUserEmail, + }, + }); + prismock.user.create({ + data: { + id: orgOwnerUserId, + username: "org-owner", + name: "Org Owner", + email: orgOwnerUserEmail, + }, + }); + prismock.user.create({ + data: { + id: memberUserId, + username: "member", + name: "Member User", + email: memberUserEmail, + bookings: { + create: { + id: 2, + uid: "2", + title: "Booking 2", + eventTypeId: 1, + startTime: "2024-08-30T06:45:00.000Z", + endTime: "2024-08-30T07:45:00.000Z", + attendees: { + create: { + name: "Member User", + email: memberUserEmail, + timeZone: "UTC", + }, + }, + }, + }, + }, + }); + //create team + prismock.team.create({ + data: { + id: 1, + name: "Team 1", + slug: "team1", + members: { + createMany: { + data: [ + { + userId: adminUserId, + role: MembershipRole.ADMIN, + accepted: true, + }, + { + userId: ownerUserId, + role: MembershipRole.OWNER, + accepted: true, + }, + { + userId: memberUserId, + role: MembershipRole.MEMBER, + accepted: true, + }, + ], + }, + }, + }, + }); + //create Org + prismock.team.create({ + data: { + id: 2, + name: "Org", + slug: "org", + isOrganization: true, + children: { + connect: { + id: 1, + }, + }, + members: { + createMany: { + data: [ + { + userId: orgOwnerUserId, + role: MembershipRole.OWNER, + accepted: true, + }, + { + userId: memberUserId, + role: MembershipRole.MEMBER, + accepted: true, + }, + { + userId: ownerUserId, + role: MembershipRole.MEMBER, + accepted: true, + }, + { + userId: adminUserId, + role: MembershipRole.MEMBER, + accepted: true, + }, + ], + }, + }, + }, + }); + //create eventTypes + prismock.eventType.create({ + data: { + id: 1, + title: "Event 1", + slug: "event", + length: 60, + bookings: { + connect: { + id: 2, + }, + }, + }, + }); + prismock.eventType.create({ + data: { + id: 2, + title: "Event 2", + slug: "event", + length: 60, + teamId: 1, + bookings: { + connect: { + id: 1, + }, + }, + }, + }); + //link eventType to teams + prismock.eventType.update({ + where: { + id: 1, + }, + data: { + owner: { + connect: { + id: ownerUserId, + }, + }, + }, + }); + prismock.eventType.update({ + where: { + id: 2, + }, + data: { + team: { + connect: { + id: 1, + }, + }, + }, + }); + //link team to org + prismock.team.update({ + where: { + id: 1, + }, + data: { + parentId: 2, + }, + }); + // Call Prisma to create booking with attendees + prismock.booking.create({ + data: { + id: 1, + uid: "1", + title: "Booking 1", + userId: 1, + startTime: "2024-08-30T06:45:00.000Z", + endTime: "2024-08-30T07:45:00.000Z", + eventTypeId: 2, + attendees: { + create: { + name: "Admin User", + email: adminUserEmail, + timeZone: "UTC", + }, + }, + }, + }); + } + test("should not throw error for bookings where user is an attendee", async () => { const { req } = createMocks({ method: "GET", - body: {}, query: { - id: booking.id, + id: 2, }, + prisma: prismock, }); + buildMockData(); + req.userId = memberUserId; + await expect(authMiddleware(req)).resolves.not.toThrow(); + }); - req.userId = trialUser.id; + test("should throw error for bookings where user is not an attendee", async () => { + const { req } = createMocks({ + method: "GET", + query: { + id: 1, + }, + prisma: prismock, + }); + buildMockData(); + req.userId = memberUserId; - try { - await authMiddleware(req); - } catch (error) { - const httpError = error as HttpError; - expect(httpError.statusCode).toBe(403); - } + await expect(authMiddleware(req)).rejects.toThrow(); }); - it("No error is thrown when user is the booking user", async () => { - const proUser = await prisma.user.findFirstOrThrow({ where: { email: "pro@example.com" } }); - const booking = await prisma.booking.findFirstOrThrow({ where: { userId: proUser.id } }); - + test("should not throw error for booking where user is the event type owner", async () => { const { req } = createMocks({ method: "GET", - body: {}, query: { - id: booking.id, + id: 2, }, + prisma: prismock, }); + buildMockData(); + req.userId = ownerUserId; + await expect(authMiddleware(req)).resolves.not.toThrow(); + }); - req.userId = proUser.id; + test("should not throw error for booking where user is team owner or admin", async () => { + const { req: req1 } = createMocks({ + method: "GET", + query: { + id: 1, + }, + prisma: prismock, + }); + const { req: req2 } = createMocks({ + method: "GET", + query: { + id: 1, + }, + prisma: prismock, + }); + buildMockData(); - await authMiddleware(req); + req1.userId = adminUserId; + req2.userId = ownerUserId; + + await expect(authMiddleware(req1)).resolves.not.toThrow(); + await expect(authMiddleware(req2)).resolves.not.toThrow(); }); + test("should throw error for booking where user is not team owner or admin", async () => { + const { req } = createMocks({ + method: "GET", + query: { + id: 1, + }, + prisma: prismock, + }); + buildMockData(); - it("No error is thrown when user is system-wide admin", async () => { - const adminUser = await prisma.user.findFirstOrThrow({ where: { email: "admin@example.com" } }); - const proUser = await prisma.user.findFirstOrThrow({ where: { email: "pro@example.com" } }); - const booking = await prisma.booking.findFirstOrThrow({ where: { userId: proUser.id } }); + req.userId = memberUserId; + await expect(authMiddleware(req)).rejects.toThrow(); + }); + test("should not throw error when user is system-wide admin", async () => { const { req } = createMocks({ method: "GET", - body: {}, query: { - id: booking.id, + id: 2, }, + prisma: prismock, }); - - req.userId = adminUser.id; + buildMockData(); + req.userId = adminUserId; req.isSystemWideAdmin = true; await authMiddleware(req); }); - it("No error is thrown when user is org-wide admin", async () => { - const adminUser = await prisma.user.findFirstOrThrow({ where: { email: "owner1-acme@example.com" } }); - const memberUser = await prisma.user.findFirstOrThrow({ where: { email: "member1-acme@example.com" } }); - const booking = await prisma.booking.findFirstOrThrow({ where: { userId: memberUser.id } }); - + it("should throw error when user is org-wide admin", async () => { const { req } = createMocks({ method: "GET", - body: {}, query: { - id: booking.id, + id: 1, }, + prisma: prismock, }); - - req.userId = adminUser.id; + buildMockData(); + req.userId = orgOwnerUserId; req.isOrganizationOwnerOrAdmin = true; await authMiddleware(req); diff --git a/apps/api/v1/test/lib/event-types/[id]/_delete.test.ts b/apps/api/v1/test/lib/event-types/[id]/_delete.test.ts new file mode 100644 index 00000000000000..b7f7d377be676d --- /dev/null +++ b/apps/api/v1/test/lib/event-types/[id]/_delete.test.ts @@ -0,0 +1,94 @@ +import prismaMock from "../../../../../../../tests/libs/__mocks__/prismaMock"; + +import type { Request, Response } from "express"; +import type { NextApiRequest, NextApiResponse } from "next"; +import { createMocks } from "node-mocks-http"; +import { describe, expect, test, beforeEach, vi } from "vitest"; + +import { buildEventType } from "@calcom/lib/test/builder"; +import { MembershipRole } from "@calcom/prisma/enums"; + +import handler from "../../../../pages/api/event-types/[id]/_delete"; + +type CustomNextApiRequest = NextApiRequest & Request; +type CustomNextApiResponse = NextApiResponse & Response; + +describe("DELETE /api/event-types/[id]", () => { + const eventTypeId = 1234567; + const teamId = 9999; + const adminUser = 1111; + const memberUser = 2222; + beforeEach(() => { + vi.resetAllMocks(); + // Mocking membership.findFirst + prismaMock.membership.findFirst.mockImplementation(({ where }) => { + const { userId, teamId, accepted, role } = where; + const mockData = [ + { userId: 1111, teamId: teamId, accepted: true, role: MembershipRole.ADMIN }, + { userId: 2222, teamId: teamId, accepted: true, role: MembershipRole.MEMBER }, + ]; + // Return the correct user based on the query conditions + return mockData.find( + (membership) => + membership.userId === userId && + membership.teamId === teamId && + membership.accepted === accepted && + role.in.includes(membership.role) + ); + }); + + // Mocking eventType.findFirst + prismaMock.eventType.findFirst.mockResolvedValue( + buildEventType({ + id: eventTypeId, + teamId, + }) + ); + + // Mocking team.findUnique + prismaMock.team.findUnique.mockResolvedValue({ + id: teamId, + members: [ + { userId: memberUser, role: MembershipRole.MEMBER, teamId: teamId }, + { userId: adminUser, role: MembershipRole.ADMIN, teamId: teamId }, + ], + }); + }); + + describe("Error", async () => { + test("Fails to remove event type if user is not OWNER/ADMIN of team associated with event type", async () => { + const { req, res } = createMocks({ + method: "DELETE", + body: {}, + query: { + id: eventTypeId, + }, + }); + + // Assign userId to the request objects + req.userId = memberUser; + + await handler(req, res); + expect(res.statusCode).toBe(403); // Check if the deletion was successful + }); + }); + + describe("Success", async () => { + test("Removes event type if user is owner of team associated with event type", async () => { + // Mocks for DELETE request + const { req, res } = createMocks({ + method: "DELETE", + body: {}, + query: { + id: eventTypeId, + }, + }); + + // Assign userId to the request objects + req.userId = adminUser; + + await handler(req, res); + expect(res.statusCode).toBe(200); // Check if the deletion was successful + }); + }); +});