Skip to content

Commit

Permalink
Allow CSV/TSV/Excel download from shared dashboards
Browse files Browse the repository at this point in the history
- Always show dropdown menu
- Append ?api_key for dashboards using the path /public/
- Compare api_key with dashboard API keys associated with the requested query
  • Loading branch information
eradman committed Nov 22, 2024
1 parent baa9bbd commit 7cff63c
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,13 @@ function visualizationWidgetMenuOptions({ widget, canEditDashboard, onParameters
const canEditParameters = canEditDashboard && !isEmpty(invoke(widget, "query.getParametersDefs"));
const widgetQueryResult = widget.getQueryResult();
const isQueryResultEmpty = !widgetQueryResult || !widgetQueryResult.isEmpty || widgetQueryResult.isEmpty();

const downloadLink = fileType => widgetQueryResult.getLink(widget.getQuery().id, fileType);
const downloadName = fileType => widgetQueryResult.getName(widget.getQuery().name, fileType);
const parts = window.location.pathname.split("/").reverse();
var apiKey = null;
if (parts.length > 3 && parts[2] === "public" && parts[0].length === 40) {
apiKey = parts[0];
}
const downloadLink = (fileType) => widgetQueryResult.getLink(widget.getQuery().id, fileType, apiKey);
const downloadName = (fileType) => widgetQueryResult.getName(widget.getQuery().name, fileType);
return compact([
<Menu.Item key="download_csv" disabled={isQueryResultEmpty}>
{!isQueryResultEmpty ? (
Expand Down Expand Up @@ -149,7 +153,7 @@ function VisualizationWidgetFooter({ widget, isPublic, onRefresh, onExpand }) {
const updatedAt = invoke(widgetQueryResult, "getUpdatedAt");
const [refreshClickButtonId, setRefreshClickButtonId] = useState();

const refreshWidget = buttonId => {
const refreshWidget = (buttonId) => {
if (!refreshClickButtonId) {
setRefreshClickButtonId(buttonId);
onRefresh().finally(() => setRefreshClickButtonId(null));
Expand All @@ -163,7 +167,8 @@ function VisualizationWidgetFooter({ widget, isPublic, onRefresh, onExpand }) {
<PlainButton
className="refresh-button hidden-print btn btn-sm btn-default btn-transparent"
onClick={() => refreshWidget(1)}
data-test="RefreshButton">
data-test="RefreshButton"
>
<i className={cx("zmdi zmdi-refresh", { "zmdi-hc-spin": refreshClickButtonId === 1 })} aria-hidden="true" />
<span className="sr-only">
{refreshClickButtonId === 1 ? "Refreshing, please wait. " : "Press to refresh. "}
Expand All @@ -184,7 +189,8 @@ function VisualizationWidgetFooter({ widget, isPublic, onRefresh, onExpand }) {
{!isPublic && (
<PlainButton
className="btn btn-sm btn-default hidden-print btn-transparent btn__refresh"
onClick={() => refreshWidget(2)}>
onClick={() => refreshWidget(2)}
>
<i className={cx("zmdi zmdi-refresh", { "zmdi-hc-spin": refreshClickButtonId === 2 })} aria-hidden="true" />
<span className="sr-only">
{refreshClickButtonId === 2 ? "Refreshing, please wait." : "Press to refresh."}
Expand Down Expand Up @@ -250,7 +256,7 @@ class VisualizationWidget extends React.Component {
onLoad();
}

onLocalFiltersChange = localFilters => {
onLocalFiltersChange = (localFilters) => {
this.setState({ localFilters });
};

Expand All @@ -263,7 +269,7 @@ class VisualizationWidget extends React.Component {
EditParameterMappingsDialog.showModal({
dashboard,
widget,
}).onClose(valuesChanged => {
}).onClose((valuesChanged) => {
// refresh widget if any parameter value has been updated
if (valuesChanged) {
onRefresh();
Expand Down Expand Up @@ -306,7 +312,8 @@ class VisualizationWidget extends React.Component {
className="body-row-auto spinner-container"
role="status"
aria-live="polite"
aria-relevant="additions removals">
aria-relevant="additions removals"
>
<div className="spinner">
<i className="zmdi zmdi-refresh zmdi-hc-spin zmdi-hc-5x" aria-hidden="true" />
<span className="sr-only">Loading...</span>
Expand All @@ -321,7 +328,7 @@ class VisualizationWidget extends React.Component {
const { localParameters } = this.state;
const widgetQueryResult = widget.getQueryResult();
const isRefreshing = isLoading && !!(widgetQueryResult && widgetQueryResult.getStatus());
const onParametersEdit = parameters => {
const onParametersEdit = (parameters) => {
const paramOrder = map(parameters, "name");
widget.options.paramOrder = paramOrder;
widget.save("options", { paramOrder });
Expand Down Expand Up @@ -354,7 +361,8 @@ class VisualizationWidget extends React.Component {
onExpand={this.expandWidget}
/>
}
tileProps={{ "data-refreshing": isRefreshing }}>
tileProps={{ "data-refreshing": isRefreshing }}
>
{this.renderVisualization()}
</Widget>
);
Expand Down
9 changes: 4 additions & 5 deletions client/app/components/dashboards/dashboard-widget/Widget.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ function WidgetDeleteButton({ onClick }) {
title="Remove From Dashboard"
onClick={onClick}
data-test="WidgetDeleteButton"
aria-label="Close">
aria-label="Close"
>
<i className="zmdi zmdi-close" aria-hidden="true" />
</PlainButton>
</div>
Expand All @@ -69,7 +70,6 @@ class Widget extends React.Component {
header: PropTypes.node,
footer: PropTypes.node,
canEdit: PropTypes.bool,
isPublic: PropTypes.bool,
refreshStartedAt: Moment,
menuOptions: PropTypes.node,
tileProps: PropTypes.object, // eslint-disable-line react/forbid-prop-types
Expand All @@ -82,7 +82,6 @@ class Widget extends React.Component {
header: null,
footer: null,
canEdit: false,
isPublic: false,
refreshStartedAt: null,
menuOptions: null,
tileProps: {},
Expand All @@ -109,8 +108,8 @@ class Widget extends React.Component {
};

render() {
const { className, children, header, footer, canEdit, isPublic, menuOptions, tileProps } = this.props;
const showDropdownButton = !isPublic && (canEdit || !isEmpty(menuOptions));
const { className, children, header, footer, canEdit, menuOptions, tileProps } = this.props;
const showDropdownButton = canEdit || !isEmpty(menuOptions);
return (
<div className="widget-wrapper">
<div className={cx("tile body-container", className)} {...tileProps}>
Expand Down
6 changes: 5 additions & 1 deletion redash/handlers/query_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,11 @@ def get(self, query_id=None, query_result_id=None, filetype="json"):
abort(404, message="No cached result found for this query.")

if query_result:
require_access(query_result.data_source, self.current_user, view_only)
api_key = request.args.get("api_key")
if query and api_key in query.dashboard_api_keys:
pass # shared dashboard
else:
require_access(query_result.data_source, self.current_user, view_only)

if isinstance(self.current_user, models.ApiUser):
event = {
Expand Down
2 changes: 1 addition & 1 deletion redash/serializers/query_result.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def _get_column_lists(columns):

def serialize_query_result(query_result, is_api_user):
if is_api_user:
publicly_needed_keys = ["data", "retrieved_at"]
publicly_needed_keys = ["data", "id", "retrieved_at"]
return project(query_result.to_dict(), publicly_needed_keys)
else:
return query_result.to_dict()
Expand Down
30 changes: 30 additions & 0 deletions tests/handlers/test_query_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,36 @@ def test_access_with_query_api_key(self):
)
self.assertEqual(rv.status_code, 200)

def test_access_query_using_dashboard_api_key(self):
query = self.factory.create_query()
query_result = self.factory.create_query_result(query_text=query.query_text)

dashboard1 = self.factory.create_dashboard()
visualization1 = self.factory.create_visualization(query_rel=query)
self.factory.create_widget(visualization=visualization1, dashboard=dashboard1)
api_key = self.factory.create_api_key(object=dashboard1).api_key

rv = self.make_request(
"get",
"/api/queries/{}/results/{}.json?api_key={}".format(query.id, query_result.id, api_key),
user=False,
)
self.assertEqual(rv.status_code, 200)

def test_reject_query_using_unrelated_dashboard_api_key(self):
query = self.factory.create_query()
query_result = self.factory.create_query_result(query_text=query.query_text)

dashboard1 = self.factory.create_dashboard()
api_key = self.factory.create_api_key(object=dashboard1).api_key

rv = self.make_request(
"get",
"/api/queries/{}/results/{}.json?api_key={}".format(query.id, query_result.id, api_key),
user=False,
)
self.assertEqual(rv.status_code, 403)

def test_access_with_query_api_key_without_query_result_id(self):
ds = self.factory.create_data_source(group=self.factory.org.default_group, view_only=False)
query = self.factory.create_query()
Expand Down
2 changes: 1 addition & 1 deletion tests/serializers/test_query_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def test_serializes_all_keys_for_authenticated_users(self):
def test_doesnt_serialize_sensitive_keys_for_unauthenticated_users(self):
query_result = self.factory.create_query_result(data={})
serialized = serialize_query_result(query_result, True)
self.assertSetEqual(set(["data", "retrieved_at"]), set(serialized.keys()))
self.assertSetEqual(set(["data", "id", "retrieved_at"]), set(serialized.keys()))


class DsvSerializationTest(BaseTestCase):
Expand Down

0 comments on commit 7cff63c

Please sign in to comment.