Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion superset/jinja_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)

This comment was marked as resolved.

Comment thread
Vitor-Avila marked this conversation as resolved.
Outdated
if not dataset:
raise DatasetNotFoundError(f"Dataset ID {dataset_id} not found.")

Expand Down
73 changes: 73 additions & 0 deletions tests/unit_tests/jinja_context_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Loading