fix: team admin/owner can't change others member role (#23161)
* fix: team admin/owner can't change member role * update * Update role-management.factory.ts * address review
This commit is contained in:
@@ -110,13 +110,15 @@ describe("RoleManagementFactory", () => {
|
||||
it("should allow role change when user has permission", async () => {
|
||||
mockPermissionCheckService.checkPermission.mockResolvedValue(true);
|
||||
const manager = await factory.createRoleManager(organizationId);
|
||||
await expect(manager.checkPermissionToChangeRole(userId, organizationId)).resolves.not.toThrow();
|
||||
await expect(
|
||||
manager.checkPermissionToChangeRole(userId, organizationId, "org")
|
||||
).resolves.not.toThrow();
|
||||
});
|
||||
|
||||
it("should throw UNAUTHORIZED when user lacks permission", async () => {
|
||||
mockPermissionCheckService.checkPermission.mockResolvedValue(false);
|
||||
const manager = await factory.createRoleManager(organizationId);
|
||||
await expect(manager.checkPermissionToChangeRole(userId, organizationId)).rejects.toThrow(
|
||||
await expect(manager.checkPermissionToChangeRole(userId, organizationId, "org")).rejects.toThrow(
|
||||
new RoleManagementError(
|
||||
"You do not have permission to change roles",
|
||||
RoleManagementErrorCode.UNAUTHORIZED
|
||||
@@ -193,13 +195,15 @@ describe("RoleManagementFactory", () => {
|
||||
customRoleId: null,
|
||||
});
|
||||
const manager = await factory.createRoleManager(organizationId);
|
||||
await expect(manager.checkPermissionToChangeRole(userId, organizationId)).resolves.not.toThrow();
|
||||
await expect(
|
||||
manager.checkPermissionToChangeRole(userId, organizationId, "org")
|
||||
).resolves.not.toThrow();
|
||||
});
|
||||
|
||||
it("should throw UNAUTHORIZED when user is not owner", async () => {
|
||||
vi.mocked(isOrganisationAdmin).mockResolvedValue(false);
|
||||
const manager = await factory.createRoleManager(organizationId);
|
||||
await expect(manager.checkPermissionToChangeRole(userId, organizationId)).rejects.toThrow(
|
||||
await expect(manager.checkPermissionToChangeRole(userId, organizationId, "org")).rejects.toThrow(
|
||||
new RoleManagementError(
|
||||
"Only owners or admin can update roles",
|
||||
RoleManagementErrorCode.UNAUTHORIZED
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { FeaturesRepository } from "@calcom/features/flags/features.repository";
|
||||
import { isOrganisationAdmin } from "@calcom/lib/server/queries/organisations";
|
||||
import { isTeamAdmin } from "@calcom/lib/server/queries/teams";
|
||||
import { prisma } from "@calcom/prisma";
|
||||
import { MembershipRole } from "@calcom/prisma/enums";
|
||||
|
||||
@@ -10,7 +11,7 @@ import { RoleService } from "./role.service";
|
||||
|
||||
interface IRoleManager {
|
||||
isPBACEnabled: boolean;
|
||||
checkPermissionToChangeRole(userId: number, organizationId: number): Promise<void>;
|
||||
checkPermissionToChangeRole(userId: number, targetId: number, scope: "org" | "team"): Promise<void>;
|
||||
assignRole(
|
||||
userId: number,
|
||||
organizationId: number,
|
||||
@@ -29,11 +30,11 @@ class PBACRoleManager implements IRoleManager {
|
||||
private readonly permissionCheckService: PermissionCheckService
|
||||
) {}
|
||||
|
||||
async checkPermissionToChangeRole(userId: number, organizationId: number): Promise<void> {
|
||||
async checkPermissionToChangeRole(userId: number, targetId: number, scope: "org" | "team"): Promise<void> {
|
||||
const hasPermission = await this.permissionCheckService.checkPermission({
|
||||
userId,
|
||||
teamId: organizationId,
|
||||
permission: "organization.changeMemberRole",
|
||||
teamId: targetId,
|
||||
permission: scope === "team" ? "team.changeMemberRole" : "organization.changeMemberRole",
|
||||
fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN],
|
||||
});
|
||||
|
||||
@@ -95,9 +96,11 @@ class PBACRoleManager implements IRoleManager {
|
||||
|
||||
class LegacyRoleManager implements IRoleManager {
|
||||
public isPBACEnabled = false;
|
||||
async checkPermissionToChangeRole(userId: number, organizationId: number): Promise<void> {
|
||||
const membership = await isOrganisationAdmin(userId, organizationId);
|
||||
|
||||
async checkPermissionToChangeRole(userId: number, targetId: number, scope: "org" | "team"): Promise<void> {
|
||||
const membership =
|
||||
scope === "team"
|
||||
? !!(await isTeamAdmin(userId, targetId))
|
||||
: !!(await isOrganisationAdmin(userId, targetId));
|
||||
// Only OWNER/ADMIN can update role
|
||||
if (!membership) {
|
||||
throw new RoleManagementError(
|
||||
|
||||
@@ -47,7 +47,7 @@ export const updateUserHandler = async ({ ctx, input }: UpdateUserOptions) => {
|
||||
const roleManager = await RoleManagementFactory.getInstance().createRoleManager(organizationId);
|
||||
|
||||
try {
|
||||
await roleManager.checkPermissionToChangeRole(userId, organizationId);
|
||||
await roleManager.checkPermissionToChangeRole(userId, organizationId, "org");
|
||||
} catch (error) {
|
||||
if (error instanceof RoleManagementError) {
|
||||
throw new TRPCError({ code: "UNAUTHORIZED", message: error.message });
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { RoleManagementFactory } from "@calcom/features/pbac/services/role-management.factory";
|
||||
import { isTeamAdmin, isTeamOwner } from "@calcom/lib/server/queries/teams";
|
||||
import { isTeamOwner } from "@calcom/lib/server/queries/teams";
|
||||
import { TeamRepository } from "@calcom/lib/server/repository/team";
|
||||
import { prisma } from "@calcom/prisma";
|
||||
import { MembershipRole } from "@calcom/prisma/enums";
|
||||
@@ -32,7 +32,7 @@ export const changeMemberRoleHandler = async ({ ctx, input }: ChangeMemberRoleOp
|
||||
|
||||
// Check permission to change roles
|
||||
try {
|
||||
await roleManager.checkPermissionToChangeRole(ctx.user.id, organizationId);
|
||||
await roleManager.checkPermissionToChangeRole(ctx.user.id, input.teamId, "team");
|
||||
} catch (error) {
|
||||
throw new TRPCError({
|
||||
code: "UNAUTHORIZED",
|
||||
@@ -45,8 +45,6 @@ export const changeMemberRoleHandler = async ({ ctx, input }: ChangeMemberRoleOp
|
||||
typeof input.role === "string" &&
|
||||
Object.values(MembershipRole).includes(input.role as MembershipRole)
|
||||
) {
|
||||
// Traditional role assignment logic
|
||||
if (!(await isTeamAdmin(ctx.user?.id, input.teamId))) throw new TRPCError({ code: "UNAUTHORIZED" });
|
||||
// Only owners can award owner role.
|
||||
if (input.role === MembershipRole.OWNER && !(await isTeamOwner(ctx.user?.id, input.teamId)))
|
||||
throw new TRPCError({ code: "UNAUTHORIZED" });
|
||||
|
||||
Reference in New Issue
Block a user