From 2176ef7c7d716f7ee751a481ba6df9c790f0fa69 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 10 Feb 2021 10:47:04 +0000 Subject: [PATCH 1/4] fix: add config to disable dataset ownership on the old api --- superset/config.py | 4 +++ superset/connectors/sqla/views.py | 48 ++++++++++++++++++++++++++----- superset/views/core.py | 6 ++-- superset/views/datasource.py | 8 ++++-- 4 files changed, 54 insertions(+), 12 deletions(-) diff --git a/superset/config.py b/superset/config.py index e69e7b82d6eb1..241a0e95e63e4 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1057,6 +1057,10 @@ class CeleryConfig: # pylint: disable=too-few-public-methods 'class="alert-link">here.' ) +# Turn this key to False to disable ownership check on the old dataset MVC and +# datasource API /datasource/save. +OLD_API_CHECK_DATASET_OWNERSHIP = True + # SQLA table mutator, every time we fetch the metadata for a certain table # (superset.connectors.sqla.models.SqlaTable), we call this hook # to allow mutating the object with this callback. diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index 903cef2579066..423fc4e649331 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -173,13 +173,25 @@ class TableColumnInlineView( # pylint: disable=too-many-ancestors edit_form_extra_fields = add_form_extra_fields def pre_add(self, item: "models.SqlMetric") -> None: - check_ownership(item.table) + logger.warning( + "This endpoint is deprecated and will be removed in version 2.0.0" + ) + if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: + check_ownership(item.table) def pre_update(self, item: "models.SqlMetric") -> None: - check_ownership(item.table) + logger.warning( + "This endpoint is deprecated and will be removed in version 2.0.0" + ) + if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: + check_ownership(item.table) def pre_delete(self, item: "models.SqlMetric") -> None: - check_ownership(item.table) + logger.warning( + "This endpoint is deprecated and will be removed in version 2.0.0" + ) + if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: + check_ownership(item.table) class SqlMetricInlineView( # pylint: disable=too-many-ancestors @@ -256,13 +268,25 @@ class SqlMetricInlineView( # pylint: disable=too-many-ancestors edit_form_extra_fields = add_form_extra_fields def pre_add(self, item: "models.SqlMetric") -> None: - check_ownership(item.table) + logger.warning( + "This endpoint is deprecated and will be removed in version 2.0.0" + ) + if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: + check_ownership(item.table) def pre_update(self, item: "models.SqlMetric") -> None: - check_ownership(item.table) + logger.warning( + "This endpoint is deprecated and will be removed in version 2.0.0" + ) + if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: + check_ownership(item.table) def pre_delete(self, item: "models.SqlMetric") -> None: - check_ownership(item.table) + logger.warning( + "This endpoint is deprecated and will be removed in version 2.0.0" + ) + if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: + check_ownership(item.table) class RowLevelSecurityListWidget( @@ -476,10 +500,17 @@ class TableModelView( # pylint: disable=too-many-ancestors } def pre_add(self, item: "TableModelView") -> None: + logger.warning( + "This endpoint is deprecated and will be removed in version 2.0.0" + ) validate_sqlatable(item) def pre_update(self, item: "TableModelView") -> None: - check_ownership(item) + logger.warning( + "This endpoint is deprecated and will be removed in version 2.0.0" + ) + if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: + check_ownership(item) def post_add( # pylint: disable=arguments-differ self, @@ -522,6 +553,9 @@ def edit(self, pk: str) -> FlaskResponse: def refresh( # pylint: disable=no-self-use, too-many-branches self, tables: Union["TableModelView", List["TableModelView"]] ) -> FlaskResponse: + logger.warning( + "This endpoint is deprecated and will be removed in version 2.0.0" + ) if not isinstance(tables, list): tables = [tables] diff --git a/superset/views/core.py b/superset/views/core.py index cf0ca7d4dddd5..fbb842b0075d5 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -989,7 +989,7 @@ def schemas( # pylint: disable=no-self-use self, db_id: int, force_refresh: str = "false" ) -> FlaskResponse: logger.warning( - "This API endpoint is deprecated and will be removed in version 1.0.0" + "This API endpoint is deprecated and will be removed in version 2.0.0" ) db_id = int(db_id) database = db.session.query(Database).get(db_id) @@ -1754,7 +1754,7 @@ def publish( # pylint: disable=no-self-use ) -> FlaskResponse: """Gets and toggles published status on dashboards""" logger.warning( - "This API endpoint is deprecated and will be removed in version 1.0.0" + "This API endpoint is deprecated and will be removed in version 2.0.0" ) session = db.session() Role = ab_models.Role @@ -2071,7 +2071,7 @@ def select_star( ) -> FlaskResponse: logging.warning( "%s.select_star " - "This API endpoint is deprecated and will be removed in version 1.0.0", + "This API endpoint is deprecated and will be removed in version 2.0.0", self.__class__.__name__, ) stats_logger.incr(f"{self.__class__.__name__}.select_star.init") diff --git a/superset/views/datasource.py b/superset/views/datasource.py index d4ae9efc1fcf0..fee5a67d5ee41 100644 --- a/superset/views/datasource.py +++ b/superset/views/datasource.py @@ -22,7 +22,7 @@ from flask_appbuilder.security.decorators import has_access_api from flask_babel import _ -from superset import db +from superset import app, db from superset.connectors.connector_registry import ConnectorRegistry from superset.datasets.commands.exceptions import DatasetForbiddenError from superset.exceptions import SupersetException, SupersetSecurityException @@ -53,7 +53,11 @@ def save(self) -> FlaskResponse: ) orm_datasource.database_id = database_id - if "owners" in datasource_dict and orm_datasource.owner_class is not None: + if ( + app.config["OLD_API_CHECK_DATASET_OWNERSHIP"] + and "owners" in datasource_dict + and orm_datasource.owner_class is not None + ): # Check ownership try: check_ownership(orm_datasource) From c4056428a0926a7ff8564504db5760e66eef3dbe Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 10 Feb 2021 11:08:32 +0000 Subject: [PATCH 2/4] fix CI docker build --- .github/workflows/docker_build_push.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/docker_build_push.sh b/.github/workflows/docker_build_push.sh index 1b4fea803ad06..013c7a5ed7d34 100755 --- a/.github/workflows/docker_build_push.sh +++ b/.github/workflows/docker_build_push.sh @@ -21,14 +21,14 @@ SHA=$(git rev-parse HEAD) REPO_NAME="apache/superset" if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then - REFSPEC=$(echo "${GITHUB_HEAD_REF}" | sed 's/[^a-zA-Z0-9]/-/' | head -c 40) + REFSPEC=$(echo "${GITHUB_HEAD_REF}" | sed 's/[^a-zA-Z0-9]/-/g' | head -c 40) PR_NUM=$(echo "${GITHUB_REF}" | sed 's:refs/pull/::' | sed 's:/merge::') LATEST_TAG="pr-${PR_NUM}" elif [[ "${GITHUB_EVENT_NAME}" == "release" ]]; then REFSPEC=$(echo "${GITHUB_REF}" | sed 's:refs/tags/::' | head -c 40) LATEST_TAG="${REFSPEC}" else - REFSPEC=$(echo "${GITHUB_REF}" | sed 's:refs/heads/::' | sed 's/[^a-zA-Z0-9]/-/' | head -c 40) + REFSPEC=$(echo "${GITHUB_REF}" | sed 's:refs/heads/::' | sed 's/[^a-zA-Z0-9]/-/g' | head -c 40) LATEST_TAG="${REFSPEC}" fi From 4e015618755c108e2ba81b01b3a35421ca0446a8 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 11 Feb 2021 08:57:37 +0000 Subject: [PATCH 3/4] fix logic --- superset/views/datasource.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/superset/views/datasource.py b/superset/views/datasource.py index fee5a67d5ee41..be6f73ec9239f 100644 --- a/superset/views/datasource.py +++ b/superset/views/datasource.py @@ -53,16 +53,13 @@ def save(self) -> FlaskResponse: ) orm_datasource.database_id = database_id - if ( - app.config["OLD_API_CHECK_DATASET_OWNERSHIP"] - and "owners" in datasource_dict - and orm_datasource.owner_class is not None - ): + if "owners" in datasource_dict and orm_datasource.owner_class is not None: # Check ownership - try: - check_ownership(orm_datasource) - except SupersetSecurityException: - raise DatasetForbiddenError() + if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]: + try: + check_ownership(orm_datasource) + except SupersetSecurityException: + raise DatasetForbiddenError() datasource_dict["owners"] = ( db.session.query(orm_datasource.owner_class) From 8ecc8075aa686262f901fab187fc06d2b678f7f9 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 11 Feb 2021 09:03:25 +0000 Subject: [PATCH 4/4] add deprecation comment on the config --- superset/config.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/superset/config.py b/superset/config.py index 241a0e95e63e4..028a23789b3b3 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1059,6 +1059,8 @@ class CeleryConfig: # pylint: disable=too-few-public-methods # Turn this key to False to disable ownership check on the old dataset MVC and # datasource API /datasource/save. +# +# Warning: This config key is deprecated and will be removed in version 2.0.0" OLD_API_CHECK_DATASET_OWNERSHIP = True # SQLA table mutator, every time we fetch the metadata for a certain table