fix: get bookings handler for pbac and fallback roles

[tomerqodo]

Benchmark PR from agentic-review-benchmarks#1

Summary by CodeRabbit

• New Features

  • Enhanced permission system to support organization-scoped filtering when accessing teams and viewing bookings.

• Tests

  • Added integration and unit tests for organization-scoped team filtering and permission-based booking retrieval.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR extends permission-based access control (PBAC) repository methods and their consumers with optional organization ID filtering. An orgId?: number parameter is propagated from the interface definition through the service layer to the handler, enabling permission queries scoped to specific organizations while maintaining backward compatibility.

Changes

Cohort / File(s)	Summary
Permission Domain Interface packages/features/pbac/domain/repositories/IPermissionRepository.ts	Added optional orgId?: number parameter to getTeamIdsWithPermission and getTeamIdsWithPermissions method signatures with corresponding documentation.
Permission Repository Implementation packages/features/pbac/infrastructure/repositories/PermissionRepository.ts	Extended public method signatures to accept orgId?: number. Introduced two private helper methods (getTeamsWithPBACPermissions and getTeamsWithFallbackRoles) supporting org scoping. Refactored permission resolution to parallelize PBAC and fallback-role queries, applying organization filtering to both direct membership and hierarchical (child-org/child-team) database paths.
Permission Service Layer packages/features/pbac/services/permission-check.service.ts	Updated getTeamIdsWithPermission and getTeamIdsWithPermissions to accept and propagate optional orgId parameter to repository calls with documentation.
Booking Handler Implementation packages/trpc/server/routers/viewer/bookings/get.handler.ts	Replaced admin/owner membership-based access control with permission-service-driven logic. Refactored to derive accessible event types and user IDs from team-based permission results, removing prior ad-hoc admin/owner checks and consolidating filtering logic.
Integration & Unit Tests packages/features/pbac/infrastructure/repositories/__tests__/PermissionRepository.integration-test.ts, packages/trpc/server/routers/viewer/bookings/get.handler.test.ts	Added integration tests covering org-scoped team filtering and parent-child team hierarchies. Added comprehensive unit test suite for booking handler covering PBAC permission checks, fallback roles, team-based event filtering, and error paths.

Sequence Diagram(s)

sequenceDiagram
    participant Handler as Booking Handler
    participant Service as PermissionCheckService
    participant Repo as PermissionRepository
    participant DB as Database

    Handler->>Service: getTeamIdsWithPermission({userId, permission, fallbackRoles, orgId?})
    Service->>Repo: getTeamIdsWithPermission({userId, permission, fallbackRoles, orgId?})
    
    rect rgba(100, 150, 200, 0.5)
    Note over Repo: Parallel Execution
    par PBAC Path
        Repo->>DB: Query teams by direct PBAC permission<br/>(filtered by orgId if provided)
    and Fallback Roles Path
        Repo->>DB: Query teams by fallback roles<br/>(filtered by orgId if provided)
    end
    end
    
    DB-->>Repo: PBAC teams + Fallback teams
    Repo->>Repo: Deduplicate & merge team IDs
    Repo-->>Service: Promise<number[]>
    Service-->>Handler: Team IDs with permission
    
    Handler->>Handler: Filter event types & users by team permissions
    Handler-->>Client: Accessible bookings

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A hop through the orgs, so fine,
With teamIds all aligned,
Permission flows with purpose true,
Scoped just right—now orgId too!
The rabbits rejoice, permissions take flight! 🚀

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: get bookings handler for pbac and fallback roles' accurately describes the main change in this PR, which refactors the bookings handler to use permission-based access control (PBAC) and fallback roles instead of admin/owner-based logic.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch coderabbit_full_head_fix_get_bookings_handler_for_pbac_and_fallback_roles_pr1

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[tomerqodo]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[tomerqodo]

@coderabbitai review

[tomerqodo]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[hussam789]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

packages/features/pbac/infrastructure/repositories/PermissionRepository.ts (1)

211-223: ⚠️ Potential issue | 🔴 Critical

Missing orgId propagation drops org scoping.

getTeamIdsWithPermission ignores orgId when delegating, so org-scoped calls can return teams outside the intended organization.

🛠️ Proposed fix

-    return this.getTeamIdsWithPermissions({ userId, permissions: [permission], fallbackRoles });
+    return this.getTeamIdsWithPermissions({ userId, permissions: [permission], fallbackRoles, orgId });

packages/trpc/server/routers/viewer/bookings/get.handler.ts (1)

144-155: ⚠️ Potential issue | 🔴 Critical

Authorization bypass when userIds include self + unauthorized users.

If filters.userIds contains the current user plus any unauthorized IDs, the current check allows the whole set. That enables access beyond permitted scope.

🛠️ Proposed fix

-    const areUserIdsWithinUserOrgOrTeam = filters.userIds.every((userId) =>
-      allAccessibleUserIds.includes(userId)
-    );
-
-    const isCurrentUser = filters.userIds.includes(user.id);
+    const unauthorizedUserIds = filters.userIds.filter(
+      (userId) => userId !== user.id && !allAccessibleUserIds.includes(userId)
+    );

-    if (!areUserIdsWithinUserOrgOrTeam && !isCurrentUser) {
+    if (unauthorizedUserIds.length > 0) {
       throw new TRPCError({
         code: "FORBIDDEN",
         message: "You do not have permissions to fetch bookings for specified userIds",
       });
     }