feat: pbac - org pages check update permissions#24164
Conversation
WalkthroughThis PR replaces organization admin-only guards with general organization or permission-based validation across several organization settings pages. Calls to validateUserHasOrgAdmin were switched to either validateUserHasOrg or validateUserHasOrgPerms. validateUserHasOrgPerms is invoked with explicit permission strings (e.g., "organization.attributes.create", "organization.attributes.update", "organization.update") and fallbackRoles [MembershipRole.OWNER, MembershipRole.ADMIN]; MembershipRole is imported from @calcom/prisma/enums where used. Import paths and call-site signatures were updated accordingly; page/component exports and metadata generation were not changed. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.tsx📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Files:
**/*.{ts,tsx,js,jsx}⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (1)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/delegation-credential/page.tsx (2)
🔇 Additional comments (3)
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. Comment |
|
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: |
| const { canEdit } = await getResourcePermissions({ | ||
| userId: session.user.id, | ||
| teamId: session.user.profile.organizationId, | ||
| resource: Resource.Organization, | ||
| userRole: session.user.org.role, | ||
| fallbackRoles: { | ||
| update: { | ||
| roles: [MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Permission check happens here
| const { canRead, canEdit } = await getResourcePermissions({ | ||
| userId: session.user.id, | ||
| teamId: session.user.profile.organizationId, | ||
| resource: Resource.Organization, | ||
| userRole: session.user.org.role, | ||
| fallbackRoles: { | ||
| read: { | ||
| roles: [MembershipRole.MEMBER, MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }, | ||
| update: { | ||
| roles: [MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Permission check happens here
| const { canEdit } = await getResourcePermissions({ | ||
| userId: session.user.id, | ||
| teamId: session.user.profile.organizationId, | ||
| resource: Resource.Organization, | ||
| userRole: session.user.org.role, | ||
| fallbackRoles: { | ||
| update: { | ||
| roles: [MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Permission check happens here
| const { canRead, canEdit, canDelete, canCreate } = await getResourcePermissions({ | ||
| userId: session.user.id, | ||
| teamId: session.user.profile.organizationId, | ||
| resource: Resource.Attributes, | ||
| userRole: session.user.org.role, | ||
| fallbackRoles: { | ||
| read: { | ||
| roles: [MembershipRole.MEMBER, MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }, | ||
| update: { | ||
| roles: [MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }, | ||
| delete: { | ||
| roles: [MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }, | ||
| create: { | ||
| roles: [MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Permission check happens here
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/privacy/page.tsx (1)
26-28: Redundant validation aftervalidateUserHasOrg.The
validateUserHasOrgfunction (line 24) already validates these exact conditions and redirects if they fail. This additional check is unnecessary and adds maintenance overhead.Apply this diff to remove the redundant validation:
const session = await validateUserHasOrg(); - if (!session?.user.id || !session?.user.profile?.organizationId || !session?.user.org) { - return redirect("/settings/profile"); - } - const { canRead, canEdit } = await getResourcePermissions({apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/sso/page.tsx (1)
25-27: Redundant validation aftervalidateUserHasOrg.The
validateUserHasOrgfunction (line 23) already validates these conditions and redirects if they fail. This additional check is unnecessary.Apply this diff to remove the redundant validation:
const session = await validateUserHasOrg(); - if (!session?.user.id || !session?.user.profile?.organizationId || !session?.user.org) { - return redirect("/settings/organizations/general"); - } - const { canEdit } = await getResourcePermissions({
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/[id]/edit/page.tsx(2 hunks)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/create/page.tsx(2 hunks)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/page.tsx(2 hunks)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/delegation-credential/page.tsx(2 hunks)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/dsync/page.tsx(2 hunks)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/privacy/page.tsx(2 hunks)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/sso/page.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/privacy/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/dsync/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/create/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/sso/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/[id]/edit/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/delegation-credential/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/privacy/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/dsync/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/create/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/sso/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/[id]/edit/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/delegation-credential/page.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/privacy/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/dsync/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/create/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/sso/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/[id]/edit/page.tsxapps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/delegation-credential/page.tsx
🧬 Code graph analysis (7)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/privacy/page.tsx (2)
packages/features/auth/lib/next-auth-options.ts (1)
session(746-771)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/actions/validateUserHasOrg.tsx (1)
validateUserHasOrg(19-30)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/page.tsx (2)
packages/features/auth/lib/next-auth-options.ts (1)
session(746-771)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/actions/validateUserHasOrg.tsx (1)
validateUserHasOrg(19-30)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/dsync/page.tsx (2)
packages/features/auth/lib/next-auth-options.ts (1)
session(746-771)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/actions/validateUserHasOrg.tsx (1)
validateUserHasOrg(19-30)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/create/page.tsx (2)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/actions/validateUserHasOrgPerms.tsx (1)
validateUserHasOrgPerms(9-34)packages/platform/libraries/index.ts (1)
MembershipRole(34-34)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/sso/page.tsx (2)
packages/features/auth/lib/next-auth-options.ts (1)
session(746-771)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/actions/validateUserHasOrg.tsx (1)
validateUserHasOrg(19-30)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/[id]/edit/page.tsx (2)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/actions/validateUserHasOrgPerms.tsx (1)
validateUserHasOrgPerms(9-34)packages/platform/libraries/index.ts (1)
MembershipRole(34-34)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/delegation-credential/page.tsx (2)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/actions/validateUserHasOrgPerms.tsx (1)
validateUserHasOrgPerms(9-34)packages/platform/libraries/index.ts (1)
MembershipRole(34-34)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (10)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/privacy/page.tsx (2)
10-10: LGTM! Permission check successfully migrated to PBAC model.The change from
validateUserHasOrgAdmintovalidateUserHasOrgcorrectly enables PBAC-based access control, allowing members with custom admin roles to access this page.Also applies to: 24-24
30-43: Permission model correctly implements read/write separation.The fallback roles appropriately allow all members to read privacy settings while restricting updates to admins and owners, aligning with the PR's PBAC objectives.
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/[id]/edit/page.tsx (1)
4-4: LGTM! Correctly migrated to permission-based validation.The change to
validateUserHasOrgPermswith explicit permission"organization.attributes.update"and fallback roles[OWNER, ADMIN]properly restricts attribute editing to authorized users while enabling PBAC for custom roles.Also applies to: 6-6, 18-21
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/create/page.tsx (1)
4-4: LGTM! Permission-based validation correctly implemented.The migration to
validateUserHasOrgPermswith permission"organization.attributes.create"and fallback roles[OWNER, ADMIN]maintains appropriate access control while enabling PBAC support for custom roles.Also applies to: 6-6, 18-21
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/sso/page.tsx (2)
10-10: LGTM! Permission check migrated to PBAC model.The change from
validateUserHasOrgAdmintovalidateUserHasOrgcorrectly enables PBAC-based access control for SSO configuration.Also applies to: 23-23
29-39: Permission model appropriately restricts SSO configuration.The update permission with fallback roles
[ADMIN, OWNER]correctly limits SSO configuration changes to authorized administrators, aligning with security best practices.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/dsync/page.tsx (2)
9-9: LGTM! Permission check migrated to PBAC model.The change from
validateUserHasOrgAdmintovalidateUserHasOrgcorrectly enables PBAC-based access control for directory sync configuration.Also applies to: 22-22
24-34: Permission model appropriately restricts directory sync configuration.The update permission with fallback roles
[ADMIN, OWNER]correctly limits directory sync changes to authorized administrators. The implementation cleanly avoids redundant validation checks present in other files.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/attributes/page.tsx (1)
10-10: LGTM! Permission model correctly updated.The change from
validateUserHasOrgAdmintovalidateUserHasOrgenables the PBAC flow described in the PR objectives. Users with custom roles (whererole="Member"butcustom_role_idis set to an admin role) will now pass the initial organization check, and the actual authorization is properly enforced viagetResourcePermissionswith thefallbackRolesconfiguration on lines 25-44.Also applies to: 23-23
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/(org-admin-only)/delegation-credential/page.tsx (1)
6-6: Permissions are consistent across org-admin pages.
EachvalidateUserHasOrgPermscall uses a page-specific permission string with the same fallback roles (MembershipRole.OWNER,MembershipRole.ADMIN), so no changes needed.
...er)/settings/(settings-layout)/organizations/(org-admin-only)/delegation-credential/page.tsx
Show resolved
Hide resolved
hbjORbj
changed the title
E2E results are ready! |
3 participants
What does this PR do?
This PR allows members who have been upgraded to a custom role (user.role stays as Member) to access organization resources with the PBAC permission. Perviously we still were checking for
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/actions/validateUserHasOrgAdmin.tsxhasOrgAdmin.This is the edgecasae when you enable PBAC on current org (not new org) and users who are members get made into a cus
How should this be tested?
Enable PBAC on an org.
Manually update a customrole_id of a memebrship to admin_role -> Leave role as "member"
navigate around the organisation settings and veirfy you can view the pages
Checklist