Skip to content

feat: integrate PermissionProvider and permission hooks#1704

Open
rishiraj38 wants to merge 2 commits into
layer5io:masterfrom
rishiraj38:feat/permission-provider
Open

feat: integrate PermissionProvider and permission hooks#1704
rishiraj38 wants to merge 2 commits into
layer5io:masterfrom
rishiraj38:feat/permission-provider

Conversation

@rishiraj38

@rishiraj38 rishiraj38 commented Jul 10, 2026

Copy link
Copy Markdown
Member

Summary

This PR implements a generic, CASL-agnostic PermissionProvider in 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 hardcoded sessionStorage lookups from within Sistent.


Changes Made

Sistent Library (/sistent)
  1. PermissionProvider (src/custom/PermissionProvider.tsx)
    • Added a new CASL-agnostic context/hook wrapper. It accepts a generic userHasPermission: (key: Key) => boolean callback and optional userContext information.
  2. PermissionShield (src/custom/permissions.tsx)
    • Removed direct dependency on sessionStorage (e.g., sessionStorage.getItem('currentOrg')).
    • Now reads userName, orgName, and roleNames directly from PermissionProvider's userContext.
  3. Base Components (Button, IconButton, MenuItem, ListItem, ListItemButton)
    • Updated components to accept permissionKey (from @meshery/schemas) and permissionAction: 'showShield' | 'disable' | 'hide'.
    • The components automatically evaluate permissions via the provider hook and apply the appropriate action if the user lacks the permission (e.g., automatically rendering the PermissionShield tooltip overlay).
Meshery UI (/meshery/ui)
  1. _app.tsx
    • Mounted the PermissionProvider at the root, passing down a CASL adapter (ability.can) and feeding userContext from the Redux store/queries.
  2. Navigator.tsx & navigatorComponents.tsx
    • Converted Navigator item configurations to pass the canonical Keys directly.
    • Removed custom CAN() imports and calls inside Navigator.tsx in favor of declarative permissionKey and permissionAction props on ListItemComponent.

Benefits

  • Decoupled Architecture: Sistent and its base components are now completely unaware of CASL. Transitioning away from CASL in the future will require changing only one function inside _app.tsx.
  • Cleaner Code: Eliminates redundant disabled={!CAN(...)} logic and wrapper elements on the caller's side.
  • No Storage Leakage: Removed direct dependencies on sessionStorage inside the UI library.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +55 to +56
case 'hide':
return null as unknown as JSX.Element;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
case 'hide':
return null as unknown as JSX.Element;
case 'hide':
return <></>;

Comment on lines +37 to +38
case 'hide':
return null as unknown as JSX.Element;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
case 'hide':
return null as unknown as JSX.Element;
case 'hide':
return <></>;

Comment on lines +285 to +299
<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>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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>
            )}

@banana-three-join banana-three-join left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  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.

Comment thread src/custom/PermissionProvider.tsx
Signed-off-by: Rishi Raj <rishiraj438gt@gmail.com>
@rishiraj38 rishiraj38 force-pushed the feat/permission-provider branch from 4e92c45 to 1908e4d Compare July 10, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants