fix: Allow embedded guest user datasource access with dashboard context (#25081)

This commit is contained in:
Jack Fragassi 2023-08-28 09:48:21 -07:00 committed by GitHub
parent b240b795b5
commit 2b8d8da22a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 277 additions and 10 deletions

View File

@ -1882,17 +1882,23 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
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.

View File

@ -1653,6 +1653,7 @@ class FilterBoxViz(BaseViz):
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")

View File

@ -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,

View File

@ -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())

View File

@ -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 @@ class TestGuestUserSecurity(SupersetTestCase):
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 @@ class TestGuestUserSecurity(SupersetTestCase):
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 @@ class TestGuestUserDashboardAccess(SupersetTestCase):
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,
},
)
}
)

View File

@ -1649,7 +1649,7 @@ class TestSecurityManager(SupersetTestCase):
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)