You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2024/10/01 13:58:12 UTC
(superset) 01/01: fix(embedded): sankey charts
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch sankey-embedded
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 561509f7cb62905d611717ea9a00f714ccbf6a4f
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Tue Oct 1 09:57:57 2024 -0400
fix(embedded): sankey charts
---
superset/security/manager.py | 13 ++-
tests/unit_tests/security/manager_test.py | 146 ++++++++++++++++++++++++++++++
2 files changed, 155 insertions(+), 4 deletions(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 27484d6a8c..9fd84ed58f 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -177,7 +177,11 @@ def query_context_modified(query_context: "QueryContext") -> bool:
)
# compare columns and metrics in form_data with stored values
- for key in ["metrics", "columns", "groupby"]:
+ for key, equivalent in [
+ ("metrics", ["metrics"]),
+ ("columns", ["columns", "groupby"]),
+ ("groupby", ["columns", "groupby"]),
+ ]:
requested_values = {freeze_value(value) for value in form_data.get(key) or []}
stored_values = {
freeze_value(value) for value in stored_chart.params_dict.get(key) or []
@@ -193,9 +197,10 @@ def query_context_modified(query_context: "QueryContext") -> bool:
}
if stored_query_context:
for query in stored_query_context.get("queries") or []:
- stored_values.update(
- {freeze_value(value) for value in query.get(key) or []}
- )
+ for key in equivalent:
+ stored_values.update(
+ {freeze_value(value) for value in query.get(key) or []}
+ )
if not queries_values.issubset(stored_values):
return True
diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py
index 660a2023e6..f163b114f3 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -17,6 +17,8 @@
# pylint: disable=invalid-name, unused-argument, redefined-outer-name
+import json
+
import pytest
from flask_appbuilder.security.sqla.models import Role, User
from pytest_mock import MockerFixture
@@ -691,6 +693,150 @@ def test_query_context_modified_mixed_chart(mocker: MockerFixture) -> None:
assert not query_context_modified(query_context)
+def test_query_context_modified_sankey_tampered(mocker: MockerFixture) -> None:
+ """
+ Test the `query_context_modified` function for a sankey chart request.
+ """
+ query_context = mocker.MagicMock()
+ query_context.queries = [
+ {
+ "apply_fetch_values_predicate": False,
+ "columns": ["bot_id", "channel_id"],
+ "extras": {"having": "", "where": ""},
+ "filter": [
+ {
+ "col": "bot_profile__updated",
+ "op": "TEMPORAL_RANGE",
+ "val": "No filter",
+ }
+ ],
+ "from_dttm": None,
+ "granularity": None,
+ "inner_from_dttm": None,
+ "inner_to_dttm": None,
+ "is_rowcount": False,
+ "is_timeseries": False,
+ "metrics": ["count"],
+ "order_desc": True,
+ "orderby": [],
+ "row_limit": 10000,
+ "row_offset": 0,
+ "series_columns": [],
+ "series_limit": 0,
+ "series_limit_metric": None,
+ "time_shift": None,
+ "to_dttm": None,
+ }
+ ]
+ query_context.form_data = {
+ "datasource": "12__table",
+ "viz_type": "sankey_v2",
+ "slice_id": 97,
+ "url_params": {},
+ "source": "bot_id",
+ "target": "channel_id",
+ "metric": "count",
+ "adhoc_filters": [
+ {
+ "clause": "WHERE",
+ "comparator": "No filter",
+ "expressionType": "SIMPLE",
+ "operator": "TEMPORAL_RANGE",
+ "subject": "bot_profile__updated",
+ }
+ ],
+ "row_limit": 10000,
+ "color_scheme": "supersetColors",
+ "dashboards": [11],
+ "extra_form_data": {},
+ "label_colors": {},
+ "shared_label_colors": {},
+ "extra_filters": [],
+ "dashboardId": 11,
+ "force": False,
+ "result_format": "json",
+ "result_type": "full",
+ }
+ query_context.slice_.id = 97
+ query_context.slice_.params_dict = {
+ "datasource": "12__table",
+ "viz_type": "sankey_v2",
+ "slice_id": 97,
+ "source": "bot_id",
+ "target": "channel_id",
+ "metric": "count",
+ "adhoc_filters": [
+ {
+ "clause": "WHERE",
+ "comparator": "No filter",
+ "expressionType": "SIMPLE",
+ "operator": "TEMPORAL_RANGE",
+ "subject": "bot_profile__updated",
+ }
+ ],
+ "row_limit": 10000,
+ "color_scheme": "supersetColors",
+ "extra_form_data": {},
+ "dashboards": [11],
+ }
+ query_context.slice_.query_context = json.dumps(
+ {
+ "datasource": {"id": 12, "type": "table"},
+ "force": False,
+ "queries": [
+ {
+ "filters": [
+ {
+ "col": "bot_profile__updated",
+ "op": "TEMPORAL_RANGE",
+ "val": "No filter",
+ }
+ ],
+ "extras": {"having": "", "where": ""},
+ "applied_time_extras": {},
+ "columns": [],
+ "metrics": ["count"],
+ "annotation_layers": [],
+ "row_limit": 10000,
+ "series_limit": 0,
+ "order_desc": True,
+ "url_params": {},
+ "custom_params": {},
+ "custom_form_data": {},
+ "groupby": ["bot_id", "channel_id"],
+ }
+ ],
+ "form_data": {
+ "datasource": "12__table",
+ "viz_type": "sankey_v2",
+ "slice_id": 97,
+ "source": "bot_id",
+ "target": "channel_id",
+ "metric": "count",
+ "adhoc_filters": [
+ {
+ "clause": "WHERE",
+ "comparator": "No filter",
+ "expressionType": "SIMPLE",
+ "operator": "TEMPORAL_RANGE",
+ "subject": "bot_profile__updated",
+ }
+ ],
+ "row_limit": 10000,
+ "color_scheme": "supersetColors",
+ "extra_form_data": {},
+ "dashboards": [11],
+ "force": False,
+ "result_format": "json",
+ "result_type": "full",
+ },
+ "result_format": "json",
+ "result_type": "full",
+ }
+ )
+ assert not query_context_modified(query_context)
+
+
def test_get_catalog_perm() -> None:
"""
Test the `get_catalog_perm` method.