From e0d040c377b3d7252fe223bde37b7164e7a951c4 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Mon, 1 Jul 2019 11:55:25 -0700 Subject: [PATCH] [fix] Handling of non-existent datasource (#7755) --- superset/views/core.py | 46 +++++++++++++++++++++++++++++------------ superset/views/utils.py | 32 ++++++++++++++++++++++------ tests/core_tests.py | 8 +++++++ 3 files changed, 67 insertions(+), 19 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index cc0690bbce..a75eae41de 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -64,7 +64,7 @@ from superset import ( ) from superset.connectors.connector_registry import ConnectorRegistry from superset.connectors.sqla.models import AnnotationDatasource, SqlaTable -from superset.exceptions import SupersetException +from superset.exceptions import SupersetException, SupersetSecurityException from superset.forms import CsvToDatabaseForm from superset.jinja_context import get_template_processor from superset.legacy import update_time_range @@ -134,24 +134,36 @@ def is_owner(obj, user): return obj and user in obj.owners -def check_datasource_perms(self, datasource_type=None, datasource_id=None): +def check_datasource_perms( + self, datasource_type: str = None, datasource_id: int = None +) -> None: """ Check if user can access a cached response from explore_json. This function takes `self` since it must have the same signature as the the decorated method. + :param datasource_type: The datasource type, i.e., 'druid' or 'table' + :param datasource_id: The datasource ID + :raises SupersetSecurityException: If the user cannot access the resource """ + form_data = get_form_data()[0] - datasource_id, datasource_type = get_datasource_info( - datasource_id, datasource_type, form_data - ) + + try: + datasource_id, datasource_type = get_datasource_info( + datasource_id, datasource_type, form_data + ) + except SupersetException as e: + raise SupersetSecurityException(str(e)) + viz_obj = get_viz( datasource_type=datasource_type, datasource_id=datasource_id, form_data=form_data, force=False, ) + security_manager.assert_datasource_permission(viz_obj.datasource) @@ -1403,9 +1415,14 @@ class Superset(BaseSupersetView): force = request.args.get("force") == "true" form_data = get_form_data()[0] - datasource_id, datasource_type = get_datasource_info( - datasource_id, datasource_type, form_data - ) + + try: + datasource_id, datasource_type = get_datasource_info( + datasource_id, datasource_type, form_data + ) + except SupersetException as e: + return json_error_response(utils.error_msg_from_exception(e)) + viz_obj = get_viz( datasource_type=datasource_type, datasource_id=datasource_id, @@ -1449,12 +1466,15 @@ class Superset(BaseSupersetView): def explore(self, datasource_type=None, datasource_id=None): user_id = g.user.get_id() if g.user else None form_data, slc = get_form_data(use_slice_data=True) - - datasource_id, datasource_type = get_datasource_info( - datasource_id, datasource_type, form_data - ) - error_redirect = "/chart/list/" + + try: + datasource_id, datasource_type = get_datasource_info( + datasource_id, datasource_type, form_data + ) + except SupersetException: + return redirect(error_redirect) + datasource = ConnectorRegistry.get_datasource( datasource_type, datasource_id, db.session ) diff --git a/superset/views/utils.py b/superset/views/utils.py index 8ee0e1c8d8..97660c5cf7 100644 --- a/superset/views/utils.py +++ b/superset/views/utils.py @@ -16,7 +16,7 @@ # under the License. # pylint: disable=C,R,W from collections import defaultdict -from typing import Any, Dict, List +from typing import Any, Dict, List, Optional, Tuple from urllib import parse from flask import g, request @@ -25,6 +25,7 @@ import simplejson as json from superset import app, db, viz from superset.connectors.connector_registry import ConnectorRegistry +from superset.exceptions import SupersetException from superset.legacy import update_time_range import superset.models.core as models from superset.utils.core import QueryStatus @@ -144,20 +145,39 @@ def get_form_data(slice_id=None, use_slice_data=False): return form_data, slc -def get_datasource_info(datasource_id, datasource_type, form_data): - """Compatibility layer for handling of datasource info +def get_datasource_info( + datasource_id: Optional[int], + datasource_type: Optional[str], + form_data: Dict[str, Any], +) -> Tuple[int, Optional[str]]: + """ + Compatibility layer for handling of datasource info datasource_id & datasource_type used to be passed in the URL directory, now they should come as part of the form_data, - This function allows supporting both without duplicating code""" + + This function allows supporting both without duplicating code + + :param datasource_id: The datasource ID + :param datasource_type: The datasource type, i.e., 'druid' or 'table' + :param form_data: The URL form data + :returns: The datasource ID and type + :raises SupersetException: If the datasource no longer exists + """ + datasource = form_data.get("datasource", "") + if "__" in datasource: datasource_id, datasource_type = datasource.split("__") # The case where the datasource has been deleted - datasource_id = None if datasource_id == "None" else datasource_id + if datasource_id == "None": + datasource_id = None if not datasource_id: - raise Exception("The datasource associated with this chart no longer exists") + raise SupersetException( + "The datasource associated with this chart no longer exists" + ) + datasource_id = int(datasource_id) return datasource_id, datasource_type diff --git a/tests/core_tests.py b/tests/core_tests.py index e6b7cc1f5f..2fdd6ad8a7 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -727,6 +727,14 @@ class CoreTests(SupersetTestCase): self.assertEqual(data["status"], None) self.assertEqual(data["error"], None) + def test_slice_payload_no_datasource(self): + self.login(username="admin") + data = self.get_json_resp("/superset/explore_json/", raise_on_error=False) + + self.assertEqual( + data["error"], "The datasource associated with this chart no longer exists" + ) + @mock.patch("superset.security.SupersetSecurityManager.schemas_accessible_by_user") @mock.patch("superset.security.SupersetSecurityManager.database_access") @mock.patch("superset.security.SupersetSecurityManager.all_datasource_access")