fix: always check team.parentID even if no membership (calcom#23976)

* fix: always check team.parentID even if no membership

* refactor permission handling in create event type handler

* remove hasMembership

---------

Co-authored-by: Morgan <33722304+ThyMinimalDev@users.noreply.github.com>

Author: sean-brydon

packages/features/pbac/domain/repositories/IPermissionRepository.ts

@@ -37,6 +37,11 @@ export interface IPermissionRepository {
     customRoleId: string | null;
   } | null>;
 
+  getTeamById(teamId: number): Promise<{
+    id: number;
+    parentId: number | null;
+  } | null>;
+
   checkRolePermission(roleId: string, permission: PermissionString): Promise<boolean>;
   checkRolePermissions(roleId: string, permissions: PermissionString[]): Promise<boolean>;
 

packages/features/pbac/infrastructure/repositories/PermissionRepository.ts

@@ -94,6 +94,16 @@ export class PermissionRepository implements IPermissionRepository {
     });
   }
 
+  async getTeamById(teamId: number) {
+    return this.client.team.findUnique({
+      where: { id: teamId },
+      select: {
+        id: true,
+        parentId: true,
+      },
+    });
+  }
+
   async checkRolePermission(roleId: string, permission: PermissionString): Promise<boolean> {
     const { resource, action } = parsePermissionString(permission);
     const hasPermission = await this.client.rolePermission.findFirst({

packages/features/pbac/services/__tests__/permission-check.service.test.ts

@@ -37,6 +37,7 @@ describe("PermissionCheckService", () => {
       getMembershipByMembershipId: vi.fn(),
       getMembershipByUserAndTeam: vi.fn(),
       getOrgMembership: vi.fn(),
+      getTeamById: vi.fn(),
       getUserMemberships: vi.fn(),
       checkRolePermission: vi.fn(),
       checkRolePermissions: vi.fn(),
@@ -67,28 +68,14 @@ describe("PermissionCheckService", () => {
 
   describe("checkPermission", () => {
     it("should check permission with PBAC enabled", async () => {
-      const membership = {
+      mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
+      mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({
         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(true);
 
       const result = await service.checkPermission({
@@ -99,12 +86,8 @@ describe("PermissionCheckService", () => {
       });
 
       expect(result).toBe(true);
-      expect(MembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({
-        userId: 1,
-        teamId: 1,
-      });
       expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledWith(1, "pbac");
-      expect(mockRepository.getMembershipByMembershipId).toHaveBeenCalledWith(1);
+      expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1);
       expect(mockRepository.checkRolePermission).toHaveBeenCalledWith("admin_role", "eventType.create");
     });
 
@@ -140,7 +123,8 @@ describe("PermissionCheckService", () => {
       expect(mockRepository.checkRolePermission).not.toHaveBeenCalled();
     });
 
-    it("should return false if membership not found", async () => {
+    it("should return false if membership not found when PBAC disabled", async () => {
+      mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(false);
       (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(null);
 
       const result = await service.checkPermission({
@@ -153,21 +137,47 @@ describe("PermissionCheckService", () => {
       expect(result).toBe(false);
     });
 
-    it("should return false if PBAC enabled but no customRoleId", async () => {
-      const membership = {
+    it("should check org-level permissions when user has no team membership but PBAC is enabled", async () => {
+      mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
+      mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce(null);
+      mockRepository.getTeamById.mockResolvedValueOnce({ id: 1, parentId: 2 });
+      mockRepository.getOrgMembership.mockResolvedValueOnce({
+        id: 100,
+        teamId: 2,
+        userId: 1,
+        customRoleId: "org_admin_role",
+      });
+      mockRepository.checkRolePermission.mockResolvedValueOnce(true);
+
+      const result = await service.checkPermission({
+        userId: 1,
+        teamId: 1,
+        permission: "eventType.create",
+        fallbackRoles: ["ADMIN", "OWNER"],
+      });
+
+      expect(result).toBe(true);
+      expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1);
+      expect(mockRepository.getTeamById).toHaveBeenCalledWith(1);
+      expect(mockRepository.getOrgMembership).toHaveBeenCalledWith(1, 2);
+      expect(mockRepository.checkRolePermission).toHaveBeenCalledWith("org_admin_role", "eventType.create");
+    });
+
+    it("should return false if PBAC enabled but no customRoleId on team or org", async () => {
+      mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
+      mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({
         id: 1,
         teamId: 1,
         userId: 1,
-        accepted: true,
-        role: "ADMIN" as MembershipRole,
         customRoleId: null,
-        disableImpersonation: false,
-        createdAt: new Date(),
-        updatedAt: new Date(),
-      };
-
-      (MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership);
-      mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
+        team: { parentId: 2 },
+      });
+      mockRepository.getOrgMembership.mockResolvedValueOnce({
+        id: 100,
+        teamId: 2,
+        userId: 1,
+        customRoleId: null,
+      });
 
       const result = await service.checkPermission({
         userId: 1,
@@ -182,28 +192,14 @@ describe("PermissionCheckService", () => {
 
   describe("checkPermissions", () => {
     it("should check multiple permissions with PBAC enabled", async () => {
-      const membership = {
+      mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
+      mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({
         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.checkRolePermissions.mockResolvedValueOnce(true);
 
       const result = await service.checkPermissions({
@@ -214,12 +210,8 @@ describe("PermissionCheckService", () => {
       });
 
       expect(result).toBe(true);
-      expect(MembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({
-        userId: 1,
-        teamId: 1,
-      });
       expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledWith(1, "pbac");
-      expect(mockRepository.getMembershipByMembershipId).toHaveBeenCalledWith(1);
+      expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1);
       expect(mockRepository.checkRolePermissions).toHaveBeenCalledWith("admin_role", [
         "eventType.create",
         "team.invite",
@@ -259,25 +251,12 @@ describe("PermissionCheckService", () => {
     });
 
     it("should return false when permissions array is empty", async () => {
-      const membership = {
+      mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
+      mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({
         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({
-        id: membership.id,
-        teamId: membership.teamId,
-        userId: membership.userId,
-        customRoleId: membership.customRoleId,
         team: { parentId: null },
       });
       mockRepository.getOrgMembership.mockResolvedValueOnce(null);
@@ -293,6 +272,35 @@ describe("PermissionCheckService", () => {
       expect(result).toBe(false);
       expect(mockRepository.checkRolePermissions).toHaveBeenCalledWith("admin_role", []);
     });
+
+    it("should check org-level permissions when user has no team membership with checkPermissions", async () => {
+      mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
+      mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce(null);
+      mockRepository.getTeamById.mockResolvedValueOnce({ id: 1, parentId: 2 });
+      mockRepository.getOrgMembership.mockResolvedValueOnce({
+        id: 100,
+        teamId: 2,
+        userId: 1,
+        customRoleId: "org_admin_role",
+      });
+      mockRepository.checkRolePermissions.mockResolvedValueOnce(true);
+
+      const result = await service.checkPermissions({
+        userId: 1,
+        teamId: 1,
+        permissions: ["eventType.create", "team.invite"],
+        fallbackRoles: ["ADMIN", "OWNER"],
+      });
+
+      expect(result).toBe(true);
+      expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1);
+      expect(mockRepository.getTeamById).toHaveBeenCalledWith(1);
+      expect(mockRepository.getOrgMembership).toHaveBeenCalledWith(1, 2);
+      expect(mockRepository.checkRolePermissions).toHaveBeenCalledWith("org_admin_role", [
+        "eventType.create",
+        "team.invite",
+      ]);
+    });
   });
 
   describe("getUserPermissions", () => {
@@ -392,6 +400,33 @@ describe("PermissionCheckService", () => {
   });
 
   describe("getResourcePermissions", () => {
+    it("should return org permissions when user has no team membership but has org membership", async () => {
+      mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
+      mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce(null);
+      mockRepository.getTeamById.mockResolvedValueOnce({ id: 1, parentId: 2 });
+      mockRepository.getOrgMembership.mockResolvedValueOnce({
+        id: 100,
+        teamId: 2,
+        userId: 1,
+        customRoleId: "org_role",
+      });
+      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.getTeamById).toHaveBeenCalledWith(1);
+      expect(mockRepository.getOrgMembership).toHaveBeenCalledWith(1, 2);
+      expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenCalledWith(
+        "org_role",
+        Resource.EventType
+      );
+    });
     it("should return empty array when PBAC is disabled", async () => {
       mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(false);
 

packages/features/pbac/services/permission-check.service.ts

@@ -71,8 +71,8 @@ export class PermissionCheckService {
         teamActions.forEach((action) => actions.add(action));
       }
 
-      // Get org-level permissions as fallback
-      if (membership?.team?.parentId && orgMembership?.customRoleId) {
+      // Get org-level permissions (works even without team membership)
+      if (orgMembership?.customRoleId) {
         const orgActions = await this.repository.getResourcePermissionsByRoleId(
           orgMembership.customRoleId,
           resource
@@ -108,27 +108,23 @@ export class PermissionCheckService {
         return false;
       }
 
-      const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({
-        userId,
-        teamId,
-      });
-
-      if (!membership) return false;
-
       const isPBACEnabled = await this.featuresRepository.checkIfTeamHasFeature(
         teamId,
         this.PBAC_FEATURE_FLAG
       );
 
       if (isPBACEnabled) {
-        if (!membership.customRoleId) {
-          this.logger.info(`PBAC is enabled for ${teamId} but no custom role is set on membership relation`);
-          return false;
-        }
-
-        return this.hasPermission({ membershipId: membership.id }, permission);
+        // Check if user has permission through team or org membership
+        return this.hasPermission({ userId, teamId }, permission);
       }
 
+      // Fallback to role-based check only if user has team membership
+      const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({
+        userId,
+        teamId,
+      });
+
+      if (!membership) return false;
       return this.checkFallbackRoles(membership.role, fallbackRoles);
     } catch (error) {
       this.logger.error(error);
@@ -157,27 +153,23 @@ export class PermissionCheckService {
         return false;
       }
 
-      const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({
-        userId,
-        teamId,
-      });
-
-      if (!membership) return false;
-
       const isPBACEnabled = await this.featuresRepository.checkIfTeamHasFeature(
         teamId,
         this.PBAC_FEATURE_FLAG
       );
 
       if (isPBACEnabled) {
-        if (!membership.customRoleId) {
-          this.logger.info(`PBAC is enabled for ${teamId} but no custom role is set on membership relation`);
-          return false;
-        }
-
-        return this.hasPermissions({ membershipId: membership.id }, permissions);
+        // Check if user has permissions through team or org membership
+        return this.hasPermissions({ userId, teamId }, permissions);
       }
 
+      // Fallback to role-based check only if user has team membership
+      const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({
+        userId,
+        teamId,
+      });
+
+      if (!membership) return false;
       return this.checkFallbackRoles(membership.role, fallbackRoles);
     } catch (error) {
       this.logger.error(error);
@@ -241,8 +233,16 @@ export class PermissionCheckService {
       membership = await this.repository.getMembershipByUserAndTeam(query.userId, query.teamId);
     }
 
+    // Get org membership either through the team membership or directly from teamId
     if (membership?.team.parentId) {
+      // User has team membership, check org through that
       orgMembership = await this.repository.getOrgMembership(membership.userId, membership.team.parentId);
+    } else if (query.userId && query.teamId) {
+      // No team membership, but check if team belongs to an org
+      const team = await this.repository.getTeamById(query.teamId);
+      if (team?.parentId) {
+        orgMembership = await this.repository.getOrgMembership(query.userId, team.parentId);
+      }
     }
 
     return { membership, orgMembership };