pt
/
superset
mirror of https://github.com/apache/superset.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: permission checks on import (#23200)

This commit is contained in:
Beto Dealmeida 2023-03-15 08:31:09 -07:00 committed by GitHub
parent 24c472a4a3
commit ec6318b379
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 334 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
superset/charts/commands/importers/v1/utils.py
View File

@ -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"])

21
superset/commands/importers/v1/examples.py
View File

@ -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

10
superset/dashboards/commands/importers/v1/utils.py
View File

@ -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()

13
superset/databases/commands/importers/v1/utils.py
View File

@ -20,18 +20,27 @@ from typing import Any, Dict
from sqlalchemy.orm import Session
from superset import security_manager
from superset.commands.exceptions import ImportFailedError
from superset.databases.ssh_tunnel.models import SSHTunnel
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")

9
superset/datasets/commands/importers/v1/utils.py
View File

@ -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()

4
superset/examples/utils.py
View File

@ -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()

11
tests/integration_tests/charts/commands_tests.py
View File

@ -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),

12
tests/integration_tests/dashboards/commands_tests.py
View File

@ -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),
@ -567,7 +568,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 = []
@ -579,8 +580,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 = {

136
tests/integration_tests/databases/commands_tests.py
View File

@ -75,10 +75,8 @@ from tests.integration_tests.fixtures.importexport import (
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")
@ -96,10 +94,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({})
@ -401,8 +397,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),
@ -427,7 +426,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.
@ -435,6 +435,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"]}
@ -463,8 +465,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 = {
@ -504,8 +509,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),
@ -524,8 +532,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),
@ -629,7 +640,7 @@ class TestImportDatabasesCommand(SupersetTestCase):
}
}
@mock.patch("superset.databases.schemas.is_feature_enabled")
@patch("superset.databases.schemas.is_feature_enabled")
def test_import_v1_database_masked_ssh_tunnel_password(
self, mock_schema_is_feature_enabled
):
@ -650,7 +661,7 @@ class TestImportDatabasesCommand(SupersetTestCase):
}
}
@mock.patch("superset.databases.schemas.is_feature_enabled")
@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
):
@ -674,11 +685,15 @@ class TestImportDatabasesCommand(SupersetTestCase):
}
}
@mock.patch("superset.databases.schemas.is_feature_enabled")
@patch("superset.databases.schemas.is_feature_enabled")
@patch("superset.security.manager.g")
def test_import_v1_database_with_ssh_tunnel_password(
self, mock_schema_is_feature_enabled
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"
@ -713,11 +728,16 @@ class TestImportDatabasesCommand(SupersetTestCase):
db.session.delete(database)
db.session.commit()
@mock.patch("superset.databases.schemas.is_feature_enabled")
@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_schema_is_feature_enabled
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"
@ -754,7 +774,7 @@ class TestImportDatabasesCommand(SupersetTestCase):
db.session.delete(database)
db.session.commit()
@mock.patch("superset.databases.schemas.is_feature_enabled")
@patch("superset.databases.schemas.is_feature_enabled")
def test_import_v1_database_masked_ssh_tunnel_no_credentials(
self, mock_schema_is_feature_enabled
):
@ -770,7 +790,7 @@ class TestImportDatabasesCommand(SupersetTestCase):
command.run()
assert str(excinfo.value) == "Must provide credentials for the SSH Tunnel"
@mock.patch("superset.databases.schemas.is_feature_enabled")
@patch("superset.databases.schemas.is_feature_enabled")
def test_import_v1_database_masked_ssh_tunnel_multiple_credentials(
self, mock_schema_is_feature_enabled
):
@ -788,7 +808,7 @@ class TestImportDatabasesCommand(SupersetTestCase):
str(excinfo.value) == "Cannot have multiple credentials for the SSH Tunnel"
)
@mock.patch("superset.databases.schemas.is_feature_enabled")
@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
):
@ -839,11 +859,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
):
@ -862,11 +880,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
):
@ -887,11 +903,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
):
@ -911,11 +925,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
):
@ -936,11 +948,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
):
@ -963,9 +973,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.
@ -988,8 +998,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.
@ -1029,7 +1039,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.
@ -1083,7 +1093,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)
@ -1092,9 +1102,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
):
@ -1111,9 +1121,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
):
@ -1130,9 +1140,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
):

17
tests/integration_tests/datasets/commands_tests.py
View File

@ -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),

10
tests/integration_tests/queries/saved_queries/commands_tests.py
View File

@ -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),

45
tests/unit_tests/charts/commands/importers/v1/import_test.py
View File

@ -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"
)

16
tests/unit_tests/commands/importers/v1/assets_test.py
View File

@ -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 = {

46
tests/unit_tests/dashboards/commands/importers/v1/import_test.py
View File

@ -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"
)

44
tests/unit_tests/databases/commands/importers/v1/import_test.py
View File

@ -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"
)

38
tests/unit_tests/datasets/commands/importers/v1/import_test.py
View File

@ -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