diff --git a/packages/features/pbac/domain/repositories/IPermissionRepository.ts b/packages/features/pbac/domain/repositories/IPermissionRepository.ts index d4f9773830e04d..0ecc6f7c0d8535 100644 --- a/packages/features/pbac/domain/repositories/IPermissionRepository.ts +++ b/packages/features/pbac/domain/repositories/IPermissionRepository.ts @@ -9,7 +9,9 @@ export interface IPermissionRepository { teamId: number; userId: number; customRoleId: string | null; - team_parentId?: number; + team: { + parentId: number | null; + }; } | null>; getMembershipByUserAndTeam( @@ -20,7 +22,9 @@ export interface IPermissionRepository { teamId: number; userId: number; customRoleId: string | null; - team_parentId?: number; + team: { + parentId: number | null; + }; } | null>; getOrgMembership( @@ -45,6 +49,11 @@ export interface IPermissionRepository { resource: Resource ): Promise<(CrudAction | CustomAction)[]>; + /** + * Gets all permissions for a specific resource by role ID + */ + getResourcePermissionsByRoleId(roleId: string, resource: Resource): Promise<(CrudAction | CustomAction)[]>; + /** * Gets all team IDs where the user has a specific permission */ diff --git a/packages/features/pbac/infrastructure/repositories/PermissionRepository.ts b/packages/features/pbac/infrastructure/repositories/PermissionRepository.ts index 75a1764bcb7dcd..52858b4b41956d 100644 --- a/packages/features/pbac/infrastructure/repositories/PermissionRepository.ts +++ b/packages/features/pbac/infrastructure/repositories/PermissionRepository.ts @@ -179,6 +179,23 @@ export class PermissionRepository implements IPermissionRepository { return teamPermissions.map((p) => p.action as CrudAction | CustomAction); } + async getResourcePermissionsByRoleId( + roleId: string, + resource: Resource + ): Promise<(CrudAction | CustomAction)[]> { + const permissions = await this.client.rolePermission.findMany({ + where: { + roleId, + OR: [{ resource }, { resource: Resource.All }], + }, + select: { + action: true, + resource: true, + }, + }); + return permissions.map((p) => p.action as CrudAction | CustomAction); + } + async getTeamIdsWithPermission(userId: number, permission: PermissionString): Promise { return this.getTeamIdsWithPermissions(userId, [permission]); } 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 ce892e8bc9e998..c415a8dddd01e5 100644 --- a/packages/features/pbac/services/__tests__/permission-check.service.test.ts +++ b/packages/features/pbac/services/__tests__/permission-check.service.test.ts @@ -6,6 +6,7 @@ import type { MembershipRole } from "@calcom/prisma/enums"; import type { IPermissionRepository } from "../../domain/repositories/IPermissionRepository"; import type { PermissionString } from "../../domain/types/permission-registry"; +import { Resource } from "../../domain/types/permission-registry"; import { PermissionCheckService } from "../permission-check.service"; import type { PermissionService } from "../permission.service"; @@ -34,6 +35,7 @@ describe("PermissionCheckService", () => { checkRolePermission: vi.fn(), checkRolePermissions: vi.fn(), getResourcePermissions: vi.fn(), + getResourcePermissionsByRoleId: vi.fn(), getTeamIdsWithPermission: vi.fn(), getTeamIdsWithPermissions: vi.fn(), } as MockRepository; @@ -78,6 +80,7 @@ describe("PermissionCheckService", () => { teamId: membership.teamId, userId: membership.userId, customRoleId: membership.customRoleId, + team: { parentId: null }, }); mockRepository.getOrgMembership.mockResolvedValueOnce(null); mockRepository.checkRolePermission.mockResolvedValueOnce(true); @@ -192,6 +195,7 @@ describe("PermissionCheckService", () => { teamId: membership.teamId, userId: membership.userId, customRoleId: membership.customRoleId, + team: { parentId: null }, }); mockRepository.getOrgMembership.mockResolvedValueOnce(null); mockRepository.checkRolePermissions.mockResolvedValueOnce(true); @@ -268,6 +272,7 @@ describe("PermissionCheckService", () => { teamId: membership.teamId, userId: membership.userId, customRoleId: membership.customRoleId, + team: { parentId: null }, }); mockRepository.getOrgMembership.mockResolvedValueOnce(null); mockRepository.checkRolePermissions.mockResolvedValueOnce(false); @@ -379,4 +384,253 @@ describe("PermissionCheckService", () => { expect(mockRepository.getTeamIdsWithPermissions).toHaveBeenCalledWith(1, permissions); }); }); + + describe("getResourcePermissions", () => { + it("should return empty array when PBAC is disabled", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(false); + + const result = await service.getResourcePermissions({ + userId: 1, + teamId: 1, + resource: Resource.EventType, + }); + + expect(result).toEqual([]); + expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledWith(1, "pbac"); + expect(mockRepository.getMembershipByUserAndTeam).not.toHaveBeenCalled(); + }); + + it("should return team-level permissions when PBAC is enabled and no org membership", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + id: 1, + teamId: 1, + userId: 1, + customRoleId: "team_role", + team: { parentId: null }, + }); + mockRepository.getOrgMembership.mockResolvedValueOnce(null); + mockRepository.getResourcePermissionsByRoleId.mockResolvedValueOnce(["create", "read"]); + + const result = await service.getResourcePermissions({ + userId: 1, + teamId: 1, + resource: Resource.EventType, + }); + + expect(result).toEqual(["eventType.create", "eventType.read"]); + expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1); + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenCalledWith( + "team_role", + Resource.EventType + ); + expect(mockRepository.getOrgMembership).not.toHaveBeenCalled(); + }); + + it("should return only team permissions when team has no parentId", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + id: 1, + teamId: 1, + userId: 1, + customRoleId: "team_role", + team: { parentId: null }, + }); + mockRepository.getResourcePermissionsByRoleId.mockResolvedValueOnce(["create", "read", "update"]); + + const result = await service.getResourcePermissions({ + userId: 1, + teamId: 1, + resource: Resource.EventType, + }); + + expect(result).toEqual(["eventType.create", "eventType.read", "eventType.update"]); + expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1); + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenCalledTimes(1); + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenCalledWith( + "team_role", + Resource.EventType + ); + expect(mockRepository.getOrgMembership).not.toHaveBeenCalled(); + }); + + it("should return combined team and org permissions when both exist", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + id: 1, + teamId: 1, + userId: 1, + customRoleId: "team_role", + team: { parentId: 2 }, + }); + mockRepository.getOrgMembership.mockResolvedValueOnce({ + id: 2, + teamId: 2, + userId: 1, + customRoleId: "org_role", + }); + + // Mock team permissions - create implies read + mockRepository.getResourcePermissionsByRoleId + .mockResolvedValueOnce(["create", "read"]) // team permissions + .mockResolvedValueOnce(["update", "delete", "read"]); // org permissions - update/delete imply read + + const result = await service.getResourcePermissions({ + userId: 1, + teamId: 1, + resource: Resource.EventType, + }); + + expect(result).toEqual(["eventType.create", "eventType.read", "eventType.update", "eventType.delete"]); + expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1); + expect(mockRepository.getOrgMembership).toHaveBeenCalledWith(1, 2); + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenCalledTimes(2); + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenNthCalledWith( + 1, + "team_role", + Resource.EventType + ); + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenNthCalledWith( + 2, + "org_role", + Resource.EventType + ); + }); + + it("should deduplicate permissions when team and org have overlapping permissions", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + id: 1, + teamId: 1, + userId: 1, + customRoleId: "team_role", + team: { parentId: 2 }, + }); + mockRepository.getOrgMembership.mockResolvedValueOnce({ + id: 2, + teamId: 2, + userId: 1, + customRoleId: "org_role", + }); + + // Mock overlapping permissions - both include read, update implies read + mockRepository.getResourcePermissionsByRoleId + .mockResolvedValueOnce(["create", "read"]) // team permissions - create implies read + .mockResolvedValueOnce(["read", "update"]); // org permissions - update implies read, explicit read + + const result = await service.getResourcePermissions({ + userId: 1, + teamId: 1, + resource: Resource.EventType, + }); + + expect(result).toEqual(["eventType.create", "eventType.read", "eventType.update"]); + expect(result).toHaveLength(3); // Should not have duplicate "read" + }); + + it("should return only org permissions when team has no custom role", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + id: 1, + teamId: 1, + userId: 1, + customRoleId: null, + team: { parentId: 2 }, + }); + mockRepository.getOrgMembership.mockResolvedValueOnce({ + id: 2, + teamId: 2, + userId: 1, + customRoleId: "org_role", + }); + + // Update and delete imply read access + mockRepository.getResourcePermissionsByRoleId.mockResolvedValueOnce(["update", "delete", "read"]); + + const result = await service.getResourcePermissions({ + userId: 1, + teamId: 1, + resource: Resource.EventType, + }); + + expect(result).toEqual(["eventType.update", "eventType.delete", "eventType.read"]); + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenCalledTimes(1); + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenCalledWith( + "org_role", + Resource.EventType + ); + }); + + it("should return empty array when no membership found", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce(null); + + const result = await service.getResourcePermissions({ + userId: 1, + teamId: 1, + resource: Resource.EventType, + }); + + expect(result).toEqual([]); + expect(mockRepository.getResourcePermissionsByRoleId).not.toHaveBeenCalled(); + }); + + it("should return empty array and log error when repository throws", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + mockRepository.getMembershipByUserAndTeam.mockRejectedValueOnce(new Error("Database error")); + + const result = await service.getResourcePermissions({ + userId: 1, + teamId: 1, + resource: Resource.EventType, + }); + + expect(result).toEqual([]); + }); + + it("should enforce correct hierarchy - org permissions should take precedence over team permissions", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + id: 1, + teamId: 1, + userId: 1, + customRoleId: "admin_team_role", + team: { parentId: 2 }, + }); + mockRepository.getOrgMembership.mockResolvedValueOnce({ + id: 2, + teamId: 2, + userId: 1, + customRoleId: "restricted_org_role", + }); + + // Team role has broad permissions - CUD actions include read + // Org role has restricted permissions (only read) + mockRepository.getResourcePermissionsByRoleId + .mockResolvedValueOnce(["create", "read", "update"]) // team permissions - CRU + .mockResolvedValueOnce(["delete"]); // org permissions - delete only + + const result = await service.getResourcePermissions({ + userId: 1, + teamId: 1, + resource: Resource.EventType, + }); + + // User gets eventType.delete because they have this in the org + expect(result).toEqual(["eventType.create", "eventType.read", "eventType.update", "eventType.delete"]); + + // Verify both team and org permissions were fetched + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenCalledTimes(2); + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenNthCalledWith( + 1, + "admin_team_role", + Resource.EventType + ); + expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenNthCalledWith( + 2, + "restricted_org_role", + Resource.EventType + ); + }); + }); }); diff --git a/packages/features/pbac/services/permission-check.service.ts b/packages/features/pbac/services/permission-check.service.ts index 9063566ed7f1fd..c96120b2d3dd51 100644 --- a/packages/features/pbac/services/permission-check.service.ts +++ b/packages/features/pbac/services/permission-check.service.ts @@ -6,7 +6,12 @@ import type { MembershipRole } from "@calcom/prisma/enums"; import { PermissionMapper } from "../domain/mappers/PermissionMapper"; import type { PermissionCheck, TeamPermissions } from "../domain/models/Permission"; import type { IPermissionRepository } from "../domain/repositories/IPermissionRepository"; -import type { PermissionString, Resource } from "../domain/types/permission-registry"; +import type { + PermissionString, + Resource, + CrudAction, + CustomAction, +} from "../domain/types/permission-registry"; import { PermissionRepository } from "../infrastructure/repositories/PermissionRepository"; import { PermissionService } from "./permission.service"; @@ -53,8 +58,28 @@ export class PermissionCheckService { return []; } - const actions = await this.repository.getResourcePermissions(userId, teamId, resource); - return actions.map((action) => PermissionMapper.toPermissionString({ resource, action })); + const { membership, orgMembership } = await this.getMembership({ userId, teamId }); + const actions = new Set(); + + // Get team-level permissions + if (membership?.customRoleId) { + const teamActions = await this.repository.getResourcePermissionsByRoleId( + membership.customRoleId, + resource + ); + teamActions.forEach((action) => actions.add(action)); + } + + // Get org-level permissions as fallback + if (membership?.team?.parentId && orgMembership?.customRoleId) { + const orgActions = await this.repository.getResourcePermissionsByRoleId( + orgMembership.customRoleId, + resource + ); + orgActions.forEach((action) => actions.add(action)); + } + + return Array.from(actions).map((action) => PermissionMapper.toPermissionString({ resource, action })); } catch (error) { this.logger.error(error); return []; @@ -215,8 +240,8 @@ export class PermissionCheckService { membership = await this.repository.getMembershipByUserAndTeam(query.userId, query.teamId); } - if (membership?.team_parentId) { - orgMembership = await this.repository.getOrgMembership(membership.userId, membership.team_parentId); + if (membership?.team.parentId) { + orgMembership = await this.repository.getOrgMembership(membership.userId, membership.team.parentId); } return { membership, orgMembership };