From a922fe578d47c5522bad779bd9d8f551651232a8 Mon Sep 17 00:00:00 2001 From: Eunjae Lee Date: Fri, 19 Sep 2025 15:12:22 +0200 Subject: [PATCH 1/2] fix: remove page permission check for /insights --- .../insights/checkInsightsPagePermission.ts | 10 -- .../insights/server/hasInsightsPermission.ts | 39 ------ .../features/insights/server/trpc-router.ts | 112 ++++++++---------- 3 files changed, 49 insertions(+), 112 deletions(-) delete mode 100644 packages/features/insights/server/hasInsightsPermission.ts diff --git a/apps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.ts b/apps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.ts index 49a5af32fc81b5..cb86e9e3913ec6 100644 --- a/apps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.ts +++ b/apps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.ts @@ -3,7 +3,6 @@ import { redirect } from "next/navigation"; import { getServerSession } from "@calcom/features/auth/lib/getServerSession"; import { FeaturesRepository } from "@calcom/features/flags/features.repository"; -import { hasInsightsPermission } from "@calcom/features/insights/server/hasInsightsPermission"; import { prisma } from "@calcom/prisma"; import { buildLegacyRequest } from "@lib/buildLegacyCtx"; @@ -21,14 +20,5 @@ export async function checkInsightsPagePermission() { redirect("/auth/login"); } - const hasPermission = await hasInsightsPermission({ - userId: session.user.id, - organizationId: session.user.org?.id, - }); - - if (!hasPermission) { - redirect("/"); - } - return session; } diff --git a/packages/features/insights/server/hasInsightsPermission.ts b/packages/features/insights/server/hasInsightsPermission.ts deleted file mode 100644 index a28c66a5489ce5..00000000000000 --- a/packages/features/insights/server/hasInsightsPermission.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service"; -import { MembershipRepository } from "@calcom/lib/server/repository/membership"; -import { MembershipRole } from "@calcom/prisma/enums"; - -export async function hasInsightsPermission({ - userId, - organizationId, -}: { - userId: number; - organizationId: number | null | undefined; -}) { - if (organizationId) { - const permissionCheckService = new PermissionCheckService(); - return await permissionCheckService.checkPermission({ - userId, - teamId: organizationId, - permission: "insights.read", - fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN, MembershipRole.MEMBER], // even members can see their own data - }); - } else { - const permissionCheckService = new PermissionCheckService(); - const teamIds = await MembershipRepository.findUserTeamIds({ userId }); - - for (const teamId of teamIds) { - const hasPermission = await permissionCheckService.checkPermission({ - userId, - teamId, - permission: "insights.read", - fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN, MembershipRole.MEMBER], // even members can see their own data - }); - - if (hasPermission) { - return true; - } - } - } - - return false; -} diff --git a/packages/features/insights/server/trpc-router.ts b/packages/features/insights/server/trpc-router.ts index c8d55f681e1f00..0fd2ece8a904d9 100644 --- a/packages/features/insights/server/trpc-router.ts +++ b/packages/features/insights/server/trpc-router.ts @@ -22,7 +22,6 @@ import { router } from "@calcom/trpc/server/trpc"; import { TRPCError } from "@trpc/server"; -import { hasInsightsPermission } from "./hasInsightsPermission"; import { getTimeView, getDateRanges, type GetDateRangesParams } from "./insightsDateUtils"; import { RoutingEventsInsights } from "./routing-events"; import { VirtualQueuesInsights } from "./virtual-queues"; @@ -240,21 +239,6 @@ const userBelongsToTeamProcedure = authedProcedure.use(async ({ ctx, next, getRa }); }); -const insightsPbacProcedure = userBelongsToTeamProcedure.use(async ({ ctx, next }) => { - const hasPermission = await hasInsightsPermission({ - userId: ctx.user.id, - organizationId: ctx.user.organizationId, - }); - - if (!hasPermission) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - - return next({ - ctx, - }); -}); - const userSelect = { id: true, name: true, @@ -353,7 +337,7 @@ function createInsightsRoutingService( } export const insightsRouter = router({ - bookingKPIStats: insightsPbacProcedure + bookingKPIStats: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const currentPeriodService = createInsightsBookingService(ctx, input); @@ -458,31 +442,33 @@ export const insightsRouter = router({ }, }; }), - eventTrends: insightsPbacProcedure.input(bookingRepositoryBaseInputSchema).query(async ({ ctx, input }) => { - const { columnFilters, timeZone } = input; - const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters); - - // Calculate timeView and dateRanges - const timeView = getTimeView(startDate, endDate); - const dateRanges = getDateRanges({ - startDate, - endDate, - timeView, - timeZone, - weekStart: ctx.user.weekStart, - }); + eventTrends: userBelongsToTeamProcedure + .input(bookingRepositoryBaseInputSchema) + .query(async ({ ctx, input }) => { + const { columnFilters, timeZone } = input; + const { startDate, endDate } = extractDateRangeFromColumnFilters(columnFilters); - const insightsBookingService = createInsightsBookingService(ctx, input); - try { - return await insightsBookingService.getEventTrendsStats({ + // Calculate timeView and dateRanges + const timeView = getTimeView(startDate, endDate); + const dateRanges = getDateRanges({ + startDate, + endDate, + timeView, timeZone, - dateRanges, + weekStart: ctx.user.weekStart, }); - } catch (e) { - throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); - } - }), - popularEvents: insightsPbacProcedure + + const insightsBookingService = createInsightsBookingService(ctx, input); + try { + return await insightsBookingService.getEventTrendsStats({ + timeZone, + dateRanges, + }); + } catch (e) { + throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); + } + }), + popularEvents: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -493,7 +479,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - averageEventDuration: insightsPbacProcedure + averageEventDuration: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const { columnFilters, timeZone } = input; @@ -554,7 +540,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithMostCancelledBookings: insightsPbacProcedure + membersWithMostCancelledBookings: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -568,7 +554,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithMostCompletedBookings: insightsPbacProcedure + membersWithMostCompletedBookings: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -583,7 +569,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithLeastCompletedBookings: insightsPbacProcedure + membersWithLeastCompletedBookings: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -598,7 +584,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithMostBookings: insightsPbacProcedure + membersWithMostBookings: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -609,7 +595,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithLeastBookings: insightsPbacProcedure + membersWithLeastBookings: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -773,7 +759,7 @@ export const insightsRouter = router({ return usersInTeam.map((membership) => membership.user); }), - eventTypeList: insightsPbacProcedure + eventTypeList: userBelongsToTeamProcedure .input( z.object({ teamId: z.coerce.number().nullish(), @@ -799,13 +785,13 @@ export const insightsRouter = router({ return eventTypeList; }), - recentRatings: insightsPbacProcedure + recentRatings: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const insightsBookingService = createInsightsBookingService(ctx, input); return await insightsBookingService.getRecentRatingsStats(); }), - membersWithMostNoShow: insightsPbacProcedure + membersWithMostNoShow: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -816,19 +802,19 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithHighestRatings: insightsPbacProcedure + membersWithHighestRatings: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const insightsBookingService = createInsightsBookingService(ctx, input); return await insightsBookingService.getMembersRatingStats("DESC"); }), - membersWithLowestRatings: insightsPbacProcedure + membersWithLowestRatings: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const insightsBookingService = createInsightsBookingService(ctx, input); return await insightsBookingService.getMembersRatingStats("ASC"); }), - rawData: insightsPbacProcedure + rawData: userBelongsToTeamProcedure .input( bookingRepositoryBaseInputSchema.extend({ limit: z.number().max(100).optional(), @@ -850,7 +836,7 @@ export const insightsRouter = router({ } }), - getRoutingFormsForFilters: insightsPbacProcedure + getRoutingFormsForFilters: userBelongsToTeamProcedure .input(z.object({ userId: z.number().optional(), teamId: z.number().optional(), isAll: z.boolean() })) .query(async ({ ctx, input }) => { const { teamId, isAll } = input; @@ -861,13 +847,13 @@ export const insightsRouter = router({ organizationId: ctx.user.organizationId ?? undefined, }); }), - routingFormsByStatus: insightsPbacProcedure + routingFormsByStatus: userBelongsToTeamProcedure .input(insightsRoutingServiceInputSchema) .query(async ({ ctx, input }) => { const insightsRoutingService = createInsightsRoutingService(ctx, input); return await insightsRoutingService.getRoutingFormStats(); }), - routingFormResponses: insightsPbacProcedure + routingFormResponses: userBelongsToTeamProcedure .input(insightsRoutingServicePaginatedInputSchema) .query(async ({ ctx, input }) => { const insightsRoutingService = createInsightsRoutingService(ctx, input); @@ -877,7 +863,7 @@ export const insightsRouter = router({ offset: input.offset, }); }), - routingFormResponsesForDownload: insightsPbacProcedure + routingFormResponsesForDownload: userBelongsToTeamProcedure .input(insightsRoutingServicePaginatedInputSchema) .query(async ({ ctx, input }) => { const headersPromise = RoutingEventsInsights.getRoutingFormHeaders({ @@ -902,7 +888,7 @@ export const insightsRouter = router({ dataPromise, }); }), - getRoutingFormFieldOptions: insightsPbacProcedure + getRoutingFormFieldOptions: userBelongsToTeamProcedure .input( z.object({ userId: z.number().optional(), @@ -919,7 +905,7 @@ export const insightsRouter = router({ }); return options; }), - failedBookingsByField: insightsPbacProcedure + failedBookingsByField: userBelongsToTeamProcedure .input(insightsRoutingServiceInputSchema) .query(async ({ ctx, input }) => { const insightsRoutingService = createInsightsRoutingService(ctx, input); @@ -929,7 +915,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - routingFormResponsesHeaders: insightsPbacProcedure + routingFormResponsesHeaders: userBelongsToTeamProcedure .input( z.object({ userId: z.number().optional(), @@ -949,7 +935,7 @@ export const insightsRouter = router({ return headers || []; }), - routedToPerPeriod: insightsPbacProcedure + routedToPerPeriod: userBelongsToTeamProcedure .input(routedToPerPeriodInputSchema) .query(async ({ ctx, input }) => { const { period, limit, searchQuery, ...rest } = input; @@ -965,7 +951,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - routedToPerPeriodCsv: insightsPbacProcedure + routedToPerPeriodCsv: userBelongsToTeamProcedure .input(routedToPerPeriodCsvInputSchema) .query(async ({ ctx, input }) => { const { period, searchQuery, ...rest } = input; @@ -998,7 +984,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - getRoutingFunnelData: insightsPbacProcedure + getRoutingFunnelData: userBelongsToTeamProcedure .input(routingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const timeView = getTimeView(input.startDate, input.endDate); @@ -1016,7 +1002,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - bookingsByHourStats: insightsPbacProcedure + bookingsByHourStats: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const { timeZone } = input; @@ -1030,7 +1016,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - recentNoShowGuests: insightsPbacProcedure + recentNoShowGuests: userBelongsToTeamProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const insightsBookingService = createInsightsBookingService(ctx, input); From 7398a69854ba12d2e85c5a1a3791d3648817130b Mon Sep 17 00:00:00 2001 From: Eunjae Lee Date: Fri, 19 Sep 2025 16:59:05 +0200 Subject: [PATCH 2/2] add e2e test --- apps/web/playwright/insights.e2e.ts | 77 +++++++++++++++++++- apps/web/playwright/lib/test-helpers/pbac.ts | 29 ++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 apps/web/playwright/lib/test-helpers/pbac.ts diff --git a/apps/web/playwright/insights.e2e.ts b/apps/web/playwright/insights.e2e.ts index d775545e997804..5f81d411d3dc06 100644 --- a/apps/web/playwright/insights.e2e.ts +++ b/apps/web/playwright/insights.e2e.ts @@ -1,10 +1,13 @@ import { expect } from "@playwright/test"; +import { FeaturesRepository } from "@calcom/features/flags/features.repository"; +import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service"; import { randomString } from "@calcom/lib/random"; -import prisma from "@calcom/prisma"; +import { prisma } from "@calcom/prisma"; import { clearFilters, applySelectFilter } from "./filter-helpers"; import { test } from "./lib/fixtures"; +import { createAllPermissionsArray, enablePBACForTeam } from "./lib/test-helpers/pbac"; test.describe.configure({ mode: "parallel" }); @@ -272,4 +275,76 @@ test.describe("Insights", async () => { await expect(chartCard).toBeVisible(); } }); + + test("should be able to access insights page with custom role lacking insights.read permission", async ({ + page, + users, + }) => { + const owner = await users.create(undefined, { + hasTeam: true, + isUnpublished: true, + isOrg: true, + }); + + const userOne = await users.create(); + const userTwo = await users.create(); + + const { teamOne } = await createTeamsAndMembership(userOne.id, userTwo.id); + + const orgMembership = await owner.getOrgMembership(); + const orgId = orgMembership.team.id; + + await enablePBACForTeam(orgId); + await enablePBACForTeam(teamOne.id); + + const permissions = createAllPermissionsArray().filter( + ({ resource, action }) => !(resource === "insights" && action === "read") + ); + + const customRole = await prisma.role.create({ + data: { + id: `e2e_no_insights_${orgId}_${Date.now()}`, + name: "E2E Role Without Insights", + description: "E2E role for testing - has all permissions except insights.read", + color: "#dc2626", + teamId: orgId, + type: "CUSTOM", + permissions: { + create: permissions, + }, + }, + }); + + await prisma.membership.update({ + where: { + userId_teamId: { + userId: userOne.id, + teamId: teamOne.id, + }, + }, + data: { + customRoleId: customRole.id, + }, + }); + + const featuresRepository = new FeaturesRepository(prisma); + const isPBACEnabled = await featuresRepository.checkIfTeamHasFeature(orgId, "pbac"); + expect(isPBACEnabled).toBe(true); + + const permissionService = new PermissionCheckService(); + const hasPermission = await permissionService.checkPermission({ + userId: userOne.id, + teamId: teamOne.id, + permission: "insights.read", + fallbackRoles: [], + }); + expect(hasPermission).toBe(false); + + await userOne.apiLogin(); + await page.goto("/insights"); + + // Verify the user can access the insights page + await page.locator('[data-testid^="insights-filters-"]').waitFor(); + expect(page.url()).toContain("/insights"); + }); }); diff --git a/apps/web/playwright/lib/test-helpers/pbac.ts b/apps/web/playwright/lib/test-helpers/pbac.ts new file mode 100644 index 00000000000000..806ae55a095615 --- /dev/null +++ b/apps/web/playwright/lib/test-helpers/pbac.ts @@ -0,0 +1,29 @@ +import { PERMISSION_REGISTRY } from "@calcom/features/pbac/domain/types/permission-registry"; +import { prisma } from "@calcom/prisma"; + +// Create array of all permissions from PERMISSION_REGISTRY +export const createAllPermissionsArray = () => { + const allPermissions: { resource: string; action: string }[] = []; + + Object.entries(PERMISSION_REGISTRY).forEach(([resource, resourceConfig]) => { + if (resource === "*") { + return; + } + Object.entries(resourceConfig).forEach(([action, _details]) => { + allPermissions.push({ resource, action }); + }); + }); + + return allPermissions; +}; + +export const enablePBACForTeam = async (teamId: number) => { + await prisma.teamFeatures.create({ + data: { + featureId: "pbac", + teamId: teamId, + assignedBy: "e2e", + assignedAt: new Date(), + }, + }); +};