feat: event type pbac by sean-brydon · Pull Request #22618 · calcom/cal.com

sean-brydon · 2025-07-18T08:38:48Z

Seed your database by "yarn seed-pbac"

Look at the roles created in orgs/teams roles and permissions in settings.

Create new roles/edit old ones to remove all permission from event type - assign it to a user.
Impersonate the user and try to CRUD an event type.
Tick on permissions in the role and keep testing the event type.

Impersonate a team without pbac enabled and ensure everything works as expected

coderabbitai · 2025-07-18T08:38:54Z

Walkthrough

This PR integrates PBAC across event types and workflows. It introduces server-side permission checks via PermissionCheckService/getResourcePermissions, a createEventPbacProcedure wrapper for TRPC mutations, and new use cases/utilities (TeamAccessUseCase, EventGroupBuilder, ProfilePermissionProcessor, filter/permission/transform utils). Event type routers (delete/update/duplicate) now use PBAC, and handlers for event types/workflows enforce read/create/update permissions with role fallbacks. Frontend adds an EventPermissionProvider, permission hooks/guards, and gates UI (workflows tab, actions) by permissions. Settings permission UI switches to scope-aware registries. Tests added for new utilities and handlers. No public API shape changes except minor prop additions/defaults and a context type inclusion.

Possibly related PRs

revert: "fix: remove fallback permssions on organization Id. (#22769)" #23013: Modifies getResourcePermissions/permission-check behavior that this PR relies on for PBAC authorization across event types/workflows.
feat: dogfood pbac internally on IS_CALCOM instances #23537: Changes PermissionCheckService fallback logic, directly impacting the PBAC checks introduced here.
feat: add Workflow resource to PBAC system with permission enforcement #22845: Adds PBAC-based workflow permission checks and UI gating, overlapping with this PR’s workflow permission enforcement and guards.

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "feat: event type pbac" concisely captures the primary change — adding permission-based access control for event types. The provided diffs and PR objectives show wide-ranging PBAC work across server, client, tests, and migrations, so the title is accurate and focused. It is clear enough for teammates scanning the commit/PR history.
Description Check	✅ Passed	The PR description contains step-by-step test instructions (seed DB, modify roles, impersonate users/teams) that are directly related to the event-type PBAC changes in the diff. Although it emphasizes manual testing rather than a high-level summary of code changes, it remains on-topic and therefore satisfies this lenient description check. It provides useful QA steps for reviewers.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4f120f6 and a310077.

📒 Files selected for processing (1)

apps/web/modules/event-types/views/event-types-listing-view.tsx (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

apps/web/modules/event-types/views/event-types-listing-view.tsx

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat/event-type-pbac

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

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

github-actions · 2025-07-18T08:38:58Z

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "[DO NOT MERGE] feat: event type pbac". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

delve-auditor · 2025-07-18T08:41:34Z

✅ No security or compliance issues detected. Reviewed everything up to 1bfed57.

Security Overview

🔎 Scanned files: 29 changed file(s)

Detected Code Changes

Change Type	Relevant files
Enhancement	► attributes-list-view.tsx Add permission checks for organization attributes ► resource-permissions.ts Implement PBAC resource permissions logic ► organizations/update.handler.ts Update organization permissions handling
Configuration changes	► permission-registry.ts Add attribute permissions configuration ► common.json Add new permission translations
Refactor	► page.tsx files Update organization settings pages with permission checks ► teamAccessUseCase.ts Extract team access logic

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

* feat: add PBAC permission checks to insights - Add layout-level protection for insights pages with session.user.org.id check - Add API-level protection for insights tRPC endpoints using insightsPbacProcedure - Enhance routing form insights with PBAC team filtering - Use insights.read permission with OWNER/ADMIN fallback roles - Replace all userBelongsToTeamProcedure with insightsPbacProcedure in insights router - Fix lint issues: remove unused variables and functions, fix non-null assertion Implements Permission-Based Access Control (PBAC) for insights functionality following the same patterns as event type PBAC implementation from PR #22618. Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * fix: clean up unused imports in routing-forms - Remove unused isFormCreateEditAllowed import - Remove unused TRPCError import - Change MembershipRole to type import for better tree-shaking Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * address feedback * clean up permission check * revert unnecessary changes * fix procedure * allow MEMBER for insights * support teams not under orgs * fix type error * revert unnecessary changes --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

volnei · 2025-09-10T12:09:40Z

packages/trpc/server/routers/viewer/eventTypes/utils/permissionUtils.ts

+  for (const membership of memberships) {
+    const orgMembership = teamMemberships.find(
+      (teamM) => teamM.teamId === membership.team.parentId
+    )?.membershipRole;
+
+    const effectiveRole = getEffectiveRole(orgMembership, membership.role);
+    const permissions = await getTeamPermissions(userId, membership.team.id, effectiveRole);
+
+    permissionsMap.set(membership.team.id, permissions);
+  }


I think would be good to parallelize this promises..

// pseudo - not tested await Promise.all( memberships.map(async (membership) => { const orgMembership = orgRoleByTeamId.get(m.team.parentId) ?? 'member'; const effectiveRole = getEffectiveRole(orgMembership, membership.role); const permissions = await getTeamPermissions(userId, membership.team.id, effectiveRole); return [m.team.id, permissions] as const; })

volnei · 2025-09-10T12:15:11Z

apps/web/modules/event-types/views/event-types-listing-view.tsx

+        // Fallback: allow admin and owner roles
+        return profile.membershipRole === "ADMIN" || profile.membershipRole === "OWNER";


All this "ADMIN"s and "OWNER"s I think would be good to live in a constants file.