Code Review Bench PR #25434 - fix: get bookings handler for pbac and fallback roles

packages/features/pbac/domain/repositories/IPermissionRepository.ts

@@ -63,19 +63,23 @@ export interface IPermissionRepository {
 
   /**
    * Gets all team IDs where the user has a specific permission
+   * @param orgId Optional organization ID to scope results to. When provided, only returns teams within this organization.
    */
   getTeamIdsWithPermission(params: {
     userId: number;
     permission: PermissionString;
     fallbackRoles: MembershipRole[];
+    orgId?: number;
   }): Promise<number[]>;
 
   /**
    * Gets all team IDs where the user has all of the specified permissions
+   * @param orgId Optional organization ID to scope results to. When provided, only returns teams within this organization.
    */
   getTeamIdsWithPermissions(params: {
     userId: number;
     permissions: PermissionString[];
     fallbackRoles: MembershipRole[];
+    orgId?: number;
   }): Promise<number[]>;
 }

packages/features/pbac/infrastructure/repositories/PermissionRepository.ts

@@ -212,10 +212,12 @@ export class PermissionRepository implements IPermissionRepository {
     userId,
     permission,
     fallbackRoles,
+    orgId,
   }: {
     userId: number;
     permission: PermissionString;
     fallbackRoles: MembershipRole[];
+    orgId?: number;
   }): Promise<number[]> {
     return this.getTeamIdsWithPermissions({ userId, permissions: [permission], fallbackRoles });
   }
