diff --git a/docs/docs/installation/installing-superset-from-scratch.mdx b/docs/docs/installation/installing-superset-from-scratch.mdx index 3a12c9db3ac1..5efdb3e8f1f2 100644 --- a/docs/docs/installation/installing-superset-from-scratch.mdx +++ b/docs/docs/installation/installing-superset-from-scratch.mdx @@ -64,7 +64,7 @@ We don't recommend using the system installed Python. Instead, first install the brew install readline pkg-config libffi openssl mysql postgres ``` -You should install a recent version of Python (the official docker image uses 3.8.12). We'd recommend using a Python version manager like [pyenv](https://github.com/pyenv/pyenv) (and also [pyenv-virtualenv](https://github.com/pyenv/pyenv-virtualenv)). +You should install a recent version of Python (the official docker image uses 3.8.13). We'd recommend using a Python version manager like [pyenv](https://github.com/pyenv/pyenv) (and also [pyenv-virtualenv](https://github.com/pyenv/pyenv-virtualenv)). Let's also make sure we have the latest version of `pip` and `setuptools`: diff --git a/superset/databases/commands/create.py b/superset/databases/commands/create.py index 658dbb535e9e..c85f5f1b4782 100644 --- a/superset/databases/commands/create.py +++ b/superset/databases/commands/create.py @@ -63,7 +63,6 @@ def run(self) -> Model: security_manager.add_permission_view_menu( "schema_access", security_manager.get_schema_perm(database, schema) ) - security_manager.add_permission_view_menu("database_access", database.perm) db.session.commit() except DAOCreateFailedError as ex: db.session.rollback() diff --git a/superset/databases/commands/update.py b/superset/databases/commands/update.py index fadc8ba254c2..d0e50bbe2945 100644 --- a/superset/databases/commands/update.py +++ b/superset/databases/commands/update.py @@ -44,10 +44,13 @@ def __init__(self, model_id: int, data: Dict[str, Any]): def run(self) -> Model: self.validate() + if not self._model: + raise DatabaseNotFoundError() + old_database_name = self._model.database_name + try: database = DatabaseDAO.update(self._model, self._properties, commit=False) database.set_sqlalchemy_uri(database.sqlalchemy_uri) - security_manager.add_permission_view_menu("database_access", database.perm) # adding a new database we always want to force refresh schema list # TODO Improve this simplistic implementation for catching DB conn fails try: @@ -55,7 +58,24 @@ def run(self) -> Model: except Exception as ex: db.session.rollback() raise DatabaseConnectionFailedError() from ex + # Update database schema permissions + new_schemas: List[str] = [] for schema in schemas: + old_view_menu_name = security_manager.get_schema_perm( + old_database_name, schema + ) + new_view_menu_name = security_manager.get_schema_perm( + database.database_name, schema + ) + schema_pvm = security_manager.find_permission_view_menu( + "schema_access", old_view_menu_name + ) + # Update the schema permission if the database name changed + if schema_pvm and old_database_name != database.database_name: + schema_pvm.view_menu.name = new_view_menu_name + else: + new_schemas.append(schema) + for schema in new_schemas: security_manager.add_permission_view_menu( "schema_access", security_manager.get_schema_perm(database, schema) ) diff --git a/superset/models/core.py b/superset/models/core.py index 3f5a5cb2f3ec..617c23ef9edb 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -795,7 +795,8 @@ def get_dialect(self) -> Dialect: sqla.event.listen(Database, "after_insert", security_manager.set_perm) -sqla.event.listen(Database, "after_update", security_manager.set_perm) +sqla.event.listen(Database, "after_update", security_manager.database_after_update) +sqla.event.listen(Database, "after_delete", security_manager.database_after_delete) class Log(Model): # pylint: disable=too-few-public-methods diff --git a/superset/security/manager.py b/superset/security/manager.py index dfa6ce1dd212..a2665205baaa 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -56,7 +56,7 @@ from flask_appbuilder.widgets import ListWidget from flask_login import AnonymousUserMixin, LoginManager from jwt.api_jwt import _jwt_global_obj -from sqlalchemy import and_, or_ +from sqlalchemy import and_, inspect, or_ from sqlalchemy.engine.base import Connection from sqlalchemy.orm import Session from sqlalchemy.orm.mapper import Mapper @@ -270,6 +270,14 @@ def get_schema_perm( # pylint: disable=no-self-use return None + @staticmethod + def get_database_perm(database_id: int, database_name: str) -> str: + return f"[{database_name}].(id:{database_id})" + + @staticmethod + def get_dataset_perm(dataset_id: int, dataset_name: str, database_name: str) -> str: + return f"[{database_name}].[{dataset_name}](id:{dataset_id})" + def unpack_database_and_schema( # pylint: disable=no-self-use self, schema_permission: str ) -> DatabaseAndSchema: @@ -933,6 +941,222 @@ def _is_granter_pvm( # pylint: disable=no-self-use return pvm.permission.name in {"can_override_role_permissions", "can_approve"} + def database_after_delete( + self, + mapper: Mapper, + connection: Connection, + target: "Database", + ) -> None: + self._delete_vm_database_access( + mapper, connection, target.id, target.database_name + ) + + def _delete_vm_database_access( + self, + mapper: Mapper, + connection: Connection, + database_id: int, + database_name: str, + ) -> None: + view_menu_table = self.viewmenu_model.__table__ # pylint: disable=no-member + permission_view_menu_table = ( + self.permissionview_model.__table__ # pylint: disable=no-member + ) + view_menu_name = self.get_database_perm(database_id, database_name) + # Clean database access permission + db_pvm = self.find_permission_view_menu("database_access", view_menu_name) + if not db_pvm: + logger.warning( + "Could not find previous database permission %s", + view_menu_name, + ) + return + connection.execute( + permission_view_menu_table.delete().where( + permission_view_menu_table.c.id == db_pvm.id + ) + ) + self.on_permission_after_delete(mapper, connection, db_pvm) + connection.execute( + view_menu_table.delete().where(view_menu_table.c.id == db_pvm.view_menu_id) + ) + + # Clean database schema permissions + schema_pvms = ( + self.get_session.query(self.permissionview_model) + .join(self.permission_model) + .join(self.viewmenu_model) + .filter(self.permission_model.name == "schema_access") + .filter(self.viewmenu_model.name.like(f"[{database_name}].[%]")) + .all() + ) + for schema_pvm in schema_pvms: + connection.execute( + permission_view_menu_table.delete().where( + permission_view_menu_table.c.id == schema_pvm.id + ) + ) + self.on_permission_after_delete(mapper, connection, schema_pvm) + connection.execute( + view_menu_table.delete().where( + view_menu_table.c.id == schema_pvm.view_menu_id + ) + ) + + def _update_vm_database_access( + self, + mapper: Mapper, + connection: Connection, + old_database_name: str, + target: "Database", + ) -> Optional[ViewMenu]: + view_menu_table = self.viewmenu_model.__table__ # pylint: disable=no-member + new_database_name = target.database_name + old_view_menu_name = self.get_database_perm(target.id, old_database_name) + new_view_menu_name = self.get_database_perm(target.id, new_database_name) + db_pvm = self.find_permission_view_menu("database_access", old_view_menu_name) + if not db_pvm: + logger.warning( + "Could not find previous database permission %s", + old_view_menu_name, + ) + return None + new_updated_pvm = self.find_permission_view_menu( + "database_access", new_view_menu_name + ) + if new_updated_pvm: + logger.info( + "New permission [%s] already exists, deleting the previous", + new_view_menu_name, + ) + self._delete_vm_database_access( + mapper, connection, target.id, old_database_name + ) + return None + connection.execute( + view_menu_table.update() + .where(view_menu_table.c.id == db_pvm.view_menu_id) + .values(name=new_view_menu_name) + ) + new_db_view_menu = self.find_view_menu(new_view_menu_name) + + self.on_view_menu_after_update(mapper, connection, new_db_view_menu) + return new_db_view_menu + + def _update_vm_datasources_access( # pylint: disable=too-many-locals + self, + mapper: Mapper, + connection: Connection, + old_database_name: str, + target: "Database", + ) -> List[ViewMenu]: + """ + Updates all datasource access permission when a database name changes + + :param connection: Current connection (called on SQLAlchemy event listener scope) + :param old_database_name: the old database name + :param target: The new database name + :return: A list of changed view menus (permission resource names) + """ + from superset.connectors.sqla.models import ( # pylint: disable=import-outside-toplevel + SqlaTable, + ) + from superset.models.slice import ( # pylint: disable=import-outside-toplevel + Slice, + ) + + view_menu_table = self.viewmenu_model.__table__ # pylint: disable=no-member + sqlatable_table = SqlaTable.__table__ # pylint: disable=no-member + chart_table = Slice.__table__ # pylint: disable=no-member + new_database_name = target.database_name + datasets = ( + self.get_session.query(SqlaTable) + .filter(SqlaTable.database_id == target.id) + .all() + ) + updated_view_menus: List[ViewMenu] = [] + for dataset in datasets: + old_dataset_vm_name = self.get_dataset_perm( + dataset.id, dataset.table_name, old_database_name + ) + new_dataset_vm_name = self.get_dataset_perm( + dataset.id, dataset.table_name, new_database_name + ) + new_dataset_view_menu = self.find_view_menu(new_dataset_vm_name) + if new_dataset_view_menu: + continue + connection.execute( + view_menu_table.update() + .where(view_menu_table.c.name == old_dataset_vm_name) + .values(name=new_dataset_vm_name) + ) + # Update dataset (SqlaTable perm field) + connection.execute( + sqlatable_table.update() + .where( + sqlatable_table.c.id == dataset.id, + sqlatable_table.c.perm == old_dataset_vm_name, + ) + .values(perm=new_dataset_vm_name) + ) + # Update charts (Slice perm field) + connection.execute( + chart_table.update() + .where(chart_table.c.perm == old_dataset_vm_name) + .values(perm=new_dataset_vm_name) + ) + self.on_view_menu_after_update(mapper, connection, new_dataset_view_menu) + updated_view_menus.append(self.find_view_menu(new_dataset_view_menu)) + return updated_view_menus + + def database_after_update( + self, + mapper: Mapper, + connection: Connection, + target: "Database", + ) -> None: + # Check if database name has changed + state = inspect(target) + history = state.get_history("database_name", True) + if not history.has_changes() or not history.deleted: + return + + old_database_name = history.deleted[0] + # update database access permission + self._update_vm_database_access(mapper, connection, old_database_name, target) + # update datasource access + self._update_vm_datasources_access( + mapper, connection, old_database_name, target + ) + + def on_view_menu_after_update( + self, mapper: Mapper, connection: Connection, target: ViewMenu + ) -> None: + """ + Hook that allows for further custom operations when a new ViewMenu + is updated + + Since the update may be performed on after_update event. We cannot + update ViewMenus using a session, so any SQLAlchemy events hooked to + `ViewMenu` will not trigger an after_update. + + :param mapper: The table mapper + :param connection: The DB-API connection + :param target: The mapped instance being persisted + """ + + def on_permission_after_delete( + self, mapper: Mapper, connection: Connection, target: Permission + ) -> None: + """ + Hook that allows for further custom operations when a permission + is deleted by sqlalchemy events. + + :param mapper: The table mapper + :param connection: The DB-API connection + :param target: The mapped instance being persisted + """ + def on_permission_after_insert( self, mapper: Mapper, connection: Connection, target: Permission ) -> None: @@ -1005,6 +1229,7 @@ def set_perm( ) target.perm = target_get_perm + # check schema perm for datasets if ( hasattr(target, "schema_perm") and target.schema_perm != target.get_schema_perm() diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 476cf27aab22..2e91260859e1 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -354,6 +354,186 @@ def test_set_perm_database(self): session.delete(stored_db) session.commit() + def test_after_update_database__perm_database_access(self): + session = db.session + database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://") + session.add(database) + session.commit() + stored_db = ( + session.query(Database).filter_by(database_name="tmp_database").one() + ) + + self.assertIsNotNone( + security_manager.find_permission_view_menu( + "database_access", stored_db.perm + ) + ) + + stored_db.database_name = "tmp_database2" + session.commit() + + # Assert that the old permission was updated + self.assertIsNone( + security_manager.find_permission_view_menu( + "database_access", f"[tmp_database].(id:{stored_db.id})" + ) + ) + # Assert that the db permission was updated + self.assertIsNotNone( + security_manager.find_permission_view_menu( + "database_access", f"[tmp_database2].(id:{stored_db.id})" + ) + ) + session.delete(stored_db) + session.commit() + + def test_after_update_database__perm_database_access_exists(self): + session = db.session + # Add a bogus existing permission before the change + + database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://") + session.add(database) + session.commit() + stored_db = ( + session.query(Database).filter_by(database_name="tmp_database").one() + ) + security_manager.add_permission_view_menu( + "database_access", f"[tmp_database2].(id:{stored_db.id})" + ) + + self.assertIsNotNone( + security_manager.find_permission_view_menu( + "database_access", stored_db.perm + ) + ) + + stored_db.database_name = "tmp_database2" + session.commit() + + # Assert that the old permission was updated + self.assertIsNone( + security_manager.find_permission_view_menu( + "database_access", f"[tmp_database].(id:{stored_db.id})" + ) + ) + # Assert that the db permission was updated + self.assertIsNotNone( + security_manager.find_permission_view_menu( + "database_access", f"[tmp_database2].(id:{stored_db.id})" + ) + ) + session.delete(stored_db) + session.commit() + + def test_after_update_database__perm_datasource_access(self): + session = db.session + database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://") + session.add(database) + session.commit() + + table1 = SqlaTable( + schema="tmp_schema", + table_name="tmp_table1", + database=database, + ) + session.add(table1) + table2 = SqlaTable( + schema="tmp_schema", + table_name="tmp_table2", + database=database, + ) + session.add(table2) + session.commit() + slice1 = Slice( + datasource_id=table1.id, + datasource_type=DatasourceType.TABLE, + datasource_name="tmp_table1", + slice_name="tmp_slice1", + ) + session.add(slice1) + session.commit() + slice1 = session.query(Slice).filter_by(slice_name="tmp_slice1").one() + table1 = session.query(SqlaTable).filter_by(table_name="tmp_table1").one() + table2 = session.query(SqlaTable).filter_by(table_name="tmp_table2").one() + + # assert initial perms + self.assertIsNotNone( + security_manager.find_permission_view_menu( + "datasource_access", f"[tmp_database].[tmp_table1](id:{table1.id})" + ) + ) + self.assertIsNotNone( + security_manager.find_permission_view_menu( + "datasource_access", f"[tmp_database].[tmp_table2](id:{table2.id})" + ) + ) + self.assertEqual(slice1.perm, f"[tmp_database].[tmp_table1](id:{table1.id})") + self.assertEqual(table1.perm, f"[tmp_database].[tmp_table1](id:{table1.id})") + self.assertEqual(table2.perm, f"[tmp_database].[tmp_table2](id:{table2.id})") + + stored_db = ( + session.query(Database).filter_by(database_name="tmp_database").one() + ) + stored_db.database_name = "tmp_database2" + session.commit() + + # Assert that the old permissions were updated + self.assertIsNone( + security_manager.find_permission_view_menu( + "datasource_access", f"[tmp_database].[tmp_table1](id:{table1.id})" + ) + ) + self.assertIsNone( + security_manager.find_permission_view_menu( + "datasource_access", f"[tmp_database].[tmp_table2](id:{table2.id})" + ) + ) + + # Assert that the db permission was updated + self.assertIsNotNone( + security_manager.find_permission_view_menu( + "datasource_access", f"[tmp_database2].[tmp_table1](id:{table1.id})" + ) + ) + self.assertIsNotNone( + security_manager.find_permission_view_menu( + "datasource_access", f"[tmp_database2].[tmp_table2](id:{table2.id})" + ) + ) + self.assertEqual(slice1.perm, f"[tmp_database2].[tmp_table1](id:{table1.id})") + self.assertEqual(table1.perm, f"[tmp_database2].[tmp_table1](id:{table1.id})") + self.assertEqual(table2.perm, f"[tmp_database2].[tmp_table2](id:{table2.id})") + + session.delete(slice1) + session.delete(table1) + session.delete(table2) + session.delete(stored_db) + session.commit() + + def test_after_delete_database__perm_database_access(self): + session = db.session + database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://") + session.add(database) + session.commit() + stored_db = ( + session.query(Database).filter_by(database_name="tmp_database").one() + ) + + self.assertIsNotNone( + security_manager.find_permission_view_menu( + "database_access", stored_db.perm + ) + ) + session.delete(stored_db) + session.commit() + + # Assert that the old permission was updated + self.assertIsNone( + security_manager.find_permission_view_menu( + "database_access", f"[tmp_database].(id:{stored_db.id})" + ) + ) + def test_hybrid_perm_database(self): database = Database(database_name="tmp_database3", sqlalchemy_uri="sqlite://")