You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2024/02/06 11:12:07 UTC
(superset) 01/02: Add granular permissions
This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch diego/ch77449/dashboard-granular-permissions
in repository https://gitbox.apache.org/repos/asf/superset.git
commit aa55625e2488bb23d7171ec3612ff491e6d3b3ca
Author: geido <di...@gmail.com>
AuthorDate: Tue Feb 6 13:11:30 2024 +0200
Add granular permissions
---
.../Chart/ChartContextMenu/ChartContextMenu.tsx | 13 ++--
.../Chart/ChartContextMenu/useContextMenu.test.tsx | 34 +++++++---
.../components/Chart/DrillBy/DrillByModal.test.tsx | 2 +-
.../Chart/DrillDetail/DrillDetailModal.test.tsx | 2 +-
.../SliceHeaderControls.test.tsx | 78 ++++++++++++++++++----
.../components/SliceHeaderControls/index.tsx | 23 ++++---
superset/security/manager.py | 4 +-
tests/integration_tests/security_tests.py | 6 +-
8 files changed, 120 insertions(+), 42 deletions(-)
diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
index c03b1a1301..1e7fcf3d7f 100644
--- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
+++ b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
@@ -90,8 +90,11 @@ const ChartContextMenu = (
const canExplore = useSelector((state: RootState) =>
findPermission('can_explore', 'Superset', state.user?.roles),
);
- const canViewDrill = useSelector((state: RootState) =>
- findPermission('can_view_and_drill', 'Dashboard', state.user?.roles),
+ const canDrillBy = useSelector((state: RootState) =>
+ findPermission('can_write', 'ExploreFormDataRestAPI', state.user?.roles),
+ );
+ const canDrillToDetail = useSelector((state: RootState) =>
+ findPermission('can_drill_to_detail', 'Dashboard', state.user?.roles),
);
const canDatasourceSamples = useSelector((state: RootState) =>
findPermission('can_samples', 'Datasource', state.user?.roles),
@@ -111,17 +114,15 @@ const ChartContextMenu = (
}>({ clientX: 0, clientY: 0 });
const menuItems = [];
- const canExploreOrView = canExplore || canViewDrill;
const showDrillToDetail =
isFeatureEnabled(FeatureFlag.DrillToDetail) &&
- canExploreOrView &&
- canDatasourceSamples &&
+ ((canExplore && canDatasourceSamples) || canDrillToDetail) &&
isDisplayed(ContextMenuItem.DrillToDetail);
const showDrillBy =
isFeatureEnabled(FeatureFlag.DrillBy) &&
- canExploreOrView &&
+ (canExplore || canDrillBy) &&
isDisplayed(ContextMenuItem.DrillBy);
const showCrossFilters =
diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx
index 2e7937ffbd..a78d35dece 100644
--- a/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx
+++ b/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx
@@ -92,27 +92,45 @@ test('Context menu contains all displayed items only', () => {
expect(screen.getByText('Drill by')).toBeInTheDocument();
});
-test('Context menu shows all items tied to can_view_and_drill permission', () => {
+test('Context menu shows "Drill by"', () => {
+ const result = setup({
+ roles: {
+ Admin: [['can_write', 'ExploreFormDataRestAPI']],
+ },
+ });
+ result.current.onContextMenu(0, 0, {});
+ expect(screen.getByText('Drill by')).toBeInTheDocument();
+});
+
+test('Context menu does not show "Drill by"', () => {
+ const result = setup({
+ roles: {
+ Admin: [['invalid_permission', 'Dashboard']],
+ },
+ });
+ result.current.onContextMenu(0, 0, {});
+ expect(screen.queryByText('Drill by')).not.toBeInTheDocument();
+});
+
+test('Context menu shows "Drill to detail"', () => {
const result = setup({
roles: {
Admin: [
- ['can_view_and_drill', 'Dashboard'],
['can_samples', 'Datasource'],
+ ['can_explore', 'Superset'],
],
},
});
result.current.onContextMenu(0, 0, {});
expect(screen.getByText('Drill to detail')).toBeInTheDocument();
- expect(screen.getByText('Drill by')).toBeInTheDocument();
- expect(screen.getByText('Add cross-filter')).toBeInTheDocument();
});
-test('Context menu does not show "Drill to detail" without proper permissions', () => {
+test('Context menu does not show "Drill to detail"', () => {
const result = setup({
- roles: { Admin: [['can_view_and_drill', 'Dashboard']] },
+ roles: {
+ Admin: [['can_explore', 'Superset']],
+ },
});
result.current.onContextMenu(0, 0, {});
expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
- expect(screen.getByText('Drill by')).toBeInTheDocument();
- expect(screen.getByText('Add cross-filter')).toBeInTheDocument();
});
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
index fb43fbfcb3..898c033927 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
@@ -246,7 +246,7 @@ test('should render "Edit chart" as disabled without can_explore permission', as
{
user: {
...drillByModalState.user,
- roles: { Admin: [['test_invalid_role', 'Superset']] },
+ roles: { Admin: [['invalid_permission', 'Superset']] },
},
},
);
diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.test.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.test.tsx
index 5b6cc71241..f5185972cc 100644
--- a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.test.tsx
+++ b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.test.tsx
@@ -108,7 +108,7 @@ test('should render "Edit chart" as disabled without can_explore permission', as
await renderModal({
user: {
...drillToDetailModalState.user,
- roles: { Admin: [['test_invalid_role', 'Superset']] },
+ roles: { Admin: [['invalid_permission', 'Superset']] },
},
});
expect(screen.getByRole('button', { name: 'Edit chart' })).toBeDisabled();
diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
index 31a3cad9f6..0970904843 100644
--- a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
+++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
@@ -19,9 +19,9 @@
import userEvent from '@testing-library/user-event';
import React from 'react';
-import { getMockStore } from 'spec/fixtures/mockStore';
import { render, screen } from 'spec/helpers/testing-library';
import { FeatureFlag } from '@superset-ui/core';
+import mockState from 'spec/fixtures/mockState';
import SliceHeaderControls, { SliceHeaderControlsProps } from '.';
jest.mock('src/components/Dropdown', () => {
@@ -104,8 +104,6 @@ const renderWrapper = (
roles?: Record<string, string[][]>,
) => {
const props = overrideProps || createProps();
- const store = getMockStore();
- const mockState = store.getState();
return render(<SliceHeaderControls {...props} />, {
useRedux: true,
useRouter: true,
@@ -113,7 +111,9 @@ const renderWrapper = (
...mockState,
user: {
...mockState.user,
- roles: roles ?? mockState.user.roles,
+ roles: roles ?? {
+ Admin: [['can_samples', 'Datasource']],
+ },
},
},
});
@@ -310,18 +310,18 @@ test('Should show "Drill to detail"', () => {
global.featureFlags = {
[FeatureFlag.DrillToDetail]: true,
};
- const props = createProps();
+ const props = {
+ ...createProps(),
+ supersetCanExplore: true,
+ };
props.slice.slice_id = 18;
renderWrapper(props, {
- Admin: [
- ['can_view_and_drill', 'Dashboard'],
- ['can_samples', 'Datasource'],
- ],
+ Admin: [['can_samples', 'Datasource']],
});
expect(screen.getByText('Drill to detail')).toBeInTheDocument();
});
-test('Should show menu items tied to can_view_and_drill permission', () => {
+test('Should not show "Drill to detail"', () => {
// @ts-ignore
global.featureFlags = {
[FeatureFlag.DrillToDetail]: true,
@@ -332,21 +332,71 @@ test('Should show menu items tied to can_view_and_drill permission', () => {
};
props.slice.slice_id = 18;
renderWrapper(props, {
- Admin: [['can_view_and_drill', 'Dashboard']],
+ Admin: [['invalid_permission', 'Dashboard']],
+ });
+ expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
+});
+
+test('Should show "View query"', () => {
+ const props = {
+ ...createProps(),
+ supersetCanExplore: false,
+ };
+ props.slice.slice_id = 18;
+ renderWrapper(props, {
+ Admin: [['can_view_query', 'Dashboard']],
});
expect(screen.getByText('View query')).toBeInTheDocument();
+});
+
+test('Should not show "View query"', () => {
+ const props = {
+ ...createProps(),
+ supersetCanExplore: false,
+ };
+ props.slice.slice_id = 18;
+ renderWrapper(props, {
+ Admin: [['invalid_permission', 'Dashboard']],
+ });
+ expect(screen.queryByText('View query')).not.toBeInTheDocument();
+});
+
+test('Should show "View as table"', () => {
+ const props = {
+ ...createProps(),
+ supersetCanExplore: false,
+ };
+ props.slice.slice_id = 18;
+ renderWrapper(props, {
+ Admin: [['can_view_table', 'Dashboard']],
+ });
expect(screen.getByText('View as table')).toBeInTheDocument();
- expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
});
-test('Should not show the "Edit chart" without proper permissions', () => {
+test('Should not show "View as table"', () => {
const props = {
...createProps(),
supersetCanExplore: false,
};
props.slice.slice_id = 18;
renderWrapper(props, {
- Admin: [['can_view_and_drill', 'Dashboard']],
+ Admin: [['invalid_permission', 'Dashboard']],
+ });
+ expect(screen.queryByText('View as table')).not.toBeInTheDocument();
+});
+
+test('Should not show the "Edit chart" button', () => {
+ const props = {
+ ...createProps(),
+ supersetCanExplore: false,
+ };
+ props.slice.slice_id = 18;
+ renderWrapper(props, {
+ Admin: [
+ ['can_samples', 'Datasource'],
+ ['can_view_query', 'Dashboard'],
+ ['can_view_table', 'Dashboard'],
+ ],
});
expect(screen.queryByText('Edit chart')).not.toBeInTheDocument();
});
diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
index d4b9a1a0b9..5043ab26a3 100644
--- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
+++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
@@ -273,13 +273,19 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
getChartMetadataRegistry()
.get(props.slice.viz_type)
?.behaviors?.includes(Behavior.InteractiveChart);
- const canViewDrill = useSelector((state: RootState) =>
- findPermission('can_view_and_drill', 'Dashboard', state.user?.roles),
+ const canExplore = props.supersetCanExplore;
+ const canDrillToDetail = useSelector((state: RootState) =>
+ findPermission('can_drill_to_detail', 'Dashboard', state.user?.roles),
);
- const canExploreOrView = props.supersetCanExplore || canViewDrill;
const canDatasourceSamples = useSelector((state: RootState) =>
findPermission('can_samples', 'Datasource', state.user?.roles),
);
+ const canViewQuery = useSelector((state: RootState) =>
+ findPermission('can_view_query', 'Dashboard', state.user?.roles),
+ );
+ const canViewTable = useSelector((state: RootState) =>
+ findPermission('can_view_table', 'Dashboard', state.user?.roles),
+ );
const refreshChart = () => {
if (props.updatedDttm) {
props.forceRefresh(props.slice.slice_id, props.dashboardId);
@@ -429,7 +435,7 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
</Menu.Item>
)}
- {props.supersetCanExplore && (
+ {canExplore && (
<Menu.Item key={MENU_KEYS.EXPLORE_CHART}>
<Link to={props.exploreUrl}>
<Tooltip title={getSliceHeaderTooltip(props.slice.slice_name)}>
@@ -448,7 +454,7 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
</>
)}
- {canExploreOrView && (
+ {(canExplore || canViewQuery) && (
<Menu.Item key={MENU_KEYS.VIEW_QUERY}>
<ModalTrigger
triggerNode={
@@ -463,7 +469,7 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
</Menu.Item>
)}
- {canExploreOrView && (
+ {(canExplore || canViewTable) && (
<Menu.Item key={MENU_KEYS.VIEW_RESULTS}>
<ViewResultsModalTrigger
canExplore={props.supersetCanExplore}
@@ -486,15 +492,14 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
)}
{isFeatureEnabled(FeatureFlag.DrillToDetail) &&
- canExploreOrView &&
- canDatasourceSamples && (
+ ((canExplore && canDatasourceSamples) || canDrillToDetail) && (
<DrillDetailMenuItems
chartId={slice.slice_id}
formData={props.formData}
/>
)}
- {(slice.description || canExploreOrView) && <Menu.Divider />}
+ {(slice.description || canExplore) && <Menu.Divider />}
{supersetCanShare && (
<Menu.SubMenu title={t('Share')}>
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 8268b19411..ceaa99f8a2 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -726,7 +726,9 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
self.add_permission_view_menu("can_csv", "Superset")
self.add_permission_view_menu("can_share_dashboard", "Superset")
self.add_permission_view_menu("can_share_chart", "Superset")
- self.add_permission_view_menu("can_view_and_drill", "Dashboard")
+ self.add_permission_view_menu("can_view_query", "Dashboard")
+ self.add_permission_view_menu("can_view_table", "Dashboard")
+ self.add_permission_view_menu("can_drill_to_detail", "Dashboard")
def create_missing_perms(self) -> None:
"""
diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py
index d9c98aa253..fb64f06330 100644
--- a/tests/integration_tests/security_tests.py
+++ b/tests/integration_tests/security_tests.py
@@ -1337,7 +1337,8 @@ class TestRolePermission(SupersetTestCase):
self.assertIn(("can_explore_json", "Superset"), perm_set)
self.assertIn(("can_explore_json", "Superset"), perm_set)
self.assertIn(("can_userinfo", "UserDBModelView"), perm_set)
- self.assertIn(("can_view_and_drill", "Dashboard"), perm_set)
+ self.assertIn(("can_view_table", "Dashboard"), perm_set)
+ self.assertIn(("can_view_query", "Dashboard"), perm_set)
self.assert_can_menu("Databases", perm_set)
self.assert_can_menu("Datasets", perm_set)
self.assert_can_menu("Data", perm_set)
@@ -1505,7 +1506,8 @@ class TestRolePermission(SupersetTestCase):
self.assertIn(("can_share_dashboard", "Superset"), gamma_perm_set)
self.assertIn(("can_explore_json", "Superset"), gamma_perm_set)
self.assertIn(("can_userinfo", "UserDBModelView"), gamma_perm_set)
- self.assertIn(("can_view_and_drill", "Dashboard"), gamma_perm_set)
+ self.assertIn(("can_view_table", "Dashboard"), gamma_perm_set)
+ self.assertIn(("can_view_query", "Dashboard"), gamma_perm_set)
def test_views_are_secured(self):
"""Preventing the addition of unsecured views without has_access decorator"""