fix: Allow embedded guest user datasource access with dashboard conte…

…xt (#25081)

(cherry picked from commit 2b8d8da)

superset/security/manager.py

@@ -1886,17 +1886,23 @@ def raise_for_access(
                 or self.is_owner(datasource)
                 or (
                     # Grant access to the datasource only if dashboard RBAC is enabled
+                    # or the user is an embedded guest user with access to the dashboard
                     # and said datasource is associated with the dashboard chart in
                     # question.
                     form_data
-                    and is_feature_enabled("DASHBOARD_RBAC")
                     and (dashboard_id := form_data.get("dashboardId"))
                     and (
                         dashboard_ := self.get_session.query(Dashboard)
                         .filter(Dashboard.id == dashboard_id)
                         .one_or_none()
                     )
-                    and dashboard_.roles
+                    and (
+                        (is_feature_enabled("DASHBOARD_RBAC") and dashboard_.roles)
+                        or (
+                            is_feature_enabled("EMBEDDED_SUPERSET")
+                            and self.is_guest_user()
+                        )
+                    )
                     and (
                         (
                             # Native filter.

superset/viz.py

@@ -1654,6 +1654,7 @@ def run_extra_queries(self) -> None:
                 query_obj["orderby"] = [(metric, asc)]
             self.get_query_context_factory().create(
                 datasource={"id": self.datasource.id, "type": self.datasource.type},
+                form_data=self.form_data,
                 queries=[query_obj],
             ).raise_for_access()
             df = self.get_df_payload(query_obj=query_obj).get("df")

tests/integration_tests/fixtures/birth_names_dashboard.py

@@ -59,6 +59,14 @@ def load_birth_names_dashboard_with_slices_module_scope(load_birth_names_data):
         _cleanup(dash_id_to_delete, slices_ids_to_delete)
 
 
+@pytest.fixture(scope="class")
+def load_birth_names_dashboard_with_slices_class_scope(load_birth_names_data):
+    with app.app_context():
+        dash_id_to_delete, slices_ids_to_delete = _create_dashboards()
+        yield
+        _cleanup(dash_id_to_delete, slices_ids_to_delete)
+
+
 def _create_dashboards():
     table = _create_table(
         table_name=BIRTH_NAMES_TBL_NAME,

tests/integration_tests/fixtures/world_bank_dashboard.py

@@ -84,6 +84,14 @@ def load_world_bank_dashboard_with_slices_module_scope(load_world_bank_data):
         _cleanup(dash_id_to_delete, slices_ids_to_delete)
 
 
+@pytest.fixture(scope="class")
+def load_world_bank_dashboard_with_slices_class_scope(load_world_bank_data):
+    with app.app_context():
+        dash_id_to_delete, slices_ids_to_delete = create_dashboard_for_loaded_data()
+        yield
+        _cleanup(dash_id_to_delete, slices_ids_to_delete)
+
+
 def create_dashboard_for_loaded_data():
     with app.app_context():
         table = create_table_metadata(WB_HEALTH_POPULATION, get_example_database())

tests/integration_tests/security/guest_token_security_tests.py

@@ -15,25 +15,34 @@
 # specific language governing permissions and limitations
 # under the License.
 """Unit tests for Superset"""
-from unittest import mock
+import json
+from unittest.mock import Mock, patch
 
 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.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.core import get_example_default_schema
+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,
+    load_birth_names_dashboard_with_slices_class_scope,
     load_birth_names_data,
 )
+from tests.integration_tests.fixtures.world_bank_dashboard import (
+    load_world_bank_dashboard_with_slices,
+    load_world_bank_dashboard_with_slices_class_scope,
+    load_world_bank_data,
+)
 
 
-@mock.patch.dict(
+@patch.dict(
     "superset.extensions.feature_flag_manager._feature_flags",
     EMBEDDED_SUPERSET=True,
 )
@@ -55,7 +64,7 @@ def test_is_guest_user__guest_user(self):
         is_guest = security_manager.is_guest_user(self.authorized_guest())
         self.assertTrue(is_guest)
 
-    @mock.patch.dict(
+    @patch.dict(
         "superset.extensions.feature_flag_manager._feature_flags",
         EMBEDDED_SUPERSET=False,
     )
@@ -91,14 +100,14 @@ def test_get_guest_user_roles_implicit(self):
         self.assertEqual(guest.roles, roles)
 
 
-@mock.patch.dict(
+@patch.dict(
     "superset.extensions.feature_flag_manager._feature_flags",
     EMBEDDED_SUPERSET=True,
 )
-@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices_class_scope")
 class TestGuestUserDashboardAccess(SupersetTestCase):
     def setUp(self) -> None:
-        self.dash = db.session.query(Dashboard).filter_by(slug="births").first()
+        self.dash = self.get_dash_by_slug("births")
         self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
         self.authorized_guest = security_manager.get_guest_user_from_token(
             {
@@ -195,3 +204,238 @@ def test_raise_for_access_dashboard_as_guest_no_rbac(self):
 
         db.session.delete(dash)
         db.session.commit()
+
+
+@patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags",
+    EMBEDDED_SUPERSET=True,
+)
+@pytest.mark.usefixtures(
+    "create_dataset",
+    "load_birth_names_dashboard_with_slices_class_scope",
+    "load_world_bank_dashboard_with_slices_class_scope",
+)
+class TestGuestUserDatasourceAccess(SupersetTestCase):
+    """
+    Guest users should only have access to datasources that are associated with a
+    dashboard they have access to, and only with that dashboard context present
+    """
+
+    @pytest.fixture(scope="class")
+    def create_dataset(self):
+        with self.create_app().app_context():
+            dataset = SqlaTable(
+                table_name="dummy_sql_table",
+                database=get_example_database(),
+                schema=get_example_default_schema(),
+                sql="select 123 as intcol, 'abc' as strcol",
+            )
+            session = db.session
+            session.add(dataset)
+            session.commit()
+
+            yield dataset
+
+            # rollback
+            session.delete(dataset)
+            session.commit()
+
+    def setUp(self) -> None:
+        self.dash = self.get_dash_by_slug("births")
+        self.other_dash = self.get_dash_by_slug("world_health")
+        self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+        self.authorized_guest = security_manager.get_guest_user_from_token(
+            {
+                "user": {},
+                "resources": [{"type": "dashboard", "id": str(self.embedded.uuid)}],
+            }
+        )
+        self.unauthorized_guest = security_manager.get_guest_user_from_token(
+            {
+                "user": {},
+                "resources": [
+                    {"type": "dashboard", "id": "06383667-3e02-4e5e-843f-44e9c5896b6c"}
+                ],
+            }
+        )
+        self.chart = self.get_slice("Girls", db.session, expunge_from_session=False)
+        self.datasource = self.chart.datasource
+        self.other_chart = self.get_slice(
+            "Treemap", db.session, expunge_from_session=False
+        )
+        self.other_datasource = self.other_chart.datasource
+        self.native_filter_datasource = (
+            db.session.query(SqlaTable).filter_by(table_name="dummy_sql_table").first()
+        )
+        self.dash.json_metadata = json.dumps(
+            {
+                "native_filter_configuration": [
+                    {
+                        "id": "NATIVE_FILTER-ABCDEFGH",
+                        "targets": [{"datasetId": self.native_filter_datasource.id}],
+                    },
+                ]
+            }
+        )
+
+    def test_raise_for_access__happy_path(self):
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            security_manager.raise_for_access(
+                **{
+                    kwarg: Mock(
+                        datasource=self.datasource,
+                        form_data={
+                            "dashboardId": self.dash.id,
+                            "slice_id": self.chart.id,
+                        },
+                    )
+                }
+            )
+
+    def test_raise_for_access__native_filter_happy_path(self):
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            security_manager.raise_for_access(
+                **{
+                    kwarg: Mock(
+                        datasource=self.native_filter_datasource,
+                        form_data={
+                            "dashboardId": self.dash.id,
+                            "native_filter_id": "NATIVE_FILTER-ABCDEFGH",
+                            "type": "NATIVE_FILTER",
+                        },
+                    )
+                }
+            )
+
+    def test_raise_for_access__no_dashboard_in_form_data(self):
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=self.datasource,
+                            form_data={
+                                "slice_id": self.chart.id,
+                            },
+                        )
+                    }
+                )
+
+    def test_raise_for_access__no_chart_in_form_data(self):
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=self.datasource,
+                            form_data={
+                                "dashboardId": self.dash.id,
+                            },
+                        )
+                    }
+                )
+
+    def test_raise_for_access__chart_not_on_dashboard(self):
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=self.other_datasource,
+                            form_data={
+                                "dashboardId": self.dash.id,
+                                "slice_id": self.other_chart.id,
+                            },
+                        )
+                    }
+                )
+
+    def test_raise_for_access__chart_doesnt_belong_to_datasource(self):
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=self.other_datasource,
+                            form_data={
+                                "dashboardId": self.dash.id,
+                                "slice_id": self.chart.id,
+                            },
+                        )
+                    }
+                )
+
+    def test_raise_for_access__native_filter_no_id_in_form_data(self):
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=self.native_filter_datasource,
+                            form_data={
+                                "dashboardId": self.dash.id,
+                                "type": "NATIVE_FILTER",
+                            },
+                        )
+                    }
+                )
+
+    def test_raise_for_access__native_filter_datasource_not_associated(self):
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=self.other_datasource,
+                            form_data={
+                                "dashboardId": self.dash.id,
+                                "native_filter_id": "NATIVE_FILTER-ABCDEFGH",
+                                "type": "NATIVE_FILTER",
+                            },
+                        )
+                    }
+                )
+
+    @patch.dict(
+        "superset.extensions.feature_flag_manager._feature_flags",
+        EMBEDDED_SUPERSET=False,
+    )
+    def test_raise_for_access__embedded_feature_flag_off(self):
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=self.datasource,
+                            form_data={
+                                "dashboardId": self.dash.id,
+                                "slice_id": self.chart.id,
+                            },
+                        )
+                    }
+                )
+
+    def test_raise_for_access__unauthorized_guest_user(self):
+        g.user = self.unauthorized_guest
+        for kwarg in ["viz", "query_context"]:
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=self.datasource,
+                            form_data={
+                                "dashboardId": self.dash.id,
+                                "slice_id": self.chart.id,
+                            },
+                        )
+                    }
+                )

tests/integration_tests/security_tests.py

@@ -1649,7 +1649,7 @@ def test_raise_for_access_query(self, mock_can_access, mock_is_owner):
     def test_raise_for_access_query_context(
         self, mock_can_access_schema, mock_can_access, mock_is_owner, mock_g
     ):
-        query_context = Mock(datasource=self.get_datasource_mock())
+        query_context = Mock(datasource=self.get_datasource_mock(), form_data={})
 
         mock_can_access_schema.return_value = True
         security_manager.raise_for_access(query_context=query_context)