This is an automated email from the ASF dual-hosted git repository. maximebeauchemin pushed a commit to branch main_db_not_needed in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
commit b57fe3a7fb3c1a51ef4edec112ef0875574de805 Author: Maxime Beauchemin <[email protected]> AuthorDate: Tue Aug 6 21:47:18 2019 -0700 prevent 'main' database connection creation --- superset/cli.py | 105 +++++++++++++++++-------------------------------- superset/security.py | 7 ++-- superset/views/core.py | 11 +----- tests/base_tests.py | 26 +++++++++--- tests/sqllab_tests.py | 21 ++++------ 5 files changed, 70 insertions(+), 100 deletions(-) diff --git a/superset/cli.py b/superset/cli.py index 5721220..48ae311 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -48,7 +48,6 @@ def make_shell_context(): @app.cli.command() def init(): """Inits the Superset application""" - utils.get_or_create_main_db() utils.get_example_database() appbuilder.add_permissions(update_perms=True) security_manager.sync_role_definitions() @@ -430,75 +429,43 @@ def load_test_users_run(): Syncs permissions for those users/roles """ if config.get("TESTING"): - security_manager.sync_role_definitions() - gamma_sqllab_role = security_manager.add_role("gamma_sqllab") - for perm in security_manager.find_role("Gamma").permissions: - security_manager.add_permission_role(gamma_sqllab_role, perm) - utils.get_or_create_main_db() - db_perm = utils.get_main_database().perm - security_manager.add_permission_view_menu("database_access", db_perm) - db_pvm = security_manager.find_permission_view_menu( - view_menu_name=db_perm, permission_name="database_access" + + sm = security_manager + + main_db = utils.get_main_database() + examples_db = utils.get_example_database() + + main_pv = sm.add_permission_view_menu("database_access", main_db.perm) + examples_pv = sm.add_permission_view_menu("database_access", examples_db.perm) + + sm.sync_role_definitions() + gamma_sqllab_role = sm.add_role("gamma_sqllab") + sm.add_permission_role(gamma_sqllab_role, examples_pv) + sm.add_permission_role(gamma_sqllab_role, main_pv) + + for role in ["Gamma", "sql_lab"]: + for perm in sm.find_role(role).permissions: + sm.add_permission_role(gamma_sqllab_role, perm) + + users = ( + ("admin", "Admin"), + ("gamma", "Gamma"), + ("gamma2", "Gamma"), + ("gamma_sqllab", "gamma_sqllab"), + ("alpha", "Alpha"), ) - gamma_sqllab_role.permissions.append(db_pvm) - for perm in security_manager.find_role("sql_lab").permissions: - security_manager.add_permission_role(gamma_sqllab_role, perm) - - admin = security_manager.find_user("admin") - if not admin: - security_manager.add_user( - "admin", - "admin", - " user", - "[email protected]", - security_manager.find_role("Admin"), - password="general", - ) - - gamma = security_manager.find_user("gamma") - if not gamma: - security_manager.add_user( - "gamma", - "gamma", - "user", - "[email protected]", - security_manager.find_role("Gamma"), - password="general", - ) - - gamma2 = security_manager.find_user("gamma2") - if not gamma2: - security_manager.add_user( - "gamma2", - "gamma2", - "user", - "[email protected]", - security_manager.find_role("Gamma"), - password="general", - ) - - gamma_sqllab_user = security_manager.find_user("gamma_sqllab") - if not gamma_sqllab_user: - security_manager.add_user( - "gamma_sqllab", - "gamma_sqllab", - "user", - "[email protected]", - gamma_sqllab_role, - password="general", - ) - - alpha = security_manager.find_user("alpha") - if not alpha: - security_manager.add_user( - "alpha", - "alpha", - "user", - "[email protected]", - security_manager.find_role("Alpha"), - password="general", - ) - security_manager.get_session.commit() + for username, role in users: + user = sm.find_user(username) + if not user: + sm.add_user( + username, + username, + "user", + username + "@fab.org", + sm.find_role(role), + password="general", + ) + sm.get_session.commit() @app.cli.command() diff --git a/superset/security.py b/superset/security.py index bea1571..918d7d1 100644 --- a/superset/security.py +++ b/superset/security.py @@ -200,7 +200,6 @@ class SupersetSecurityManager(SecurityManager): :param database: The Superset database :returns: Whether the user can access the Superset database """ - return ( self.all_datasource_access() or self.all_database_access() @@ -269,9 +268,9 @@ class SupersetSecurityManager(SecurityManager): :param tables: The list of denied SQL table names :returns: The error message """ - - return f"""You need access to the following tables: {", ".join(tables)}, all - database access or `all_datasource_access` permission""" + quoted_tables = [f"`{t}`" for t in tables] + return f"""You need access to the following tables: {", ".join(quoted_tables)}, + `all_database_access` or `all_datasource_access` permission""" def get_table_access_link(self, tables: List[str]) -> Optional[str]: """ diff --git a/superset/views/core.py b/superset/views/core.py index fab1800..7c94826 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2446,10 +2446,7 @@ class Superset(BaseSupersetView): ) if rejected_tables: return json_error_response( - security_manager.get_table_access_error_msg( - "{}".format(rejected_tables) - ), - status=403, + security_manager.get_table_access_error_msg(rejected_tables), status=403 ) payload = utils.zlib_decompress(blob, decode=not results_backend_use_msgpack) @@ -2691,11 +2688,7 @@ class Superset(BaseSupersetView): query.sql, query.database, query.schema ) if rejected_tables: - flash( - security_manager.get_table_access_error_msg( - "{}".format(rejected_tables) - ) - ) + flash(security_manager.get_table_access_error_msg(rejected_tables)) return redirect("/") blob = None if results_backend and query.results_key: diff --git a/tests/base_tests.py b/tests/base_tests.py index 8ac5bcd..ae046d0 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -28,7 +28,7 @@ from superset.connectors.druid.models import DruidCluster, DruidDatasource from superset.connectors.sqla.models import SqlaTable from superset.models import core as models from superset.models.core import Database -from superset.utils.core import get_main_database +from superset.utils.core import get_example_database, get_main_database BASE_DIR = app.config.get("BASE_DIR") @@ -168,6 +168,14 @@ class SupersetTestCase(unittest.TestCase): ): security_manager.del_permission_role(public_role, perm) + def _get_database_by_name(self, database_name="main"): + if database_name == "main": + return get_main_database() + elif database_name == "examples": + return get_example_database() + else: + raise ValueError("Database doesn't exist") + def run_sql( self, sql, @@ -175,11 +183,12 @@ class SupersetTestCase(unittest.TestCase): user_name=None, raise_on_error=False, query_limit=None, + database_name="main", ): if user_name: self.logout() - self.login(username=(user_name if user_name else "admin")) - dbid = get_main_database().id + self.login(username=(user_name or "admin")) + dbid = self._get_database_by_name(database_name).id resp = self.get_json_resp( "/superset/sql_json/", raise_on_error=False, @@ -195,11 +204,18 @@ class SupersetTestCase(unittest.TestCase): raise Exception("run_sql failed") return resp - def validate_sql(self, sql, client_id=None, user_name=None, raise_on_error=False): + def validate_sql( + self, + sql, + client_id=None, + user_name=None, + raise_on_error=False, + database_name="main", + ): if user_name: self.logout() self.login(username=(user_name if user_name else "admin")) - dbid = get_main_database().id + dbid = self._get_database_by_name(database_name).id resp = self.get_json_resp( "/superset/validate_sql_json/", raise_on_error=False, diff --git a/tests/sqllab_tests.py b/tests/sqllab_tests.py index 1774c26..35a7d96 100644 --- a/tests/sqllab_tests.py +++ b/tests/sqllab_tests.py @@ -19,7 +19,6 @@ from datetime import datetime, timedelta import json import unittest -from flask_appbuilder.security.sqla import models as ab_models import prison from superset import db, security_manager @@ -85,16 +84,10 @@ class SqlLabTests(SupersetTestCase): def test_sql_json_has_access(self): main_db = get_main_database() - security_manager.add_permission_view_menu("database_access", main_db.perm) - db.session.commit() - main_db_permission_view = ( - db.session.query(ab_models.PermissionView) - .join(ab_models.ViewMenu) - .join(ab_models.Permission) - .filter(ab_models.ViewMenu.name == "[main].(id:1)") - .filter(ab_models.Permission.name == "database_access") - .first() + main_db_permission_view = security_manager.add_permission_view_menu( + "database_access", main_db.perm ) + astronaut = security_manager.add_role("Astronaut") security_manager.add_permission_role(astronaut, main_db_permission_view) # Astronaut role is Gamma + sqllab + main db permissions @@ -160,11 +153,13 @@ class SqlLabTests(SupersetTestCase): def test_search_query_on_db_id(self): self.run_some_queries() self.login("admin") + main_dbid = get_main_database().id + # Test search queries on database Id - data = self.get_json_resp("/superset/search_queries?database_id=1") + data = self.get_json_resp(f"/superset/search_queries?database_id={main_dbid}") self.assertEquals(3, len(data)) db_ids = [k["dbId"] for k in data] - self.assertEquals([1, 1, 1], db_ids) + self.assertEquals([main_dbid for i in range(3)], db_ids) resp = self.get_resp("/superset/search_queries?database_id=-1") data = json.loads(resp) @@ -281,7 +276,7 @@ class SqlLabTests(SupersetTestCase): def test_df_conversion_tuple(self): cols = ["string_col", "int_col", "list_col", "float_col"] - data = [(u"Text", 111, [123], 1.0)] + data = [("Text", 111, [123], 1.0)] cdf = SupersetDataFrame(data, cols, BaseEngineSpec) self.assertEquals(len(data), cdf.size)
