From bbe4e016d8b0bdca247f46d84912f8e90454362a Mon Sep 17 00:00:00 2001 From: Vitor Avila <96086495+Vitor-Avila@users.noreply.github.com> Date: Mon, 31 Jul 2023 18:10:57 -0300 Subject: [PATCH] fix(embedded): adding logic to check dataset used by filters (#24808) (cherry picked from commit 7f9b0380e0e9f192402efda014ff39469881859b) --- superset/security/manager.py | 25 +++++++++- .../security/guest_token_security_tests.py | 46 +++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 28354ac18dafe..b4b66439760c4 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 @@ -2062,8 +2063,28 @@ def can_access_based_on_dashboard(datasource: "BaseDatasource") -> bool: .filter(Dashboard.id.in_(dashboard_ids)) ) - exists = db.session.query(query.exists()).scalar() - return exists + if db.session.query(query.exists()).scalar(): + return True + + # check for datasets that are only used by filters + dashboards_json = ( + db.session.query(Dashboard.json_metadata) + .filter(Dashboard.id.in_(dashboard_ids)) + .all() + ) + for json_ in dashboards_json: + try: + json_metadata = json.loads(json_.json_metadata) + for filter_ in json_metadata.get("native_filter_configuration", []): + filter_dataset_ids = [ + target.get("datasetId") for target in filter_.get("targets", []) + ] + if datasource.id in filter_dataset_ids: + return True + except ValueError: + pass + + return False @staticmethod def _get_current_epoch_time() -> float: diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py index e0517c9b28ae3..e5bf589184396 100644 --- a/tests/integration_tests/security/guest_token_security_tests.py +++ b/tests/integration_tests/security/guest_token_security_tests.py @@ -15,18 +15,21 @@ # specific language governing permissions and limitations # under the License. """Unit tests for Superset""" +import json from unittest import mock import pytest from flask import g from superset import db, security_manager +from superset.connectors.sqla.models import SqlaTable from superset.daos.dashboard import EmbeddedDashboardDAO from superset.dashboards.commands.exceptions import DashboardAccessDeniedError from superset.exceptions import SupersetSecurityException from superset.models.dashboard import Dashboard from superset.security.guest_token import GuestTokenResourceType from superset.sql_parse import Table +from superset.utils.database import get_example_database from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, @@ -233,3 +236,46 @@ def test_raise_for_dashboard_access_as_guest_no_rbac(self): db.session.delete(dash) db.session.commit() + + def test_can_access_datasource_used_in_dashboard_filter(self): + """ + Test that a user can access a datasource used only by a filter in a dashboard + they have access to. + """ + # Create a test dataset + test_dataset = SqlaTable( + database_id=get_example_database().id, + schema="main", + table_name="test_table_embedded_filter", + ) + db.session.add(test_dataset) + db.session.commit() + + # Create an embedabble dashboard with a filter powered by the test dataset + test_dashboard = Dashboard() + test_dashboard.dashboard_title = "Test Embedded Dashboard" + test_dashboard.json_metadata = json.dumps( + { + "native_filter_configuration": [ + {"targets": [{"datasetId": test_dataset.id}]} + ] + } + ) + test_dashboard.owners = [] + test_dashboard.slices = [] + test_dashboard.published = False + db.session.add(test_dashboard) + db.session.commit() + self.embedded = EmbeddedDashboardDAO.upsert(test_dashboard, []) + + # grant access to the dashboad + g.user = self.authorized_guest + g.user.resources = [{"type": "dashboard", "id": str(self.embedded.uuid)}] + g.user.roles = [security_manager.get_public_role()] + + # The user should have access to the datasource via the dashboard + security_manager.raise_for_access(datasource=test_dataset) + + db.session.delete(test_dashboard) + db.session.delete(test_dataset) + db.session.commit()