diff --git a/packages/trpc/server/routers/viewer/teams/removeMember.handler.ts b/packages/trpc/server/routers/viewer/teams/removeMember.handler.ts index 730fb90cc4..41556a153f 100644 --- a/packages/trpc/server/routers/viewer/teams/removeMember.handler.ts +++ b/packages/trpc/server/routers/viewer/teams/removeMember.handler.ts @@ -1,121 +1,72 @@ -import { FeaturesRepository } from "@calcom/features/flags/features.repository"; -import { Resource, CustomAction } from "@calcom/features/pbac/domain/types/permission-registry"; -import { getSpecificPermissions } from "@calcom/features/pbac/lib/resource-permissions"; import { checkRateLimitAndThrowError } from "@calcom/lib/checkRateLimitAndThrowError"; -import { isTeamOwner } from "@calcom/features/ee/teams/lib/queries"; -import { TeamService } from "@calcom/lib/server/service/teamService"; -import { prisma } from "@calcom/prisma"; -import { MembershipRole } from "@calcom/prisma/enums"; -import type { TrpcSessionUser } from "@calcom/trpc/server/types"; import { TRPCError } from "@trpc/server"; import type { TRemoveMemberInputSchema } from "./removeMember.schema"; +import { RemoveMemberServiceFactory } from "./removeMember/RemoveMemberServiceFactory"; type RemoveMemberOptions = { ctx: { - user: NonNullable; - sourceIp?: string; + user: { + id: number; + organization?: { + isOrgAdmin: boolean; + }; + }; }; input: TRemoveMemberInputSchema; }; -export const removeMemberHandler = async ({ ctx, input }: RemoveMemberOptions) => { +export const removeMemberHandler = async ({ + ctx: { + user: { id: userId, organization }, + }, + input, +}: RemoveMemberOptions) => { await checkRateLimitAndThrowError({ - identifier: `removeMember.${ctx.user.id}`, + identifier: `removeMember.${userId}`, }); const { memberIds, teamIds, isOrg } = input; + const isOrgAdmin = organization?.isOrgAdmin ?? false; - // Check PBAC permissions for each team - const hasRemovePermission = await Promise.all( - teamIds.map(async (teamId) => { - // Get user's membership role in this team - const membership = await prisma.membership.findFirst({ - where: { - userId: ctx.user.id, - teamId: teamId, - }, - select: { - role: true, - }, - }); + // Note: This assumes that all teams in the request have the same PBAC setting 9999% chance they do. + const primaryTeamId = teamIds[0]; + if (!primaryTeamId) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "At least one team ID must be provided", + }); + } - if (!membership) return false; + // Get the appropriate service based on feature flag + const service = await RemoveMemberServiceFactory.create(primaryTeamId); - // Check PBAC permissions for removing team members - const permissions = await getSpecificPermissions({ - userId: ctx.user.id, - teamId: teamId, - resource: isOrg ? Resource.Organization : Resource.Team, - userRole: membership.role, - actions: [CustomAction.Remove], - fallbackRoles: { - [CustomAction.Remove]: { - roles: [MembershipRole.ADMIN, MembershipRole.OWNER], - }, - }, - }); + const { hasPermission } = await service.checkRemovePermissions({ + userId, + isOrgAdmin, + memberIds, + teamIds, + isOrg, + }); - return permissions[CustomAction.Remove]; - }) - ).then((results) => results.every((result) => result)); - - // Check if user is trying to remove themselves (allowed for non-owners) - const isRemovingSelf = memberIds.length === 1 && memberIds[0] === ctx.user.id; - - // Allow if user has remove permission OR if they're removing themselves - if (!hasRemovePermission && !isRemovingSelf) { + if (!hasPermission) { throw new TRPCError({ code: "UNAUTHORIZED" }); } - // TODO(SEAN): Remove this after PBAC is rolled out. - // Check if any team has PBAC enabled - const featuresRepository = new FeaturesRepository(prisma); - const pbacEnabledForTeams = await Promise.all( - teamIds.map(async (teamId) => await featuresRepository.checkIfTeamHasFeature(teamId, "pbac")) + await service.validateRemoval( + { + userId, + isOrgAdmin, + memberIds, + teamIds, + isOrg, + }, + hasPermission ); - const isAnyTeamPBACEnabled = pbacEnabledForTeams.some((enabled) => enabled); - // Only apply traditional owner-based logic if PBAC is not enabled for any teams - if (!isAnyTeamPBACEnabled) { - // Only a team owner can remove another team owner. - const isAnyMemberOwnerAndCurrentUserNotOwner = await Promise.all( - memberIds.map(async (memberId) => { - const isAnyTeamOwnerAndCurrentUserNotOwner = await Promise.all( - teamIds.map(async (teamId) => { - return (await isTeamOwner(memberId, teamId)) && !(await isTeamOwner(ctx.user.id, teamId)); - }) - ).then((results) => results.some((result) => result)); - - return isAnyTeamOwnerAndCurrentUserNotOwner; - }) - ).then((results) => results.some((result) => result)); - - if (isAnyMemberOwnerAndCurrentUserNotOwner) { - throw new TRPCError({ - code: "UNAUTHORIZED", - message: "Only a team owner can remove another team owner.", - }); - } - - // Check if user is trying to remove themselves from a team they own (prevent this) - if (isRemovingSelf && hasRemovePermission) { - // Additional check: ensure they're not an owner trying to remove themselves - const isOwnerOfAnyTeam = await Promise.all( - teamIds.map(async (teamId) => await isTeamOwner(ctx.user.id, teamId)) - ).then((results) => results.some((result) => result)); - - if (isOwnerOfAnyTeam) { - throw new TRPCError({ - code: "FORBIDDEN", - message: "You can not remove yourself from a team you own.", - }); - } - } - } - - await TeamService.removeMembers({ teamIds, userIds: memberIds, isOrg }); + // Perform the removal + await service.removeMembers(memberIds, teamIds, isOrg); }; export default removeMemberHandler; diff --git a/packages/trpc/server/routers/viewer/teams/removeMember/BaseRemoveMemberService.ts b/packages/trpc/server/routers/viewer/teams/removeMember/BaseRemoveMemberService.ts new file mode 100644 index 0000000000..a814d01e1b --- /dev/null +++ b/packages/trpc/server/routers/viewer/teams/removeMember/BaseRemoveMemberService.ts @@ -0,0 +1,21 @@ +import { TeamService } from "@calcom/lib/server/service/teamService"; + +import type { + IRemoveMemberService, + RemoveMemberContext, + RemoveMemberPermissionResult, +} from "./IRemoveMemberService"; + +/** + * Base abstract class for remove member services + * Provides common functionality and defines abstract methods for specific implementations + */ +export abstract class BaseRemoveMemberService implements IRemoveMemberService { + abstract checkRemovePermissions(context: RemoveMemberContext): Promise; + + abstract validateRemoval(context: RemoveMemberContext, hasPermission: boolean): Promise; + + async removeMembers(memberIds: number[], teamIds: number[], isOrg: boolean): Promise { + await TeamService.removeMembers({ teamIds, userIds: memberIds, isOrg }); + } +} diff --git a/packages/trpc/server/routers/viewer/teams/removeMember/IRemoveMemberService.ts b/packages/trpc/server/routers/viewer/teams/removeMember/IRemoveMemberService.ts new file mode 100644 index 0000000000..ca1305b485 --- /dev/null +++ b/packages/trpc/server/routers/viewer/teams/removeMember/IRemoveMemberService.ts @@ -0,0 +1,31 @@ +import type { MembershipRole } from "@calcom/prisma/enums"; + +export interface RemoveMemberContext { + userId: number; + isOrgAdmin: boolean; + memberIds: number[]; + teamIds: number[]; + isOrg: boolean; +} + +export interface RemoveMemberPermissionResult { + hasPermission: boolean; + userRoles?: Map; +} + +export interface IRemoveMemberService { + /** + * Checks if the user has permission to remove members from teams + */ + checkRemovePermissions(context: RemoveMemberContext): Promise; + + /** + * Validates that the removal can proceed (e.g., owner checks) + */ + validateRemoval(context: RemoveMemberContext, hasPermission: boolean): Promise; + + /** + * Performs the actual removal of members from teams + */ + removeMembers(memberIds: number[], teamIds: number[], isOrg: boolean): Promise; +} diff --git a/packages/trpc/server/routers/viewer/teams/removeMember/LegacyRemoveMemberService.ts b/packages/trpc/server/routers/viewer/teams/removeMember/LegacyRemoveMemberService.ts new file mode 100644 index 0000000000..4b5266d1a6 --- /dev/null +++ b/packages/trpc/server/routers/viewer/teams/removeMember/LegacyRemoveMemberService.ts @@ -0,0 +1,108 @@ +import * as teamQueries from "@calcom/features/ee/teams/lib/queries"; +import { prisma } from "@calcom/prisma"; +import { MembershipRole } from "@calcom/prisma/enums"; + +import { TRPCError } from "@trpc/server"; + +import { BaseRemoveMemberService } from "./BaseRemoveMemberService"; +import type { RemoveMemberContext, RemoveMemberPermissionResult } from "./IRemoveMemberService"; + +export class LegacyRemoveMemberService extends BaseRemoveMemberService { + async checkRemovePermissions(context: RemoveMemberContext): Promise { + const { userId, isOrgAdmin, teamIds } = context; + + // Org admins have full permission + if (isOrgAdmin) { + // Return admin role for all teams + const userRoles = new Map(); + teamIds.forEach((teamId) => { + userRoles.set(teamId, MembershipRole.ADMIN); + }); + return { + hasPermission: true, + userRoles, + }; + } + + // Get all memberships in a single query + const membershipRoles = await prisma.membership.findMany({ + where: { + userId, + teamId: { + in: teamIds, + }, + }, + select: { + role: true, + teamId: true, + }, + }); + + // Create a map for O(1) lookup + const userRoles = new Map(); + membershipRoles.forEach((m) => { + userRoles.set(m.teamId, m.role); + }); + + // Check if user has admin or owner role in all teams + const allowedRoles: MembershipRole[] = [MembershipRole.ADMIN, MembershipRole.OWNER]; + let hasPermission = true; + + for (const teamId of teamIds) { + const userRole = userRoles.get(teamId); + if (!userRole || !allowedRoles.includes(userRole)) { + hasPermission = false; + break; + } + } + + return { + hasPermission, + userRoles, + }; + } + + async validateRemoval(context: RemoveMemberContext, hasPermission: boolean): Promise { + const { userId, memberIds, teamIds, isOrgAdmin } = context; + const isRemovingSelf = memberIds.length === 1 && memberIds[0] === userId; + + // Only a team owner can remove another team owner (org admins are exempt) + if (!isOrgAdmin) { + const isAnyMemberOwnerAndCurrentUserNotOwner = await Promise.all( + memberIds.map(async (memberId) => { + const isAnyTeamOwnerAndCurrentUserNotOwner = await Promise.all( + teamIds.map(async (teamId) => { + return ( + (await teamQueries.isTeamOwner(memberId, teamId)) && + !(await teamQueries.isTeamOwner(userId, teamId)) + ); + }) + ).then((results) => results.some((result) => result)); + + return isAnyTeamOwnerAndCurrentUserNotOwner; + }) + ).then((results) => results.some((result) => result)); + + if (isAnyMemberOwnerAndCurrentUserNotOwner) { + throw new TRPCError({ + code: "UNAUTHORIZED", + message: "Only a team owner can remove another team owner.", + }); + } + } + + // Check if user is trying to remove themselves from a team they own (prevent this) + if (isRemovingSelf && hasPermission) { + const isOwnerOfAnyTeam = await Promise.all( + teamIds.map(async (teamId) => await teamQueries.isTeamOwner(userId, teamId)) + ).then((results) => results.some((result) => result)); + + if (isOwnerOfAnyTeam) { + throw new TRPCError({ + code: "FORBIDDEN", + message: "You can not remove yourself from a team you own.", + }); + } + } + } +} diff --git a/packages/trpc/server/routers/viewer/teams/removeMember/PBACRemoveMemberService.ts b/packages/trpc/server/routers/viewer/teams/removeMember/PBACRemoveMemberService.ts new file mode 100644 index 0000000000..9f4683b3b7 --- /dev/null +++ b/packages/trpc/server/routers/viewer/teams/removeMember/PBACRemoveMemberService.ts @@ -0,0 +1,60 @@ +import * as teamQueries from "@calcom/features/ee/teams/lib/queries"; +import { PermissionMapper } from "@calcom/features/pbac/domain/mappers/PermissionMapper"; +import { Resource, CustomAction } from "@calcom/features/pbac/domain/types/permission-registry"; +import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service"; + +import { TRPCError } from "@trpc/server"; + +import { BaseRemoveMemberService } from "./BaseRemoveMemberService"; +import type { RemoveMemberContext, RemoveMemberPermissionResult } from "./IRemoveMemberService"; + +export class PBACRemoveMemberService extends BaseRemoveMemberService { + private permissionService = new PermissionCheckService(); + + async checkRemovePermissions(context: RemoveMemberContext): Promise { + const { userId, teamIds, isOrg } = context; + + const resource = isOrg ? Resource.Organization : Resource.Team; + const removePermission = PermissionMapper.toPermissionString({ + resource, + action: CustomAction.Remove, + }); + + const teamsWithPermission = await this.permissionService.getTeamIdsWithPermission( + userId, + removePermission + ); + + // Convert to Set for O(1) lookup + const teamsWithPermissionSet = new Set(teamsWithPermission); + + // Check if user has permission for ALL requested teams + const hasPermission = teamIds.every((teamId) => teamsWithPermissionSet.has(teamId)); + + return { + hasPermission, + }; + } + + async validateRemoval(context: RemoveMemberContext, hasPermission: boolean): Promise { + const { userId, memberIds, teamIds } = context; + const isRemovingSelf = memberIds.length === 1 && memberIds[0] === userId; + + /** + * TODO: Figure out the best way to prevent someone bricking a team + * by removing all people with updateRole permissions + */ + if (isRemovingSelf && hasPermission) { + const isOwnerOfAnyTeam = await Promise.all( + teamIds.map(async (teamId) => await teamQueries.isTeamOwner(userId, teamId)) + ).then((results) => results.some((result) => result)); + + if (isOwnerOfAnyTeam) { + throw new TRPCError({ + code: "FORBIDDEN", + message: "You can not remove yourself from a team you own.", + }); + } + } + } +} diff --git a/packages/trpc/server/routers/viewer/teams/removeMember/RemoveMemberServiceFactory.ts b/packages/trpc/server/routers/viewer/teams/removeMember/RemoveMemberServiceFactory.ts new file mode 100644 index 0000000000..50ffd2a94d --- /dev/null +++ b/packages/trpc/server/routers/viewer/teams/removeMember/RemoveMemberServiceFactory.ts @@ -0,0 +1,21 @@ +import { FeaturesRepository } from "@calcom/features/flags/features.repository"; +import { prisma } from "@calcom/prisma"; + +import type { IRemoveMemberService } from "./IRemoveMemberService"; +import { LegacyRemoveMemberService } from "./LegacyRemoveMemberService"; +import { PBACRemoveMemberService } from "./PBACRemoveMemberService"; + +export class RemoveMemberServiceFactory { + /** + * Creates the appropriate RemoveMemberService based on whether PBAC is enabled + * Caches the service per team/org to avoid repeated feature flag checks + */ + static async create(teamId: number): Promise { + const featuresRepository = new FeaturesRepository(prisma); + const isPBACEnabled = await featuresRepository.checkIfTeamHasFeature(teamId, "pbac"); + + const service = isPBACEnabled ? new PBACRemoveMemberService() : new LegacyRemoveMemberService(); + + return service; + } +} diff --git a/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/LegacyRemoveMemberService.test.ts b/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/LegacyRemoveMemberService.test.ts new file mode 100644 index 0000000000..e22f4b2b50 --- /dev/null +++ b/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/LegacyRemoveMemberService.test.ts @@ -0,0 +1,501 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; + +import * as teamQueries from "@calcom/features/ee/teams/lib/queries"; +import { TeamService } from "@calcom/lib/server/service/teamService"; +import { prisma } from "@calcom/prisma"; +import { MembershipRole } from "@calcom/prisma/enums"; + +import { TRPCError } from "@trpc/server"; + +import { LegacyRemoveMemberService } from "../LegacyRemoveMemberService"; + +vi.mock("@calcom/prisma", () => ({ + prisma: { + membership: { + findMany: vi.fn(), + }, + }, +})); + +vi.mock("@calcom/lib/server/service/teamService"); +vi.mock("@calcom/features/ee/teams/lib/queries"); + +describe("LegacyRemoveMemberService", () => { + let service: LegacyRemoveMemberService; + + beforeEach(() => { + vi.clearAllMocks(); + service = new LegacyRemoveMemberService(); + }); + + describe("checkRemovePermissions", () => { + describe("Org Admin Scenarios", () => { + it("should allow org admin to remove members from teams they are NOT part of", async () => { + const userId = 1; + const isOrgAdmin = true; + const teamIds = [100, 200]; // Teams the org admin is not part of + const memberIds = [2, 3]; + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin, + memberIds, + teamIds, + isOrg: true, + }); + + expect(result.hasPermission).toBe(true); + // Should not query database for org admin + expect(prisma.membership.findMany).not.toHaveBeenCalled(); + }); + + it("should bypass membership checks for org admins", async () => { + const userId = 1; + const isOrgAdmin = true; + const teamIds = [1, 2, 3]; + const memberIds = [4, 5, 6]; + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin, + memberIds, + teamIds, + isOrg: true, + }); + + expect(result.hasPermission).toBe(true); + expect(result.userRoles).toBeInstanceOf(Map); + + // Org admin should have ADMIN role for all teams + teamIds.forEach((teamId) => { + expect(result.userRoles?.get(teamId)).toBe(MembershipRole.ADMIN); + }); + + // Should not query database + expect(prisma.membership.findMany).not.toHaveBeenCalled(); + }); + + it("should allow org admin to remove from multiple teams at once", async () => { + const userId = 1; + const isOrgAdmin = true; + const teamIds = [1, 2, 3, 4, 5]; + const memberIds = [10, 20]; + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin, + memberIds, + teamIds, + isOrg: true, + }); + + expect(result.hasPermission).toBe(true); + expect(prisma.membership.findMany).not.toHaveBeenCalled(); + }); + + it("should work for org admin even with isOrg=false", async () => { + const userId = 1; + const isOrgAdmin = true; + const teamIds = [1]; + const memberIds = [2]; + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin, + memberIds, + teamIds, + isOrg: false, // Note: isOrg is false + }); + + expect(result.hasPermission).toBe(true); + expect(prisma.membership.findMany).not.toHaveBeenCalled(); + }); + }); + + describe("Regular User Scenarios", () => { + it("should allow ADMIN to remove members", async () => { + const userId = 1; + const teamIds = [1, 2]; + const memberIds = [3]; + + vi.mocked(prisma.membership.findMany).mockResolvedValue([ + { id: 1, userId, teamId: 1, role: MembershipRole.ADMIN } as any, + { id: 2, userId, teamId: 2, role: MembershipRole.ADMIN } as any, + ]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }); + + expect(result.hasPermission).toBe(true); + expect(result.userRoles?.get(1)).toBe(MembershipRole.ADMIN); + expect(result.userRoles?.get(2)).toBe(MembershipRole.ADMIN); + }); + + it("should allow OWNER to remove members", async () => { + const userId = 1; + const teamIds = [1]; + const memberIds = [2]; + + vi.mocked(prisma.membership.findMany).mockResolvedValue([ + { id: 1, userId, teamId: 1, role: MembershipRole.OWNER } as any, + ]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }); + + expect(result.hasPermission).toBe(true); + expect(result.userRoles?.get(1)).toBe(MembershipRole.OWNER); + }); + + it("should deny MEMBER from removing members", async () => { + const userId = 1; + const teamIds = [1]; + const memberIds = [2]; + + vi.mocked(prisma.membership.findMany).mockResolvedValue([ + { id: 1, userId, teamId: 1, role: MembershipRole.MEMBER } as any, + ]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }); + + expect(result.hasPermission).toBe(false); + }); + + it("should deny non-member from removing members", async () => { + const userId = 1; + const teamIds = [1]; + const memberIds = [2]; + + vi.mocked(prisma.membership.findMany).mockResolvedValue([]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }); + + expect(result.hasPermission).toBe(false); + }); + + it("should require ADMIN/OWNER role in ALL teams for multi-team removal", async () => { + const userId = 1; + const teamIds = [1, 2, 3]; + const memberIds = [4]; + + vi.mocked(prisma.membership.findMany).mockResolvedValue([ + { id: 1, userId, teamId: 1, role: MembershipRole.ADMIN } as any, + { id: 2, userId, teamId: 2, role: MembershipRole.MEMBER } as any, // Not admin/owner + { id: 3, userId, teamId: 3, role: MembershipRole.OWNER } as any, + ]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }); + + expect(result.hasPermission).toBe(false); + }); + + it("should allow when user has ADMIN/OWNER in all teams", async () => { + const userId = 1; + const teamIds = [1, 2, 3]; + const memberIds = [4, 5]; + + vi.mocked(prisma.membership.findMany).mockResolvedValue([ + { id: 1, userId, teamId: 1, role: MembershipRole.ADMIN } as any, + { id: 2, userId, teamId: 2, role: MembershipRole.OWNER } as any, + { id: 3, userId, teamId: 3, role: MembershipRole.ADMIN } as any, + ]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }); + + expect(result.hasPermission).toBe(true); + }); + }); + }); + + describe("validateRemoval", () => { + describe("Owner Protection", () => { + it("should prevent non-owner from removing owner", async () => { + const userId = 1; + const memberIds = [2]; + const teamIds = [1]; + const userRoles = new Map([[1, MembershipRole.ADMIN]]); + + // Member 2 is owner, but userId 1 is not owner + vi.mocked(teamQueries.isTeamOwner) + .mockResolvedValueOnce(true) // isTeamOwner(2, 1) - member is owner + .mockResolvedValueOnce(false); // isTeamOwner(1, 1) - current user is not owner + + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + true + ) + ).rejects.toThrow( + expect.objectContaining({ + code: "UNAUTHORIZED", + message: "Only a team owner can remove another team owner.", + }) + ); + }); + + it("should allow owner to remove another owner", async () => { + const userId = 1; + const memberIds = [2]; + const teamIds = [1]; + const userRoles = new Map([[1, MembershipRole.OWNER]]); + + vi.mocked(teamQueries.isTeamOwner).mockResolvedValue(true); + + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + true + ) + ).resolves.not.toThrow(); + }); + + it("should allow org admin to remove owner", async () => { + const userId = 1; + const memberIds = [2]; + const teamIds = [1]; + const userRoles = new Map([[1, MembershipRole.ADMIN]]); + + vi.mocked(teamQueries.isTeamOwner).mockResolvedValue(true); + + // Org admin can remove owner + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: true, // Org admin + memberIds, + teamIds, + isOrg: true, + }, + { hasPermission: true, userRoles } + ) + ).resolves.not.toThrow(); + + // isTeamOwner should not be called for org admins + expect(teamQueries.isTeamOwner).not.toHaveBeenCalled(); + }); + }); + + describe("Self-Removal Prevention", () => { + it("should prevent owner from removing themselves", async () => { + const userId = 1; + const memberIds = [1]; // Same as userId + const teamIds = [1]; + const userRoles = new Map([[1, MembershipRole.OWNER]]); + + vi.mocked(teamQueries.isTeamOwner).mockResolvedValue(true); // User is owner + + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + true // hasPermission - this should be boolean, not object + ) + ).rejects.toThrow( + expect.objectContaining({ + code: "FORBIDDEN", + message: "You can not remove yourself from a team you own.", + }) + ); + }); + + it("should allow admin to remove themselves", async () => { + const userId = 1; + const memberIds = [1]; // Same as userId + const teamIds = [1]; + const userRoles = new Map([[1, MembershipRole.ADMIN]]); + + vi.mocked(teamQueries.isTeamOwner).mockResolvedValue(false); + + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + { hasPermission: true, userRoles } + ) + ).resolves.not.toThrow(); + }); + + it("should allow member to remove themselves", async () => { + const userId = 1; + const memberIds = [1]; // Same as userId + const teamIds = [1]; + const userRoles = new Map([[1, MembershipRole.MEMBER]]); + + vi.mocked(teamQueries.isTeamOwner).mockResolvedValue(false); + + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + { hasPermission: true, userRoles } + ) + ).resolves.not.toThrow(); + }); + }); + + describe("Multi-Member Validation", () => { + it("should validate each member independently", async () => { + const userId = 1; + const memberIds = [2, 3, 4]; + const teamIds = [1]; + const userRoles = new Map([[1, MembershipRole.ADMIN]]); + + // Member 2 is not owner, member 3 is owner, member 4 is not owner + // Current user (1) is not owner + vi.mocked(teamQueries.isTeamOwner) + .mockResolvedValueOnce(false) // isTeamOwner(2, 1) - member 2 is not owner + .mockResolvedValueOnce(true) // isTeamOwner(3, 1) - member 3 is owner + .mockResolvedValueOnce(false) // isTeamOwner(1, 1) - current user is not owner + .mockResolvedValueOnce(false); // isTeamOwner(4, 1) - member 4 is not owner (if reached) + + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + { hasPermission: true, userRoles } + ) + ).rejects.toThrow( + expect.objectContaining({ + code: "UNAUTHORIZED", + message: "Only a team owner can remove another team owner.", + }) + ); + + expect(teamQueries.isTeamOwner).toHaveBeenCalledTimes(4); // Checks member 2, user 1, member 3 (owner), user 1 + }); + }); + }); + + describe("removeMembers", () => { + it("should call TeamService.removeMembers with correct parameters", async () => { + const memberIds = [1, 2, 3]; + const teamIds = [4, 5]; + const isOrg = true; + + vi.mocked(TeamService.removeMembers).mockResolvedValue(undefined); + + await service.removeMembers(memberIds, teamIds, isOrg); + + expect(TeamService.removeMembers).toHaveBeenCalledWith({ + userIds: memberIds, + teamIds, + isOrg, + }); + }); + + it("should propagate errors from TeamService", async () => { + const memberIds = [1]; + const teamIds = [2]; + const isOrg = false; + + const error = new Error("Database error"); + vi.mocked(TeamService.removeMembers).mockRejectedValue(error); + + await expect(service.removeMembers(memberIds, teamIds, isOrg)).rejects.toThrow("Database error"); + }); + }); + + describe("Edge Cases", () => { + it("should handle empty memberIds array", async () => { + const userId = 1; + const memberIds: number[] = []; + const teamIds = [1]; + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }); + + // Should still check permissions even with no members to remove + expect(prisma.membership.findMany).toHaveBeenCalled(); + }); + + it("should handle permission check with no teams in database", async () => { + const userId = 1; + const teamIds = [999]; // Non-existent team + const memberIds = [2]; + + vi.mocked(prisma.membership.findMany).mockResolvedValue([]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }); + + expect(result.hasPermission).toBe(false); + }); + }); +}); diff --git a/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/PBACRemoveMemberService.test.ts b/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/PBACRemoveMemberService.test.ts new file mode 100644 index 0000000000..c3da482ed9 --- /dev/null +++ b/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/PBACRemoveMemberService.test.ts @@ -0,0 +1,474 @@ +import { describe, expect, it, vi, beforeEach, type Mock } from "vitest"; + +import * as teamQueries from "@calcom/features/ee/teams/lib/queries"; +import { PermissionMapper } from "@calcom/features/pbac/domain/mappers/PermissionMapper"; +import { Resource, CustomAction } from "@calcom/features/pbac/domain/types/permission-registry"; +import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service"; +import { TeamService } from "@calcom/lib/server/service/teamService"; +import { prisma } from "@calcom/prisma"; +import { MembershipRole } from "@calcom/prisma/enums"; + +import { PBACRemoveMemberService } from "../PBACRemoveMemberService"; + +vi.mock("@calcom/prisma", () => ({ + prisma: { + membership: { + findMany: vi.fn(), + }, + }, +})); + +vi.mock("@calcom/lib/server/service/teamService"); +vi.mock("@calcom/features/ee/teams/lib/queries"); +vi.mock("@calcom/features/pbac/services/permission-check.service"); +vi.mock("@calcom/features/pbac/domain/mappers/PermissionMapper"); + +describe("PBACRemoveMemberService", () => { + let service: PBACRemoveMemberService; + let mockPermissionCheckService: { + getTeamIdsWithPermission: Mock; + }; + + beforeEach(() => { + vi.clearAllMocks(); + + mockPermissionCheckService = { + getTeamIdsWithPermission: vi.fn(), + }; + + vi.mocked(PermissionCheckService).mockImplementation(() => mockPermissionCheckService as any); + + service = new PBACRemoveMemberService(); + }); + + describe("checkRemovePermissions", () => { + describe("PBAC Permission Checks", () => { + it("should check team.remove permission for team context", async () => { + const userId = 1; + const teamIds = [1, 2]; + const memberIds = [3]; + const isOrg = false; + + const removePermission = "team.remove"; + vi.mocked(PermissionMapper.toPermissionString).mockReturnValue(removePermission); + mockPermissionCheckService.getTeamIdsWithPermission.mockResolvedValue([1, 2]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg, + }); + + expect(result.hasPermission).toBe(true); + + expect(PermissionMapper.toPermissionString).toHaveBeenCalledWith({ + resource: Resource.Team, + action: CustomAction.Remove, + }); + + expect(mockPermissionCheckService.getTeamIdsWithPermission).toHaveBeenCalledWith( + userId, + removePermission + ); + }); + + it("should check organization.remove permission for org context", async () => { + const userId = 1; + const teamIds = [1, 2]; + const memberIds = [3]; + const isOrg = true; + + const removePermission = "organization.remove"; + vi.mocked(PermissionMapper.toPermissionString).mockReturnValue(removePermission); + mockPermissionCheckService.getTeamIdsWithPermission.mockResolvedValue([1, 2]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg, + }); + + expect(result.hasPermission).toBe(true); + + expect(PermissionMapper.toPermissionString).toHaveBeenCalledWith({ + resource: Resource.Organization, + action: CustomAction.Remove, + }); + }); + + it("should deny when user lacks permission for some teams", async () => { + const userId = 1; + const teamIds = [1, 2, 3]; + const memberIds = [4]; + const isOrg = false; + + const removePermission = "team.remove"; + vi.mocked(PermissionMapper.toPermissionString).mockReturnValue(removePermission); + // User only has permission for teams 1 and 2, not 3 + mockPermissionCheckService.getTeamIdsWithPermission.mockResolvedValue([1, 2]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg, + }); + + expect(result.hasPermission).toBe(false); + }); + + it("should allow when user has permission for all teams", async () => { + const userId = 1; + const teamIds = [1, 2, 3]; + const memberIds = [4, 5]; + const isOrg = false; + + const removePermission = "team.remove"; + vi.mocked(PermissionMapper.toPermissionString).mockReturnValue(removePermission); + mockPermissionCheckService.getTeamIdsWithPermission.mockResolvedValue([1, 2, 3, 4]); // Has more permissions + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg, + }); + + expect(result.hasPermission).toBe(true); + }); + + it("should deny when user has no permissions", async () => { + const userId = 1; + const teamIds = [1]; + const memberIds = [2]; + const isOrg = false; + + const removePermission = "team.remove"; + vi.mocked(PermissionMapper.toPermissionString).mockReturnValue(removePermission); + mockPermissionCheckService.getTeamIdsWithPermission.mockResolvedValue([]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg, + }); + + expect(result.hasPermission).toBe(false); + }); + }); + + describe("Multi-team Operations", () => { + it("should require permission for ALL teams in request", async () => { + const userId = 1; + const teamIds = [10, 20, 30, 40]; + const memberIds = [100]; + const isOrg = false; + + const removePermission = "team.remove"; + vi.mocked(PermissionMapper.toPermissionString).mockReturnValue(removePermission); + // Missing permission for team 30 + mockPermissionCheckService.getTeamIdsWithPermission.mockResolvedValue([10, 20, 40]); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg, + }); + + expect(result.hasPermission).toBe(false); + }); + + it("should handle large team lists efficiently", async () => { + const userId = 1; + const teamIds = Array.from({ length: 100 }, (_, i) => i + 1); + const memberIds = [999]; + const isOrg = true; + + const removePermission = "organization.remove"; + vi.mocked(PermissionMapper.toPermissionString).mockReturnValue(removePermission); + mockPermissionCheckService.getTeamIdsWithPermission.mockResolvedValue(teamIds); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg, + }); + + expect(result.hasPermission).toBe(true); + }); + }); + }); + + describe("validateRemoval", () => { + describe("Owner Protection", () => { + it("should allow removing owners with PBAC permissions", async () => { + const userId = 1; + const memberIds = [2]; + const teamIds = [1]; + + vi.mocked(teamQueries.isTeamOwner).mockResolvedValue(true); // Member 2 is owner + + // Get user's role + vi.mocked(prisma.membership.findMany).mockResolvedValue([ + { id: 1, userId, teamId: 1, role: MembershipRole.ADMIN } as any, + ]); + + // PBAC service doesn't have owner-to-owner protection logic + // If user has PBAC remove permission, they can remove owners (unlike Legacy service) + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + true // hasPermission + ) + ).resolves.not.toThrow(); + }); + + it("should allow owner to remove another owner with PBAC", async () => { + const userId = 1; + const memberIds = [2]; + const teamIds = [1]; + + vi.mocked(teamQueries.isTeamOwner).mockResolvedValue(true); // Member 2 is owner + + // User is owner + vi.mocked(prisma.membership.findMany).mockResolvedValue([ + { id: 1, userId, teamId: 1, role: MembershipRole.OWNER } as any, + ]); + + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + true + ) + ).resolves.not.toThrow(); + }); + }); + + describe("Self-Removal Prevention", () => { + it("should prevent owner from removing themselves", async () => { + const userId = 1; + const memberIds = [1]; // Same as userId + const teamIds = [1]; + + vi.mocked(teamQueries.isTeamOwner).mockResolvedValue(true); // User is owner of the team + + vi.mocked(prisma.membership.findMany).mockResolvedValue([ + { id: 1, userId, teamId: 1, role: MembershipRole.OWNER } as any, + ]); + + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + true + ) + ).rejects.toThrow( + expect.objectContaining({ + code: "FORBIDDEN", + message: "You can not remove yourself from a team you own.", + }) + ); + }); + + it("should allow admin to remove themselves", async () => { + const userId = 1; + const memberIds = [1]; + const teamIds = [1]; + + vi.mocked(teamQueries.isTeamOwner).mockResolvedValue(false); + + vi.mocked(prisma.membership.findMany).mockResolvedValue([ + { id: 1, userId, teamId: 1, role: MembershipRole.ADMIN } as any, + ]); + + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + true + ) + ).resolves.not.toThrow(); + }); + }); + + describe("Multi-Member Validation", () => { + it("should allow removing multiple members including owners", async () => { + const userId = 1; + const memberIds = [2, 3, 4]; + const teamIds = [1]; + + // User is admin + vi.mocked(prisma.membership.findMany).mockResolvedValue([ + { id: 1, userId, teamId: 1, role: MembershipRole.ADMIN } as any, + ]); + + // Member 3 is owner + vi.mocked(teamQueries.isTeamOwner) + .mockResolvedValueOnce(false) // member 2 + .mockResolvedValueOnce(true) // member 3 is owner + .mockResolvedValueOnce(false); // member 4 + + // PBAC service doesn't validate owner-to-owner removal + // It only prevents self-removal by owners + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + true + ) + ).resolves.not.toThrow(); + }); + }); + + describe("Edge Cases", () => { + it("should handle validation when hasPermission is false", async () => { + const userId = 1; + const memberIds = [2]; + const teamIds = [1]; + + // Should not perform validation if no permission + await expect( + service.validateRemoval( + { + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg: false, + }, + false // No permission + ) + ).resolves.not.toThrow(); + + // Should not check ownership or roles + expect(teamQueries.isTeamOwner).not.toHaveBeenCalled(); + expect(prisma.membership.findMany).not.toHaveBeenCalled(); + }); + }); + }); + + describe("removeMembers", () => { + it("should call TeamService.removeMembers with correct parameters", async () => { + const memberIds = [1, 2, 3]; + const teamIds = [4, 5]; + const isOrg = true; + + vi.mocked(TeamService.removeMembers).mockResolvedValue(undefined); + + await service.removeMembers(memberIds, teamIds, isOrg); + + expect(TeamService.removeMembers).toHaveBeenCalledWith({ + userIds: memberIds, + teamIds, + isOrg, + }); + }); + + it("should propagate errors from TeamService", async () => { + const memberIds = [1]; + const teamIds = [2]; + const isOrg = false; + + const error = new Error("Database connection failed"); + vi.mocked(TeamService.removeMembers).mockRejectedValue(error); + + await expect(service.removeMembers(memberIds, teamIds, isOrg)).rejects.toThrow( + "Database connection failed" + ); + }); + }); + + describe("Service Initialization", () => { + it("should create PermissionCheckService on instantiation", () => { + const newService = new PBACRemoveMemberService(); + + expect(PermissionCheckService).toHaveBeenCalled(); + }); + }); + + describe("Permission Service Integration", () => { + it("should handle permission service errors gracefully", async () => { + const userId = 1; + const teamIds = [1]; + const memberIds = [2]; + const isOrg = false; + + const removePermission = "team.remove"; + vi.mocked(PermissionMapper.toPermissionString).mockReturnValue(removePermission); + + mockPermissionCheckService.getTeamIdsWithPermission.mockRejectedValue( + new Error("Permission service unavailable") + ); + + await expect( + service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg, + }) + ).rejects.toThrow("Permission service unavailable"); + }); + + it("should handle empty permission results", async () => { + const userId = 1; + const teamIds = [1, 2]; + const memberIds = [3]; + const isOrg = false; + + const removePermission = "team.remove"; + vi.mocked(PermissionMapper.toPermissionString).mockReturnValue(removePermission); + mockPermissionCheckService.getTeamIdsWithPermission.mockResolvedValue(null as any); + + const result = await service.checkRemovePermissions({ + userId, + isOrgAdmin: false, + memberIds, + teamIds, + isOrg, + }); + + expect(result.hasPermission).toBe(false); + }); + }); +}); diff --git a/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/RemoveMemberServiceFactory.test.ts b/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/RemoveMemberServiceFactory.test.ts new file mode 100644 index 0000000000..6709079a31 --- /dev/null +++ b/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/RemoveMemberServiceFactory.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; + +import { FeaturesRepository } from "@calcom/features/flags/features.repository"; + +import { LegacyRemoveMemberService } from "../LegacyRemoveMemberService"; +import { PBACRemoveMemberService } from "../PBACRemoveMemberService"; +import { RemoveMemberServiceFactory } from "../RemoveMemberServiceFactory"; + +vi.mock("@calcom/features/flags/features.repository"); +vi.mock("../LegacyRemoveMemberService"); +vi.mock("../PBACRemoveMemberService"); + +describe("RemoveMemberServiceFactory", () => { + let mockFeaturesRepository: { + checkIfTeamHasFeature: vi.Mock; + }; + + beforeEach(() => { + vi.clearAllMocks(); + + mockFeaturesRepository = { + checkIfTeamHasFeature: vi.fn(), + }; + + vi.mocked(FeaturesRepository).mockImplementation(() => mockFeaturesRepository as any); + }); + + describe("Service Creation", () => { + it("should create LegacyRemoveMemberService when PBAC is disabled", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValue(false); + + const teamId = 1; + const service = await RemoveMemberServiceFactory.create(teamId); + + expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledWith(teamId, "pbac"); + expect(service).toBeInstanceOf(LegacyRemoveMemberService); + }); + + it("should create PBACRemoveMemberService when PBAC is enabled", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValue(true); + + const teamId = 1; + const service = await RemoveMemberServiceFactory.create(teamId); + + expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledWith(teamId, "pbac"); + expect(service).toBeInstanceOf(PBACRemoveMemberService); + }); + }); + + describe("Service Creation for Different Teams", () => { + it("should create different services for different teams", async () => { + // Team 1 has PBAC disabled + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(false); + // Team 2 has PBAC enabled + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true); + + const service1 = await RemoveMemberServiceFactory.create(1); + const service2 = await RemoveMemberServiceFactory.create(2); + + expect(service1).toBeInstanceOf(LegacyRemoveMemberService); + expect(service2).toBeInstanceOf(PBACRemoveMemberService); + expect(service1).not.toBe(service2); + + expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledTimes(2); + expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledWith(1, "pbac"); + expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledWith(2, "pbac"); + }); + + it("should create service for each call (no caching in current implementation)", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValue(true); + + const teamId = 1; + + // Make multiple calls + const service1 = await RemoveMemberServiceFactory.create(teamId); + const service2 = await RemoveMemberServiceFactory.create(teamId); + const service3 = await RemoveMemberServiceFactory.create(teamId); + + // All should be different instances (no caching currently) + expect(service1).not.toBe(service2); + expect(service2).not.toBe(service3); + + // Feature flag should be called each time + expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledTimes(3); + }); + }); + + describe("Error Handling", () => { + it("should propagate errors from features repository", async () => { + const error = new Error("Features repository error"); + mockFeaturesRepository.checkIfTeamHasFeature.mockRejectedValue(error); + + await expect(RemoveMemberServiceFactory.create(1)).rejects.toThrow("Features repository error"); + }); + + it("should handle null/undefined feature flag gracefully", async () => { + mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValue(null as any); + + const service = await RemoveMemberServiceFactory.create(1); + + // Should default to Legacy when feature flag is falsy + expect(service).toBeInstanceOf(LegacyRemoveMemberService); + }); + }); +}); diff --git a/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/removeMember.handler.test.ts b/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/removeMember.handler.test.ts new file mode 100644 index 0000000000..c8a3574391 --- /dev/null +++ b/packages/trpc/server/routers/viewer/teams/removeMember/__tests__/removeMember.handler.test.ts @@ -0,0 +1,308 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; + +import { checkRateLimitAndThrowError } from "@calcom/lib/checkRateLimitAndThrowError"; + +import { TRPCError } from "@trpc/server"; + +import { removeMemberHandler } from "../../removeMember.handler"; +import type { IRemoveMemberService } from "../IRemoveMemberService"; +import { RemoveMemberServiceFactory } from "../RemoveMemberServiceFactory"; + +vi.mock("@calcom/lib/checkRateLimitAndThrowError"); +vi.mock("../RemoveMemberServiceFactory"); + +describe("removeMemberHandler", () => { + const mockService: IRemoveMemberService = { + checkRemovePermissions: vi.fn(), + validateRemoval: vi.fn(), + removeMembers: vi.fn(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(checkRateLimitAndThrowError).mockResolvedValue({ + success: true, + remaining: 99, + limit: 100, + reset: new Date().getTime() + 60 * 1000, + }); + vi.mocked(RemoveMemberServiceFactory.create).mockResolvedValue(mockService); + vi.mocked(mockService.checkRemovePermissions).mockResolvedValue({ + hasPermission: true, + userRoles: new Map(), + }); + }); + + describe("Rate Limiting", () => { + it("should check rate limit before processing", async () => { + const userId = 1; + const input = { + teamIds: [1], + memberIds: [2], + isOrg: false, + }; + + await removeMemberHandler({ + ctx: { user: { id: userId } }, + input, + }); + + expect(checkRateLimitAndThrowError).toHaveBeenCalledWith({ + identifier: `removeMember.${userId}`, + }); + }); + }); + + describe("Input Validation", () => { + it("should throw BAD_REQUEST when no team IDs provided", async () => { + const input = { + teamIds: [], + memberIds: [2], + isOrg: false, + }; + + await expect( + removeMemberHandler({ + ctx: { user: { id: 1 } }, + input, + }) + ).rejects.toThrow( + expect.objectContaining({ + code: "BAD_REQUEST", + message: "At least one team ID must be provided", + }) + ); + }); + }); + + describe("Service Factory Integration", () => { + it("should create service using primary team ID", async () => { + const primaryTeamId = 1; + const input = { + teamIds: [primaryTeamId, 2, 3], + memberIds: [4, 5], + isOrg: false, + }; + + mockService.checkRemovePermissions = vi.fn().mockResolvedValue({ hasPermission: true }); + + await removeMemberHandler({ + ctx: { user: { id: 1 } }, + input, + }); + + expect(RemoveMemberServiceFactory.create).toHaveBeenCalledWith(primaryTeamId); + }); + }); + + describe("Permission Checking", () => { + it("should pass org admin status to permission check", async () => { + const userId = 1; + const isOrgAdmin = true; + const input = { + teamIds: [1], + memberIds: [2], + isOrg: true, + }; + + mockService.checkRemovePermissions = vi.fn().mockResolvedValue({ hasPermission: true }); + + await removeMemberHandler({ + ctx: { + user: { + id: userId, + organization: { isOrgAdmin }, + }, + }, + input, + }); + + expect(mockService.checkRemovePermissions).toHaveBeenCalledWith({ + userId, + isOrgAdmin, + memberIds: input.memberIds, + teamIds: input.teamIds, + isOrg: input.isOrg, + }); + }); + + it("should default isOrgAdmin to false when not provided", async () => { + const userId = 1; + const input = { + teamIds: [1], + memberIds: [2], + isOrg: false, + }; + + mockService.checkRemovePermissions = vi.fn().mockResolvedValue({ hasPermission: true }); + + await removeMemberHandler({ + ctx: { user: { id: userId } }, + input, + }); + + expect(mockService.checkRemovePermissions).toHaveBeenCalledWith( + expect.objectContaining({ + isOrgAdmin: false, + }) + ); + }); + + it("should throw UNAUTHORIZED when user lacks permission", async () => { + const input = { + teamIds: [1], + memberIds: [2], + isOrg: false, + }; + + mockService.checkRemovePermissions = vi.fn().mockResolvedValue({ hasPermission: false }); + + await expect( + removeMemberHandler({ + ctx: { user: { id: 1 } }, + input, + }) + ).rejects.toThrow( + expect.objectContaining({ + code: "UNAUTHORIZED", + }) + ); + + expect(mockService.validateRemoval).not.toHaveBeenCalled(); + expect(mockService.removeMembers).not.toHaveBeenCalled(); + }); + }); + + describe("Removal Flow", () => { + it("should complete full removal flow when user has permission", async () => { + const userId = 1; + const isOrgAdmin = false; + const input = { + teamIds: [1, 2], + memberIds: [3, 4], + isOrg: false, + }; + + const hasPermissionResult = { + hasPermission: true, + userRoles: new Map([ + [1, "ADMIN"], + [2, "OWNER"], + ]), + }; + + mockService.checkRemovePermissions = vi.fn().mockResolvedValue(hasPermissionResult); + mockService.validateRemoval = vi.fn().mockResolvedValue(undefined); + mockService.removeMembers = vi.fn().mockResolvedValue(undefined); + + await removeMemberHandler({ + ctx: { user: { id: userId } }, + input, + }); + + // Verify service calls in order + expect(mockService.checkRemovePermissions).toHaveBeenCalledWith({ + userId, + isOrgAdmin, + memberIds: input.memberIds, + teamIds: input.teamIds, + isOrg: input.isOrg, + }); + + expect(mockService.validateRemoval).toHaveBeenCalledWith( + { + userId, + isOrgAdmin, + memberIds: input.memberIds, + teamIds: input.teamIds, + isOrg: input.isOrg, + }, + hasPermissionResult.hasPermission + ); + + expect(mockService.removeMembers).toHaveBeenCalledWith(input.memberIds, input.teamIds, input.isOrg); + }); + + it("should handle org admin removing members from teams they are not part of", async () => { + const userId = 1; + const isOrgAdmin = true; + const input = { + teamIds: [100, 200], // Teams the org admin is not part of + memberIds: [3, 4], + isOrg: true, + }; + + const hasPermissionResult = { hasPermission: true }; + + mockService.checkRemovePermissions = vi.fn().mockResolvedValue(hasPermissionResult); + mockService.validateRemoval = vi.fn().mockResolvedValue(undefined); + mockService.removeMembers = vi.fn().mockResolvedValue(undefined); + + await removeMemberHandler({ + ctx: { + user: { + id: userId, + organization: { isOrgAdmin }, + }, + }, + input, + }); + + expect(mockService.checkRemovePermissions).toHaveBeenCalledWith({ + userId, + isOrgAdmin: true, + memberIds: input.memberIds, + teamIds: input.teamIds, + isOrg: input.isOrg, + }); + + expect(mockService.removeMembers).toHaveBeenCalled(); + }); + }); + + describe("Error Handling", () => { + it("should propagate validation errors", async () => { + const input = { + teamIds: [1], + memberIds: [2], + isOrg: false, + }; + + mockService.checkRemovePermissions = vi.fn().mockResolvedValue({ hasPermission: true }); + mockService.validateRemoval = vi + .fn() + .mockRejectedValue(new TRPCError({ code: "BAD_REQUEST", message: "Cannot remove owner" })); + + await expect( + removeMemberHandler({ + ctx: { user: { id: 1 } }, + input, + }) + ).rejects.toThrow( + expect.objectContaining({ + code: "BAD_REQUEST", + message: "Cannot remove owner", + }) + ); + }); + + it("should propagate service removal errors", async () => { + const input = { + teamIds: [1], + memberIds: [2], + isOrg: false, + }; + + mockService.checkRemovePermissions = vi.fn().mockResolvedValue({ hasPermission: true }); + mockService.validateRemoval = vi.fn().mockResolvedValue(undefined); + mockService.removeMembers = vi.fn().mockRejectedValue(new Error("Database error")); + + await expect( + removeMemberHandler({ + ctx: { user: { id: 1 } }, + input, + }) + ).rejects.toThrow("Database error"); + }); + }); +}); diff --git a/packages/trpc/server/routers/viewer/teams/removeMember/index.ts b/packages/trpc/server/routers/viewer/teams/removeMember/index.ts new file mode 100644 index 0000000000..bcafd99007 --- /dev/null +++ b/packages/trpc/server/routers/viewer/teams/removeMember/index.ts @@ -0,0 +1,9 @@ +export { RemoveMemberServiceFactory } from "./RemoveMemberServiceFactory"; +export type { + IRemoveMemberService, + RemoveMemberContext, + RemoveMemberPermissionResult, +} from "./IRemoveMemberService"; +export { BaseRemoveMemberService } from "./BaseRemoveMemberService"; +export { PBACRemoveMemberService } from "./PBACRemoveMemberService"; +export { LegacyRemoveMemberService } from "./LegacyRemoveMemberService";