fix: remove page permission check for /insights (#23945)

* fix: remove page permission check for /insights

* add e2e test
This commit is contained in:
Eunjae Lee
2025-09-19 16:04:33 +00:00
committed by GitHub
parent 6ee752aed8
commit 4e1920471d
5 changed files with 154 additions and 113 deletions
@@ -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;
}
+76 -1
View File
@@ -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");
});
});
@@ -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(),
},
});
};
@@ -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;
}
@@ -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);
eventTrends: userBelongsToTeamProcedure
.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,
});
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);