refactor: unify dashboard menu permission routing#88
Merged
Conversation
Collaborator
|
@codex review |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR centralizes dashboard route permission logic so that sidebar visibility, route guarding, and post-auth “landing” redirects all follow the same access rules (including treating /oidc as admin-only).
Changes:
- Added shared dashboard route metadata/helpers and a
useFirstAccessibleDashboardRoutehook to resolve the first accessible dashboard route (or fallback). - Moved admin-status resolution into the permissions flow and updated dashboard guarding/sidebar visibility to use the shared access helper.
- Updated dashboard/auth entrypoints (home redirect, login/SSO callback, 403 back button) and performance cards to avoid linking to inaccessible routes.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| types/app-config.d.ts | Removes NavItem.isAdminOnly from the shared nav item type. |
| lib/dashboard-route-meta.ts | Introduces centralized admin-only routing + menu-derived route list + access helpers. |
| lib/console-permissions.ts | Removes legacy ADMIN_ONLY_PATHS constant from permissions layer. |
| hooks/use-permissions.tsx | Adds admin-resolution step (/is-admin) and exposes hasResolvedAdmin to coordinate permission readiness. |
| hooks/use-first-accessible-dashboard-route.ts | New hook to compute the first accessible dashboard route once permissions are ready. |
| config/navs.ts | Removes isAdminOnly flags from nav config; visibility now relies on permission routing. |
| components/user/dropdown.tsx | Removes redundant admin-status initialization from the dropdown. |
| components/dashboard-auth-guard.tsx | Uses centralized canAccessDashboardRoute and waits for admin/policy readiness before guarding. |
| components/app-sidebar.tsx | Filters nav visibility using centralized access logic; updates “home” link to first accessible route. |
| app/(dashboard)/status/page.tsx | Avoids linking summary cards to /browser when the user can’t access it. |
| app/(dashboard)/page.tsx | Replaces hard redirect to /browser with client-side redirect to first accessible route. |
| app/(dashboard)/403/page.tsx | “Back to Home” now routes to first accessible route. |
| app/(auth)/auth/oidc-callback/page.tsx | Redirects to a safe redirect target or first accessible route after SSO login. |
| app/(auth)/auth/login/page.tsx | Redirects authenticated users to the first accessible route instead of always /browser. |
Comments suppressed due to low confidence (1)
lib/console-permissions.ts:26
PAGE_PERMISSIONSstill includes an entry for/oidc, but/oidcis now treated as admin-only viaADMIN_ONLY_DASHBOARD_ROUTES/canAccessDashboardRoute. Having both mechanisms in play for the same route is contradictory and can mislead future changes (e.g., someone may grantconsole:OIDCSettingsexpecting it to work for non-admins). Consider removing/oidcfromPAGE_PERMISSIONS(or clearly documenting that admin-only routing overrides scope-based permissions).
export const PAGE_PERMISSIONS: Record<string, ConsoleScope[]> = {
"/browser": [CONSOLE_SCOPES.VIEW_BROWSER],
"/buckets": [CONSOLE_SCOPES.VIEW_BROWSER],
"/access-keys": [CONSOLE_SCOPES.VIEW_ACCESS_KEYS],
"/policies": [CONSOLE_SCOPES.VIEW_POLICIES],
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing