fix: remove legacy logic into legacy service (#24095)

* fix: remove legacy logic into legacy service * Update packages/features/pbac/services/role-management.factory.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-09-26 11:56:44 +00:00
parent 89789b1e3b
commit 8b7947a172
2 changed files with 115 additions and 52 deletions
@@ -1,6 +1,8 @@
+import { isTeamOwner } from "@calcom/features/ee/teams/lib/queries";
 import { FeaturesRepository } from "@calcom/features/flags/features.repository";
 import { isOrganisationAdmin } from "@calcom/lib/server/queries/organisations";
 import { prisma } from "@calcom/prisma";
+import type { Membership } from "@calcom/prisma/client";
 import { MembershipRole } from "@calcom/prisma/enums";

 import { RoleManagementError, RoleManagementErrorCode } from "../domain/errors/role-management.error";
@@ -10,7 +12,13 @@ import { RoleService } from "./role.service";

 interface IRoleManager {
  isPBACEnabled: boolean;
-  checkPermissionToChangeRole(userId: number, targetId: number, scope: "org" | "team"): Promise<void>;
+  checkPermissionToChangeRole(
+    userId: number,
+    targetId: number,
+    scope: "org" | "team",
+    memberId?: number,
+    newRole?: MembershipRole | string
+  ): Promise<void>;
  assignRole(
    userId: number,
    organizationId: number,
@@ -29,7 +37,17 @@ class PBACRoleManager implements IRoleManager {
    private readonly permissionCheckService: PermissionCheckService
  ) {}

-  async checkPermissionToChangeRole(userId: number, targetId: number, scope: "org" | "team"): Promise<void> {
+  async checkPermissionToChangeRole(
+    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({
      userId,
      teamId: targetId,
@@ -95,7 +113,69 @@ class PBACRoleManager implements IRoleManager {

 class LegacyRoleManager implements IRoleManager {
  public isPBACEnabled = false;
-  async checkPermissionToChangeRole(userId: number, targetId: number, scope: "org" | "team"): Promise<void> {
+
+  protected async validateRoleChange(
+    userId: number,
+    teamId: number,
+    memberId: number,
+    newRole: MembershipRole | string,
+    memberships: Membership[]
+  ): Promise<void> {
+    // Only validate for traditional MembershipRole values
+    if (typeof newRole !== "string" || !Object.values(MembershipRole).includes(newRole as MembershipRole)) {
+      return;
+    }
+
+    const targetMembership = memberships.find((m) => m.userId === memberId);
+    const myMembership = memberships.find((m) => m.userId === userId);
+    const teamOwners = memberships.filter((m) => m.role === MembershipRole.OWNER);
+    const teamHasMoreThanOneOwner = teamOwners.length > 1;
+
+    if (!targetMembership) {
+      throw new RoleManagementError("Target membership not found", RoleManagementErrorCode.UNAUTHORIZED);
+    }
+
+    // Only owners can award owner role
+    if (newRole === MembershipRole.OWNER && !(await isTeamOwner(userId, teamId))) {
+      throw new RoleManagementError("Only owners can award owner role", RoleManagementErrorCode.UNAUTHORIZED);
+    }
+
+    // Admins cannot change the role of an owner
+    if (myMembership?.role === MembershipRole.ADMIN && targetMembership?.role === MembershipRole.OWNER) {
+      throw new RoleManagementError(
+        "You can not change the role of an owner if you are an admin.",
+        RoleManagementErrorCode.UNAUTHORIZED
+      );
+    }
+
+    // Cannot change the role of the only owner
+    if (targetMembership?.role === MembershipRole.OWNER && !teamHasMoreThanOneOwner) {
+      throw new RoleManagementError(
+        "You can not change the role of the only owner of a team.",
+        RoleManagementErrorCode.UNAUTHORIZED
+      );
+    }
+
+    // Admins cannot promote themselves to a higher role (except to MEMBER which is a demotion)
+    if (
+      myMembership?.role === MembershipRole.ADMIN &&
+      memberId === userId &&
+      newRole !== MembershipRole.MEMBER
+    ) {
+      throw new RoleManagementError(
+        "You can not change yourself to a higher role.",
+        RoleManagementErrorCode.UNAUTHORIZED
+      );
+    }
+  }
+
+  async checkPermissionToChangeRole(
+    userId: number,
+    targetId: number,
+    scope: "org" | "team",
+    memberId?: number,
+    newRole?: MembershipRole | string
+  ): Promise<void> {
    let hasPermission = false;
    if (scope === "team") {
      const team = await prisma.membership.findFirst({
@@ -118,12 +198,25 @@ class LegacyRoleManager implements IRoleManager {
        RoleManagementErrorCode.UNAUTHORIZED
      );
    }
+
+    // Additional validation for team role changes in legacy mode
+    if (scope === "team" && memberId && newRole) {
+      const memberships = await prisma.membership.findMany({
+        where: {
+          teamId: targetId,
+          accepted: true,
+        },
+      });
+      await this.validateRoleChange(userId, targetId, memberId, newRole, memberships);
+    }
  }

  async assignRole(
    userId: number,
    organizationId: number,
    role: MembershipRole | string,
+    // Used in other implementation
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
    _membershipId: number
  ): Promise<void> {
    await prisma.membership.update({
@@ -139,6 +232,8 @@ class LegacyRoleManager implements IRoleManager {
    });
  }

+  // Used in other implementation
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  async getAllRoles(_organizationId: number): Promise<{ id: string; name: string }[]> {
    return [
      { id: MembershipRole.OWNER, name: "Owner" },
@@ -147,6 +242,8 @@ class LegacyRoleManager implements IRoleManager {
    ];
  }

+  // Used in other implementation
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  async getTeamRoles(_teamId: number): Promise<{ id: string; name: string }[]> {
    return [
      { id: MembershipRole.OWNER, name: "Owner" },
@@ -156,6 +253,8 @@ class LegacyRoleManager implements IRoleManager {
  }
 }

+export { LegacyRoleManager };
+
 export class RoleManagementFactory {
  private static instance: RoleManagementFactory;
  private featuresRepository: FeaturesRepository;
@@ -163,6 +262,8 @@ export class RoleManagementFactory {
  private permissionCheckService: PermissionCheckService;

  private constructor() {
+    // Not used but needed for DI
+    // eslint-disable-next-line @typescript-eslint/no-unused-expressions
    (this.featuresRepository = new FeaturesRepository(prisma)), (this.roleService = new RoleService());
    this.permissionCheckService = new PermissionCheckService();
  }
@@ -1,8 +1,6 @@
 import { RoleManagementFactory } from "@calcom/features/pbac/services/role-management.factory";
-import { isTeamOwner } from "@calcom/features/ee/teams/lib/queries";
 import { TeamRepository } from "@calcom/lib/server/repository/team";
 import { prisma } from "@calcom/prisma";
-import { MembershipRole } from "@calcom/prisma/enums";
 import type { TrpcSessionUser } from "@calcom/trpc/server/types";

 import { TRPCError } from "@trpc/server";
@@ -30,9 +28,15 @@ export const changeMemberRoleHandler = async ({ ctx, input }: ChangeMemberRoleOp
  // Create role manager for this organization/team
  const roleManager = await RoleManagementFactory.getInstance().createRoleManager(organizationId);

-  // Check permission to change roles
+  // Check permission to change roles (includes legacy validation for LegacyRoleManager)
  try {
-    await roleManager.checkPermissionToChangeRole(ctx.user.id, input.teamId, "team");
+    await roleManager.checkPermissionToChangeRole(
+      ctx.user.id,
+      input.teamId,
+      "team",
+      input.memberId,
+      input.role
+    );
  } catch (error) {
    throw new TRPCError({
      code: "UNAUTHORIZED",
@@ -40,59 +44,17 @@ export const changeMemberRoleHandler = async ({ ctx, input }: ChangeMemberRoleOp
    });
  }

-  // For traditional role checks, fall back to existing logic
-  if (
-    typeof input.role === "string" &&
-    Object.values(MembershipRole).includes(input.role as MembershipRole)
-  ) {
-    // Only owners can award owner role.
-    if (input.role === MembershipRole.OWNER && !(await isTeamOwner(ctx.user?.id, input.teamId)))
-      throw new TRPCError({ code: "UNAUTHORIZED" });
-  }
-
-  const memberships = await prisma.membership.findMany({
+  // Get the target membership for the assignRole method
+  const targetMembership = await prisma.membership.findUnique({
    where: {
-      teamId: input.teamId,
+      userId_teamId: { userId: input.memberId, teamId: input.teamId },
    },
  });

-  const targetMembership = memberships.find((m) => m.userId === input.memberId);
-  const myMembership = memberships.find((m) => m.userId === ctx.user.id);
-  const teamOwners = memberships.filter((m) => m.role === MembershipRole.OWNER);
-  const teamHasMoreThanOneOwner = teamOwners.length > 1;
-
  if (!targetMembership) {
    throw new TRPCError({ code: "NOT_FOUND", message: "Target membership not found" });
  }

-  if (myMembership?.role === MembershipRole.ADMIN && targetMembership?.role === MembershipRole.OWNER) {
-    throw new TRPCError({
-      code: "FORBIDDEN",
-      message: "You can not change the role of an owner if you are an admin.",
-    });
-  }
-
-  if (targetMembership?.role === MembershipRole.OWNER && !teamHasMoreThanOneOwner) {
-    throw new TRPCError({
-      code: "FORBIDDEN",
-      message: "You can not change the role of the only owner of a team.",
-    });
-  }
-
-  // TODO(SEAN): Remove this logic once PBAC is rolled out as there is no concept of higher roles for custom roles. Only default.
-  if (
-    myMembership?.role === MembershipRole.ADMIN &&
-    input.memberId === ctx.user.id &&
-    typeof input.role === "string" &&
-    Object.values(MembershipRole).includes(input.role as MembershipRole) &&
-    input.role !== MembershipRole.MEMBER
-  ) {
-    throw new TRPCError({
-      code: "FORBIDDEN",
-      message: "You can not change yourself to a higher role.",
-    });
-  }
-
  // Use role manager to assign the role
  try {
    await roleManager.assignRole(input.memberId, input.teamId, input.role, targetMembership.id);