mirror of https://github.com/apache/superset.git
fix: CSV Export permission is not consistent (#13713)
This commit is contained in:
parent
a75e4af99b
commit
9a22fb00d9
|
@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -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, {
|
||||
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" />}
|
||||
|
|
|
@ -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}
|
||||
|
|
|
@ -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"))
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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):
|
||||
|
|
|
@ -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):
|
||||
"""
|
||||
|
|
Loading…
Reference in New Issue