diff --git a/UPDATING.md b/UPDATING.md index 054afafe2b890..47a5dd396e1ec 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -23,6 +23,8 @@ assists people when migrating to a new version. ## Next +* [10674](https://github.com/apache/incubator-superset/pull/10674): Breaking change: PUBLIC_ROLE_LIKE_GAMMA was removed is favour of the new PUBLIC_ROLE_LIKE so it can be set it whatever role you want. + * [10590](https://github.com/apache/incubator-superset/pull/10590): Breaking change: this PR will convert iframe chart into dashboard markdown component, and remove all `iframe`, `separator`, and `markup` slices (and support) from Superset. If you have important data in those slices, please backup manually. * [10562](https://github.com/apache/incubator-superset/pull/10562): EMAIL_REPORTS_WEBDRIVER is deprecated use WEBDRIVER_TYPE instead. diff --git a/superset/config.py b/superset/config.py index 0dfde99054ed4..ff136e462241a 100644 --- a/superset/config.py +++ b/superset/config.py @@ -260,10 +260,10 @@ def _try_json_readsha( # pylint: disable=unused-argument # --------------------------------------------------- # Roles config # --------------------------------------------------- -# Grant public role the same set of permissions as for the GAMMA role. +# Grant public role the same set of permissions as for a selected builtin role. # This is useful if one wants to enable anonymous users to view # dashboards. Explicit grant on specific datasets is still required. -PUBLIC_ROLE_LIKE_GAMMA = False +PUBLIC_ROLE_LIKE: Optional[str] = None # --------------------------------------------------- # Babel config for translations diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index e4065aeeea15d..601a69ddb63dd 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -585,7 +585,7 @@ def get_schema_perm(self) -> Optional[str]: return security_manager.get_schema_perm(self.database, self.schema) def get_perm(self) -> str: - return ("[{obj.database}].[{obj.table_name}]" "(id:{obj.id})").format(obj=self) + return f"[{self.database}].[{self.table_name}](id:{self.id})" @property def name(self) -> str: diff --git a/superset/security/manager.py b/superset/security/manager.py index f939d6d258eb8..f5376d8f6d215 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -17,6 +17,7 @@ # pylint: disable=too-few-public-methods """A set of constants and methods to manage permissions and security""" import logging +import re from typing import Any, Callable, cast, List, Optional, Set, Tuple, TYPE_CHECKING, Union from flask import current_app, g @@ -172,6 +173,15 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods ACCESSIBLE_PERMS = {"can_userinfo"} + data_access_permissions = ( + "database_access", + "schema_access", + "datasource_access", + "all_datasource_access", + "all_database_access", + "all_query_access", + ) + def get_schema_perm( # pylint: disable=no-self-use self, database: Union["Database", str], schema: Optional[str] = None ) -> Optional[str]: @@ -609,8 +619,15 @@ def sync_role_definitions(self) -> None: self.set_role("granter", self._is_granter_pvm) self.set_role("sql_lab", self._is_sql_lab_pvm) + # Configure public role + if conf["PUBLIC_ROLE_LIKE"]: + self.copy_role(conf["PUBLIC_ROLE_LIKE"], self.auth_role_public, merge=True) if conf.get("PUBLIC_ROLE_LIKE_GAMMA", False): - self.set_role("Public", self._is_gamma_pvm) + logger.warning( + "The config `PUBLIC_ROLE_LIKE_GAMMA` is deprecated and will be removed " + "in Superset 1.0. Please use `PUBLIC_ROLE_LIKE ` instead." + ) + self.copy_role("Gamma", self.auth_role_public, merge=True) self.create_missing_perms() @@ -618,6 +635,58 @@ def sync_role_definitions(self) -> None: self.get_session.commit() self.clean_perms() + def _get_pvms_from_builtin_role(self, role_name: str) -> List[PermissionView]: + """ + Gets a list of model PermissionView permissions infered from a builtin role + definition + """ + role_from_permissions_names = self.builtin_roles.get(role_name, []) + all_pvms = self.get_session.query(PermissionView).all() + role_from_permissions = [] + for pvm_regex in role_from_permissions_names: + view_name_regex = pvm_regex[0] + permission_name_regex = pvm_regex[1] + for pvm in all_pvms: + if re.match(view_name_regex, pvm.view_menu.name) and re.match( + permission_name_regex, pvm.permission.name + ): + if pvm not in role_from_permissions: + role_from_permissions.append(pvm) + return role_from_permissions + + def copy_role( + self, role_from_name: str, role_to_name: str, merge: bool = True + ) -> None: + """ + Copies permissions from a role to another. + + Note: Supports regex defined builtin roles + + :param role_from_name: The FAB role name from where the permissions are taken + :param role_to_name: The FAB role name from where the permissions are copied to + :param merge: If merge is true, keep data access permissions + if they already exist on the target role + """ + + logger.info("Copy/Merge %s to %s", role_from_name, role_to_name) + # If it's a builtin role extract permissions from it + if role_from_name in self.builtin_roles: + role_from_permissions = self._get_pvms_from_builtin_role(role_from_name) + else: + role_from_permissions = list(self.find_role(role_from_name).permissions) + role_to = self.add_role(role_to_name) + # If merge, recover existing data access permissions + if merge: + for permission_view in role_to.permissions: + if ( + permission_view not in role_from_permissions + and permission_view.permission.name in self.data_access_permissions + ): + role_from_permissions.append(permission_view) + role_to.permissions = role_from_permissions + self.get_session.merge(role_to) + self.get_session.commit() + def set_role( self, role_name: str, pvm_check: Callable[[PermissionView], bool] ) -> None: @@ -629,14 +698,15 @@ def set_role( """ logger.info("Syncing %s perms", role_name) - sesh = self.get_session - pvms = sesh.query(PermissionView).all() + pvms = self.get_session.query(PermissionView).all() pvms = [p for p in pvms if p.permission and p.view_menu] role = self.add_role(role_name) - role_pvms = [p for p in pvms if pvm_check(p)] + role_pvms = [ + permission_view for permission_view in pvms if pvm_check(permission_view) + ] role.permissions = role_pvms - sesh.merge(role) - sesh.commit() + self.get_session.merge(role) + self.get_session.commit() def _is_admin_only(self, pvm: Model) -> bool: """ diff --git a/superset/templates/superset/public_welcome.html b/superset/templates/superset/public_welcome.html new file mode 100644 index 0000000000000..0d821512eb782 --- /dev/null +++ b/superset/templates/superset/public_welcome.html @@ -0,0 +1,23 @@ +{# +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +#} +{% extends "appbuilder/base.html" %} + +{% block content %} +

