fix: RBAC hide right menu (#16902)

* add provider to right menu * add roles code * bad pull
2026-04-19 08:04:53 +00:00 · 2021-09-30 14:33:54 -04:00
parent 87290ddcc8
commit 87baac7650
3 changed files with 204 additions and 126 deletions
--- a/superset-frontend/src/components/Menu/Menu.test.tsx
+++ b/superset-frontend/src/components/Menu/Menu.test.tsx
@@ -17,12 +17,32 @@
 * under the License.
 */
 import React from 'react';
+import * as reactRedux from 'react-redux';
 import { render, screen } from 'spec/helpers/testing-library';
 import userEvent from '@testing-library/user-event';
 import { Menu } from './Menu';
 import { dropdownItems } from './MenuRight';

+const user = {
+  createdOn: '2021-04-27T18:12:38.952304',
+  email: 'admin',
+  firstName: 'admin',
+  isActive: true,
+  lastName: 'admin',
+  permissions: {},
+  roles: {
+    Admin: [
+      ['can_sqllab', 'Superset'],
+      ['can_write', 'Dashboard'],
+      ['can_write', 'Chart'],
+    ],
+  },
+  userId: 1,
+  username: 'admin',
+};
+
 const mockedProps = {
+  user,
  data: {
    menu: [
      {
@@ -136,17 +156,27 @@ const notanonProps = {
  },
 };

+const useSelectorMock = jest.spyOn(reactRedux, 'useSelector');
+
+beforeEach(() => {
+  // setup a DOM element as a render target
+  useSelectorMock.mockClear();
+});
+
 test('should render', () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  const { container } = render(<Menu {...mockedProps} />);
  expect(container).toBeInTheDocument();
 });

 test('should render the navigation', () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  render(<Menu {...mockedProps} />);
  expect(screen.getByRole('navigation')).toBeInTheDocument();
 });

 test('should render the brand', () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  const {
    data: {
      brand: { alt, icon },
@@ -158,6 +188,7 @@ test('should render the brand', () => {
 });

 test('should render all the top navbar menu items', () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  const {
    data: { menu },
  } = mockedProps;
@@ -168,6 +199,7 @@ test('should render all the top navbar menu items', () => {
 });

 test('should render the top navbar child menu items', async () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  const {
    data: { menu },
  } = mockedProps;
@@ -184,6 +216,7 @@ test('should render the top navbar child menu items', async () => {
 });

 test('should render the dropdown items', async () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  render(<Menu {...notanonProps} />);
  const dropdown = screen.getByTestId('new-dropdown-icon');
  userEvent.hover(dropdown);
@@ -211,12 +244,14 @@ test('should render the dropdown items', async () => {
 });

 test('should render the Settings', async () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  render(<Menu {...mockedProps} />);
  const settings = await screen.findByText('Settings');
  expect(settings).toBeInTheDocument();
 });

 test('should render the Settings menu item', async () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  render(<Menu {...mockedProps} />);
  userEvent.hover(screen.getByText('Settings'));
  const label = await screen.findByText('Security');
@@ -224,6 +259,7 @@ test('should render the Settings menu item', async () => {
 });

 test('should render the Settings dropdown child menu items', async () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  const {
    data: { settings },
  } = mockedProps;
@@ -234,16 +270,19 @@ test('should render the Settings dropdown child menu items', async () => {
 });

 test('should render the plus menu (+) when user is not anonymous', () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  render(<Menu {...notanonProps} />);
  expect(screen.getByTestId('new-dropdown')).toBeInTheDocument();
 });

 test('should NOT render the plus menu (+) when user is anonymous', () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  render(<Menu {...mockedProps} />);
  expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument();
 });

 test('should render the user actions when user is not anonymous', async () => {
+  useSelectorMock.mockReturnValue({ roles: mockedProps.user.roles });
  const {
    data: {
      navbar_right: { user_info_url, user_logout_url },
@@ -263,11 +302,13 @@ test('should render the user actions when user is not anonymous', async () => {
 });

 test('should NOT render the user actions when user is anonymous', () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  render(<Menu {...mockedProps} />);
  expect(screen.queryByText('User')).not.toBeInTheDocument();
 });

 test('should render the Profile link when available', async () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  const {
    data: {
      navbar_right: { user_profile_url },
@@ -282,6 +323,7 @@ test('should render the Profile link when available', async () => {
 });

 test('should render the About section and version_string, sha or build_number when available', async () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  const {
    data: {
      navbar_right: { version_sha, version_string, build_number },
@@ -301,6 +343,7 @@ test('should render the About section and version_string, sha or build_number wh
 });

 test('should render the Documentation link when available', async () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  const {
    data: {
      navbar_right: { documentation_url },
@@ -313,6 +356,7 @@ test('should render the Documentation link when available', async () => {
 });

 test('should render the Bug Report link when available', async () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  const {
    data: {
      navbar_right: { bug_report_url },
@@ -325,6 +369,7 @@ test('should render the Bug Report link when available', async () => {
 });

 test('should render the Login link when user is anonymous', () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  const {
    data: {
      navbar_right: { user_login_url },
@@ -337,6 +382,13 @@ test('should render the Login link when user is anonymous', () => {
 });

 test('should render the Language Picker', () => {
+  useSelectorMock.mockReturnValue({ roles: user.roles });
  render(<Menu {...mockedProps} />);
  expect(screen.getByLabelText('Languages')).toBeInTheDocument();
 });
+
+test('should hide create button without proper roles', () => {
+  useSelectorMock.mockReturnValue({ roles: [] });
+  render(<Menu {...notanonProps} />);
+  expect(screen.queryByTestId('new-dropdown')).not.toBeInTheDocument();
+});
--- a/superset-frontend/src/components/Menu/MenuRight.tsx
+++ b/superset-frontend/src/components/Menu/MenuRight.tsx
@@ -21,6 +21,9 @@ import { MainNav as Menu } from 'src/common/components';
 import { t, styled, css, SupersetTheme } from '@superset-ui/core';
 import { Link } from 'react-router-dom';
 import Icons from 'src/components/Icons';
+import findPermission from 'src/dashboard/util/findPermission';
+import { useSelector } from 'react-redux';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
 import LanguagePicker from './LanguagePicker';
 import { NavBarProps, MenuObjectProps } from './Menu';

@@ -29,16 +32,22 @@ export const dropdownItems = [
    label: t('SQL query'),
    url: '/superset/sqllab?new=true',
    icon: 'fa-fw fa-search',
+    perm: 'can_sqllab',
+    view: 'Superset',
  },
  {
    label: t('Chart'),
    url: '/chart/add',
    icon: 'fa-fw fa-bar-chart',
+    perm: 'can_write',
+    view: 'Dashboard',
  },
  {
    label: t('Dashboard'),
    url: '/dashboard/new',
    icon: 'fa-fw fa-dashboard',
+    perm: 'can_write',
+    view: 'Chart',
  },
 ];

@@ -83,134 +92,146 @@ const RightMenu = ({
  settings,
  navbarRight,
  isFrontendRoute,
-}: RightMenuProps) => (
-  <StyledDiv align={align}>
-    <Menu mode="horizontal">
-      {!navbarRight.user_is_anonymous && (
-        <SubMenu
-          data-test="new-dropdown"
-          title={
-            <StyledI data-test="new-dropdown-icon" className="fa fa-plus" />
-          }
-          icon={<Icons.TriangleDown />}
-        >
-          {dropdownItems.map(menu => (
-            <Menu.Item key={menu.label}>
-              <a href={menu.url}>
-                <i
-                  data-test={`menu-item-${menu.label}`}
-                  className={`fa ${menu.icon}`}
-                />{' '}
-                {menu.label}
-              </a>
-            </Menu.Item>
-          ))}
-        </SubMenu>
-      )}
-      <SubMenu title="Settings" icon={<Icons.TriangleDown iconSize="xl" />}>
-        {settings.map((section, index) => [
-          <Menu.ItemGroup key={`${section.label}`} title={section.label}>
-            {section.childs?.map(child => {
-              if (typeof child !== 'string') {
-                return (
-                  <Menu.Item key={`${child.label}`}>
-                    {isFrontendRoute(child.url) ? (
-                      <Link to={child.url || ''}>{child.label}</Link>
-                    ) : (
-                      <a href={child.url}>{child.label}</a>
-                    )}
-                  </Menu.Item>
-                );
-              }
-              return null;
-            })}
-          </Menu.ItemGroup>,
-          index < settings.length - 1 && <Menu.Divider />,
-        ])}
+}: RightMenuProps) => {
+  const { roles } = useSelector<any, UserWithPermissionsAndRoles>(
+    state => state.user,
+  );

-        {!navbarRight.user_is_anonymous && [
-          <Menu.Divider key="user-divider" />,
-          <Menu.ItemGroup key="user-section" title={t('User')}>
-            {navbarRight.user_profile_url && (
-              <Menu.Item key="profile">
-                <a href={navbarRight.user_profile_url}>{t('Profile')}</a>
-              </Menu.Item>
+  // if user has any of these roles the dropdown will appear
+  const canSql = findPermission('can_sqllab', 'Superset', roles);
+  const canDashboard = findPermission('can_write', 'Dashboard', roles);
+  const canChart = findPermission('can_write', 'Chart', roles);
+  const showActionDropdown = canSql || canChart || canDashboard;
+  return (
+    <StyledDiv align={align}>
+      <Menu mode="horizontal">
+        {!navbarRight.user_is_anonymous && showActionDropdown && (
+          <SubMenu
+            data-test="new-dropdown"
+            title={
+              <StyledI data-test="new-dropdown-icon" className="fa fa-plus" />
+            }
+            icon={<Icons.TriangleDown />}
+          >
+            {dropdownItems.map(
+              menu =>
+                findPermission(menu.perm, menu.view, roles) && (
+                  <Menu.Item key={menu.label}>
+                    <a href={menu.url}>
+                      <i
+                        data-test={`menu-item-${menu.label}`}
+                        className={`fa ${menu.icon}`}
+                      />{' '}
+                      {menu.label}
+                    </a>
+                  </Menu.Item>
+                ),
            )}
-            {navbarRight.user_info_url && (
-              <Menu.Item key="info">
-                <a href={navbarRight.user_info_url}>{t('Info')}</a>
+          </SubMenu>
+        )}
+        <SubMenu title="Settings" icon={<Icons.TriangleDown iconSize="xl" />}>
+          {settings.map((section, index) => [
+            <Menu.ItemGroup key={`${section.label}`} title={section.label}>
+              {section.childs?.map(child => {
+                if (typeof child !== 'string') {
+                  return (
+                    <Menu.Item key={`${child.label}`}>
+                      {isFrontendRoute(child.url) ? (
+                        <Link to={child.url || ''}>{child.label}</Link>
+                      ) : (
+                        <a href={child.url}>{child.label}</a>
+                      )}
+                    </Menu.Item>
+                  );
+                }
+                return null;
+              })}
+            </Menu.ItemGroup>,
+            index < settings.length - 1 && <Menu.Divider />,
+          ])}
+
+          {!navbarRight.user_is_anonymous && [
+            <Menu.Divider key="user-divider" />,
+            <Menu.ItemGroup key="user-section" title={t('User')}>
+              {navbarRight.user_profile_url && (
+                <Menu.Item key="profile">
+                  <a href={navbarRight.user_profile_url}>{t('Profile')}</a>
+                </Menu.Item>
+              )}
+              {navbarRight.user_info_url && (
+                <Menu.Item key="info">
+                  <a href={navbarRight.user_info_url}>{t('Info')}</a>
+                </Menu.Item>
+              )}
+              <Menu.Item key="logout">
+                <a href={navbarRight.user_logout_url}>{t('Logout')}</a>
              </Menu.Item>
-            )}
-            <Menu.Item key="logout">
-              <a href={navbarRight.user_logout_url}>{t('Logout')}</a>
-            </Menu.Item>
-          </Menu.ItemGroup>,
-        ]}
-        {(navbarRight.version_string ||
-          navbarRight.version_sha ||
-          navbarRight.build_number) && [
-          <Menu.Divider key="version-info-divider" />,
-          <Menu.ItemGroup key="about-section" title={t('About')}>
-            <div className="about-section">
-              {navbarRight.show_watermark && (
-                <div css={versionInfoStyles}>
-                  {t('Powered by Apache Superset')}
-                </div>
-              )}
-              {navbarRight.version_string && (
-                <div css={versionInfoStyles}>
-                  Version: {navbarRight.version_string}
-                </div>
-              )}
-              {navbarRight.version_sha && (
-                <div css={versionInfoStyles}>
-                  SHA: {navbarRight.version_sha}
-                </div>
-              )}
-              {navbarRight.build_number && (
-                <div css={versionInfoStyles}>
-                  Build: {navbarRight.build_number}
-                </div>
-              )}
-            </div>
-          </Menu.ItemGroup>,
-        ]}
-      </SubMenu>
-      {navbarRight.show_language_picker && (
-        <LanguagePicker
-          locale={navbarRight.locale}
-          languages={navbarRight.languages}
-        />
+            </Menu.ItemGroup>,
+          ]}
+          {(navbarRight.version_string || navbarRight.version_sha) && [
+            <Menu.Divider key="version-info-divider" />,
+            <Menu.ItemGroup key="about-section" title={t('About')}>
+              <div className="about-section">
+                {navbarRight.show_watermark && (
+                  <div css={versionInfoStyles}>
+                    {t('Powered by Apache Superset')}
+                  </div>
+                )}
+                {navbarRight.version_string && (
+                  <div css={versionInfoStyles}>
+                    Version: {navbarRight.version_string}
+                  </div>
+                )}
+                {navbarRight.version_sha && (
+                  <div css={versionInfoStyles}>
+                    SHA: {navbarRight.version_sha}
+                  </div>
+                )}
+                {navbarRight.build_number && (
+                  <div css={versionInfoStyles}>
+                    Build: {navbarRight.build_number}
+                  </div>
+                )}
+              </div>
+            </Menu.ItemGroup>,
+          ]}
+        </SubMenu>
+        {navbarRight.show_language_picker && (
+          <LanguagePicker
+            locale={navbarRight.locale}
+            languages={navbarRight.languages}
+          />
+        )}
+      </Menu>
+      {navbarRight.documentation_url && (
+        <StyledAnchor
+          href={navbarRight.documentation_url}
+          target="_blank"
+          rel="noreferrer"
+          title={t('Documentation')}
+        >
+          <i className="fa fa-question" />
+          &nbsp;
+        </StyledAnchor>
      )}
-    </Menu>
-    {navbarRight.documentation_url && (
-      <StyledAnchor
-        href={navbarRight.documentation_url}
-        target="_blank"
-        rel="noreferrer"
-        title={t('Documentation')}
-      >
-        <i className="fa fa-question" />
-        &nbsp;
-      </StyledAnchor>
-    )}
-    {navbarRight.bug_report_url && (
-      <StyledAnchor
-        href={navbarRight.bug_report_url}
-        target="_blank"
-        rel="noreferrer"
-        title={t('Report a bug')}
-      >
-        <i className="fa fa-bug" />
-      </StyledAnchor>
-    )}
-    {navbarRight.user_is_anonymous && (
-      <StyledAnchor href={navbarRight.user_login_url}>
-        <i className="fa fa-fw fa-sign-in" />
-        {t('Login')}
-      </StyledAnchor>
-    )}
-  </StyledDiv>
-);
+      {navbarRight.bug_report_url && (
+        <StyledAnchor
+          href={navbarRight.bug_report_url}
+          target="_blank"
+          rel="noreferrer"
+          title={t('Report a bug')}
+        >
+          <i className="fa fa-bug" />
+        </StyledAnchor>
+      )}
+      {navbarRight.user_is_anonymous && (
+        <StyledAnchor href={navbarRight.user_login_url}>
+          <i className="fa fa-fw fa-sign-in" />
+          {t('Login')}
+        </StyledAnchor>
+      )}
+    </StyledDiv>
+  );
+};

 export default RightMenu;
--- a/superset-frontend/src/views/menu.tsx
+++ b/superset-frontend/src/views/menu.tsx
@@ -27,6 +27,9 @@ import { ThemeProvider } from '@superset-ui/core';
 import Menu from 'src/components/Menu/Menu';
 import { theme } from 'src/preamble';

+import { Provider } from 'react-redux';
+import { store } from './store';
+
 const container = document.getElementById('app');
 const bootstrapJson = container?.getAttribute('data-bootstrap') ?? '{}';
 const bootstrap = JSON.parse(bootstrapJson);
@@ -40,7 +43,9 @@ const app = (
  // @ts-ignore: emotion types defs are incompatible between core and cache
  <CacheProvider value={emotionCache}>
    <ThemeProvider theme={theme}>
-      <Menu data={menu} />
+      <Provider store={store}>
+        <Menu data={menu} />
+      </Provider>
    </ThemeProvider>
  </CacheProvider>
 );