Skip to content

fix: align checkBookingAccessWithPBAC with listing logic for personal event types#28071

Draft
eunjae-lee wants to merge 1 commit intomainfrom
devin/1771514859-fix-booking-access-personal-event-types
Draft

fix: align checkBookingAccessWithPBAC with listing logic for personal event types#28071
eunjae-lee wants to merge 1 commit intomainfrom
devin/1771514859-fix-booking-access-personal-event-types

Conversation

@eunjae-lee
Copy link
Contributor

What does this PR do?

Fixes a policy misalignment between the booking listing endpoint and the booking details access check for personal event types (event types with no teamId).

The listing query (get.handler.ts, query #7) includes bookings created by any member of a team where the current user has booking.read permission — including personal event type bookings. But when clicking to view details, checkBookingAccessWithPBAC early-returned false at line 176-180 because there was no teamId on the event type. Users with proper team/org permissions could see the booking in the list but got a "forbidden" error viewing details.

Changes:

  • Replaced the early return false for !booking.eventType?.teamId with logic that looks up the booking organizer's teams/org and checks booking.read permission — mirroring the listing query
  • Refactored to use this.permissionCheckService (class-level instance) instead of creating a new local instance, consistent with doesUserIdHaveAccessToBooking
  • Added 12 unit tests covering all checkBookingAccessWithPBAC code paths

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  1. Have an org/team admin user with booking.read permission
  2. Have a team member create a booking using a personal event type (no teamId)
  3. As the admin, navigate to the bookings list — the personal booking should appear
  4. Click the booking to view details — before this fix it returned "forbidden"; after it should show details
  5. Verify a non-admin user still cannot view another user's personal event type booking details
  6. Verify existing cases (owner, host, team event type bookings) still work correctly

Important for reviewers

  • checkBookingAccessWithPBAC now uses "booking.read" permission for the new personal-event-type checks (matching the listing in get.handler.ts). Note that doesUserIdHaveAccessToBooking in the same class uses "booking.readOrgBookings" / "booking.readTeamBookings" for its equivalent Cases 4 & 5. This difference is intentional per the spec to match the listing behavior, but worth confirming this is the desired alignment.
  • No schema/select changes needed — bookingAuthorizationCheckSelect already includes userId.

Link to Devin run: https://app.devin.ai/sessions/093e52a39f484789887ff4d70f1c0229
Requested by: @eunjae-lee

… event types

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/features/bookings/services/BookingAccessService.ts">

<violation number="1" location="packages/features/bookings/services/BookingAccessService.ts:180">
P1: Permission inconsistency: this method uses `"booking.read"` for the personal-event-type org/team checks, but the sibling method `doesUserIdHaveAccessToBooking` in the same class uses the more specific `"booking.readOrgBookings"` and `"booking.readTeamBookings"` for the identical logical scenario (Cases 4 & 5). Similarly, `get-booking.ts` uses `"booking.readOrgBookings"`. This divergence means the two code paths may grant/deny access differently for the same user and booking. Please verify this is the intended permission and consider aligning with the granular permissions used elsewhere, or document why the generic permission is correct here.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

const hasPermission = await this.permissionCheckService.checkPermission({
userId,
teamId: booking.eventType.teamId,
permission: "booking.read",
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Permission inconsistency: this method uses "booking.read" for the personal-event-type org/team checks, but the sibling method doesUserIdHaveAccessToBooking in the same class uses the more specific "booking.readOrgBookings" and "booking.readTeamBookings" for the identical logical scenario (Cases 4 & 5). Similarly, get-booking.ts uses "booking.readOrgBookings". This divergence means the two code paths may grant/deny access differently for the same user and booking. Please verify this is the intended permission and consider aligning with the granular permissions used elsewhere, or document why the generic permission is correct here.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/bookings/services/BookingAccessService.ts, line 180:

<comment>Permission inconsistency: this method uses `"booking.read"` for the personal-event-type org/team checks, but the sibling method `doesUserIdHaveAccessToBooking` in the same class uses the more specific `"booking.readOrgBookings"` and `"booking.readTeamBookings"` for the identical logical scenario (Cases 4 & 5). Similarly, `get-booking.ts` uses `"booking.readOrgBookings"`. This divergence means the two code paths may grant/deny access differently for the same user and booking. Please verify this is the intended permission and consider aligning with the granular permissions used elsewhere, or document why the generic permission is correct here.</comment>

<file context>
@@ -174,19 +173,44 @@ export class BookingAccessService {
+      const hasPermission = await this.permissionCheckService.checkPermission({
+        userId,
+        teamId: booking.eventType.teamId,
+        permission: "booking.read",
+        fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
+      });
</file context>
Fix with Cubic

@github-actions
Copy link
Contributor

github-actions bot commented Feb 19, 2026

Devin AI is addressing Cubic AI's review feedback

New feedback has been sent to the existing Devin session.

View Devin Session


✅ No changes pushed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments