Fix filter on invites (#25330)
Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>
This commit is contained in:
co-authored by
Udit Takkar
parent
0af77be127
commit
0d77fc8474
@@ -1,4 +1,4 @@
|
||||
import { describe, it, vi, expect } from "vitest";
|
||||
import { describe, it, vi, expect, beforeEach } from "vitest";
|
||||
|
||||
import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
|
||||
import { MembershipRole } from "@calcom/prisma/enums";
|
||||
@@ -16,11 +16,21 @@ import {
|
||||
canBeInvited,
|
||||
getAutoJoinStatus,
|
||||
checkInputEmailIsValid,
|
||||
createMemberships,
|
||||
} from "./utils";
|
||||
|
||||
const { mockCreateMany } = vi.hoisted(() => {
|
||||
const mockCreateManyFn = vi.fn();
|
||||
return { mockCreateMany: mockCreateManyFn };
|
||||
});
|
||||
|
||||
vi.mock("@calcom/prisma", () => {
|
||||
return {
|
||||
prisma: vi.fn(),
|
||||
prisma: {
|
||||
membership: {
|
||||
createMany: mockCreateMany,
|
||||
},
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
@@ -37,6 +47,27 @@ vi.mock("@calcom/features/pbac/services/permission-check.service", () => {
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("@calcom/lib/logger", () => {
|
||||
const mockSubLogger = {
|
||||
debug: vi.fn(),
|
||||
error: vi.fn(),
|
||||
log: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
info: vi.fn(),
|
||||
getSubLogger: vi.fn(() => mockSubLogger),
|
||||
};
|
||||
return {
|
||||
default: {
|
||||
getSubLogger: vi.fn(() => mockSubLogger),
|
||||
error: vi.fn(),
|
||||
debug: vi.fn(),
|
||||
log: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
info: vi.fn(),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
const mockedRegularTeam: TeamWithParent = {
|
||||
id: 1,
|
||||
name: "Team A",
|
||||
@@ -609,6 +640,178 @@ describe("Invite Member Utils", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("createMemberships - Privilege Escalation Prevention", () => {
|
||||
beforeEach(() => {
|
||||
mockCreateMany.mockClear();
|
||||
mockCreateMany.mockResolvedValue({ count: 0 });
|
||||
});
|
||||
|
||||
it("should NOT escalate privilege when attacker has OWNER role in unrelated team", async () => {
|
||||
const attackerTeamId = 999; // Attacker's personal team
|
||||
const victimTeamId = 100; // Victim team being invited to
|
||||
const victimOrgId = 200; // Parent organization of victim team
|
||||
|
||||
const attacker: UserWithMembership & {
|
||||
newRole: MembershipRole;
|
||||
needToCreateOrgMembership: boolean | null;
|
||||
} = {
|
||||
...mockUser,
|
||||
id: 1,
|
||||
email: "attacker@example.com",
|
||||
username: "attacker",
|
||||
teams: [
|
||||
// Attacker controls this - OWNER role in their personal team (first in array)
|
||||
{ teamId: attackerTeamId, userId: 1, accepted: true, role: MembershipRole.OWNER },
|
||||
// Some other unrelated membership
|
||||
{ teamId: 50, userId: 1, accepted: true, role: MembershipRole.MEMBER },
|
||||
],
|
||||
newRole: MembershipRole.MEMBER, // Inviter wants to invite as MEMBER
|
||||
needToCreateOrgMembership: true,
|
||||
};
|
||||
|
||||
await createMemberships({
|
||||
teamId: victimTeamId,
|
||||
language: "en",
|
||||
invitees: [attacker],
|
||||
parentId: victimOrgId,
|
||||
accepted: false,
|
||||
});
|
||||
|
||||
// Verify createMany was called
|
||||
expect(mockCreateMany).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Get the data that was passed to createMany
|
||||
const callArgs = mockCreateMany.mock.calls[0][0];
|
||||
const createdMemberships = callArgs.data;
|
||||
|
||||
// Should create 2 memberships: one for team, one for org
|
||||
expect(createdMemberships).toHaveLength(2);
|
||||
|
||||
// Check team membership - should be MEMBER (inviter's choice), NOT OWNER
|
||||
const teamMembership = createdMemberships.find((m: any) => m.teamId === victimTeamId);
|
||||
expect(teamMembership).toBeDefined();
|
||||
expect(teamMembership.role).toBe(MembershipRole.MEMBER);
|
||||
expect(teamMembership.userId).toBe(attacker.id);
|
||||
expect(teamMembership.accepted).toBe(false);
|
||||
|
||||
// Check org membership
|
||||
const orgMembership = createdMemberships.find((m: any) => m.teamId === victimOrgId);
|
||||
expect(orgMembership).toBeDefined();
|
||||
expect(orgMembership.role).toBe(MembershipRole.MEMBER);
|
||||
});
|
||||
|
||||
it("should preserve ADMIN role when user is already ADMIN in parent organization", async () => {
|
||||
const teamId = 100;
|
||||
const parentOrgId = 200;
|
||||
|
||||
const existingAdmin: UserWithMembership & {
|
||||
newRole: MembershipRole;
|
||||
needToCreateOrgMembership: boolean | null;
|
||||
} = {
|
||||
...mockUser,
|
||||
id: 2,
|
||||
email: "admin@example.com",
|
||||
username: "admin",
|
||||
teams: [
|
||||
// User is already ADMIN in the parent org
|
||||
{ teamId: parentOrgId, userId: 2, accepted: true, role: MembershipRole.ADMIN },
|
||||
],
|
||||
newRole: MembershipRole.MEMBER, // Inviter wants MEMBER, but should preserve ADMIN
|
||||
needToCreateOrgMembership: false, // Already has org membership
|
||||
};
|
||||
|
||||
await createMemberships({
|
||||
teamId,
|
||||
language: "en",
|
||||
invitees: [existingAdmin],
|
||||
parentId: parentOrgId,
|
||||
accepted: true,
|
||||
});
|
||||
|
||||
const callArgs = mockCreateMany.mock.calls[0][0];
|
||||
const createdMemberships = callArgs.data;
|
||||
|
||||
// Should only create team membership (org membership already exists)
|
||||
expect(createdMemberships).toHaveLength(1);
|
||||
|
||||
// Should preserve ADMIN role since user is ADMIN in parent org
|
||||
const teamMembership = createdMemberships.find((m: any) => m.teamId === teamId);
|
||||
expect(teamMembership.role).toBe(MembershipRole.ADMIN);
|
||||
});
|
||||
|
||||
it("should use inviter's role when user has no membership in parent organization", async () => {
|
||||
const teamId = 100;
|
||||
const parentOrgId = 200;
|
||||
const unrelatedTeamId = 999;
|
||||
|
||||
const user: UserWithMembership & {
|
||||
newRole: MembershipRole;
|
||||
needToCreateOrgMembership: boolean | null;
|
||||
} = {
|
||||
...mockUser,
|
||||
id: 3,
|
||||
email: "user@example.com",
|
||||
username: "user",
|
||||
teams: [
|
||||
// User has OWNER in unrelated team, but NOT in parent org
|
||||
{ teamId: unrelatedTeamId, userId: 3, accepted: true, role: MembershipRole.OWNER },
|
||||
],
|
||||
newRole: MembershipRole.MEMBER,
|
||||
needToCreateOrgMembership: true,
|
||||
};
|
||||
|
||||
await createMemberships({
|
||||
teamId,
|
||||
language: "en",
|
||||
invitees: [user],
|
||||
parentId: parentOrgId,
|
||||
accepted: false,
|
||||
});
|
||||
|
||||
const callArgs = mockCreateMany.mock.calls[0][0];
|
||||
const createdMemberships = callArgs.data;
|
||||
|
||||
const teamMembership = createdMemberships.find((m: any) => m.teamId === teamId);
|
||||
// Should use inviter's chosen role (MEMBER), not OWNER from unrelated team
|
||||
expect(teamMembership.role).toBe(MembershipRole.MEMBER);
|
||||
});
|
||||
|
||||
it("should use inviter's role when inviting to team without parent organization", async () => {
|
||||
const teamId = 100;
|
||||
|
||||
const user: UserWithMembership & {
|
||||
newRole: MembershipRole;
|
||||
needToCreateOrgMembership: boolean | null;
|
||||
} = {
|
||||
...mockUser,
|
||||
id: 4,
|
||||
email: "user2@example.com",
|
||||
username: "user2",
|
||||
teams: [
|
||||
// User has OWNER in unrelated team
|
||||
{ teamId: 999, userId: 4, accepted: true, role: MembershipRole.OWNER },
|
||||
],
|
||||
newRole: MembershipRole.MEMBER,
|
||||
needToCreateOrgMembership: null,
|
||||
};
|
||||
|
||||
await createMemberships({
|
||||
teamId,
|
||||
language: "en",
|
||||
invitees: [user],
|
||||
parentId: null, // No parent org
|
||||
accepted: false,
|
||||
});
|
||||
|
||||
const callArgs = mockCreateMany.mock.calls[0][0];
|
||||
const createdMemberships = callArgs.data;
|
||||
|
||||
expect(createdMemberships).toHaveLength(1);
|
||||
// Should use inviter's chosen role when no parentId
|
||||
expect(createdMemberships[0].role).toBe(MembershipRole.MEMBER);
|
||||
});
|
||||
});
|
||||
});
|
||||
function getSampleProfile({ organizationId }: { organizationId?: number } = {}): {
|
||||
id: number;
|
||||
|
||||
@@ -5,11 +5,11 @@ import { getOrgFullOrigin } from "@calcom/ee/organizations/lib/orgDomains";
|
||||
import { sendTeamInviteEmail } from "@calcom/emails/organization-email-service";
|
||||
import { checkAdminOrOwner } from "@calcom/features/auth/lib/checkAdminOrOwner";
|
||||
import { updateNewTeamMemberEventTypes } from "@calcom/features/ee/teams/lib/queries";
|
||||
import { OnboardingPathService } from "@calcom/features/onboarding/lib/onboarding-path.service";
|
||||
import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
|
||||
import { createAProfileForAnExistingUser } from "@calcom/features/profile/lib/createAProfileForAnExistingUser";
|
||||
import { ProfileRepository } from "@calcom/features/profile/repositories/ProfileRepository";
|
||||
import { UserRepository } from "@calcom/features/users/repositories/UserRepository";
|
||||
import { OnboardingPathService } from "@calcom/features/onboarding/lib/onboarding-path.service";
|
||||
import { DEFAULT_SCHEDULE, getAvailabilityFromSchedule } from "@calcom/lib/availability";
|
||||
import { ENABLE_PROFILE_SWITCHER, WEBAPP_URL } from "@calcom/lib/constants";
|
||||
import logger from "@calcom/lib/logger";
|
||||
@@ -437,7 +437,9 @@ export async function createMemberships({
|
||||
try {
|
||||
await prisma.membership.createMany({
|
||||
data: invitees.flatMap((invitee) => {
|
||||
const organizationRole = invitee?.teams?.[0]?.role;
|
||||
const organizationRole = parentId
|
||||
? invitee?.teams?.find((membership) => membership.teamId === parentId)?.role
|
||||
: undefined;
|
||||
const data = [];
|
||||
const createdAt = new Date();
|
||||
// membership for the team
|
||||
|
||||
Reference in New Issue
Block a user