feat: dogfood pbac internally on IS_CALCOM instances (#23537)

* dogfood pbac

* fix constants mock

* remove is_calcom dependancy - not needed due to ff
This commit is contained in:
sean-brydon
2025-09-03 13:48:07 +01:00
committed by GitHub
parent f4b23b3a03
commit afb54ca048
2 changed files with 153 additions and 3 deletions
@@ -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({
@@ -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
*/