diff --git a/packages/features/pbac/services/__tests__/permission-check.service.test.ts b/packages/features/pbac/services/__tests__/permission-check.service.test.ts index c415a8dddd01e5..577d2221c4f4ad 100644 --- a/packages/features/pbac/services/__tests__/permission-check.service.test.ts +++ b/packages/features/pbac/services/__tests__/permission-check.service.test.ts @@ -147,6 +147,43 @@ describe("PermissionCheckService", () => { expect(result).toBe(false); }); + it("should fallback to legacy roles when PBAC permission check fails (dogfood)", async () => { + const membership = { + id: 1, + teamId: 1, + userId: 1, + accepted: true, + role: "ADMIN" as MembershipRole, + customRoleId: "admin_role", + disableImpersonation: false, + createdAt: new Date(), + updatedAt: new Date(), + }; + + (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({ + id: membership.id, + teamId: membership.teamId, + userId: membership.userId, + customRoleId: membership.customRoleId, + team: { parentId: null }, + }); + mockRepository.getOrgMembership.mockResolvedValueOnce(null); + mockRepository.checkRolePermission.mockResolvedValueOnce(false); // Permission check fails + + const result = await service.checkPermission({ + userId: 1, + teamId: 1, + permission: "eventType.create", + fallbackRoles: ["ADMIN", "OWNER"], + }); + + // Should fallback to role check and pass since user is ADMIN + expect(result).toBe(true); + expect(mockRepository.checkRolePermission).toHaveBeenCalled(); + }); + it("should return false if PBAC enabled but no customRoleId", async () => { const membership = { id: 1, @@ -252,7 +289,7 @@ describe("PermissionCheckService", () => { expect(mockRepository.checkRolePermissions).not.toHaveBeenCalled(); }); - it("should return false when permissions array is empty", async () => { + it("should fallback to legacy roles when PBAC permissions check fails (dogfood)", async () => { const membership = { id: 1, teamId: 1, @@ -265,6 +302,43 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; + (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({ + id: membership.id, + teamId: membership.teamId, + userId: membership.userId, + customRoleId: membership.customRoleId, + team: { parentId: null }, + }); + mockRepository.getOrgMembership.mockResolvedValueOnce(null); + mockRepository.checkRolePermissions.mockResolvedValueOnce(false); // Permissions check fails + + const result = await service.checkPermissions({ + userId: 1, + teamId: 1, + permissions: ["eventType.create", "team.invite"], + fallbackRoles: ["ADMIN", "OWNER"], + }); + + // Should fallback to role check and pass since user is ADMIN + expect(result).toBe(true); + expect(mockRepository.checkRolePermissions).toHaveBeenCalled(); + }); + + it("should return false when permissions array is empty", async () => { + const membership = { + id: 1, + teamId: 1, + userId: 1, + accepted: true, + role: "MEMBER" as MembershipRole, // Change to MEMBER so fallback also fails + customRoleId: "admin_role", + disableImpersonation: false, + createdAt: new Date(), + updatedAt: new Date(), + }; + (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({ diff --git a/packages/features/pbac/services/permission-check.service.ts b/packages/features/pbac/services/permission-check.service.ts index 8b9c67d2e54409..3c9ab0236d03d1 100644 --- a/packages/features/pbac/services/permission-check.service.ts +++ b/packages/features/pbac/services/permission-check.service.ts @@ -16,6 +16,8 @@ import type { import { PermissionRepository } from "../infrastructure/repositories/PermissionRepository"; import { PermissionService } from "./permission.service"; +const DOGFOOD_PBAC_INTERNALLY = true; + export class PermissionCheckService { private readonly PBAC_FEATURE_FLAG = "pbac" as const; private readonly logger = logger.getSubLogger({ prefix: ["PermissionCheckService"] }); @@ -126,7 +128,21 @@ export class PermissionCheckService { return false; } - return this.hasPermission({ membershipId: membership.id }, permission); + const hasPbacPermission = await this.hasPermission({ membershipId: membership.id }, permission); + + if (DOGFOOD_PBAC_INTERNALLY && !hasPbacPermission) { + return this.dogfoodFallback({ + userId, + teamId, + membershipId: membership.id, + permissions: [permission], + hasPbacPermission, + fallbackRoles, + membershipRole: membership.role, + }); + } + + return hasPbacPermission; } return this.checkFallbackRoles(membership.role, fallbackRoles); @@ -175,7 +191,21 @@ export class PermissionCheckService { return false; } - return this.hasPermissions({ membershipId: membership.id }, permissions); + const hasPbacPermission = await this.hasPermissions({ membershipId: membership.id }, permissions); + + if (DOGFOOD_PBAC_INTERNALLY && !hasPbacPermission) { + return this.dogfoodFallback({ + userId, + teamId, + membershipId: membership.id, + permissions, + hasPbacPermission, + fallbackRoles, + membershipRole: membership.role, + }); + } + + return hasPbacPermission; } return this.checkFallbackRoles(membership.role, fallbackRoles); @@ -252,6 +282,52 @@ export class PermissionCheckService { return allowedRoles.includes(userRole); } + /** + * Internal dogfooding fallback for PBAC permissions + * + * This method is used internally at Cal.com to help us transition to PBAC. + * When PBAC is enabled but a user doesn't have the necessary permission strings, + * we fall back to checking their legacy fallback roles to avoid breaking existing workflows. + * + * An alert is logged to Axiom when this fallback occurs so we can identify and fix + * missing permissions in our PBAC configuration. + * + * @private + * @param params - Object containing userId, teamId, membershipId, permissions, hasPbacPermission, fallbackRoles, and membershipRole + * @returns boolean - Whether the user has permission via fallback roles + */ + private dogfoodFallback(params: { + userId: number; + teamId: number; + membershipId: number; + permissions: PermissionString[]; + hasPbacPermission: boolean; + fallbackRoles: MembershipRole[]; + membershipRole: MembershipRole; + }): boolean { + const { userId, teamId, membershipId, permissions, hasPbacPermission, fallbackRoles, membershipRole } = + params; + + const debugInfo = { + userId, + teamId, + membershipId, + permissions, + hasPbacPermission, + fallbackRoles, + }; + + this.logger.warn( + `PBAC INTERNAL - Failed but user doesnt have permission string to carry out this action. Falling back to fallback roles. \n JSON${JSON.stringify( + debugInfo, + null, + 2 + )}` + ); + + return this.checkFallbackRoles(membershipRole, fallbackRoles); + } + /** * Gets all team IDs where the user has a specific permission */