@@ -224,10 +226,12 @@ export class PermissionRepository implements IPermissionRepository {
     userId,
     permissions,
     fallbackRoles,
+    orgId,
   }: {
     userId: number;
     permissions: PermissionString[];
     fallbackRoles: MembershipRole[];
+    orgId?: number;
   }): Promise<number[]> {
     // Validate that permissions array is not empty to prevent privilege escalation
     if (permissions.length === 0) {
@@ -241,8 +245,29 @@ export class PermissionRepository implements IPermissionRepository {
 
     const permissionPairsJson = JSON.stringify(permissionPairs);
 
-    // Teams with PBAC permissions (direct memberships + child teams via org membership)
-    const teamsWithPermissionPromise = this.client.$queryRaw<{ teamId: number }[]>`
+    const [teamsWithPermission, teamsWithFallbackRoles] = await Promise.all([
+      this.getTeamsWithPBACPermissions(userId, permissionPairsJson, permissions.length, orgId),
+      this.getTeamsWithFallbackRoles(userId, fallbackRoles, orgId),
+    ]);
+
+    const pbacTeamIds = teamsWithPermission.map((team) => team.teamId);
+    const fallbackTeamIds = teamsWithFallbackRoles.map((team) => team.teamId);
+
+    const allTeamIds = Array.from(new Set([...pbacTeamIds, ...fallbackTeamIds]));
+    return allTeamIds;
+  }
+
+  /**
+   * Gets teams where user has PBAC permissions (direct memberships + child teams via org membership)
+   * @param orgId Optional organization ID to scope results. When null/undefined, returns all teams.
+   */
+  private async getTeamsWithPBACPermissions(
+    userId: number,
+    permissionPairsJson: string,
+    permissionsCount: number,
+    orgId?: number | null
+  ): Promise<{ teamId: number }[]> {
+    return this.client.$queryRaw<{ teamId: number }[]>`
       WITH required_permissions AS (
         SELECT 
           required_perm->>'resource' as resource,
@@ -252,9 +277,11 @@ export class PermissionRepository implements IPermissionRepository {
       SELECT DISTINCT m."teamId"
       FROM "Membership" m
       INNER JOIN "Role" r ON m."customRoleId" = r.id
+      INNER JOIN "Team" t ON m."teamId" = t.id
       WHERE m."userId" = ${userId}
         AND m."accepted" = true
         AND m."customRoleId" IS NOT NULL
+        AND (${orgId}::bigint IS NULL OR t."id" = ${orgId} OR t."parentId" = ${orgId})
         AND (
           SELECT COUNT(*)
           FROM required_permissions rp_req
@@ -269,7 +296,7 @@ export class PermissionRepository implements IPermissionRepository {
                 (rp."resource" = rp_req.resource AND rp."action" = rp_req.action)
               )
           )
-        ) = ${permissions.length}
+        ) = ${permissionsCount}
       UNION
       SELECT DISTINCT child."id"
       FROM "Membership" m
@@ -279,6 +306,7 @@ export class PermissionRepository implements IPermissionRepository {
       WHERE m."userId" = ${userId}
         AND m."accepted" = true
         AND m."customRoleId" IS NOT NULL
+        AND (${orgId}::bigint IS NULL OR org."id" = ${orgId} OR child."id" = ${orgId})
         AND (
           SELECT COUNT(*)
           FROM required_permissions rp_req
@@ -293,11 +321,20 @@ export class PermissionRepository implements IPermissionRepository {
                 (rp."resource" = rp_req.resource AND rp."action" = rp_req.action)
               )
           )
-        ) = ${permissions.length}
+        ) = ${permissionsCount}
     `;
+  }
 
-    // Teams with fallback roles (direct memberships + child teams via org membership, PBAC disabled)
-    const teamsWithFallbackRolesPromise = this.client.$queryRaw<{ teamId: number }[]>`
+  /**
+   * Gets teams where user has fallback roles (direct memberships + child teams via org membership, PBAC disabled)
+   * @param orgId Optional organization ID to scope results. When null/undefined, returns all teams.
+   */
+  private async getTeamsWithFallbackRoles(
+    userId: number,
+    fallbackRoles: MembershipRole[],
+    orgId?: number | null
+  ): Promise<{ teamId: number }[]> {
+    return this.client.$queryRaw<{ teamId: number }[]>`
       SELECT DISTINCT m."teamId"
       FROM "Membership" m
       INNER JOIN "Team" t ON m."teamId" = t.id
@@ -306,6 +343,7 @@ export class PermissionRepository implements IPermissionRepository {
         AND m."accepted" = true
         AND m."role"::text = ANY(${fallbackRoles})
         AND f."teamId" IS NULL
+        AND (${orgId}::bigint IS NULL OR t."id" = ${orgId} OR t."parentId" = ${orgId})
       UNION
       SELECT DISTINCT child."id"
       FROM "Membership" m
@@ -316,17 +354,7 @@ export class PermissionRepository implements IPermissionRepository {
         AND m."accepted" = true
         AND m."role"::text = ANY(${fallbackRoles})
         AND f."teamId" IS NULL
+        AND (${orgId}::bigint IS NULL OR org."id" = ${orgId} OR child."id" = ${orgId} OR child."parentId" = ${orgId})
     `;
-
-    const [teamsWithPermission, teamsWithFallbackRoles] = await Promise.all([
-      teamsWithPermissionPromise,
-      teamsWithFallbackRolesPromise,
-    ]);
-
-    const pbacTeamIds = teamsWithPermission.map((team) => team.teamId);
-    const fallbackTeamIds = teamsWithFallbackRoles.map((team) => team.teamId);
-
-    const allTeamIds = Array.from(new Set([...pbacTeamIds, ...fallbackTeamIds]));
-    return allTeamIds;
   }
 }

packages/features/pbac/infrastructure/repositories/__tests__/PermissionRepository.integration-test.ts

@@ -969,5 +969,132 @@ describe("PermissionRepository - Integration Tests", () => {
 
       expect(result).toContain(testTeamId);
     });
+
+    it("should filter teams by orgId when provided", async () => {
+      // Create two organizations
+      const org1 = await prisma.team.create({
+        data: {
+          name: `Org 1 ${Date.now()}`,
+          slug: `org1-${Date.now()}`,
+          isOrganization: true,
+        },
+      });
+
+      const org2 = await prisma.team.create({
+        data: {
+          name: `Org 2 ${Date.now()}`,
+          slug: `org2-${Date.now()}`,
+          isOrganization: true,
+        },
+      });
+
+      // Create teams within each organization
+      const team1 = await prisma.team.create({
+        data: {
+          name: `Team 1 ${Date.now()}`,
+          slug: `team1-${Date.now()}`,
+          parentId: org1.id,
+        },
+      });
+
+      const team2 = await prisma.team.create({
+        data: {
+          name: `Team 2 ${Date.now()}`,
+          slug: `team2-${Date.now()}`,
+          parentId: org2.id,
+        },
+      });
+
+      // Create memberships with ADMIN role in both organizations
+      await prisma.membership.create({
+        data: {
+          userId: testUserId,
+          teamId: org1.id,
+          role: MembershipRole.ADMIN,
+          accepted: true,
+        },
+      });
+
+      await prisma.membership.create({
+        data: {
+          userId: testUserId,
+          teamId: org2.id,
+          role: MembershipRole.ADMIN,
+          accepted: true,
+        },
+      });
+
+      // Without orgId, should return both organizations
+      const resultWithoutScope = await repository.getTeamIdsWithPermissions({
+        userId: testUserId,
+        permissions: ["eventType.create"],
+        fallbackRoles: [MembershipRole.ADMIN],
+      });
+
+      expect(resultWithoutScope).toContain(org1.id);
+      expect(resultWithoutScope).toContain(org2.id);
+
+      // With orgId = org1, should only return org1 and its child teams
+      const resultWithScope = await repository.getTeamIdsWithPermissions({
+        userId: testUserId,
+        permissions: ["eventType.create"],
+        fallbackRoles: [MembershipRole.ADMIN],
+        orgId: org1.id,
+      });
+
+      expect(resultWithScope).toContain(org1.id);
+      expect(resultWithScope).toContain(team1.id);
+      expect(resultWithScope).not.toContain(org2.id);
+      expect(resultWithScope).not.toContain(team2.id);
+
+      // Cleanup
+      await prisma.membership.deleteMany({ where: { userId: testUserId } });
+      await prisma.team.deleteMany({ where: { id: { in: [org1.id, org2.id, team1.id, team2.id] } } });
+    });
+
+    it("should include child teams when orgId is provided", async () => {
+      // Create organization
+      const org = await prisma.team.create({
+        data: {
+          name: `Org ${Date.now()}`,
+          slug: `org-${Date.now()}`,
+          isOrganization: true,
+        },
+      });
+
+      // Create child team
+      const childTeam = await prisma.team.create({
+        data: {
+          name: `Child Team ${Date.now()}`,
+          slug: `child-team-${Date.now()}`,
+          parentId: org.id,
+        },
+      });
+
+      // Create membership with ADMIN role in organization
+      await prisma.membership.create({
+        data: {
+          userId: testUserId,
+          teamId: org.id,
+          role: MembershipRole.ADMIN,
+          accepted: true,
+        },
+      });
+
+      // With orgId, should return both org and child team
+      const result = await repository.getTeamIdsWithPermissions({
+        userId: testUserId,
+        permissions: ["eventType.create"],
+        fallbackRoles: [MembershipRole.ADMIN],
+        orgId: org.id,
+      });
+
+      expect(result).toContain(org.id);
+      expect(result).toContain(childTeam.id);
+
+      // Cleanup
+      await prisma.membership.deleteMany({ where: { userId: testUserId } });
+      await prisma.team.deleteMany({ where: { id: { in: [org.id, childTeam.id] } } });
+    });
   });
 });

