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:06 UTC
(superset) branch diego/ch77449/dashboard-granular-permissions created (now 1cd12c6d44)
This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a change to branch diego/ch77449/dashboard-granular-permissions
in repository https://gitbox.apache.org/repos/asf/superset.git
at 1cd12c6d44 Override API permissions POC
This branch includes the following new commits:
new aa55625e24 Add granular permissions
new 1cd12c6d44 Override API permissions POC
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 02/02: Override API permissions POC
Posted by di...@apache.org.
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 1cd12c6d443e16ca2360397ca15e7ef3f1d55a8b
Author: geido <di...@gmail.com>
AuthorDate: Tue Feb 6 13:11:49 2024 +0200
Override API permissions POC
---
superset/utils/decorators.py | 25 +++++++++++++++++++++++++
superset/views/datasource/views.py | 3 ++-
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py
index 43b6909caf..74eec9b4df 100644
--- a/superset/utils/decorators.py
+++ b/superset/utils/decorators.py
@@ -20,11 +20,14 @@ import logging
import time
from collections.abc import Iterator
from contextlib import contextmanager
+from functools import wraps
from typing import Any, Callable, TYPE_CHECKING
from uuid import UUID
from flask import current_app, g, Response
+from flask_appbuilder.security.decorators import has_access_api
+from superset import security_manager
from superset.utils import core as utils
from superset.utils.dates import now_as_float
@@ -191,3 +194,25 @@ def debounce(duration: float | int = 0.1) -> Callable[..., Any]:
def on_security_exception(self: Any, ex: Exception) -> Response:
return self.response(403, **{"message": utils.error_msg_from_exception(ex)})
+
+
+def has_api_override_permission(permissions_targets: list[tuple[str, str]]) -> Callable:
+ """
+ Decorator to check for multiple override permissions, each tied to a specific target.
+ :param permissions_targets: A list of tuples where each tuple contains [permission, target_view].
+ """
+
+ def decorator(f: Callable) -> Callable:
+ @wraps(f)
+ def decorated_function(*args, **kwargs) -> Callable:
+ # Iterate through the list of permission-target pairs
+ for permission, target_view in permissions_targets:
+ # Check for each custom override permission with its specific target
+ if security_manager.can_access(permission, target_view):
+ return f(*args, **kwargs)
+ # Fallback to the standard has_access_api decorator if none of the override permissions are present
+ return has_access_api(f)(*args, **kwargs)
+
+ return decorated_function
+
+ return decorator
diff --git a/superset/views/datasource/views.py b/superset/views/datasource/views.py
index 2e46faf0af..1d67c6e1a1 100644
--- a/superset/views/datasource/views.py
+++ b/superset/views/datasource/views.py
@@ -39,6 +39,7 @@ from superset.exceptions import SupersetException, SupersetSecurityException
from superset.models.core import Database
from superset.superset_typing import FlaskResponse
from superset.utils.core import DatasourceType
+from superset.utils.decorators import has_api_override_permission
from superset.views.base import (
api,
BaseSupersetView,
@@ -189,7 +190,7 @@ class Datasource(BaseSupersetView):
return self.json_response(external_metadata)
@expose("/samples", methods=("POST",))
- @has_access_api
+ @has_api_override_permission([("can_drill_to_detail", "Dashboard")])
@api
@handle_api_exception
def samples(self) -> FlaskResponse:
(superset) 01/02: Add granular permissions
Posted by di...@apache.org.
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"""