From 8aaa84142b58efcc168ac3663a01280064e7d75f Mon Sep 17 00:00:00 2001 From: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com> Date: Wed, 3 Dec 2025 23:44:35 +0530 Subject: [PATCH] fix: unable to edit member as a team owner/admin (#25587) * fix: unaable to edit member as owner/admin * Clean up comments in EditMemberSheet.test.tsx Removed comments explaining the fix for permissions prop. * revert --- .../teams/[id]/members/page.tsx | 68 +--- .../teams/components/EditMemberSheet.test.tsx | 327 ++++++++++++++++++ .../ee/teams/components/EditMemberSheet.tsx | 8 +- .../ee/teams/components/MemberList.tsx | 1 + .../pbac/lib/team-member-permissions.ts | 80 +++++ 5 files changed, 423 insertions(+), 61 deletions(-) create mode 100644 packages/features/ee/teams/components/EditMemberSheet.test.tsx create mode 100644 packages/features/pbac/lib/team-member-permissions.ts diff --git a/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/members/page.tsx b/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/members/page.tsx index 97adac481d..3cf44ce444 100644 --- a/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/members/page.tsx +++ b/apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/members/page.tsx @@ -5,13 +5,11 @@ import { headers, cookies } from "next/headers"; import { redirect } from "next/navigation"; import { getServerSession } from "@calcom/features/auth/lib/getServerSession"; -import { Resource, CustomAction } from "@calcom/features/pbac/domain/types/permission-registry"; -import { getSpecificPermissions } from "@calcom/features/pbac/lib/resource-permissions"; +import { getTeamMemberPermissions } from "@calcom/features/pbac/lib/team-member-permissions"; import { RoleManagementFactory } from "@calcom/features/pbac/services/role-management.factory"; import SettingsHeader from "@calcom/features/settings/appDir/SettingsHeader"; import { PrismaAttributeRepository } from "@calcom/lib/server/repository/PrismaAttributeRepository"; import { prisma } from "@calcom/prisma"; -import { MembershipRole } from "@calcom/prisma/enums"; import { viewerTeamsRouter } from "@calcom/trpc/server/routers/viewer/teams/_router"; import { buildLegacyRequest } from "@lib/buildLegacyCtx"; @@ -33,7 +31,7 @@ const getCachedTeamRoles = unstable_cache( try { const roleManager = await RoleManagementFactory.getInstance().createRoleManager(organizationId); return await roleManager.getTeamRoles(teamId); - } catch (error) { + } catch { // PBAC not enabled or error occurred, return empty array return []; } @@ -49,7 +47,7 @@ const getCachedTeamAttributes = unstable_cache( try { return await attributeRepo.findAllByOrgIdWithOptions({ orgId: organizationId }); - } catch (error) { + } catch { return []; } }, @@ -79,65 +77,15 @@ const Page = async ({ params }: { params: Promise<{ id: string }> }) => { const organizationId = team.parentId || teamId; // Load PBAC roles and attributes if available - const [roles, attributes] = await Promise.all([ + const [roles, attributes, memberPermissions] = await Promise.all([ getCachedTeamRoles(teamId, organizationId), getCachedTeamAttributes(organizationId), + getTeamMemberPermissions({ + userId: session.user.id, + team, + }), ]); - const fallbackRolesCanListMembers: MembershipRole[] = [MembershipRole.ADMIN, MembershipRole.OWNER]; - - // If the team is not private we allow members to list other members - if (!team.isPrivate) { - fallbackRolesCanListMembers.push(MembershipRole.MEMBER); - } - - // Get specific PBAC permissions for team member actions - const permissions = await getSpecificPermissions({ - userId: session.user.id, - teamId: teamId, - resource: Resource.Team, - userRole: team.membership.role, - actions: [ - CustomAction.Invite, - CustomAction.ChangeMemberRole, - CustomAction.Remove, - CustomAction.ListMembers, - CustomAction.ListMembersPrivate, - CustomAction.Impersonate, - ], - fallbackRoles: { - [CustomAction.Invite]: { - roles: [MembershipRole.ADMIN, MembershipRole.OWNER], - }, - [CustomAction.ChangeMemberRole]: { - roles: [MembershipRole.ADMIN, MembershipRole.OWNER], - }, - [CustomAction.Remove]: { - roles: [MembershipRole.ADMIN, MembershipRole.OWNER], - }, - [CustomAction.ListMembers]: { - roles: fallbackRolesCanListMembers, - }, - [CustomAction.Impersonate]: { - roles: [MembershipRole.ADMIN, MembershipRole.OWNER], - }, - [CustomAction.ListMembersPrivate]: { - roles: fallbackRolesCanListMembers, - }, - }, - }); - - // Map specific permissions to member actions - const memberPermissions = { - canListMembers: team.isPrivate - ? permissions[CustomAction.ListMembersPrivate] - : permissions[CustomAction.ListMembers], - canInvite: permissions[CustomAction.Invite], - canChangeMemberRole: permissions[CustomAction.ChangeMemberRole], - canRemove: permissions[CustomAction.Remove], - canImpersonate: permissions[CustomAction.Impersonate], - }; - const facetedTeamValues = { roles, teams: [team], diff --git a/packages/features/ee/teams/components/EditMemberSheet.test.tsx b/packages/features/ee/teams/components/EditMemberSheet.test.tsx new file mode 100644 index 0000000000..77f8e480db --- /dev/null +++ b/packages/features/ee/teams/components/EditMemberSheet.test.tsx @@ -0,0 +1,327 @@ +import { render } from "@testing-library/react"; +import React, { type ReactNode } from "react"; +import { vi } from "vitest"; + +import type { MemberPermissions } from "@calcom/features/users/components/UserTable/types"; +import { MembershipRole } from "@calcom/prisma/enums"; + +import { EditMemberSheet } from "./EditMemberSheet"; +import type { State, User } from "./MemberList"; + +// Mock dependencies +vi.mock("@calcom/lib/hooks/useLocale", () => ({ + useLocale: () => ({ + t: (key: string) => key, + }), +})); + +vi.mock("@calcom/trpc/react", () => ({ + trpc: { + viewer: { + pbac: { + getTeamRoles: { + useQuery: () => ({ + data: undefined, + isPending: false, + }), + }, + }, + teams: { + getUserConnectedApps: { + useQuery: () => ({ + data: {}, + isPending: false, + }), + }, + changeMemberRole: { + useMutation: () => ({ + mutate: vi.fn(), + mutateAsync: vi.fn(), + }), + }, + listMembers: { + cancel: vi.fn(), + getInfiniteData: vi.fn(), + invalidate: vi.fn(), + }, + get: { + setData: vi.fn(), + invalidate: vi.fn(), + }, + }, + }, + useUtils: () => ({ + viewer: { + teams: { + listMembers: { + cancel: vi.fn(), + getInfiniteData: vi.fn(), + invalidate: vi.fn(), + }, + get: { + setData: vi.fn(), + invalidate: vi.fn(), + }, + }, + }, + }), + }, +})); + +const mockSetEditMode = vi.fn(); +const mockSetMutationLoading = vi.fn(); + +vi.mock("@calcom/features/users/components/UserTable/EditSheet/store", () => ({ + useEditMode: vi.fn((selector) => { + const state = { + editMode: false, + setEditMode: mockSetEditMode, + mutationLoading: false, + setMutationLoading: mockSetMutationLoading, + }; + if (typeof selector === "function") { + return selector(state); + } + return state; + }), +})); + +// Mock SheetFooterControls to verify props +let capturedProps: { canChangeMemberRole?: boolean; canEditAttributesForUser?: boolean }[] = []; +vi.mock("@calcom/features/users/components/UserTable/EditSheet/SheetFooterControls", () => ({ + SheetFooterControls: (props: { canChangeMemberRole?: boolean; canEditAttributesForUser?: boolean }) => { + capturedProps.push(props); + return React.createElement("div", { "data-testid": "sheet-footer-controls" }, "SheetFooterControls"); + }, +})); + +// Mock other UI components +vi.mock("@calcom/ui/components/sheet", () => ({ + Sheet: ({ children }: { children: ReactNode }) => + React.createElement("div", { "data-testid": "sheet" }, children), + SheetContent: ({ children }: { children: ReactNode }) => + React.createElement("div", { "data-testid": "sheet-content" }, children), + SheetHeader: ({ children }: { children: ReactNode }) => + React.createElement("div", { "data-testid": "sheet-header" }, children), + SheetBody: ({ children }: { children: ReactNode }) => + React.createElement("div", { "data-testid": "sheet-body" }, children), + SheetFooter: ({ children }: { children: ReactNode }) => + React.createElement("div", { "data-testid": "sheet-footer" }, children), +})); + +vi.mock("@calcom/ui/components/form", () => ({ + Form: ({ children }: { children: ReactNode }) => React.createElement("form", null, children), + ToggleGroup: () => React.createElement("div", { "data-testid": "toggle-group" }, "ToggleGroup"), + Select: () => React.createElement("div", { "data-testid": "select" }, "Select"), +})); + +vi.mock("@calcom/ui/components/avatar", () => ({ + Avatar: () => React.createElement("div", { "data-testid": "avatar" }, "Avatar"), +})); + +vi.mock("@calcom/ui/components/skeleton", () => ({ + Skeleton: ({ children }: { children: ReactNode }) => React.createElement("div", null, children), + Loader: () => React.createElement("div", { "data-testid": "loader" }, "Loading..."), +})); + +vi.mock("@calcom/features/users/components/UserTable/EditSheet/DisplayInfo", () => ({ + DisplayInfo: () =>
DisplayInfo
, +})); + +describe("EditMemberSheet", () => { + const mockDispatch = vi.fn(); + + // Create a minimal mock user that satisfies the User type + const mockUser = { + id: 1, + name: "Test User", + email: "test@example.com", + username: "testuser", + role: MembershipRole.MEMBER, + accepted: true, + avatarUrl: "", + bookerUrl: "https://cal.com", + lastActiveAt: "2024-01-01", + customRoleId: null, + } as User; + + const mockState: State = { + editSheet: { + user: mockUser, + showModal: true, + }, + deleteMember: { + showModal: false, + }, + impersonateMember: { + showModal: false, + }, + teamAvailability: { + showModal: false, + }, + }; + + beforeEach(() => { + vi.clearAllMocks(); + mockSetEditMode.mockClear(); + mockSetMutationLoading.mockClear(); + capturedProps = []; + }); + + describe("Fix verification: canChangeMemberRole prop passing", () => { + it("should pass canChangeMemberRole=true to SheetFooterControls when permissions.canChangeMemberRole is true", () => { + const permissions: MemberPermissions = { + canListMembers: false, + canInvite: false, + canChangeMemberRole: true, + canRemove: false, + canImpersonate: false, + }; + + render( + + ); + + // Verify SheetFooterControls was called with canChangeMemberRole=true + expect(capturedProps).toContainEqual( + expect.objectContaining({ + canChangeMemberRole: true, + }) + ); + }); + + it("should pass canChangeMemberRole=false to SheetFooterControls when permissions.canChangeMemberRole is false", () => { + const permissions: MemberPermissions = { + canListMembers: false, + canInvite: false, + canChangeMemberRole: false, + canRemove: false, + canImpersonate: false, + }; + + render( + + ); + + // Verify SheetFooterControls was called with canChangeMemberRole=false + expect(capturedProps).toContainEqual( + expect.objectContaining({ + canChangeMemberRole: false, + }) + ); + }); + + it("should pass canChangeMemberRole=undefined to SheetFooterControls when permissions is undefined", () => { + render( + + ); + + // Verify SheetFooterControls was called with canChangeMemberRole=undefined + expect(capturedProps).toContainEqual( + expect.objectContaining({ + canChangeMemberRole: undefined, + }) + ); + }); + + it("should pass canChangeMemberRole=undefined to SheetFooterControls when permissions object exists but canChangeMemberRole is undefined", () => { + const permissions: Partial = { + canListMembers: true, + canInvite: false, + // canChangeMemberRole is intentionally omitted + }; + + render( + + ); + + // Verify SheetFooterControls was called with canChangeMemberRole=undefined + expect(capturedProps).toContainEqual( + expect.objectContaining({ + canChangeMemberRole: undefined, + }) + ); + }); + + it("should pass canEditAttributesForUser from permissions to SheetFooterControls", () => { + const permissions: MemberPermissions = { + canListMembers: false, + canInvite: false, + canChangeMemberRole: true, + canRemove: false, + canImpersonate: false, + canEditAttributesForUser: true, + }; + + render( + + ); + + // Verify SheetFooterControls was called with both props + expect(capturedProps).toContainEqual( + expect.objectContaining({ + canChangeMemberRole: true, + canEditAttributesForUser: true, + }) + ); + }); + + it("should correctly extract canChangeMemberRole from permissions object (verifies fix)", () => { + // This test specifically verifies the fix where permissions?.canChangeMemberRole + // is now correctly passed to SheetFooterControls + const permissions: MemberPermissions = { + canListMembers: false, + canInvite: false, + canChangeMemberRole: true, // This should be passed to SheetFooterControls + canRemove: false, + canImpersonate: false, + }; + + render( + + ); + + expect(capturedProps).toContainEqual( + expect.objectContaining({ + canChangeMemberRole: true, + }) + ); + }); + }); +}); diff --git a/packages/features/ee/teams/components/EditMemberSheet.tsx b/packages/features/ee/teams/components/EditMemberSheet.tsx index 61bbb6693c..8611d8d9f3 100644 --- a/packages/features/ee/teams/components/EditMemberSheet.tsx +++ b/packages/features/ee/teams/components/EditMemberSheet.tsx @@ -8,6 +8,7 @@ import { shallow } from "zustand/shallow"; import { DisplayInfo } from "@calcom/features/users/components/UserTable/EditSheet/DisplayInfo"; import { SheetFooterControls } from "@calcom/features/users/components/UserTable/EditSheet/SheetFooterControls"; import { useEditMode } from "@calcom/features/users/components/UserTable/EditSheet/store"; +import type { MemberPermissions } from "@calcom/features/users/components/UserTable/types"; import { useLocale } from "@calcom/lib/hooks/useLocale"; import { MembershipRole } from "@calcom/prisma/enums"; import { trpc } from "@calcom/trpc/react"; @@ -34,11 +35,13 @@ export function EditMemberSheet({ dispatch, currentMember, teamId, + permissions, }: { state: State; dispatch: Dispatch; currentMember: MembershipRole; teamId: number; + permissions?: MemberPermissions; }) { const { t } = useLocale(); const { user } = state.editSheet; @@ -277,7 +280,10 @@ export function EditMemberSheet({ - + ) : ( diff --git a/packages/features/ee/teams/components/MemberList.tsx b/packages/features/ee/teams/components/MemberList.tsx index b2be14f2ee..ff1ee1627c 100644 --- a/packages/features/ee/teams/components/MemberList.tsx +++ b/packages/features/ee/teams/components/MemberList.tsx @@ -830,6 +830,7 @@ function MemberListContent(props: Props) { state={state} currentMember={props.team.membership.role} teamId={props.team.id} + permissions={props.permissions} /> )} diff --git a/packages/features/pbac/lib/team-member-permissions.ts b/packages/features/pbac/lib/team-member-permissions.ts new file mode 100644 index 0000000000..ad27cdf276 --- /dev/null +++ b/packages/features/pbac/lib/team-member-permissions.ts @@ -0,0 +1,80 @@ +import type { MemberPermissions } from "@calcom/features/users/components/UserTable/types"; +import { MembershipRole } from "@calcom/prisma/enums"; + +import { Resource, CustomAction } from "../domain/types/permission-registry"; +import { getSpecificPermissions } from "./resource-permissions"; + +interface TeamWithMembership { + id: number; + isPrivate: boolean; + membership: { + role: MembershipRole; + accepted: boolean; + }; +} + +interface GetTeamMemberPermissionsOptions { + userId: number; + team: TeamWithMembership; +} + +/** + * Gets team member permissions using PBAC or fallback to role-based permissions. + */ +export async function getTeamMemberPermissions({ + userId, + team, +}: GetTeamMemberPermissionsOptions): Promise { + // Determine fallback roles for ListMembers based on team privacy + const fallbackRolesCanListMembers: MembershipRole[] = [MembershipRole.ADMIN, MembershipRole.OWNER]; + if (!team.isPrivate) { + fallbackRolesCanListMembers.push(MembershipRole.MEMBER); + } + + // Get specific PBAC permissions for team member actions + const permissions = await getSpecificPermissions({ + userId, + teamId: team.id, + resource: Resource.Team, + userRole: team.membership.role, + actions: [ + CustomAction.Invite, + CustomAction.ChangeMemberRole, + CustomAction.Remove, + CustomAction.ListMembers, + CustomAction.ListMembersPrivate, + CustomAction.Impersonate, + ], + fallbackRoles: { + [CustomAction.Invite]: { + roles: [MembershipRole.ADMIN, MembershipRole.OWNER], + }, + [CustomAction.ChangeMemberRole]: { + roles: [MembershipRole.ADMIN, MembershipRole.OWNER], + }, + [CustomAction.Remove]: { + roles: [MembershipRole.ADMIN, MembershipRole.OWNER], + }, + [CustomAction.ListMembers]: { + roles: fallbackRolesCanListMembers, + }, + [CustomAction.Impersonate]: { + roles: [MembershipRole.ADMIN, MembershipRole.OWNER], + }, + [CustomAction.ListMembersPrivate]: { + roles: fallbackRolesCanListMembers, + }, + }, + }); + + // Map specific permissions to member actions + return { + canListMembers: team.isPrivate + ? permissions[CustomAction.ListMembersPrivate] + : permissions[CustomAction.ListMembers], + canInvite: permissions[CustomAction.Invite], + canChangeMemberRole: permissions[CustomAction.ChangeMemberRole], + canRemove: permissions[CustomAction.Remove], + canImpersonate: permissions[CustomAction.Impersonate], + }; +}