Skip to content

Commit

Permalink
fix: improve explore REST api validations (apache#27395)
Browse files Browse the repository at this point in the history
  • Loading branch information
dpgaspar authored Mar 5, 2024
1 parent f7c5d24 commit 9518509
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 3 deletions.
11 changes: 9 additions & 2 deletions superset/commands/explore/get.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from superset.exceptions import SupersetException
from superset.explore.exceptions import WrongEndpointError
from superset.explore.permalink.exceptions import ExplorePermalinkGetFailedError
from superset.extensions import security_manager
from superset.utils import core as utils
from superset.views.utils import (
get_datasource_info,
Expand All @@ -61,7 +62,6 @@ def __init__(
# pylint: disable=too-many-locals,too-many-branches,too-many-statements
def run(self) -> Optional[dict[str, Any]]:
initial_form_data = {}

if self._permalink_key is not None:
command = GetExplorePermalinkCommand(self._permalink_key)
permalink_value = command.run()
Expand Down Expand Up @@ -110,12 +110,19 @@ def run(self) -> Optional[dict[str, Any]]:
self._datasource_type = SqlaTable.type

datasource: Optional[BaseDatasource] = None

if self._datasource_id is not None:
with contextlib.suppress(DatasourceNotFound):
datasource = DatasourceDAO.get_datasource(
cast(str, self._datasource_type), self._datasource_id
)
datasource_name = datasource.name if datasource else _("[Missing Dataset]")

datasource_name = _("[Missing Dataset]")

if datasource:
datasource_name = datasource.name
security_manager.can_access_datasource(datasource)

viz_type = form_data.get("viz_type")
if not viz_type and datasource and datasource.default_endpoint:
raise WrongEndpointError(redirect=datasource.default_endpoint)
Expand Down
20 changes: 19 additions & 1 deletion tests/integration_tests/explore/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ def test_get_from_permalink_unknown_key(test_client, login_as_admin):


@patch("superset.security.SupersetSecurityManager.can_access_datasource")
def test_get_dataset_access_denied(
def test_get_dataset_access_denied_with_form_data_key(
mock_can_access_datasource, test_client, login_as_admin, dataset
):
message = "Dataset access denied"
Expand All @@ -214,6 +214,24 @@ def test_get_dataset_access_denied(
assert data["message"] == message


@patch("superset.security.SupersetSecurityManager.can_access_datasource")
def test_get_dataset_access_denied(
mock_can_access_datasource, test_client, login_as_admin, dataset
):
message = "Dataset access denied"
mock_can_access_datasource.side_effect = DatasetAccessDeniedError(
message=message, datasource_id=dataset.id, datasource_type=dataset.type
)
resp = test_client.get(
f"api/v1/explore/?datasource_id={dataset.id}&datasource_type={dataset.type}"
)
data = json.loads(resp.data.decode("utf-8"))
assert resp.status_code == 403
assert data["datasource_id"] == dataset.id
assert data["datasource_type"] == dataset.type
assert data["message"] == message


@patch("superset.daos.datasource.DatasourceDAO.get_datasource")
def test_wrong_endpoint(mock_get_datasource, test_client, login_as_admin, dataset):
dataset.default_endpoint = "another_endpoint"
Expand Down

0 comments on commit 9518509

Please sign in to comment.