diff --git a/superset/security/guest_payload.py b/superset/security/guest_payload.py new file mode 100644 index 000000000000..63ee0b66babc --- /dev/null +++ b/superset/security/guest_payload.py @@ -0,0 +1,326 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Helpers for comparing a guest user's chart data request against the stored chart. + +Used by ``query_context_modified`` to ensure guest users on embedded dashboards can +only read the metrics and columns the chart was saved with. +""" + +from typing import Any + +from superset.utils import json + +#: Chart ``params`` keys that can hold a chart's metrics. A request always carries its +#: metrics under ``metrics``, but each chart type saves them under its own control name: +#: big number and pie store ``metric``, bubble stores ``x``/``y``/``size``, and so on. +#: Broader than ``METRIC_FORM_DATA_PARAMS`` in ``superset.connectors.sqla.models``, +#: which omits several of these; kept separate to avoid importing that module here. +STORED_METRIC_PARAMS = ( + "metric", + "metric_2", + "metrics", + "metrics_b", + "percent_metrics", + "point_radius_fixed", + "right_axis_metric", + "secondary_metric", + "series_limit_metric", + "series_limit_metric_b", + "size", + "timeseries_limit_metric", + "timeseries_limit_metric_b", + "tooltip_metrics", + "x", + "y", +) + +#: Chart ``params`` keys that can hold a chart's columns or group-bys. As with the +#: metrics above, a request carries them under ``columns``/``groupby`` while charts save +#: them under control names such as ``all_columns`` (table), ``source``/``target`` +#: (sankey), or ``x_axis`` (time series). +STORED_COLUMN_PARAMS = ( + "all_columns", + "all_columns_x", + "column", + "columns", + "dimension", + "end_spatial", + "end_time", + "entity", + "geom_column", + "granularity_sqla", + "groupby", + "groupbyColumns", + "groupbyRows", + "groupby_b", + "id", + "js_columns", + "line_column", + "name", + "order_by_cols", + "parent", + "series", + "series_columns", + "source", + "source_category", + "spatial", + "start_spatial", + "start_time", + "target", + "target_category", + "tooltip_columns", + "tooltip_contents", + "x_axis", + "y_axis", +) + +#: Keys that ``normalizeTimeColumn`` (superset-ui-core) adds when it synthesizes a +#: chart's saved x-axis into a ``BASE_AXIS`` column. They describe how the axis is +#: rendered rather than which data is read. +BASE_AXIS_SYNTHETIC_KEYS = frozenset({"columnType", "isColumnReference", "timeGrain"}) + + +def freeze_value(value: Any) -> str: + """ + Used to compare column and metric sets. + """ + return json.dumps(value, sort_keys=True) + + +def collapse_column_reference(value: Any) -> Any: + """ + Reduce an adhoc column that merely names a physical column to that column's name. + + A physical column is saved in a chart's ``params`` as a bare string, but the + frontend may send it as an adhoc column that only points back at it — either + tagged ``columnType: "BASE_AXIS"`` with ``isColumnReference`` (how a saved x-axis + is normalized), or as a plain adhoc column whose ``sqlExpression`` is its own + label. Neither reads data beyond the saved column, yet neither appears verbatim in + ``params``, so a guest merely loading such a chart would otherwise be rejected as + a tamperer. + + Collapsing only rewrites the reference to the name it points at; the result still + has to match a value stored on the chart, so pointing at an unrelated column or + wrapping free-form SQL grants no additional access. Adhoc x-axis columns keep + their definition and shed only the synthetic markers. Other values are returned + unchanged. + """ + if not isinstance(value, dict): + return value + + expression = value.get("sqlExpression") + if isinstance(expression, str) and ( + value.get("isColumnReference") or expression == value.get("label") + ): + return expression + + if value.get("columnType") == "BASE_AXIS": + return { + key: val + for key, val in value.items() + if key not in BASE_AXIS_SYNTHETIC_KEYS + } + + return value + + +def unwrap_orderby(value: Any) -> Any: + """ + Return the expression an ``orderby`` entry sorts on. + + Entries are ``[expression, is_ascending]`` pairs; only the expression selects data + and therefore needs to be validated. + """ + if ( + isinstance(value, (list, tuple)) + and len(value) == 2 + and isinstance(value[1], bool) + ): + return value[0] + return value + + +def _as_items(value: Any) -> list[Any]: + """ + The individual values held by a control, which may be scalar or list-valued. + + Scalar-valued controls (``metric``, ``x_axis``, heatmap's ``groupby``, ...) hold a + single value. Wrapping them keeps a bare string from being iterated character by + character, which would compare nothing meaningful. + """ + 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] + + +def _decode_orderby_pair(value: Any) -> Any: + """ + Decode an ``order_by_cols`` entry, which stores its pair JSON-encoded in a string. + + Only strings holding a ``[column, is_ascending]`` list are decoded, so a column + whose name happens to be valid JSON is left alone. + """ + if not isinstance(value, str): + return value + try: + decoded = json.loads(value) + except ValueError: + return value + return decoded if isinstance(decoded, list) else value + + +#: Controls whose stored value is a nested lat/lon/geohash configuration rather than a +#: column name. The frontend decomposes them with ``getSpatialColumns()`` and queries +#: the columns they name, so the stored config has to be decomposed the same way to be +#: comparable against the flat column names a request carries. +SPATIAL_PARAMS = frozenset({"end_spatial", "spatial", "start_spatial"}) + +#: Keys a spatial configuration can name a column under, per ``getSpatialColumns()``. +SPATIAL_COLUMN_KEYS = ("geohashCol", "latCol", "lonCol", "lonlatCol") + +#: Control holding a chart definition JSON-encoded inside a string, with its own metrics +#: and columns nested within. Scanning the outer ``params`` keys never looks inside it. +EMBEDDED_CHART_PARAM = "selected_chart" + + +def decompose_spatial(value: Any) -> list[Any]: + """ + The columns a spatial configuration names, mirroring ``getSpatialColumns()``. + + A configuration names its columns under ``lonCol``/``latCol`` (latlong), + ``lonlatCol`` (delimited) or ``geohashCol`` (geohash). All of them are returned + regardless of the declared ``type``, since every one is a column the chart is saved + with; the request still has to name one of them exactly. + """ + if not isinstance(value, dict): + return [value] + return [value[key] for key in SPATIAL_COLUMN_KEYS if value.get(key)] + + +def decompose_tooltip_contents(value: Any) -> list[Any]: + """ + The columns a deck.gl tooltip entry reads, mirroring ``extractTooltipColumns()``. + + An entry is either a bare column name or a tooltip-config object wrapping one. Only + ``item_type: "column"`` entries reach the query's columns; metric entries are read + from data already fetched and add nothing to select. + """ + if isinstance(value, dict): + if value.get("item_type") == "column" and value.get("column_name"): + return [value["column_name"]] + return [] + return [value] + + +def decompose_fixed_or_metric(value: Any) -> list[Any]: + """ + The metric a fixed-or-metric control holds, if it holds one. + + deck.gl's ``point_radius_fixed`` is either a legacy bare metric name or + ``{"type": "fix" | "metric", "value": ...}``, where only the ``metric`` variant + contributes a metric to the query. + """ + if not isinstance(value, dict): + return [value] + if value.get("type") == "metric" and value.get("value") is not None: + return [value["value"]] + return [] + + +def embedded_chart_params(value: Any) -> dict[str, Any]: + """ + The ``params`` of a chart definition nested inside a control's value. + + Cartodiagram stores the chart it renders per feature in ``selected_chart`` as a + JSON string whose ``params`` is itself a JSON string. Its metrics and columns are + the ones actually queried, so they have to be reachable. + """ + definition = _decode_json(value) + if not isinstance(definition, dict): + return {} + params = _decode_json(definition.get("params")) + return params if isinstance(params, dict) else {} + + +def _decode_json(value: Any) -> Any: + """ + Decode a JSON-encoded string, leaving anything else untouched. + """ + if not isinstance(value, str): + return value + try: + return json.loads(value) + except ValueError: + return None + + +def _decompose(key: str, value: Any) -> list[Any]: + """ + The individual metrics or columns a control's value holds. + + Most controls hold their values flat, as a name or a list of names. The ones that + wrap them in a nested structure are decomposed into the names they point at, so that + they compare against the flat names a request carries. + """ + if key in SPATIAL_PARAMS: + return decompose_spatial(value) + if key == "tooltip_contents": + return decompose_tooltip_contents(value) + if key == "point_radius_fixed": + return decompose_fixed_or_metric(value) + return [value] + + +def _normalize(value: Any) -> str: + """ + Frozen form of a single metric or column, ready to be compared. + + Sort entries are reduced to the expression they sort on, and adhoc references to the + column they point at, so that the same underlying metric or column compares equal + however the request and the chart's ``params`` happen to spell it. + """ + value = unwrap_orderby(_decode_orderby_pair(unwrap_orderby(value))) + return freeze_value(collapse_column_reference(value)) + + +def requested_values(values: Any) -> set[str]: + """ + Frozen values from a request, comparable against ``stored_param_values``. + """ + return {_normalize(value) for value in _as_items(values)} + + +def stored_param_values(params: dict[str, Any], keys: tuple[str, ...]) -> set[str]: + """ + Frozen values stored under any of the given chart ``params`` keys. + + A chart nested in ``selected_chart`` is descended into, since its own metrics and + columns are what get queried. Matching itself stays exact; only the shape of each + value is normalized. + """ + values: set[str] = set() + for key in keys: + for item in _as_items(params.get(key)): + values |= {_normalize(part) for part in _decompose(key, item)} + + if nested := embedded_chart_params(params.get(EMBEDDED_CHART_PARAM)): + values |= stored_param_values(nested, keys) + + return values diff --git a/superset/security/manager.py b/superset/security/manager.py index 86339a8bd960..d5bbb8260961 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -60,6 +60,13 @@ DatasetInvalidPermissionEvaluationException, SupersetSecurityException, ) +from superset.security.guest_payload import ( + freeze_value, # noqa: F401 (re-exported for backwards compatibility) + requested_values, + STORED_COLUMN_PARAMS, + STORED_METRIC_PARAMS, + stored_param_values, +) from superset.security.guest_token import ( GuestToken, GuestTokenResources, @@ -191,13 +198,6 @@ def pre_delete(self, item: Model) -> None: ViewMenuModelView.include_route_methods = {RouteMethod.LIST} -def freeze_value(value: Any) -> str: - """ - Used to compare column and metric sets. - """ - return json.dumps(value, sort_keys=True) - - def query_context_modified(query_context: "QueryContext") -> bool: """ Check if a query context has been modified. @@ -222,32 +222,31 @@ def query_context_modified(query_context: "QueryContext") -> bool: else None ) + # A request carries its metrics under `metrics` and its columns under + # `columns`/`groupby`, but a chart stores them under whichever control names its viz + # type uses, so each request key is compared against every equivalent stored key. + # An `orderby` may sort on any metric or column the chart already reads. + order_by_params = STORED_METRIC_PARAMS + STORED_COLUMN_PARAMS + ("orderby",) + # compare columns and metrics in form_data with stored values for key, equivalent in [ - ("metrics", ["metrics"]), - ("columns", ["columns", "groupby"]), - ("groupby", ["columns", "groupby"]), - ("orderby", ["orderby"]), + ("metrics", STORED_METRIC_PARAMS), + ("columns", STORED_COLUMN_PARAMS), + ("groupby", STORED_COLUMN_PARAMS), + ("orderby", order_by_params), ]: - 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 [] - } - if not requested_values.issubset(stored_values): + stored_values = stored_param_values(stored_chart.params_dict, equivalent) + if not requested_values(form_data.get(key)).issubset(stored_values): return True # compare queries in query_context - queries_values = { - freeze_value(value) - for query in query_context.queries - for value in getattr(query, key, []) or [] - } + queries_values = set() + for query in query_context.queries: + queries_values |= requested_values(getattr(query, key, [])) + if stored_query_context: - for query in stored_query_context.get("queries") or []: - for key in equivalent: - stored_values.update( - {freeze_value(value) for value in query.get(key) or []} - ) + for stored_query in stored_query_context.get("queries") or []: + stored_values |= stored_param_values(stored_query, equivalent) 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 ca4b0b8df8bb..008353102362 100644 --- a/tests/unit_tests/security/manager_test.py +++ b/tests/unit_tests/security/manager_test.py @@ -19,6 +19,7 @@ import json # noqa: TID251 from types import SimpleNamespace +from typing import Any import pytest from flask_appbuilder.security.sqla.models import Role, User @@ -1045,6 +1046,344 @@ def test_query_context_modified_orderby(mocker: MockerFixture) -> None: assert query_context_modified(query_context) +def build_guest_query_context( + mocker: MockerFixture, + params: dict[str, Any], + form_data: dict[str, Any], + queries: list[QueryObject] | None = None, +) -> Any: + """ + A guest request for the chart described by `params`. + + `form_data` holds only the keys under test; the slice id is filled in so the request + is for the stored chart rather than a different one. + """ + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = params + query_context.form_data = {"slice_id": 42, **form_data} + query_context.queries = queries if queries is not None else [] + return query_context + + +def test_query_context_modified_gantt_tooltip_metrics( + mocker: MockerFixture, +) -> None: + """ + Test that a Gantt chart's `tooltip_metrics` are accepted as stored metrics. + + Gantt's `buildQuery` sends `tooltip_metrics` as the query's `metrics`. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "gantt_chart", + "start_time": "start", + "end_time": "end", + "y_axis": "task", + "tooltip_metrics": ["count"], + }, + {"metrics": ["count"], "columns": ["start", "end", "task"]}, + ) + assert not query_context_modified(query_context) + + +def test_query_context_modified_mixed_timeseries_query_b( + mocker: MockerFixture, +) -> None: + """ + Test that a mixed timeseries chart's Query B controls are accepted. + + `retainFormDataSuffix` strips the `_b` suffix, so `groupby_b` and + `timeseries_limit_metric_b` become Query B's columns and sort metric. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "mixed_timeseries", + "metrics": ["count"], + "groupby": ["gender"], + "metrics_b": ["sum__num"], + "groupby_b": ["state"], + "timeseries_limit_metric_b": "sum__num", + }, + {}, + [ + QueryObject(metrics=["count"], columns=["gender"]), + QueryObject( + metrics=["sum__num"], + columns=["state"], + orderby=[("sum__num", False)], + ), + ], + ) + assert not query_context_modified(query_context) + + +def test_query_context_modified_graph_categories(mocker: MockerFixture) -> None: + """ + Test that a graph chart's category controls are accepted as stored columns. + + Graph aliases `source_category` and `target_category` to `columns`. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "graph_chart", + "source": "src", + "target": "tgt", + "source_category": "src_cat", + "target_category": "tgt_cat", + "metric": "count", + }, + {"columns": ["src", "tgt", "src_cat", "tgt_cat"], "metrics": ["count"]}, + ) + assert not query_context_modified(query_context) + + +def test_query_context_modified_deckgl_spatial(mocker: MockerFixture) -> None: + """ + Test that a deck.gl chart's nested spatial configuration is decomposed. + + `getSpatialColumns()` turns the config into the columns the query selects, so the + stored config has to be decomposed the same way. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "deck_screengrid", + "spatial": {"type": "latlong", "lonCol": "LON", "latCol": "LAT"}, + "size": "count", + "js_columns": ["extra"], + }, + {"columns": ["LON", "LAT", "extra"], "metrics": ["count"]}, + ) + assert not query_context_modified(query_context) + + +def test_query_context_modified_deckgl_arc_spatial(mocker: MockerFixture) -> None: + """ + Test that an Arc chart's start and end spatial configurations are decomposed. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "deck_arc", + "start_spatial": {"type": "delimited", "lonlatCol": "from_latlon"}, + "end_spatial": {"type": "geohash", "geohashCol": "to_geohash"}, + "dimension": "carrier", + }, + {"columns": ["from_latlon", "to_geohash", "carrier"]}, + ) + assert not query_context_modified(query_context) + + +def test_query_context_modified_deckgl_spatial_tampered( + mocker: MockerFixture, +) -> None: + """ + Test that a column absent from a spatial configuration is still rejected. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "deck_screengrid", + "spatial": {"type": "latlong", "lonCol": "LON", "latCol": "LAT"}, + }, + {"columns": ["LON", "LAT", "salary"]}, + ) + assert query_context_modified(query_context) + + +def test_query_context_modified_deckgl_tooltip_contents( + mocker: MockerFixture, +) -> None: + """ + Test that deck.gl tooltip-config objects are reduced to the columns they wrap. + + `extractTooltipColumns()` adds `item_type: "column"` entries to the query's columns. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "deck_path", + "line_column": "path", + "tooltip_contents": [ + {"item_type": "column", "column_name": "name"}, + {"item_type": "metric", "metric_name": "count"}, + "plain_column", + ], + }, + {"columns": ["path", "name", "plain_column"]}, + ) + assert not query_context_modified(query_context) + + +def test_query_context_modified_deckgl_tooltip_metric_not_a_column( + mocker: MockerFixture, +) -> None: + """ + Test that a tooltip metric entry does not become a permitted column. + + Metric entries are read from data already fetched and never reach the query's + columns, so requesting one as a column is a modification. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "deck_path", + "line_column": "path", + "tooltip_contents": [{"item_type": "metric", "metric_name": "count"}], + }, + {"columns": ["path", "count"]}, + ) + assert query_context_modified(query_context) + + +def test_query_context_modified_point_radius_fixed_metric( + mocker: MockerFixture, +) -> None: + """ + Test that a metric-typed `point_radius_fixed` is accepted as a stored metric. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "deck_scatter", + "spatial": {"type": "latlong", "lonCol": "LON", "latCol": "LAT"}, + "point_radius_fixed": {"type": "metric", "value": "count"}, + }, + { + "columns": ["LON", "LAT"], + "metrics": ["count"], + "orderby": [["count", False]], + }, + ) + assert not query_context_modified(query_context) + + +def test_query_context_modified_point_radius_fixed_legacy( + mocker: MockerFixture, +) -> None: + """ + Test that the legacy bare-string form of `point_radius_fixed` is accepted. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "deck_scatter", + "spatial": {"type": "latlong", "lonCol": "LON", "latCol": "LAT"}, + "point_radius_fixed": "count", + }, + {"columns": ["LON", "LAT"], "metrics": ["count"]}, + ) + assert not query_context_modified(query_context) + + +def test_query_context_modified_point_radius_fixed_value_not_a_metric( + mocker: MockerFixture, +) -> None: + """ + Test that a fixed-typed `point_radius_fixed` grants no metric. + + A `fix` value is a radius in pixels, never queried, so a metric of the same name + still has to be stored elsewhere on the chart. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "deck_scatter", + "spatial": {"type": "latlong", "lonCol": "LON", "latCol": "LAT"}, + "point_radius_fixed": {"type": "fix", "value": "count"}, + }, + {"columns": ["LON", "LAT"], "metrics": ["count"]}, + ) + assert query_context_modified(query_context) + + +def test_query_context_modified_cartodiagram(mocker: MockerFixture) -> None: + """ + Test that a Cartodiagram's nested chart definition is descended into. + + `selected_chart` holds a JSON-encoded chart whose own `params` is a JSON string; its + metrics and group-bys are what the delegated `buildQuery` actually queries, with the + geometry column prepended to the group-bys. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "cartodiagram", + "geom_column": "geom", + "selected_chart": json.dumps( + { + "viz_type": "pie", + "params": json.dumps( + {"metric": "count", "groupby": ["gender"]}, + ), + } + ), + }, + {"groupby": ["geom", "gender"], "metrics": ["count"]}, + ) + assert not query_context_modified(query_context) + + +def test_query_context_modified_cartodiagram_tampered( + mocker: MockerFixture, +) -> None: + """ + Test that a column absent from a Cartodiagram's nested chart is still rejected. + """ + query_context = build_guest_query_context( + mocker, + { + "viz_type": "cartodiagram", + "geom_column": "geom", + "selected_chart": json.dumps( + { + "viz_type": "pie", + "params": json.dumps( + {"metric": "count", "groupby": ["gender"]}, + ), + } + ), + }, + {"groupby": ["geom", "salary"], "metrics": ["count"]}, + ) + assert query_context_modified(query_context) + + +def test_query_context_modified_malformed_selected_chart( + mocker: MockerFixture, +) -> None: + """ + Test that an unparseable `selected_chart` neither crashes nor grants access. + """ + for selected_chart in ["not json", json.dumps({"params": "not json"}), None, 42]: + query_context = build_guest_query_context( + mocker, + { + "viz_type": "cartodiagram", + "geom_column": "geom", + "selected_chart": selected_chart, + }, + {"groupby": ["geom"]}, + ) + assert not query_context_modified(query_context) + + tampered = build_guest_query_context( + mocker, + { + "viz_type": "cartodiagram", + "geom_column": "geom", + "selected_chart": selected_chart, + }, + {"groupby": ["geom", "salary"]}, + ) + assert query_context_modified(tampered) + + def test_get_catalog_perm() -> None: """ Test the `get_catalog_perm` method. @@ -1547,3 +1886,399 @@ def test_validate_child_in_parent_multilayer_null_params( assert not sm._validate_child_in_parent_multilayer( child_slice_id=1, parent_slice=parent_slice ) + + +def test_query_context_modified_stored_groupby(mocker: MockerFixture) -> None: + """ + Test the `query_context_modified` function for a chart storing only `groupby`. + + A request always sends its columns under `columns`, while a chart with a group-by + stores them under `groupby`, so the two control names must compare as equivalent. + """ + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "metrics": ["count"], + "groupby": ["deal_size"], + } + + query_context.form_data = { + "slice_id": 42, + "metrics": ["count"], + "columns": ["deal_size"], + } + query_context.queries = [QueryObject(metrics=["count"], columns=["deal_size"])] + assert not query_context_modified(query_context) + + +def test_query_context_modified_stored_metric(mocker: MockerFixture) -> None: + """ + Test the `query_context_modified` function for a big number chart. + + Big number stores its single metric under `metric`, but requests it as `metrics`. + """ + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = {"metric": "count"} + + query_context.form_data = { + "slice_id": 42, + "metric": "count", + "metrics": ["count"], + } + query_context.queries = [QueryObject(metrics=["count"])] + assert not query_context_modified(query_context) + + +def test_query_context_modified_stored_all_columns(mocker: MockerFixture) -> None: + """ + Test the `query_context_modified` function for a table chart in raw mode. + + A raw mode table stores its columns under `all_columns`. + """ + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "query_mode": "raw", + "all_columns": ["product_line", "status"], + } + + query_context.form_data = { + "slice_id": 42, + "query_mode": "raw", + "all_columns": ["product_line", "status"], + "columns": ["product_line", "status"], + } + query_context.queries = [QueryObject(columns=["product_line", "status"])] + assert not query_context_modified(query_context) + + +def test_query_context_modified_base_axis(mocker: MockerFixture) -> None: + """ + Test the `query_context_modified` function for a time series chart. + + The frontend replaces the chart's saved `x_axis` with a synthesized `BASE_AXIS` + column reference, which must compare equal to the stored physical column name. + """ + base_axis: dict[str, Any] = { + "columnType": "BASE_AXIS", + "expressionType": "SQL", + "isColumnReference": True, + "label": "order_date", + "sqlExpression": "order_date", + "timeGrain": "P1M", + } + + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "metrics": ["count"], + "groupby": ["deal_size"], + "x_axis": "order_date", + } + + query_context.form_data = { + "slice_id": 42, + "metrics": ["count"], + "x_axis": "order_date", + "columns": [base_axis, "deal_size"], + } + query_context.queries = [ + QueryObject(metrics=["count"], columns=[base_axis, "deal_size"]) # type: ignore + ] + assert not query_context_modified(query_context) + + +def test_query_context_modified_base_axis_tampered(mocker: MockerFixture) -> None: + """ + Test the `query_context_modified` function when a `BASE_AXIS` is forged. + + Tagging a column the chart does not read as `BASE_AXIS` must not grant access to it. + """ + forged_axis: dict[str, Any] = { + "columnType": "BASE_AXIS", + "expressionType": "SQL", + "isColumnReference": True, + "label": "credit_limit", + "sqlExpression": "credit_limit", + "timeGrain": "P1M", + } + + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "metrics": ["count"], + "groupby": ["deal_size"], + "x_axis": "order_date", + } + + query_context.form_data = { + "slice_id": 42, + "metrics": ["count"], + "x_axis": "order_date", + "columns": [forged_axis, "deal_size"], + } + query_context.queries = [ + QueryObject(metrics=["count"], columns=[forged_axis, "deal_size"]) # type: ignore + ] + assert query_context_modified(query_context) + + +def test_query_context_modified_base_axis_metric_tampered( + mocker: MockerFixture, +) -> None: + """ + Test the `query_context_modified` function when `BASE_AXIS` markers are smuggled + onto a metric, to disguise free-form SQL as a column reference. + """ + smuggled_metric: dict[str, Any] = { + "columnType": "BASE_AXIS", + "expressionType": "SQL", + "isColumnReference": True, + "label": "MAX(credit_limit)", + "sqlExpression": "MAX(credit_limit)", + } + + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "metrics": ["count"], + "x_axis": "order_date", + } + + query_context.form_data = { + "slice_id": 42, + "metrics": [smuggled_metric], + "x_axis": "order_date", + } + query_context.queries = [QueryObject(metrics=[smuggled_metric])] # type: ignore + assert query_context_modified(query_context) + + +def test_query_context_modified_orderby_own_metric(mocker: MockerFixture) -> None: + """ + Test the `query_context_modified` function when sorting by the chart's own metric. + + Charts legitimately order by a metric or column they already read, neither of which + is stored under `orderby`. + """ + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "metrics": ["count"], + "groupby": ["deal_size"], + } + + query_context.form_data = { + "slice_id": 42, + "metrics": ["count"], + "columns": ["deal_size"], + "orderby": [["count", False]], + } + query_context.queries = [ + QueryObject( + metrics=["count"], + columns=["deal_size"], + orderby=[("count", False)], + ) + ] + assert not query_context_modified(query_context) + + +def test_query_context_modified_orderby_unrelated_column( + mocker: MockerFixture, +) -> None: + """ + Test the `query_context_modified` function when ordering by an unrelated column. + """ + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "metrics": ["count"], + "groupby": ["deal_size"], + } + + query_context.form_data = { + "slice_id": 42, + "metrics": ["count"], + "columns": ["deal_size"], + "orderby": [["credit_limit", False]], + } + query_context.queries = [ + QueryObject( + metrics=["count"], + columns=["deal_size"], + orderby=[("credit_limit", False)], + ) + ] + assert query_context_modified(query_context) + + +def test_query_context_modified_scalar_groupby(mocker: MockerFixture) -> None: + """ + Test the `query_context_modified` function for a scalar-valued `groupby`. + + Heatmap stores its group-by as a bare string rather than a list; it must not be + compared character by character. + """ + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "metric": "count", + "x_axis": "product_line", + "groupby": "deal_size", + } + + query_context.form_data = { + "slice_id": 42, + "metric": "count", + "x_axis": "product_line", + "groupby": "deal_size", + "metrics": ["count"], + } + query_context.queries = [ + QueryObject(metrics=["count"], columns=["product_line", "deal_size"]) + ] + assert not query_context_modified(query_context) + + +def test_query_context_modified_column_reference(mocker: MockerFixture) -> None: + """ + Test the `query_context_modified` function for an adhoc column reference. + + Box plot sends its saved temporal column as an adhoc column whose `sqlExpression` is + its own label, which stands for the physical column stored in `params`. + """ + column_reference: dict[str, Any] = { + "expressionType": "SQL", + "label": "order_date", + "sqlExpression": "order_date", + } + + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "metrics": ["count"], + "columns": ["order_date"], + "groupby": ["product_line"], + } + + query_context.form_data = { + "slice_id": 42, + "metrics": ["count"], + "columns": ["order_date"], + "groupby": ["product_line"], + } + query_context.queries = [ + QueryObject( + metrics=["count"], + columns=[column_reference, "product_line"], # type: ignore + ) + ] + assert not query_context_modified(query_context) + + +def test_query_context_modified_column_reference_tampered( + mocker: MockerFixture, +) -> None: + """ + Test the `query_context_modified` function when an adhoc column wraps free-form SQL. + + A column reference only collapses to the name it points at, so SQL disguised as a + label must still be rejected. + """ + disguised: dict[str, Any] = { + "expressionType": "SQL", + "label": "UPPER(secret)", + "sqlExpression": "UPPER(secret)", + } + + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "metrics": ["count"], + "columns": ["order_date"], + } + + query_context.form_data = { + "slice_id": 42, + "metrics": ["count"], + "columns": ["order_date"], + } + query_context.queries = [ + QueryObject(metrics=["count"], columns=[disguised]) # type: ignore # type: ignore + ] + assert query_context_modified(query_context) + + +def test_query_context_modified_pivot_table_groupby(mocker: MockerFixture) -> None: + """ + Test the `query_context_modified` function for a pivot table. + + Pivot table splits its group-bys across `groupbyColumns` and `groupbyRows`. + """ + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "metrics": ["count"], + "groupbyColumns": ["product_line"], + "groupbyRows": ["country", "city"], + } + + query_context.form_data = { + "slice_id": 42, + "metrics": ["count"], + "groupbyColumns": ["product_line"], + "groupbyRows": ["country", "city"], + } + query_context.queries = [ + QueryObject( + metrics=["count"], + columns=["product_line", "country", "city"], + ) + ] + assert not query_context_modified(query_context) + + +def test_query_context_modified_order_by_cols(mocker: MockerFixture) -> None: + """ + Test the `query_context_modified` function for `order_by_cols`. + + Entries are JSON-encoded `[column, is_ascending]` pairs, and the column they sort on + is read by the query. + """ + query_context = mocker.MagicMock() + query_context.slice_.id = 42 + query_context.slice_.query_context = None + query_context.slice_.params_dict = { + "start_time": "start_time", + "end_time": "end_time", + "series": "priority", + "order_by_cols": ['["status",false]'], + } + + query_context.form_data = { + "slice_id": 42, + "start_time": "start_time", + "end_time": "end_time", + "series": "priority", + "order_by_cols": ['["status",false]'], + } + query_context.queries = [ + QueryObject( + columns=["start_time", "end_time", "priority", "status"], + ) + ] + assert not query_context_modified(query_context)