dashboard: User-aware dashboard navigation and route access #5153
kevmtt
started this conversation in
Feature Requests
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Summary
The React dashboard has no way to decide navigation or route access based on who is logged in. The only per-viewer condition anywhere in the nav pipeline is
NavMenuItem.requiresPermission, and no route is guarded at all. This proposal adds a small user context shared by three new extension points: reactive nav modifiers, a per-itemisVisiblepredicate, and route access rules. I would like to implement it.Motivation
Two concrete cases.
ReadCatalogandReadOrderfor the POS itself to work, so permissions cannot express "everything except POS is hidden and unreachable". Today the only options are to accept a sidebar full of entries they must not use, or to invent a parallel set of custom permissions purely to drive the UI.The common thread is that permissions answer "what may this user call on the API", which is not the same question as "what should this user see and be able to open". Today the second question can only be answered by the first.
Current behaviour in 3.7.2
Verified in the
packages/dashboardsource:src/lib/components/layout/nav-main.tsxis the only per-viewer filter.isItemAllowedchecksrequiresPermissionviausePermissions(), sections whose items are all filtered out are dropped, and a top-level link is checked directly.src/lib/hooks/use-permissions.ts: a permission list is OR (permissions.some), evaluated against the currently selected channel.src/lib/framework/extension-api/define-dashboard-extension.ts: the function form ofnavSectionsruns once, at extension load, in phase 2, and its result is written into the global registry. It receives onlyNavMenuConfig. It is not React, it cannot see the user, and it does not re-run on channel switch.src/app/routes/_authenticated.tsx:beforeLoadchecksisAuthenticatedand nothing else.DashboardRouteDefinitionhas no field for access control, andsrc/lib/framework/page/use-extended-router.tsxskips any extension route whose path already exists, so a core route cannot be shadowed by a restricted one.src/lib/providers/auth.tsx:CurrentUserInformationselectsactiveAdministrator { id firstName lastName emailAddress }andme { channels { id token code permissions } }. Roles are not fetched, and unlikeprofile.tsxthe query is not wrapped inaddCustomFields(), so administrator custom fields are not available either.src/lib/components/shared/permission-guard.tsxexists but rendersnull, and is used for buttons and page blocks, never for routes. There is no "not authorized" screen in the package.What is missing
Proposal
One user context object, three consumers. The three parts are separable and could land as three PRs.
1. A shared user context
Extend
CurrentUserInformationwithactiveAdministrator { user { roles { id code description permissions } } }and wrap it inaddCustomFields(), then expose:This part is useful on its own. Administrator custom fields and roles are a common thing to branch dashboard UI on, and today every plugin that needs them re-queries
activeAdministratoritself.2. Nav resolution becomes a pipeline
Keep the registry as the source config and resolve it at render time in a new
useNavMenu()hook:NavMainbecomes a consumer of that hook, which also makes the filtering unit-testable without rendering a sidebar.New extension surface:
and on the item itself, ANDed with
requiresPermissionso the two compose:The existing one-shot
navSectionsfunction form stays exactly as it is. Reactive modifiers are a separate registration, so no existing extension changes behaviour.Two small additions decide whether this is pleasant or miserable to use:
framework/defaults.ts, but today an author has to read the source to learn them.keepOnly(config, ids),removeItems(config, ids),updateItem(config, id, patch).3. Route access
For extension routes,
DashboardRouteDefinitiongains the same two conditions:Built-in routes are file-based across the whole
app/routestree, so a per-file option is not realistic. Instead, a central registration matched by path, enforced in the_authenticatedbeforeLoadthat already exists and already hascontext.authin scope:Most specific match wins, and no match means allowed, so this is additive. A denied route renders a new
NotAuthorizedPagerather than a blank screen, which the package currently has no equivalent of.I considered deriving route access from the resolved nav ("if you cannot see it you cannot open it") and rejected it, because detail routes, deep links and modal routes have no nav entry and would break.
Worked example: floor staff
This is not an authorization boundary, and the docs should say so
Everything above is client-side. A user who can call
orderson the Admin API can still call it with curl whatever the dashboard renders. The real boundary stays where it is: permissions, channel-scoped roles, and resolver-level guards. What this proposal fixes is that today there is no supported way to make the dashboard agree with a server-side rule, so plugin authors either ship a sidebar full of entries their users must not use, or invent custom permissions purely to drive the UI. I would add a note to this effect in the docs for bothrequiresPermissionand the new API.Backwards compatibility
Additive. No existing field changes meaning, the current one-shot
navSectionsmodifier keeps its exact semantics, and an absentcanAccessorrouteAccessentry means allowed. The one behavioural change is that nav resolution moves from load time to render time, so a modifier that previously ran once now runs on every sidebar render. That applies only to the new reactive lane.Open questions
hasRoleOnActiveChannelmay be worth adding.navMenu: { modifiers, filters }above, but flatnavMenuModifiersandnavMenuFiltersprops would match the existing style of the extension API more closely.filterslane calls registered hooks in a loop. That is safe in practice, sinceAppSidebarrenders behindextensionsLoadedand the registration count is fixed for the session, but it is the one part of this proposal that trades a sharp edge for reach. It could be dropped from a first version.routeAccessalso apply to the breadcrumbs and anywhere else that reads the same nav config?Prior art
Implementation
I would like to implement this, against the
minorbranch, with@since 3.8.0on the new public APIs, split into three PRs matching the three parts above and starting with the user context.All reactions