diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx b/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx index e58c715d51df6..c1114e74d7475 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx @@ -399,7 +399,7 @@ class PropertiesModal extends React.PureComponent { />

{t( - 'Roles is a list which defines access to the dashboard. These roles are always applied in addition to restrictions on dataset level access. If no roles defined then the dashboard is available to all roles.', + 'Roles is a list which defines access to the dashboard. Granting a role access to a dashboard will bypass dataset level checks. If no roles defined then the dashboard is available to all roles.', )}

diff --git a/superset/security/manager.py b/superset/security/manager.py index 1c4419cfc5241..5fcc583032e0f 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -22,6 +22,7 @@ from flask import current_app, g from flask_appbuilder import Model +from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_appbuilder.security.sqla.manager import SecurityManager from flask_appbuilder.security.sqla.models import ( assoc_permissionview_role, @@ -61,7 +62,6 @@ from superset.sql_parse import Table from superset.viz import BaseViz - logger = logging.getLogger(__name__) @@ -923,7 +923,9 @@ def set_perm( # pylint: disable=no-self-use,unused-argument ) ) - def raise_for_access( # pylint: disable=too-many-arguments,too-many-branches + def raise_for_access( + # pylint: disable=too-many-arguments,too-many-branches, + # pylint: disable=too-many-locals self, database: Optional["Database"] = None, datasource: Optional["BaseDatasource"] = None, @@ -994,9 +996,15 @@ def raise_for_access( # pylint: disable=too-many-arguments,too-many-branches assert datasource + from superset.extensions import feature_flag_manager + if not ( self.can_access_schema(datasource) or self.can_access("datasource_access", datasource.perm or "") + or ( + feature_flag_manager.is_feature_enabled("DASHBOARD_RBAC") + and self.can_access_based_on_dashboard(datasource) + ) ): raise SupersetSecurityException( self.get_datasource_access_error_object(datasource) @@ -1099,8 +1107,8 @@ def get_rls_ids(self, table: "BaseDatasource") -> List[int]: ids.sort() # Combinations rather than permutations return ids - # pylint: disable=no-self-use - def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None: + @staticmethod + def raise_for_dashboard_access(dashboard: "Dashboard") -> None: """ Raise an exception if the user cannot access the dashboard. @@ -1125,3 +1133,24 @@ def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None: if not can_access: raise DashboardAccessDeniedError() + + @staticmethod + def can_access_based_on_dashboard(datasource: "BaseDatasource") -> bool: + from superset import db + from superset.dashboards.filters import DashboardFilter + from superset.models.slice import Slice + from superset.models.dashboard import Dashboard + + datasource_class = type(datasource) + query = ( + db.session.query(datasource_class) + .join(Slice.table) + .filter(datasource_class.id == datasource.id) + ) + + query = DashboardFilter("id", SQLAInterface(Dashboard, db.session)).apply( + query, None + ) + + exists = db.session.query(query.exists()).scalar() + return exists diff --git a/superset/views/dashboard/mixin.py b/superset/views/dashboard/mixin.py index 273cfdf0b3c2f..9a5510728b4ca 100644 --- a/superset/views/dashboard/mixin.py +++ b/superset/views/dashboard/mixin.py @@ -65,8 +65,7 @@ class DashboardMixin: # pylint: disable=too-few-public-methods "owners": _("Owners is a list of users who can alter the dashboard."), "roles": _( "Roles is a list which defines access to the dashboard. " - "These roles are always applied in addition to restrictions on dataset " - "level access. " + "Granting a role access to a dashboard will bypass dataset level checks." "If no roles defined then the dashboard is available to all roles." ), "published": _( diff --git a/tests/dashboards/security/security_rbac_tests.py b/tests/dashboards/security/security_rbac_tests.py index 19885d9582320..42cab6c7d9979 100644 --- a/tests/dashboards/security/security_rbac_tests.py +++ b/tests/dashboards/security/security_rbac_tests.py @@ -27,7 +27,11 @@ create_datasource_table_to_db, create_slice_to_db, ) +from tests.fixtures.birth_names_dashboard import load_birth_names_dashboard_with_slices from tests.fixtures.public_role import public_role_like_gamma +from tests.fixtures.query_context import get_query_context + +CHART_DATA_URI = "api/v1/chart/data" @mock.patch.dict( @@ -65,16 +69,26 @@ def test_get_dashboard_view__owner_can_access(self): # assert self.assert_dashboard_view_response(response, dashboard_to_access) + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_get_dashboard_view__user_can_not_access_without_permission(self): username = random_str() new_role = f"role_{random_str()}" self.create_user_with_roles(username, [new_role], should_create_roles=True) - dashboard_to_access = create_dashboard_to_db(published=True) + slice = ( + db.session.query(Slice) + .filter_by(slice_name="Girl Name Cloud") + .one_or_none() + ) + dashboard_to_access = create_dashboard_to_db(published=True, slices=[slice]) self.login(username) # act response = self.get_dashboard_view_response(dashboard_to_access) + request_payload = get_query_context("birth_names") + rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data") + self.assertEqual(rv.status_code, 401) + # assert self.assert403(response) @@ -98,6 +112,7 @@ def test_get_dashboard_view__user_with_dashboard_permission_can_not_access_draft # post revoke_access_to_dashboard(dashboard_to_access, new_role) + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_get_dashboard_view__user_access_with_dashboard_permission(self): # arrange @@ -105,9 +120,12 @@ def test_get_dashboard_view__user_access_with_dashboard_permission(self): new_role = f"role_{random_str()}" self.create_user_with_roles(username, [new_role], should_create_roles=True) - dashboard_to_access = create_dashboard_to_db( - published=True, slices=[create_slice_to_db()] + slice = ( + db.session.query(Slice) + .filter_by(slice_name="Girl Name Cloud") + .one_or_none() ) + dashboard_to_access = create_dashboard_to_db(published=True, slices=[slice]) self.login(username) grant_access_to_dashboard(dashboard_to_access, new_role) @@ -117,6 +135,10 @@ def test_get_dashboard_view__user_access_with_dashboard_permission(self): # assert self.assert_dashboard_view_response(response, dashboard_to_access) + request_payload = get_query_context("birth_names") + rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data") + self.assertEqual(rv.status_code, 200) + # post revoke_access_to_dashboard(dashboard_to_access, new_role)