fix: consolidate booking access checks into doesUserIdHaveAccessToBooking (#28071)

* fix: align checkBookingAccessWithPBAC with listing logic for personal event types Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * fix: align permission strings with doesUserIdHaveAccessToBooking granular permissions Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * change implementation * remove unused code * test: add getBookingDetails tests for personal event type booking access Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * fix: remove unused MembershipRole import Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * test: convert BookingDetailsService tests to integration tests Replace unit tests with mocks by integration tests using real database. Tests verify org/team admin access to personal event type bookings. - Org admin viewing team member's personal event booking (fails on main, passes on PR) - Team admin viewing team member's personal event booking (fails on main, passes on PR) - Non-admin denied access (passes on both) - Owner viewing own booking (passes on both) - Non-existent booking returns Forbidden (passes on both) Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * test: refactor integration tests to use repositories instead of direct prisma calls Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * test: replace direct prisma usage with TestXXXRepository pattern in integration tests Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * test: use production repositories (OrganizationRepository, BookingRepository, UserRepository) instead of test repos for setup Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * test: inline test repository logic into integration test file The TestXXXRepository classes were thin wrappers around single prisma calls with barely any logic, so the indirection wasn't justified. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-11 14:46:57 -03:00
parent d3dad196d6
commit 7ff2eaf293
6 changed files with 284 additions and 108 deletions
@@ -2,11 +2,7 @@ import { withReporting } from "@calcom/lib/sentryWrapper";
 import type { PrismaClient } from "@calcom/prisma";
 import type { Booking, Prisma } from "@calcom/prisma/client";
 import { BookingStatus, RRTimestampBasis } from "@calcom/prisma/enums";
-import {
-  bookingAuthorizationCheckSelect,
-  bookingDetailsSelect,
-  bookingMinimalSelect,
-} from "@calcom/prisma/selects/booking";
+import { bookingDetailsSelect, bookingMinimalSelect } from "@calcom/prisma/selects/booking";
 import { credentialForCalendarServiceSelect } from "@calcom/prisma/selects/credential";
 import { workflowSelect } from "../../ee/workflows/lib/getAllWorkflows";
 import type {
@@ -674,15 +670,6 @@ export class BookingRepository implements IBookingRepository {
    });
  }

-  async findByUidForAuthorizationCheck({ bookingUid }: { bookingUid: string }) {
-    return await this.prismaClient.booking.findUnique({
-      where: {
-        uid: bookingUid,
-      },
-      select: bookingAuthorizationCheckSelect,
-    });
-  }
-
  async findByUidForDetails({ bookingUid }: { bookingUid: string }) {
    return await this.prismaClient.booking.findUnique({
      where: {
@@ -1,9 +1,7 @@
-import { describe, it, expect, beforeEach, vi } from "vitest";
-
 import { UserRepository } from "@calcom/features/users/repositories/UserRepository";
 import type { PrismaClient } from "@calcom/prisma";
 import { MembershipRole } from "@calcom/prisma/enums";
-
+import { beforeEach, describe, expect, it, vi } from "vitest";
 import { BookingRepository } from "../repositories/BookingRepository";
 import { BookingAccessService } from "./BookingAccessService";

@@ -341,4 +339,5 @@ describe("BookingAccessService", () => {
      });
    });
  });
+
 });
@@ -2,7 +2,6 @@ import { PermissionCheckService } from "@calcom/features/pbac/services/permissio
 import { UserRepository } from "@calcom/features/users/repositories/UserRepository";
 import type { PrismaClient } from "@calcom/prisma";
 import { MembershipRole } from "@calcom/prisma/enums";
-
 import { BookingRepository } from "../repositories/BookingRepository";

 type BookingForAccessCheck = NonNullable<Awaited<ReturnType<BookingRepository["findByUidIncludeEventType"]>>>;
@@ -131,62 +130,4 @@ export class BookingAccessService {

    return false;
  }
