You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by va...@apache.org on 2025/10/06 04:40:28 UTC

(superset) branch fix/metric-macro-embedded created (now 32039295f8)

This is an automated email from the ASF dual-hosted git repository.

vavila pushed a change to branch fix/metric-macro-embedded
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 32039295f8 fix: Support metric macro for  embedded users

This branch includes the following new commits:

     new 32039295f8 fix: Support metric macro for  embedded users

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(superset) 01/01: fix: Support metric macro for embedded users

Posted by va...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vavila pushed a commit to branch fix/metric-macro-embedded
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 32039295f84301882ff150ce3c04945170ca17ff
Author: Vitor Avila <vi...@preset.io>
AuthorDate: Mon Oct 6 01:39:31 2025 -0300

    fix: Support metric macro for  embedded users
---
 superset/jinja_context.py              |  8 +++-
 tests/unit_tests/jinja_context_test.py | 73 ++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/superset/jinja_context.py b/superset/jinja_context.py
index d43744fac2..f359e1521e 100644
--- a/superset/jinja_context.py
+++ b/superset/jinja_context.py
@@ -1086,7 +1086,13 @@ def metric_macro(
     if not dataset_id:
         dataset_id = get_dataset_id_from_context(metric_key)
 
-    dataset = DatasetDAO.find_by_id(dataset_id)
+    # Embedded user access is validated at the dashboard level, so we bypass
+    # the regular DAO filter for them
+    dataset = (
+        DatasetDAO.find_by_id(dataset_id)
+        if not security_manager.is_guest_user()
+        else DatasetDAO.find_by_id(dataset_id, skip_base_filter=True)
+    )
     if not dataset:
         raise DatasetNotFoundError(f"Dataset ID {dataset_id} not found.")
 
diff --git a/tests/unit_tests/jinja_context_test.py b/tests/unit_tests/jinja_context_test.py
index 30ada69681..42d4cfad96 100644
--- a/tests/unit_tests/jinja_context_test.py
+++ b/tests/unit_tests/jinja_context_test.py
@@ -1566,3 +1566,76 @@ def test_jinja2_server_error_handling(mocker: MockerFixture) -> None:
         assert "Internal Jinja2 template error" in str(exception)
         assert "MemoryError" in str(exception)
         assert "Out of memory" in str(exception)
+
+
+def test_metric_macro_regular_user_uses_base_filter(mocker: MockerFixture) -> None:
+    """
+    Test that the ``metric_macro`` uses base filter for regular users.
+
+    Regular users should have standard RBAC/RLS filters applied when accessing datasets.
+    """
+    mock_is_guest_user = mocker.patch("superset.security_manager.is_guest_user")
+    mock_is_guest_user.return_value = False
+
+    DatasetDAO = mocker.patch("superset.daos.dataset.DatasetDAO")  # noqa: N806
+    DatasetDAO.find_by_id.return_value = SqlaTable(
+        table_name="test_dataset",
+        metrics=[
+            SqlMetric(metric_name="count", expression="COUNT(*)"),
+        ],
+        database=Database(database_name="my_database", sqlalchemy_uri="sqlite://"),
+        schema="my_schema",
+        sql=None,
+    )
+
+    env = SandboxedEnvironment(undefined=DebugUndefined)
+    assert metric_macro(env, {}, "count", 1) == "COUNT(*)"
+
+    # Verify that find_by_id was called without skip_base_filter
+    DatasetDAO.find_by_id.assert_called_once_with(1)
+
+
+def test_metric_macro_regular_user_raises_no_access(mocker: MockerFixture) -> None:
+    """
+    Test that the ``metric_macro`` raises for regular user without dataset access.
+    """
+    mock_is_guest_user = mocker.patch("superset.security_manager.is_guest_user")
+    mock_is_guest_user.return_value = False
+
+    DatasetDAO = mocker.patch("superset.daos.dataset.DatasetDAO")  # noqa: N806
+    DatasetDAO.find_by_id.return_value = None
+
+    env = SandboxedEnvironment(undefined=DebugUndefined)
+    with pytest.raises(DatasetNotFoundError) as excinfo:
+        assert metric_macro(env, {}, "count", 1) == "COUNT(*)"
+
+    assert str(excinfo.value) == "Dataset ID 1 not found."
+    DatasetDAO.find_by_id.assert_called_once_with(1)
+
+
+def test_metric_macro_embedded_user_skips_base_filter(mocker: MockerFixture) -> None:
+    """
+    Test that the ``metric_macro`` skips base filter for embedded users.
+
+    Embedded users have dashboard-level access control via their embedding token,
+    so we bypass the regular dataset DAO filters for them.
+    """
+    mock_is_guest_user = mocker.patch("superset.security_manager.is_guest_user")
+    mock_is_guest_user.return_value = True
+
+    DatasetDAO = mocker.patch("superset.daos.dataset.DatasetDAO")  # noqa: N806
+    DatasetDAO.find_by_id.return_value = SqlaTable(
+        table_name="test_dataset",
+        metrics=[
+            SqlMetric(metric_name="count", expression="COUNT(*)"),
+        ],
+        database=Database(database_name="my_database", sqlalchemy_uri="sqlite://"),
+        schema="my_schema",
+        sql=None,
+    )
+
+    env = SandboxedEnvironment(undefined=DebugUndefined)
+    assert metric_macro(env, {}, "count", 1) == "COUNT(*)"
+
+    # Verify that find_by_id was called with skip_base_filter=True
+    DatasetDAO.find_by_id.assert_called_once_with(1, skip_base_filter=True)