diff --git a/superset/charts/commands/importers/v1/utils.py b/superset/charts/commands/importers/v1/utils.py index d4c69d3ada0..cea8af7b4cd 100644 --- a/superset/charts/commands/importers/v1/utils.py +++ b/superset/charts/commands/importers/v1/utils.py @@ -21,17 +21,24 @@ from typing import Any, Dict from flask import g from sqlalchemy.orm import Session +from superset import security_manager +from superset.commands.exceptions import ImportFailedError from superset.models.slice import Slice def import_chart( session: Session, config: Dict[str, Any], overwrite: bool = False ) -> Slice: + can_write = security_manager.can_access("can_write", "Chart") existing = session.query(Slice).filter_by(uuid=config["uuid"]).first() if existing: - if not overwrite: + if not overwrite or not can_write: return existing config["id"] = existing.id + elif not can_write: + raise ImportFailedError( + "Chart doesn't exist and user doesn't have permission to create charts" + ) # TODO (betodealmeida): move this logic to import_from_dict config["params"] = json.dumps(config["params"]) diff --git a/superset/commands/importers/v1/examples.py b/superset/commands/importers/v1/examples.py index 99aa831faaa..8d14c8298a3 100644 --- a/superset/commands/importers/v1/examples.py +++ b/superset/commands/importers/v1/examples.py @@ -21,7 +21,7 @@ from sqlalchemy.orm import Session from sqlalchemy.orm.exc import MultipleResultsFound from sqlalchemy.sql import select -from superset import db +from superset import db, security_manager from superset.charts.commands.importers.v1 import ImportChartsCommand from superset.charts.commands.importers.v1.utils import import_chart from superset.charts.schemas import ImportV1ChartSchema @@ -42,7 +42,7 @@ from superset.datasets.commands.importers.v1 import ImportDatasetsCommand from superset.datasets.commands.importers.v1.utils import import_dataset from superset.datasets.schemas import ImportV1DatasetSchema from superset.models.dashboard import dashboard_slices -from superset.utils.core import get_example_default_schema +from superset.utils.core import get_example_default_schema, override_user from superset.utils.database import get_example_database @@ -69,7 +69,13 @@ class ImportExamplesCommand(ImportModelsCommand): # rollback to prevent partial imports try: - self._import(db.session, self._configs, self.overwrite, self.force_data) + with override_user(security_manager.find_user(username="admin")): + self._import( + db.session, + self._configs, + self.overwrite, + self.force_data, + ) db.session.commit() except Exception as ex: db.session.rollback() @@ -119,13 +125,12 @@ class ImportExamplesCommand(ImportModelsCommand): if config["schema"] is None: config["schema"] = get_example_default_schema() - dataset = import_dataset( - session, config, overwrite=overwrite, force_data=force_data - ) - try: dataset = import_dataset( - session, config, overwrite=overwrite, force_data=force_data + session, + config, + overwrite=overwrite, + force_data=force_data, ) except MultipleResultsFound: # Multiple result can be found for datasets. There was a bug in diff --git a/superset/dashboards/commands/importers/v1/utils.py b/superset/dashboards/commands/importers/v1/utils.py index 513d1efcdb2..30bad57cb14 100644 --- a/superset/dashboards/commands/importers/v1/utils.py +++ b/superset/dashboards/commands/importers/v1/utils.py @@ -22,6 +22,8 @@ from typing import Any, Dict, Set from flask import g from sqlalchemy.orm import Session +from superset import security_manager +from superset.commands.exceptions import ImportFailedError from superset.models.dashboard import Dashboard logger = logging.getLogger(__name__) @@ -146,11 +148,17 @@ def update_id_refs( # pylint: disable=too-many-locals def import_dashboard( session: Session, config: Dict[str, Any], overwrite: bool = False ) -> Dashboard: + can_write = security_manager.can_access("can_write", "Dashboard") existing = session.query(Dashboard).filter_by(uuid=config["uuid"]).first() if existing: - if not overwrite: + if not overwrite or not can_write: return existing config["id"] = existing.id + elif not can_write: + raise ImportFailedError( + "Dashboard doesn't exist and user doesn't " + "have permission to create dashboards" + ) # TODO (betodealmeida): move this logic to import_from_dict config = config.copy() diff --git a/superset/databases/commands/importers/v1/utils.py b/superset/databases/commands/importers/v1/utils.py index 6704ccd4655..673e4f86df5 100644 --- a/superset/databases/commands/importers/v1/utils.py +++ b/superset/databases/commands/importers/v1/utils.py @@ -20,17 +20,26 @@ from typing import Any, Dict from sqlalchemy.orm import Session +from superset import security_manager +from superset.commands.exceptions import ImportFailedError from superset.models.core import Database def import_database( - session: Session, config: Dict[str, Any], overwrite: bool = False + session: Session, + config: Dict[str, Any], + overwrite: bool = False, ) -> Database: + can_write = security_manager.can_access("can_write", "Database") existing = session.query(Database).filter_by(uuid=config["uuid"]).first() if existing: - if not overwrite: + if not overwrite or not can_write: return existing config["id"] = existing.id + elif not can_write: + raise ImportFailedError( + "Database doesn't exist and user doesn't have permission to create databases" + ) # https://github.com/apache/superset/pull/16756 renamed ``csv`` to ``file``. config["allow_file_upload"] = config.pop("allow_csv_upload") diff --git a/superset/datasets/commands/importers/v1/utils.py b/superset/datasets/commands/importers/v1/utils.py index b3fb2a80412..2363cf74976 100644 --- a/superset/datasets/commands/importers/v1/utils.py +++ b/superset/datasets/commands/importers/v1/utils.py @@ -28,6 +28,8 @@ from sqlalchemy.orm import Session from sqlalchemy.orm.exc import MultipleResultsFound from sqlalchemy.sql.visitors import VisitableType +from superset import security_manager +from superset.commands.exceptions import ImportFailedError from superset.connectors.sqla.models import SqlaTable from superset.datasets.commands.exceptions import DatasetForbiddenDataURI from superset.models.core import Database @@ -104,11 +106,16 @@ def import_dataset( overwrite: bool = False, force_data: bool = False, ) -> SqlaTable: + can_write = security_manager.can_access("can_write", "Dataset") existing = session.query(SqlaTable).filter_by(uuid=config["uuid"]).first() if existing: - if not overwrite: + if not overwrite or not can_write: return existing config["id"] = existing.id + elif not can_write: + raise ImportFailedError( + "Dataset doesn't exist and user doesn't have permission to create datasets" + ) # TODO (betodealmeida): move this logic to import_from_dict config = config.copy() diff --git a/superset/examples/utils.py b/superset/examples/utils.py index 4568e4f1964..aea1f0f93d1 100644 --- a/superset/examples/utils.py +++ b/superset/examples/utils.py @@ -92,7 +92,9 @@ def load_configs_from_directory( contents[METADATA_FILE_NAME] = yaml.dump(metadata) command = ImportExamplesCommand( - contents, overwrite=overwrite, force_data=force_data + contents, + overwrite=overwrite, + force_data=force_data, ) try: command.run() diff --git a/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py index da9a7550acb..fb2073499b4 100644 --- a/tests/integration_tests/charts/commands_tests.py +++ b/tests/integration_tests/charts/commands_tests.py @@ -164,9 +164,10 @@ class TestExportChartsCommand(SupersetTestCase): class TestImportChartsCommand(SupersetTestCase): @patch("superset.charts.commands.importers.v1.utils.g") - def test_import_v1_chart(self, mock_g): + @patch("superset.security.manager.g") + def test_import_v1_chart(self, sm_g, utils_g): """Test that we can import a chart""" - mock_g.user = security_manager.find_user("admin") + admin = sm_g.user = utils_g.user = security_manager.find_user("admin") contents = { "metadata.yaml": yaml.safe_dump(chart_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), @@ -227,7 +228,7 @@ class TestImportChartsCommand(SupersetTestCase): assert database.database_name == "imported_database" assert chart.table.database == database - assert chart.owners == [mock_g.user] + assert chart.owners == [admin] chart.owners = [] dataset.owners = [] @@ -237,8 +238,10 @@ class TestImportChartsCommand(SupersetTestCase): db.session.delete(database) db.session.commit() - def test_import_v1_chart_multiple(self): + @patch("superset.security.manager.g") + def test_import_v1_chart_multiple(self, sm_g): """Test that a chart can be imported multiple times""" + sm_g.user = security_manager.find_user("admin") contents = { "metadata.yaml": yaml.safe_dump(chart_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), diff --git a/tests/integration_tests/dashboards/commands_tests.py b/tests/integration_tests/dashboards/commands_tests.py index 927865e4a04..ad9152585e9 100644 --- a/tests/integration_tests/dashboards/commands_tests.py +++ b/tests/integration_tests/dashboards/commands_tests.py @@ -485,9 +485,10 @@ class TestImportDashboardsCommand(SupersetTestCase): db.session.commit() @patch("superset.dashboards.commands.importers.v1.utils.g") - def test_import_v1_dashboard(self, mock_g): + @patch("superset.security.manager.g") + def test_import_v1_dashboard(self, sm_g, utils_g): """Test that we can import a dashboard""" - mock_g.user = security_manager.find_user("admin") + admin = sm_g.user = utils_g.user = security_manager.find_user("admin") contents = { "metadata.yaml": yaml.safe_dump(dashboard_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), @@ -570,7 +571,7 @@ class TestImportDashboardsCommand(SupersetTestCase): database = dataset.database assert str(database.uuid) == database_config["uuid"] - assert dashboard.owners == [mock_g.user] + assert dashboard.owners == [admin] dashboard.owners = [] chart.owners = [] @@ -582,8 +583,11 @@ class TestImportDashboardsCommand(SupersetTestCase): db.session.delete(database) db.session.commit() - def test_import_v1_dashboard_multiple(self): + @patch("superset.security.manager.g") + def test_import_v1_dashboard_multiple(self, mock_g): """Test that a dashboard can be imported multiple times""" + mock_g.user = security_manager.find_user("admin") + num_dashboards = db.session.query(Dashboard).count() contents = { diff --git a/tests/integration_tests/databases/commands_tests.py b/tests/integration_tests/databases/commands_tests.py index 7e4fcaad789..7b2d4fdd270 100644 --- a/tests/integration_tests/databases/commands_tests.py +++ b/tests/integration_tests/databases/commands_tests.py @@ -40,7 +40,7 @@ from superset.databases.commands.importers.v1 import ImportDatabasesCommand from superset.databases.commands.tables import TablesDatabaseCommand from superset.databases.commands.test_connection import TestConnectionDatabaseCommand from superset.databases.commands.validate import ValidateDatabaseParametersCommand -from superset.databases.schemas import DatabaseTestConnectionSchema +from superset.databases.ssh_tunnel.models import SSHTunnel from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.exceptions import ( SupersetErrorsException, @@ -63,16 +63,19 @@ from tests.integration_tests.fixtures.energy_dashboard import ( from tests.integration_tests.fixtures.importexport import ( database_config, database_metadata_config, + database_with_ssh_tunnel_config_mix_credentials, + database_with_ssh_tunnel_config_no_credentials, + database_with_ssh_tunnel_config_password, + database_with_ssh_tunnel_config_private_key, + database_with_ssh_tunnel_config_private_pass_only, dataset_config, dataset_metadata_config, ) class TestCreateDatabaseCommand(SupersetTestCase): - @mock.patch( - "superset.databases.commands.test_connection.event_logger.log_with_context" - ) - @mock.patch("superset.utils.core.g") + @patch("superset.databases.commands.test_connection.event_logger.log_with_context") + @patch("superset.utils.core.g") def test_create_duplicate_error(self, mock_g, mock_logger): example_db = get_example_database() mock_g.user = security_manager.find_user("admin") @@ -90,10 +93,8 @@ class TestCreateDatabaseCommand(SupersetTestCase): "DatabaseRequiredFieldValidationError" ) - @mock.patch( - "superset.databases.commands.test_connection.event_logger.log_with_context" - ) - @mock.patch("superset.utils.core.g") + @patch("superset.databases.commands.test_connection.event_logger.log_with_context") + @patch("superset.utils.core.g") def test_multiple_error_logging(self, mock_g, mock_logger): mock_g.user = security_manager.find_user("admin") command = CreateDatabaseCommand({}) @@ -395,8 +396,11 @@ class TestExportDatabasesCommand(SupersetTestCase): class TestImportDatabasesCommand(SupersetTestCase): - def test_import_v1_database(self): + @patch("superset.security.manager.g") + def test_import_v1_database(self, mock_g): """Test that a database can be imported""" + mock_g.user = security_manager.find_user("admin") + contents = { "metadata.yaml": yaml.safe_dump(database_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), @@ -421,7 +425,8 @@ class TestImportDatabasesCommand(SupersetTestCase): db.session.delete(database) db.session.commit() - def test_import_v1_database_broken_csv_fields(self): + @patch("superset.security.manager.g") + def test_import_v1_database_broken_csv_fields(self, mock_g): """ Test that a database can be imported with broken schema. @@ -429,6 +434,8 @@ class TestImportDatabasesCommand(SupersetTestCase): the V1 schema. This test ensures that we can import databases that were exported with the broken schema. """ + mock_g.user = security_manager.find_user("admin") + broken_config = database_config.copy() broken_config["allow_file_upload"] = broken_config.pop("allow_csv_upload") broken_config["extra"] = {"schemas_allowed_for_file_upload": ["upload"]} @@ -457,8 +464,11 @@ class TestImportDatabasesCommand(SupersetTestCase): db.session.delete(database) db.session.commit() - def test_import_v1_database_multiple(self): + @patch("superset.security.manager.g") + def test_import_v1_database_multiple(self, mock_g): """Test that a database can be imported multiple times""" + mock_g.user = security_manager.find_user("admin") + num_databases = db.session.query(Database).count() contents = { @@ -498,8 +508,11 @@ class TestImportDatabasesCommand(SupersetTestCase): db.session.delete(database) db.session.commit() - def test_import_v1_database_with_dataset(self): + @patch("superset.security.manager.g") + def test_import_v1_database_with_dataset(self, mock_g): """Test that a database can be imported with datasets""" + mock_g.user = security_manager.find_user("admin") + contents = { "databases/imported_database.yaml": yaml.safe_dump(database_config), "datasets/imported_dataset.yaml": yaml.safe_dump(dataset_config), @@ -518,8 +531,11 @@ class TestImportDatabasesCommand(SupersetTestCase): db.session.delete(database) db.session.commit() - def test_import_v1_database_with_dataset_multiple(self): + @patch("superset.security.manager.g") + def test_import_v1_database_with_dataset_multiple(self, mock_g): """Test that a database can be imported multiple times w/o changing datasets""" + mock_g.user = security_manager.find_user("admin") + contents = { "databases/imported_database.yaml": yaml.safe_dump(database_config), "datasets/imported_dataset.yaml": yaml.safe_dump(dataset_config), @@ -623,6 +639,200 @@ class TestImportDatabasesCommand(SupersetTestCase): } } + @patch("superset.databases.schemas.is_feature_enabled") + def test_import_v1_database_masked_ssh_tunnel_password( + self, mock_schema_is_feature_enabled + ): + """Test that database imports with masked ssh_tunnel passwords are rejected""" + mock_schema_is_feature_enabled.return_value = True + masked_database_config = database_with_ssh_tunnel_config_password.copy() + contents = { + "metadata.yaml": yaml.safe_dump(database_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(masked_database_config), + } + command = ImportDatabasesCommand(contents) + with pytest.raises(CommandInvalidError) as excinfo: + command.run() + assert str(excinfo.value) == "Error importing database" + assert excinfo.value.normalized_messages() == { + "databases/imported_database.yaml": { + "_schema": ["Must provide a password for the ssh tunnel"] + } + } + + @patch("superset.databases.schemas.is_feature_enabled") + def test_import_v1_database_masked_ssh_tunnel_private_key_and_password( + self, mock_schema_is_feature_enabled + ): + """Test that database imports with masked ssh_tunnel private_key and private_key_password are rejected""" + mock_schema_is_feature_enabled.return_value = True + masked_database_config = database_with_ssh_tunnel_config_private_key.copy() + contents = { + "metadata.yaml": yaml.safe_dump(database_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(masked_database_config), + } + command = ImportDatabasesCommand(contents) + with pytest.raises(CommandInvalidError) as excinfo: + command.run() + assert str(excinfo.value) == "Error importing database" + assert excinfo.value.normalized_messages() == { + "databases/imported_database.yaml": { + "_schema": [ + "Must provide a private key for the ssh tunnel", + "Must provide a private key password for the ssh tunnel", + ] + } + } + + @patch("superset.databases.schemas.is_feature_enabled") + @patch("superset.security.manager.g") + def test_import_v1_database_with_ssh_tunnel_password( + self, + mock_g, + mock_schema_is_feature_enabled, + ): + """Test that a database with ssh_tunnel password can be imported""" + mock_g.user = security_manager.find_user("admin") + mock_schema_is_feature_enabled.return_value = True + masked_database_config = database_with_ssh_tunnel_config_password.copy() + masked_database_config["ssh_tunnel"]["password"] = "TEST" + contents = { + "metadata.yaml": yaml.safe_dump(database_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(masked_database_config), + } + command = ImportDatabasesCommand(contents) + command.run() + + database = ( + db.session.query(Database).filter_by(uuid=database_config["uuid"]).one() + ) + assert database.allow_file_upload + assert database.allow_ctas + assert database.allow_cvas + assert database.allow_dml + assert not database.allow_run_async + assert database.cache_timeout is None + assert database.database_name == "imported_database" + assert database.expose_in_sqllab + assert database.extra == "{}" + assert database.sqlalchemy_uri == "sqlite:///test.db" + + model_ssh_tunnel = ( + db.session.query(SSHTunnel) + .filter(SSHTunnel.database_id == database.id) + .one() + ) + self.assertEqual(model_ssh_tunnel.password, "TEST") + + db.session.delete(database) + db.session.commit() + + @patch("superset.databases.schemas.is_feature_enabled") + @patch("superset.security.manager.g") + def test_import_v1_database_with_ssh_tunnel_private_key_and_password( + self, + mock_g, + mock_schema_is_feature_enabled, + ): + """Test that a database with ssh_tunnel private_key and private_key_password can be imported""" + mock_g.user = security_manager.find_user("admin") + + mock_schema_is_feature_enabled.return_value = True + masked_database_config = database_with_ssh_tunnel_config_private_key.copy() + masked_database_config["ssh_tunnel"]["private_key"] = "TestPrivateKey" + masked_database_config["ssh_tunnel"]["private_key_password"] = "TEST" + contents = { + "metadata.yaml": yaml.safe_dump(database_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(masked_database_config), + } + command = ImportDatabasesCommand(contents) + command.run() + + database = ( + db.session.query(Database).filter_by(uuid=database_config["uuid"]).one() + ) + assert database.allow_file_upload + assert database.allow_ctas + assert database.allow_cvas + assert database.allow_dml + assert not database.allow_run_async + assert database.cache_timeout is None + assert database.database_name == "imported_database" + assert database.expose_in_sqllab + assert database.extra == "{}" + assert database.sqlalchemy_uri == "sqlite:///test.db" + + model_ssh_tunnel = ( + db.session.query(SSHTunnel) + .filter(SSHTunnel.database_id == database.id) + .one() + ) + self.assertEqual(model_ssh_tunnel.private_key, "TestPrivateKey") + self.assertEqual(model_ssh_tunnel.private_key_password, "TEST") + + db.session.delete(database) + db.session.commit() + + @patch("superset.databases.schemas.is_feature_enabled") + def test_import_v1_database_masked_ssh_tunnel_no_credentials( + self, mock_schema_is_feature_enabled + ): + """Test that databases with ssh_tunnels that have no credentials are rejected""" + mock_schema_is_feature_enabled.return_value = True + masked_database_config = database_with_ssh_tunnel_config_no_credentials.copy() + contents = { + "metadata.yaml": yaml.safe_dump(database_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(masked_database_config), + } + command = ImportDatabasesCommand(contents) + with pytest.raises(CommandInvalidError) as excinfo: + command.run() + assert str(excinfo.value) == "Must provide credentials for the SSH Tunnel" + + @patch("superset.databases.schemas.is_feature_enabled") + def test_import_v1_database_masked_ssh_tunnel_multiple_credentials( + self, mock_schema_is_feature_enabled + ): + """Test that databases with ssh_tunnels that have multiple credentials are rejected""" + mock_schema_is_feature_enabled.return_value = True + masked_database_config = database_with_ssh_tunnel_config_mix_credentials.copy() + contents = { + "metadata.yaml": yaml.safe_dump(database_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(masked_database_config), + } + command = ImportDatabasesCommand(contents) + with pytest.raises(CommandInvalidError) as excinfo: + command.run() + assert ( + str(excinfo.value) == "Cannot have multiple credentials for the SSH Tunnel" + ) + + @patch("superset.databases.schemas.is_feature_enabled") + def test_import_v1_database_masked_ssh_tunnel_only_priv_key_psswd( + self, mock_schema_is_feature_enabled + ): + """Test that databases with ssh_tunnels that have multiple credentials are rejected""" + mock_schema_is_feature_enabled.return_value = True + masked_database_config = ( + database_with_ssh_tunnel_config_private_pass_only.copy() + ) + contents = { + "metadata.yaml": yaml.safe_dump(database_metadata_config), + "databases/imported_database.yaml": yaml.safe_dump(masked_database_config), + } + command = ImportDatabasesCommand(contents) + with pytest.raises(CommandInvalidError) as excinfo: + command.run() + assert str(excinfo.value) == "Error importing database" + assert excinfo.value.normalized_messages() == { + "databases/imported_database.yaml": { + "_schema": [ + "Must provide a private key for the ssh tunnel", + "Must provide a private key password for the ssh tunnel", + ] + } + } + @patch("superset.databases.commands.importers.v1.import_dataset") def test_import_v1_rollback(self, mock_import_dataset): """Test than on an exception everything is rolled back""" @@ -648,11 +858,9 @@ class TestImportDatabasesCommand(SupersetTestCase): class TestTestConnectionDatabaseCommand(SupersetTestCase): - @mock.patch("superset.databases.dao.Database._get_sqla_engine") - @mock.patch( - "superset.databases.commands.test_connection.event_logger.log_with_context" - ) - @mock.patch("superset.utils.core.g") + @patch("superset.databases.dao.Database._get_sqla_engine") + @patch("superset.databases.commands.test_connection.event_logger.log_with_context") + @patch("superset.utils.core.g") def test_connection_db_exception( self, mock_g, mock_event_logger, mock_get_sqla_engine ): @@ -671,11 +879,9 @@ class TestTestConnectionDatabaseCommand(SupersetTestCase): ) mock_event_logger.assert_called() - @mock.patch("superset.databases.dao.Database._get_sqla_engine") - @mock.patch( - "superset.databases.commands.test_connection.event_logger.log_with_context" - ) - @mock.patch("superset.utils.core.g") + @patch("superset.databases.dao.Database._get_sqla_engine") + @patch("superset.databases.commands.test_connection.event_logger.log_with_context") + @patch("superset.utils.core.g") def test_connection_do_ping_exception( self, mock_g, mock_event_logger, mock_get_sqla_engine ): @@ -696,11 +902,9 @@ class TestTestConnectionDatabaseCommand(SupersetTestCase): == SupersetErrorType.GENERIC_DB_ENGINE_ERROR ) - @mock.patch("superset.databases.commands.test_connection.func_timeout") - @mock.patch( - "superset.databases.commands.test_connection.event_logger.log_with_context" - ) - @mock.patch("superset.utils.core.g") + @patch("superset.databases.commands.test_connection.func_timeout") + @patch("superset.databases.commands.test_connection.event_logger.log_with_context") + @patch("superset.utils.core.g") def test_connection_do_ping_timeout( self, mock_g, mock_event_logger, mock_func_timeout ): @@ -720,11 +924,9 @@ class TestTestConnectionDatabaseCommand(SupersetTestCase): == SupersetErrorType.CONNECTION_DATABASE_TIMEOUT ) - @mock.patch("superset.databases.dao.Database._get_sqla_engine") - @mock.patch( - "superset.databases.commands.test_connection.event_logger.log_with_context" - ) - @mock.patch("superset.utils.core.g") + @patch("superset.databases.dao.Database._get_sqla_engine") + @patch("superset.databases.commands.test_connection.event_logger.log_with_context") + @patch("superset.utils.core.g") def test_connection_superset_security_connection( self, mock_g, mock_event_logger, mock_get_sqla_engine ): @@ -745,11 +947,9 @@ class TestTestConnectionDatabaseCommand(SupersetTestCase): mock_event_logger.assert_called() - @mock.patch("superset.databases.dao.Database._get_sqla_engine") - @mock.patch( - "superset.databases.commands.test_connection.event_logger.log_with_context" - ) - @mock.patch("superset.utils.core.g") + @patch("superset.databases.dao.Database._get_sqla_engine") + @patch("superset.databases.commands.test_connection.event_logger.log_with_context") + @patch("superset.utils.core.g") def test_connection_db_api_exc( self, mock_g, mock_event_logger, mock_get_sqla_engine ): @@ -772,9 +972,9 @@ class TestTestConnectionDatabaseCommand(SupersetTestCase): mock_event_logger.assert_called() -@mock.patch("superset.db_engine_specs.base.is_hostname_valid") -@mock.patch("superset.db_engine_specs.base.is_port_open") -@mock.patch("superset.databases.commands.validate.DatabaseDAO") +@patch("superset.db_engine_specs.base.is_hostname_valid") +@patch("superset.db_engine_specs.base.is_port_open") +@patch("superset.databases.commands.validate.DatabaseDAO") def test_validate(DatabaseDAO, is_port_open, is_hostname_valid, app_context): """ Test parameter validation. @@ -797,8 +997,8 @@ def test_validate(DatabaseDAO, is_port_open, is_hostname_valid, app_context): command.run() -@mock.patch("superset.db_engine_specs.base.is_hostname_valid") -@mock.patch("superset.db_engine_specs.base.is_port_open") +@patch("superset.db_engine_specs.base.is_hostname_valid") +@patch("superset.db_engine_specs.base.is_port_open") def test_validate_partial(is_port_open, is_hostname_valid, app_context): """ Test parameter validation when only some parameters are present. @@ -838,7 +1038,7 @@ def test_validate_partial(is_port_open, is_hostname_valid, app_context): ] -@mock.patch("superset.db_engine_specs.base.is_hostname_valid") +@patch("superset.db_engine_specs.base.is_hostname_valid") def test_validate_partial_invalid_hostname(is_hostname_valid, app_context): """ Test parameter validation when only some parameters are present. @@ -892,7 +1092,7 @@ def test_validate_partial_invalid_hostname(is_hostname_valid, app_context): class TestTablesDatabaseCommand(SupersetTestCase): - @mock.patch("superset.databases.dao.DatabaseDAO.find_by_id") + @patch("superset.databases.dao.DatabaseDAO.find_by_id") def test_database_tables_list_with_unknown_database(self, mock_find_by_id): mock_find_by_id.return_value = None command = TablesDatabaseCommand(1, "test", False) @@ -901,9 +1101,9 @@ class TestTablesDatabaseCommand(SupersetTestCase): command.run() assert str(excinfo.value) == ("Database not found.") - @mock.patch("superset.databases.dao.DatabaseDAO.find_by_id") - @mock.patch("superset.security.manager.SupersetSecurityManager.can_access_database") - @mock.patch("superset.utils.core.g") + @patch("superset.databases.dao.DatabaseDAO.find_by_id") + @patch("superset.security.manager.SupersetSecurityManager.can_access_database") + @patch("superset.utils.core.g") def test_database_tables_superset_exception( self, mock_g, mock_can_access_database, mock_find_by_id ): @@ -920,9 +1120,9 @@ class TestTablesDatabaseCommand(SupersetTestCase): command.run() assert str(excinfo.value) == "Test Error" - @mock.patch("superset.databases.dao.DatabaseDAO.find_by_id") - @mock.patch("superset.security.manager.SupersetSecurityManager.can_access_database") - @mock.patch("superset.utils.core.g") + @patch("superset.databases.dao.DatabaseDAO.find_by_id") + @patch("superset.security.manager.SupersetSecurityManager.can_access_database") + @patch("superset.utils.core.g") def test_database_tables_exception( self, mock_g, mock_can_access_database, mock_find_by_id ): @@ -939,9 +1139,9 @@ class TestTablesDatabaseCommand(SupersetTestCase): == "Unexpected error occurred, please check your logs for details" ) - @mock.patch("superset.databases.dao.DatabaseDAO.find_by_id") - @mock.patch("superset.security.manager.SupersetSecurityManager.can_access_database") - @mock.patch("superset.utils.core.g") + @patch("superset.databases.dao.DatabaseDAO.find_by_id") + @patch("superset.security.manager.SupersetSecurityManager.can_access_database") + @patch("superset.utils.core.g") def test_database_tables_list_tables( self, mock_g, mock_can_access_database, mock_find_by_id ): diff --git a/tests/integration_tests/datasets/commands_tests.py b/tests/integration_tests/datasets/commands_tests.py index 0ce98477a0b..6f5b796c6a3 100644 --- a/tests/integration_tests/datasets/commands_tests.py +++ b/tests/integration_tests/datasets/commands_tests.py @@ -327,10 +327,11 @@ class TestImportDatasetsCommand(SupersetTestCase): db.session.commit() @patch("superset.datasets.commands.importers.v1.utils.g") + @patch("superset.security.manager.g") @pytest.mark.usefixtures("load_energy_table_with_slice") - def test_import_v1_dataset(self, mock_g): + def test_import_v1_dataset(self, sm_g, utils_g): """Test that we can import a dataset""" - mock_g.user = security_manager.find_user("admin") + admin = sm_g.user = utils_g.user = security_manager.find_user("admin") contents = { "metadata.yaml": yaml.safe_dump(dataset_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), @@ -360,7 +361,7 @@ class TestImportDatasetsCommand(SupersetTestCase): ) # user should be included as one of the owners - assert dataset.owners == [mock_g.user] + assert dataset.owners == [admin] # database is also imported assert str(dataset.database.uuid) == "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89" @@ -395,8 +396,11 @@ class TestImportDatasetsCommand(SupersetTestCase): db.session.delete(dataset.database) db.session.commit() - def test_import_v1_dataset_multiple(self): + @patch("superset.security.manager.g") + def test_import_v1_dataset_multiple(self, mock_g): """Test that a dataset can be imported multiple times""" + mock_g.user = security_manager.find_user("admin") + contents = { "metadata.yaml": yaml.safe_dump(dataset_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), @@ -483,8 +487,11 @@ class TestImportDatasetsCommand(SupersetTestCase): } } - def test_import_v1_dataset_existing_database(self): + @patch("superset.security.manager.g") + def test_import_v1_dataset_existing_database(self, mock_g): """Test that a dataset can be imported when the database already exists""" + mock_g.user = security_manager.find_user("admin") + # first import database... contents = { "metadata.yaml": yaml.safe_dump(database_metadata_config), diff --git a/tests/integration_tests/fixtures/importexport.py b/tests/integration_tests/fixtures/importexport.py index b624f3e63ce..0f695f044e1 100644 --- a/tests/integration_tests/fixtures/importexport.py +++ b/tests/integration_tests/fixtures/importexport.py @@ -360,6 +360,111 @@ database_config: Dict[str, Any] = { "uuid": "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89", "version": "1.0.0", } +database_with_ssh_tunnel_config_private_key: Dict[str, Any] = { + "allow_csv_upload": True, + "allow_ctas": True, + "allow_cvas": True, + "allow_dml": True, + "allow_run_async": False, + "cache_timeout": None, + "database_name": "imported_database", + "expose_in_sqllab": True, + "extra": {}, + "sqlalchemy_uri": "sqlite:///test.db", + "uuid": "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89", + "ssh_tunnel": { + "server_address": "localhost", + "server_port": 22, + "username": "Test", + "private_key": "XXXXXXXXXX", + "private_key_password": "XXXXXXXXXX", + }, + "version": "1.0.0", +} + +database_with_ssh_tunnel_config_password: Dict[str, Any] = { + "allow_csv_upload": True, + "allow_ctas": True, + "allow_cvas": True, + "allow_dml": True, + "allow_run_async": False, + "cache_timeout": None, + "database_name": "imported_database", + "expose_in_sqllab": True, + "extra": {}, + "sqlalchemy_uri": "sqlite:///test.db", + "uuid": "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89", + "ssh_tunnel": { + "server_address": "localhost", + "server_port": 22, + "username": "Test", + "password": "XXXXXXXXXX", + }, + "version": "1.0.0", +} + +database_with_ssh_tunnel_config_no_credentials: Dict[str, Any] = { + "allow_csv_upload": True, + "allow_ctas": True, + "allow_cvas": True, + "allow_dml": True, + "allow_run_async": False, + "cache_timeout": None, + "database_name": "imported_database", + "expose_in_sqllab": True, + "extra": {}, + "sqlalchemy_uri": "sqlite:///test.db", + "uuid": "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89", + "ssh_tunnel": { + "server_address": "localhost", + "server_port": 22, + "username": "Test", + }, + "version": "1.0.0", +} + +database_with_ssh_tunnel_config_mix_credentials: Dict[str, Any] = { + "allow_csv_upload": True, + "allow_ctas": True, + "allow_cvas": True, + "allow_dml": True, + "allow_run_async": False, + "cache_timeout": None, + "database_name": "imported_database", + "expose_in_sqllab": True, + "extra": {}, + "sqlalchemy_uri": "sqlite:///test.db", + "uuid": "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89", + "ssh_tunnel": { + "server_address": "localhost", + "server_port": 22, + "username": "Test", + "password": "XXXXXXXXXX", + "private_key": "XXXXXXXXXX", + }, + "version": "1.0.0", +} + +database_with_ssh_tunnel_config_private_pass_only: Dict[str, Any] = { + "allow_csv_upload": True, + "allow_ctas": True, + "allow_cvas": True, + "allow_dml": True, + "allow_run_async": False, + "cache_timeout": None, + "database_name": "imported_database", + "expose_in_sqllab": True, + "extra": {}, + "sqlalchemy_uri": "sqlite:///test.db", + "uuid": "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89", + "ssh_tunnel": { + "server_address": "localhost", + "server_port": 22, + "username": "Test", + "private_key_password": "XXXXXXXXXX", + }, + "version": "1.0.0", +} dataset_config: Dict[str, Any] = { "table_name": "imported_dataset", diff --git a/tests/integration_tests/queries/saved_queries/commands_tests.py b/tests/integration_tests/queries/saved_queries/commands_tests.py index bd904191554..5c7b8622098 100644 --- a/tests/integration_tests/queries/saved_queries/commands_tests.py +++ b/tests/integration_tests/queries/saved_queries/commands_tests.py @@ -142,8 +142,11 @@ class TestExportSavedQueriesCommand(SupersetTestCase): class TestImportSavedQueriesCommand(SupersetTestCase): - def test_import_v1_saved_queries(self): + @patch("superset.security.manager.g") + def test_import_v1_saved_queries(self, mock_g): """Test that we can import a saved query""" + mock_g.user = security_manager.find_user("admin") + contents = { "metadata.yaml": yaml.safe_dump(saved_queries_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), @@ -169,8 +172,11 @@ class TestImportSavedQueriesCommand(SupersetTestCase): db.session.delete(database) db.session.commit() - def test_import_v1_saved_queries_multiple(self): + @patch("superset.security.manager.g") + def test_import_v1_saved_queries_multiple(self, mock_g): """Test that a saved query can be imported multiple times""" + mock_g.user = security_manager.find_user("admin") + contents = { "metadata.yaml": yaml.safe_dump(saved_queries_metadata_config), "databases/imported_database.yaml": yaml.safe_dump(database_config), diff --git a/tests/unit_tests/charts/commands/importers/v1/import_test.py b/tests/unit_tests/charts/commands/importers/v1/import_test.py index e29fd70fb8a..06e0063fe93 100644 --- a/tests/unit_tests/charts/commands/importers/v1/import_test.py +++ b/tests/unit_tests/charts/commands/importers/v1/import_test.py @@ -18,19 +18,26 @@ import copy +import pytest +from pytest_mock import MockFixture from sqlalchemy.orm.session import Session +from superset.commands.exceptions import ImportFailedError -def test_import_chart(session: Session) -> None: + +def test_import_chart(mocker: MockFixture, session: Session) -> None: """ Test importing a chart. """ + from superset import security_manager from superset.charts.commands.importers.v1.utils import import_chart from superset.connectors.sqla.models import SqlaTable from superset.models.core import Database from superset.models.slice import Slice from tests.integration_tests.fixtures.importexport import chart_config + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() Slice.metadata.create_all(engine) # pylint: disable=no-member @@ -45,16 +52,19 @@ def test_import_chart(session: Session) -> None: assert chart.external_url is None -def test_import_chart_managed_externally(session: Session) -> None: +def test_import_chart_managed_externally(mocker: MockFixture, session: Session) -> None: """ Test importing a chart that is managed externally. """ + from superset import security_manager from superset.charts.commands.importers.v1.utils import import_chart from superset.connectors.sqla.models import SqlaTable from superset.models.core import Database from superset.models.slice import Slice from tests.integration_tests.fixtures.importexport import chart_config + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() Slice.metadata.create_all(engine) # pylint: disable=no-member @@ -67,3 +77,34 @@ def test_import_chart_managed_externally(session: Session) -> None: chart = import_chart(session, config) assert chart.is_managed_externally is True assert chart.external_url == "https://example.org/my_chart" + + +def test_import_chart_without_permission( + mocker: MockFixture, + session: Session, +) -> None: + """ + Test importing a chart when a user doesn't have permissions to create. + """ + from superset import security_manager + from superset.charts.commands.importers.v1.utils import import_chart + from superset.connectors.sqla.models import SqlaTable + from superset.models.core import Database + from superset.models.slice import Slice + from tests.integration_tests.fixtures.importexport import chart_config + + mocker.patch.object(security_manager, "can_access", return_value=False) + + engine = session.get_bind() + Slice.metadata.create_all(engine) # pylint: disable=no-member + + config = copy.deepcopy(chart_config) + config["datasource_id"] = 1 + config["datasource_type"] = "table" + + with pytest.raises(ImportFailedError) as excinfo: + import_chart(session, config) + assert ( + str(excinfo.value) + == "Chart doesn't exist and user doesn't have permission to create charts" + ) diff --git a/tests/unit_tests/commands/importers/v1/assets_test.py b/tests/unit_tests/commands/importers/v1/assets_test.py index 1a345ff2b91..d48eed1be7c 100644 --- a/tests/unit_tests/commands/importers/v1/assets_test.py +++ b/tests/unit_tests/commands/importers/v1/assets_test.py @@ -17,6 +17,7 @@ import copy +from pytest_mock import MockFixture from sqlalchemy.orm.session import Session from sqlalchemy.sql import select @@ -30,14 +31,17 @@ from tests.unit_tests.fixtures.assets_configs import ( ) -def test_import_new_assets(session: Session) -> None: +def test_import_new_assets(mocker: MockFixture, session: Session) -> None: """ Test that all new assets are imported correctly. """ + from superset import security_manager from superset.commands.importers.v1.assets import ImportAssetsCommand from superset.models.dashboard import dashboard_slices from superset.models.slice import Slice + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() Slice.metadata.create_all(engine) # pylint: disable=no-member configs = { @@ -59,14 +63,17 @@ def test_import_new_assets(session: Session) -> None: assert len(dashboard_ids) == expected_number_of_dashboards -def test_import_adds_dashboard_charts(session: Session) -> None: +def test_import_adds_dashboard_charts(mocker: MockFixture, session: Session) -> None: """ Test that existing dashboards are updated with new charts. """ + from superset import security_manager from superset.commands.importers.v1.assets import ImportAssetsCommand from superset.models.dashboard import dashboard_slices from superset.models.slice import Slice + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() Slice.metadata.create_all(engine) # pylint: disable=no-member base_configs = { @@ -95,14 +102,17 @@ def test_import_adds_dashboard_charts(session: Session) -> None: assert len(dashboard_ids) == expected_number_of_dashboards -def test_import_removes_dashboard_charts(session: Session) -> None: +def test_import_removes_dashboard_charts(mocker: MockFixture, session: Session) -> None: """ Test that existing dashboards are updated without old charts. """ + from superset import security_manager from superset.commands.importers.v1.assets import ImportAssetsCommand from superset.models.dashboard import dashboard_slices from superset.models.slice import Slice + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() Slice.metadata.create_all(engine) # pylint: disable=no-member base_configs = { diff --git a/tests/unit_tests/dashboards/commands/importers/v1/import_test.py b/tests/unit_tests/dashboards/commands/importers/v1/import_test.py index 08f681d916b..e07a23f6bf8 100644 --- a/tests/unit_tests/dashboards/commands/importers/v1/import_test.py +++ b/tests/unit_tests/dashboards/commands/importers/v1/import_test.py @@ -18,19 +18,26 @@ import copy +import pytest +from pytest_mock import MockFixture from sqlalchemy.orm.session import Session +from superset.commands.exceptions import ImportFailedError -def test_import_dashboard(session: Session) -> None: + +def test_import_dashboard(mocker: MockFixture, session: Session) -> None: """ Test importing a dashboard. """ + from superset import security_manager from superset.connectors.sqla.models import SqlaTable from superset.dashboards.commands.importers.v1.utils import import_dashboard from superset.models.core import Database from superset.models.slice import Slice from tests.integration_tests.fixtures.importexport import dashboard_config + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() Slice.metadata.create_all(engine) # pylint: disable=no-member @@ -43,16 +50,22 @@ def test_import_dashboard(session: Session) -> None: assert dashboard.external_url is None -def test_import_dashboard_managed_externally(session: Session) -> None: +def test_import_dashboard_managed_externally( + mocker: MockFixture, + session: Session, +) -> None: """ Test importing a dashboard that is managed externally. """ + from superset import security_manager from superset.connectors.sqla.models import SqlaTable from superset.dashboards.commands.importers.v1.utils import import_dashboard from superset.models.core import Database from superset.models.slice import Slice from tests.integration_tests.fixtures.importexport import dashboard_config + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() Slice.metadata.create_all(engine) # pylint: disable=no-member @@ -63,3 +76,32 @@ def test_import_dashboard_managed_externally(session: Session) -> None: dashboard = import_dashboard(session, config) assert dashboard.is_managed_externally is True assert dashboard.external_url == "https://example.org/my_dashboard" + + +def test_import_dashboard_without_permission( + mocker: MockFixture, + session: Session, +) -> None: + """ + Test importing a dashboard when a user doesn't have permissions to create. + """ + from superset import security_manager + from superset.connectors.sqla.models import SqlaTable + from superset.dashboards.commands.importers.v1.utils import import_dashboard + from superset.models.core import Database + from superset.models.slice import Slice + from tests.integration_tests.fixtures.importexport import dashboard_config + + mocker.patch.object(security_manager, "can_access", return_value=False) + + engine = session.get_bind() + Slice.metadata.create_all(engine) # pylint: disable=no-member + + config = copy.deepcopy(dashboard_config) + + with pytest.raises(ImportFailedError) as excinfo: + import_dashboard(session, config) + assert ( + str(excinfo.value) + == "Dashboard doesn't exist and user doesn't have permission to create dashboards" + ) diff --git a/tests/unit_tests/databases/commands/importers/v1/import_test.py b/tests/unit_tests/databases/commands/importers/v1/import_test.py index e665bcb505d..f9d2695f260 100644 --- a/tests/unit_tests/databases/commands/importers/v1/import_test.py +++ b/tests/unit_tests/databases/commands/importers/v1/import_test.py @@ -18,17 +18,24 @@ import copy +import pytest +from pytest_mock import MockFixture from sqlalchemy.orm.session import Session +from superset.commands.exceptions import ImportFailedError -def test_import_database(session: Session) -> None: + +def test_import_database(mocker: MockFixture, session: Session) -> None: """ Test importing a database. """ + from superset import security_manager from superset.databases.commands.importers.v1.utils import import_database from superset.models.core import Database from tests.integration_tests.fixtures.importexport import database_config + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() Database.metadata.create_all(engine) # pylint: disable=no-member @@ -58,14 +65,20 @@ def test_import_database(session: Session) -> None: assert database.allow_dml is False -def test_import_database_managed_externally(session: Session) -> None: +def test_import_database_managed_externally( + mocker: MockFixture, + session: Session, +) -> None: """ Test importing a database that is managed externally. """ + from superset import security_manager from superset.databases.commands.importers.v1.utils import import_database from superset.models.core import Database from tests.integration_tests.fixtures.importexport import database_config + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() Database.metadata.create_all(engine) # pylint: disable=no-member @@ -76,3 +89,30 @@ def test_import_database_managed_externally(session: Session) -> None: database = import_database(session, config) assert database.is_managed_externally is True assert database.external_url == "https://example.org/my_database" + + +def test_import_database_without_permission( + mocker: MockFixture, + session: Session, +) -> None: + """ + Test importing a database when a user doesn't have permissions to create. + """ + from superset import security_manager + from superset.databases.commands.importers.v1.utils import import_database + from superset.models.core import Database + from tests.integration_tests.fixtures.importexport import database_config + + mocker.patch.object(security_manager, "can_access", return_value=False) + + engine = session.get_bind() + Database.metadata.create_all(engine) # pylint: disable=no-member + + config = copy.deepcopy(database_config) + + with pytest.raises(ImportFailedError) as excinfo: + import_database(session, config) + assert ( + str(excinfo.value) + == "Database doesn't exist and user doesn't have permission to create databases" + ) diff --git a/tests/unit_tests/datasets/commands/importers/v1/import_test.py b/tests/unit_tests/datasets/commands/importers/v1/import_test.py index 5b52ac7f1da..839374425b4 100644 --- a/tests/unit_tests/datasets/commands/importers/v1/import_test.py +++ b/tests/unit_tests/datasets/commands/importers/v1/import_test.py @@ -25,20 +25,27 @@ from unittest.mock import Mock, patch import pytest from flask import current_app +from pytest_mock import MockFixture from sqlalchemy.orm.session import Session -from superset.datasets.commands.exceptions import DatasetForbiddenDataURI +from superset.datasets.commands.exceptions import ( + DatasetForbiddenDataURI, + ImportFailedError, +) from superset.datasets.commands.importers.v1.utils import validate_data_uri -def test_import_dataset(session: Session) -> None: +def test_import_dataset(mocker: MockFixture, session: Session) -> None: """ Test importing a dataset. """ + from superset import security_manager from superset.connectors.sqla.models import SqlaTable from superset.datasets.commands.importers.v1.utils import import_dataset from superset.models.core import Database + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() SqlaTable.metadata.create_all(engine) # pylint: disable=no-member @@ -143,15 +150,18 @@ def test_import_dataset(session: Session) -> None: assert sqla_table.database.id == database.id -def test_import_dataset_duplicate_column(session: Session) -> None: +def test_import_dataset_duplicate_column(mocker: MockFixture, session: Session) -> None: """ Test importing a dataset with a column that already exists. """ + from superset import security_manager from superset.columns.models import Column as NewColumn from superset.connectors.sqla.models import SqlaTable, TableColumn from superset.datasets.commands.importers.v1.utils import import_dataset from superset.models.core import Database + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() SqlaTable.metadata.create_all(engine) # pylint: disable=no-member @@ -266,15 +276,18 @@ def test_import_dataset_duplicate_column(session: Session) -> None: assert sqla_table.database.id == database.id -def test_import_column_extra_is_string(session: Session) -> None: +def test_import_column_extra_is_string(mocker: MockFixture, session: Session) -> None: """ Test importing a dataset when the column extra is a string. """ + from superset import security_manager from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn from superset.datasets.commands.importers.v1.utils import import_dataset from superset.datasets.schemas import ImportV1DatasetSchema from superset.models.core import Database + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() SqlaTable.metadata.create_all(engine) # pylint: disable=no-member @@ -347,12 +360,17 @@ def test_import_column_extra_is_string(session: Session) -> None: @patch("superset.datasets.commands.importers.v1.utils.request") -def test_import_column_allowed_data_url(request: Mock, session: Session) -> None: +def test_import_column_allowed_data_url( + request: Mock, + mocker: MockFixture, + session: Session, +) -> None: """ Test importing a dataset when using data key to fetch data from a URL. """ import io + from superset import security_manager from superset.connectors.sqla.models import SqlaTable from superset.datasets.commands.importers.v1.utils import import_dataset from superset.datasets.schemas import ImportV1DatasetSchema @@ -360,6 +378,8 @@ def test_import_column_allowed_data_url(request: Mock, session: Session) -> None request.urlopen.return_value = io.StringIO("col1\nvalue1\nvalue2\n") + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() SqlaTable.metadata.create_all(engine) # pylint: disable=no-member @@ -419,15 +439,21 @@ def test_import_column_allowed_data_url(request: Mock, session: Session) -> None ).fetchall() -def test_import_dataset_managed_externally(session: Session) -> None: +def test_import_dataset_managed_externally( + mocker: MockFixture, + session: Session, +) -> None: """ Test importing a dataset that is managed externally. """ + from superset import security_manager from superset.connectors.sqla.models import SqlaTable from superset.datasets.commands.importers.v1.utils import import_dataset from superset.models.core import Database from tests.integration_tests.fixtures.importexport import dataset_config + mocker.patch.object(security_manager, "can_access", return_value=True) + engine = session.get_bind() SqlaTable.metadata.create_all(engine) # pylint: disable=no-member