Welcome to Apache Superset

+{% endblock %} diff --git a/superset/views/core.py b/superset/views/core.py index d6ee6907fe7c7..243b9c63ef37f 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2523,6 +2523,8 @@ def show_traceback(self) -> FlaskResponse: # pylint: disable=no-self-use def welcome(self) -> FlaskResponse: """Personalized welcome page""" if not g.user or not g.user.get_id(): + if conf.get("PUBLIC_ROLE_LIKE_GAMMA", False) or conf["PUBLIC_ROLE_LIKE"]: + return self.render_template("superset/public_welcome.html") return redirect(appbuilder.get_url_for_login) welcome_dashboard_id = ( diff --git a/tests/dashboard_tests.py b/tests/dashboard_tests.py index f05c98082bb67..27926e4f6a350 100644 --- a/tests/dashboard_tests.py +++ b/tests/dashboard_tests.py @@ -320,6 +320,9 @@ def test_public_user_dashboard_access(self): resp = self.get_resp("/api/v1/dashboard/") self.assertNotIn("/superset/dashboard/world_health/", resp) + # Cleanup + self.revoke_public_access_to_table(table) + def test_dashboard_with_created_by_can_be_accessed_by_public_users(self): self.logout() table = db.session.query(SqlaTable).filter_by(table_name="birth_names").one() @@ -332,6 +335,8 @@ def test_dashboard_with_created_by_can_be_accessed_by_public_users(self): db.session.commit() assert "Births" in self.get_resp("/superset/dashboard/births/") + # Cleanup + self.revoke_public_access_to_table(table) def test_only_owners_can_save(self): dash = db.session.query(Dashboard).filter_by(slug="births").first() @@ -400,6 +405,9 @@ def test_users_can_view_published_dashboard(self): self.assertNotIn(f"/superset/dashboard/{hidden_dash_slug}/", resp) self.assertIn(f"/superset/dashboard/{published_dash_slug}/", resp) + # Cleanup + self.revoke_public_access_to_table(table) + def test_users_can_view_own_dashboard(self): user = security_manager.find_user("gamma") my_dash_slug = f"my_dash_{random()}" diff --git a/tests/security_tests.py b/tests/security_tests.py index 60d20fde09dc7..fa8782295d98d 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -20,7 +20,7 @@ from unittest.mock import Mock, patch import prison -from flask import g +from flask import current_app, g import tests.test_app from superset import app, appbuilder, db, security_manager, viz @@ -531,6 +531,52 @@ def test_gamma_user_schema_access_to_charts(self): ) # wb_health_population slice, has access self.assertNotIn("Girl Name Cloud", data) # birth_names slice, no access + def test_public_sync_role_data_perms(self): + """ + Security: Tests if the sync role method preserves data access permissions + if they already exist on a public role. + Also check that non data access permissions are removed + """ + table = db.session.query(SqlaTable).filter_by(table_name="birth_names").one() + self.grant_public_access_to_table(table) + public_role = security_manager.get_public_role() + unwanted_pvm = security_manager.find_permission_view_menu( + "menu_access", "Security" + ) + public_role.permissions.append(unwanted_pvm) + db.session.commit() + + security_manager.sync_role_definitions() + public_role = security_manager.get_public_role() + public_role_resource_names = [ + permission.view_menu.name for permission in public_role.permissions + ] + + assert table.get_perm() in public_role_resource_names + assert "Security" not in public_role_resource_names + + # Cleanup + self.revoke_public_access_to_table(table) + + def test_public_sync_role_builtin_perms(self): + """ + Security: Tests public role creation based on a builtin role + """ + current_app.config["PUBLIC_ROLE_LIKE"] = "TestRole" + + security_manager.sync_role_definitions() + public_role = security_manager.get_public_role() + public_role_resource_names = [ + [permission.view_menu.name, permission.permission.name] + for permission in public_role.permissions + ] + for pvm in current_app.config["FAB_ROLES"]["TestRole"]: + assert pvm in public_role_resource_names + + # Cleanup + current_app.config["PUBLIC_ROLE_LIKE"] = "Gamma" + security_manager.sync_role_definitions() + def test_sqllab_gamma_user_schema_access_to_sqllab(self): session = db.session @@ -724,7 +770,10 @@ def test_is_gamma_pvm(self): def test_gamma_permissions_basic(self): self.assert_can_gamma(get_perm_tuples("Gamma")) - self.assert_cannot_alpha(get_perm_tuples("Alpha")) + self.assert_cannot_alpha(get_perm_tuples("Gamma")) + + def test_public_permissions_basic(self): + self.assert_can_gamma(get_perm_tuples("Public")) @unittest.skipUnless( SupersetTestCase.is_module_installed("pydruid"), "pydruid not installed" @@ -788,6 +837,9 @@ def assert_can_all(view_menu): assert_can_all("SliceModelView") assert_can_all("DashboardModelView") + assert_cannot_write("UserDBModelView") + assert_cannot_write("RoleModelView") + self.assertIn(("can_add_slices", "Superset"), gamma_perm_set) self.assertIn(("can_copy_dash", "Superset"), gamma_perm_set) self.assertIn(("can_created_dashboards", "Superset"), gamma_perm_set) diff --git a/tests/superset_test_config.py b/tests/superset_test_config.py index c88c5847bbb75..b62f67822fe6c 100644 --- a/tests/superset_test_config.py +++ b/tests/superset_test_config.py @@ -53,7 +53,10 @@ def GET_FEATURE_FLAGS_FUNC(ff): TESTING = True WTF_CSRF_ENABLED = False -PUBLIC_ROLE_LIKE_GAMMA = True + +FAB_ROLES = {"TestRole": [["Security", "menu_access"], ["List Users", "menu_access"]]} + +PUBLIC_ROLE_LIKE = "Gamma" AUTH_ROLE_PUBLIC = "Public" EMAIL_NOTIFICATIONS = False ENABLE_ROW_LEVEL_SECURITY = True diff --git a/tests/superset_test_config_thumbnails.py b/tests/superset_test_config_thumbnails.py index 3b97604e9ae03..af30af4e4d61f 100644 --- a/tests/superset_test_config_thumbnails.py +++ b/tests/superset_test_config_thumbnails.py @@ -50,7 +50,7 @@ def GET_FEATURE_FLAGS_FUNC(ff): TESTING = True WTF_CSRF_ENABLED = False -PUBLIC_ROLE_LIKE_GAMMA = True +PUBLIC_ROLE_LIKE = "Gamma" AUTH_ROLE_PUBLIC = "Public" EMAIL_NOTIFICATIONS = False