You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/03/21 19:18:43 UTC
(superset) branch 3.1 updated (efa94c4ab0 -> 2ef0c13be9)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
from efa94c4ab0 fix: pass valid SQL to SM (#27464)
new 9887b4ed33 fix: guest queries (#27566)
new 48f851f261 fix: Skips Hive tests that are blocking PRs (#27605)
new fc611a8873 fix(db_engine_specs): Update convert_dttm to work correctly with CrateDB (#27567)
new 2ef0c13be9 fix: bump sqlglot to support materialized CTEs (#27576)
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
requirements/base.txt | 2 +-
setup.py | 2 +-
superset/db_engine_specs/crate.py | 2 +-
superset/security/manager.py | 24 +++--
tests/integration_tests/charts/data/api_tests.py | 9 ++
tests/unit_tests/db_engine_specs/test_crate.py | 2 +-
tests/unit_tests/security/manager_test.py | 119 ++++++++++++++++++++++-
7 files changed, 145 insertions(+), 15 deletions(-)
(superset) 02/04: fix: Skips Hive tests that are blocking PRs (#27605)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 48f851f2615f27cf06d1d3f0b035445f5fb64a63
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Thu Mar 21 13:26:24 2024 -0300
fix: Skips Hive tests that are blocking PRs (#27605)
(cherry picked from commit 718cd64657248f846a03a73167d2dc32d1f9dec5)
---
tests/integration_tests/charts/data/api_tests.py | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py
index 4def03ff4e..1cb607d770 100644
--- a/tests/integration_tests/charts/data/api_tests.py
+++ b/tests/integration_tests/charts/data/api_tests.py
@@ -62,6 +62,8 @@ from superset.common.chart_data import ChartDataResultFormat, ChartDataResultTyp
from tests.common.query_context_generator import ANNOTATION_LAYERS
from tests.integration_tests.fixtures.query_context import get_query_context
+from tests.integration_tests.test_app import app
+
CHART_DATA_URI = "api/v1/chart/data"
CHARTS_FIXTURE_COUNT = 10
@@ -79,6 +81,13 @@ INCOMPATIBLE_ADHOC_COLUMN_FIXTURE: AdhocColumn = {
}
+@pytest.fixture(autouse=True)
+def skip_by_backend():
+ with app.app_context():
+ if backend() == "hive":
+ pytest.skip("Skipping tests for Hive backend")
+
+
class BaseTestChartDataApi(SupersetTestCase):
query_context_payload_template = None
(superset) 01/04: fix: guest queries (#27566)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 9887b4ed33b4cd5319e0ce2107825787fc62da52
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Tue Mar 19 11:20:52 2024 -0400
fix: guest queries (#27566)
(cherry picked from commit 36290ce72fa806e8b6c063511ea434a97d91c3a9)
---
superset/security/manager.py | 24 +++---
tests/unit_tests/security/manager_test.py | 119 +++++++++++++++++++++++++++++-
2 files changed, 132 insertions(+), 11 deletions(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index f125269fb9..bc235ad505 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -68,6 +68,7 @@ from superset.security.guest_token import (
GuestTokenUser,
GuestUser,
)
+from superset.superset_typing import Metric
from superset.utils.core import (
DatasourceName,
DatasourceType,
@@ -144,6 +145,13 @@ RoleModelView.edit_columns = ["name", "permissions", "user"]
RoleModelView.related_views = []
+def freeze_metric(metric: Metric) -> str:
+ """
+ Used to compare metric sets.
+ """
+ return json.dumps(metric, sort_keys=True)
+
+
def query_context_modified(query_context: "QueryContext") -> bool:
"""
Check if a query context has been modified.
@@ -154,9 +162,9 @@ def query_context_modified(query_context: "QueryContext") -> bool:
form_data = query_context.form_data
stored_chart = query_context.slice_
- # sanity checks
+ # native filter requests
if form_data is None or stored_chart is None:
- return True
+ return False
# cannot request a different chart
if form_data.get("slice_id") != stored_chart.id:
@@ -164,11 +172,10 @@ def query_context_modified(query_context: "QueryContext") -> bool:
# compare form_data
requested_metrics = {
- frozenset(metric.items()) if isinstance(metric, dict) else metric
- for metric in form_data.get("metrics") or []
+ freeze_metric(metric) for metric in form_data.get("metrics") or []
}
stored_metrics = {
- frozenset(metric.items()) if isinstance(metric, dict) else metric
+ freeze_metric(metric)
for metric in stored_chart.params_dict.get("metrics") or []
}
if not requested_metrics.issubset(stored_metrics):
@@ -176,7 +183,7 @@ def query_context_modified(query_context: "QueryContext") -> bool:
# compare queries in query_context
queries_metrics = {
- frozenset(metric.items()) if isinstance(metric, dict) else metric
+ freeze_metric(metric)
for query in query_context.queries
for metric in query.metrics or []
}
@@ -185,10 +192,7 @@ def query_context_modified(query_context: "QueryContext") -> bool:
stored_query_context = json.loads(cast(str, stored_chart.query_context))
for query in stored_query_context.get("queries") or []:
stored_metrics.update(
- {
- frozenset(metric.items()) if isinstance(metric, dict) else metric
- for metric in query.get("metrics") or []
- }
+ {freeze_metric(metric) for metric in query.get("metrics") or []}
)
return not queries_metrics.issubset(stored_metrics)
diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py
index 22ec0dda4a..7ed32b0abb 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -26,7 +26,7 @@ from superset.connectors.sqla.models import Database, SqlaTable
from superset.exceptions import SupersetSecurityException
from superset.extensions import appbuilder
from superset.models.slice import Slice
-from superset.security.manager import SupersetSecurityManager
+from superset.security.manager import query_context_modified, SupersetSecurityManager
from superset.sql_parse import Table
from superset.superset_typing import AdhocMetric
from superset.utils.core import override_user
@@ -414,3 +414,120 @@ def test_raise_for_access_chart_owner(
sm.raise_for_access(
chart=slice,
)
+
+
+def test_query_context_modified(
+ mocker: MockFixture,
+ stored_metrics: list[AdhocMetric],
+) -> None:
+ """
+ Test the `query_context_modified` function.
+
+ The function is used to ensure guest users are not modifying the request payload on
+ embedded dashboard, preventing users from modifying it to access metrics different
+ from the ones stored in dashboard charts.
+ """
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metrics": stored_metrics,
+ }
+
+ query_context.form_data = {
+ "slice_id": 42,
+ "metrics": stored_metrics,
+ }
+ query_context.queries = [QueryObject(metrics=stored_metrics)] # type: ignore
+ assert not query_context_modified(query_context)
+
+
+def test_query_context_modified_tampered(
+ mocker: MockFixture,
+ stored_metrics: list[AdhocMetric],
+) -> None:
+ """
+ Test the `query_context_modified` function when the request is tampered with.
+
+ The function is used to ensure guest users are not modifying the request payload on
+ embedded dashboard, preventing users from modifying it to access metrics different
+ from the ones stored in dashboard charts.
+ """
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metrics": stored_metrics,
+ }
+
+ tampered_metrics = [
+ {
+ "column": None,
+ "expressionType": "SQL",
+ "hasCustomLabel": False,
+ "label": "COUNT(*) + 2",
+ "sqlExpression": "COUNT(*) + 2",
+ }
+ ]
+
+ query_context.form_data = {
+ "slice_id": 42,
+ "metrics": tampered_metrics,
+ }
+ query_context.queries = [QueryObject(metrics=tampered_metrics)] # type: ignore
+ assert query_context_modified(query_context)
+
+
+def test_query_context_modified_native_filter(mocker: MockFixture) -> None:
+ """
+ Test the `query_context_modified` function with a native filter request.
+
+ A native filter request has no chart (slice) associated with it.
+ """
+ query_context = mocker.MagicMock()
+ query_context.slice_ = None
+
+ assert not query_context_modified(query_context)
+
+
+def test_query_context_modified_mixed_chart(mocker: MockFixture) -> None:
+ """
+ Test the `query_context_modified` function for a mixed chart request.
+
+ The metrics in the mixed chart are a nested dictionary (due to `columns`), and need
+ to be serialized to JSON with the keys sorted in order to compare the request
+ metrics with the chart metrics.
+ """
+ stored_metrics = [
+ {
+ "optionName": "metric_vgops097wej_g8uff99zhk7",
+ "label": "AVG(num)",
+ "expressionType": "SIMPLE",
+ "column": {"column_name": "num", "type": "BIGINT(20)"},
+ "aggregate": "AVG",
+ }
+ ]
+ # different order (remember, dicts have order!)
+ requested_metrics = [
+ {
+ "aggregate": "AVG",
+ "column": {"column_name": "num", "type": "BIGINT(20)"},
+ "expressionType": "SIMPLE",
+ "label": "AVG(num)",
+ "optionName": "metric_vgops097wej_g8uff99zhk7",
+ }
+ ]
+
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metrics": stored_metrics,
+ }
+
+ query_context.form_data = {
+ "slice_id": 42,
+ "metrics": requested_metrics,
+ }
+ query_context.queries = [QueryObject(metrics=requested_metrics)] # type: ignore
+ assert not query_context_modified(query_context)
(superset) 03/04: fix(db_engine_specs): Update convert_dttm to work correctly with CrateDB (#27567)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit fc611a8873f45a88393e60774a896bb6106e6d90
Author: hlcianfagna <11...@users.noreply.github.com>
AuthorDate: Thu Mar 21 18:35:40 2024 +0000
fix(db_engine_specs): Update convert_dttm to work correctly with CrateDB (#27567)
(cherry picked from commit fcceaf081c85c501ce946a114447751d43a1f8fb)
---
superset/db_engine_specs/crate.py | 2 +-
tests/unit_tests/db_engine_specs/test_crate.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/superset/db_engine_specs/crate.py b/superset/db_engine_specs/crate.py
index 46ce1e08ff..4952bd5a0d 100644
--- a/superset/db_engine_specs/crate.py
+++ b/superset/db_engine_specs/crate.py
@@ -59,7 +59,7 @@ class CrateEngineSpec(BaseEngineSpec):
sqla_type = cls.get_sqla_column_type(target_type)
if isinstance(sqla_type, types.TIMESTAMP):
- return f"{dttm.timestamp() * 1000}"
+ return f"CAST('{dttm.isoformat()}' AS TIMESTAMP)"
return None
@classmethod
diff --git a/tests/unit_tests/db_engine_specs/test_crate.py b/tests/unit_tests/db_engine_specs/test_crate.py
index 2cb1cd7896..d2bace955c 100644
--- a/tests/unit_tests/db_engine_specs/test_crate.py
+++ b/tests/unit_tests/db_engine_specs/test_crate.py
@@ -59,7 +59,7 @@ def test_alter_new_orm_column() -> None:
@pytest.mark.parametrize(
"target_type,expected_result",
[
- ("TimeStamp", "1546398245678.9"),
+ ("TimeStamp", "CAST('2019-01-02T03:04:05.678900' AS TIMESTAMP)"),
("UnknownType", None),
],
)
(superset) 04/04: fix: bump sqlglot to support materialized CTEs (#27576)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 2ef0c13be93fa54ae931fe8988e85c12001eb95f
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Wed Mar 20 15:26:01 2024 -0400
fix: bump sqlglot to support materialized CTEs (#27576)
---
requirements/base.txt | 2 +-
setup.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/requirements/base.txt b/requirements/base.txt
index de25938a01..98ee569109 100644
--- a/requirements/base.txt
+++ b/requirements/base.txt
@@ -335,7 +335,7 @@ sqlalchemy-utils==0.38.3
# via
# apache-superset
# flask-appbuilder
-sqlglot==20.8.0
+sqlglot==23.0.2
# via apache-superset
sqlparse==0.4.4
# via apache-superset
diff --git a/setup.py b/setup.py
index 7050d7b497..62fffb5b59 100644
--- a/setup.py
+++ b/setup.py
@@ -126,7 +126,7 @@ setup(
"slack_sdk>=3.19.0, <4",
"sqlalchemy>=1.4, <2",
"sqlalchemy-utils>=0.38.3, <0.39",
- "sqlglot>=20,<21",
+ "sqlglot>=23.0.2,<24",
"sqlparse>=0.4.4, <0.5",
"tabulate>=0.8.9, <0.9",
"typing-extensions>=4, <5",