This is an automated email from the ASF dual-hosted git repository.
hugh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 87baac7 fix: RBAC hide right menu (#16902)
87baac7 is described below
commit 87baac7650bdb6a8f8f82cf4992567a2c5c73cba
Author: Hugh A. Miles II <[email protected]>
AuthorDate: Thu Sep 30 14:33:54 2021 -0400
fix: RBAC hide right menu (#16902)
* add provider to right menu
* add roles code
* bad pull
---
.../src/components/Menu/Menu.test.tsx | 52 ++++
.../src/components/Menu/MenuRight.tsx | 269 +++++++++++----------
superset-frontend/src/views/menu.tsx | 7 +-
3 files changed, 203 insertions(+), 125 deletions(-)
diff --git a/superset-frontend/src/components/Menu/Menu.test.tsx
b/superset-frontend/src/components/Menu/Menu.test.tsx
index f3a2fd1..8dd8f56 100644
--- 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();
+});
diff --git a/superset-frontend/src/components/Menu/MenuRight.tsx
b/superset-frontend/src/components/Menu/MenuRight.tsx
index 1626713..348bc46 100644
--- 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>
- )}
- {navbarRight.user_info_url && (
- <Menu.Item key="info">
- <a href={navbarRight.user_info_url}>{t('Info')}</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>
+ ),
)}
- <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>
+ </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.build_number && (
- <div css={versionInfoStyles}>
- Build: {navbarRight.build_number}
- </div>
+ {navbarRight.user_info_url && (
+ <Menu.Item key="info">
+ <a href={navbarRight.user_info_url}>{t('Info')}</a>
+ </Menu.Item>
)}
- </div>
- </Menu.ItemGroup>,
- ]}
- </SubMenu>
- {navbarRight.show_language_picker && (
- <LanguagePicker
- locale={navbarRight.locale}
- languages={navbarRight.languages}
- />
+ <Menu.Item key="logout">
+ <a href={navbarRight.user_logout_url}>{t('Logout')}</a>
+ </Menu.Item>
+ </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" />
+
+ </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>
)}
- </Menu>
- {navbarRight.documentation_url && (
- <StyledAnchor
- href={navbarRight.documentation_url}
- target="_blank"
- rel="noreferrer"
- title={t('Documentation')}
- >
- <i className="fa fa-question" />
-
- </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>
-);
+ </StyledDiv>
+ );
+};
export default RightMenu;
diff --git a/superset-frontend/src/views/menu.tsx
b/superset-frontend/src/views/menu.tsx
index 35f6252..68b72d7 100644
--- 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>
);
