From 11370476063687005148a7b898f194276163ade5 Mon Sep 17 00:00:00 2001 From: Eunjae Lee Date: Tue, 9 Sep 2025 15:26:20 +0200 Subject: [PATCH] feat: add PBAC permission checks to insights (#23592) * feat: add PBAC permission checks to insights - Add layout-level protection for insights pages with session.user.org.id check - Add API-level protection for insights tRPC endpoints using insightsPbacProcedure - Enhance routing form insights with PBAC team filtering - Use insights.read permission with OWNER/ADMIN fallback roles - Replace all userBelongsToTeamProcedure with insightsPbacProcedure in insights router - Fix lint issues: remove unused variables and functions, fix non-null assertion Implements Permission-Based Access Control (PBAC) for insights functionality following the same patterns as event type PBAC implementation from PR #22618. Co-Authored-By: eunjae@cal.com * fix: clean up unused imports in routing-forms - Remove unused isFormCreateEditAllowed import - Remove unused TRPCError import - Change MembershipRole to type import for better tree-shaking Co-Authored-By: eunjae@cal.com * address feedback * clean up permission check * revert unnecessary changes * fix procedure * allow MEMBER for insights * support teams not under orgs * fix type error * revert unnecessary changes --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../insights/checkInsightsPagePermission.ts | 34 +++++ .../(use-page-wrapper)/insights/layout.tsx | 30 ++-- .../app/(use-page-wrapper)/insights/page.tsx | 8 +- .../insights/router-position/page.tsx | 4 + .../insights/routing/page.tsx | 4 + .../insights/server/hasInsightsPermission.ts | 39 +++++ .../insights/server/routing-events.ts | 2 +- .../features/insights/server/trpc-router.ts | 136 +++++++++--------- 8 files changed, 159 insertions(+), 98 deletions(-) create mode 100644 apps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.ts create 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 new file mode 100644 index 0000000000..49a5af32fc --- /dev/null +++ b/apps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.ts @@ -0,0 +1,34 @@ +import { cookies, headers } from "next/headers"; +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"; + +export async function checkInsightsPagePermission() { + const featuresRepository = new FeaturesRepository(prisma); + const insightsEnabled = await featuresRepository.checkIfFeatureIsEnabledGlobally("insights"); + + if (!insightsEnabled) { + redirect("/"); + } + + const session = await getServerSession({ req: buildLegacyRequest(await headers(), await cookies()) }); + if (!session?.user?.id) { + redirect("/auth/login"); + } + + const hasPermission = await hasInsightsPermission({ + userId: session.user.id, + organizationId: session.user.org?.id, + }); + + if (!hasPermission) { + redirect("/"); + } + + return session; +} diff --git a/apps/web/app/(use-page-wrapper)/insights/layout.tsx b/apps/web/app/(use-page-wrapper)/insights/layout.tsx index e5518d9fd6..056397f754 100644 --- a/apps/web/app/(use-page-wrapper)/insights/layout.tsx +++ b/apps/web/app/(use-page-wrapper)/insights/layout.tsx @@ -1,34 +1,22 @@ import { getTranslate } from "app/_utils"; -import { notFound } from "next/navigation"; import { CTA_CONTAINER_CLASS_NAME } from "@calcom/features/data-table/lib/utils"; -import { FeaturesRepository } from "@calcom/features/flags/features.repository"; import Shell from "@calcom/features/shell/Shell"; -import { prisma } from "@calcom/prisma"; import UpgradeTipWrapper from "./UpgradeTipWrapper"; export default async function InsightsLayout({ children }: { children: React.ReactNode }) { - const featuresRepository = new FeaturesRepository(prisma); - const insightsEnabled = await featuresRepository.checkIfFeatureIsEnabledGlobally("insights"); - - if (!insightsEnabled) { - return notFound(); - } - const t = await getTranslate(); return ( - <> -
- }> - {children} - -
- +
+ }> + {children} + +
); } diff --git a/apps/web/app/(use-page-wrapper)/insights/page.tsx b/apps/web/app/(use-page-wrapper)/insights/page.tsx index 53f930568d..b40b4a683c 100644 --- a/apps/web/app/(use-page-wrapper)/insights/page.tsx +++ b/apps/web/app/(use-page-wrapper)/insights/page.tsx @@ -1,13 +1,11 @@ import { _generateMetadata } from "app/_utils"; -import { cookies, headers } from "next/headers"; -import { getServerSession } from "@calcom/features/auth/lib/getServerSession"; import prisma from "@calcom/prisma"; -import { buildLegacyRequest } from "@lib/buildLegacyCtx"; - import InsightsPage from "~/insights/insights-view"; +import { checkInsightsPagePermission } from "./checkInsightsPagePermission"; + export const generateMetadata = async () => await _generateMetadata( (t) => t("insights"), @@ -18,7 +16,7 @@ export const generateMetadata = async () => ); const ServerPage = async () => { - const session = await getServerSession({ req: buildLegacyRequest(await headers(), await cookies()) }); + const session = await checkInsightsPagePermission(); const { timeZone } = await prisma.user.findUniqueOrThrow({ where: { id: session?.user.id ?? -1 }, diff --git a/apps/web/app/(use-page-wrapper)/insights/router-position/page.tsx b/apps/web/app/(use-page-wrapper)/insights/router-position/page.tsx index dd85b74bd7..5a6dabcdbf 100644 --- a/apps/web/app/(use-page-wrapper)/insights/router-position/page.tsx +++ b/apps/web/app/(use-page-wrapper)/insights/router-position/page.tsx @@ -2,6 +2,8 @@ import { _generateMetadata } from "app/_utils"; import InsightsVirtualQueuesPage from "~/insights/insights-virtual-queues-view"; +import { checkInsightsPagePermission } from "../checkInsightsPagePermission"; + export const generateMetadata = async () => await _generateMetadata( (t) => t("insights"), @@ -12,5 +14,7 @@ export const generateMetadata = async () => ); export default async function Page() { + await checkInsightsPagePermission(); + return ; } diff --git a/apps/web/app/(use-page-wrapper)/insights/routing/page.tsx b/apps/web/app/(use-page-wrapper)/insights/routing/page.tsx index 72e91224a9..f7faaa3889 100644 --- a/apps/web/app/(use-page-wrapper)/insights/routing/page.tsx +++ b/apps/web/app/(use-page-wrapper)/insights/routing/page.tsx @@ -2,6 +2,8 @@ import { _generateMetadata } from "app/_utils"; import InsightsRoutingPage from "~/insights/insights-routing-view"; +import { checkInsightsPagePermission } from "../checkInsightsPagePermission"; + export const generateMetadata = async () => await _generateMetadata( (t) => t("insights"), @@ -12,5 +14,7 @@ export const generateMetadata = async () => ); export default async function Page() { + await checkInsightsPagePermission(); + return ; } diff --git a/packages/features/insights/server/hasInsightsPermission.ts b/packages/features/insights/server/hasInsightsPermission.ts new file mode 100644 index 0000000000..a28c66a548 --- /dev/null +++ b/packages/features/insights/server/hasInsightsPermission.ts @@ -0,0 +1,39 @@ +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/routing-events.ts b/packages/features/insights/server/routing-events.ts index b426af88de..0c43d7bc39 100644 --- a/packages/features/insights/server/routing-events.ts +++ b/packages/features/insights/server/routing-events.ts @@ -1,4 +1,4 @@ -import { Prisma } from "@prisma/client"; +import type { Prisma } from "@prisma/client"; // eslint-disable-next-line no-restricted-imports import mapKeys from "lodash/mapKeys"; // eslint-disable-next-line no-restricted-imports diff --git a/packages/features/insights/server/trpc-router.ts b/packages/features/insights/server/trpc-router.ts index c16aae8e0a..bdfb0ca394 100644 --- a/packages/features/insights/server/trpc-router.ts +++ b/packages/features/insights/server/trpc-router.ts @@ -13,12 +13,12 @@ import { import { getInsightsBookingService } from "@calcom/lib/di/containers/InsightsBooking"; import { getInsightsRoutingService } from "@calcom/lib/di/containers/InsightsRouting"; import type { PrismaClient } from "@calcom/prisma"; -import { BookingStatus } from "@calcom/prisma/enums"; import authedProcedure from "@calcom/trpc/server/procedures/authedProcedure"; 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"; @@ -43,8 +43,6 @@ interface BuildBaseWhereConditionType { ctx: BuildBaseWhereConditionCtxType; } -const bookingStatusSchema = z.enum(["NO_BOOKING", ...Object.values(BookingStatus)]).optional(); - export const buildBaseWhereCondition = async ({ teamId, eventTypeId, @@ -172,22 +170,6 @@ export const buildBaseWhereCondition = async ({ }; }; -const buildHashMapForUsers = < - T extends { avatarUrl: string | null; id: number; username: string | null; [key: string]: unknown } ->( - usersFromTeam: T[] -) => { - const userHashMap = new Map & { avatarUrl: string }>(); - usersFromTeam.forEach((user) => { - userHashMap.set(user.id, { - ...user, - // TODO: Use AVATAR_FALLBACK when avatar.png endpoint is fased out - avatarUrl: user.avatarUrl || `/${user.username}/avatar.png`, - }); - }); - return userHashMap; -}; - const userBelongsToTeamProcedure = authedProcedure.use(async ({ ctx, next, getRawInput }) => { const parse = UserBelongsToTeamInput.safeParse(await getRawInput()); if (!parse.success) { @@ -254,6 +236,21 @@ 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, @@ -311,8 +308,6 @@ export interface IResultTeamList { isOrg?: boolean; } -const BATCH_SIZE = 1000; // Adjust based on your needs - /** * Helper function to create InsightsBookingService with standardized parameters */ @@ -360,7 +355,7 @@ function createInsightsRoutingService( } export const insightsRouter = router({ - bookingKPIStats: userBelongsToTeamProcedure + bookingKPIStats: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const currentPeriodService = createInsightsBookingService(ctx, input); @@ -460,32 +455,30 @@ export const insightsRouter = router({ }, }; }), - eventTrends: userBelongsToTeamProcedure - .input(bookingRepositoryBaseInputSchema) - .query(async ({ ctx, input }) => { - const { startDate, endDate, timeZone } = input; + eventTrends: insightsPbacProcedure.input(bookingRepositoryBaseInputSchema).query(async ({ ctx, input }) => { + const { startDate, endDate, timeZone } = input; - // Calculate timeView and dateRanges - const timeView = getTimeView(startDate, endDate); - const dateRanges = getDateRanges({ - startDate, - endDate, - timeView, + // Calculate timeView and dateRanges + const timeView = getTimeView(startDate, endDate); + const dateRanges = getDateRanges({ + startDate, + endDate, + timeView, + timeZone, + weekStart: ctx.user.weekStart, + }); + + const insightsBookingService = createInsightsBookingService(ctx, input); + try { + return await insightsBookingService.getEventTrendsStats({ timeZone, - weekStart: ctx.user.weekStart, + dateRanges, }); - - const insightsBookingService = createInsightsBookingService(ctx, input); - try { - return await insightsBookingService.getEventTrendsStats({ - timeZone, - dateRanges, - }); - } catch (e) { - throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); - } - }), - popularEvents: userBelongsToTeamProcedure + } catch (e) { + throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); + } + }), + popularEvents: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -496,7 +489,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - averageEventDuration: userBelongsToTeamProcedure + averageEventDuration: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const { startDate, endDate, timeZone } = input; @@ -536,7 +529,8 @@ export const insightsRouter = router({ for (const booking of allBookings) { const periodStart = dayjs(booking.createdAt).startOf(startOfEndOf).format("ll"); if (resultMap.has(periodStart)) { - const current = resultMap.get(periodStart)!; + const current = resultMap.get(periodStart); + if (!current) continue; current.totalDuration += booking.eventLength || 0; current.count += 1; } @@ -552,7 +546,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithMostCancelledBookings: userBelongsToTeamProcedure + membersWithMostCancelledBookings: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -563,7 +557,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithMostCompletedBookings: userBelongsToTeamProcedure + membersWithMostCompletedBookings: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input, "startTime"); @@ -574,7 +568,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithLeastCompletedBookings: userBelongsToTeamProcedure + membersWithLeastCompletedBookings: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input, "startTime"); @@ -585,7 +579,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithMostBookings: userBelongsToTeamProcedure + membersWithMostBookings: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -596,7 +590,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithLeastBookings: userBelongsToTeamProcedure + membersWithLeastBookings: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -760,7 +754,7 @@ export const insightsRouter = router({ return usersInTeam.map((membership) => membership.user); }), - eventTypeList: userBelongsToTeamProcedure + eventTypeList: insightsPbacProcedure .input( z.object({ teamId: z.coerce.number().nullish(), @@ -786,13 +780,13 @@ export const insightsRouter = router({ return eventTypeList; }), - recentRatings: userBelongsToTeamProcedure + recentRatings: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const insightsBookingService = createInsightsBookingService(ctx, input); return await insightsBookingService.getRecentRatingsStats(); }), - membersWithMostNoShow: userBelongsToTeamProcedure + membersWithMostNoShow: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ input, ctx }) => { const insightsBookingService = createInsightsBookingService(ctx, input); @@ -803,19 +797,19 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - membersWithHighestRatings: userBelongsToTeamProcedure + membersWithHighestRatings: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const insightsBookingService = createInsightsBookingService(ctx, input); return await insightsBookingService.getMembersRatingStats("DESC"); }), - membersWithLowestRatings: userBelongsToTeamProcedure + membersWithLowestRatings: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const insightsBookingService = createInsightsBookingService(ctx, input); return await insightsBookingService.getMembersRatingStats("ASC"); }), - rawData: userBelongsToTeamProcedure + rawData: insightsPbacProcedure .input( bookingRepositoryBaseInputSchema.extend({ limit: z.number().max(100).optional(), @@ -837,10 +831,10 @@ export const insightsRouter = router({ } }), - getRoutingFormsForFilters: userBelongsToTeamProcedure + getRoutingFormsForFilters: insightsPbacProcedure .input(z.object({ userId: z.number().optional(), teamId: z.number().optional(), isAll: z.boolean() })) .query(async ({ ctx, input }) => { - const { userId, teamId, isAll } = input; + const { teamId, isAll } = input; return await RoutingEventsInsights.getRoutingFormsForFilters({ userId: ctx.user.id, teamId, @@ -848,13 +842,13 @@ export const insightsRouter = router({ organizationId: ctx.user.organizationId ?? undefined, }); }), - routingFormsByStatus: userBelongsToTeamProcedure + routingFormsByStatus: insightsPbacProcedure .input(insightsRoutingServiceInputSchema) .query(async ({ ctx, input }) => { const insightsRoutingService = createInsightsRoutingService(ctx, input); return await insightsRoutingService.getRoutingFormStats(); }), - routingFormResponses: userBelongsToTeamProcedure + routingFormResponses: insightsPbacProcedure .input(insightsRoutingServicePaginatedInputSchema) .query(async ({ ctx, input }) => { const insightsRoutingService = createInsightsRoutingService(ctx, input); @@ -864,7 +858,7 @@ export const insightsRouter = router({ offset: input.offset, }); }), - routingFormResponsesForDownload: userBelongsToTeamProcedure + routingFormResponsesForDownload: insightsPbacProcedure .input(insightsRoutingServicePaginatedInputSchema) .query(async ({ ctx, input }) => { const headersPromise = RoutingEventsInsights.getRoutingFormHeaders({ @@ -889,7 +883,7 @@ export const insightsRouter = router({ dataPromise, }); }), - getRoutingFormFieldOptions: userBelongsToTeamProcedure + getRoutingFormFieldOptions: insightsPbacProcedure .input( z.object({ userId: z.number().optional(), @@ -906,7 +900,7 @@ export const insightsRouter = router({ }); return options; }), - failedBookingsByField: userBelongsToTeamProcedure + failedBookingsByField: insightsPbacProcedure .input(insightsRoutingServiceInputSchema) .query(async ({ ctx, input }) => { const insightsRoutingService = createInsightsRoutingService(ctx, input); @@ -916,7 +910,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - routingFormResponsesHeaders: userBelongsToTeamProcedure + routingFormResponsesHeaders: insightsPbacProcedure .input( z.object({ userId: z.number().optional(), @@ -936,7 +930,7 @@ export const insightsRouter = router({ return headers || []; }), - routedToPerPeriod: userBelongsToTeamProcedure + routedToPerPeriod: insightsPbacProcedure .input(routedToPerPeriodInputSchema) .query(async ({ ctx, input }) => { const { period, limit, searchQuery, ...rest } = input; @@ -952,7 +946,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - routedToPerPeriodCsv: userBelongsToTeamProcedure + routedToPerPeriodCsv: insightsPbacProcedure .input(routedToPerPeriodCsvInputSchema) .query(async ({ ctx, input }) => { const { period, searchQuery, ...rest } = input; @@ -985,7 +979,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - getRoutingFunnelData: userBelongsToTeamProcedure + getRoutingFunnelData: insightsPbacProcedure .input(routingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const timeView = getTimeView(input.startDate, input.endDate); @@ -1003,7 +997,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - bookingsByHourStats: userBelongsToTeamProcedure + bookingsByHourStats: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const { timeZone } = input; @@ -1017,7 +1011,7 @@ export const insightsRouter = router({ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); } }), - recentNoShowGuests: userBelongsToTeamProcedure + recentNoShowGuests: insightsPbacProcedure .input(bookingRepositoryBaseInputSchema) .query(async ({ ctx, input }) => { const insightsBookingService = createInsightsBookingService(ctx, input, "startTime");