You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jo...@apache.org on 2023/08/22 16:58:52 UTC
[superset] branch master updated: fix: Native filter dashboard RBAC aware dataset permission (#25029)
This is an automated email from the ASF dual-hosted git repository.
johnbodley pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 60889d27ed fix: Native filter dashboard RBAC aware dataset permission (#25029)
60889d27ed is described below
commit 60889d27edeeb306cff763743254ca0655faf4b5
Author: John Bodley <45...@users.noreply.github.com>
AuthorDate: Tue Aug 22 09:58:43 2023 -0700
fix: Native filter dashboard RBAC aware dataset permission (#25029)
---
.../dashboard/components/nativeFilters/utils.ts | 2 +
superset/security/manager.py | 35 +++++++++---
tests/integration_tests/security_tests.py | 63 ++++++++++++++++++++--
3 files changed, 91 insertions(+), 9 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
index b4284a8a63..7194e2435f 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
@@ -55,6 +55,7 @@ export const getFormData = ({
granularity_sqla,
type,
dashboardId,
+ id,
}: Partial<Filter> & {
dashboardId: number;
datasetId?: number;
@@ -94,6 +95,7 @@ export const getFormData = ({
viz_type: filterType,
type,
dashboardId,
+ native_filter_id: id,
};
};
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 2a3bec531d..128d0dda8c 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -16,6 +16,7 @@
# under the License.
# pylint: disable=too-many-lines
"""A set of constants and methods to manage permissions and security"""
+import json
import logging
import re
import time
@@ -1872,14 +1873,36 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
.one_or_none()
)
and dashboard_.roles
- and (slice_id := form_data.get("slice_id"))
and (
- slc := self.get_session.query(Slice)
- .filter(Slice.id == slice_id)
- .one_or_none()
+ (
+ # Native filter.
+ form_data.get("type") == "NATIVE_FILTER"
+ and (native_filter_id := form_data.get("native_filter_id"))
+ and dashboard_.json_metadata
+ and (json_metadata := json.loads(dashboard_.json_metadata))
+ and any(
+ target.get("datasetId") == datasource.id
+ for fltr in json_metadata.get(
+ "native_filter_configuration",
+ [],
+ )
+ for target in fltr.get("targets", [])
+ if native_filter_id == fltr.get("id")
+ )
+ )
+ or (
+ # Chart.
+ form_data.get("type") != "NATIVE_FILTER"
+ and (slice_id := form_data.get("slice_id"))
+ and (
+ slc := self.get_session.query(Slice)
+ .filter(Slice.id == slice_id)
+ .one_or_none()
+ )
+ and slc in dashboard_.slices
+ and slc.datasource == datasource
+ )
)
- and slc in dashboard_.slices
- and slc.datasource == datasource
and self.can_access_dashboard(dashboard_)
)
):
diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py
index 4767e5af0e..f741ec4315 100644
--- a/tests/integration_tests/security_tests.py
+++ b/tests/integration_tests/security_tests.py
@@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.
# isort:skip_file
+import json
import inspect
import time
import unittest
@@ -1718,6 +1719,21 @@ class TestSecurityManager(SupersetTestCase):
world_health = self.get_dash_by_slug("world_health")
treemap = self.get_slice("Treemap", db.session, expunge_from_session=False)
+ births.json_metadata = json.dumps(
+ {
+ "native_filter_configuration": [
+ {
+ "id": "NATIVE_FILTER-ABCDEFGH",
+ "targets": [{"datasetId": birth_names.id}],
+ },
+ {
+ "id": "NATIVE_FILTER-IJKLMNOP",
+ "targets": [{"datasetId": treemap.id}],
+ },
+ ]
+ }
+ )
+
mock_g.user = security_manager.find_user("gamma")
mock_is_owner.return_value = False
mock_can_access.return_value = False
@@ -1725,7 +1741,6 @@ class TestSecurityManager(SupersetTestCase):
for kwarg in ["query_context", "viz"]:
births.roles = []
- db.session.flush()
# No dashboard roles.
with self.assertRaises(SupersetSecurityException):
@@ -1742,7 +1757,6 @@ class TestSecurityManager(SupersetTestCase):
)
births.roles = [self.get_role("Gamma")]
- db.session.flush()
# Undefined dashboard.
with self.assertRaises(SupersetSecurityException):
@@ -1807,7 +1821,50 @@ class TestSecurityManager(SupersetTestCase):
}
)
- db.session.rollback()
+ # Ill-defined native filter.
+ with self.assertRaises(SupersetSecurityException):
+ security_manager.raise_for_access(
+ **{
+ kwarg: Mock(
+ datasource=birth_names,
+ form_data={
+ "dashboardId": births.id,
+ "type": "NATIVE_FILTER",
+ },
+ )
+ }
+ )
+
+ # Native filter not associated with said datasource.
+ with self.assertRaises(SupersetSecurityException):
+ security_manager.raise_for_access(
+ **{
+ kwarg: Mock(
+ datasource=birth_names,
+ form_data={
+ "dashboardId": births.id,
+ "native_filter_id": "NATIVE_FILTER-IJKLMNOP",
+ "type": "NATIVE_FILTER",
+ },
+ )
+ }
+ )
+
+ # Native filter associated with said datasource.
+ security_manager.raise_for_access(
+ **{
+ kwarg: Mock(
+ datasource=birth_names,
+ form_data={
+ "dashboardId": births.id,
+ "native_filter_id": "NATIVE_FILTER-ABCDEFGH",
+ "type": "NATIVE_FILTER",
+ },
+ )
+ }
+ )
+
+ db.session.expunge_all()
@patch("superset.security.manager.g")
def test_get_user_roles(self, mock_g):