-
-
Notifications
You must be signed in to change notification settings - Fork 83
Feat: Team Management With Permissions and Roles #229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… passed to rbac related guard as props
|
To generate Unit Tests for this PR, please click here. |
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThis update introduces comprehensive role-based access control (RBAC) checks throughout the application's settings UI and API. It restructures feature flag API routes to separate read and write access, applies RBAC guards to various UI components, and adjusts tab visibility and layouts based on user permissions. Minor formatting and fallback handling changes are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI_Component
participant RBACGuard
participant Server
User->>UI_Component: Attempt interactive action (e.g., update avatar, toggle flag)
UI_Component->>RBACGuard: Check permission (resource, action)
alt Permission granted
RBACGuard-->>UI_Component: Allow action/render
UI_Component->>Server: Send request (if applicable)
Server-->>UI_Component: Respond with result
else Permission denied
RBACGuard-->>UI_Component: Block action/hide element
end
sequenceDiagram
participant Client
participant API_ReadGroup
participant API_WriteGroup
participant RBAC_Middleware
Client->>API_ReadGroup: GET /feature-flags
API_ReadGroup-->>Client: Return feature flags (no RBAC)
Client->>API_WriteGroup: PUT /feature-flags
API_WriteGroup->>RBAC_Middleware: Check write permission
alt Permission granted
RBAC_Middleware-->>API_WriteGroup: Allow
API_WriteGroup-->>Client: Update feature flag
else Permission denied
RBAC_Middleware-->>API_WriteGroup: Deny
API_WriteGroup-->>Client: Access denied
end
Poem
✨ Finishing Touches
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. 🪧 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 (
|
|
@coderabbitai pause |
|
Tip For best results, initiate chat on the files or code changes. @raghavyuva I've stopped the review process as requested. The current review operation has been halted. For future reference, you can also use 🐰 stops mid-hop and sits quietly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
view/app/settings/general/components/SecuritySection.tsx (1)
54-63: LGTM! Appropriate RBAC protection for password reset.The RBAC guard correctly protects the password reset functionality with
resource="user" action="update".Consider adding a
fallbackprop to provide user feedback when the action is not permitted, rather than having the button disappear entirely.- <RBACGuard resource="user" action="update"> + <RBACGuard + resource="user" + action="update" + fallback={<p className="text-sm text-muted-foreground">{t('settings.security.password.reset.noPermission')}</p>} + >view/app/settings/general/components/AvatarSection.tsx (1)
24-30: LGTM! Consistent RBAC protection for avatar updates.The RBAC guard appropriately protects avatar upload functionality with
resource="user" action="update", maintaining consistency with other user settings components.Similar to
SecuritySection.tsx, consider adding a fallback to inform users when they lack permission to update their avatar.- <RBACGuard resource="user" action="update"> + <RBACGuard + resource="user" + action="update" + fallback={<p className="text-sm text-muted-foreground text-center">{t('settings.account.avatar.noPermission')}</p>} + >view/app/settings/general/components/FeatureFlagsSettings.tsx (1)
58-104: Consider the user experience impact of nested RBAC guards.The nested RBAC structure (outer read guard, inner update guards) is logically correct but may create a confusing user experience. Users with read-only permissions will see the feature toggles but won't be able to interact with them, potentially leading to frustration.
Consider either:
- Adding visual indicators (e.g., tooltips) explaining why switches are disabled
- Restructuring to hide the switches entirely for users without update permissions
The current implementation is functionally correct but could benefit from UX improvements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
api/api/versions.json(1 hunks)api/internal/routes.go(2 hunks)view/app/settings/general/components/AccountSection.tsx(6 hunks)view/app/settings/general/components/AvatarSection.tsx(2 hunks)view/app/settings/general/components/FeatureFlagsSettings.tsx(2 hunks)view/app/settings/general/components/SecuritySection.tsx(2 hunks)view/app/settings/general/components/TwoFactorSetup.tsx(4 hunks)view/app/settings/general/page.tsx(3 hunks)view/app/settings/notifications/page.tsx(3 hunks)view/components/rbac/PermissionGuard.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
view/app/settings/general/components/AvatarSection.tsx (1)
view/components/rbac/RBACGuard.tsx (1)
RBACGuard(16-59)
view/app/settings/notifications/page.tsx (1)
view/lib/rbac.ts (1)
useRBAC(26-58)
🔇 Additional comments (6)
api/api/versions.json (1)
6-10: LGTM! Routine version metadata update.The release date update and formatting improvement for the changes array are appropriate for this PR.
api/internal/routes.go (2)
225-235: LGTM! Excellent separation of read/write access for feature flags.The architectural decision to allow unrestricted read access while protecting write operations with RBAC is sound and follows the principle of least privilege.
392-396: FeatureFlagRoutes signature usage verified
All calls toFeatureFlagRouteshave been updated to the new(readGroup, writeGroup, controller)signature.
• api/internal/routes.go:router.FeatureFlagRoutes(featureFlagReadGroup, featureFlagWriteGroup, featureFlagController)view/app/settings/general/components/TwoFactorSetup.tsx (1)
19-19: RBAC implementation looks good and consistent.The RBAC guards are properly implemented around all 2FA action buttons using consistent
resource="user"andaction="update"permissions. This appropriately restricts 2FA management operations to users with update permissions on user resources.Also applies to: 74-78, 87-91, 114-122
view/app/settings/general/page.tsx (1)
12-13: Proper conditional rendering based on RBAC permissions.The implementation correctly uses
canAccessResource('feature-flags', 'read')to conditionally render the Feature Flags tab and dynamically adjust the grid layout. This provides a clean user experience where users only see tabs they have permission to access.Also applies to: 17-17, 43-44, 53-59, 98-98
view/app/settings/general/components/AccountSection.tsx (1)
25-25: Consistent RBAC implementation across user preference controls.The RBAC guards are consistently applied to all user preference controls using the appropriate
resource="user"andaction="update"permissions. This creates a uniform access control pattern across username updates, email verification, theme settings, font preferences, language selection, and auto-update configuration.Also applies to: 122-129, 161-174, 202-204, 208-243, 257-263, 277-283
✅ Actions performedReviews paused. |
Feat: Team Management With Permissions and Roles
Summary by CodeRabbit
New Features
Refactor
Chores