diff --git a/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/__tests__/usePermissions.test.ts b/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/__tests__/usePermissions.test.ts new file mode 100644 index 0000000000..adeda4a091 --- /dev/null +++ b/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/__tests__/usePermissions.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from "vitest"; + +import { usePermissions } from "../usePermissions"; + +describe("usePermissions", () => { + const { getResourcePermissionLevel } = usePermissions(); + + describe("getResourcePermissionLevel", () => { + it("should return 'all' for any resource when *.* permission is present", () => { + const permissions = ["*.*", "eventType.create", "eventType.read"]; + + expect(getResourcePermissionLevel("eventType", permissions)).toBe("all"); + expect(getResourcePermissionLevel("booking", permissions)).toBe("all"); + expect(getResourcePermissionLevel("team", permissions)).toBe("all"); + }); + + it("should return 'all' for resource with all individual permissions", () => { + const permissions = ["eventType.create", "eventType.read", "eventType.update", "eventType.delete"]; + + expect(getResourcePermissionLevel("eventType", permissions)).toBe("all"); + }); + it("should return 'read' for resource with only read permission", () => { + const permissions = ["eventType.read"]; + + expect(getResourcePermissionLevel("eventType", permissions)).toBe("read"); + }); + + it("should return 'none' for resource with no permissions", () => { + const permissions = ["booking.create"]; + + expect(getResourcePermissionLevel("eventType", permissions)).toBe("none"); + }); + + it("should handle * resource correctly", () => { + const permissionsWithAll = ["*.*"]; + const permissionsWithoutAll = ["eventType.read"]; + + expect(getResourcePermissionLevel("*", permissionsWithAll)).toBe("all"); + expect(getResourcePermissionLevel("*", permissionsWithoutAll)).toBe("none"); + }); + + it("should prioritize *.* over individual permissions", () => { + const permissions = ["*.*", "eventType.read"]; // Has global all but only read for eventType individually + + expect(getResourcePermissionLevel("eventType", permissions)).toBe("all"); + }); + }); +}); diff --git a/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/usePermissions.ts b/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/usePermissions.ts index 0a62f552e4..ade70cd4e2 100644 --- a/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/usePermissions.ts +++ b/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/usePermissions.ts @@ -46,6 +46,11 @@ export function usePermissions(): UsePermissionsReturn { const resourceConfig = PERMISSION_REGISTRY[resource as keyof typeof PERMISSION_REGISTRY]; if (!resourceConfig) return "none"; + // Check if global all permissions (*.*) is present + if (permissions.includes("*.*")) { + return "all"; + } + // Filter out internal keys like _resource when checking permissions const allResourcePerms = Object.keys(resourceConfig) .filter((action) => !action.startsWith("_")) diff --git a/packages/features/pbac/infrastructure/repositories/RoleRepository.ts b/packages/features/pbac/infrastructure/repositories/RoleRepository.ts index c0c6a0f008..d54c1de7ae 100644 --- a/packages/features/pbac/infrastructure/repositories/RoleRepository.ts +++ b/packages/features/pbac/infrastructure/repositories/RoleRepository.ts @@ -108,7 +108,7 @@ export class RoleRepository { await trx.rolePermission.deleteMany({ where: { roleId, - AND: permissionChanges.toRemove.map((p) => ({ + OR: permissionChanges.toRemove.map((p) => ({ resource: p.resource, action: p.action, })), diff --git a/packages/features/pbac/services/__tests__/role.service.test.ts b/packages/features/pbac/services/__tests__/role.service.test.ts index 4eeda76e0a..712b3bf276 100644 --- a/packages/features/pbac/services/__tests__/role.service.test.ts +++ b/packages/features/pbac/services/__tests__/role.service.test.ts @@ -67,7 +67,15 @@ describe("RoleService", () => { teamId: roleData.teamId, color: "#000000", type: RoleType.CUSTOM, - permissions: [{ id: "perm-1", resource: "eventType", action: "create" }], + permissions: [ + { + id: "perm-1", + resource: "eventType", + action: "create", + roleId: "new-role", + createdAt: new Date(), + }, + ], createdAt: new Date(), updatedAt: new Date(), }; @@ -215,6 +223,53 @@ describe("RoleService", () => { expect(result).toBeDefined(); }); + it("should properly remove multiple permissions when changing from all to none", async () => { + const roleId = "role-id"; + const permissions = [] as PermissionString[]; // Setting to none + const existingPermissions = [ + { id: "1", roleId, resource: "eventType", action: "create" }, + { id: "2", roleId, resource: "eventType", action: "read" }, + { id: "3", roleId, resource: "eventType", action: "update" }, + { id: "4", roleId, resource: "eventType", action: "delete" }, + ]; + + const permissionChanges = { + toAdd: [], + toRemove: [ + { id: "1", roleId, resource: "eventType", action: "create" }, + { id: "2", roleId, resource: "eventType", action: "read" }, + { id: "3", roleId, resource: "eventType", action: "update" }, + { id: "4", roleId, resource: "eventType", action: "delete" }, + ], + }; + + const role: Role = { + id: roleId, + name: "Test Role", + teamId: 1, + type: RoleType.CUSTOM, + color: "#000000", + permissions: [], + createdAt: new Date(), + updatedAt: new Date(), + }; + + mockRepository.findById.mockResolvedValueOnce(role); + mockRepository.getPermissions.mockResolvedValueOnce(existingPermissions); + mockPermissionDiffService.calculateDiff.mockReturnValueOnce(permissionChanges); + mockRepository.update.mockResolvedValueOnce(role); + + const result = await service.update({ roleId, permissions }); + + expect(mockPermissionDiffService.calculateDiff).toHaveBeenCalledWith(permissions, existingPermissions); + expect(mockRepository.update).toHaveBeenCalledWith(roleId, permissionChanges, { + color: undefined, + name: undefined, + }); + expect(result).toBeDefined(); + expect(result.permissions).toHaveLength(0); + }); + it("should throw error if role does not exist", async () => { const roleId = "non-existent-role"; const permissions = ["eventType.create"] as PermissionString[];