You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2021/05/04 07:21:07 UTC
[superset] branch master updated: fix: CSV Export permission is not
consistent (#13713)
This is an automated email from the ASF dual-hosted git repository.
villebro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 9a22fb0 fix: CSV Export permission is not consistent (#13713)
9a22fb0 is described below
commit 9a22fb00d9a94ab9f7aac41ca8915952ed8a75b6
Author: Duy Nguyen Hoang <nh...@gmail.com>
AuthorDate: Tue May 4 14:19:58 2021 +0700
fix: CSV Export permission is not consistent (#13713)
---
.../components/ExploreActionButtons_spec.jsx | 44 +++++++++++++++++++++-
.../explore/components/ExploreActionButtons.tsx | 22 ++++++-----
.../src/explore/components/ExploreChartHeader.jsx | 2 +-
superset/charts/api.py | 6 ++-
superset/cli.py | 7 ++++
superset/views/core.py | 11 +++++-
tests/base_api_tests.py | 4 +-
tests/charts/api_tests.py | 13 +++++++
8 files changed, 93 insertions(+), 16 deletions(-)
diff --git a/superset-frontend/spec/javascripts/explore/components/ExploreActionButtons_spec.jsx b/superset-frontend/spec/javascripts/explore/components/ExploreActionButtons_spec.jsx
index 5a405b6..6598f3f 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(
+ <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();
+ });
+ });
});
diff --git a/superset-frontend/src/explore/components/ExploreActionButtons.tsx b/superset-frontend/src/explore/components/ExploreActionButtons.tsx
index cb4d2ba..4ce1e41 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={<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" />}
diff --git a/superset-frontend/src/explore/components/ExploreChartHeader.jsx b/superset-frontend/src/explore/components/ExploreChartHeader.jsx
index bf57edc..ca8842b 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 61be4aa..c9430e5 100644
--- a/superset/charts/api.py
+++ b/superset/charts/api.py
@@ -66,7 +66,7 @@ from superset.commands.exceptions import CommandInvalidError
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 @@ class ChartRestApi(BaseSupersetModelRestApi):
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 83ab526..15a8ce9 100755
--- a/superset/cli.py
+++ b/superset/cli.py
@@ -732,16 +732,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 5a04f41..8cbf3df 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -578,6 +578,15 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
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:
@@ -739,7 +748,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
# 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 f23b01e..76084ec 100644
--- a/tests/base_api_tests.py
+++ b/tests/base_api_tests.py
@@ -211,13 +211,15 @@ class ApiOwnersTestCaseMixin:
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 d3b1c97..33927bb 100644
--- a/tests/charts/api_tests.py
+++ b/tests/charts/api_tests.py
@@ -1183,6 +1183,19 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
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):
"""