feat: team roles ui pbac #22585
feat: team roles ui pbac #22585
Conversation
WalkthroughThis update introduces support for scoped permissions and roles within both organizational and team contexts. A new Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/RoleSheet.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/DeleteRoleModal.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/actions.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧬 Code Graph Analysis (1)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/DeleteRoleModal.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/17/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (07/18/25)1 label was added to this PR based on Keith Williams's automation. |
dosubot
bot
added
teams
|
✅ No security or compliance issues detected. Reviewed everything up to 5980f9c. Security Overview
Detected Code Changes
Reply to this PR with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/roles/page.tsx (2)
114-115: Address the fallback roles limitationThe comment indicates that the current approach doesn't account for fallback roles. This could lead to incorrect permission checks if users have permissions through fallback roles.
Would you like me to open an issue to track implementing fallback role support in permission checks?
132-132: Remove unnecessary empty commentThis empty comment line appears to be a leftover and can be removed.
- // Use Team scope for team roles (this will automatically filter appropriate resources) -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/SettingsLayoutAppDirClient.tsx(3 hunks)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/RoleSheet.tsx(4 hunks)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/RolesList.tsx(4 hunks)apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/roles/page.tsx(1 hunks)packages/features/pbac/domain/types/permission-registry.ts(6 hunks)packages/features/pbac/infrastructure/repositories/RoleRepository.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/RoleSheet.tsx (1)
packages/features/pbac/domain/types/permission-registry.ts (1)
getPermissionsForScope(72-97)
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/SettingsLayoutAppDirClient.tsx (2)
packages/features/flags/config.ts (1)
TeamFeatures(29-29)packages/features/flags/hooks/useIsFeatureEnabledForTeam.ts (2)
TeamFeatures(5-5)useIsFeatureEnabledForTeam(7-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Security Check
🔇 Additional comments (15)
packages/features/pbac/infrastructure/repositories/RoleRepository.ts (1)
23-28: LGTM! Query logic correctly expanded to include both team-specific and system roles.The OR condition appropriately fetches both team-specific custom roles and system-wide default roles, which aligns with the PBAC model where teams should have access to both their custom roles and global system roles.
packages/features/pbac/domain/types/permission-registry.ts (4)
30-33: LGTM! Well-defined enum for permission scoping.The
Scopeenum clearly defines the two permission contexts (Team and Organization) with descriptive string values.
40-40: LGTM! Well-designed interface extension with backward compatibility.The optional
scopeproperty allows permissions to be scoped to multiple contexts while maintaining backward compatibility for existing permissions.
67-97: LGTM! Robust implementation of scope-based permission filtering.The function correctly:
- Filters permissions based on the requested scope
- Maintains backward compatibility for permissions without explicit scope
- Only includes resources that have at least one matching action
- Properly handles the
_resourcemetadata propertyThe logic is clear and well-documented.
110-110: LGTM! Appropriate scope annotations for security and functionality.The scope annotations are logically applied:
- "All" permissions restricted to Organization scope enhances security
- Team-specific permissions (e.g., ReadTeamBookings) correctly scoped to Team
- Organization-specific permissions (e.g., ManageBilling) correctly scoped to Organization
- Selective application maintains backward compatibility for permissions without explicit scope requirements
Also applies to: 180-180, 228-228, 235-235, 242-242, 249-249, 256-256, 263-263, 270-270, 277-277, 295-295, 302-302
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/RolesList.tsx (2)
3-3: LGTM! Clean interface extension with proper typing.The optional
scopeprop maintains backward compatibility while enabling scoped permission rendering.Also applies to: 50-50
59-59: LGTM! Proper prop forwarding enables scoped permission rendering.The scope parameter is correctly accepted and passed down to the RoleSheet component, enabling dynamic permission filtering based on context.
Also applies to: 113-113
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/organizations/roles/_components/RoleSheet.tsx (4)
10-14: LGTM! Import changes properly support scoped permission filtering.The imports are correctly updated to use the new scoped permission system, replacing direct registry access with the filtering function.
58-58: LGTM! Props extension maintains backward compatibility with sensible defaults.The optional
scopeprop withScope.Organizationdefault preserves existing behavior while enabling team-scoped permission filtering when needed.Also applies to: 61-61
109-119: LGTM! Filtering logic correctly updated for scoped permissions.The
filteredResourcesmemoization now usesgetPermissionsForScope(scope)to dynamically filter permissions based on the current scope, with proper dependency tracking.
236-236: LGTM! Permission rendering updated to use scoped registry.The simple permissions view now correctly uses
getPermissionsForScope(scope)to display only permissions relevant to the current scope.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/SettingsLayoutAppDirClient.tsx (3)
296-323: LGTM! Well-implemented conditional navigation component.The
TeamRolesNavItemcomponent correctly:
- Follows Rules of Hooks by calling the hook unconditionally
- Checks both sub-team status (
team.parentId) and parent PBAC enablement- Uses appropriate fallback value for
parentIdwhen checking feature flags- Implements clean early return pattern for conditional rendering
325-325: LGTM! Clean prop threading enables feature flag checking.The
teamFeaturesprop is properly passed down to enable theTeamRolesNavItemcomponent to check parent team feature flags.Also applies to: 622-622
434-435: LGTM! Well-integrated conditional navigation with clear documentation.The
TeamRolesNavItemis appropriately placed in the team navigation with a clear comment explaining its conditional nature.apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/roles/page.tsx (1)
62-71: LGTM!The metadata generation follows Next.js best practices and correctly includes the team ID in the URL path.
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/roles/page.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/roles/page.tsx
Show resolved
Hide resolved
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/roles/page.tsx
Show resolved
Hide resolved
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/roles/page.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/roles/page.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/roles/page.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(use-page-wrapper)/settings/(settings-layout)/teams/[id]/roles/page.tsx
Outdated
Show resolved
Hide resolved
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
| @@ -293,7 +293,36 @@ interface SettingsSidebarContainerProps { | |||
| teamFeatures?: Record<number, TeamFeatures>; | |||
| } | |||
|
|
|||
| const TeamListCollapsible = () => { | |||
| const TeamRolesNavItem = ({ | |||
There was a problem hiding this comment.
Link for roles tab if there is a parentId and the parent has pbac enabled
| @@ -102,16 +107,16 @@ export function RoleSheet({ role, open, onOpenChange, teamId }: RoleSheetProps) | |||
| const { isAdvancedMode, permissions, color } = form.watch(); | |||
|
|
|||
| const filteredResources = useMemo(() => { | |||
| return Object.keys(PERMISSION_REGISTRY).filter((resource) => | |||
| const scopedRegistry = getPermissionsForScope(scope); | |||
There was a problem hiding this comment.
Filter permissions by the scope of this role sheet (team,org)
| where: { | ||
| OR: [ | ||
| { teamId }, // Team-specific custom roles | ||
| { teamId: null, type: "SYSTEM" }, // Default/system roles available to all teams |
There was a problem hiding this comment.
Bring back default roles as i missed this in a merge
|
newly created team role doesn't show up in the list Screenshot.2025-07-17.at.14.46.11.mp4 |
E2E results are ready! |
What does this PR do?
This PR adds the UI for sub teams to be able to create custom roles within an organization when PBAC is enabled. Also introduces the concept of "scope" in the permission registry.
Video demo: https://cap.link/63a1m4tm9xw0qff
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Enable PBAC on an organization
Create roles on teams + orgs.
Checklist