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 15:26:59 UTC

(superset) 02/02: Improve 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 9902ed3bb2d85f86b61d6354dc98220fe5d9c4e2
Author: geido <di...@gmail.com>
AuthorDate: Tue Feb 6 17:26:40 2024 +0200

    Improve permissions
---
 .../Chart/ChartContextMenu/ChartContextMenu.tsx    |  9 ++++-----
 .../Chart/ChartContextMenu/useContextMenu.test.tsx |  6 +++++-
 .../components/SliceHeaderControls/index.tsx       | 17 +++++++---------
 superset/security/manager.py                       |  1 -
 superset/utils/decorators.py                       | 23 ----------------------
 superset/views/datasource/views.py                 |  1 -
 6 files changed, 16 insertions(+), 41 deletions(-)

diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
index 63a5f2b25b..e48fd8293c 100644
--- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
+++ b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
@@ -93,12 +93,11 @@ const ChartContextMenu = (
   const canWriteExploreFormData = 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),
   );
+  const canDrillBy = canExplore && canWriteExploreFormData;
+  const canDrillToDetail = canExplore && canDatasourceSamples;
   const crossFiltersEnabled = useSelector<RootState, boolean>(
     ({ dashboardInfo }) => dashboardInfo.crossFiltersEnabled,
   );
@@ -117,12 +116,12 @@ const ChartContextMenu = (
 
   const showDrillToDetail =
     isFeatureEnabled(FeatureFlag.DrillToDetail) &&
-    ((canExplore && canDatasourceSamples) || canDrillToDetail) &&
+    canDrillToDetail &&
     isDisplayed(ContextMenuItem.DrillToDetail);
 
   const showDrillBy =
     isFeatureEnabled(FeatureFlag.DrillBy) &&
-    (canExplore || canWriteExploreFormData) &&
+    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 a78d35dece..4662bbbe16 100644
--- a/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx
+++ b/superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx
@@ -64,6 +64,7 @@ const setup = ({
           Admin: [
             ['can_explore', 'Superset'],
             ['can_samples', 'Datasource'],
+            ['can_write', 'ExploreFormDataRestAPI'],
           ],
         },
       },
@@ -95,7 +96,10 @@ test('Context menu contains all displayed items only', () => {
 test('Context menu shows "Drill by"', () => {
   const result = setup({
     roles: {
-      Admin: [['can_write', 'ExploreFormDataRestAPI']],
+      Admin: [
+        ['can_write', 'ExploreFormDataRestAPI'],
+        ['can_explore', 'Superset'],
+      ],
     },
   });
   result.current.onContextMenu(0, 0, {});
diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
index 5043ab26a3..7636ca6451 100644
--- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
+++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
@@ -274,12 +274,10 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
       .get(props.slice.viz_type)
       ?.behaviors?.includes(Behavior.InteractiveChart);
   const canExplore = props.supersetCanExplore;
-  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),
   );
+  const canDrillToDetail = canExplore && canDatasourceSamples;
   const canViewQuery = useSelector((state: RootState) =>
     findPermission('can_view_query', 'Dashboard', state.user?.roles),
   );
@@ -491,13 +489,12 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
         </Menu.Item>
       )}
 
-      {isFeatureEnabled(FeatureFlag.DrillToDetail) &&
-        ((canExplore && canDatasourceSamples) || canDrillToDetail) && (
-          <DrillDetailMenuItems
-            chartId={slice.slice_id}
-            formData={props.formData}
-          />
-        )}
+      {isFeatureEnabled(FeatureFlag.DrillToDetail) && canDrillToDetail && (
+        <DrillDetailMenuItems
+          chartId={slice.slice_id}
+          formData={props.formData}
+        />
+      )}
 
       {(slice.description || canExplore) && <Menu.Divider />}
 
diff --git a/superset/security/manager.py b/superset/security/manager.py
index ceaa99f8a2..e6410032ea 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -728,7 +728,6 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
         self.add_permission_view_menu("can_share_chart", "Superset")
         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/superset/utils/decorators.py b/superset/utils/decorators.py
index 74eec9b4df..0a81106f48 100644
--- a/superset/utils/decorators.py
+++ b/superset/utils/decorators.py
@@ -20,7 +20,6 @@ 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
 
@@ -194,25 +193,3 @@ 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 1d67c6e1a1..57c4fc5cc4 100644
--- a/superset/views/datasource/views.py
+++ b/superset/views/datasource/views.py
@@ -190,7 +190,6 @@ class Datasource(BaseSupersetView):
         return self.json_response(external_metadata)
 
     @expose("/samples", methods=("POST",))
-    @has_api_override_permission([("can_drill_to_detail", "Dashboard")])
     @api
     @handle_api_exception
     def samples(self) -> FlaskResponse: