Skip to content

Commit 7242048

Browse files
authored
fix: Add PBAC permission checks for insights access (calcom#25381)
* fix: Add PBAC permission checks for insights access - Add checkInsightsPermission() helper that properly checks insights.read permission with PBAC support - Update userBelongsToTeamProcedure to use PBAC-aware permission check for org-level access - Update teamListForUser query to filter teams based on insights.read permission instead of only checking base ADMIN/OWNER roles - Maintain backward compatibility with fallback to traditional role checks (ADMIN/OWNER) when PBAC is disabled - Org admins (base role ADMIN/OWNER) continue to have automatic insights access as a privileged position - Team-level admins with custom roles now properly checked for insights.read permission Fixes issue where users with custom PBAC roles couldn't access insights even if they had insights.read permission. Related: CAL-XXXX * perf: Optimize team permission checks to avoid N+1 queries Replace individual permission checks per team with bulk query using getTeamIdsWithPermission(). This reduces database queries from N (one per team) to a single optimized query. - Use PermissionCheckService.getTeamIdsWithPermission() for bulk permission checking - Filter teams based on returned team IDs instead of individual checks - Maintains same functionality with significantly better performance for users with many teams * perf: Fetch only teams with insights access instead of filtering after Move permission check before team query to filter at database level. Previously fetched all teams then filtered in JavaScript. Now only fetches teams the user has insights access to. - Check permissions first using getTeamIdsWithPermission() - Add teamId filter to membership query (teamId: { in: teamIdsWithAccess }) - Remove JavaScript filter step (done at DB level) - Reduces data transfer and improves query efficiency
1 parent f136b71 commit 7242048

1 file changed

Lines changed: 31 additions & 27 deletions

File tree

packages/features/insights/server/trpc-router.ts

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@ import {
1515
routedToPerPeriodCsvInputSchema,
1616
bookingRepositoryBaseInputSchema,
1717
} from "@calcom/features/insights/server/raw-data.schema";
18+
import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
1819
import type { PrismaClient } from "@calcom/prisma";
1920
import type { Prisma } from "@calcom/prisma/client";
21+
import { MembershipRole } from "@calcom/prisma/enums";
2022
import authedProcedure from "@calcom/trpc/server/procedures/authedProcedure";
2123
import { router } from "@calcom/trpc/server/trpc";
2224

@@ -213,17 +215,8 @@ const userBelongsToTeamProcedure = authedProcedure.use(async ({ ctx, next, getRa
213215
}
214216
}
215217

216-
const membershipOrg = await ctx.insightsDb.membership.findFirst({
217-
where: {
218-
userId: ctx.user.id,
219-
teamId: ctx.user.organizationId,
220-
accepted: true,
221-
role: {
222-
in: ["OWNER", "ADMIN"],
223-
},
224-
},
225-
});
226-
if (!membershipOrg) {
218+
const hasOrgAccess = await checkInsightsPermission(ctx.user.id, ctx.user.organizationId);
219+
if (!hasOrgAccess) {
227220
throw new TRPCError({ code: "UNAUTHORIZED" });
228221
}
229222
isOwnerAdminOfParentTeam = true;
@@ -247,6 +240,16 @@ const userSelect = {
247240
avatarUrl: true,
248241
};
249242

243+
async function checkInsightsPermission(userId: number, teamId: number): Promise<boolean> {
244+
const permissionCheckService = new PermissionCheckService();
245+
return await permissionCheckService.checkPermission({
246+
userId,
247+
teamId,
248+
permission: "insights.read",
249+
fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN],
250+
});
251+
}
252+
250253
const emptyResponseBookingKPIStats = {
251254
empty: true,
252255
created: {
@@ -651,22 +654,29 @@ export const insightsRouter = router({
651654
return result;
652655
}
653656

654-
// Look if user it's admin/owner in multiple teams
657+
// Get all team IDs where user has insights.read permission in a single optimized query
658+
// This properly handles both PBAC permissions and traditional role-based access
659+
const permissionCheckService = new PermissionCheckService();
660+
const teamIdsWithAccess = await permissionCheckService.getTeamIdsWithPermission({
661+
userId: user.id,
662+
permission: "insights.read",
663+
fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN],
664+
});
665+
666+
if (teamIdsWithAccess.length === 0) {
667+
return [];
668+
}
669+
670+
// Fetch only teams where user has both membership AND insights access
671+
// This avoids fetching unnecessary team data by filtering at the database level
655672
const belongsToTeams = await ctx.insightsDb.membership.findMany({
656673
where: {
657674
team: {
658675
slug: { not: null },
659676
},
660677
accepted: true,
661678
userId: user.id,
662-
OR: [
663-
{
664-
role: "ADMIN",
665-
},
666-
{
667-
role: "OWNER",
668-
},
669-
],
679+
teamId: { in: teamIdsWithAccess },
670680
},
671681
include: {
672682
team: {
@@ -681,13 +691,7 @@ export const insightsRouter = router({
681691
},
682692
});
683693

684-
if (belongsToTeams.length === 0) {
685-
return [];
686-
}
687-
688-
const result: IResultTeamList[] = belongsToTeams.map((membership) => {
689-
return { ...membership.team };
690-
});
694+
const result: IResultTeamList[] = belongsToTeams.map((membership) => ({ ...membership.team }));
691695

692696
return result;
693697
}),

0 commit comments

Comments
 (0)