fix: CSV Export permission is not consistent (apache#13713)

superset-frontend/spec/javascripts/explore/components/ExploreActionButtons_spec.jsx

@@ -17,14 +17,19 @@
  * under the License.
  */
 import React from 'react';
-import { shallow } from 'enzyme';
+import { shallow, mount } from 'enzyme';
 import { mockStore } from 'spec/fixtures/mockStore';
 import ExploreActionButtons from 'src/explore/components/ExploreActionButtons';
+import * as exploreUtils from 'src/explore/exploreUtils';
+
+import { supersetTheme, ThemeProvider } from '@superset-ui/core';
+import { Provider } from 'react-redux';
+import sinon from 'sinon';
 
 describe('ExploreActionButtons', () => {
   const defaultProps = {
     actions: {},
-    canDownload: 'True',
+    canDownloadCSV: 'True',
     latestQueryFormData: {},
     queryEndpoint: 'localhost',
     chartHeight: '30px',
@@ -42,4 +47,39 @@ describe('ExploreActionButtons', () => {
     );
     expect(wrapper.dive().children()).toHaveLength(6);
   });
+
+  describe('ExploreActionButtons and no permission to download CSV', () => {
+    let wrapper;
+    const defaultProps = {
+      actions: {},
+      canDownloadCSV: false,
+      latestQueryFormData: {},
+      queryEndpoint: 'localhost',
+      chartHeight: '30px',
+    };
+
+    beforeEach(() => {
+      wrapper = mount(
+        <ThemeProvider theme={supersetTheme}>
+          <ExploreActionButtons {...defaultProps} />
+        </ThemeProvider>,
+        {
+          wrappingComponent: Provider,
+          wrappingComponentProps: {
+            store: mockStore,
+          },
+        },
+      );
+    });
+
+    it('should render csv buttons but is disabled and not clickable', () => {
+      const spyExportChart = sinon.spy(exploreUtils, 'exportChart');
+
+      const csvButton = wrapper.find('div.disabled');
+      expect(wrapper).toHaveLength(1);
+      csvButton.simulate('click');
+      expect(spyExportChart.callCount).toBe(0);
+      spyExportChart.restore();
+    });
+  });
 });

superset-frontend/src/explore/components/ExploreActionButtons.tsx

@@ -40,7 +40,7 @@ type ActionButtonProps = {
 
 type ExploreActionButtonsProps = {
   actions: { redirectSQLLab: Function; openPropertiesModal: Function };
-  canDownload: boolean;
+  canDownloadCSV: boolean;
   chartStatus: string;
   latestQueryFormData: {};
   queriesResponse: {};
@@ -83,7 +83,7 @@ const ActionButton = (props: ActionButtonProps) => {
 const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
   const {
     actions,
-    canDownload,
+    canDownloadCSV,
     chartStatus,
     latestQueryFormData,
     queriesResponse,
@@ -118,20 +118,22 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
     }
   };
 
-  const doExportCSV = exportChart.bind(this, {
-    formData: latestQueryFormData,
-    resultType: 'results',
-    resultFormat: 'csv',
-  });
+  const doExportCSV = canDownloadCSV
+    ? exportChart.bind(this, {
+        formData: latestQueryFormData,
+        resultType: 'results',
+        resultFormat: 'csv',
+      })
+    : null;
 
-  const doExportChart = exportChart.bind(this, {
+  const doExportJson = exportChart.bind(this, {
     formData: latestQueryFormData,
     resultType: 'results',
     resultFormat: 'json',
   });
 
   const exportToCSVClasses = cx('btn btn-default btn-sm', {
-    disabled: !canDownload,
+    disabled: !canDownloadCSV,
   });
 
   return (
@@ -175,7 +177,7 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
             icon={<i className="fa fa-file-code-o" />}
             text=".JSON"
             tooltip={t('Export to .JSON format')}
-            onClick={doExportChart}
+            onClick={doExportJson}
           />
           <ActionButton
             icon={<i className="fa fa-file-text-o" />}

superset-frontend/src/explore/components/ExploreChartHeader.jsx

@@ -209,7 +209,7 @@ export class ExploreChartHeader extends React.PureComponent {
               openPropertiesModal: this.openPropertiesModal,
             }}
             slice={this.props.slice}
-            canDownload={this.props.can_download}
+            canDownloadCSV={this.props.can_download}
             chartStatus={chartStatus}
             latestQueryFormData={latestQueryFormData}
             queryResponse={queryResponse}

superset/charts/api.py

@@ -66,7 +66,7 @@
 from superset.commands.importers.v1.utils import get_contents_from_bundle
 from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
 from superset.exceptions import QueryObjectValidationError
-from superset.extensions import event_logger
+from superset.extensions import event_logger, security_manager
 from superset.models.slice import Slice
 from superset.tasks.thumbnails import cache_chart_thumbnail
 from superset.utils.async_query_manager import AsyncQueryTokenException
@@ -491,6 +491,10 @@ def get_data_response(
 
         result_format = result["query_context"].result_format
         if result_format == ChartDataResultFormat.CSV:
+            # Verify user has permission to export CSV file
+            if not security_manager.can_access("can_csv", "Superset"):
+                return self.response_403()
+
             # return the first result
             data = result["queries"][0]["data"]
             return CsvResponse(data, headers=generate_download_headers("csv"))

superset/cli.py

@@ -736,16 +736,23 @@ def load_test_users_run() -> None:
         gamma_sqllab_role = sm.add_role("gamma_sqllab")
         sm.add_permission_role(gamma_sqllab_role, examples_pv)
 
+        gamma_no_csv_role = sm.add_role("gamma_no_csv")
+        sm.add_permission_role(gamma_no_csv_role, examples_pv)
+
         for role in ["Gamma", "sql_lab"]:
             for perm in sm.find_role(role).permissions:
                 sm.add_permission_role(gamma_sqllab_role, perm)
 
+                if str(perm) != "can csv on Superset":
+                    sm.add_permission_role(gamma_no_csv_role, perm)
+
         users = (
             ("admin", "Admin"),
             ("gamma", "Gamma"),
             ("gamma2", "Gamma"),
             ("gamma_sqllab", "gamma_sqllab"),
             ("alpha", "Alpha"),
+            ("gamma_no_csv", "gamma_no_csv"),
         )
         for username, role in users:
             user = sm.find_user(username)

superset/views/core.py

@@ -580,6 +580,15 @@ def explore_json(
                 response_type = response_option
                 break
 
+        # Verify user has permission to export CSV file
+        if (
+            response_type == utils.ChartDataResultFormat.CSV
+            and not security_manager.can_access("can_csv", "Superset")
+        ):
+            return json_error_response(
+                _("You don't have the rights to ") + _("download as csv"), status=403,
+            )
+
         form_data = get_form_data()[0]
 
         try:
@@ -741,7 +750,7 @@ def explore(  # pylint: disable=too-many-locals,too-many-return-statements,too-m
         # slc perms
         slice_add_perm = security_manager.can_access("can_write", "Chart")
         slice_overwrite_perm = is_owner(slc, g.user) if slc else False
-        slice_download_perm = security_manager.can_access("can_read", "Chart")
+        slice_download_perm = security_manager.can_access("can_csv", "Superset")
 
         form_data["datasource"] = str(datasource_id) + "__" + cast(str, datasource_type)
 

tests/base_api_tests.py

@@ -211,13 +211,15 @@ def test_get_filter_related_owners(self):
         rv = self.client.get(uri)
         assert rv.status_code == 200
         response = json.loads(rv.data.decode("utf-8"))
-        assert 3 == response["count"]
+        assert 4 == response["count"]
         sorted_results = sorted(response["result"], key=lambda value: value["text"])
         expected_results = [
             {"text": "gamma user", "value": 2},
             {"text": "gamma2 user", "value": 3},
+            {"text": "gamma_no_csv user", "value": 6},
             {"text": "gamma_sqllab user", "value": 4},
         ]
+        # TODO Check me
         assert expected_results == sorted_results
 
     def test_get_ids_related_owners(self):

tests/charts/api_tests.py

@@ -1183,6 +1183,19 @@ def test_chart_data_csv_result_format(self):
         rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data")
         self.assertEqual(rv.status_code, 200)
 
+    # Test chart csv without permission
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+    def test_chart_data_csv_result_format_permission_denined(self):
+        """
+        Chart data API: Test chart data with CSV result format
+        """
+        self.login(username="gamma_no_csv")
+        request_payload = get_query_context("birth_names")
+        request_payload["result_format"] = "csv"
+
+        rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data")
+        self.assertEqual(rv.status_code, 403)
+
     @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
     def test_chart_data_mixed_case_filter_op(self):
         """