diff --git a/superset-frontend/spec/javascripts/explore/components/ExploreActionButtons_spec.jsx b/superset-frontend/spec/javascripts/explore/components/ExploreActionButtons_spec.jsx index 5a405b6c5e9c..6598f3f0d423 100644 --- a/superset-frontend/spec/javascripts/explore/components/ExploreActionButtons_spec.jsx +++ b/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( + + + , + { + 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(); + }); + }); }); diff --git a/superset-frontend/src/explore/components/ExploreActionButtons.tsx b/superset-frontend/src/explore/components/ExploreActionButtons.tsx index cb4d2bab6341..4ce1e414e506 100644 --- a/superset-frontend/src/explore/components/ExploreActionButtons.tsx +++ b/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={} text=".JSON" tooltip={t('Export to .JSON format')} - onClick={doExportChart} + onClick={doExportJson} /> } diff --git a/superset-frontend/src/explore/components/ExploreChartHeader.jsx b/superset-frontend/src/explore/components/ExploreChartHeader.jsx index bf57edc89e6c..ca8842b9645b 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader.jsx +++ b/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} diff --git a/superset/charts/api.py b/superset/charts/api.py index 61be4aa9ae95..c9430e51de02 100644 --- a/superset/charts/api.py +++ b/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")) diff --git a/superset/cli.py b/superset/cli.py index a83a389bfa10..4b1259f2d9f1 100755 --- a/superset/cli.py +++ b/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) diff --git a/superset/views/core.py b/superset/views/core.py index dafc73d281c3..4c29cccd99a0 100755 --- a/superset/views/core.py +++ b/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) diff --git a/tests/base_api_tests.py b/tests/base_api_tests.py index f23b01e8ae7f..76084ecbc2ef 100644 --- a/tests/base_api_tests.py +++ b/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): diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py index d3b1c9753d88..33927bbe1e7a 100644 --- a/tests/charts/api_tests.py +++ b/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): """