🚀: Implement permission-based access control for protected routes#2405
🚀: Implement permission-based access control for protected routes#2405prakharsingh-74 wants to merge 6 commits intokubestellar:devfrom
Conversation
|
Welcome to KubeStellar! 🚀 Thank you for submitting this Pull Request. Before your PR can be merged, please ensure: ✅ DCO Sign-off - All commits must be signed off with ✅ PR Title - Must start with an emoji: ✨ (feature), 🐛 (bug fix), 📖 (docs), 🌱 (infra/tests), Getting Started with KubeStellar: Contributor Resources:
🌟 Help KubeStellar Grow - We Need Adopters! Our roadmap is driven entirely by adopter feedback. Whether you're using KubeStellar yourself or know someone who could benefit from multi-cluster Kubernetes: 📋 Take our Multi-Cluster Survey - Share your use cases and help shape our direction! A maintainer will review your PR soon. Feel free to ask questions in the comments or on Slack! |
|
Hi @prakharsingh-74. Thanks for your PR. I'm waiting for a kubestellar member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Signed-off-by: prakharsingh-74 <prakharsingh7014@gmail.com>
33c3178 to
b6c6d8e
Compare
|
/ok-to-test |
There was a problem hiding this comment.
Pull request overview
Implements frontend permission-based route guarding so authenticated users can only access protected pages when they have the required admin flag and/or component-level permissions (addressing issue #2079).
Changes:
- Added permission utility helpers (
hasPermission,hasAllPermissions,hasAnyPermission). - Added a
useUserPermissions()hook to fetch/api/me(user + permissions). - Enhanced
ProtectedRouteand updated route config to enforce per-route permission requirements and admin-only access.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| frontend/src/utils/permissionUtils.ts | Introduces reusable permission-checking helpers. |
| frontend/src/hooks/useUserPermissions.ts | Adds react-query hook for fetching current user permissions from /api/me. |
| frontend/src/components/ProtectedRoute.tsx | Adds permission + admin checks and session-expiry redirect behavior. |
| frontend/src/routes/routes-config.tsx | Attaches permission requirements to multiple protected routes. |
Comments suppressed due to low confidence (2)
frontend/src/routes/routes-config.tsx:85
- The
requiredPermission.componentvalues used here (e.g.,clusters,workloads,binding-policies,plugins) don’t match the permission component IDs currently managed/returned elsewhere (dashboard,resources,system,usersvia/api/memock and User Management UI). As-is, users (including admins in mocks/e2e) will likely be denied access to these routes because the keys won’t exist inuser.permissions. Align the route guard component IDs with the backend/admin UI permission taxonomy, or update the permission model (UI + mocks + API contract) in the same PR so these routes can be granted correctly.
path: 'its',
element: (
<ProtectedRoute requiredPermission={{ component: 'clusters', level: 'read' }}>
<Suspense fallback={<LoadingFallback message="Loading ITS..." size="small" />}>
<ITSLazy />
</Suspense>
</ProtectedRoute>
frontend/src/routes/routes-config.tsx:145
- Only the explicit plugin manager/marketplace routes are permission-gated here. Dynamically injected
pluginRoutesare created inPluginLoaderwrapped with<ProtectedRoute>but without anyrequiredPermission, so plugin pages can still be accessed by any authenticated user. Consider enforcing a permission requirement for pluginRoutes as well (either by passing arequiredPermissionwhen generating plugin routes, or wrapping/mappingpluginRouteshere).
<ProtectedRoute requiredPermission={{ component: 'plugins', level: 'read' }}>
<Suspense
fallback={<LoadingFallback message="Loading Plugin Manager..." size="medium" />}
>
<PluginManagerLazy />
</Suspense>
</ProtectedRoute>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@oksaumya and @MAVRICK-1 can you please review this PR? |
|
/lgtm |
|
LGTM label has been added. DetailsGit tree hash: 0671e45225201d8e70e23685e9decfb7c6b2b5a2 |
|
/approve |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: naman9271 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Fixes #2079
Description
Problem
Users without proper permissions could access and view all resources, system pages, and admin features regardless of their permission settings. This is a critical security vulnerability that allows unauthorized access to:
Steps to Reproduce:
Solution
Implemented a comprehensive permission-based access control system with three layers of protection:
Changes Made
const { data: user, isLoading } = useUserPermissions();Route Protection Mapping
Testing Instructions
Setup Test Users
User 1: No Permissions
User 2: Read Only
User 3: Admin
Test Cases
Test 1: Unauthorized Access
Test 2: Read-Only Access
Test 3: Admin Access
2 .Navigate to /admin/users
Test 4: Session Expiry
Checklist
Please ensure the following before submitting your PR: