diff --git a/superset/security/manager.py b/superset/security/manager.py index 819fc5cba42c..837085e698cc 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -760,6 +760,29 @@ def freeze_value(value: Any) -> str: return json.dumps(_strip_overridable_keys(value), sort_keys=True) +def _ensure_list(value: Any) -> list[Any]: + """ + Normalize a value to a list for iteration. + + Some viz types (e.g. heatmap_v2's 'groupby' control) store a single + value as a bare string rather than a one-item list. Iterating a string + directly yields its individual characters, which silently breaks the + guest payload comparison for any such chart. + + ``None`` and an empty string are treated as "no value set" (mirroring + ``_stored_param_values``'s treatment of an unset control) and return + ``[]`` — an unset scalar control must not be compared as if the guest + had explicitly requested an empty string. A ``list``/``tuple`` is + filtered the same way, element by element; any other scalar is wrapped + in a single-item list. + """ + if value is None or value == "": + return [] + if isinstance(value, (list, tuple, set)): + return [item for item in value if item is not None and item != ""] + return [value] + + # Frontend-only markers that ``normalizeTimeColumn`` adds when it synthesizes a # chart's x-axis into a ``BASE_AXIS`` column. Like ``timeGrain`` they decorate # the column without changing which data is queried, so they must not count as @@ -905,10 +928,10 @@ def _native_filter_query_modified( # Columns, group-by, and series columns may only reference target column(s); # adhoc (free-form SQL) columns cannot be validated, so reject them. for key in ("columns", "groupby", "series_columns"): - for col in getattr(query, key, None) or []: + for col in _ensure_list(getattr(query, key, None)): if not isinstance(col, str) or col not in allowed_columns: return True - for metric in getattr(query, "metrics", None) or []: + for metric in _ensure_list(getattr(query, "metrics", None)): if not _native_filter_term_allowed(metric, allowed_columns, allowed_metrics): return True # A series-limit metric ranks the top-N groups in the inner query, so it is @@ -921,7 +944,7 @@ def _native_filter_query_modified( ): return True # order-by entries are ``(expression, asc)`` pairs. - for order in getattr(query, "orderby", None) or []: + for order in _ensure_list(getattr(query, "orderby", None)): expr = order[0] if isinstance(order, (list, tuple)) and order else order if not _native_filter_term_allowed(expr, allowed_columns, allowed_metrics): return True @@ -1157,11 +1180,14 @@ def _collect_stored_orderby_entries( Frozen saved orderby entries a guest may replay exactly. """ allowed: set[str] = { - freeze_value(entry) for entry in stored_chart.params_dict.get("orderby") or [] + freeze_value(entry) + for entry in _ensure_list(stored_chart.params_dict.get("orderby")) } if stored_query_context: for query in stored_query_context.get("queries") or []: - allowed.update(freeze_value(entry) for entry in query.get("orderby") or []) + allowed.update( + freeze_value(entry) for entry in _ensure_list(query.get("orderby")) + ) return allowed @@ -1169,11 +1195,7 @@ def _metric_control_values(value: Any) -> list[Any]: """ Return non-empty values from a metric-valued control. """ - if value is None or value == "": - return [] - if isinstance(value, (list, tuple)): - return [item for item in value if item is not None and item != ""] - return [value] + return _ensure_list(value) def _add_frozen_metric_control_values(allowed: set[str], value: Any) -> None: @@ -1372,7 +1394,7 @@ def _add_allowed_sql_from_query_context( if composed: extras_allowed.add(composed) for key in ("columns", "groupby"): - for col in query.get(key) or []: + for col in _ensure_list(query.get(key)): if isinstance(col, dict) and col.get("sqlExpression"): col_allowed.add(col["sqlExpression"]) @@ -1569,11 +1591,11 @@ def _sql_filters_modified( # Vector 2: SQL adhoc filters in form_data stored_sql_filters: set[str] = { freeze_value(flt) - for flt in stored_chart.params_dict.get("adhoc_filters") or [] + for flt in _ensure_list(stored_chart.params_dict.get("adhoc_filters")) if isinstance(flt, dict) and flt.get("expressionType") == "SQL" } - for flt in form_data.get("adhoc_filters") or []: + for flt in _ensure_list(form_data.get("adhoc_filters")): if not isinstance(flt, dict): continue if flt.get("expressionType") == "SQL": @@ -1624,38 +1646,13 @@ def _stored_param_values(params: dict[str, Any], keys: tuple[str, ...]) -> set[s """ values: set[str] = set() for key in keys: - value = params.get(key) - if value is None or value == "": - continue - items = value if isinstance(value, (list, tuple)) else [value] + items = _ensure_list(params.get(key)) values.update( freeze_value(item) for item in items if item is not None and item != "" ) return values -def _ensure_list(value: Any) -> list[Any]: - """ - Normalize a value to a list for iteration. - - Some viz types (e.g. heatmap_v2's 'groupby' control) store a single - value as a bare string rather than a one-item list. Iterating a string - directly yields its individual characters, which silently breaks the - guest payload comparison for any such chart. - - ``None`` and an empty string are treated as "no value set" (mirroring - ``_stored_param_values``'s treatment of an unset control) and return - ``[]`` — an unset scalar control must not be compared as if the guest - had explicitly requested an empty string. A ``list``/``tuple`` is - filtered the same way, element by element; any other scalar is wrapped - in a single-item list. - """ - if value is None: - return [] - items = value if isinstance(value, (list, tuple)) else [value] - return [item for item in items if item is not None and item != ""] - - def _columns_metrics_modified( query_context: "QueryContext", form_data: dict[str, Any], @@ -1711,7 +1708,7 @@ def _columns_metrics_modified( for equiv_key in equivalent: stored_values.update( _payload_value_identity(value, is_metric=is_metric) - for value in query.get(equiv_key) or [] + for value in _ensure_list(query.get(equiv_key)) ) if not queries_values.issubset(stored_values): diff --git a/tests/unit_tests/db_engine_specs/test_crdb.py b/tests/unit_tests/db_engine_specs/test_crdb.py index 6f8ca71b2330..01bd98af1eb3 100644 --- a/tests/unit_tests/db_engine_specs/test_crdb.py +++ b/tests/unit_tests/db_engine_specs/test_crdb.py @@ -44,6 +44,26 @@ def test_convert_dttm( assert_convert_dttm(spec, target_type, expected_result, dttm) +def test_cockroachdb_properties() -> None: + from superset.db_engine_specs.cockroachdb import CockroachDbEngineSpec + from superset.db_engine_specs.postgres import PostgresEngineSpec + + assert CockroachDbEngineSpec.engine == "cockroachdb" + assert CockroachDbEngineSpec.engine_name == "CockroachDB" + assert issubclass(CockroachDbEngineSpec, PostgresEngineSpec) + assert CockroachDbEngineSpec._extended_aggregations == {} + + +def test_cockroachdb_metadata() -> None: + from superset.db_engine_specs.cockroachdb import CockroachDbEngineSpec + + metadata = CockroachDbEngineSpec.metadata + assert "CockroachDB is a distributed SQL database" in metadata["description"] + assert metadata["logo"] == "cockroachdb.png" + assert "sqlalchemy-cockroachdb" in metadata["pypi_packages"] + assert metadata["default_port"] == 26257 + + def test_dialect_loads_under_installed_sqlalchemy() -> None: """ ``create_engine`` resolves and imports the ``cockroachdb`` SQLAlchemy diff --git a/tests/unit_tests/db_engine_specs/test_duckdb.py b/tests/unit_tests/db_engine_specs/test_duckdb.py index 7ba1413f5034..76b472fbb433 100644 --- a/tests/unit_tests/db_engine_specs/test_duckdb.py +++ b/tests/unit_tests/db_engine_specs/test_duckdb.py @@ -395,3 +395,22 @@ def test_extended_aggregation_func_median_stddev_var_executes() -> None: query = select(func(literal_column("sales"))).select_from(text("t")) result = conn.execute(query).scalar() assert result == pytest.approx(expected_value) + + +def test_motherduck_properties() -> None: + from superset.db_engine_specs.duckdb import DuckDBEngineSpec, MotherDuckEngineSpec + + assert MotherDuckEngineSpec.engine == "motherduck" + assert MotherDuckEngineSpec.engine_name == "MotherDuck" + assert issubclass(MotherDuckEngineSpec, DuckDBEngineSpec) + + +def test_motherduck_metadata() -> None: + from superset.db_engine_specs.duckdb import MotherDuckEngineSpec + + metadata = MotherDuckEngineSpec.metadata + assert ( + "MotherDuck is a serverless cloud analytics platform" in metadata["description"] + ) + assert metadata["logo"] == "motherduck.png" + assert "duckdb-engine" in metadata["pypi_packages"] diff --git a/tests/unit_tests/db_engine_specs/test_dynamodb.py b/tests/unit_tests/db_engine_specs/test_dynamodb.py index 17b3921b093e..175f282223ec 100644 --- a/tests/unit_tests/db_engine_specs/test_dynamodb.py +++ b/tests/unit_tests/db_engine_specs/test_dynamodb.py @@ -46,6 +46,50 @@ def test_convert_dttm( assert_convert_dttm(spec, target_type, expected_result, dttm) +def test_dynamodb_properties() -> None: + from superset.db_engine_specs.dynamodb import DynamoDBEngineSpec + + assert DynamoDBEngineSpec.engine == "dynamodb" + assert DynamoDBEngineSpec.engine_name == "Amazon DynamoDB" + + +def test_dynamodb_metadata() -> None: + from superset.db_engine_specs.dynamodb import DynamoDBEngineSpec + + metadata = DynamoDBEngineSpec.metadata + assert "Amazon DynamoDB is a serverless NoSQL database" in metadata["description"] + assert metadata["logo"] == "aws.png" + assert "pydynamodb" in metadata["pypi_packages"] + + +def test_epoch_to_dttm() -> None: + from superset.db_engine_specs.dynamodb import DynamoDBEngineSpec + + assert ( + DynamoDBEngineSpec.epoch_to_dttm().format(col="ts") + == "datetime(ts, 'unixepoch')" + ) + + +@pytest.mark.parametrize( + "time_grain,expected", + [ + (None, "ts"), + ("PT1S", "DATETIME(STRFTIME('%Y-%m-%dT%H:%M:%S', ts))"), + ("PT1M", "DATETIME(STRFTIME('%Y-%m-%dT%H:%M:00', ts))"), + ("PT1H", "DATETIME(STRFTIME('%Y-%m-%dT%H:00:00', ts))"), + ("P1D", "DATETIME(ts, 'start of day')"), + ("P1M", "DATETIME(ts, 'start of month')"), + ("P1Y", "DATETIME(ts, 'start of year')"), + ], +) +def test_time_grain_expressions(time_grain: str | None, expected: str) -> None: + from superset.db_engine_specs.dynamodb import DynamoDBEngineSpec + + actual = DynamoDBEngineSpec._time_grain_expressions[time_grain].format(col="ts") + assert actual == expected + + def test_convert_dttm_bounds_compare_with_iso_8601_strings() -> None: from superset.db_engine_specs.dynamodb import ( DynamoDBEngineSpec as spec, # noqa: N813 diff --git a/tests/unit_tests/db_engine_specs/test_firebolt.py b/tests/unit_tests/db_engine_specs/test_firebolt.py index 5f417d06c52c..3a5a34d2fe39 100644 --- a/tests/unit_tests/db_engine_specs/test_firebolt.py +++ b/tests/unit_tests/db_engine_specs/test_firebolt.py @@ -21,6 +21,7 @@ import pytest +from superset.constants import TimeGrain from tests.unit_tests.db_engine_specs.utils import assert_convert_dttm from tests.unit_tests.fixtures.common import dttm # noqa: F401 @@ -59,3 +60,43 @@ def test_epoch_to_dttm() -> None: FireboltEngineSpec.epoch_to_dttm().format(col="timestamp_column") == "from_unixtime(timestamp_column)" ) + + +def test_firebolt_properties() -> None: + from superset.db_engine_specs.firebolt import FireboltEngineSpec + + assert FireboltEngineSpec.engine == "firebolt" + assert FireboltEngineSpec.engine_name == "Firebolt" + assert FireboltEngineSpec.default_driver == "firebolt" + + +def test_firebolt_metadata() -> None: + from superset.db_engine_specs.firebolt import FireboltEngineSpec + + metadata = FireboltEngineSpec.metadata + assert "Firebolt is a cloud data warehouse" in metadata["description"] + assert metadata["logo"] == "firebolt.png" + assert "firebolt-sqlalchemy" in metadata["pypi_packages"] + + +@pytest.mark.parametrize( + "time_grain,expected", + [ + (None, "ts"), + (TimeGrain.SECOND, "date_trunc('second', CAST(ts AS TIMESTAMP))"), + (TimeGrain.MINUTE, "date_trunc('minute', CAST(ts AS TIMESTAMP))"), + (TimeGrain.HOUR, "date_trunc('hour', CAST(ts AS TIMESTAMP))"), + (TimeGrain.DAY, "date_trunc('day', CAST(ts AS TIMESTAMP))"), + (TimeGrain.WEEK, "date_trunc('week', CAST(ts AS TIMESTAMP))"), + (TimeGrain.MONTH, "date_trunc('month', CAST(ts AS TIMESTAMP))"), + (TimeGrain.QUARTER, "date_trunc('quarter', CAST(ts AS TIMESTAMP))"), + (TimeGrain.YEAR, "date_trunc('year', CAST(ts AS TIMESTAMP))"), + ], +) +def test_time_grain_expressions(time_grain: str | None, expected: str) -> None: + from superset.db_engine_specs.firebolt import FireboltEngineSpec + + assert ( + FireboltEngineSpec._time_grain_expressions[time_grain].format(col="ts") + == expected + ) diff --git a/tests/unit_tests/db_engine_specs/test_gsheets.py b/tests/unit_tests/db_engine_specs/test_gsheets.py index abc220c32944..55887cd94284 100644 --- a/tests/unit_tests/db_engine_specs/test_gsheets.py +++ b/tests/unit_tests/db_engine_specs/test_gsheets.py @@ -1276,6 +1276,24 @@ def test_convert_dttm( assert_convert_dttm(GSheetsEngineSpec, target_type, expected_result, dttm) +def test_gsheets_properties() -> None: + from superset.db_engine_specs.gsheets import GSheetsEngineSpec + from superset.db_engine_specs.shillelagh import ShillelaghEngineSpec + + assert GSheetsEngineSpec.engine == "gsheets" + assert GSheetsEngineSpec.engine_name == "Google Sheets" + assert issubclass(GSheetsEngineSpec, ShillelaghEngineSpec) + + +def test_gsheets_metadata() -> None: + from superset.db_engine_specs.gsheets import GSheetsEngineSpec + + metadata = GSheetsEngineSpec.metadata + assert "Google Sheets" in metadata["description"] + assert metadata["logo"] == "google-sheets.svg" + assert "shillelagh[gsheetsapi]" in metadata["pypi_packages"] + + def test_upload_dates(mocker: MockerFixture) -> None: """ Test that date and numpy values are uploaded as JSON values. diff --git a/tests/unit_tests/db_engine_specs/test_mssql.py b/tests/unit_tests/db_engine_specs/test_mssql.py index 80ef2a3847ea..91273e018e36 100644 --- a/tests/unit_tests/db_engine_specs/test_mssql.py +++ b/tests/unit_tests/db_engine_specs/test_mssql.py @@ -879,3 +879,21 @@ def test_get_catalog_from_engine_params_empty_database_query_param_falls_back_to ) assert "database" not in url.query assert MssqlEngineSpec.get_catalog_from_engine_params(url, {}) == "path_db" + + +def test_azure_synapse_properties() -> None: + from superset.db_engine_specs.mssql import AzureSynapseSpec, MssqlEngineSpec + + assert AzureSynapseSpec.engine == "mssql" + assert AzureSynapseSpec.engine_name == "Azure Synapse" + assert AzureSynapseSpec.default_driver == "pyodbc" + assert issubclass(AzureSynapseSpec, MssqlEngineSpec) + + +def test_azure_synapse_metadata() -> None: + from superset.db_engine_specs.mssql import AzureSynapseSpec + + metadata = AzureSynapseSpec.metadata + assert "Azure Synapse Analytics" in metadata["description"] + assert metadata["logo"] == "azure.svg" + assert "pymssql" in metadata["pypi_packages"] diff --git a/tests/unit_tests/db_engine_specs/test_oceanbase.py b/tests/unit_tests/db_engine_specs/test_oceanbase.py index a9288f631cfa..e7274c508165 100644 --- a/tests/unit_tests/db_engine_specs/test_oceanbase.py +++ b/tests/unit_tests/db_engine_specs/test_oceanbase.py @@ -14,48 +14,21 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from superset.db_engine_specs.mysql import MySQLEngineSpec +from superset.db_engine_specs.oceanbase import OceanBaseEngineSpec -from typing import Any, Optional -import pytest -from sqlalchemy import JSON, types +def test_oceanbase_properties() -> None: + assert OceanBaseEngineSpec.engine == "oceanbase" + assert OceanBaseEngineSpec.engine_name == "OceanBase" + assert OceanBaseEngineSpec.default_driver == "oceanbase" + assert OceanBaseEngineSpec.max_column_name_length == 128 + assert issubclass(OceanBaseEngineSpec, MySQLEngineSpec) + assert OceanBaseEngineSpec._extended_aggregations == {} -from superset.db_engine_specs.oceanbase import ARRAY, MAP, NUMBER, NUMERIC -from superset.utils.core import GenericDataType -from tests.unit_tests.db_engine_specs.utils import assert_column_spec - -@pytest.mark.parametrize( - "native_type,sqla_type,attrs,generic_type,is_dttm", - [ - # Numeric - ("tinyint", types.SMALLINT, None, GenericDataType.NUMERIC, False), - ("largeint", types.BIGINT, None, GenericDataType.NUMERIC, False), - ("decimal(38,18)", types.DECIMAL, None, GenericDataType.NUMERIC, False), - ("number(38,18)", NUMBER, None, GenericDataType.NUMERIC, False), - ("numeric(38,18)", NUMERIC, None, GenericDataType.NUMERIC, False), - ("double", types.FLOAT, None, GenericDataType.NUMERIC, False), - # String - ("char(10)", types.CHAR, None, GenericDataType.STRING, False), - ("varchar(65533)", types.VARCHAR, None, GenericDataType.STRING, False), - ("binary", types.BINARY, None, GenericDataType.STRING, False), - ("text", types.TEXT, None, GenericDataType.STRING, False), - # Complex type - ("array", ARRAY, None, GenericDataType.STRING, False), - ("map", MAP, None, GenericDataType.STRING, False), - ("json", JSON, None, GenericDataType.STRING, False), - ("jsonb", JSON, None, GenericDataType.STRING, False), - ], -) -def test_get_column_spec( - native_type: str, - sqla_type: type[types.TypeEngine], - attrs: Optional[dict[str, Any]], - generic_type: GenericDataType, - is_dttm: bool, -) -> None: - from superset.db_engine_specs.oceanbase import ( - OceanBaseEngineSpec as spec, # noqa: N813 - ) - - assert_column_spec(spec, native_type, sqla_type, attrs, generic_type, is_dttm) +def test_oceanbase_metadata() -> None: + metadata = OceanBaseEngineSpec.metadata + assert "OceanBase is a distributed relational database" in metadata["description"] + assert metadata["logo"] == "oceanbase.svg" + assert metadata["homepage_url"] == "https://www.oceanbase.com/" diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py index 0ac1b700842f..88ce1e2c443d 100644 --- a/tests/unit_tests/security/manager_test.py +++ b/tests/unit_tests/security/manager_test.py @@ -1938,6 +1938,29 @@ def test_query_context_modified_unset_scalar_control_not_tampered( assert not query_context_modified(query_context) +def test_query_context_modified_scalar_columns_param( + mocker: MockerFixture, +) -> None: + """ + Ensure single string values for columns or metrics in form_data and query + objects do not trigger false positive tamper rejection. + """ + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "columns": "region", + "metrics": "sales", + } + query_context.form_data = { + "slice_id": 42, + "columns": "region", + "metrics": "sales", + } + query_context.queries = [QueryObject(columns=["region"], metrics=["sales"])] + assert not query_context_modified(query_context) + + def _native_filter_ctx( mocker: MockerFixture, queries: list[Any],