feat: integrate PermissionProvider and permission hooks#1704
feat: integrate PermissionProvider and permission hooks#1704rishiraj38 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request integrates several base components (such as Button, IconButton, ListItem, ListItemButton, and MenuItem) with a new PermissionProvider to support customizable permission actions ('showShield', 'disable', or 'hide'). It also refactors PermissionShield to retrieve user, organization, and role context from the provider instead of sessionStorage. The review feedback suggests replacing unsafe type assertions (null as unknown as JSX.Element) with empty fragments (<></>) when hiding components, and conditionally rendering the "User" row in the PermissionShield tooltip to prevent empty rows when the username is undefined.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| case 'hide': | ||
| return null as unknown as JSX.Element; |
There was a problem hiding this comment.
Using null as unknown as JSX.Element is an unsafe type assertion. Since the component is typed to return JSX.Element, you can return an empty fragment <></> instead. This is fully type-safe, avoids any casting, and renders nothing in the DOM just like null.
| case 'hide': | |
| return null as unknown as JSX.Element; | |
| case 'hide': | |
| return <></>; |
| case 'hide': | ||
| return null as unknown as JSX.Element; |
There was a problem hiding this comment.
Using null as unknown as JSX.Element is an unsafe type assertion. Since the component is typed to return JSX.Element, you can return an empty fragment <></> instead. This is fully type-safe, avoids any casting, and renders nothing in the DOM just like null.
| case 'hide': | |
| return null as unknown as JSX.Element; | |
| case 'hide': | |
| return <></>; |
| <Box sx={{ display: 'flex', justifyContent: 'space-between', gap: 1 }}> | ||
| <Typography sx={{ fontSize: '0.68rem', color: '#9E9E9E', fontWeight: 500 }}> | ||
| User | ||
| </Typography> | ||
| <Typography | ||
| sx={{ | ||
| background: 'rgba(255, 255, 255, 0.02)', | ||
| border: '1px solid rgba(255, 255, 255, 0.05)', | ||
| borderRadius: '6px', | ||
| p: 1, | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| gap: 0.75, | ||
| mb: 0.75 | ||
| fontSize: '0.72rem', | ||
| color: '#FFFFFF', | ||
| fontWeight: 600, | ||
| textAlign: 'right' | ||
| }} | ||
| > | ||
| <Box sx={{ display: 'flex', justifyContent: 'space-between', gap: 1 }}> | ||
| <Typography sx={{ fontSize: '0.68rem', color: '#9E9E9E', fontWeight: 500 }}> | ||
| User | ||
| </Typography> | ||
| <Typography | ||
| sx={{ | ||
| fontSize: '0.72rem', | ||
| color: '#FFFFFF', | ||
| fontWeight: 600, | ||
| textAlign: 'right' | ||
| }} | ||
| > | ||
| {userName} | ||
| </Typography> | ||
| </Box> | ||
| <Box sx={{ display: 'flex', justifyContent: 'space-between', gap: 1 }}> | ||
| <Typography sx={{ fontSize: '0.68rem', color: '#9E9E9E', fontWeight: 500 }}> | ||
| Org | ||
| </Typography> | ||
| <Typography | ||
| sx={{ | ||
| fontSize: '0.72rem', | ||
| color: '#FFFFFF', | ||
| fontWeight: 600, | ||
| textAlign: 'right' | ||
| }} | ||
| > | ||
| {orgName || 'Private Org'} | ||
| </Typography> | ||
| </Box> | ||
| <Box sx={{ display: 'flex', justifyContent: 'space-between', gap: 1 }}> | ||
| <Typography sx={{ fontSize: '0.68rem', color: '#9E9E9E', fontWeight: 500 }}> | ||
| Role(s) | ||
| </Typography> | ||
| <Typography | ||
| sx={{ | ||
| fontSize: '0.72rem', | ||
| color: '#FFFFFF', | ||
| fontWeight: 600, | ||
| textAlign: 'right' | ||
| }} | ||
| > | ||
| {roleNames.length > 0 ? roleNames.join(', ') : 'None'} | ||
| </Typography> | ||
| </Box> | ||
| </Box> | ||
| {userContext.userName} | ||
| </Typography> | ||
| </Box> |
There was a problem hiding this comment.
If userContext.userName is undefined (for example, if only orgName is provided), this will render an empty "User" row in the tooltip. It is better to conditionally render the "User" row only when userContext.userName is present.
{userContext.userName && (
<Box sx={{ display: 'flex', justifyContent: 'space-between', gap: 1 }}>
<Typography sx={{ fontSize: '0.68rem', color: '#9E9E9E', fontWeight: 500 }}>
User
</Typography>
<Typography
sx={{
fontSize: '0.72rem',
color: '#FFFFFF',
fontWeight: 600,
textAlign: 'right'
}}
>
{userContext.userName}
</Typography>
</Box>
)}
a786e72 to
4e92c45
Compare
banana-three-join
left a comment
There was a problem hiding this comment.
if (!permissionKey) {
return <MuiListItem {...rest} ref={ref} />;
}
// User HAS permission → render normally
if (hasPermission) {
return <MuiListItem {...rest} ref={ref} />;
}
These conditional evaluations are contradictory and should be avoided. A user without a permission key shouldn't have access to the same layouts and components as someone with a permission key. Instead, the components should be explicitly rendered in either a hidden, disabled or visible state.
On that note, I believe there isn't much that much of a difference between the disabled and show guard state so the disabled state should be removed.
Signed-off-by: Rishi Raj <rishiraj438gt@gmail.com>
4e92c45 to
1908e4d
Compare
Summary
This PR implements a generic, CASL-agnostic
PermissionProviderin Sistent and integrates it into Meshery UI. By moving permission checks into Sistent's base components and checking them declaratively via props, we decouple Sistent from direct CASL dependency and eliminate hardcodedsessionStoragelookups from within Sistent.Changes Made
Sistent Library (
/sistent)PermissionProvider(src/custom/PermissionProvider.tsx)userHasPermission: (key: Key) => booleancallback and optionaluserContextinformation.PermissionShield(src/custom/permissions.tsx)sessionStorage(e.g.,sessionStorage.getItem('currentOrg')).userName,orgName, androleNamesdirectly fromPermissionProvider'suserContext.Button,IconButton,MenuItem,ListItem,ListItemButton)permissionKey(from@meshery/schemas) andpermissionAction: 'showShield' | 'disable' | 'hide'.PermissionShieldtooltip overlay).Meshery UI (
/meshery/ui)_app.tsxPermissionProviderat the root, passing down a CASL adapter (ability.can) and feedinguserContextfrom the Redux store/queries.Navigator.tsx&navigatorComponents.tsxKeysdirectly.CAN()imports and calls insideNavigator.tsxin favor of declarativepermissionKeyandpermissionActionprops onListItemComponent.Benefits
_app.tsx.disabled={!CAN(...)}logic and wrapper elements on the caller's side.sessionStorageinside the UI library.