-
-  /**
-   * Checks if a user has access to view booking details using:
-   * 1. Owner/host check
-   * 2. PBAC permission check for team bookings
-   */
-  async checkBookingAccessWithPBAC({
-    userId,
-    bookingUid,
-  }: {
-    userId: number;
-    bookingUid: string;
-  }): Promise<boolean> {
-    const bookingRepo = new BookingRepository(this.prismaClient);
-
-    const booking = await bookingRepo.findByUidForAuthorizationCheck({ bookingUid });
-
-    if (!booking) {
-      return false;
-    }
-
-    // Check 1: User is the owner of the booking
-    const isOwner = booking.userId === userId;
-    if (isOwner) {
-      return true;
-    }
-
-    // Check 2: User is a host (checking eventType.users and eventType.hosts)
-    const attendeeEmails = new Set(booking.attendees?.map((attendee) => attendee.email) || []);
-
-    const isHostViaEventTypeUsers = booking.eventType?.users?.some(
-      (user) => user.id === userId && attendeeEmails.has(user.email)
-    );
-
-    const isHostViaEventTypeHosts = booking.eventType?.hosts?.some(
-      (host) => host.user?.id === userId && attendeeEmails.has(host.user.email)
-    );
-
-    if (isHostViaEventTypeUsers || isHostViaEventTypeHosts) {
-      return true;
-    }
-
-    // Check 3: PBAC permission check for team bookings
-    if (!booking.eventType?.teamId) {
-      // No team associated with booking and user is not owner/host
-      return false;
-    }
-
-    const permissionCheckService = new PermissionCheckService();
-    const hasPermission = await permissionCheckService.checkPermission({
-      userId,
-      teamId: booking.eventType.teamId,
-      permission: "booking.read",
-      fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
-    });
-
-    return hasPermission;
-  }
 }