packages/features/pbac/services/permission-check.service.ts

@@ -3,6 +3,7 @@ import { MembershipRepository } from "@calcom/features/membership/repositories/M
 import logger from "@calcom/lib/logger";
 import prisma from "@calcom/prisma";
 import type { MembershipRole } from "@calcom/prisma/enums";
+import { TRPCError } from "@trpc/server";
 
 import { PermissionMapper } from "../domain/mappers/PermissionMapper";
 import type { PermissionCheck, TeamPermissions } from "../domain/models/Permission";
@@ -136,7 +137,7 @@ export class PermissionCheckService {
           userId,
           teamId: team.parentId,
         });
-        
+
         // Use the highest role between team and org
         if (orgMembership) {
           effectiveRole = this.getHighestRole(effectiveRole, orgMembership.role);
@@ -200,7 +201,7 @@ export class PermissionCheckService {
           userId,
           teamId: team.parentId,
         });
-        
+
         // Use the highest role between team and org
         if (orgMembership) {
           effectiveRole = this.getHighestRole(effectiveRole, orgMembership.role);
@@ -293,10 +294,7 @@ export class PermissionCheckService {
     return allowedRoles.includes(userRole);
   }
 
-  private getHighestRole(
-    role1: MembershipRole | null,
-    role2: MembershipRole | null
-  ): MembershipRole | null {
+  private getHighestRole(role1: MembershipRole | null, role2: MembershipRole | null): MembershipRole | null {
     if (!role1) return role2;
     if (!role2) return role1;
 
@@ -311,15 +309,18 @@ export class PermissionCheckService {
 
   /**
    * Gets all team IDs where the user has a specific permission
+   * @param orgId Optional organization ID to scope results to. When provided, only returns teams within this organization.
    */
   async getTeamIdsWithPermission({
     userId,
     permission,
     fallbackRoles,
+    orgId,
   }: {
     userId: number;
     permission: PermissionString;
     fallbackRoles: MembershipRole[];
+    orgId?: number;
   }): Promise<number[]> {
     try {
       const validationResult = this.permissionService.validatePermission(permission);
@@ -328,7 +329,12 @@ export class PermissionCheckService {
         return [];
       }
 
-      return await this.repository.getTeamIdsWithPermission({ userId, permission, fallbackRoles });
+      return await this.repository.getTeamIdsWithPermission({
+        userId,
+        permission,
+        fallbackRoles,
+        orgId,
+      });
     } catch (error) {
       this.logger.error(error);
       return [];
@@ -337,15 +343,18 @@ export class PermissionCheckService {
 
   /**
    * Gets all team IDs where the user has all of the specified permissions
+   * @param orgId Optional organization ID to scope results to. When provided, only returns teams within this organization.
    */
   async getTeamIdsWithPermissions({
     userId,
     permissions,
     fallbackRoles,
+    orgId,
   }: {
     userId: number;
     permissions: PermissionString[];
     fallbackRoles: MembershipRole[];
+    orgId?: number;
   }): Promise<number[]> {
     try {
       const validationResult = this.permissionService.validatePermissions(permissions);
@@ -354,7 +363,12 @@ export class PermissionCheckService {
         return [];
       }
 
-      return await this.repository.getTeamIdsWithPermissions({ userId, permissions, fallbackRoles });
+      return await this.repository.getTeamIdsWithPermissions({
+        userId,
+        permissions,
+        fallbackRoles,
+        orgId,
+      });
     } catch (error) {
       this.logger.error(error);
       return [];