fix: Always enforce one owner with pbac (#24144)

* Always enforce one owner * updates from merge * remove redudant eslint rules * Update packages/features/pbac/services/pbac-role-manager.service.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2025-10-01 11:06:18 +01:00
parent 83bf717d6d
commit e3742dbc66
3 changed files with 260 additions and 7 deletions
@@ -19,6 +19,8 @@ vi.mock("@calcom/prisma", () => ({
  prisma: {
    membership: {
      update: vi.fn(),
+      findUnique: vi.fn(),
+      count: vi.fn(),
    },
  },
 }));
@@ -129,6 +131,19 @@ describe("RoleManagementFactory", () => {

    describe("assignRole", () => {
      it("should assign default role correctly", async () => {
+        // Mock non-owner membership to bypass owner validation
+        vi.mocked(prisma.membership.findUnique).mockResolvedValue({
+          id: membershipId,
+          userId,
+          teamId: organizationId,
+          role: MembershipRole.MEMBER,
+          customRoleId: null,
+          accepted: true,
+          disableImpersonation: false,
+          createdAt: new Date(),
+          updatedAt: new Date(),
+        });
+
        mockRoleService.assignRoleToMember.mockResolvedValue(undefined);
        const manager = await factory.createRoleManager(organizationId);
        await manager.assignRole(userId, organizationId, MembershipRole.ADMIN, membershipId);
@@ -140,6 +155,20 @@ describe("RoleManagementFactory", () => {

      it("should assign custom role after validation", async () => {
        const customRoleId = "custom-role-123";
+
+        // Mock non-owner membership to bypass owner validation
+        vi.mocked(prisma.membership.findUnique).mockResolvedValue({
+          id: membershipId,
+          userId,
+          teamId: organizationId,
+          role: MembershipRole.MEMBER,
+          customRoleId: null,
+          accepted: true,
+          disableImpersonation: false,
+          createdAt: new Date(),
+          updatedAt: new Date(),
+        });
+
        mockRoleService.roleBelongsToTeam.mockResolvedValue(true);
        mockRoleService.assignRoleToMember.mockResolvedValue(undefined);

@@ -154,6 +183,19 @@ describe("RoleManagementFactory", () => {
        const customRoleId = "invalid-role";
        mockRoleService.roleBelongsToTeam.mockResolvedValue(false);

+        // Mock non-owner membership to bypass owner validation
+        vi.mocked(prisma.membership.findUnique).mockResolvedValue({
+          id: membershipId,
+          userId,
+          teamId: organizationId,
+          role: MembershipRole.MEMBER,
+          customRoleId: null,
+          accepted: true,
+          disableImpersonation: false,
+          createdAt: new Date(),
+          updatedAt: new Date(),
+        });
+
        const manager = await factory.createRoleManager(organizationId);
        await expect(
          manager.assignRole(userId, organizationId, customRoleId as MembershipRole, membershipId)
@@ -161,6 +203,161 @@ describe("RoleManagementFactory", () => {
          new RoleManagementError("You do not have access to this role", RoleManagementErrorCode.INVALID_ROLE)
        );
      });
+
+      it("should prevent changing the last owner to non-owner role", async () => {
+        // Mock current membership as owner with customRoleId
+        vi.mocked(prisma.membership.findUnique).mockResolvedValue({
+          id: membershipId,
+          userId,
+          teamId: organizationId,
+          role: MembershipRole.MEMBER,
+          customRoleId: DEFAULT_ROLE_IDS[MembershipRole.OWNER],
+          accepted: true,
+          disableImpersonation: false,
+          createdAt: new Date(),
+          updatedAt: new Date(),
+        });
+
+        // Mock count showing only 1 owner
+        vi.mocked(prisma.membership.count).mockResolvedValue(1);
+
+        const manager = await factory.createRoleManager(organizationId);
+        await expect(
+          manager.assignRole(userId, organizationId, MembershipRole.ADMIN, membershipId)
+        ).rejects.toThrow(
+          new RoleManagementError(
+            "Cannot change the role of the last owner in the organization",
+            RoleManagementErrorCode.UNAUTHORIZED
+          )
+        );
+      });
+
+      it("should prevent changing the last legacy owner to non-owner role", async () => {
+        // Mock current membership as legacy owner (role field)
+        vi.mocked(prisma.membership.findUnique).mockResolvedValue({
+          id: membershipId,
+          userId,
+          teamId: organizationId,
+          role: MembershipRole.OWNER,
+          customRoleId: null,
+          accepted: true,
+          disableImpersonation: false,
+          createdAt: new Date(),
+          updatedAt: new Date(),
+        });
+
+        // Mock count showing only 1 owner
+        vi.mocked(prisma.membership.count).mockResolvedValue(1);
+
+        const manager = await factory.createRoleManager(organizationId);
+        await expect(
+          manager.assignRole(userId, organizationId, MembershipRole.ADMIN, membershipId)
+        ).rejects.toThrow(
+          new RoleManagementError(
+            "Cannot change the role of the last owner in the organization",
+            RoleManagementErrorCode.UNAUTHORIZED
+          )
+        );
+      });
+
+      it("should allow changing owner role when multiple owners exist", async () => {
+        // Mock current membership as owner
+        vi.mocked(prisma.membership.findUnique).mockResolvedValue({
+          id: membershipId,
+          userId,
+          teamId: organizationId,
+          role: MembershipRole.MEMBER,
+          customRoleId: DEFAULT_ROLE_IDS[MembershipRole.OWNER],
+          accepted: true,
+          disableImpersonation: false,
+          createdAt: new Date(),
+          updatedAt: new Date(),
+        });
+
+        // Mock count showing multiple owners
+        vi.mocked(prisma.membership.count).mockResolvedValue(2);
+        mockRoleService.assignRoleToMember.mockResolvedValue(undefined);
+
+        const manager = await factory.createRoleManager(organizationId);
+        await expect(
+          manager.assignRole(userId, organizationId, MembershipRole.ADMIN, membershipId)
+        ).resolves.not.toThrow();
+
+        expect(mockRoleService.assignRoleToMember).toHaveBeenCalledWith(
+          DEFAULT_ROLE_IDS[MembershipRole.ADMIN],
+          membershipId
+        );
+      });
+
+      it("should allow changing owner to owner (same role)", async () => {
+        // Mock current membership as owner
+        vi.mocked(prisma.membership.findUnique).mockResolvedValue({
+          id: membershipId,
+          userId,
+          teamId: organizationId,
+          role: MembershipRole.MEMBER,
+          customRoleId: DEFAULT_ROLE_IDS[MembershipRole.OWNER],
+          accepted: true,
+          disableImpersonation: false,
+          createdAt: new Date(),
+          updatedAt: new Date(),
+        });
+
+        // Mock count showing only 1 owner (should be bypassed)
+        vi.mocked(prisma.membership.count).mockResolvedValue(1);
+        mockRoleService.assignRoleToMember.mockResolvedValue(undefined);
+
+        const manager = await factory.createRoleManager(organizationId);
+        await expect(
+          manager.assignRole(userId, organizationId, MembershipRole.OWNER, membershipId)
+        ).resolves.not.toThrow();
+
+        expect(mockRoleService.assignRoleToMember).toHaveBeenCalledWith(
+          DEFAULT_ROLE_IDS[MembershipRole.OWNER],
+          membershipId
+        );
+      });
+
+      it("should allow changing non-owner role without validation", async () => {
+        // Mock current membership as non-owner
+        vi.mocked(prisma.membership.findUnique).mockResolvedValue({
+          id: membershipId,
+          userId,
+          teamId: organizationId,
+          role: MembershipRole.MEMBER,
+          customRoleId: null,
+          accepted: true,
+          disableImpersonation: false,
+          createdAt: new Date(),
+          updatedAt: new Date(),
+        });
+
+        mockRoleService.assignRoleToMember.mockResolvedValue(undefined);
+
+        const manager = await factory.createRoleManager(organizationId);
+        await expect(
+          manager.assignRole(userId, organizationId, MembershipRole.ADMIN, membershipId)
+        ).resolves.not.toThrow();
+
+        expect(mockRoleService.assignRoleToMember).toHaveBeenCalledWith(
+          DEFAULT_ROLE_IDS[MembershipRole.ADMIN],
+          membershipId
+        );
+        // Should not call count since it's not an owner
+        expect(prisma.membership.count).not.toHaveBeenCalled();
+      });
+
+      it("should throw error when membership is not found", async () => {
+        // Mock membership not found
+        vi.mocked(prisma.membership.findUnique).mockResolvedValue(null);
+
+        const manager = await factory.createRoleManager(organizationId);
+        await expect(
+          manager.assignRole(userId, organizationId, MembershipRole.ADMIN, membershipId)
+        ).rejects.toThrow(
+          new RoleManagementError("Membership not found", RoleManagementErrorCode.UNAUTHORIZED)
+        );
+      });
    });
  });

@@ -1,9 +1,10 @@
+import { prisma } from "@calcom/prisma";
 import { MembershipRole } from "@calcom/prisma/enums";

 import { RoleManagementError, RoleManagementErrorCode } from "../domain/errors/role-management.error";
 import { DEFAULT_ROLE_IDS } from "../lib/constants";
-import type { IRoleManager } from "./role-manager.interface";
 import { PermissionCheckService } from "./permission-check.service";
+import type { IRoleManager } from "./role-manager.interface";
 import { RoleService } from "./role.service";

 export class PBACRoleManager implements IRoleManager {
@@ -18,11 +19,7 @@ export class PBACRoleManager implements IRoleManager {
    userId: number,
    targetId: number,
    scope: "org" | "team",
-    // Not required for this instance
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
    _memberId?: number,
-    // Not required for this instance
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
    _newRole?: MembershipRole | string
  ): Promise<void> {
    const hasPermission = await this.permissionCheckService.checkPermission({
@@ -40,16 +37,75 @@ export class PBACRoleManager implements IRoleManager {
    }
  }

+  private async validateNotLastOwner(
+    organizationId: number,
+    membershipId: number,
+    newRole: MembershipRole | string
+  ): Promise<void> {
+    // Get current membership details
+    const currentMembership = await prisma.membership.findUnique({
+      where: { id: membershipId },
+      select: {
+        id: true,
+        customRoleId: true,
+        role: true,
+        teamId: true,
+      },
+    });
+
+    if (!currentMembership) {
+      throw new RoleManagementError("Membership not found", RoleManagementErrorCode.UNAUTHORIZED);
+    }
+
+    // Check if the current membership has owner role (either through customRoleId or role field)
+    const hasOwnerRole =
+      currentMembership.customRoleId === DEFAULT_ROLE_IDS[MembershipRole.OWNER] ||
+      currentMembership.role === MembershipRole.OWNER;
+
+    // If current membership is not an owner, no validation needed
+    if (!hasOwnerRole) {
+      return;
+    }
+
+    // Check if new role is still owner - if so, no validation needed
+    const newRoleIsOwner =
+      newRole === MembershipRole.OWNER || newRole === DEFAULT_ROLE_IDS[MembershipRole.OWNER];
+
+    if (newRoleIsOwner) {
+      return;
+    }
+
+    // Count total owners in the organization/team
+    const ownerCount = await prisma.membership.count({
+      where: {
+        teamId: organizationId,
+        accepted: true,
+        OR: [{ customRoleId: DEFAULT_ROLE_IDS[MembershipRole.OWNER] }, { role: MembershipRole.OWNER }],
+      },
+    });
+
+    // If this is the last owner, prevent the role change
+    if (ownerCount <= 1) {
+      throw new RoleManagementError(
+        "Cannot change the role of the last owner in the organization",
+        RoleManagementErrorCode.UNAUTHORIZED
+      );
+    }
+  }
+
  async assignRole(
    _userId: number,
    organizationId: number,
    role: MembershipRole | string,
    membershipId: number
  ): Promise<void> {
+    await this.validateNotLastOwner(organizationId, membershipId, role);
+
    // Check if role is one of the default MembershipRole enum values
    const isDefaultRole = role in DEFAULT_ROLE_IDS;

    // Also check if the role is a default role ID value
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    const isDefaultRoleId = Object.values(DEFAULT_ROLE_IDS).includes(role as any);

    if (isDefaultRole) {
@@ -86,4 +142,4 @@ export class PBACRoleManager implements IRoleManager {
      name: role.name,
    }));
  }
-}
+}
@@ -4,7 +4,7 @@ import { prisma } from "@calcom/prisma";
 import { LegacyRoleManager } from "./legacy-role-manager.service";
 import { PBACRoleManager } from "./pbac-role-manager.service";
 import { PermissionCheckService } from "./permission-check.service";
-import type { IRoleManager } from "./role-manager.interface";
+import { IRoleManager } from "./role-manager.interface";
 import { RoleService } from "./role.service";

 export class RoleManagementFactory {