@@ -0,0 +1,280 @@
+import { describe, it, expect, beforeAll, afterAll } from "vitest";
+
+import { BookingRepository } from "@calcom/features/bookings/repositories/BookingRepository";
+import { OrganizationRepository } from "@calcom/features/ee/organizations/repositories/OrganizationRepository";
+import { TeamRepository } from "@calcom/features/ee/teams/repositories/TeamRepository";
+import { EventTypeRepository } from "@calcom/features/eventtypes/repositories/eventTypeRepository";
+import { MembershipRepository } from "@calcom/features/membership/repositories/MembershipRepository";
+import { UserRepository } from "@calcom/features/users/repositories/UserRepository";
+import { prisma } from "@calcom/prisma";
+import { BookingStatus, CreationSource, MembershipRole } from "@calcom/prisma/enums";
+
+import { BookingDetailsService } from "./BookingDetailsService";
+
+/**
+ * Integration tests for BookingDetailsService.getBookingDetails
+ *
+ * These tests verify that org/team admins can view booking details
+ * for bookings made by team members using personal event types (no teamId).
+ *
+ * On main (before the fix), the access check (checkBookingAccessWithPBAC) returns false
+ * for personal event type bookings when the viewer is not the owner/host — even if
+ * they are an org/team admin. These tests fail on main, proving the bug.
+ *
+ * On the PR branch, getBookingDetails uses doesUserIdHaveAccessToBooking which
+ * correctly checks org/team admin access for personal event type bookings.
+ */
+
+// Track resource IDs for cleanup
+const createdBookingIds: number[] = [];
+const createdTeamIds: number[] = [];
+const createdUserIds: number[] = [];
+
+describe("BookingDetailsService (Integration Tests)", () => {
+  const timestamp = Date.now();
+
+  const orgRepo = new OrganizationRepository({ prismaClient: prisma });
+  const bookingRepo = new BookingRepository(prisma);
+  const userRepo = new UserRepository(prisma);
+  const eventTypeRepo = new EventTypeRepository(prisma);
+  const teamRepo = new TeamRepository(prisma);
+
+  // Users
+  let bookingOwnerId: number;
+  let orgAdminId: number;
+  let teamAdminId: number;
+  let regularUserId: number;
+
+  // Org and team
+  let orgId: number;
+  let teamId: number;
+
+  // Booking
+  let personalBookingUid: string;
+
+  beforeAll(async () => {
+    // 1. Create an organization via OrganizationRepository
+    const org = await orgRepo.create({
+      name: `Test Org ${timestamp}`,
+      slug: `test-org-${timestamp}`,
+      isOrganizationConfigured: true,
+      isOrganizationAdminReviewed: true,
+      autoAcceptEmail: `test-${timestamp}.com`,
+      seats: null,
+      pricePerSeat: null,
+      isPlatform: false,
+      logoUrl: null,
+      bio: null,
+      brandColor: null,
+      bannerUrl: null,
+    });
+    orgId = org.id;
+    createdTeamIds.push(org.id);
+
+    // 2. Create a team within the org (no production TeamRepository.create() exists)
+    const team = await prisma.team.create({
+      data: {
+        name: `Test Team ${timestamp}`,
+        slug: `test-team-${timestamp}`,
+        parentId: org.id,
+      },
+      select: { id: true },
+    });
+    teamId = team.id;
+    createdTeamIds.push(team.id);
+
+    // 3. Create booking owner WITH organizationId via UserRepository
+    //    (eliminates need for separate updateOrganizationId call)
+    const bookingOwner = await userRepo.create({
+      email: `booking-owner-${timestamp}@test.com`,
+      username: `booking-owner-${timestamp}`,
+      organizationId: orgId,
+      creationSource: CreationSource.WEBAPP,
+      locked: false,
+    });
+    bookingOwnerId = bookingOwner.id;
+    createdUserIds.push(bookingOwner.id);
+
+    // 4. Create other users via UserRepository
+    const orgAdmin = await userRepo.create({
+      email: `org-admin-${timestamp}@test.com`,
+      username: `org-admin-${timestamp}`,
+      organizationId: null,
+      creationSource: CreationSource.WEBAPP,
+      locked: false,
+    });
+    orgAdminId = orgAdmin.id;
+    createdUserIds.push(orgAdmin.id);
+
+    const teamAdmin = await userRepo.create({
+      email: `team-admin-${timestamp}@test.com`,
+      username: `team-admin-${timestamp}`,
+      organizationId: null,
+      creationSource: CreationSource.WEBAPP,
+      locked: false,
+    });
+    teamAdminId = teamAdmin.id;
+    createdUserIds.push(teamAdmin.id);
+
+    const regularUser = await userRepo.create({
+      email: `regular-user-${timestamp}@test.com`,
+      username: `regular-user-${timestamp}`,
+      organizationId: null,
+      creationSource: CreationSource.WEBAPP,
+      locked: false,
+    });
+    regularUserId = regularUser.id;
+    createdUserIds.push(regularUser.id);
+
+    // 5. Create memberships via MembershipRepository (static method)
+    await MembershipRepository.create({
+      userId: bookingOwnerId,
+      teamId: orgId,
+      role: MembershipRole.MEMBER,
+      accepted: true,
+    });
+
+    await MembershipRepository.create({
+      userId: bookingOwnerId,
+      teamId: teamId,
+      role: MembershipRole.MEMBER,
+      accepted: true,
+    });
+
+    await MembershipRepository.create({
+      userId: orgAdminId,
+      teamId: orgId,
+      role: MembershipRole.ADMIN,
+      accepted: true,
+    });
+
+    await MembershipRepository.create({
+      userId: teamAdminId,
+      teamId: teamId,
+      role: MembershipRole.ADMIN,
+      accepted: true,
+    });
+
+    // 6. Create a personal event type (no teamId) via EventTypeRepository
+    const personalEventType = await eventTypeRepo.create({
+      title: `Personal Event ${timestamp}`,
+      slug: `personal-event-${timestamp}`,
+      length: 30,
+      userId: bookingOwnerId,
+    });
+
+    // 7. Create a booking on the personal event type via BookingRepository
+    personalBookingUid = `personal-booking-${timestamp}`;
+    const booking = await bookingRepo.createBookingForManagedEventReassignment({
+      uid: personalBookingUid,
+      userId: bookingOwnerId,
+      userPrimaryEmail: `booking-owner-${timestamp}@test.com`,
+      eventTypeId: personalEventType.id,
+      title: `Personal Booking ${timestamp}`,
+      description: null,
+      startTime: new Date("2026-03-01T10:00:00.000Z"),
+      endTime: new Date("2026-03-01T10:30:00.000Z"),
+      status: BookingStatus.ACCEPTED,
+      location: null,
+      smsReminderNumber: null,
+      idempotencyKey: `idempotency-${timestamp}`,
+      iCalUID: `ical-${timestamp}@test.com`,
+      iCalSequence: 0,
+      attendees: [{ email: "attendee@test.com", name: "Test Attendee", timeZone: "UTC", locale: "en" }],
+    });
+    createdBookingIds.push(booking.id);
+  });
+
+  afterAll(async () => {
+    // Clean up bookings and attendees
+    if (createdBookingIds.length > 0) {
+      await prisma.attendee.deleteMany({ where: { bookingId: { in: createdBookingIds } } });
+      await prisma.booking.deleteMany({ where: { id: { in: createdBookingIds } } });
+    }
+
+    // Delete personal event types not associated with a team
+    if (createdUserIds.length > 0) {
+      await prisma.eventType.deleteMany({ where: { userId: { in: createdUserIds } } });
+    }
+
+    // Delete users (memberships, profiles, schedules, availability)
+    // Must happen before team deletion so user.organizationId FK is removed
+    if (createdUserIds.length > 0) {
+      await prisma.membership.deleteMany({ where: { userId: { in: createdUserIds } } });
+      await prisma.profile.deleteMany({ where: { userId: { in: createdUserIds } } });
+      for (const userId of createdUserIds) {
+        await prisma.availability.deleteMany({ where: { Schedule: { userId } } });
+        await prisma.schedule.deleteMany({ where: { userId } });
+        await prisma.user.delete({ where: { id: userId } });
+      }
+    }
+
+    // TeamRepository.deleteById handles remaining memberships + managed event types
+    for (const id of [...createdTeamIds].reverse()) {
+      await teamRepo.deleteById({ id });
+    }
+  });
+
+  describe("getBookingDetails - personal event type bookings", () => {
+    it("should allow the booking owner to view their own booking details", async () => {
+      const service = new BookingDetailsService(prisma);
+
+      const result = await service.getBookingDetails({
+        userId: bookingOwnerId,
+        bookingUid: personalBookingUid,
+      });
+
+      expect(result).toBeDefined();
+      expect(result.rescheduledToBooking).toBeNull();
+      expect(result.previousBooking).toBeNull();
+    });
+
+    it("should allow an org admin to view booking details for a team member's personal event type booking", async () => {
+      const service = new BookingDetailsService(prisma);
+
+      const result = await service.getBookingDetails({
+        userId: orgAdminId,
+        bookingUid: personalBookingUid,
+      });
+
+      expect(result).toBeDefined();
+      expect(result.rescheduledToBooking).toBeNull();
+      expect(result.previousBooking).toBeNull();
+    });
+
+    it("should allow a team admin to view booking details for a team member's personal event type booking", async () => {
+      const service = new BookingDetailsService(prisma);
+
+      const result = await service.getBookingDetails({
+        userId: teamAdminId,
+        bookingUid: personalBookingUid,
+      });
+
+      expect(result).toBeDefined();
+      expect(result.rescheduledToBooking).toBeNull();
+      expect(result.previousBooking).toBeNull();
+    });
+
+    it("should deny access to a non-admin user viewing another user's personal event type booking", async () => {
+      const service = new BookingDetailsService(prisma);
+
+      await expect(
+        service.getBookingDetails({
+          userId: regularUserId,
+          bookingUid: personalBookingUid,
+        })
+      ).rejects.toThrow("You do not have permission to view this booking");
+    });
+
+    it("should throw when booking does not exist", async () => {
+      const service = new BookingDetailsService(prisma);
+
+      await expect(
+        service.getBookingDetails({
+          userId: bookingOwnerId,
+          bookingUid: "non-existent-booking-uid",
+        })
+      ).rejects.toThrow("You do not have permission to view this booking");
+    });
+  });
+});
@@ -1,6 +1,5 @@
 import { ErrorWithCode } from "@calcom/lib/errors";
 import type { PrismaClient } from "@calcom/prisma";
-
 import { BookingRepository } from "../repositories/BookingRepository";
 import { BookingAccessService } from "./BookingAccessService";

@@ -14,7 +13,7 @@ export class BookingDetailsService {
  }

  async getBookingDetails({ userId, bookingUid }: { userId: number; bookingUid: string }) {
-    const hasAccess = await this.bookingAccessService.checkBookingAccessWithPBAC({
+    const hasAccess = await this.bookingAccessService.doesUserIdHaveAccessToBooking({
      userId,
      bookingUid,
    });
@@ -13,36 +13,6 @@ export const bookingMinimalSelect = {
  createdAt: true,
 } satisfies Prisma.BookingSelect;

-export const bookingAuthorizationCheckSelect = {
-  userId: true,
-  eventType: {
-    select: {
-      teamId: true,
-      users: {
-        select: {
-          id: true,
-          email: true,
-        },
-      },
-      hosts: {
-        select: {
-          user: {
-            select: {
-              id: true,
-              email: true,
-            },
-          },
-        },
-      },
-    },
-  },
-  attendees: {
-    select: {
-      email: true,
-    },
-  },
-} satisfies Prisma.BookingSelect;
-
 export const bookingDetailsSelect = {
  uid: true,
  rescheduled: true,