diff --git a/packages/features/pbac/services/role-management.factory.ts b/packages/features/pbac/services/role-management.factory.ts index 2f90860e47..8f65435395 100644 --- a/packages/features/pbac/services/role-management.factory.ts +++ b/packages/features/pbac/services/role-management.factory.ts @@ -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; + checkPermissionToChangeRole( + userId: number, + targetId: number, + scope: "org" | "team", + memberId?: number, + newRole?: MembershipRole | string + ): Promise; 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 { + 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 { 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 { + + protected async validateRoleChange( + userId: number, + teamId: number, + memberId: number, + newRole: MembershipRole | string, + memberships: Membership[] + ): Promise { + // 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 { 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 { 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(); } diff --git a/packages/trpc/server/routers/viewer/teams/changeMemberRole.handler.ts b/packages/trpc/server/routers/viewer/teams/changeMemberRole.handler.ts index 47a7351e4a..4c04ab20f1 100644 --- a/packages/trpc/server/routers/viewer/teams/changeMemberRole.handler.ts +++ b/packages/trpc/server/routers/viewer/teams/changeMemberRole.handler.ts @@ -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);