From ca30ca4cbc93e7b9898c387342f57dea4b42d01f Mon Sep 17 00:00:00 2001
From: John Bodley
Date: Wed, 4 Sep 2019 17:25:16 -0700
Subject: [PATCH] [metric] Adding security for restricted metrics
---
superset/security.py | 24 ++++++++++++++++++-
superset/views/api.py | 2 +-
superset/views/core.py | 16 ++++++-------
tests/security_tests.py | 53 ++++++++++++++++++++++++++++++++++++++++-
4 files changed, 84 insertions(+), 11 deletions(-)
diff --git a/superset/security.py b/superset/security.py
index 8c95f81471180..caff0aa9c1253 100644
--- a/superset/security.py
+++ b/superset/security.py
@@ -39,7 +39,9 @@
from superset.exceptions import SupersetSecurityException
if TYPE_CHECKING:
+ from superset.common.query_context import QueryContext
from superset.models.core import Database, BaseDatasource
+ from superset.viz import BaseViz
from superset.utils.core import DatasourceName # noqa: E402
@@ -783,7 +785,7 @@ def assert_datasource_permission(self, datasource: "BaseDatasource") -> None:
Assert the the user has permission to access the Superset datasource.
:param datasource: The Superset datasource
- :rasies SupersetSecurityException: If the user does not have permission
+ :raises SupersetSecurityException: If the user does not have permission
"""
if not self.datasource_access(datasource):
@@ -791,3 +793,23 @@ def assert_datasource_permission(self, datasource: "BaseDatasource") -> None:
self.get_datasource_access_error_msg(datasource),
self.get_datasource_access_link(datasource),
)
+
+ def assert_query_context_permission(self, query_context: "QueryContext") -> None:
+ """
+ Assert the the user has permission to access the query context.
+
+ :param query_context: The query context
+ :raises SupersetSecurityException: If the user does not have permission
+ """
+
+ self.assert_datasource_permission(query_context.datasource)
+
+ def assert_viz_permission(self, viz: "BaseViz") -> None:
+ """
+ Assert the the user has permission to access the visualization.
+
+ :param viz: The visualization
+ :raises SupersetSecurityException: If the user does not have permission
+ """
+
+ self.assert_datasource_permission(viz.datasource)
diff --git a/superset/views/api.py b/superset/views/api.py
index fed3bd4708420..6d962305a0a37 100644
--- a/superset/views/api.py
+++ b/superset/views/api.py
@@ -41,7 +41,7 @@ def query(self):
params: query_context: json_blob
"""
query_context = QueryContext(**json.loads(request.form.get("query_context")))
- security_manager.assert_datasource_permission(query_context.datasource)
+ security_manager.assert_query_context_permission(query_context)
payload_json = query_context.get_payload()
return json.dumps(
payload_json, default=utils.json_int_dttm_ser, ignore_nan=True
diff --git a/superset/views/core.py b/superset/views/core.py
index 188334e4ff7db..7057c5fe73140 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -168,7 +168,7 @@ def check_datasource_perms(
force=False,
)
- security_manager.assert_datasource_permission(viz_obj.datasource)
+ security_manager.assert_viz_permission(viz_obj)
def check_slice_perms(self, slice_id):
@@ -177,18 +177,18 @@ def check_slice_perms(self, slice_id):
This function takes `self` since it must have the same signature as the
the decorated method.
-
"""
+
form_data, slc = get_form_data(slice_id, use_slice_data=True)
- datasource_type = slc.datasource.type
- datasource_id = slc.datasource.id
+
viz_obj = get_viz(
- datasource_type=datasource_type,
- datasource_id=datasource_id,
+ datasource_type=slc.datasource.type,
+ datasource_id=slc.datasource.id,
form_data=form_data,
force=False,
)
- security_manager.assert_datasource_permission(viz_obj.datasource)
+
+ security_manager.assert_viz_permission(viz_obj)
def _deserialize_results_payload(
@@ -2933,7 +2933,7 @@ def slice_query(self, slice_id):
get the database query string for this slice
"""
viz_obj = get_viz(slice_id)
- security_manager.assert_datasource_permission(viz_obj.datasource)
+ security_manager.assert_viz_permission(viz_obj)
return self.get_query_string_response(viz_obj)
@api
diff --git a/tests/security_tests.py b/tests/security_tests.py
index f9c9ad90ebace..3a2cba25ccc08 100644
--- a/tests/security_tests.py
+++ b/tests/security_tests.py
@@ -16,8 +16,10 @@
# under the License.
import inspect
import unittest
+from unittest.mock import Mock, patch
-from superset import app, appbuilder, security_manager
+from superset import app, appbuilder, security_manager, viz
+from superset.exceptions import SupersetSecurityException
from .base_tests import SupersetTestCase
@@ -324,3 +326,52 @@ def test_views_are_secured(self):
if unsecured_views:
view_str = "\n".join([str(v) for v in unsecured_views])
raise Exception(f"Some views are not secured:\n{view_str}")
+
+
+class SecurityManagerTests(SupersetTestCase):
+ """
+ Testing the Security Manager.
+ """
+
+ @patch("superset.security.SupersetSecurityManager.datasource_access")
+ def test_assert_datasource_permission(self, mock_datasource_access):
+ datasource = self.get_datasource_mock()
+
+ # Datasource with the "datasource_access" permission.
+ mock_datasource_access.return_value = True
+ security_manager.assert_datasource_permission(datasource)
+
+ # Datasource without the "datasource_access" permission.
+ mock_datasource_access.return_value = False
+
+ with self.assertRaises(SupersetSecurityException):
+ security_manager.assert_datasource_permission(datasource)
+
+ @patch("superset.security.SupersetSecurityManager.datasource_access")
+ def test_assert_query_context_permission(self, mock_datasource_access):
+ query_context = Mock()
+ query_context.datasource = self.get_datasource_mock()
+
+ # Query context with the "datasource_access" permission.
+ mock_datasource_access.return_value = True
+ security_manager.assert_query_context_permission(query_context)
+
+ # Query context without the "datasource_access" permission.
+ mock_datasource_access.return_value = False
+
+ with self.assertRaises(SupersetSecurityException):
+ security_manager.assert_query_context_permission(query_context)
+
+ @patch("superset.security.SupersetSecurityManager.datasource_access")
+ def test_assert_viz_permission(self, mock_datasource_access):
+ test_viz = viz.TableViz(self.get_datasource_mock(), form_data={})
+
+ # Visualization with the "datasource_access" permission.
+ mock_datasource_access.return_value = True
+ security_manager.assert_viz_permission(test_viz)
+
+ # Visualization without the "datasource_access" permission.
+ mock_datasource_access.return_value = False
+
+ with self.assertRaises(SupersetSecurityException):
+ security_manager.assert_viz_permission(test_viz)