mirror of
https://github.com/apache/superset.git
synced 2026-06-11 10:39:15 +00:00
Compare commits
2 Commits
fix/helm-r
...
fix/chart-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d544bff071 | ||
|
|
f79a88c685 |
@@ -103,6 +103,19 @@ class DatasourceTypeUpdateRequiredValidationError(ValidationError):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class ChartQueryContextDatasourceMismatchValidationError(ValidationError):
|
||||||
|
"""
|
||||||
|
Raised when a query-context-only update carries a datasource that does not
|
||||||
|
match the chart's own datasource.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def __init__(self) -> None:
|
||||||
|
super().__init__(
|
||||||
|
_("The query context datasource does not match the chart datasource"),
|
||||||
|
field_name="query_context",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class ChartNotFoundError(CommandException):
|
class ChartNotFoundError(CommandException):
|
||||||
message = "Chart not found."
|
message = "Chart not found."
|
||||||
|
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ from superset.commands.chart.exceptions import (
|
|||||||
ChartForbiddenError,
|
ChartForbiddenError,
|
||||||
ChartInvalidError,
|
ChartInvalidError,
|
||||||
ChartNotFoundError,
|
ChartNotFoundError,
|
||||||
|
ChartQueryContextDatasourceMismatchValidationError,
|
||||||
ChartUpdateFailedError,
|
ChartUpdateFailedError,
|
||||||
DashboardsForbiddenError,
|
DashboardsForbiddenError,
|
||||||
DashboardsNotFoundValidationError,
|
DashboardsNotFoundValidationError,
|
||||||
@@ -41,6 +42,7 @@ from superset.exceptions import SupersetSecurityException
|
|||||||
from superset.models.dashboard import Dashboard
|
from superset.models.dashboard import Dashboard
|
||||||
from superset.models.slice import Slice
|
from superset.models.slice import Slice
|
||||||
from superset.tags.models import ObjectType
|
from superset.tags.models import ObjectType
|
||||||
|
from superset.utils import json
|
||||||
from superset.utils.decorators import on_error, transaction
|
from superset.utils.decorators import on_error, transaction
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
@@ -101,6 +103,51 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
|
|||||||
if not security_manager.is_owner(dash):
|
if not security_manager.is_owner(dash):
|
||||||
raise DashboardsForbiddenError()
|
raise DashboardsForbiddenError()
|
||||||
|
|
||||||
|
def _validate_query_context_datasource(
|
||||||
|
self, exceptions: list[ValidationError]
|
||||||
|
) -> None:
|
||||||
|
"""
|
||||||
|
Ensure a query-context-only update keeps the chart's own datasource.
|
||||||
|
|
||||||
|
The submitted query context is only verified when it carries a parseable
|
||||||
|
``datasource`` object; a payload that references a different datasource than
|
||||||
|
the chart's persisted one is rejected. Payloads without a datasource fall
|
||||||
|
back to the chart's datasource at execution time and need no check.
|
||||||
|
"""
|
||||||
|
if not self._model:
|
||||||
|
return
|
||||||
|
|
||||||
|
raw_query_context = self._properties.get("query_context")
|
||||||
|
if not raw_query_context:
|
||||||
|
return
|
||||||
|
|
||||||
|
try:
|
||||||
|
query_context = json.loads(raw_query_context)
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
# An unparseable payload cannot be verified or replayed; leave it for
|
||||||
|
# downstream handling rather than guessing at its intent.
|
||||||
|
return
|
||||||
|
|
||||||
|
datasource = (
|
||||||
|
query_context.get("datasource") if isinstance(query_context, dict) else None
|
||||||
|
)
|
||||||
|
if not isinstance(datasource, dict):
|
||||||
|
return
|
||||||
|
|
||||||
|
try:
|
||||||
|
ids_match = int(datasource["id"]) == self._model.datasource_id
|
||||||
|
except (KeyError, TypeError, ValueError):
|
||||||
|
ids_match = False
|
||||||
|
|
||||||
|
datasource_type = datasource.get("type")
|
||||||
|
types_match = (
|
||||||
|
datasource_type is None
|
||||||
|
or str(datasource_type) == self._model.datasource_type
|
||||||
|
)
|
||||||
|
|
||||||
|
if not ids_match or not types_match:
|
||||||
|
exceptions.append(ChartQueryContextDatasourceMismatchValidationError())
|
||||||
|
|
||||||
def validate(self) -> None: # noqa: C901
|
def validate(self) -> None: # noqa: C901
|
||||||
exceptions: list[ValidationError] = []
|
exceptions: list[ValidationError] = []
|
||||||
dashboard_ids = self._properties.get("dashboards")
|
dashboard_ids = self._properties.get("dashboards")
|
||||||
@@ -134,6 +181,12 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
|
|||||||
raise ChartForbiddenError() from ex
|
raise ChartForbiddenError() from ex
|
||||||
except ValidationError as ex:
|
except ValidationError as ex:
|
||||||
exceptions.append(ex)
|
exceptions.append(ex)
|
||||||
|
else:
|
||||||
|
# The query-context-only path skips the ownership check so report and
|
||||||
|
# alert workers can refresh a chart's cached payload. Keep that payload
|
||||||
|
# bound to the chart's own datasource so it cannot be repointed at an
|
||||||
|
# unrelated one.
|
||||||
|
self._validate_query_context_datasource(exceptions)
|
||||||
|
|
||||||
# validate tags
|
# validate tags
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -17,10 +17,11 @@
|
|||||||
import pytest
|
import pytest
|
||||||
from pytest_mock import MockerFixture
|
from pytest_mock import MockerFixture
|
||||||
|
|
||||||
from superset.commands.chart.exceptions import ChartForbiddenError
|
from superset.commands.chart.exceptions import ChartForbiddenError, ChartInvalidError
|
||||||
from superset.commands.chart.update import UpdateChartCommand
|
from superset.commands.chart.update import UpdateChartCommand
|
||||||
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
|
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
|
||||||
from superset.exceptions import SupersetSecurityException
|
from superset.exceptions import SupersetSecurityException
|
||||||
|
from superset.utils import json
|
||||||
|
|
||||||
|
|
||||||
def _ownership_exc() -> SupersetSecurityException:
|
def _ownership_exc() -> SupersetSecurityException:
|
||||||
@@ -91,3 +92,73 @@ def test_update_chart_owner_can_perform_regular_update(
|
|||||||
|
|
||||||
find_by_id.assert_called_once_with(1)
|
find_by_id.assert_called_once_with(1)
|
||||||
raise_for_ownership.assert_called_once()
|
raise_for_ownership.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def _query_context_payload(datasource: object) -> dict[str, object]:
|
||||||
|
return {
|
||||||
|
"query_context": json.dumps({"datasource": datasource, "queries": []}),
|
||||||
|
"query_context_generation": True,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_chart_query_context_matching_datasource_is_allowed(
|
||||||
|
mocker: MockerFixture,
|
||||||
|
) -> None:
|
||||||
|
"""A query context that targets the chart's own datasource is accepted."""
|
||||||
|
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
|
||||||
|
find_by_id.return_value = mocker.MagicMock(
|
||||||
|
id=1, tags=[], dashboards=[], datasource_id=42, datasource_type="table"
|
||||||
|
)
|
||||||
|
mocker.patch("superset.commands.chart.update.security_manager.raise_for_ownership")
|
||||||
|
|
||||||
|
UpdateChartCommand(
|
||||||
|
1, _query_context_payload({"id": 42, "type": "table"})
|
||||||
|
).validate()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"datasource",
|
||||||
|
[
|
||||||
|
{"id": 99, "type": "table"}, # different id
|
||||||
|
{"id": 42, "type": "query"}, # different type
|
||||||
|
{"id": "99", "type": "table"}, # different id as string
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_update_chart_query_context_mismatched_datasource_is_rejected(
|
||||||
|
mocker: MockerFixture,
|
||||||
|
datasource: dict[str, object],
|
||||||
|
) -> None:
|
||||||
|
"""A query context pointing at a different datasource is rejected with a 4xx."""
|
||||||
|
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
|
||||||
|
find_by_id.return_value = mocker.MagicMock(
|
||||||
|
id=1, tags=[], dashboards=[], datasource_id=42, datasource_type="table"
|
||||||
|
)
|
||||||
|
mocker.patch("superset.commands.chart.update.security_manager.raise_for_ownership")
|
||||||
|
|
||||||
|
with pytest.raises(ChartInvalidError):
|
||||||
|
UpdateChartCommand(1, _query_context_payload(datasource)).validate()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"query_context",
|
||||||
|
[
|
||||||
|
"{}", # no datasource key
|
||||||
|
'{"datasource": null}', # null datasource
|
||||||
|
"not-json", # unparseable payload
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_update_chart_query_context_without_datasource_is_allowed(
|
||||||
|
mocker: MockerFixture,
|
||||||
|
query_context: str,
|
||||||
|
) -> None:
|
||||||
|
"""Payloads with no verifiable datasource fall back to the chart's own."""
|
||||||
|
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
|
||||||
|
find_by_id.return_value = mocker.MagicMock(
|
||||||
|
id=1, tags=[], dashboards=[], datasource_id=42, datasource_type="table"
|
||||||
|
)
|
||||||
|
mocker.patch("superset.commands.chart.update.security_manager.raise_for_ownership")
|
||||||
|
|
||||||
|
UpdateChartCommand(
|
||||||
|
1,
|
||||||
|
{"query_context": query_context, "query_context_generation": True},
|
||||||
|
).validate()
|
||||||
|
|||||||
81
tests/unit_tests/utils/test_split.py
Normal file
81
tests/unit_tests/utils/test_split.py
Normal file
@@ -0,0 +1,81 @@
|
|||||||
|
# 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.
|
||||||
|
|
||||||
|
from superset.utils.core import split
|
||||||
|
|
||||||
|
|
||||||
|
def test_split_empty_string():
|
||||||
|
assert list(split("")) == [""]
|
||||||
|
|
||||||
|
|
||||||
|
def test_split_leading_delimiter():
|
||||||
|
assert list(split(" a")) == [
|
||||||
|
"",
|
||||||
|
"a",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_split_trailing_delimiter():
|
||||||
|
assert list(split("a ")) == [
|
||||||
|
"a",
|
||||||
|
"",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_split_only_delimiter():
|
||||||
|
assert list(split(" ")) == [
|
||||||
|
"",
|
||||||
|
"",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_split_nested_parentheses():
|
||||||
|
assert list(
|
||||||
|
split(
|
||||||
|
"a,(b,(c,d))",
|
||||||
|
delimiter=",",
|
||||||
|
)
|
||||||
|
) == [
|
||||||
|
"a",
|
||||||
|
"(b,(c,d))",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_branch_separator_found():
|
||||||
|
assert list(split("a b")) == [
|
||||||
|
"a",
|
||||||
|
"b",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_branch_separator_not_found():
|
||||||
|
assert list(split("ab")) == [
|
||||||
|
"ab",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_branch_parentheses():
|
||||||
|
assert list(split("(a b)")) == [
|
||||||
|
"(a b)",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_branch_escaped_quote():
|
||||||
|
assert list(split(r'"a\"b c" d')) == [
|
||||||
|
r'"a\"b c"',
|
||||||
|
"d",
|
||||||
|
]
|
||||||
Reference in New Issue
Block a user