fix: Issues observed when inviting members to organization either through teams migration during onboarding or explicit user invite (#22901)

* fix: make team migrations sequential to avoid race conditions Instead of processing team migrations in parallel using Promise.all, process them sequentially to avoid race conditions when creating profiles and memberships for team members. This simplifies the implementation and avoids the need for complex retry logic or database-level concurrency controls. * Promise.all accepts an array of promises and not an array with array in it as the first item. This fixes the issue where these promises were unhandled * Fix the issue where an existing pending membership with team stays pending even though theorganization membership gets created as accepted
2025-08-13 10:48:17 +00:00
1 parent 94a04f0dc2
commit b7fda31678
@@ -95,21 +95,18 @@ export const createTeamsHandler = async ({ ctx, input }: CreateTeamsOptions) =>
    teamNames.map((item) => slugify(item)).includes(slug)
  );

-  await Promise.all(
-    moveTeams
-      .filter((team) => team.shouldMove)
-      .map(async ({ id: teamId, newSlug }) => {
-        await moveTeam({
-          teamId,
-          newSlug,
-          org: {
-            ...organization,
-            ownerId: organizationOwner.id,
-          },
-          creationSource,
-        });
-      })
-  );
+  // Process team migrations sequentially to avoid race conditions - Moving a team invites members to the organization again and there are known unique constraints failure in membership and profile creation if done in parallel and a user happens to be part of more than one team
+  for (const team of moveTeams.filter((team) => team.shouldMove)) {
+    await moveTeam({
+      teamId: team.id,
+      newSlug: team.newSlug,
+      org: {
+        ...organization,
+        ownerId: organizationOwner.id,
+      },
+      creationSource,
+    });
+  }

  if (duplicatedSlugs.length === teamNames.length) {
    return { duplicatedSlugs };
@@ -0,0 +1,438 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
+
+import { prisma } from "@calcom/prisma";
+import type { Team, User, Membership, Profile } from "@calcom/prisma/client";
+import { MembershipRole } from "@calcom/prisma/enums";
+import type { TrpcSessionUser } from "@calcom/trpc/server/types";
+
+import inviteMemberHandler, { inviteMembersWithNoInviterPermissionCheck } from "./inviteMember.handler";
+
+// Helper functions for database verification
+async function verifyProfileExists(userId: number, organizationId: number): Promise<Profile | null> {
+  return await prisma.profile.findUnique({
+    where: {
+      userId_organizationId: {
+        userId,
+        organizationId,
+      },
+    },
+  });
+}
+
+async function verifyMembershipExists(userId: number, teamId: number): Promise<Membership | null> {
+  return await prisma.membership.findUnique({
+    where: {
+      userId_teamId: {
+        userId,
+        teamId,
+      },
+    },
+  });
+}
+
+async function verifyUserOrganizationConsistency(userId: number): Promise<{
+  profileCount: number;
+  acceptedMembershipCount: number;
+  pendingMembershipCount: number;
+}> {
+  const profiles = await prisma.profile.count({ where: { userId } });
+  const acceptedMemberships = await prisma.membership.count({
+    where: { userId, accepted: true },
+  });
+  const pendingMemberships = await prisma.membership.count({
+    where: { userId, accepted: false },
+  });
+
+  return {
+    profileCount: profiles,
+    acceptedMembershipCount: acceptedMemberships,
+    pendingMembershipCount: pendingMemberships,
+  };
+}
+
+// Test data creation helpers with unique identifiers
+function generateUniqueId() {
+  return `${Date.now()}-${Math.random().toString(36).substring(7)}`;
+}
+
+async function createTestUser(data: { email: string; username: string; name?: string }): Promise<User> {
+  const uniqueId = generateUniqueId();
+  const uniqueEmail = data.email.includes("@")
+    ? data.email.replace("@", `-${uniqueId}@`)
+    : `${data.email}-${uniqueId}@example.com`;
+  const uniqueUsername = `${data.username}-${uniqueId}`;
+
+  // Pre-emptive cleanup to avoid conflicts
+  try {
+    await prisma.user.deleteMany({
+      where: { OR: [{ email: uniqueEmail }, { username: uniqueUsername }] },
+    });
+  } catch {}
+
+  return await prisma.user.create({
+    data: {
+      email: uniqueEmail,
+      username: uniqueUsername,
+      name: data.name || uniqueUsername,
+    },
+  });
+}
+
+async function createTestTeam(data: {
+  name: string;
+  slug: string;
+  isOrganization?: boolean;
+  parentId?: number;
+  metadata?: any;
+  organizationSettings?: {
+    orgAutoAcceptEmail?: string;
+    isOrganizationVerified?: boolean;
+  };
+}): Promise<Team> {
+  const uniqueId = generateUniqueId();
+  const uniqueSlug = `${data.slug}-${uniqueId}`;
+
+  // Pre-emptive cleanup to avoid conflicts
+  try {
+    await prisma.team.deleteMany({
+      where: { slug: uniqueSlug },
+    });
+  } catch {
+    // Ignore cleanup errors
+  }
+
+  const team = await prisma.team.create({
+    data: {
+      name: `${data.name} ${uniqueId}`,
+      slug: uniqueSlug,
+      isOrganization: data.isOrganization || false,
+      parentId: data.parentId,
+      metadata: data.metadata,
+    },
+  });
+
+  // If this is an organization and organizationSettings are provided, create them
+  if (data.isOrganization && data.organizationSettings) {
+    await prisma.organizationSettings.create({
+      data: {
+        organizationId: team.id,
+        orgAutoAcceptEmail: data.organizationSettings.orgAutoAcceptEmail,
+        isOrganizationVerified: data.organizationSettings.isOrganizationVerified,
+      },
+    });
+  }
+
+  return team;
+}
+
+// Mock translations fetch
+vi.mock("node-fetch", () => ({
+  default: vi.fn(() =>
+    Promise.resolve({
+      ok: true,
+      json: () => Promise.resolve({}),
+    })
+  ),
+}));
+
+// Mock email sending
+vi.mock("@calcom/emails", () => ({
+  sendTeamInviteEmail: vi.fn(() => Promise.resolve()),
+  sendOrganizationAutoJoinEmail: vi.fn(() => Promise.resolve()),
+}));
+
+// Mock for getTranslation
+vi.mock("@calcom/lib/server/i18n", () => ({
+  getTranslation: vi.fn(() => Promise.resolve((key: string) => key)),
+}));
+
+describe("inviteMember.handler Integration Tests", () => {
+  // Track created test data for cleanup
+  let testUsers: User[] = [];
+  let testTeams: Team[] = [];
+
+  beforeEach(async () => {
+    // Reset tracking arrays
+    testUsers = [];
+    testTeams = [];
+  });
+
+  afterEach(async () => {
+    // Clean up test data in reverse dependency order
+    const userIds = testUsers.map((u) => u.id);
+    const teamIds = testTeams.map((t) => t.id);
+
+    if (userIds.length > 0 || teamIds.length > 0) {
+      await prisma.$transaction([
+        // Clean verification tokens for test users
+        prisma.verificationToken.deleteMany({
+          where: { identifier: { in: testUsers.map((u) => u.email) } },
+        }),
+        // Clean memberships
+        prisma.membership.deleteMany({
+          where: {
+            OR: [{ userId: { in: userIds } }, { teamId: { in: teamIds } }],
+          },
+        }),
+        // Clean profiles
+        prisma.profile.deleteMany({
+          where: {
+            OR: [{ userId: { in: userIds } }, { organizationId: { in: teamIds } }],
+          },
+        }),
+        // Clean teams
+        prisma.team.deleteMany({
+          where: { id: { in: teamIds } },
+        }),
+        // Clean users
+        prisma.user.deleteMany({
+          where: { id: { in: userIds } },
+        }),
+      ]);
+    }
+  });
+
+  // Helper to track created entities
+  function trackUser(user: User): User {
+    testUsers.push(user);
+    return user;
+  }
+
+  function trackTeam(team: Team): Team {
+    testTeams.push(team);
+    return team;
+  }
+
+  // Helper to create a proper user context for inviteMemberHandler
+  function createUserContext(user: User, organizationId: number | null = null): NonNullable<TrpcSessionUser> {
+    const baseUser = {
+      id: user.id,
+      email: user.email,
+      username: user.username || "",
+      name: user.name,
+      emailVerified: user.emailVerified,
+      avatarUrl: user.avatarUrl,
+      avatar: `https://example.com/avatar.png`,
+      locale: user.locale || "en",
+      timeZone: user.timeZone || "UTC",
+      role: user.role,
+      allowDynamicBooking: user.allowDynamicBooking || true,
+      completedOnboarding: user.completedOnboarding,
+      twoFactorEnabled: user.twoFactorEnabled,
+      identityProvider: user.identityProvider,
+      brandColor: user.brandColor,
+      darkBrandColor: user.darkBrandColor,
+      theme: user.theme,
+      createdDate: user.createdDate,
+      disableImpersonation: user.disableImpersonation,
+      movedToProfileId: user.movedToProfileId,
+      organizationId,
+      organization: organizationId
+        ? {
+            id: organizationId,
+            isOrgAdmin: true,
+            metadata: {},
+            requestedSlug: null,
+          }
+        : {
+            id: null,
+            isOrgAdmin: false,
+            metadata: {},
+            requestedSlug: null,
+          },
+      defaultBookerLayouts: null,
+      profile: organizationId
+        ? {
+            id: user.id,
+            upId: `${user.username}-${organizationId}`,
+            username: user.username || "",
+            userId: user.id,
+            organizationId,
+            organization: {
+              id: organizationId,
+              name: "Test Organization",
+              slug: "test-org",
+              isOrganization: true,
+              metadata: {},
+            },
+          }
+        : {
+            id: user.id,
+            upId: `${user.username}-default`,
+            username: user.username || "",
+            userId: user.id,
+            organizationId: null,
+            organization: null,
+          },
+    };
+
+    return baseUser as unknown as NonNullable<TrpcSessionUser>;
+  }
+
+  describe("Organization Direct Invite Flow", () => {
+    it("should not auto-accept user's membership that was unaccepted when migrating to org with non-matching autoAcceptEmailDomain", async () => {
+      const organization = trackTeam(
+        await createTestTeam({
+          name: "Test Organization",
+          slug: "test-org",
+          isOrganization: true,
+          metadata: {},
+          organizationSettings: {
+            orgAutoAcceptEmail: "company.com",
+            isOrganizationVerified: true,
+          },
+        })
+      );
+
+      const team = trackTeam(
+        await createTestTeam({
+          name: "Test Team",
+          slug: "test-team",
+          isOrganization: false,
+          parentId: organization.id,
+        })
+      );
+
+      const inviterUser = trackUser(
+        await createTestUser({
+          email: "inviter@company.com",
+          username: "inviter",
+        })
+      );
+
+      await prisma.membership.create({
+        data: {
+          userId: inviterUser.id,
+          teamId: organization.id,
+          role: MembershipRole.OWNER,
+          accepted: true,
+        },
+      });
+
+      // Act: Invite user with non-matching domain
+      const nonMatchingUser = trackUser(
+        await createTestUser({
+          email: "external@other.com",
+          username: "external",
+        })
+      );
+
+      await prisma.membership.create({
+        data: {
+          userId: nonMatchingUser.id,
+          teamId: team.id,
+          role: MembershipRole.MEMBER,
+          accepted: false,
+        },
+      });
+
+      await inviteMemberHandler({
+        ctx: {
+          user: createUserContext(inviterUser, organization.id),
+          session: {} as any,
+        } as any,
+        input: {
+          teamId: organization.id,
+          usernameOrEmail: nonMatchingUser.email,
+          role: MembershipRole.MEMBER,
+          language: "en",
+          creationSource: "WEBAPP" as const,
+        },
+      });
+
+      // Assert: No profile should be created
+      const profile = await verifyProfileExists(nonMatchingUser.id, organization.id);
+      expect(profile).toBeFalsy();
+
+      // Membership should not be auto-accepted
+      const membership = await verifyMembershipExists(nonMatchingUser.id, organization.id);
+      expect(membership).toBeTruthy();
+      expect(membership?.accepted).toBe(false);
+
+      // Verify that the user is not a member of the organization
+      const nonMatchingUserMembershipWithTeam = await verifyMembershipExists(nonMatchingUser.id, team.id);
+      expect(nonMatchingUserMembershipWithTeam).toBeTruthy();
+      expect(nonMatchingUserMembershipWithTeam?.accepted).toBe(false);
+    });
+
+    it("should auto-accept user's membership that was unaccepted when migrating to org with matching autoAcceptEmailDomain", async () => {
+      // Setup: User with unverified email who has an unaccepted team membership
+      const unverifiedUserWithUnacceptedMembership = trackUser(
+        await createTestUser({
+          email: "john.doe@acme.com",
+          username: "john.doe",
+        })
+      );
+
+      // Add unverified email status - will need to update in DB
+      await prisma.user.update({
+        where: { id: unverifiedUserWithUnacceptedMembership.id },
+        data: { emailVerified: null },
+      });
+
+      const organization = trackTeam(
+        await createTestTeam({
+          name: "Acme",
+          slug: "acme",
+          isOrganization: true,
+          metadata: {},
+          organizationSettings: {
+            orgAutoAcceptEmail: "acme.com", // Matches user's email domain
+            isOrganizationVerified: true,
+          },
+        })
+      );
+
+      // Create a regular team first where user has unaccepted membership
+      const regularTeam = trackTeam(
+        await createTestTeam({
+          name: "Regular Team",
+          slug: "regular-team",
+          isOrganization: false,
+          parentId: organization.id,
+        })
+      );
+
+      // Add unaccepted team membership
+      await prisma.membership.create({
+        data: {
+          teamId: regularTeam.id,
+          userId: unverifiedUserWithUnacceptedMembership.id,
+          accepted: false, // KEY: Unaccepted membership
+          role: MembershipRole.MEMBER,
+        },
+      });
+
+      // Act: Simulate migration by inviting the user to the organization
+      await inviteMembersWithNoInviterPermissionCheck({
+        inviterName: null,
+        teamId: organization.id,
+        language: "en",
+        creationSource: "WEBAPP" as const,
+        orgSlug: organization.slug,
+        invitations: [
+          {
+            usernameOrEmail: unverifiedUserWithUnacceptedMembership.email,
+            role: MembershipRole.MEMBER,
+          },
+        ],
+      });
+
+      const orgMembership = await verifyMembershipExists(
+        unverifiedUserWithUnacceptedMembership.id,
+        organization.id
+      );
+      const profile = await verifyProfileExists(unverifiedUserWithUnacceptedMembership.id, organization.id);
+
+      expect(profile?.userId).toBe(unverifiedUserWithUnacceptedMembership.id);
+      expect(profile?.organizationId).toBe(organization.id);
+
+      // Verify original team membership is accepted now
+      const originalMembership = await verifyMembershipExists(
+        unverifiedUserWithUnacceptedMembership.id,
+        regularTeam.id
+      );
+      expect(originalMembership?.accepted).toBe(true);
+    });
+  });
+});
@@ -1,3 +1,8 @@
+/**
+ * This file is deprecated in favour of inviteMember.handler.integration-test.ts
+ *
+ * It mocks a lot of things that are untested and integration tests make more sense for this handler
+ */
 import { scenarios as checkRateLimitAndThrowErrorScenarios } from "../../../../../../../tests/libs/__mocks__/checkRateLimitAndThrowError";
 import { mock as getTranslationMock } from "../../../../../../../tests/libs/__mocks__/getTranslation";
 import {
@@ -409,7 +414,7 @@ describe("inviteMemberHandler", () => {

      // Call inviteMembersWithNoInviterPermissionCheck directly with isDirectUserAction=false
      const { inviteMembersWithNoInviterPermissionCheck } = await import("./inviteMember.handler");
-      
+
      const result = await inviteMembersWithNoInviterPermissionCheck({
        inviterName: loggedInUser.name,
        teamId: team.id,
@@ -487,7 +492,7 @@ describe("inviteMemberHandler", () => {

      // Call inviteMembersWithNoInviterPermissionCheck directly with isDirectUserAction=false
      const { inviteMembersWithNoInviterPermissionCheck } = await import("./inviteMember.handler");
-      
+
      const result = await inviteMembersWithNoInviterPermissionCheck({
        inviterName: "Test Inviter",
        teamId: team.id,
@@ -517,6 +522,7 @@ describe("inviteMemberHandler", () => {
      );
    });
  });
+
  it("When rate limit exceeded, it should throw error", async () => {
    const userToBeInvited = buildExistingUser({
      id: 1,
@@ -39,6 +39,9 @@ const mockedReturnSuccessCheckPerms = {
  role: MembershipRole.ADMIN,
  userId: 1,
  teamId: 1,
+  createdAt: null,
+  updatedAt: null,
+  customRoleId: null,
  team: {
    id: 1,
    name: "Team A",
@@ -95,6 +98,11 @@ const mockedRegularTeam: TeamWithParent = {
  smsLockState: "LOCKED",
  createdByOAuthClientId: null,
  smsLockReviewedByAdmin: false,
+  hideTeamProfileLink: false,
+  rrResetInterval: null,
+  rrTimestampBasis: "CREATED_AT",
+  bookingLimits: null,
+  includeManagedEventsInLimits: false,
 };

 const mockedSubTeam = {
@@ -263,9 +271,13 @@ describe("Invite Member Utils", () => {
          adminGetsNoSlotsNotification: false,
          isAdminReviewed: false,
          isAdminAPIEnabled: false,
+          allowSEOIndexing: false,
+          orgProfileRedirectsToVerifiedDomain: false,
+          disablePhoneOnlySMSNotifications: false,
        },
        slug: "abc",
        parent: null,
+        isOrganization: true,
      };
      const result = getOrgState(true, { ...mockedRegularTeam, ...team });
      expect(result).toEqual({
@@ -293,6 +305,9 @@ describe("Invite Member Utils", () => {
            adminGetsNoSlotsNotification: false,
            isAdminReviewed: false,
            isAdminAPIEnabled: false,
+            allowSEOIndexing: false,
+            orgProfileRedirectsToVerifiedDomain: false,
+            disablePhoneOnlySMSNotifications: false,
          },
        },
      };
@@ -410,6 +425,7 @@ describe("Invite Member Utils", () => {
        ...mockedRegularTeam,
        parentId: null,
        id: inviteeOrgId,
+        isOrganization: true,
      };
      expect(canBeInvited(inviteeWithOrg, organization)).toBe(INVITE_STATUS.USER_ALREADY_INVITED_OR_MEMBER);
    });
@@ -445,12 +461,33 @@ describe("Invite Member Utils", () => {
      const organization = {
        ...mockedRegularTeam,
        id: organizationIdBeingInvitedTo,
+        isOrganization: true,
      };
      expect(canBeInvited(inviteeWithOrg, organization)).toBe(
        INVITE_STATUS.USER_MEMBER_OF_OTHER_ORGANIZATION
      );
    });

+    it("should return CAN_BE_INVITED if the user being invited has a profile with the organization already", () => {
+      const organizationId = 3;
+      const inviteeWithOrg: UserWithMembership = {
+        ...invitee,
+        profiles: [
+          getSampleProfile({
+            organizationId: organizationId,
+          }),
+        ],
+        teams: [],
+      };
+
+      const organization = {
+        ...mockedRegularTeam,
+        id: organizationId,
+        isOrganization: true,
+      };
+      expect(canBeInvited(inviteeWithOrg, organization)).toBe(INVITE_STATUS.CAN_BE_INVITED);
+    });
+
    it("should return USER_MEMBER_OF_OTHER_ORGANIZATION if the invitee is being invited to a sub-team in an organization but he belongs to another organization", () => {
      const inviteeOrganizationId = 2;
      const subTeamOrganizationId = 3;
@@ -160,10 +160,27 @@ export function canBeInvited(invitee: UserWithMembership, team: TeamWithParent)
    return INVITE_STATUS.USER_PENDING_MEMBER_OF_THE_ORG;
  }

+  const hasDifferentOrganizationProfile = invitee.profiles.some((profile) => {
+    const isRegularTeam = !team.isOrganization && !team.parentId;
+    if (isRegularTeam) {
+      // ⚠️ Inviting to a regular team but the user has a profile with some organization
+      return true;
+    }
+
+    const isOrganization = team.isOrganization && !team.parentId;
+    if (isOrganization) {
+      // ⚠️ User has profile with different organization than the organization being invited to
+      return profile.organizationId !== team.id;
+    }
+
+    // ⚠️ User having profile with an organization is invited to join a sub-team that is not part of the organization
+    return profile.organizationId != team.parentId;
+  });
+
  if (
    !ENABLE_PROFILE_SWITCHER &&
-    // Member of an organization is invited to join a team that is not a subteam of the organization
-    invitee.profiles.find((profile) => profile.organizationId != team.parentId)
+    // User having profile with an organization is invited to join a sub-team that is not part of the organization
+    hasDifferentOrganizationProfile
  ) {
    return INVITE_STATUS.USER_MEMBER_OF_OTHER_ORGANIZATION;
  }
@@ -828,7 +845,7 @@ export async function handleExistingUsersInvites({
    if (parentOrganization) {
      const parsedOrg = getParsedTeam(parentOrganization);
      // Create profiles if needed
-      await Promise.all([
+      await Promise.all(
        autoJoinUsers
          .concat(regularUsers)
          .filter((u) => u.needToCreateProfile)
@@ -841,8 +858,8 @@ export async function handleExistingUsersInvites({
              },
              organizationId: parsedOrg.id,
            })
-          ),
-      ]);
+          )
+      );
    }
  } else {
    const organization = team;
@@ -877,6 +894,22 @@ export async function handleExistingUsersInvites({
            role: user.newRole,
          },
        });
+
+        // If auto-accepting into org, also accept any pending sub-team memberships
+        if (shouldAutoAccept) {
+          await prisma.membership.updateMany({
+            where: {
+              userId: user.id,
+              accepted: false,
+              team: {
+                parentId: organization.id,
+              },
+            },
+            data: {
+              accepted: true,
+            },
+          });
+        }
        return {
          ...user,
          profile,