From c945befb44014ea36136ebb9448efba43ccdc00b Mon Sep 17 00:00:00 2001 From: Alex van Andel Date: Wed, 13 Aug 2025 00:25:28 +0100 Subject: [PATCH] refactor: permission-check mocking behaviour --- .../features/flags/features.repository.ts | 3 +- .../permission-check.service.test.ts | 237 +++++++++--------- .../pbac/services/permission-check.service.ts | 91 ++++--- packages/lib/server/repository/membership.ts | 7 +- 4 files changed, 181 insertions(+), 157 deletions(-) diff --git a/packages/features/flags/features.repository.ts b/packages/features/flags/features.repository.ts index 343ba60852f56c..5f79dbe8bf6c85 100644 --- a/packages/features/flags/features.repository.ts +++ b/packages/features/flags/features.repository.ts @@ -2,6 +2,7 @@ import { Prisma } from "@prisma/client"; import { captureException } from "@sentry/nextjs"; import type { PrismaClient } from "@calcom/prisma"; +import prisma from "@calcom/prisma"; import type { AppFlags, TeamFeatures } from "./config"; import type { IFeaturesRepository } from "./features.repository.interface"; @@ -18,7 +19,7 @@ interface CacheOptions { export class FeaturesRepository implements IFeaturesRepository { private static featuresCache: { data: any[]; expiry: number } | null = null; - constructor(private prismaClient: PrismaClient) {} + constructor(private prismaClient: PrismaClient = prisma) {} private clearCache() { FeaturesRepository.featuresCache = null; 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 96e469ecf2373a..6a2eb1919dfe2a 100644 --- a/packages/features/pbac/services/__tests__/permission-check.service.test.ts +++ b/packages/features/pbac/services/__tests__/permission-check.service.test.ts @@ -1,7 +1,7 @@ -import { vi, type Mock, describe, it, expect, beforeEach } from "vitest"; +import { vi, describe, it, expect, beforeEach } from "vitest"; import type { FeaturesRepository } from "@calcom/features/flags/features.repository"; -import { MembershipRepository } from "@calcom/lib/server/repository/membership"; +import type { MembershipRepository } from "@calcom/lib/server/repository/membership"; import type { MembershipRole } from "@calcom/prisma/enums"; import type { IPermissionRepository } from "../../domain/repositories/IPermissionRepository"; @@ -10,53 +10,44 @@ import { Resource } from "../../domain/types/permission-registry"; import { PermissionCheckService } from "../permission-check.service"; import type { PermissionService } from "../permission.service"; -vi.mock("../../infrastructure/repositories/PermissionRepository"); -vi.mock("@calcom/features/flags/features.repository"); -vi.mock("@calcom/lib/server/repository/membership"); -vi.mock("../permission.service"); - -type MockRepository = { - [K in keyof IPermissionRepository]: Mock; -}; - describe("PermissionCheckService", () => { let service: PermissionCheckService; - let mockRepository: MockRepository; - let mockFeaturesRepository: { checkIfTeamHasFeature: Mock }; - let mockPermissionService: { validatePermission: Mock; validatePermissions: Mock }; + let mockRepository: Partial; + let mockFeaturesRepository: Partial; + let mockPermissionService: Partial; + let mockMembershipRepository: Partial; beforeEach(() => { vi.clearAllMocks(); + mockRepository = { getMembershipByMembershipId: vi.fn(), - getMembershipByUserAndTeam: vi.fn(), getOrgMembership: vi.fn(), - getUserMemberships: vi.fn(), checkRolePermission: vi.fn(), checkRolePermissions: vi.fn(), - getResourcePermissions: vi.fn(), - getResourcePermissionsByRoleId: vi.fn(), + getUserMemberships: vi.fn(), getTeamIdsWithPermission: vi.fn(), getTeamIdsWithPermissions: vi.fn(), - } as MockRepository; - + getMembershipByUserAndTeam: vi.fn(), + getResourcePermissionsByRoleId: vi.fn(), + }; mockFeaturesRepository = { checkIfTeamHasFeature: vi.fn(), }; - + mockMembershipRepository = { + findUniqueByUserIdAndTeamId: vi.fn(), + }; mockPermissionService = { validatePermission: vi.fn().mockReturnValue({ isValid: true }), validatePermissions: vi.fn().mockReturnValue({ isValid: true }), }; - service = new PermissionCheckService( - mockRepository, - mockFeaturesRepository as unknown as FeaturesRepository, - mockPermissionService as unknown as PermissionService - ); - - // Mock MembershipRepository static method - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock) = vi.fn(); + service = new PermissionCheckService({ + repository: mockRepository as IPermissionRepository, + membershipRepository: mockMembershipRepository as MembershipRepository, + featuresRepository: mockFeaturesRepository as FeaturesRepository, + permissionService: mockPermissionService as PermissionService, + }); }); describe("checkPermission", () => { @@ -73,17 +64,17 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({ + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(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(true); + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce(null); + vi.spyOn(mockRepository, "checkRolePermission").mockResolvedValueOnce(true); const result = await service.checkPermission({ userId: 1, @@ -93,7 +84,7 @@ describe("PermissionCheckService", () => { }); expect(result).toBe(true); - expect(MembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({ + expect(mockMembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({ userId: 1, teamId: 1, }); @@ -115,8 +106,8 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(false); + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(false); const result = await service.checkPermission({ userId: 1, @@ -126,7 +117,7 @@ describe("PermissionCheckService", () => { }); expect(result).toBe(true); - expect(MembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({ + expect(mockMembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({ userId: 1, teamId: 1, }); @@ -135,7 +126,7 @@ describe("PermissionCheckService", () => { }); it("should return false if membership not found", async () => { - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(null); + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(null); const result = await service.checkPermission({ userId: 1, @@ -160,8 +151,8 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); const result = await service.checkPermission({ userId: 1, @@ -188,17 +179,17 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({ + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(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(true); + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce(null); + vi.spyOn(mockRepository, "checkRolePermissions").mockResolvedValueOnce(true); const result = await service.checkPermissions({ userId: 1, @@ -208,7 +199,7 @@ describe("PermissionCheckService", () => { }); expect(result).toBe(true); - expect(MembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({ + expect(mockMembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({ userId: 1, teamId: 1, }); @@ -233,8 +224,8 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(false); + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(false); const result = await service.checkPermissions({ userId: 1, @@ -244,7 +235,7 @@ describe("PermissionCheckService", () => { }); expect(result).toBe(true); - expect(MembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({ + expect(mockMembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({ userId: 1, teamId: 1, }); @@ -265,8 +256,8 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); const result = await service.checkPermissions({ userId: 1, @@ -292,7 +283,7 @@ describe("PermissionCheckService", () => { }, ]; - mockRepository.getUserMemberships.mockResolvedValueOnce(memberships); + vi.spyOn(mockRepository, "getUserMemberships").mockResolvedValueOnce(memberships); const result = await service.getUserPermissions(1); expect(result).toEqual(memberships); @@ -303,7 +294,7 @@ describe("PermissionCheckService", () => { describe("getTeamIdsWithPermission", () => { it("should return team IDs where user has the specified permission", async () => { const expectedTeamIds = [1, 2, 3]; - mockRepository.getTeamIdsWithPermission.mockResolvedValueOnce(expectedTeamIds); + vi.spyOn(mockRepository, "getTeamIdsWithPermission").mockResolvedValueOnce(expectedTeamIds); const result = await service.getTeamIdsWithPermission(1, "eventType.read"); @@ -312,7 +303,7 @@ describe("PermissionCheckService", () => { }); it("should return empty array when permission validation fails", async () => { - mockPermissionService.validatePermission.mockReturnValueOnce({ + vi.spyOn(mockPermissionService, "validatePermission").mockReturnValueOnce({ isValid: false, error: "Invalid permissions", }); @@ -324,7 +315,7 @@ describe("PermissionCheckService", () => { }); it("should return empty array and log error when repository throws", async () => { - mockRepository.getTeamIdsWithPermission.mockRejectedValueOnce(new Error("Database error")); + vi.spyOn(mockRepository, "getTeamIdsWithPermission").mockRejectedValueOnce(new Error("Database error")); const result = await service.getTeamIdsWithPermission(1, "eventType.read"); @@ -337,7 +328,7 @@ describe("PermissionCheckService", () => { it("should return team IDs where user has all specified permissions", async () => { const expectedTeamIds = [1, 2]; const permissions: PermissionString[] = ["eventType.read", "eventType.create"]; - mockRepository.getTeamIdsWithPermissions.mockResolvedValueOnce(expectedTeamIds); + vi.spyOn(mockRepository, "getTeamIdsWithPermissions").mockResolvedValueOnce(expectedTeamIds); const result = await service.getTeamIdsWithPermissions(1, permissions); @@ -346,7 +337,7 @@ describe("PermissionCheckService", () => { }); it("should return empty array when permissions validation fails", async () => { - mockPermissionService.validatePermissions.mockReturnValueOnce({ + vi.spyOn(mockPermissionService, "validatePermissions").mockReturnValueOnce({ isValid: false, error: "Invalid permissions", }); @@ -358,7 +349,7 @@ describe("PermissionCheckService", () => { }); it("should return empty array when permissions array is empty", async () => { - mockRepository.getTeamIdsWithPermissions.mockResolvedValueOnce([]); + vi.spyOn(mockRepository, "getTeamIdsWithPermissions").mockResolvedValueOnce([]); const result = await service.getTeamIdsWithPermissions(1, []); @@ -368,7 +359,9 @@ describe("PermissionCheckService", () => { it("should return empty array and log error when repository throws", async () => { const permissions: PermissionString[] = ["eventType.read", "eventType.create"]; - mockRepository.getTeamIdsWithPermissions.mockRejectedValueOnce(new Error("Database error")); + vi.spyOn(mockRepository, "getTeamIdsWithPermissions").mockRejectedValueOnce( + new Error("Database error") + ); const result = await service.getTeamIdsWithPermissions(1, permissions); @@ -379,7 +372,7 @@ describe("PermissionCheckService", () => { describe("getResourcePermissions", () => { it("should return empty array when PBAC is disabled", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(false); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(false); const result = await service.getResourcePermissions({ userId: 1, @@ -393,16 +386,16 @@ describe("PermissionCheckService", () => { }); it("should return team-level permissions when PBAC is enabled and no org membership", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByUserAndTeam").mockResolvedValueOnce({ id: 1, teamId: 1, userId: 1, customRoleId: "team_role", team: { parentId: null }, }); - mockRepository.getOrgMembership.mockResolvedValueOnce(null); - mockRepository.getResourcePermissionsByRoleId.mockResolvedValueOnce(["create", "read"]); + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce(null); + vi.spyOn(mockRepository, "getResourcePermissionsByRoleId").mockResolvedValueOnce(["create", "read"]); const result = await service.getResourcePermissions({ userId: 1, @@ -420,15 +413,19 @@ describe("PermissionCheckService", () => { }); it("should return only team permissions when team has no parentId", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByUserAndTeam").mockResolvedValueOnce({ id: 1, teamId: 1, userId: 1, customRoleId: "team_role", team: { parentId: null }, }); - mockRepository.getResourcePermissionsByRoleId.mockResolvedValueOnce(["create", "read", "update"]); + vi.spyOn(mockRepository, "getResourcePermissionsByRoleId").mockResolvedValueOnce([ + "create", + "read", + "update", + ]); const result = await service.getResourcePermissions({ userId: 1, @@ -447,15 +444,15 @@ describe("PermissionCheckService", () => { }); it("should return combined team and org permissions when both exist", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByUserAndTeam").mockResolvedValueOnce({ id: 1, teamId: 1, userId: 1, customRoleId: "team_role", team: { parentId: 2 }, }); - mockRepository.getOrgMembership.mockResolvedValueOnce({ + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce({ id: 2, teamId: 2, userId: 1, @@ -463,7 +460,7 @@ describe("PermissionCheckService", () => { }); // Mock team permissions - create implies read - mockRepository.getResourcePermissionsByRoleId + vi.spyOn(mockRepository, "getResourcePermissionsByRoleId") .mockResolvedValueOnce(["create", "read"]) // team permissions .mockResolvedValueOnce(["update", "delete", "read"]); // org permissions - update/delete imply read @@ -490,15 +487,15 @@ describe("PermissionCheckService", () => { }); it("should deduplicate permissions when team and org have overlapping permissions", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByUserAndTeam").mockResolvedValueOnce({ id: 1, teamId: 1, userId: 1, customRoleId: "team_role", team: { parentId: 2 }, }); - mockRepository.getOrgMembership.mockResolvedValueOnce({ + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce({ id: 2, teamId: 2, userId: 1, @@ -506,7 +503,7 @@ describe("PermissionCheckService", () => { }); // Mock overlapping permissions - both include read, update implies read - mockRepository.getResourcePermissionsByRoleId + vi.spyOn(mockRepository, "getResourcePermissionsByRoleId") .mockResolvedValueOnce(["create", "read"]) // team permissions - create implies read .mockResolvedValueOnce(["read", "update"]); // org permissions - update implies read, explicit read @@ -521,15 +518,15 @@ describe("PermissionCheckService", () => { }); it("should return only org permissions when team has no custom role", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByUserAndTeam").mockResolvedValueOnce({ id: 1, teamId: 1, userId: 1, customRoleId: null, team: { parentId: 2 }, }); - mockRepository.getOrgMembership.mockResolvedValueOnce({ + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce({ id: 2, teamId: 2, userId: 1, @@ -537,7 +534,11 @@ describe("PermissionCheckService", () => { }); // Update and delete imply read access - mockRepository.getResourcePermissionsByRoleId.mockResolvedValueOnce(["update", "delete", "read"]); + vi.spyOn(mockRepository, "getResourcePermissionsByRoleId").mockResolvedValueOnce([ + "update", + "delete", + "read", + ]); const result = await service.getResourcePermissions({ userId: 1, @@ -554,8 +555,8 @@ describe("PermissionCheckService", () => { }); it("should return empty array when no membership found", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce(null); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByUserAndTeam").mockResolvedValueOnce(null); const result = await service.getResourcePermissions({ userId: 1, @@ -568,8 +569,10 @@ describe("PermissionCheckService", () => { }); it("should return empty array and log error when repository throws", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByUserAndTeam.mockRejectedValueOnce(new Error("Database error")); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByUserAndTeam").mockRejectedValueOnce( + new Error("Database error") + ); const result = await service.getResourcePermissions({ userId: 1, @@ -581,15 +584,15 @@ describe("PermissionCheckService", () => { }); it("should enforce correct hierarchy - org permissions should take precedence over team permissions", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByUserAndTeam").mockResolvedValueOnce({ id: 1, teamId: 1, userId: 1, customRoleId: "admin_team_role", team: { parentId: 2 }, }); - mockRepository.getOrgMembership.mockResolvedValueOnce({ + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce({ id: 2, teamId: 2, userId: 1, @@ -598,7 +601,7 @@ describe("PermissionCheckService", () => { // Team role has broad permissions - CUD actions include read // Org role has restricted permissions (only read) - mockRepository.getResourcePermissionsByRoleId + vi.spyOn(mockRepository, "getResourcePermissionsByRoleId") .mockResolvedValueOnce(["create", "read", "update"]) // team permissions - CRU .mockResolvedValueOnce(["delete"]); // org permissions - delete only @@ -626,8 +629,8 @@ describe("PermissionCheckService", () => { }); it("should expand permissions when user has manage permission", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByUserAndTeam").mockResolvedValueOnce({ id: 1, teamId: 1, userId: 1, @@ -636,7 +639,7 @@ describe("PermissionCheckService", () => { }); // User has manage permission - mockRepository.getResourcePermissionsByRoleId.mockResolvedValueOnce(["manage", "read"]); + vi.spyOn(mockRepository, "getResourcePermissionsByRoleId").mockResolvedValueOnce(["manage", "read"]); const result = await service.getResourcePermissions({ userId: 1, @@ -654,15 +657,15 @@ describe("PermissionCheckService", () => { }); it("should expand permissions when user has manage permission at org level", async () => { - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({ + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByUserAndTeam").mockResolvedValueOnce({ id: 1, teamId: 1, userId: 1, customRoleId: "team_role", team: { parentId: 2 }, }); - mockRepository.getOrgMembership.mockResolvedValueOnce({ + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce({ id: 2, teamId: 2, userId: 1, @@ -670,7 +673,7 @@ describe("PermissionCheckService", () => { }); // Team has basic permissions, org has manage - mockRepository.getResourcePermissionsByRoleId + vi.spyOn(mockRepository, "getResourcePermissionsByRoleId") .mockResolvedValueOnce(["read"]) // team permissions .mockResolvedValueOnce(["manage"]); // org permissions @@ -703,19 +706,19 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({ + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByMembershipId").mockResolvedValueOnce({ id: membership.id, teamId: membership.teamId, userId: membership.userId, customRoleId: membership.customRoleId, team: { parentId: null }, }); - mockRepository.getOrgMembership.mockResolvedValueOnce(null); + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce(null); // User doesn't have explicit permission but has manage permission - mockRepository.checkRolePermission + vi.spyOn(mockRepository, "checkRolePermission") .mockResolvedValueOnce(false) // explicit permission check fails .mockResolvedValueOnce(true); // manage permission check succeeds @@ -744,16 +747,16 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({ + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByMembershipId").mockResolvedValueOnce({ id: membership.id, teamId: membership.teamId, userId: membership.userId, customRoleId: membership.customRoleId, team: { parentId: 2 }, }); - mockRepository.getOrgMembership.mockResolvedValueOnce({ + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce({ id: 2, teamId: 2, userId: 1, @@ -761,7 +764,7 @@ describe("PermissionCheckService", () => { }); // Team level permissions fail, org level manage permission succeeds - mockRepository.checkRolePermission + vi.spyOn(mockRepository, "checkRolePermission") .mockResolvedValueOnce(false) // team explicit permission .mockResolvedValueOnce(false) // team manage permission .mockResolvedValueOnce(true); // org manage permission @@ -792,20 +795,20 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({ + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByMembershipId").mockResolvedValueOnce({ id: membership.id, teamId: membership.teamId, userId: membership.userId, customRoleId: membership.customRoleId, team: { parentId: null }, }); - mockRepository.getOrgMembership.mockResolvedValueOnce(null); + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce(null); // Explicit permissions check fails, but manage permissions succeed - mockRepository.checkRolePermissions.mockResolvedValueOnce(false); - mockRepository.checkRolePermission + vi.spyOn(mockRepository, "checkRolePermissions").mockResolvedValueOnce(false); + vi.spyOn(mockRepository, "checkRolePermission") .mockResolvedValueOnce(true) // eventType.manage .mockResolvedValueOnce(true); // role.manage @@ -834,16 +837,16 @@ describe("PermissionCheckService", () => { updatedAt: new Date(), }; - (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership); - mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); - mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({ + vi.spyOn(mockMembershipRepository, "findUniqueByUserIdAndTeamId").mockResolvedValueOnce(membership); + vi.spyOn(mockFeaturesRepository, "checkIfTeamHasFeature").mockResolvedValueOnce(true); + vi.spyOn(mockRepository, "getMembershipByMembershipId").mockResolvedValueOnce({ id: membership.id, teamId: membership.teamId, userId: membership.userId, customRoleId: membership.customRoleId, team: { parentId: 2 }, }); - mockRepository.getOrgMembership.mockResolvedValueOnce({ + vi.spyOn(mockRepository, "getOrgMembership").mockResolvedValueOnce({ id: 2, teamId: 2, userId: 1, @@ -851,12 +854,12 @@ describe("PermissionCheckService", () => { }); // All explicit permission checks fail - mockRepository.checkRolePermissions + vi.spyOn(mockRepository, "checkRolePermissions") .mockResolvedValueOnce(false) // team permissions .mockResolvedValueOnce(false); // org permissions // Has manage for eventType but not for booking - mockRepository.checkRolePermission + vi.spyOn(mockRepository, "checkRolePermission") .mockResolvedValueOnce(true) // team eventType.manage .mockResolvedValueOnce(false) // team booking.manage .mockResolvedValueOnce(true) // org eventType.manage (duplicate check) diff --git a/packages/features/pbac/services/permission-check.service.ts b/packages/features/pbac/services/permission-check.service.ts index 70566db684c339..6a85eceee5bc6d 100644 --- a/packages/features/pbac/services/permission-check.service.ts +++ b/packages/features/pbac/services/permission-check.service.ts @@ -1,7 +1,6 @@ -import { FeaturesRepository } from "@calcom/features/flags/features.repository"; +import type { FeaturesRepository } from "@calcom/features/flags/features.repository"; import logger from "@calcom/lib/logger"; -import { MembershipRepository } from "@calcom/lib/server/repository/membership"; -import prisma from "@calcom/prisma"; +import type { MembershipRepository } from "@calcom/lib/server/repository/membership"; import type { MembershipRole } from "@calcom/prisma/enums"; import { PermissionMapper } from "../domain/mappers/PermissionMapper"; @@ -14,26 +13,39 @@ import type { CustomAction, } from "../domain/types/permission-registry"; import { PERMISSION_REGISTRY, filterResourceConfig } from "../domain/types/permission-registry"; -import { PermissionRepository } from "../infrastructure/repositories/PermissionRepository"; import { PermissionService } from "./permission.service"; +interface Dependencies { + repository: IPermissionRepository; + featuresRepository: FeaturesRepository; + membershipRepository: MembershipRepository; + permissionService: PermissionService; +} + export class PermissionCheckService { private readonly PBAC_FEATURE_FLAG = "pbac" as const; private readonly logger = logger.getSubLogger({ prefix: ["PermissionCheckService"] }); - private readonly featuresRepository: FeaturesRepository; - private readonly permissionService: PermissionService; - - constructor( - private readonly repository: IPermissionRepository = new PermissionRepository(), - featuresRepository: FeaturesRepository = new FeaturesRepository(prisma), - permissionService: PermissionService = new PermissionService() - ) { - this.featuresRepository = featuresRepository; - this.permissionService = permissionService; + + constructor(private readonly dependencies: Dependencies) {} + + static async build() { + const { FeaturesRepository } = await import("@calcom/features/flags/features.repository"); + const featuresRepository = new FeaturesRepository(); + const { PermissionRepository } = await import("../infrastructure/repositories/PermissionRepository"); + const repository = new PermissionRepository(); + const { MembershipRepository } = await import("@calcom/lib/server/repository/membership"); + const membershipRepository = new MembershipRepository(); + + return new PermissionCheckService({ + repository, + featuresRepository, + membershipRepository, + permissionService: new PermissionService(), + }); } async getUserPermissions(userId: number): Promise { - const memberships = await this.repository.getUserMemberships(userId); + const memberships = await this.dependencies.repository.getUserMemberships(userId); return memberships; } @@ -51,7 +63,7 @@ export class PermissionCheckService { resource: Resource; }): Promise { try { - const isPBACEnabled = await this.featuresRepository.checkIfTeamHasFeature( + const isPBACEnabled = await this.dependencies.featuresRepository.checkIfTeamHasFeature( teamId, this.PBAC_FEATURE_FLAG ); @@ -65,7 +77,7 @@ export class PermissionCheckService { // Get team-level permissions if (membership?.customRoleId) { - const teamActions = await this.repository.getResourcePermissionsByRoleId( + const teamActions = await this.dependencies.repository.getResourcePermissionsByRoleId( membership.customRoleId, resource ); @@ -74,7 +86,7 @@ export class PermissionCheckService { // Get org-level permissions as fallback if (membership?.team?.parentId && orgMembership?.customRoleId) { - const orgActions = await this.repository.getResourcePermissionsByRoleId( + const orgActions = await this.dependencies.repository.getResourcePermissionsByRoleId( orgMembership.customRoleId, resource ); @@ -116,20 +128,20 @@ export class PermissionCheckService { fallbackRoles: MembershipRole[]; }): Promise { try { - const validationResult = this.permissionService.validatePermission(permission); + const validationResult = this.dependencies.permissionService.validatePermission(permission); if (!validationResult.isValid) { this.logger.error(validationResult.error); return false; } - const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({ + const membership = await this.dependencies.membershipRepository.findUniqueByUserIdAndTeamId({ userId, teamId, }); if (!membership) return false; - const isPBACEnabled = await this.featuresRepository.checkIfTeamHasFeature( + const isPBACEnabled = await this.dependencies.featuresRepository.checkIfTeamHasFeature( teamId, this.PBAC_FEATURE_FLAG ); @@ -165,20 +177,20 @@ export class PermissionCheckService { fallbackRoles: MembershipRole[]; }): Promise { try { - const validationResult = this.permissionService.validatePermissions(permissions); + const validationResult = this.dependencies.permissionService.validatePermissions(permissions); if (!validationResult.isValid) { this.logger.error(validationResult.error); return false; } - const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({ + const membership = await this.dependencies.membershipRepository.findUniqueByUserIdAndTeamId({ userId, teamId, }); if (!membership) return false; - const isPBACEnabled = await this.featuresRepository.checkIfTeamHasFeature( + const isPBACEnabled = await this.dependencies.featuresRepository.checkIfTeamHasFeature( teamId, this.PBAC_FEATURE_FLAG ); @@ -207,7 +219,7 @@ export class PermissionCheckService { // First check team-level permissions if (membership?.customRoleId) { - const hasTeamPermission = await this.repository.checkRolePermission( + const hasTeamPermission = await this.dependencies.repository.checkRolePermission( membership.customRoleId, permission ); @@ -216,7 +228,7 @@ export class PermissionCheckService { // Check if user has manage permission for this resource const [resource] = permission.split("."); const managePermission = `${resource}.manage` as PermissionString; - const hasManagePermission = await this.repository.checkRolePermission( + const hasManagePermission = await this.dependencies.repository.checkRolePermission( membership.customRoleId, managePermission ); @@ -227,7 +239,7 @@ export class PermissionCheckService { if (orgMembership?.customRoleId) { const [resource] = permission.split("."); const managePermission = `${resource}.manage` as PermissionString; - return this.repository.checkRolePermission(orgMembership.customRoleId, managePermission); + return this.dependencies.repository.checkRolePermission(orgMembership.customRoleId, managePermission); } return false; @@ -246,7 +258,7 @@ export class PermissionCheckService { // First check team-level permissions if (membership?.customRoleId) { - const hasTeamPermissions = await this.repository.checkRolePermissions( + const hasTeamPermissions = await this.dependencies.repository.checkRolePermissions( membership.customRoleId, permissions ); @@ -258,7 +270,7 @@ export class PermissionCheckService { const [resource] = permission.split("."); if (!resourcesWithManage.has(resource)) { const managePermission = `${resource}.manage` as PermissionString; - const hasManagePermission = await this.repository.checkRolePermission( + const hasManagePermission = await this.dependencies.repository.checkRolePermission( membership.customRoleId, managePermission ); @@ -279,7 +291,7 @@ export class PermissionCheckService { // If no team permissions, check org-level permissions if (orgMembership?.customRoleId) { - const hasOrgPermissions = await this.repository.checkRolePermissions( + const hasOrgPermissions = await this.dependencies.repository.checkRolePermissions( orgMembership.customRoleId, permissions ); @@ -291,7 +303,7 @@ export class PermissionCheckService { const [resource] = permission.split("."); if (!resourcesWithManage.has(resource)) { const managePermission = `${resource}.manage` as PermissionString; - const hasManagePermission = await this.repository.checkRolePermission( + const hasManagePermission = await this.dependencies.repository.checkRolePermission( orgMembership.customRoleId, managePermission ); @@ -316,13 +328,16 @@ export class PermissionCheckService { let orgMembership = null; if (query.membershipId) { - membership = await this.repository.getMembershipByMembershipId(query.membershipId); + membership = await this.dependencies.repository.getMembershipByMembershipId(query.membershipId); } else if (query.userId && query.teamId) { - membership = await this.repository.getMembershipByUserAndTeam(query.userId, query.teamId); + membership = await this.dependencies.repository.getMembershipByUserAndTeam(query.userId, query.teamId); } if (membership?.team.parentId) { - orgMembership = await this.repository.getOrgMembership(membership.userId, membership.team.parentId); + orgMembership = await this.dependencies.repository.getOrgMembership( + membership.userId, + membership.team.parentId + ); } return { membership, orgMembership }; @@ -337,13 +352,13 @@ export class PermissionCheckService { */ async getTeamIdsWithPermission(userId: number, permission: PermissionString): Promise { try { - const validationResult = this.permissionService.validatePermission(permission); + const validationResult = this.dependencies.permissionService.validatePermission(permission); if (!validationResult.isValid) { this.logger.error(validationResult.error); return []; } - return await this.repository.getTeamIdsWithPermission(userId, permission); + return await this.dependencies.repository.getTeamIdsWithPermission(userId, permission); } catch (error) { this.logger.error(error); return []; @@ -355,13 +370,13 @@ export class PermissionCheckService { */ async getTeamIdsWithPermissions(userId: number, permissions: PermissionString[]): Promise { try { - const validationResult = this.permissionService.validatePermissions(permissions); + const validationResult = this.dependencies.permissionService.validatePermissions(permissions); if (!validationResult.isValid) { this.logger.error(validationResult.error); return []; } - return await this.repository.getTeamIdsWithPermissions(userId, permissions); + return await this.dependencies.repository.getTeamIdsWithPermissions(userId, permissions); } catch (error) { this.logger.error(error); return []; diff --git a/packages/lib/server/repository/membership.ts b/packages/lib/server/repository/membership.ts index 82857765ad5994..72258ce865da87 100644 --- a/packages/lib/server/repository/membership.ts +++ b/packages/lib/server/repository/membership.ts @@ -303,7 +303,7 @@ export class MembershipRepository { }); } - static async findUniqueByUserIdAndTeamId({ userId, teamId }: { userId: number; teamId: number }) { + async findUniqueByUserIdAndTeamId({ userId, teamId }: { userId: number; teamId: number }) { return await prisma.membership.findUnique({ where: { userId_teamId: { @@ -314,6 +314,11 @@ export class MembershipRepository { }); } + static async findUniqueByUserIdAndTeamId(passThroughProps: { userId: number; teamId: number }) { + const instance = new MembershipRepository(); + return instance.findUniqueByUserIdAndTeamId(passThroughProps); + } + static async findByTeamIdForAvailability({ teamId }: { teamId: number }) { const memberships = await prisma.membership.findMany({ where: { teamId },