chore: Add permission to view and drill on Dashboard context (#26798)

superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx

@@ -90,6 +90,12 @@ 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 canDatasourceSamples = useSelector((state: RootState) =>
+    findPermission('can_samples', 'Datasource', state.user?.roles),
+  );
   const crossFiltersEnabled = useSelector<RootState, boolean>(
     ({ dashboardInfo }) => dashboardInfo.crossFiltersEnabled,
   );
@@ -105,15 +111,17 @@ const ChartContextMenu = (
   }>({ clientX: 0, clientY: 0 });
 
   const menuItems = [];
+  const canExploreOrView = canExplore || canViewDrill;
 
   const showDrillToDetail =
     isFeatureEnabled(FeatureFlag.DRILL_TO_DETAIL) &&
-    canExplore &&
+    canExploreOrView &&
+    canDatasourceSamples &&
     isDisplayed(ContextMenuItem.DrillToDetail);
 
   const showDrillBy =
     isFeatureEnabled(FeatureFlag.DRILL_BY) &&
-    canExplore &&
+    canExploreOrView &&
     isDisplayed(ContextMenuItem.DrillBy);
 
   const showCrossFilters =

superset-frontend/src/components/Chart/ChartContextMenu/useContextMenu.test.tsx

@@ -38,10 +38,12 @@ const setup = ({
   onSelection = noOp,
   displayedItems = ContextMenuItem.All,
   additionalConfig = {},
+  roles = undefined,
 }: {
   onSelection?: () => void;
   displayedItems?: ContextMenuItem | ContextMenuItem[];
   additionalConfig?: Record<string, any>;
+  roles?: Record<string, string[][]>;
 } = {}) => {
   const { result } = renderHook(() =>
     useContextMenu(
@@ -58,7 +60,12 @@ const setup = ({
       ...mockState,
       user: {
         ...mockState.user,
-        roles: { Admin: [['can_explore', 'Superset']] },
+        roles: roles ?? {
+          Admin: [
+            ['can_explore', 'Superset'],
+            ['can_samples', 'Datasource'],
+          ],
+        },
       },
     },
   });
@@ -75,7 +82,7 @@ test('Context menu renders', () => {
   expect(screen.getByText('Drill by')).toBeInTheDocument();
 });
 
-test('Context menu contains all items only', () => {
+test('Context menu contains all displayed items only', () => {
   const result = setup({
     displayedItems: [ContextMenuItem.DrillToDetail, ContextMenuItem.DrillBy],
   });
@@ -84,3 +91,28 @@ test('Context menu contains all items only', () => {
   expect(screen.getByText('Drill to detail')).toBeInTheDocument();
   expect(screen.getByText('Drill by')).toBeInTheDocument();
 });
+
+test('Context menu shows all items tied to can_view_and_drill permission', () => {
+  const result = setup({
+    roles: {
+      Admin: [
+        ['can_view_and_drill', 'Dashboard'],
+        ['can_samples', 'Datasource'],
+      ],
+    },
+  });
+  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', () => {
+  const result = setup({
+    roles: { Admin: [['can_view_and_drill', 'Dashboard']] },
+  });
+  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();
+});

superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx

@@ -68,7 +68,10 @@ const dataset = {
   ],
 };
 
-const renderModal = async (modalProps: Partial<DrillByModalProps> = {}) => {
+const renderModal = async (
+  modalProps: Partial<DrillByModalProps> = {},
+  overrideState: Record<string, any> = {},
+) => {
   const DrillByModalWrapper = () => {
     const [showModal, setShowModal] = useState(false);
 
@@ -93,7 +96,10 @@ const renderModal = async (modalProps: Partial<DrillByModalProps> = {}) => {
     useDnd: true,
     useRedux: true,
     useRouter: true,
-    initialState: drillByModalState,
+    initialState: {
+      ...drillByModalState,
+      ...overrideState,
+    },
   });
 
   userEvent.click(screen.getByRole('button', { name: 'Show modal' }));
@@ -233,3 +239,29 @@ test('render breadcrumbs', async () => {
   expect(newBreadcrumbItems).toHaveLength(1);
   expect(within(breadcrumbItems[0]).getByText('gender')).toBeInTheDocument();
 });
+
+test('should render "Edit chart" as disabled without can_explore permission', async () => {
+  await renderModal(
+    {},
+    {
+      user: {
+        ...drillByModalState.user,
+        roles: { Admin: [['test_invalid_role', 'Superset']] },
+      },
+    },
+  );
+  expect(screen.getByRole('button', { name: 'Edit chart' })).toBeDisabled();
+});
+
+test('should render "Edit chart" enabled with can_explore permission', async () => {
+  await renderModal(
+    {},
+    {
+      user: {
+        ...drillByModalState.user,
+        roles: { Admin: [['can_explore', 'Superset']] },
+      },
+    },
+  );
+  expect(screen.getByRole('button', { name: 'Edit chart' })).toBeEnabled();
+});

superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx

@@ -54,6 +54,7 @@ import {
   LOG_ACTIONS_DRILL_BY_MODAL_OPENED,
   LOG_ACTIONS_FURTHER_DRILL_BY,
 } from 'src/logger/LogUtils';
+import { findPermission } from 'src/utils/findPermission';
 import { Dataset, DrillByType } from '../types';
 import DrillByChart from './DrillByChart';
 import { ContextMenuItem } from '../ChartContextMenu/ChartContextMenu';
@@ -75,6 +76,7 @@ interface ModalFooterProps {
 const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
   const dispatch = useDispatch();
   const { addDangerToast } = useToasts();
+  const theme = useTheme();
   const [url, setUrl] = useState('');
   const dashboardPageId = useContext(DashboardPageIdContext);
   const onEditChartClick = useCallback(() => {
@@ -84,6 +86,9 @@ const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
       }),
     );
   }, [dispatch, formData.slice_id]);
+  const canExplore = useSelector((state: RootState) =>
+    findPermission('can_explore', 'Superset', state.user?.roles),
+  );
 
   const [datasource_id, datasource_type] = formData.datasource.split('__');
   useEffect(() => {
@@ -103,13 +108,20 @@ const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
     datasource_type,
     formData,
   ]);
+  const isEditDisabled = !url || !canExplore;
+
   return (
     <>
       <Button
         buttonStyle="secondary"
         buttonSize="small"
         onClick={onEditChartClick}
-        disabled={!url}
+        disabled={isEditDisabled}
+        tooltip={
+          isEditDisabled
+            ? t('You do not have sufficient permissions to edit the chart')
+            : undefined
+        }
       >
         <Link
           css={css`
@@ -128,6 +140,9 @@ const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
         buttonSize="small"
         onClick={closeModal}
         data-test="close-drill-by-modal"
+        css={css`
+          margin-left: ${theme.gridUnit * 2}px;
+        `}
       >
         {t('Close')}
       </Button>

superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.test.tsx

@@ -35,9 +35,16 @@ jest.mock('react-router-dom', () => ({
 
 const { id: chartId, form_data: formData } = chartQueries[sliceId];
 const { slice_name: chartName } = formData;
+const store = getMockStoreWithNativeFilters();
+const drillToDetailModalState = {
+  ...store.getState(),
+  user: {
+    ...store.getState().user,
+    roles: { Admin: [['can_explore', 'Superset']] },
+  },
+};
 
-const renderModal = async () => {
-  const store = getMockStoreWithNativeFilters();
+const renderModal = async (overrideState: Record<string, any> = {}) => {
   const DrillDetailModalWrapper = () => {
     const [showModal, setShowModal] = useState(false);
     return (
@@ -59,7 +66,10 @@ const renderModal = async () => {
   render(<DrillDetailModalWrapper />, {
     useRouter: true,
     useRedux: true,
-    store,
+    initialState: {
+      ...drillToDetailModalState,
+      ...overrideState,
+    },
   });
 
   userEvent.click(screen.getByRole('button', { name: 'Show modal' }));
@@ -93,3 +103,13 @@ test('should forward to Explore', async () => {
     `/explore/?dashboard_page_id=&slice_id=${sliceId}`,
   );
 });
+
+test('should render "Edit chart" as disabled without can_explore permission', async () => {
+  await renderModal({
+    user: {
+      ...drillToDetailModalState.user,
+      roles: { Admin: [['test_invalid_role', 'Superset']] },
+    },
+  });
+  expect(screen.getByRole('button', { name: 'Edit chart' })).toBeDisabled();
+});

superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx

@@ -31,28 +31,52 @@ import Button from 'src/components/Button';
 import { useSelector } from 'react-redux';
 import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage';
 import { Slice } from 'src/types/Chart';
+import { RootState } from 'src/dashboard/types';
+import { findPermission } from 'src/utils/findPermission';
 import DrillDetailPane from './DrillDetailPane';
 
 interface ModalFooterProps {
-  exploreChart: () => void;
+  canExplore: boolean;
   closeModal?: () => void;
+  exploreChart: () => void;
 }
 
-const ModalFooter = ({ exploreChart, closeModal }: ModalFooterProps) => (
-  <>
-    <Button buttonStyle="secondary" buttonSize="small" onClick={exploreChart}>
-      {t('Edit chart')}
-    </Button>
-    <Button
-      buttonStyle="primary"
-      buttonSize="small"
-      onClick={closeModal}
-      data-test="close-drilltodetail-modal"
-    >
-      {t('Close')}
-    </Button>
-  </>
-);
+const ModalFooter = ({
+  canExplore,
+  closeModal,
+  exploreChart,
+}: ModalFooterProps) => {
+  const theme = useTheme();
+
+  return (
+    <>
+      <Button
+        buttonStyle="secondary"
+        buttonSize="small"
+        onClick={exploreChart}
+        disabled={!canExplore}
+        tooltip={
+          !canExplore
+            ? t('You do not have sufficient permissions to edit the chart')
+            : undefined
+        }
+      >
+        {t('Edit chart')}
+      </Button>
+      <Button
+        buttonStyle="primary"
+        buttonSize="small"
+        onClick={closeModal}
+        data-test="close-drilltodetail-modal"
+        css={css`
+          margin-left: ${theme.gridUnit * 2}px;
+        `}
+      >
+        {t('Close')}
+      </Button>
+    </>
+  );
+};
 
 interface DrillDetailModalProps {
   chartId: number;
@@ -76,6 +100,9 @@ export default function DrillDetailModal({
     (state: { sliceEntities: { slices: Record<number, Slice> } }) =>
       state.sliceEntities.slices[chartId],
   );
+  const canExplore = useSelector((state: RootState) =>
+    findPermission('can_explore', 'Superset', state.user?.roles),
+  );
 
   const exploreUrl = useMemo(
     () => `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${chartId}`,
@@ -97,7 +124,9 @@ export default function DrillDetailModal({
         }
       `}
       title={t('Drill to detail: %s', chartName)}
-      footer={<ModalFooter exploreChart={exploreChart} />}
+      footer={
+        <ModalFooter exploreChart={exploreChart} canExplore={canExplore} />
+      }
       responsive
       resizable
       resizableConfig={{

superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx

@@ -99,13 +99,23 @@ const createProps = (viz_type = 'sunburst_v2') =>
     exploreUrl: '/explore',
   } as SliceHeaderControlsProps);
 
-const renderWrapper = (overrideProps?: SliceHeaderControlsProps) => {
+const renderWrapper = (
+  overrideProps?: SliceHeaderControlsProps,
+  roles?: Record<string, string[][]>,
+) => {
   const props = overrideProps || createProps();
   const store = getMockStore();
+  const mockState = store.getState();
   return render(<SliceHeaderControls {...props} />, {
     useRedux: true,
     useRouter: true,
-    store,
+    initialState: {
+      ...mockState,
+      user: {
+        ...mockState.user,
+        roles: roles ?? mockState.user.roles,
+      },
+    },
   });
 };
 
@@ -295,13 +305,48 @@ test('Drill to detail modal is under featureflag', () => {
   expect(screen.queryByText('Drill to detail')).not.toBeInTheDocument();
 });
 
-test('Should show the "Drill to detail"', () => {
+test('Should show "Drill to detail"', () => {
   // @ts-ignore
   global.featureFlags = {
     [FeatureFlag.DRILL_TO_DETAIL]: true,
   };
   const props = createProps();
   props.slice.slice_id = 18;
-  renderWrapper(props);
+  renderWrapper(props, {
+    Admin: [
+      ['can_view_and_drill', 'Dashboard'],
+      ['can_samples', 'Datasource'],
+    ],
+  });
   expect(screen.getByText('Drill to detail')).toBeInTheDocument();
 });
+
+test('Should show menu items tied to can_view_and_drill permission', () => {
+  // @ts-ignore
+  global.featureFlags = {
+    [FeatureFlag.DRILL_TO_DETAIL]: true,
+  };
+  const props = {
+    ...createProps(),
+    supersetCanExplore: false,
+  };
+  props.slice.slice_id = 18;
+  renderWrapper(props, {
+    Admin: [['can_view_and_drill', 'Dashboard']],
+  });
+  expect(screen.getByText('View query')).toBeInTheDocument();
+  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', () => {
+  const props = {
+    ...createProps(),
+    supersetCanExplore: false,
+  };
+  props.slice.slice_id = 18;
+  renderWrapper(props, {
+    Admin: [['can_view_and_drill', 'Dashboard']],
+  });
+  expect(screen.queryByText('Edit chart')).not.toBeInTheDocument();
+});

superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx

@@ -57,6 +57,7 @@ import Modal from 'src/components/Modal';
 import { DrillDetailMenuItems } from 'src/components/Chart/DrillDetail';
 import { LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE } from 'src/logger/LogUtils';
 import { RootState } from 'src/dashboard/types';
+import { findPermission } from 'src/utils/findPermission';
 import { useCrossFiltersScopingModal } from '../nativeFilters/FilterBar/CrossFilters/ScopingModal/useCrossFiltersScopingModal';
 
 const MENU_KEYS = {
@@ -170,11 +171,13 @@ const dropdownIconsStyles = css`
 `;
 
 const ViewResultsModalTrigger = ({
+  canExplore,
   exploreUrl,
   triggerNode,
   modalTitle,
   modalBody,
 }: {
+  canExplore?: boolean;
   exploreUrl: string;
   triggerNode: ReactChild;
   modalTitle: ReactChild;
@@ -214,6 +217,14 @@ const ViewResultsModalTrigger = ({
                 buttonStyle="secondary"
                 buttonSize="small"
                 onClick={exploreChart}
+                disabled={!canExplore}
+                tooltip={
+                  !canExplore
+                    ? t(
+                        'You do not have sufficient permissions to edit the chart',
+                      )
+                    : undefined
+                }
               >
                 {t('Edit chart')}
               </Button>
@@ -221,6 +232,9 @@ const ViewResultsModalTrigger = ({
                 buttonStyle="primary"
                 buttonSize="small"
                 onClick={closeModal}
+                css={css`
+                  margin-left: ${theme.gridUnit * 2}px;
+                `}
               >
                 {t('Close')}
               </Button>
@@ -259,7 +273,13 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
     getChartMetadataRegistry()
       .get(props.slice.viz_type)
       ?.behaviors?.includes(Behavior.INTERACTIVE_CHART);
-
+  const canViewDrill = useSelector((state: RootState) =>
+    findPermission('can_view_and_drill', 'Dashboard', state.user?.roles),
+  );
+  const canExploreOrView = props.supersetCanExplore || canViewDrill;
+  const canDatasourceSamples = useSelector((state: RootState) =>
+    findPermission('can_samples', 'Datasource', state.user?.roles),
+  );
   const refreshChart = () => {
     if (props.updatedDttm) {
       props.forceRefresh(props.slice.slice_id, props.dashboardId);
@@ -428,7 +448,7 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
         </>
       )}
 
-      {props.supersetCanExplore && (
+      {canExploreOrView && (
         <Menu.Item key={MENU_KEYS.VIEW_QUERY}>
           <ModalTrigger
             triggerNode={
@@ -443,9 +463,10 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
         </Menu.Item>
       )}
 
-      {props.supersetCanExplore && (
+      {canExploreOrView && (
         <Menu.Item key={MENU_KEYS.VIEW_RESULTS}>
           <ViewResultsModalTrigger
+            canExplore={props.supersetCanExplore}
             exploreUrl={props.exploreUrl}
             triggerNode={
               <span data-test="view-query-menu-item">{t('View as table')}</span>
@@ -465,14 +486,15 @@ const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => {
       )}
 
       {isFeatureEnabled(FeatureFlag.DRILL_TO_DETAIL) &&
-        props.supersetCanExplore && (
+        canExploreOrView &&
+        canDatasourceSamples && (
           <DrillDetailMenuItems
             chartId={slice.slice_id}
             formData={props.formData}
           />
         )}
 
-      {(slice.description || props.supersetCanExplore) && <Menu.Divider />}
+      {(slice.description || canExploreOrView) && <Menu.Divider />}
 
       {supersetCanShare && (
         <Menu.SubMenu title={t('Share')}>

superset/security/manager.py

@@ -726,6 +726,7 @@ 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")
 
     def create_missing_perms(self) -> None:
         """

tests/integration_tests/security_tests.py

@@ -1337,6 +1337,7 @@ 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.assert_can_menu("Databases", perm_set)
         self.assert_can_menu("Datasets", perm_set)
         self.assert_can_menu("Data", perm_set)
@@ -1504,6 +1505,7 @@ 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)
 
     def test_views_are_secured(self):
         """Preventing the addition of unsecured views without has_access decorator"""