Files
calendar/packages/features/pbac/PBAC_REFACTORING_GUIDE.md
T
Eunjae LeeGitHubDevin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>sean-brydon
a4dbb1a90b refactor: replace isTeamAdminOrOwner with PBAC team.listMembers permission (#24006)
* refactor: replace isTeamAdminOrOwner with PBAC team.listMembers permission

- Add canListMembers prop to BookingsProps interface
- Implement server-side permission check using PermissionCheckService
- Handle organization vs team context as specified
- Use ADMIN/OWNER fallback roles for backward compatibility
- Replace user?.isTeamAdminOrOwner check in bookings column filter
- Fix React Hook dependency arrays for ESLint compliance

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* refactor: wrap canListMembers in permissions object and simplify server logic

- Wrap canListMembers in permissions object for future extensibility
- Simplify server-side logic to only use getTeamIdsWithPermission
- Remove unused imports (prisma, MembershipRole)
- Address user feedback on PR #24006

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* docs: add comment explaining canListMembers UI logic

- Clarify that teamIdsWithPermission.length > 0 check is for UI purposes
- Actual accurate filtering happens server-side for filter values

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* feat: add e2e test for member filter visibility

- Verify that users with the MEMBER role cannot see the member filter
- Test creates team with ADMIN and MEMBER users
- Confirms UI correctly reflects PBAC permissions
- Address user feedback on PR #24006

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* fix: correct e2e test team member creation pattern

- Replace invalid teamId property with hasTeam and teammates pattern
- Fix TypeScript error in booking-filters.e2e.ts
- Resolve CI type check failure

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* fix e2e test

* remove booking.read permission from member

* add guide

* update

* resource scope

* fix markdown

* update usage

* update guide

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: sean-brydon <55134778+sean-brydon@users.noreply.github.com>
2025-09-26 06:26:20 -03:00

8.6 KiB

PBAC Refactoring Guide

Quick guide for refactoring Cal.com to use Permission-Based Access Control (PBAC) instead of role-based checks and membership queries.

Core Patterns

1. Team Filtering with PBAC (API/tRPC handlers)

Before (Membership-based):

const teamsToQuery = (
  await prisma.membership.findMany({
    where: {
      userId: ctx.user.id,
      accepted: true,
      NOT: [
        {
          role: MembershipRole.MEMBER,
          team: { isPrivate: true },
        },
      ],
    },
    select: { teamId: true },
  })
).map((membership) => membership.teamId);

After (PBAC-based):

import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
import { MembershipRole } from "@calcom/prisma/enums";

const permissionCheckService = new PermissionCheckService();
const teamsToQuery = await permissionCheckService.getTeamIdsWithPermission({
  userId: ctx.user.id,
  permission: "team.listMembers",
  fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
});

2. UI Permission Checks (React components)

Before (Role-based):

// Client-side role check with role-based variable name
const isTeamAdminOrOwner = user?.isTeamAdminOrOwner ?? false;
const canSeeMembers = isTeamAdminOrOwner;

After (PBAC-based):

// Server-side permission check in page/layout
import { MembershipRole } from "@calcom/prisma/enums";

const permissionCheckService = new PermissionCheckService();
const teamIdsWithPermission = await permissionCheckService.getTeamIdsWithPermission({
  userId: session.user.id,
  permission: "team.listMembers",
  fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
});

const permissions = {
  canListMembers: teamIdsWithPermission.length > 0,
};

// Pass to component
<MyComponent permissions={permissions} />;

Key Points

1. Use Direct Permission Strings

  • Use "team.listMembers" directly instead of PermissionMapper.toPermissionString()
  • Simpler and more readable

2. No Fallback Logic Needed

  • getTeamIdsWithPermission() handles errors internally
  • Returns empty array [] when user has no permissions (this is legitimate)
  • Don't assume empty array means PBAC failure

3. Server-Side Permission Checking

  • Move permission checks from client to server - crucial for security
  • Check permissions in page/layout components, not in UI components
  • Pass permission results as props to maintain separation of concerns

4. Permissions Object Pattern

  • Wrap permission booleans in a permissions object for extensibility
  • Use descriptive names like canListMembers instead of raw permission strings
  • Makes it easy to add more permissions later

5. Permission-Based Variable Naming

  • Use permission-specific names like canListMembers instead of role-based names like isTeamAdminOrOwner
  • Follow the pattern: can[Action][Resource] (e.g., canListMembers, canUpdateTeam, canInviteUsers)
  • Makes code more readable and aligns with the actual permission being checked
  • Easier to understand what specific action is being permitted

6. Team vs Personal Resources

Rule: Use teamId to determine if permission checks are needed:

  • teamId present → Team resource → Check permissions
  • teamId null/undefined → Personal resource → Check ownership only

Example: booking.read permission

  • Has permission: Can read all team members' bookings
  • No permission: Can still read own bookings, just not others'
if (resource.teamId) {
  // Team resource - check permissions
  const hasPermission = await permissionService.hasPermission(userId, "resource.read", resource.teamId);
  if (!hasPermission) throw new ForbiddenError();
} else {
  // Personal resource - check ownership only
  if (resource.userId !== currentUserId) throw new ForbiddenError();
}

7. Common Team Permissions

  • "booking.read" - Read all team members' bookings (not just own)
  • "team.listMembers" - List team members
  • "team.read" - View team details
  • "team.update" - Edit team settings
  • "team.invite" - Invite members
  • "team.remove" - Remove members

Step-by-Step Refactoring

For API/tRPC Handlers

  1. Add imports:
import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
import { MembershipRole } from "@calcom/prisma/enums";
  1. Replace membership query:
const permissionCheckService = new PermissionCheckService();
const teamsToQuery = await permissionCheckService.getTeamIdsWithPermission({
  userId: ctx.user.id,
  permission: "team.listMembers",
  fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
});
  1. Keep existing privacy checks - PBAC doesn't replace organization-level privacy logic

For UI Components

  1. Move permission check to server (page/layout component):
import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
import { MembershipRole } from "@calcom/prisma/enums";

// In your page component
const permissionCheckService = new PermissionCheckService();
const teamIdsWithPermission = await permissionCheckService.getTeamIdsWithPermission({
  userId: session.user.id,
  permission: "team.listMembers",
  fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
});

const permissions = {
  canListMembers: teamIdsWithPermission.length > 0, // Permission-specific name
};
  1. Pass permissions as props:
<BookingsContent permissions={permissions} />
  1. Update component interface:
interface BookingsProps {
  permissions: {
    canListMembers: boolean;
  };
}
  1. Replace role checks in component:
// Replace: const isTeamAdminOrOwner = user?.isTeamAdminOrOwner ?? false
// With: const canListMembers = permissions.canListMembers

// Use permission-specific variable names throughout your component

Examples

API Handler: listSimpleMembers.handler.ts

What changed:

  • Replaced 17 lines of membership query with 2 lines of PBAC
  • Removed unnecessary imports and fallback logic
  • Used direct permission string "team.listMembers"

Result: Cleaner, more maintainable code that respects fine-grained permissions.

UI Component: Bookings Page

What changed (from PR #24006):

  • Moved permission check from client (user?.isTeamAdminOrOwner) to server
  • Added permissions prop with canListMembers boolean
  • Used teamIdsWithPermission.length > 0 pattern for UI control
  • Added E2E test to verify member filter visibility

Before:

// In component - role-based variable name
const isTeamAdminOrOwner = user?.isTeamAdminOrOwner ?? false;
const canSeeMembers = isTeamAdminOrOwner;

After:

// In page component (server-side)
const permissions = {
  canListMembers: teamIdsWithPermission.length > 0,
};

// In UI component - permission-specific variable name
const canListMembers = permissions.canListMembers;
const canSeeMembers = canListMembers; // Use permission-specific name

Result: Secure server-side permission checking with clean UI separation.

Verification

For All Refactoring

  • Run yarn type-check:ci --force
  • Ensure existing privacy checks remain intact
  • Test with users who have different permission levels

For UI Refactoring

  • Add E2E tests to verify UI element visibility based on permissions
  • Test with different user roles (ADMIN, OWNER, MEMBER)
  • Verify that permission checks happen server-side, not client-side
  • Confirm that UI elements are properly hidden/shown based on permissions props

Common Pitfalls

  • Don't check permissions client-side - always do it server-side for security
  • Don't assume empty array means failure - it's valid when user has no permissions
  • Remember to update TypeScript interfaces when adding new permission props
  • Don't use role-based variable names - use permission-specific names instead

Variable Naming Best Practices

Good: Permission-Specific Names

const canListMembers = permissions.canListMembers;
const canUpdateTeam = permissions.canUpdateTeam;
const canInviteUsers = permissions.canInviteUsers;
const canDeleteBookings = permissions.canDeleteBookings;

Bad: Role-Based Names

const isTeamAdminOrOwner = user?.isTeamAdminOrOwner;
const isAdmin = user?.role === "ADMIN";
const hasPermission = user?.isTeamAdminOrOwner;

Naming Convention

  • Use the pattern: can[Action][Resource]
  • Examples: canListMembers, canUpdateTeam, canInviteUsers
  • Makes code self-documenting and easier to understand
  • Aligns variable names with the actual permissions being checked